netdev - Re: [PATCH net-next v3 2/5] net: Disable GRO_HW when generic XDP is installed on a device.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKgT0UeqWjHEFYLzbSk5O_7xV=QYYtPd-HdnRaGgpYy=d3yhLg@mail.gmail.com>
Date:   Sat, 9 Dec 2017 14:37:57 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Michael Chan <michael.chan@...adcom.com>
Cc:     David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>,
        Andrew Gospodarek <andrew.gospodarek@...adcom.com>,
        Ariel Elior <Ariel.Elior@...ium.com>,
        everest-linux-l2@...ium.com
Subject: Re: [PATCH net-next v3 2/5] net: Disable GRO_HW when generic XDP is
 installed on a device.

On Sat, Dec 9, 2017 at 1:40 PM, Michael Chan <michael.chan@...adcom.com> wrote:
> On Sat, Dec 9, 2017 at 10:56 AM, Alexander Duyck
> <alexander.duyck@...il.com> wrote:
>> On Fri, Dec 8, 2017 at 10:27 PM, Michael Chan <michael.chan@...adcom.com> wrote:
>>> Hardware should not aggregate any packets when generic XDP is installed.
>>>
>>> Cc: Ariel Elior <Ariel.Elior@...ium.com>
>>> Cc: everest-linux-l2@...ium.com
>>> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
>>> ---
>>>  net/core/dev.c | 24 ++++++++++++++++++++++++
>>>  1 file changed, 24 insertions(+)
>>>
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 6ebd0e7..ec08ace 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -1542,6 +1542,29 @@ void dev_disable_lro(struct net_device *dev)
>>>  }
>>>  EXPORT_SYMBOL(dev_disable_lro);
>>>
>>> +/**
>>> + *     dev_disable_gro_hw - disable HW Generic Receive Offload on a device
>>> + *     @dev: device
>>> + *
>>> + *     Disable HW Generic Receive Offload (GRO_HW) on a net device.  Must be
>>> + *     called under RTNL.  This is needed if Generic XDP is installed on
>>> + *     the device.
>>> + */
>>> +static void dev_disable_gro_hw(struct net_device *dev)
>>> +{
>>> +       struct net_device *lower_dev;
>>> +       struct list_head *iter;
>>> +
>>> +       dev->wanted_features &= ~NETIF_F_GRO_HW;
>>> +       netdev_update_features(dev);
>>> +
>>> +       if (unlikely(dev->features & NETIF_F_GRO_HW))
>>> +               netdev_WARN(dev, "failed to disable GRO_HW!\n");
>>> +
>>> +       netdev_for_each_lower_dev(dev, lower_dev, iter)
>>> +               dev_disable_gro_hw(lower_dev);
>>
>> I think these two lines are redundant in dev_disable_lro, since
>> netdev_update_features should propagate the disable to all of the
>> lower devices.
>
> Right.  But for GRO_HW, there is no automatic propagation.

Right, but that is also an issue since the automatic propagation is
what prevents LRO from being re-enabled on the lower devices.

>> Also this doesn't prevent the lower devices from
>> re-enabling gro_hw.
>
> Right.  You can re-enable LRO on the upper device as well.

On the upper device yes, but not on the lower devices. That was what I
was getting at. With LRO there is netdev_sync_upper_features() and
that prevents you from enabling it if the upper device has it
disabled. The problem is right now there is nothing that sets it for
the upper devices when they are added to something like a bond so that
is one of the pieces that still has to be worked on before we can just
use the existing sync logic.