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]

Message-ID: <1287675008.2235.8.camel@achroite.uk.solarflarecom.com>
Date:	Thu, 21 Oct 2010 16:30:08 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Jesse Gross <jesse@...ira.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 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?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
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