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: <ZPkDaLo4ubFRpPg3@boxer>
Date:   Thu, 7 Sep 2023 00:55:36 +0200
From:   Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To:     Vinicius Costa Gomes <vinicius.gomes@...el.com>
CC:     <intel-wired-lan@...ts.osuosl.org>, <sasha.neftin@...el.com>,
        Ferenc Fejes <ferenc.fejes@...csson.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Tony Nguyen <anthony.l.nguyen@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Jithu Joseph <jithu.joseph@...el.com>,
        Vedang Patel <vedang.patel@...el.com>,
        Andre Guedes <andre.guedes@...el.com>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH iwl-net v1] igc: Fix infinite initialization loop with
 early XDP redirect

On Tue, Sep 05, 2023 at 02:37:52PM -0700, Vinicius Costa Gomes wrote:
> When a XDP redirect happens before the link is ready, that

When exactly link was 'ready' in your setup? You said it was enough to
launch traffic towards igc iface before running xdp-bench. Was the iface
down or up or?

> transmission will not finish and will timeout, causing an adapter
> reset. If the redirects do not stop, the adapter will not stop
> resetting.

Please highlight that this driver shares tx resources with netstack. I
believe the source of this bug is that the watchdog is responsible to call
netif_carrier_on() from a workqueue which happens to be scheduled *after*
clearing __IGC_DOWN in igc_up().

> 
> Wait for the driver to signal that there's a carrier before allowing
> transmissions to proceed.
> 
> Fixes: 4ff320361092 ("igc: Add support for XDP_REDIRECT action")
> Reported-by: Ferenc Fejes <ferenc.fejes@...csson.com>
> Closes: https://lore.kernel.org/netdev/0caf33cf6adb3a5bf137eeaa20e89b167c9986d5.camel@ericsson.com/
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@...el.com>
> Tested-by: Ferenc Fejes <ferenc.fejes@...csson.com>
> ---
>  drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 293b45717683..98de34d0ce07 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6491,7 +6491,7 @@ static int igc_xdp_xmit(struct net_device *dev, int num_frames,
>  	struct igc_ring *ring;
>  	int i, drops;
>  
> -	if (unlikely(test_bit(__IGC_DOWN, &adapter->state)))
> +	if (unlikely(!netif_carrier_ok(dev)))
>  		return -ENETDOWN;

I thought about keeping the bit check as well but given what i wrote above
it is probably redundant, so:

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>

>  
>  	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
> -- 
> 2.41.0
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ