On Thu, Dec 7, 2017 at 10:13 AM, Alexander Duyck
<alexander.du...@gmail.com> wrote:
> On Thu, Dec 7, 2017 at 12:03 AM, Michael Chan <michael.c...@broadcom.com>
> wrote:
>> @@ -7405,6 +7405,23 @@ static netdev_features_t netdev_fix_features(struct
>> net_device *dev,
>> features &= ~dev->gso_partial_features;
>> }
>>
>> + if (features & NETIF_F_GRO_HW) {
>> + /* Hardware GRO depends on GRO and RXCSUM. */
>> + if (!(features & NETIF_F_GRO)) {
>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no
>> GRO feature.\n");
>> + features &= ~NETIF_F_GRO_HW;
>> + }
>
> I'm not sure I agree with this part. The hardware offload should not
> be dependent on the software offload.
As I said before, GRO_HW is basically hardware accelerated GRO. To me,
it makes perfect sense that GRO_HW depends on GRO.
> If you are concerned with the
> XDP case and such you should probably just look at handling it more
> like how TSO is handled with a group of flags that aggregate GRO, LRO,
> and GRO_HW into a group of features that must be disabled to support
> XDP.
>
>> + if (!(features & NETIF_F_RXCSUM)) {
>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no
>> RXCSUM feature.\n");
>> + features &= ~NETIF_F_GRO_HW;
>> + }
>> + /* Hardware GRO and LRO are mutually exclusive. */
>> + if (features & NETIF_F_LRO) {
>> + netdev_dbg(dev, "Dropping NETIF_F_LRO since GRO_HW
>> is set.\n");
>> + features &= ~NETIF_F_LRO;
>> + }
>
> This is going to be problematic. What if you bond one interface that
> has LRO and one that has GRO_HW? It seems like this is going to cause
> the bond interface to lie and say LRO isn't there when it actually is,
> or worse yet it will force features off when they shouldn't be.
This is just dropping incompatible features similar to how other
incompatible feature flags are dropped in netdev_fix_features().
Hardware cannot aggregate in both LRO and GRO_HW modes at the same
time. It's one or the other.
LRO and GRO_HW are different. We may never agree on this but they are
different. If a bond needs to disable LRO, it will be propagated to
all slaves. But each slave can have GRO enabled. If GRO is enabled,
GRO_HW, being a subset of GRO, can be enabled.
