netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan
@ 2011-03-03 10:55 Xiaotian Feng
  2011-03-03 13:53 ` Ben Hutchings
  2011-03-05 10:36 ` Jiri Pirko
  0 siblings, 2 replies; 6+ messages in thread
From: Xiaotian Feng @ 2011-03-03 10:55 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Xiaotian Feng, David S. Miller, Eric Dumazet,
	Tom Herbert

Consider network topology as follows:

eth0  eth1
 |_____|
    |
  bond0 --- br0
    |
  vlan0 --- br1

bond0 serves for both br0 and vlan0, if a vlan tagged packet was sent
to br1 through bond0, bridge handling code is seeing the packet on bond0
and handing it off to my "legacy" bridge before vlan_tx_tag_present
and vlan_hwaccel_do_receive even haven't a chance to look at it.

Moving the vlan_tx_tag_present before bridge/macvlan handling code could
cure this.

Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Tom Herbert <therbert@google.com>
---
 net/core/dev.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8ae6631..d2d12c2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3079,27 +3079,27 @@ static int __netif_receive_skb(struct sk_buff *skb)
 ncls:
 #endif
 
-	/* Handle special case of bridge or macvlan */
-	rx_handler = rcu_dereference(skb->dev->rx_handler);
-	if (rx_handler) {
+	if (vlan_tx_tag_present(skb)) {
 		if (pt_prev) {
 			ret = deliver_skb(skb, pt_prev, orig_dev);
 			pt_prev = NULL;
 		}
-		skb = rx_handler(skb);
-		if (!skb)
+		if (vlan_hwaccel_do_receive(&skb)) {
+			ret = __netif_receive_skb(skb);
+			goto out;
+		} else if (unlikely(!skb))
 			goto out;
 	}
 
-	if (vlan_tx_tag_present(skb)) {
+	/* Handle special case of bridge or macvlan */
+	rx_handler = rcu_dereference(skb->dev->rx_handler);
+	if (rx_handler) {
 		if (pt_prev) {
 			ret = deliver_skb(skb, pt_prev, orig_dev);
 			pt_prev = NULL;
 		}
-		if (vlan_hwaccel_do_receive(&skb)) {
-			ret = __netif_receive_skb(skb);
-			goto out;
-		} else if (unlikely(!skb))
+		skb = rx_handler(skb);
+		if (!skb)
 			goto out;
 	}
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan
  2011-03-03 10:55 [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan Xiaotian Feng
@ 2011-03-03 13:53 ` Ben Hutchings
  2011-03-03 21:42   ` Nicolas de Pesloüan
  2011-03-05 10:36 ` Jiri Pirko
  1 sibling, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2011-03-03 13:53 UTC (permalink / raw)
  To: Xiaotian Feng
  Cc: netdev, linux-kernel, David S. Miller, Eric Dumazet, Tom Herbert

On Thu, 2011-03-03 at 18:55 +0800, Xiaotian Feng wrote:
> Consider network topology as follows:
> 
> eth0  eth1
>  |_____|
>     |
>   bond0 --- br0
>     |
>   vlan0 --- br1
> 
> bond0 serves for both br0 and vlan0, if a vlan tagged packet was sent
> to br1 through bond0, bridge handling code is seeing the packet on bond0
> and handing it off to my "legacy" bridge before vlan_tx_tag_present
> and vlan_hwaccel_do_receive even haven't a chance to look at it.
[...]

This used to work if the underlying device (bond0 in your example)
implemented VLAN tag extraction, because the VLAN group would be checked
before the bridge.  But it never worked for devices without VLAN tag
extraction.  Perhaps we should just prevent this configuration.

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan
  2011-03-03 13:53 ` Ben Hutchings
@ 2011-03-03 21:42   ` Nicolas de Pesloüan
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolas de Pesloüan @ 2011-03-03 21:42 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Xiaotian Feng, netdev, linux-kernel, David S. Miller,
	Eric Dumazet, Tom Herbert, Jiri Pirko

Le 03/03/2011 14:53, Ben Hutchings a écrit :
> On Thu, 2011-03-03 at 18:55 +0800, Xiaotian Feng wrote:
>> Consider network topology as follows:
>>
>> eth0  eth1
>>   |_____|
>>      |
>>    bond0 --- br0
>>      |
>>    vlan0 --- br1
>>
>> bond0 serves for both br0 and vlan0, if a vlan tagged packet was sent
>> to br1 through bond0, bridge handling code is seeing the packet on bond0
>> and handing it off to my "legacy" bridge before vlan_tx_tag_present
>> and vlan_hwaccel_do_receive even haven't a chance to look at it.
> [...]
>
> This used to work if the underlying device (bond0 in your example)
> implemented VLAN tag extraction, because the VLAN group would be checked
> before the bridge.  But it never worked for devices without VLAN tag
> extraction.  Perhaps we should just prevent this configuration.

If Jiri Pirko eventually move vlan processing to rx_handler, then the setup won't be possible, 
because bond0 would require two rx_handlers : one for bridge (-> br0) and one for vlan (-> vlan0).

Or we need several rx_handlers per net_device, if the above setup is a real one.

	Nicolas.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan
  2011-03-03 10:55 [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan Xiaotian Feng
  2011-03-03 13:53 ` Ben Hutchings
@ 2011-03-05 10:36 ` Jiri Pirko
  2011-03-05 13:53   ` Nicolas de Pesloüan
  1 sibling, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2011-03-05 10:36 UTC (permalink / raw)
  To: Xiaotian Feng
  Cc: netdev, linux-kernel, David S. Miller, Eric Dumazet, Tom Herbert

Thu, Mar 03, 2011 at 11:55:13AM CET, dfeng@redhat.com wrote:
>Consider network topology as follows:
>
>eth0  eth1
> |_____|
>    |
>  bond0 --- br0
>    |
>  vlan0 --- br1
>
>bond0 serves for both br0 and vlan0, if a vlan tagged packet was sent
>to br1 through bond0, bridge handling code is seeing the packet on bond0
>and handing it off to my "legacy" bridge before vlan_tx_tag_present
>and vlan_hwaccel_do_receive even haven't a chance to look at it.
>
>Moving the vlan_tx_tag_present before bridge/macvlan handling code could
>cure this.

Wouldn't this break "eth0 - br0 - br0.5"?

>
>Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
>Cc: "David S. Miller" <davem@davemloft.net>
>Cc: Eric Dumazet <eric.dumazet@gmail.com>
>Cc: Tom Herbert <therbert@google.com>
>---
> net/core/dev.c |   20 ++++++++++----------
> 1 files changed, 10 insertions(+), 10 deletions(-)
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 8ae6631..d2d12c2 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -3079,27 +3079,27 @@ static int __netif_receive_skb(struct sk_buff *skb)
> ncls:
> #endif
> 
>-	/* Handle special case of bridge or macvlan */
>-	rx_handler = rcu_dereference(skb->dev->rx_handler);
>-	if (rx_handler) {
>+	if (vlan_tx_tag_present(skb)) {
> 		if (pt_prev) {
> 			ret = deliver_skb(skb, pt_prev, orig_dev);
> 			pt_prev = NULL;
> 		}
>-		skb = rx_handler(skb);
>-		if (!skb)
>+		if (vlan_hwaccel_do_receive(&skb)) {
>+			ret = __netif_receive_skb(skb);
>+			goto out;
>+		} else if (unlikely(!skb))
> 			goto out;
> 	}
> 
>-	if (vlan_tx_tag_present(skb)) {
>+	/* Handle special case of bridge or macvlan */
>+	rx_handler = rcu_dereference(skb->dev->rx_handler);
>+	if (rx_handler) {
> 		if (pt_prev) {
> 			ret = deliver_skb(skb, pt_prev, orig_dev);
> 			pt_prev = NULL;
> 		}
>-		if (vlan_hwaccel_do_receive(&skb)) {
>-			ret = __netif_receive_skb(skb);
>-			goto out;
>-		} else if (unlikely(!skb))
>+		skb = rx_handler(skb);
>+		if (!skb)
> 			goto out;
> 	}
> 
>-- 
>1.7.1
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan
  2011-03-05 10:36 ` Jiri Pirko
@ 2011-03-05 13:53   ` Nicolas de Pesloüan
  2011-03-05 14:25     ` Jiri Pirko
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolas de Pesloüan @ 2011-03-05 13:53 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Xiaotian Feng, netdev, linux-kernel, David S. Miller,
	Eric Dumazet, Tom Herbert

Le 05/03/2011 11:36, Jiri Pirko a écrit :
> Thu, Mar 03, 2011 at 11:55:13AM CET, dfeng@redhat.com wrote:
>> Consider network topology as follows:
>>
>> eth0  eth1
>> |_____|
>>     |
>>   bond0 --- br0
>>     |
>>   vlan0 --- br1
>>
>> bond0 serves for both br0 and vlan0, if a vlan tagged packet was sent
>> to br1 through bond0, bridge handling code is seeing the packet on bond0
>> and handing it off to my "legacy" bridge before vlan_tx_tag_present
>> and vlan_hwaccel_do_receive even haven't a chance to look at it.
>>
>> Moving the vlan_tx_tag_present before bridge/macvlan handling code could
>> cure this.
>
> Wouldn't this break "eth0 - br0 - br0.5"?

I think it would. One more reason to build a single interface stacking framework...

	Nicolas.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan
  2011-03-05 13:53   ` Nicolas de Pesloüan
@ 2011-03-05 14:25     ` Jiri Pirko
  0 siblings, 0 replies; 6+ messages in thread
From: Jiri Pirko @ 2011-03-05 14:25 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Xiaotian Feng, netdev, linux-kernel, David S. Miller,
	Eric Dumazet, Tom Herbert

Sat, Mar 05, 2011 at 02:53:47PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 11:36, Jiri Pirko a écrit :
>>Thu, Mar 03, 2011 at 11:55:13AM CET, dfeng@redhat.com wrote:
>>>Consider network topology as follows:
>>>
>>>eth0  eth1
>>>|_____|
>>>    |
>>>  bond0 --- br0
>>>    |
>>>  vlan0 --- br1
>>>
>>>bond0 serves for both br0 and vlan0, if a vlan tagged packet was sent
>>>to br1 through bond0, bridge handling code is seeing the packet on bond0
>>>and handing it off to my "legacy" bridge before vlan_tx_tag_present
>>>and vlan_hwaccel_do_receive even haven't a chance to look at it.
>>>
>>>Moving the vlan_tx_tag_present before bridge/macvlan handling code could
>>>cure this.
>>
>>Wouldn't this break "eth0 - br0 - br0.5"?
>
>I think it would. One more reason to build a single interface stacking framework...

I plan to deal with vlans later. This kind of topology would not be
possible after that :)
>
>	Nicolas.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-03-05 14:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-03 10:55 [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan Xiaotian Feng
2011-03-03 13:53 ` Ben Hutchings
2011-03-03 21:42   ` Nicolas de Pesloüan
2011-03-05 10:36 ` Jiri Pirko
2011-03-05 13:53   ` Nicolas de Pesloüan
2011-03-05 14:25     ` Jiri Pirko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).