On Thu, Jun 13, 2024 at 05:51:25PM +0200, Maciej Fijalkowski wrote: > On Wed, Jun 12, 2024 at 11:15:31AM +0200, Alexander Lobakin wrote: > > From: Alexander Lobakin <aleksander.loba...@intel.com> > > Date: Wed, 12 Jun 2024 11:09:10 +0200 > > > > > From: Maciej Fijalkowski <maciej.fijalkow...@intel.com> > > > Date: Tue, 11 Jun 2024 16:21:27 +0200 > > > > [...] > > > > >>>> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c > > >>>> b/drivers/net/ethernet/intel/ice/ice_xsk.c > > >>>> index 2015f66b0cf9..1bd4b054dd80 100644 > > >>>> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c > > >>>> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c > > >>>> @@ -1048,6 +1048,10 @@ bool ice_xmit_zc(struct ice_tx_ring *xdp_ring) > > >>>> > > >>>> ice_clean_xdp_irq_zc(xdp_ring); > > >>>> > > >>>> + if (!netif_carrier_ok(xdp_ring->vsi->netdev) || > > >>>> + !netif_running(xdp_ring->vsi->netdev)) > > > > Oh BTW, I noticed some time ago that netif_running() is less precise > > than checking for %IFF_UP. > > For example, in this piece (main netdev ifup function)[0], > > netif_running() will start returning true *before* driver's .ndo_open() > > is called, but %IFF_UP will be set only after .ndo_open() is done (with > > no issues). > > I see, thanks for bringing this up! I'd like to try this out. Tony sorry > for the noise, but it seems I'll go with v4 and will decorate the > mentioned statements with unlikely(). > > > That means, I'd check for %IFF_UP honestly (maybe even before checking > > the carrier). > > I wonder whether it is the ultimate check and two existing ones (that we > are adding in this patch) could be dropped?
In netdev closing path [1], __LINK_STATE_START is cleared before IFF_UP. What we were initially experiencing when netif_running() check wasn't in place was that xsk was producing a bunch of Tx descs when device was being brought down. So let me keep what we have here and add IFF_UP check on top. Better be safe than sorry as the bug we were dealing with was pretty nasty. > > > > > [0] https://elixir.bootlin.com/linux/v6.10-rc3/source/net/core/dev.c#L1466 [1]: https://elixir.bootlin.com/linux/v6.10-rc3/source/net/core/dev.c#L1532 > > > > Thanks, > > Olek
