Re: [net-next PATCH] net: allow vlan traffic to be received under bond

Tue, Oct 11, 2011 at 12:37:53AM CEST, jpirko@redhat.com wrote:

Mon, Oct 10, 2011 at 09:16:41PM CEST, john.r.fastabend@intel.com wrote:

quoted

The following configuration used to work as I expected. At least
we could use the fcoe interfaces to do MPIO and the bond0 iface
to do load balancing or failover.

      ---eth2.228-fcoe
      |
eth2 -----|
         |
         |---- bond0
         |
eth3 -----|
      |
      ---eth3.228-fcoe

This worked because of a change we added to allow inactive slaves
to rx 'exact' matches. This functionality was kept intact with the
rx_handler mechanism. However now the vlan interface attached to the
active slave never receives traffic because the bonding rx_handler
updates the skb->dev and goto's another_round. Previously, the
vlan_do_receive() logic was called before the bonding rx_handler.

Now by the time vlan_do_receive calls vlan_find_dev() the
skb->dev is set to bond0 and it is clear no vlan is attached
to this iface. The vlan lookup fails.

This patch moves the VLAN check above the rx_handler. A VLAN
tagged frame is now routed to the eth2.228-fcoe iface in the
above schematic. Untagged frames continue to the bond0 as
normal. This case also remains intact,

eth2 --> bond0 --> vlan.228

Here the skb is VLAN tagged but the vlan lookup fails on eth2
causing the bonding rx_handler to be called. On the second
pass the vlan lookup is on the bond0 iface and completes as
expected.

Putting a VLAN.228 on both the bond0 and eth2 device will
result in eth2.228 receiving the skb. I don't think this is
completely unexpected and was the result prior to the rx_handler
result.

Note, the same setup is also used for other storage traffic that
MPIO is used with eg. iSCSI and similar setups can be contrived
without storage protocols.

Signed-off-by: John Fastabend <redacted>
---

net/core/dev.c |   22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 70ecb86..8b6118a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -3231,6 +3231,17 @@ another_round:

ncls:
#endif

+	if (vlan_tx_tag_present(skb)) {
+		if (pt_prev) {
+			ret = deliver_skb(skb, pt_prev, orig_dev);
+			pt_prev = NULL;
+		}
+		if (vlan_do_receive(&skb))
+			goto another_round;
+		else if (unlikely(!skb))
+			goto out;
+	}
+
	rx_handler = rcu_dereference(skb->dev->rx_handler);
	if (rx_handler) {
		if (pt_prev) {

@@ -3251,17 +3262,6 @@ ncls:

		}
	}

-	if (vlan_tx_tag_present(skb)) {
-		if (pt_prev) {
-			ret = deliver_skb(skb, pt_prev, orig_dev);
-			pt_prev = NULL;
-		}
-		if (vlan_do_receive(&skb))
-			goto another_round;
-		else if (unlikely(!skb))
-			goto out;
-	}
-
	/* deliver only exact match when indicated */
	null_or_dev = deliver_exact ? skb->dev : NULL;

Hmm, I must look at this again tomorrow but I have strong feeling this
will break some some scenario including vlan-bridge-macvlan.

I didn't find out anything.

Reviewed-by: Jiri Pirko <redacted>