From: Maciej Fijalkowski <[email protected]> Date: Thu, 13 Jun 2024 18:07:53 +0200
> 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 <[email protected]> >>> Date: Wed, 12 Jun 2024 11:09:10 +0200 >>> >>>> From: Maciej Fijalkowski <[email protected]> >>>> 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. Oh man, inconsistency in its best :D > 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. Sure, I didn't know %IFF_UP's not that reliable :s Thanks, Olek
