Re: [PATCH 3/5] e1000e: Allow TSO to trickle down to VLAN device

[Patrick McHardy <kaber@trash.net>]

Kok, Auke wrote:
> Patrick McHardy wrote:
>> Auke Kok wrote:
>>> Fix TSO over VLAN's by propagating settings to our VLAN devices.
>>>   
>> What exactly is this supposed to fix? If this simply wants
>> to propagate feature changes, I think it should use
>> netdev_feat_change and handle that within the VLAN code.
> 
> I asked PJ and got this reply:
> 
> //
> VLAN devices didn't get the parent's feature flags on creation.  I went
> to fix this in the kernel, people pushed back that some devices couldn't
> support both VLAN tag insertion offload and TSO, so I didn't push the
> issue.  I worked around the issue by copying the flags in the driver.
> The downside is when we turn off TSO using ethtool, we need to remove
> TSO from all VLAN devices, since the hardware segmenter is no longer
> available if the parent device doesn't have it enabled as a feature.  We
> were seeing stack-based panics with gso_segment(), which was corrected
> by removing the TSO flag from all VLAN devices.
> 
> I can't seem to find netdev_feat_change anywhere in the kernel, or
> variants of that name, so I'm not sure what Patrick is pointing us at.
> 
> -PJ
> //

That was a typo, I meant netdev_features_change(), which is invoked
by dev_ethtool() and calls the netdev notifier with NETDEV_FEAT_CHANGE,
on which the VLAN code could react. I wasn't aware of the TSO + VLAN
acceleration problems you've mentioned, do you have a pointer to more
information about this?

In any case I would prefer to avoid having drivers mess with VLAN
device flags. How about adding a device flag indicating that the
driver supports TSO + VLAN acceleration and using the
NETDEV_FEAT_CHANGE notification inside the VLAN code do adjust
the device's flags properly?