On 05/05/2017 04:01 PM, Alexander Duyck wrote:
> On Fri, May 5, 2017 at 12:20 PM, Vladislav Yasevich <vyasev...@gmail.com>
> wrote:
>> Vlan devices, like all other software devices, enable
>> NETIF_F_HW_CSUM feature. However, unlike all the othe other
>> software devices, vlans will switch to using IP|IPV6_CSUM
>> features, if the underlying devices uses them. In these situations,
>> checksum offload features on the vlan device can't be controlled
>> via ethtool.
>>
>> This patch makes vlans keep HW_CSUM feature if the underlying
>> device supports checksum offloading. This makes vlan devices
>> behave like other software devices, and restores control to the
>> user.
>>
>> A side-effect is that some offload settings (typically UFO)
>> may be enabled on the vlan device while being disabled on the HW.
>> However, the GSO code will correctly process the packets. This
>> actually results in slightly better raw throughput.
>>
>> Signed-off-by: Vladislav Yasevich <vyase...@redhat.com>
>> ---
>> net/8021q/vlan_dev.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
>> index 9ee5787..ffc8167 100644
>> --- a/net/8021q/vlan_dev.c
>> +++ b/net/8021q/vlan_dev.c
>> @@ -626,10 +626,16 @@ static netdev_features_t vlan_dev_fix_features(struct
>> net_device *dev,
>> {
>> struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
>> netdev_features_t old_features = features;
>> + netdev_features_t real_dev_features = real_dev->features;
>>
>> - features = netdev_intersect_features(features,
>> real_dev->vlan_features);
>> + features = netdev_intersect_features(features,
>> + (real_dev->vlan_features |
>> + NETIF_F_HW_CSUM));
>
> You might want to change the ordering on all this.
>
> You could start out with a value based on the intersection of
> real_dev->features and real_dev->vlan_features. Then you don't need to
> mess around with this extra piece where you are having OR in HW_CSUM.
You know, I did that and that was the patch I meant to send... I had
3 different versions of this thing trying to find the best way...
Let me repost, since some of the rest of the changes go away.
-vlad
> That way you don't risk losing track of the state of the hardware
> checksum offload in terms of vlan_features as it should completely
> clear all of the checksums if none of them are supported in hardware.
>
>> features |= NETIF_F_RXCSUM;
>
> This line would probably need to be changed to OR NETIF_F_RXCSUM with
> the real_dev->vlan_features when we perform the first intersect test.
> That way we are guaranteed to report RXCSUM if the underlying device
> supports it. It might just be worth while to force this into the
> vlan_features for all devices in register_netdevice() then we wouldn't
> need this line at all and it probably makes sense since it would allow
> us to save a few extra cycles/instructions by combining it with the
> line that was adding high dma support.
>
>> - features = netdev_intersect_features(features, real_dev->features);
>> + if (real_dev_features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
>> + real_dev_features |= NETIF_F_HW_CSUM;
>> +
>> + features = netdev_intersect_features(features, real_dev_features);
>
> This part all looks good.
>
> My only advice like I said would be to record the vlan_features
> intersection first based on the real_dev. That way you don't risk
> losing state data from real device if for some reason it doesn't
> support checksum offload with VLAN tagged frames.
>
>> features |= old_features & (NETIF_F_SOFT_FEATURES |
>> NETIF_F_GSO_SOFTWARE);
>> features |= NETIF_F_LLTX;
>> --
>> 2.7.4
>>
