ACK/cmnt: [PATCH][Trusty/Utopic] (upstream) net: generic dev_disable_lro() stacked device handling

[Dan Streetman]

On Wed, Feb 24, 2016 at 11:23 AM, Stefan Bader
<stefan.bader at canonical.com> wrote:
> I guess its not going to happen as normal stable since David Miller is no longer
> looking at those versions. Patch seems reasonable. Just a bit more background
> info somewhere (bug report) and hints about any testing done would be nice.

I added some more detail to the bug; thanks!

>
> -Stefan
>
>
> On 24.02.2016 00:13, Dan Streetman wrote:
>> BugLink: http://bugs.launchpad.net/bugs/1547680
>>
>> 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 at suse.cz>
>> Acked-by: Veaceslav Falico <vfalico at gmail.com>
>> Signed-off-by: David S. Miller <davem at davemloft.net>
>> (cherry-picked from fbe168ba91f7c327856f205699404284c2f09e36 upstream)
>> Signed-off-by: Dan Streetman <dan.streetman at canonical.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 d0f58e4..6eb47aa 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1584,6 +1584,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 f30a88b..03bac60 100644
>> --- a/drivers/net/team/team.c
>> +++ b/drivers/net/team/team.c
>> @@ -1180,6 +1180,9 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
>>               goto err_set_upper_link;
>>       }
>>
>> +     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 31f1f68..e758293 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1423,22 +1423,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);
>>
>>
>
>