RE: [PATCH net v6 3/4] net: ethernet: oa_tc6: Disable tx queues on fatal error
Selvamani Rajagopal <[email protected]>
| Newsgroups | org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <DM5PR02MB33692A7DA56968540A3F950383A62@DM5PR02MB3369.namprd02.prod.outlook.com> |
> -----Original Message----- > From: Jakub Kicinski <[email protected]> > Sent: Tuesday, August 18, 2026 9:07 AM > Subject: Re: [PATCH net v6 3/4] net: ethernet: oa_tc6: Disable tx queues on fatal error > > On Wed, 12 Aug 2026 20:00:11 -0700 Selvamani Rajagopal via B4 Relay > wrote: > > + /* disable_traffic, when set, is a point of no return to working > > + * state. TX queues are disabled. In some cases, upper layer or > > + * vendor code may inadvertently enable the queue. Intention of > > + * the disable_traffic flag is to stop traffic from flowing. > > + */ > > I don't get this comment and why this isn't part of patch 1. > Patch 1 is the place where the stop is removed: Thanks for the review. I could have made part of patch 1. Since this changes the behavior of aa_tc6_disable_traffic, I thought I should have it separately so that it is clear for the reviewer. > > - if (tc6->disable_traffic || tc6->waiting_tx_skb) { > - netif_stop_queue(tc6->netdev); > - return NETDEV_TX_BUSY; > - } > > Also the "vendor code may inadvertently enable the queue" > is some vague indication of a race? I have no idea what "vendor code" > means here, this "library" has one user. If there is a race please fix > it. If there's not - please don't add confusing comments. It is not a race. If vendor's code (or even from command line) marks the interface "down" followed by "up", queue may be re-enabled. This use case was raised by AI review during the previous submission, which is a valid question. So I thought I should capture this in the comment.