net: rps: support 802.1Q

net: rps: support 802.1Q

[Changli Gao]

For the 802.1Q packets, if the NIC doesn't support hw-accel-vlan-rx, RPS
won't inspect the internal 4 tuples to generate skb->rxhash, so this kind
of traffic can't get any benefit from RPS.

This patch adds the support for 802.1Q to RPS.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/core/dev.c |    8 ++++++++
 1 file changed, 8 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index ead0366..be7ee50 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2529,6 +2529,7 @@ void __skb_get_rxhash(struct sk_buff *skb)
 	int nhoff, hash = 0, poff;
 	const struct ipv6hdr *ip6;
 	const struct iphdr *ip;
+	const struct vlan_hdr *vlan;
 	u8 ip_proto;
 	u32 addr1, addr2;
 	u16 proto;
@@ -2565,6 +2566,13 @@ again:
 		addr2 = (__force u32) ip6->daddr.s6_addr32[3];
 		nhoff += 40;
 		break;
+	case __constant_htons(ETH_P_8021Q):
+		if (!pskb_may_pull(skb, sizeof(*vlan) + nhoff))
+			goto done;
+		vlan = (const struct vlan_hdr *) (skb->data + nhoff);
+		proto = vlan->h_vlan_encapsulated_proto;
+		nhoff += sizeof(*vlan);
+		goto again;
 	default:
 		goto done;
 	}

Re: net: rps: support 802.1Q

[David Miller]

From: Changli Gao <xiaosuo@gmail.com>
Date: Fri, 19 Aug 2011 13:05:53 +0800

> For the 802.1Q packets, if the NIC doesn't support hw-accel-vlan-rx, RPS
> won't inspect the internal 4 tuples to generate skb->rxhash, so this kind
> of traffic can't get any benefit from RPS.
> 
> This patch adds the support for 802.1Q to RPS.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

Looks fine, applied to net-next, thanks!

Re: net: rps: support 802.1Q

[Changli Gao]

On Fri, Aug 19, 2011 at 1:08 PM, David Miller <davem@davemloft.net> wrote:
> From: Changli Gao <xiaosuo@gmail.com>
> Date: Fri, 19 Aug 2011 13:05:53 +0800
>
>> For the 802.1Q packets, if the NIC doesn't support hw-accel-vlan-rx, RPS
>> won't inspect the internal 4 tuples to generate skb->rxhash, so this kind
>> of traffic can't get any benefit from RPS.
>>
>> This patch adds the support for 802.1Q to RPS.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>
> Looks fine, applied to net-next, thanks!
>

Thanks for applying it. I have another patch which adds the support
for PPPOE session messages. Do you think it worth doing?

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

Re: net: rps: support 802.1Q

[David Miller]

From: Changli Gao <xiaosuo@gmail.com>
Date: Fri, 19 Aug 2011 13:22:03 +0800

> On Fri, Aug 19, 2011 at 1:08 PM, David Miller <davem@davemloft.net> wrote:
>> From: Changli Gao <xiaosuo@gmail.com>
>> Date: Fri, 19 Aug 2011 13:05:53 +0800
>>
>>> For the 802.1Q packets, if the NIC doesn't support hw-accel-vlan-rx, RPS
>>> won't inspect the internal 4 tuples to generate skb->rxhash, so this kind
>>> of traffic can't get any benefit from RPS.
>>>
>>> This patch adds the support for 802.1Q to RPS.
>>>
>>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>>
>> Looks fine, applied to net-next, thanks!
>>
> 
> Thanks for applying it. I have another patch which adds the support
> for PPPOE session messages. Do you think it worth doing?

Sure.

Re: net: rps: support 802.1Q

[Ben Hutchings]

On Fri, 2011-08-19 at 13:05 +0800, Changli Gao wrote:
> For the 802.1Q packets, if the NIC doesn't support hw-accel-vlan-rx, RPS
> won't inspect the internal 4 tuples to generate skb->rxhash, so this kind
> of traffic can't get any benefit from RPS.
> 
> This patch adds the support for 802.1Q to RPS.
[...]
> @@ -2565,6 +2566,13 @@ again:
>  		addr2 = (__force u32) ip6->daddr.s6_addr32[3];
>  		nhoff += 40;
>  		break;
> +	case __constant_htons(ETH_P_8021Q):
> +		if (!pskb_may_pull(skb, sizeof(*vlan) + nhoff))
> +			goto done;
> +		vlan = (const struct vlan_hdr *) (skb->data + nhoff);
> +		proto = vlan->h_vlan_encapsulated_proto;
> +		nhoff += sizeof(*vlan);
> +		goto again;
>  	default:
>  		goto done;
>  	}

Should this really be reading an unlimited number of tags?  What if an
attacker starts sending packets full of VLAN tags?  Since this runs
before netfilter, there would be no way to prevent those packets burning
our CPU time.  And if there are legitimately multiple VLAN tags, they
presumably won't all have the 802.1q Ethertype.

Ben.

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

Re: net: rps: support 802.1Q

[Changli Gao]

On Fri, Aug 19, 2011 at 7:54 PM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> Should this really be reading an unlimited number of tags?

Not unlimited, but it won't stop until reaching the end of the packet.

>  What if an
> attacker starts sending packets full of VLAN tags?  Since this runs
> before netfilter, there would be no way to prevent those packets burning
> our CPU time.  And if there are legitimately multiple VLAN tags, they
> presumably won't all have the 802.1q Ethertype.
>

Do we need to limit the number of rounds to stop this kind of "bad"
packets from burning our CPU time? Then,  __netif_receive_skb() has to
be update too, so the inspection of tunnel in __skb_get_rxhash() does.
Is there a such limitation in xfrm?

Thanks.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

Re: net: rps: support 802.1Q

[Ben Hutchings]

On Fri, 2011-08-19 at 23:05 +0800, Changli Gao wrote:
> On Fri, Aug 19, 2011 at 7:54 PM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > Should this really be reading an unlimited number of tags?
> 
> Not unlimited, but it won't stop until reaching the end of the packet.

Right, I understand that the parsing is properly range-checked against
the length of the packet.

> >  What if an
> > attacker starts sending packets full of VLAN tags?  Since this runs
> > before netfilter, there would be no way to prevent those packets burning
> > our CPU time.  And if there are legitimately multiple VLAN tags, they
> > presumably won't all have the 802.1q Ethertype.
> >
> 
> Do we need to limit the number of rounds to stop this kind of "bad"
> packets from burning our CPU time?

Well, maybe.  Then again, the most effective way for an attacker to
waste a target's CPU time (aside from application-level vulnerabilities)
will often be just to send minimum size packets.

> Then,  __netif_receive_skb() has to
> be update too, so the inspection of tunnel in __skb_get_rxhash() does.

Yes, if we agree this is something worth defending against then we would
need to be consistent in limiting any such parsing loop in pre-netfilter
processing.

> Is there a such limitation in xfrm?

It appears to be limited to 6 levels of encapsulation.

Ben.

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