[PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720

[PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720

[Marek Behún]

For some reason on Armada 3720 boards (EspressoBin and Turris Mox) the
networking driver behaves weirdly when using napi_gro_receive.

For example downloading a big file from a local network (low ping) is
fast, but when downloading from a remote server (higher ping), the
download speed is at first high but drops rapidly to almost nothing or
absolutely nothing.

This is fixed when using netif_receive_skb instead of napi_gro_receive.

Signed-off-by: Marek Behun <marek.behun@nic.cz>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: netdev@vger.kernel.org

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 0ad2f3f7da85..27f3017d94c5 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1959,7 +1959,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
 
 			skb->protocol = eth_type_trans(skb, dev);
 			mvneta_rx_csum(pp, rx_status, skb);
-			napi_gro_receive(&port->napi, skb);
+			if (pp->neta_armada3700)
+				netif_receive_skb(skb);
+			else
+				napi_gro_receive(&port->napi, skb);
 
 			rcvd_pkts++;
 			rcvd_bytes += rx_bytes;
@@ -2001,7 +2004,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
 
 		mvneta_rx_csum(pp, rx_status, skb);
 
-		napi_gro_receive(&port->napi, skb);
+		if (pp->neta_armada3700)
+			netif_receive_skb(skb);
+		else
+			napi_gro_receive(&port->napi, skb);
 	}
 
 	if (rcvd_pkts) {
-- 
2.16.4

Re: [PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720

[Andrew Lunn]

On Wed, Aug 08, 2018 at 05:27:05PM +0200, Marek Behún wrote:
> For some reason on Armada 3720 boards (EspressoBin and Turris Mox) the
> networking driver behaves weirdly when using napi_gro_receive.
> 
> For example downloading a big file from a local network (low ping) is
> fast, but when downloading from a remote server (higher ping), the
> download speed is at first high but drops rapidly to almost nothing or
> absolutely nothing.
> 
> This is fixed when using netif_receive_skb instead of napi_gro_receive.

Before doing this, we should really understand what is going on. It is
probably just a driver bug which needs fixing. And GRO should be good
for performance, so we do want to use it, if possible.

    Andrew

Re: [PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720

[Dave Taht]

On Wed, Aug 8, 2018 at 10:00 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Aug 08, 2018 at 05:27:05PM +0200, Marek Behún wrote:
> > For some reason on Armada 3720 boards (EspressoBin and Turris Mox) the
> > networking driver behaves weirdly when using napi_gro_receive.
> >
> > For example downloading a big file from a local network (low ping) is
> > fast, but when downloading from a remote server (higher ping), the
> > download speed is at first high but drops rapidly to almost nothing or
> > absolutely nothing.
> >
> > This is fixed when using netif_receive_skb instead of napi_gro_receive.
>
> Before doing this, we should really understand what is going on. It is
> probably just a driver bug which needs fixing. And GRO should be good
> for performance, so we do want to use it, if possible.

I'd just disable it and worry about it later. The software gro in the
mvneta would batch up 64k and is one of the reasons why sch_cake does
gso splitting by default. (64k unsplit, downshifted to 1mbit = ~540ms
of latency). If this mvneta facility is in addition buggy, that
explains some puzzling things I've seen in various benchmarks. thx for
the steer as to what to look for!

IMHO: in general gro looks good on dumb single stream benchmarks, not
as useful on mixed routed traffic with more entropy, and batching
clutters up the mvneta receive path that otherwise could be draining
the rx ring and spitting packets into the rest of the system faster.
The mvneta is mostly (?) used on routing devices.



>
>     Andrew



-- 

Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619

Re: [PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720

[Jisheng Zhang]

+ more people

On Wed,  8 Aug 2018 17:27:05 +0200 Marek Behún wrote:

> For some reason on Armada 3720 boards (EspressoBin and Turris Mox) the
> networking driver behaves weirdly when using napi_gro_receive.
> 
> For example downloading a big file from a local network (low ping) is
> fast, but when downloading from a remote server (higher ping), the
> download speed is at first high but drops rapidly to almost nothing or
> absolutely nothing.

We also met this issue on some berlin platforms. I tried to fix the bug,
but no clue so far.

> 
> This is fixed when using netif_receive_skb instead of napi_gro_receive.

This is a workaround. The good news is this workaround also fixes the issue
we saw on berlin.

> 
> Signed-off-by: Marek Behun <marek.behun@nic.cz>
> Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> Cc: netdev@vger.kernel.org
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 0ad2f3f7da85..27f3017d94c5 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -1959,7 +1959,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
>  
>  			skb->protocol = eth_type_trans(skb, dev);
>  			mvneta_rx_csum(pp, rx_status, skb);
> -			napi_gro_receive(&port->napi, skb);
> +			if (pp->neta_armada3700)
> +				netif_receive_skb(skb);
> +			else
> +				napi_gro_receive(&port->napi, skb);
>  
>  			rcvd_pkts++;
>  			rcvd_bytes += rx_bytes;
> @@ -2001,7 +2004,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
>  
>  		mvneta_rx_csum(pp, rx_status, skb);
>  
> -		napi_gro_receive(&port->napi, skb);
> +		if (pp->neta_armada3700)
> +			netif_receive_skb(skb);
> +		else
> +			napi_gro_receive(&port->napi, skb);
>  	}
>  
>  	if (rcvd_pkts) {

Re: [PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720

[Jisheng Zhang]

Hi,

On Thu, 9 Aug 2018 12:40:41 +0800 Jisheng Zhang wrote:

> + more people
> 
> On Wed,  8 Aug 2018 17:27:05 +0200 Marek Behún wrote:
> 
> > For some reason on Armada 3720 boards (EspressoBin and Turris Mox) the
> > networking driver behaves weirdly when using napi_gro_receive.
> > 
> > For example downloading a big file from a local network (low ping) is
> > fast, but when downloading from a remote server (higher ping), the
> > download speed is at first high but drops rapidly to almost nothing or
> > absolutely nothing.  
> 
> We also met this issue on some berlin platforms. I tried to fix the bug,
> but no clue so far.
> 
> > 
> > This is fixed when using netif_receive_skb instead of napi_gro_receive.  
> 
> This is a workaround. The good news is this workaround also fixes the issue
> we saw on berlin.
> 
> > 
> > Signed-off-by: Marek Behun <marek.behun@nic.cz>
> > Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> > Cc: netdev@vger.kernel.org
> > 
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index 0ad2f3f7da85..27f3017d94c5 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -1959,7 +1959,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> >  
> >  			skb->protocol = eth_type_trans(skb, dev);
> >  			mvneta_rx_csum(pp, rx_status, skb);
> > -			napi_gro_receive(&port->napi, skb);
> > +			if (pp->neta_armada3700)
> > +				netif_receive_skb(skb);
> > +			else
> > +				napi_gro_receive(&port->napi, skb);

I think I found the root cause, if neta_armada3700 is true, the port got from
this_cpu_ptr(pp->ports) is invalid, this is bug... I'll cook a patch for this

Thanks

> >  
> >  			rcvd_pkts++;
> >  			rcvd_bytes += rx_bytes;
> > @@ -2001,7 +2004,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> >  
> >  		mvneta_rx_csum(pp, rx_status, skb);
> >  
> > -		napi_gro_receive(&port->napi, skb);
> > +		if (pp->neta_armada3700)
> > +			netif_receive_skb(skb);
> > +		else
> > +			napi_gro_receive(&port->napi, skb);
> >  	}
> >  
> >  	if (rcvd_pkts) {  
> 

Re: [PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720

[Jisheng Zhang]

On Thu, 9 Aug 2018 19:27:55 +0800 Jisheng Zhang wrote:

> Hi,
> 
> On Thu, 9 Aug 2018 12:40:41 +0800 Jisheng Zhang wrote:
> 
> > + more people
> > 
> > On Wed,  8 Aug 2018 17:27:05 +0200 Marek Behún wrote:
> >   
> > > For some reason on Armada 3720 boards (EspressoBin and Turris Mox) the
> > > networking driver behaves weirdly when using napi_gro_receive.
> > > 
> > > For example downloading a big file from a local network (low ping) is
> > > fast, but when downloading from a remote server (higher ping), the
> > > download speed is at first high but drops rapidly to almost nothing or
> > > absolutely nothing.    
> > 
> > We also met this issue on some berlin platforms. I tried to fix the bug,
> > but no clue so far.
> >   
> > > 
> > > This is fixed when using netif_receive_skb instead of napi_gro_receive.    
> > 
> > This is a workaround. The good news is this workaround also fixes the issue
> > we saw on berlin.
> >   
> > > 
> > > Signed-off-by: Marek Behun <marek.behun@nic.cz>
> > > Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> > > Cc: netdev@vger.kernel.org
> > > 
> > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > > index 0ad2f3f7da85..27f3017d94c5 100644
> > > --- a/drivers/net/ethernet/marvell/mvneta.c
> > > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > > @@ -1959,7 +1959,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> > >  
> > >  			skb->protocol = eth_type_trans(skb, dev);
> > >  			mvneta_rx_csum(pp, rx_status, skb);
> > > -			napi_gro_receive(&port->napi, skb);
> > > +			if (pp->neta_armada3700)
> > > +				netif_receive_skb(skb);
> > > +			else
> > > +				napi_gro_receive(&port->napi, skb);  
> 
> I think I found the root cause, if neta_armada3700 is true, the port got from
> this_cpu_ptr(pp->ports) is invalid, this is bug... I'll cook a patch for this

correct it as:

the port's(port is got from this_cpu_ptr(pp->ports) napi is invalid.

Patch is sent out. Could you please try?

Per my test, it solves the issue we saw on berlin.

> 
> Thanks
> 
> > >  
> > >  			rcvd_pkts++;
> > >  			rcvd_bytes += rx_bytes;
> > > @@ -2001,7 +2004,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> > >  
> > >  		mvneta_rx_csum(pp, rx_status, skb);
> > >  
> > > -		napi_gro_receive(&port->napi, skb);
> > > +		if (pp->neta_armada3700)
> > > +			netif_receive_skb(skb);
> > > +		else
> > > +			napi_gro_receive(&port->napi, skb);
> > >  	}
> > >  
> > >  	if (rcvd_pkts) {    
> >   
> 

Re: [PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720

[Marek Behún]

On Thu, 9 Aug 2018 20:08:32 +0800
Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:

> On Thu, 9 Aug 2018 19:27:55 +0800 Jisheng Zhang wrote:
> 
> > Hi,
> > 
> > On Thu, 9 Aug 2018 12:40:41 +0800 Jisheng Zhang wrote:
> >   
> > > + more people
> > > 
> > > On Wed,  8 Aug 2018 17:27:05 +0200 Marek Behún wrote:
> > >     
> > > > For some reason on Armada 3720 boards (EspressoBin and Turris
> > > > Mox) the networking driver behaves weirdly when using
> > > > napi_gro_receive.
> > > > 
> > > > For example downloading a big file from a local network (low
> > > > ping) is fast, but when downloading from a remote server
> > > > (higher ping), the download speed is at first high but drops
> > > > rapidly to almost nothing or absolutely nothing.      
> > > 
> > > We also met this issue on some berlin platforms. I tried to fix
> > > the bug, but no clue so far.
> > >     
> > > > 
> > > > This is fixed when using netif_receive_skb instead of
> > > > napi_gro_receive.      
> > > 
> > > This is a workaround. The good news is this workaround also fixes
> > > the issue we saw on berlin.
> > >     
> > > > 
> > > > Signed-off-by: Marek Behun <marek.behun@nic.cz>
> > > > Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> > > > Cc: netdev@vger.kernel.org
> > > > 
> > > > diff --git a/drivers/net/ethernet/marvell/mvneta.c
> > > > b/drivers/net/ethernet/marvell/mvneta.c index
> > > > 0ad2f3f7da85..27f3017d94c5 100644 ---
> > > > a/drivers/net/ethernet/marvell/mvneta.c +++
> > > > b/drivers/net/ethernet/marvell/mvneta.c @@ -1959,7 +1959,10 @@
> > > > static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo, 
> > > >  			skb->protocol = eth_type_trans(skb,
> > > > dev); mvneta_rx_csum(pp, rx_status, skb);
> > > > -			napi_gro_receive(&port->napi, skb);
> > > > +			if (pp->neta_armada3700)
> > > > +				netif_receive_skb(skb);
> > > > +			else
> > > > +				napi_gro_receive(&port->napi,
> > > > skb);    
> > 
> > I think I found the root cause, if neta_armada3700 is true, the
> > port got from this_cpu_ptr(pp->ports) is invalid, this is bug...
> > I'll cook a patch for this  
> 
> correct it as:
> 
> the port's(port is got from this_cpu_ptr(pp->ports) napi is invalid.
> 
> Patch is sent out. Could you please try?
> 
> Per my test, it solves the issue we saw on berlin.
> 
> > 
> > Thanks
> >   
> > > >  
> > > >  			rcvd_pkts++;
> > > >  			rcvd_bytes += rx_bytes;
> > > > @@ -2001,7 +2004,10 @@ static int mvneta_rx_swbm(struct
> > > > mvneta_port *pp, int rx_todo, 
> > > >  		mvneta_rx_csum(pp, rx_status, skb);
> > > >  
> > > > -		napi_gro_receive(&port->napi, skb);
> > > > +		if (pp->neta_armada3700)
> > > > +			netif_receive_skb(skb);
> > > > +		else
> > > > +			napi_gro_receive(&port->napi, skb);
> > > >  	}
> > > >  
> > > >  	if (rcvd_pkts) {      
> > >     
> >   
> 

Jisheng, the issue is solved with your patch. Thanks :)