diff mbox

[net-next,v2] net: generic dev_disable_lro() stacked device handling

Message ID 20141113065450.1645FA0BEF@unicorn.suse.cz
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Michal Kubecek Nov. 13, 2014, 6:54 a.m. UTC
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 <mkubecek@suse.cz>
---
 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(-)

Comments

Veaceslav Falico Nov. 13, 2014, 7:15 a.m. UTC | #1
On Thu, Nov 13, 2014 at 07:54:50AM +0100, Michal Kubecek wrote:
>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 <mkubecek@suse.cz>

Seems ok to me.

Acked-by: Veaceslav Falico <vfalico@gmail.com>

>---
> 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(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index b9b3456..8575fee 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -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) {
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index 2368395..93e2242 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -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) {
>diff --git a/net/core/dev.c b/net/core/dev.c
>index bb09b03..1ab168e 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -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);
>
>-- 
>1.8.4.5
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Nov. 13, 2014, 7:49 p.m. UTC | #2
From: Michal Kubecek <mkubecek@suse.cz>
Date: Thu, 13 Nov 2014 07:54:50 +0100 (CET)

> 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 <mkubecek@suse.cz>

Applied, thanks a lot.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b9b3456..8575fee 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -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) {
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 2368395..93e2242 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -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) {
diff --git a/net/core/dev.c b/net/core/dev.c
index bb09b03..1ab168e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -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);