netdev - RE: [External] ixgbe driver link down causes 100% load in ksoftirqd/x

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]

Date:   Fri, 28 Jan 2022 15:53:25 +0000
From:   "Maurice Baijens (Ellips B.V.)" <maurice.baijens@...ips.com>
To:     Maciej Fijalkowski <maciej.fijalkowski@...el.com>
CC:     "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE:  [External] ixgbe driver link down causes 100% load in
 ksoftirqd/x

Hello,


> -----Original Message-----
> From: Maciej Fijalkowski <maciej.fijalkowski@...el.com> 
> Sent: Friday, January 28, 2022 4:31 PM
> To: Maurice Baijens (Ellips B.V.) <maurice.baijens@...ips.com>
> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org
> Subject: Re: [External] ixgbe driver link down causes 100% load in ksoftirqd/x
>
> On Thu, Jan 20, 2022 at 09:23:06AM +0000, Maurice Baijens (Ellips B.V.) wrote:
> > Hello,
> > 
> > 
> > I have an issue with the ixgbe driver and X550Tx network adapter.
> > When I disconnect the network cable I end up with 100% load in ksoftirqd/x. I am running the adapter in
> > xdp mode (XDP_FLAGS_DRV_MODE). Problem seen in linux kernel 5.15.x and also 5.16.0+ (head).
>
> Hello,
>
> a stupid question - why do you disconnect the cable when running traffic? :)

The answer is even more stupid. Due to supply problems we sometimes have to use
dual adapters instead of single once, and if one by accident enables the wrong port,
the bug is triggered.

> If you plug this back in then what happens?

Then everything works normal again.

>
> > 
> > I traced the problem down to function ixgbe_xmit_zc in ixgbe_xsk.c:
> > 
> > if (unlikely(!ixgbe_desc_unused(xdp_ring)) ||
> >     !netif_carrier_ok(xdp_ring->netdev)) {
> >             work_done = false;
> >             break;
> > }
>
> This was done in commit c685c69fba71 ("ixgbe: don't do any AF_XDP
> zero-copy transmit if netif is not OK") - it was addressing the transient
> state when configuring the xsk pool on particular queue pair.
>
> > 
> > This function is called from ixgbe_poll() function via ixgbe_clean_xdp_tx_irq(). It sets
> > work_done to false if netif_carrier_ok() returns false (so if link is down). Because work_done
> > is always false, ixgbe_poll keeps on polling forever.
> > 
> > I made a fix by checking link in ixgbe_poll() function and if no link exiting polling mode:
> > 
> > /* If all work not completed, return budget and keep polling */
> > if ((!clean_complete) && netif_carrier_ok(adapter->netdev))
> >             return budget;
>
> Not sure about the correctness of this. Question is how should we act for
> link down - should we say that we are done with processing or should we
> wait until the link gets back?
>
> Instead of setting the work_done to false immediately for
>!netif_carrier_ok(), I'd rather break out the checks that are currently
> combined into the single statement, something like this:
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> index b3fd8e5cd85b..6a5e9cf6b5da 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> @@ -390,12 +390,14 @@ static bool ixgbe_xmit_zc(struct ixgbe_ring *xdp_ring, unsigned int budget)
>  	u32 cmd_type;
>  
>  	while (budget-- > 0) {
> -		if (unlikely(!ixgbe_desc_unused(xdp_ring)) ||
> -		    !netif_carrier_ok(xdp_ring->netdev)) {
> +		if (unlikely(!ixgbe_desc_unused(xdp_ring))) {
>  			work_done = false;
>  			break;
>  		}
>  
> +		if (!netif_carrier_ok(xdp_ring->netdev))
> +			break;
> +
>  		if (!xsk_tx_peek_desc(pool, &desc))
>  			break;
>
>
> > 
> > This is probably fine for our application as we only run in xdpdrv mode, however I am not sure this
>
> By xdpdrv I would understand that you're running XDP in standard native
> mode, however you refer to the AF_XDP Zero Copy implementation in the
> driver. But I don't think it changes anything in this thread.
>
> In the end I see some outstanding issues with ixgbe_xmit_zc(), so this
> probably might need some attention.
>
> Thanks!
> Maciej

Your suggestion for a fix sounds ok. (I have not tested it). Is someone going to fix it in the next version of the kernel,
so we don't have to apply a patch here forever? Or how should we proceed to get it fixed in the kernel?

Thank you,
Maurice


>
> > is the correct way to fix this issue and the behaviour of the normal skb mode operation is 
> > also affected by my fix.
> > 
> > So hopefully my observations are correct and someone here can fix the issue and push it upstream.
> > 
> > 
> > Best regards,
> > 	Maurice Baijens