netdev - Re: [PATCH] net: sh_eth: fix the rxdesc pointer when rx descriptor empty happens

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: <Pine.LNX.4.64.1206221610350.17552@axis700.grange>
Date:	Fri, 22 Jun 2012 16:16:38 +0200 (CEST)
From:	Guennadi Liakhovetski <g.liakhovetski@....de>
To:	"Shimoda, Yoshihiro" <yoshihiro.shimoda.uh@...esas.com>
cc:	netdev <netdev@...r.kernel.org>,
	SH-Linux <linux-sh@...r.kernel.org>
Subject: Re: [PATCH] net: sh_eth: fix the rxdesc pointer when rx descriptor
 empty happens

Hi Shimoda-san

On Thu, 21 Jun 2012, Shimoda, Yoshihiro wrote:

> Hello Guennadi-san,
> 
> 2012/06/20 22:10, Guennadi Liakhovetski wrote:
> > Hello Shimoda-san
> > 
> > On Tue, 29 May 2012, Shimoda, Yoshihiro wrote:
> > 
> >> When Receive Descriptor Empty happens, rxdesc pointer of the driver
> >> and actual next descriptor of the controller may be mismatch.
> >> This patch fixes it.
> > 
> > Unfortunately, this patch breaks networking on ecovec (sh7724). Booting 
> > with dhcp and NFS-root progresses very slowly with lots of "nfs: server 
> > not responding / Ok" messages and never completes. Reverting it in current 
> > Linus' tree fixes the problem.
> 
> Thank you very much for the report.
> The SH7724 doesn't set the RMCR register. So, the EDRRR will be clear after
> the controller receives a freme every time.
> So, the "fix the rxdesc pointer" had to check a condition.
> 
> I wrote a patch for the issue as the following.
> If possible, would you try the patch?

Yes, it does seem to fix the problem.

Tested-by: Guennadi Liakhovetski <g.liakhovetski@....de>

Thanks
Guennadi

> 
> Best regards,
> Yoshihiro Shimoda
> ---
> Subject: [PATCH] net: sh_eth: fix the condition to fix the cur_tx/dirty_rx
> 
> The following commit couldn't work if the RMCR is not set to 1.
> 
> "net: sh_eth: fix the rxdesc pointer when rx descriptor empty happens"
> commit id 79fba9f51755c704c0a7d7b7f0df10874dc0a744
> 
> If RMCR is not set, the controller will clear the EDRRR after it received
> a frame. In this case, the driver doesn't need to fix the value of
> cur_rx/dirty_rx. The driver only needs it when the controll detects
> receive descriptors are empty.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> ---
>  drivers/net/ethernet/renesas/sh_eth.c |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 667169b..79bf09b 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -1011,7 +1011,7 @@ static int sh_eth_txfree(struct net_device *ndev)
>  }
> 
>  /* Packet receive function */
> -static int sh_eth_rx(struct net_device *ndev)
> +static int sh_eth_rx(struct net_device *ndev, u32 intr_status)
>  {
>  	struct sh_eth_private *mdp = netdev_priv(ndev);
>  	struct sh_eth_rxdesc *rxdesc;
> @@ -1102,9 +1102,11 @@ static int sh_eth_rx(struct net_device *ndev)
>  	/* Restart Rx engine if stopped. */
>  	/* If we don't need to check status, don't. -KDU */
>  	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R)) {
> -		/* fix the values for the next receiving */
> -		mdp->cur_rx = mdp->dirty_rx = (sh_eth_read(ndev, RDFAR) -
> -					       sh_eth_read(ndev, RDLAR)) >> 4;
> +		/* fix the values for the next receiving if RDE is set */
> +		if (intr_status & EESR_RDE)
> +			mdp->cur_rx = mdp->dirty_rx =
> +				(sh_eth_read(ndev, RDFAR) -
> +				 sh_eth_read(ndev, RDLAR)) >> 4;
>  		sh_eth_write(ndev, EDRRR_R, EDRRR);
>  	}
> 
> @@ -1273,7 +1275,7 @@ static irqreturn_t sh_eth_interrupt(int irq, void *netdev)
>  			EESR_RTSF | /* short frame recv */
>  			EESR_PRE  | /* PHY-LSI recv error */
>  			EESR_CERF)){ /* recv frame CRC error */
> -		sh_eth_rx(ndev);
> +		sh_eth_rx(ndev, intr_status);
>  	}
> 
>  	/* Tx Check */
> -- 
> 1.7.1
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html