netdev - Re: [Intel-wired-lan] [PATCH v3 iwl-net 1/8] ice: respect netif readiness in AF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZmsVfbDTyZty2/Xh@boxer>
Date: Thu, 13 Jun 2024 17:51:25 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <larysa.zaremba@...el.com>,
	<netdev@...r.kernel.org>, <michal.kubiak@...el.com>,
	<anthony.l.nguyen@...el.com>, <jacob.e.keller@...el.com>, Chandan Kumar Rout
	<chandanx.rout@...el.com>, <magnus.karlsson@...el.com>, Shannon Nelson
	<shannon.nelson@....com>
Subject: Re: [Intel-wired-lan] [PATCH v3 iwl-net 1/8] ice: respect netif
 readiness in AF_XDP ZC related ndo's

On Wed, Jun 12, 2024 at 11:15:31AM +0200, Alexander Lobakin wrote:
> From: Alexander Lobakin <aleksander.lobakin@...el.com>
> Date: Wed, 12 Jun 2024 11:09:10 +0200
> 
> > From: Maciej Fijalkowski <maciej.fijalkowski@...el.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?

> 
> [0] https://elixir.bootlin.com/linux/v6.10-rc3/source/net/core/dev.c#L1466
> 
> Thanks,
> Olek