netdev - Re: [PATCH v2 04/14] vlan: Enable software emulation for vlan accleration.

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]

Date:	Thu, 21 Oct 2010 14:44:26 -0700
From:	Jesse Gross <jesse@...ira.com>
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH v2 04/14] vlan: Enable software emulation for vlan accleration.

On Thu, Oct 21, 2010 at 8:30 AM, Ben Hutchings
<bhutchings@...arflare.com> wrote:
> On Wed, 2010-10-20 at 16:56 -0700, Jesse Gross wrote:
>> Currently users of hardware vlan accleration need to know whether
>> the device supports it before generating packets.  However, vlan
>> acceleration will soon be available in a more flexible manner so
>> knowing ahead of time becomes much more difficult.  This adds
>> a software fallback path for vlan packets on devices without the
>> necessary offloading support, similar to other types of hardware
>> accleration.
> [...]
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 4c3ac53..1bfd96b 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1694,7 +1694,12 @@ static bool can_checksum_protocol(unsigned long features, __be16 protocol)
>>
>>  static bool dev_can_checksum(struct net_device *dev, struct sk_buff *skb)
>>  {
>> -     if (can_checksum_protocol(dev->features, skb->protocol))
>> +     int features = dev->features;
>> +
>> +     if (vlan_tx_tag_present(skb))
>> +             features &= dev->vlan_features;
>> +
>> +     if (can_checksum_protocol(features, skb->protocol))
>>               return true;
>>
>>       if (skb->protocol == htons(ETH_P_8021Q)) {
> [...]
>
> Additional context:
>
>                struct vlan_ethhdr *veh = (struct vlan_ethhdr *)skb->data;
>                if (can_checksum_protocol(dev->features & dev->vlan_features,
>                                          veh->h_vlan_encapsulated_proto))
>                        return true;
>        }
>
>        return false;
> }
>
> I don't think this will do the right thing if the NIC does VLAN tag
> insertion and checksum offload with only one layer of VLAN
> encapsulation, but the skb has two layers of VLAN encapsulation.
>
> I think we actually want something like:
>
> static bool dev_can_checksum(struct net_device *dev, struct sk_buff *skb)
> {
>        __be16 protocol = skb->protocol;
>        int features = dev->features;
>
>        if (vlan_tx_tag_present(skb)) {
>                features &= dev->vlan_features;
>        } else if (skb->protocol == htons(ETH_P_8021Q)) {
>                struct vlan_ethhdr *veh = (struct vlan_ethhdr *)skb->data;
>                protocol = veh->h_vlan_encapsulated_proto;
>                features &= dev->vlan_features;
>        }
>
>        return can_checksum_protocol(features, protocol);
> }
>
> Does that look right?

Thanks, good catch.  Yes, that looks right.  Will you submit a patch
to fix this?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html