bgmac: Remove all offloading features, including GRO.

PATCH In review Rosen Penev 5 participants 10 messages Sep 15, 2017 – Sep 16, 2017

Subsystems (3)

Networking Network drivers Broadcom iproc gbit Ethernet driver

lore

Share & tools

lore

Rosen Penev

author

2017-09-15 22:22 UTC #0 lore raw

PATCH 1/1 bgmac: Remove all offloading features, including GRO.

On a linksys E1200v1 (actually a crossflashed E1000v2), the offloading features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg 3mbps. Results:

Currently:

[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52410

[ ID] Interval        Transfer    Bandwidth       Reads   Dist(bin=16.0K)
[  4] 0.00-10.02 sec  52.4 MBytes  43.8 Mbits/sec  641    75:181:12:1:1:0:0:371
[  5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52412
[  5] 0.00-10.02 sec  52.4 MBytes  43.8 Mbits/sec  629    51:194:13:1:0:1:0:369
[  4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52414
[  4] 0.00-10.02 sec  51.9 MBytes  43.4 Mbits/sec  695    126:203:1:0:0:0:2:363
[  5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52416
[  5] 0.00-10.01 sec  52.4 MBytes  43.9 Mbits/sec  626    57:186:10:0:0:0:0:373
[  4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52420
[  4] 0.00-10.02 sec  52.4 MBytes  43.8 Mbits/sec  605    36:179:16:1:0:1:0:372
[  5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52422

After disabling everything - including GRO:

[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52440

[ ID] Interval        Transfer    Bandwidth       Reads   Dist(bin=16.0K)
[  4] 0.00-10.01 sec  55.1 MBytes  46.2 Mbits/sec  672    180:82:0:0:1:0:0:409
[  5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52442
[  5] 0.00-10.01 sec  56.0 MBytes  46.9 Mbits/sec  636    117:96:0:0:1:0:0:422
[  4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52444
[  4] 0.00-10.01 sec  55.4 MBytes  46.4 Mbits/sec  675    172:92:0:0:1:0:0:410
[  5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52446
[  5] 0.00-10.01 sec  56.0 MBytes  46.9 Mbits/sec  633    119:90:0:1:1:0:0:422
[  4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52448
[  4] 0.00-10.01 sec  55.2 MBytes  46.3 Mbits/sec  688    157:123:0:0:2:0:0:406

Signed-off-by Rosen Penev <[email protected]>

---
 drivers/net/ethernet/broadcom/bgmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/broadcom/bgmac.c +1 -1

index 48d672b204a4..c2db5673b073 100644
@@ -1534,7 +1534,7 @@ int bgmac_enet_probe(struct bgmac *bgmac)
 		goto err_dma_free;
 	}
 
-	net_dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+	net_dev->features &= NETIF_F_GRO;
 	net_dev->hw_features = net_dev->features;
 	net_dev->vlan_features = net_dev->features;

Signature · 1 line

2.13.5

Florian Fainelli

2017-09-15 22:54 UTC #1 lore raw

Quoted 3 lines · Rosen Penev

On a linksys E1200v1 (actually a crossflashed E1000v2), the offloading features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg 3mbps.

Do you have a way to generate gigabit tests and see what results you are getting? We probably are not going to see a 30% improvement just by extrapolation.

Signature · 1 line

Florian

Eric Dumazet

2017-09-16 00:03 UTC #2 lore raw

Quoted 12 lines · Florian Fainelli

Quoted 1 line · Rosen Penev
On a linksys E1200v1 (actually a crossflashed E1000v2), the
offloading
Quoted 2 lines
features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg
3mbps.
Do you have a way to generate gigabit tests and see what results you are getting? We probably are not going to see a 30% improvement just by extrapolation.

It seems silly to remove NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM from dev->features, yet leave the dead-code in the driver to handle these features.

And of course GRO was not removed, meaning the bench results were non conclusive.

[email protected]

author

2017-09-16 00:10 UTC #3 lore raw

Ok fair enough. Will only disable GRO in the driver.

Tests were done using "ethtool -K eth0 gro off" and on.

Quoted 31 lines · Eric Dumazet

Quoted 10 lines · Florian Fainelli
On September 15, 2017 3:22:18 PM PDT, Rosen Penev <[email protected]
Quoted 1 line
Quoted 1 line · wrote:
On a linksys E1200v1 (actually a crossflashed E1000v2), the
offloading
Quoted 3 lines
features give no measurable benefit to speed or latency. Furthermore,
disabling GRO actually improves iperf performance by a whoppimg
Quoted 8 lines
3mbps.
Do you have a way to generate gigabit tests and see what results you
are getting? We probably are not going to see a 30% improvement just
by extrapolation.
+1
It seems silly to remove NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM from dev->features, yet leave the dead-code in the driver to handle these features.
And of course GRO was not removed, meaning the bench results were non conclusive.

Eric Dumazet

2017-09-16 00:18 UTC #4 lore raw

Quoted 1 line · On Fri, 2017-09-15 at 17:10 -0700, [email protected] wrote:

Ok fair enough. Will only disable GRO in the driver.

Well, do not even try.

NETIF_F_SOFT_FEATURES is set by core networking stack in register_netdevice(), ( commit 212b573f5552c60265da721ff9ce32e3462a2cdd )

Absolutely no driver disables GRO (excepts the ones playing with LRO)

[email protected]

author

2017-09-16 00:24 UTC #5 lore raw

netif_receive_skb seems to have the same effect as "ethtool -K eth0 gro off".

Quoted 17 lines · Eric Dumazet

Quoted 1 line · On Fri, 2017-09-15 at 17:10 -0700, [email protected] wrote:
Ok fair enough. Will only disable GRO in the driver.
Well, do not even try.
NETIF_F_SOFT_FEATURES is set by core networking stack in register_netdevice(), ( commit 212b573f5552c60265da721ff9ce32e3462a2cdd
)
Absolutely no driver disables GRO (excepts the ones playing with LRO)

Denys Fedoryshchenko

2017-09-16 00:26 UTC #6 lore raw

In reply to Eric Dumazet

Quoted 10 lines · Eric Dumazet

Quoted 1 line · On Fri, 2017-09-15 at 17:10 -0700, [email protected] wrote:
Ok fair enough. Will only disable GRO in the driver.
Well, do not even try.
NETIF_F_SOFT_FEATURES is set by core networking stack in register_netdevice(), ( commit 212b573f5552c60265da721ff9ce32e3462a2cdd )
Absolutely no driver disables GRO (excepts the ones playing with LRO)

I believe also iperf is definitely inconclusive test.
Except iperf there is lot of different workloads and configurations, that might have different results.

David Miller

maintainer

2017-09-16 03:56 UTC #7 lore raw

In reply to Eric Dumazet

From: Eric Dumazet <[email protected]>
Date: Fri, 15 Sep 2017 17:03:30 -0700

Quoted 13 lines · Florian Fainelli

Quoted 12 lines
Quoted 1 line · Rosen Penev
On a linksys E1200v1 (actually a crossflashed E1000v2), the
offloading
Quoted 2 lines
features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg
3mbps.
Do you have a way to generate gigabit tests and see what results you are getting? We probably are not going to see a 30% improvement just by extrapolation.

Quoted 8 lines

+1
It seems silly to remove NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM from dev->features, yet leave the dead-code in the driver to handle these features.
And of course GRO was not removed, meaning the bench results were non conclusive.

My sentiments exactly, I think this is a completely unwise change.

David Miller

maintainer

2017-09-15 23:14 UTC #8 lore raw

In reply to Rosen Penev

From: Rosen Penev <[email protected]>
Date: Fri, 15 Sep 2017 15:22:18 -0700

Quoted 1 line

On a linksys E1200v1 (actually a crossflashed E1000v2), the offloading features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg 3mbps. Results:

...

drivers/net/ethernet/broadcom/bgmac.c int bgmac_enet_probe(struct bgmac *bgmac) · line 1538

Quoted 3 lines

drivers/net/ethernet/broadcom/bgmac.c +1 -1

-	net_dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+	net_dev->features &= NETIF_F_GRO;
 	net_dev->hw_features = net_dev->features;

This doesn't disable GRO.

[email protected]

author

2017-09-15 23:55 UTC #9 lore raw

you're absolutely correct. will send an updated version shortly.

Quoted 14 lines · David Miller

From: Rosen Penev <[email protected]>
Date: Fri, 15 Sep 2017 15:22:18 -0700
Quoted 1 line
On a linksys E1200v1 (actually a crossflashed E1000v2), the
offloading features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg 3mbps. Results:
...
Quoted 1 line
- net_dev->features = NETIF_F_SG | NETIF_F_IP_CSUM |
NETIF_F_IPV6_CSUM;
Quoted 2 lines
+     net_dev->features &= NETIF_F_GRO;
      net_dev->hw_features = net_dev->features;
This doesn't disable GRO.