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] [day] [month] [year] [list]
Message-ID: <346b9fec-f0a4-70d0-529c-32659490df03@csgroup.eu>
Date:   Tue, 12 Apr 2022 12:44:04 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Mans Rullgard <mans@...sr.com>,
        Pantelis Antoniou <pantelis.antoniou@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Vitaly Bordug <vbordug@...mvista.com>,
        Dan Malek <dan@...eddededge.com>,
        Joakim Tjernlund <joakim.tjernlund@...entis.se>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH] net: fs_enet: fix tx error handling



Le 17/03/2022 à 16:38, Mans Rullgard a écrit :
> In some cases, the TXE flag is apparently set without any error
> indication in the buffer descriptor status. When this happens, tx
> stalls until the tx_restart() function is called via the device
> watchdog which can take a long time.

Is there an errata from NXP about this ?

Did you report the issue to them ? What feedback did you get ?

> 
> To fix this, check for TXE in the napi poll function and trigger a
> tx_restart() call as for errors reported in the buffer descriptor.

I'm not sure to understand. You change seems to do a lot more than that. 
Especially it changes to location of the handling of errors. Previously 
errors where handled in interrupt routine. Now it's handled in the napi 
poll routine. You have to explain all that.

> 
> This change makes the FCC based Ethernet controller on MPC82xx devices
> usable. It probably breaks the other modes (FEC, SCC) which I have no
> way of testing.

You should at least change mac-scc.h and mac-fec.h to match the changes 
you did in mac-fcc.h

This would allow me to at least test the FEC one.

> 
> Signed-off-by: Mans Rullgard <mans@...sr.com>
> ---
>   .../ethernet/freescale/fs_enet/fs_enet-main.c | 47 +++++++------------
>   .../net/ethernet/freescale/fs_enet/mac-fcc.c  |  2 +-
>   2 files changed, 19 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> index 78e008b81374..4276becd07cf 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> @@ -94,14 +94,22 @@ static int fs_enet_napi(struct napi_struct *napi, int budget)
>   	int curidx;
>   	int dirtyidx, do_wake, do_restart;
>   	int tx_left = TX_RING_SIZE;
> +	u32 int_events;
>   
>   	spin_lock(&fep->tx_lock);
>   	bdp = fep->dirty_tx;
> +	do_wake = do_restart = 0;
> +
> +	int_events = (*fep->ops->get_int_events)(dev);
> +
> +	if (int_events & fep->ev_err) {
> +		(*fep->ops->ev_error)(dev, int_events);
> +		do_restart = 1;
> +	}
>   
>   	/* clear status bits for napi*/
>   	(*fep->ops->napi_clear_event)(dev);
>   
> -	do_wake = do_restart = 0;
>   	while (((sc = CBDR_SC(bdp)) & BD_ENET_TX_READY) == 0 && tx_left) {
>   		dirtyidx = bdp - fep->tx_bd_base;
>   
> @@ -318,43 +326,24 @@ fs_enet_interrupt(int irq, void *dev_id)
>   {
>   	struct net_device *dev = dev_id;
>   	struct fs_enet_private *fep;
> -	const struct fs_platform_info *fpi;
>   	u32 int_events;
> -	u32 int_clr_events;
> -	int nr, napi_ok;
> -	int handled;
>   
>   	fep = netdev_priv(dev);
> -	fpi = fep->fpi;
>   
> -	nr = 0;
> -	while ((int_events = (*fep->ops->get_int_events)(dev)) != 0) {
> -		nr++;
> +	int_events = (*fep->ops->get_int_events)(dev);
> +	if (!int_events)
> +		return IRQ_NONE;
>   
> -		int_clr_events = int_events;
> -		int_clr_events &= ~fep->ev_napi;
> +	int_events &= ~fep->ev_napi;
>   
> -		(*fep->ops->clear_int_events)(dev, int_clr_events);
> -
> -		if (int_events & fep->ev_err)
> -			(*fep->ops->ev_error)(dev, int_events);
> -
> -		if (int_events & fep->ev) {
> -			napi_ok = napi_schedule_prep(&fep->napi);
> -
> -			(*fep->ops->napi_disable)(dev);
> -			(*fep->ops->clear_int_events)(dev, fep->ev_napi);
> -
> -			/* NOTE: it is possible for FCCs in NAPI mode    */
> -			/* to submit a spurious interrupt while in poll  */
> -			if (napi_ok)
> -				__napi_schedule(&fep->napi);
> -		}
> +	(*fep->ops->clear_int_events)(dev, int_events);
>   
> +	if (napi_schedule_prep(&fep->napi)) {
> +		(*fep->ops->napi_disable)(dev);
> +		__napi_schedule(&fep->napi);
>   	}
>   
> -	handled = nr > 0;
> -	return IRQ_RETVAL(handled);
> +	return IRQ_HANDLED;
>   }
>   
>   void fs_init_bds(struct net_device *dev)
> diff --git a/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c b/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
> index b47490be872c..66c8f82a8333 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
> @@ -124,7 +124,7 @@ static int do_pd_setup(struct fs_enet_private *fep)
>   	return ret;
>   }
>   
> -#define FCC_NAPI_EVENT_MSK	(FCC_ENET_RXF | FCC_ENET_RXB | FCC_ENET_TXB)
> +#define FCC_NAPI_EVENT_MSK	(FCC_ENET_RXF | FCC_ENET_RXB | FCC_ENET_TXB | FCC_ENET_TXE)
>   #define FCC_EVENT		(FCC_ENET_RXF | FCC_ENET_TXB)
>   #define FCC_ERR_EVENT_MSK	(FCC_ENET_TXE)
>   

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ