dev_disable_lro() improvements for stacked devices

Large receive offloading is known to cause problems if received packets are passed to other host. Therefore the kernel disables it by calling dev_disable_lro() whenever a network device is enslaved in a bridge or forwarding is enabled for it (or globally). For virtual devices we need to disable LRO on the underlying physical device (which is actually receiving the packets).

Current dev_disable_lro() code handles this propagation for a vlan (including 802.1ad nested vlan), macvlan or a vlan on top of a macvlan. This patch adds LRO disabling propagation for

- macvlan on top of a vlan or any stacked combination of those
- bonding
- teaming

In the bonding and teaming case, it is necessary to disable LRO not only on slaves when dev_disable_lro() is called but also on any slave (port) added later.

Michal Kubecek (3):
net: handle more general stacking in dev_disable_lro()
team: add helper to check if device is a team master
net: propagate LRO disabling to bond and team slaves

drivers/net/bonding/bond_main.c |  3 +++
drivers/net/team/team.c         |  6 +++++-
include/linux/netdevice.h       |  7 +++++++
net/core/dev.c                  | 31 ++++++++++++++++++++++---------
4 files changed, 37 insertions(+), 10 deletions(-)

Signature · 1 line

1.8.4.5

Quoted 11 lines · Michal Kubecek

Large receive offloading is known to cause problems if received packets are passed to other host. Therefore the kernel disables it by calling dev_disable_lro() whenever a network device is enslaved in a bridge or forwarding is enabled for it (or globally). For virtual devices we need to disable LRO on the underlying physical device (which is actually receiving the packets).

Current dev_disable_lro() code handles this propagation for a vlan (including 802.1ad nested vlan), macvlan or a vlan on top of a macvlan. This patch adds LRO disabling propagation for

Quoted 3 lines

- macvlan on top of a vlan or any stacked combination of those
- bonding
- teaming

All of these drivers use the netdev_upper and friends, so why not make it generic with netdev_for_each_all_lower() in dev_disable_lro()?

Quoted 12 lines · Michal Kubecek

Quoted 11 lines

Large receive offloading is known to cause problems if received packets are passed to other host. Therefore the kernel disables it by calling dev_disable_lro() whenever a network device is enslaved in a bridge or forwarding is enabled for it (or globally). For virtual devices we need to disable LRO on the underlying physical device (which is actually receiving the packets).

Current dev_disable_lro() code handles this propagation for a vlan (including 802.1ad nested vlan), macvlan or a vlan on top of a macvlan. This patch adds LRO disabling propagation for

Quoted 6 lines

Quoted 3 lines

- macvlan on top of a vlan or any stacked combination of those
- bonding
- teaming

All of these drivers use the netdev_upper and friends, so why not make it generic with netdev_for_each_all_lower() in dev_disable_lro()?

I wanted to preserve current approach where for vlan and macvlan, LRO is disabled on the real device instead of the original one (rather than in addition to it) as LRO is always disabled on them.

Handling all four uniformly would make the code nicer but would bring unnecessary overhead of traversing the list and dev_disable_lro() recursion. On the other hand, this operation is not time critical so it might be acceptable after all.

Please do it generically.

Having a special stanza for each layered device type in dev_disable_lro() is beyond stupid. Especially when it can in fact be done cleanly.

Quoted 6 lines

Please do it generically.

Having a special stanza for each layered device type in dev_disable_lro() is beyond stupid. Especially when it can in fact be done cleanly.

I gave it some thought and I would like ask one question first:

Does the upper-lower relationship always mean that upper device receives packets through its lower device(s) so that LRO should always be disabled for lower devices whenever there are some? Or should it be limited only to an explicit list of device types (vlan, macvlan, bond, team)?

Large receive offloading is known to cause problems if received packets are passed to other host. Therefore the kernel disables it by calling dev_disable_lro() whenever a network device is enslaved in a bridge or forwarding is enabled for it (or globally). For virtual devices we need to disable LRO on the underlying physical device (which is actually receiving the packets).

Current dev_disable_lro() code handles this propagation for a vlan (including 802.1ad nested vlan), macvlan or a vlan on top of a macvlan. It doesn't handle other stacked devices and their combinations, in particular propagation from a bond to its slaves which often causes problems in virtualization setups.

As we now have generic data structures describing the upper-lower device relationship, dev_disable_lro() can be generalized to disable LRO also for all lower devices (if any) once it is disabled for the device itself.

For bonding and teaming devices, it is necessary to disable LRO not only on current slaves at the moment when dev_disable_lro() is called but also on any slave (port) added later.

v2: use lower device links for all devices (including vlan and macvlan)

Signed-off-by Michal Kubecek <[email protected]>

---

drivers/net/bonding/bond_main.c |  3 +++
drivers/net/team/team.c         |  3 +++
net/core/dev.c                  | 15 +++++----------
3 files changed, 11 insertions(+), 10 deletions(-)

drivers/net/bonding/bond_main.c +3 -0

index b9b3456..8575fee 100644
@@ -1526,6 +1526,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	}
 #endif
 
+	if (!(bond_dev->features & NETIF_F_LRO))
+		dev_disable_lro(slave_dev);
+
 	res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
 					 new_slave);
 	if (res) {

drivers/net/team/team.c +3 -0

index 2368395..93e2242 100644
@@ -1179,6 +1179,9 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
 		goto err_enable_netpoll;
 	}
 
+	if (!(dev->features & NETIF_F_LRO))
+		dev_disable_lro(port_dev);
+
 	err = netdev_rx_handler_register(port_dev, team_handle_frame,
 					 port);
 	if (err) {

net/core/dev.c +5 -10

index bb09b03..1ab168e 100644
@@ -1437,22 +1437,17 @@ EXPORT_SYMBOL(dev_close);
 */
 void dev_disable_lro(struct net_device *dev)
 {
-	/*
-	 * If we're trying to disable lro on a vlan device
-	 * use the underlying physical device instead
-	 */
-	if (is_vlan_dev(dev))
-		dev = vlan_dev_real_dev(dev);
-
-	/* the same for macvlan devices */
-	if (netif_is_macvlan(dev))
-		dev = macvlan_dev_real_dev(dev);
+	struct net_device *lower_dev;
+	struct list_head *iter;
 
 	dev->wanted_features &= ~NETIF_F_LRO;
 	netdev_update_features(dev);
 
 	if (unlikely(dev->features & NETIF_F_LRO))
 		netdev_WARN(dev, "failed to disable LRO!\n");
+
+	netdev_for_each_lower_dev(dev, lower_dev, iter)
+		dev_disable_lro(lower_dev);
 }
 EXPORT_SYMBOL(dev_disable_lro);
 

Signature · 1 line

1.8.4.5

Large receive offloading is known to cause problems if received packets are passed to other host. Therefore the kernel disables it by calling dev_disable_lro() whenever a network device is enslaved in a bridge or forwarding is enabled for it (or globally). For virtual devices we need to disable LRO on the underlying physical device (which is actually receiving the packets).

Current dev_disable_lro() code handles this propagation for a vlan (including 802.1ad nested vlan), macvlan or a vlan on top of a macvlan. It doesn't handle other stacked devices and their combinations, in particular propagation from a bond to its slaves which often causes problems in virtualization setups.

As we now have generic data structures describing the upper-lower device relationship, dev_disable_lro() can be generalized to disable LRO also for all lower devices (if any) once it is disabled for the device itself.

For bonding and teaming devices, it is necessary to disable LRO not only on current slaves at the moment when dev_disable_lro() is called but also on any slave (port) added later.

v2: use lower device links for all devices (including vlan and macvlan)

Signed-off-by Michal Kubecek <[email protected]>