[PATCH RFC 0/3] bridge: 802.1ad vlan protocol support

[PATCH RFC 0/3] bridge: 802.1ad vlan protocol support

[Toshiaki Makita]

Currently bridge vlan filtering doesn't work fine with 802.1ad protocol.
Only if a bridge is configured without pvid and the bridge receives only
802.1ad single tagged frames, it will work.
Otherwise:
- If pvid is configured, it can put only 802.1Q tags but cannot put 802.1ad
  tags.
- If 802.1Q and 802.1ad tagged frames arrive in mixture, it applies filtering
  regardless of their protocols.
- If 802.1Q is used as inner vlan tag (second vlan tag in stacked vlan),
  not only outer tag but also inner tag will be stripped on an untagged port.
Thus, we can't properly handle frames once 802.1ad is used.

Handling 802.1ad is useful if we want to allow stacked vlans to be used,
e.g., guest VMs wants to use vlan tags and the host also wants to segregate
guest's traffic from other guests' by vlan tags.

Here is the image describing how to configure a bridge to filter VMs traffic.

         +-------+p/u   +-----+  +---------+
 +----+  |       |------|vnet0|--|User A VM|
 |eth0|--|802.1ad|      +-----+  +---------+
 +----+  |bridge |p/u   +-----+  +---------+
         |       |------|vnet1|--|User B VM|
         +-------+      +-----+  +---------+
p/u: pvid/untagged

This patch set enables us to set vlan protocols per bridge.
This tries to implement a bridge like S-VLAN component in IEEE 802.1Q-2011
spec.

Note that there is another possible implementation that sets vlan protocols
per port. Some HW switches seem to take that approach.
However, I think per-bridge approach is better, because;
- I think the typical usage is segregating 802.1Q tagged traffic (like what
  is described above), and this doesn't need the ability to be set protocols
  per port. Also, If a bridge has many ports and it supports per-port
  setting, we might have to make much more extra configurations to change
  protocols of all ports, in this case.

- I assume that the main perpose to set protocol per port is to assign S-VID
  according to C-VID, or to realize two logical bridges (one is an 802.1Q
  filtering bridge and another is an 802.1ad filtering bridge) in one bridge.
  The former usually needs additional features such as vlan id mapping, and
  is likely to be complicated. If a user wants, such enhanced features can be
  accomplished by a combination of multiple bridges, so it is not absolutely
  necessary to implement these features in a bridge itself.
  The latter is simply unnecessary because we can easily make two bridges
  of which one is an 802.1Q bridge and another is an 802.1ad bridge.

Here is an example of the enhanced feature that we can realize by using
multiple bridges and veth interfaces. This way is documented in
IEEE 802.1Q-2011 clause 15.4 (C-tagged service interface).

 +----+  +-------+p/u         +------+  +----+  +--+
 |eth0|--|802.1ad|----veth----|802.1Q|--|vnet|--|VM|
 +----+  |bridge |----veth----|bridge|  +----+  +--+
         +-------+p/u         +------+
p/u: pvid/untagged

In this configuration, we can map C-VIDs to any S-VID.
For example;
 C-VID 10 and 20 to S-VID 100
 C-VID 30 to S-VID 110
This is achieved through the 802.1Q bridge that forwards C-tagged frames to
proper ports of the 802.1ad bridge.

As this patch set is RFC, this includes a patch which is not appropriate for
net-next (patch 1). If this approach is acceptable, I will submit each patch
to appropriate tree. In this version, all patches except iproute2 (patch 4)
are based on net-next.

Toshiaki Makita (3):
  bridge: Fix handling stacked vlan tags
  bridge: Prepare for 802.1ad vlan filtering support
  bridge: Support 802.1ad vlan filtering

 include/uapi/linux/if_bridge.h |   2 +
 net/bridge/br_device.c         |   1 +
 net/bridge/br_input.c          |   2 +-
 net/bridge/br_netlink.c        |  67 ++++++++++++++-------
 net/bridge/br_private.h        |  30 +++++++++-
 net/bridge/br_vlan.c           | 128 ++++++++++++++++++++++++++++++-----------
 6 files changed, 170 insertions(+), 60 deletions(-)

-- 
1.8.1.2

[PATCH RFC 1/3] bridge: Fix handling stacked vlan tags

[Toshiaki Makita]

If a bridge with vlan_filtering enabled receives frames with stacked
vlan tags, i.e., they have two vlan tags, br_vlan_untag() strips not
only the outer tag but also the inner tag.

br_vlan_untag() is called only from br_handle_vlan(), and in this case,
it is enough to set skb->vlan_tci to 0 here, because vlan_tci should have
been set before calling br_handle_vlan() (by __netif_receive_skb_core() or
br_allowed_ingress()).

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_vlan.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 8249ca7..cda93e2 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -119,22 +119,6 @@ static void __vlan_flush(struct net_port_vlans *v)
 	kfree_rcu(v, rcu);
 }
 
-/* Strip the tag from the packet.  Will return skb with tci set 0.  */
-static struct sk_buff *br_vlan_untag(struct sk_buff *skb)
-{
-	if (skb->protocol != htons(ETH_P_8021Q)) {
-		skb->vlan_tci = 0;
-		return skb;
-	}
-
-	skb->vlan_tci = 0;
-	skb = vlan_untag(skb);
-	if (skb)
-		skb->vlan_tci = 0;
-
-	return skb;
-}
-
 struct sk_buff *br_handle_vlan(struct net_bridge *br,
 			       const struct net_port_vlans *pv,
 			       struct sk_buff *skb)
@@ -150,7 +134,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
 	 */
 	br_vlan_get_tag(skb, &vid);
 	if (test_bit(vid, pv->untagged_bitmap))
-		skb = br_vlan_untag(skb);
+		skb->vlan_tci = 0;
 
 out:
 	return skb;
-- 
1.8.1.2

[PATCH RFC 2/3] bridge: Prepare for 802.1ad vlan filtering support

[Toshiaki Makita]

This enables a bridge to have vlan protocol informantion and allows vlan
filtering code to take vlan protocols into account.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_device.c  |  1 +
 net/bridge/br_input.c   |  2 +-
 net/bridge/br_private.h | 24 +++++++++++++++++++---
 net/bridge/br_vlan.c    | 53 ++++++++++++++++++++++++++++++++-----------------
 4 files changed, 58 insertions(+), 22 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index b063050..6cf2fbc 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -381,4 +381,5 @@ void br_dev_setup(struct net_device *dev)
 	br_netfilter_rtable_init(br);
 	br_stp_timer_init(br);
 	br_multicast_init(br);
+	br_vlan_init(br);
 }
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 28d5446..8b69da6 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -146,7 +146,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
 	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
 	u16 vid = 0;
 
-	br_vlan_get_tag(skb, &vid);
+	br_vlan_get_tag(skb, br_get_vlan_proto(p->br), &vid);
 	if (p->flags & BR_LEARNING)
 		br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, false);
 	return 0;	 /* process further */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index e1ca1dc..500cf0e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -292,6 +292,7 @@ struct net_bridge
 	struct kobject			*ifobj;
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	u8				vlan_enabled;
+	__be16				vlan_proto;
 	struct net_port_vlans __rcu	*vlan_info;
 #endif
 };
@@ -589,6 +590,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid);
 void br_vlan_flush(struct net_bridge *br);
 bool br_vlan_find(struct net_bridge *br, u16 vid);
 int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
+void br_vlan_init(struct net_bridge *br);
 int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags);
 int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
 void nbp_vlan_flush(struct net_bridge_port *port);
@@ -609,11 +611,12 @@ static inline struct net_port_vlans *nbp_get_vlan_info(
 /* Since bridge now depends on 8021Q module, but the time bridge sees the
  * skb, the vlan tag will always be present if the frame was tagged.
  */
-static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid)
+static inline int br_vlan_get_tag(const struct sk_buff *skb, __be16 proto,
+				  u16 *vid)
 {
 	int err = 0;
 
-	if (vlan_tx_tag_present(skb))
+	if (vlan_tx_tag_present(skb) && skb->vlan_proto == proto)
 		*vid = vlan_tx_tag_get(skb) & VLAN_VID_MASK;
 	else {
 		*vid = 0;
@@ -632,6 +635,11 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
 	return v->pvid ?: VLAN_N_VID;
 }
 
+static inline __be16 br_get_vlan_proto(const struct net_bridge *br)
+{
+	return br->vlan_proto;
+}
+
 #else
 static inline bool br_allowed_ingress(struct net_bridge *br,
 				      struct net_port_vlans *v,
@@ -674,6 +682,10 @@ static inline bool br_vlan_find(struct net_bridge *br, u16 vid)
 	return false;
 }
 
+static inline void br_vlan_init(struct net_bridge *br)
+{
+}
+
 static inline int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
 {
 	return -EOPNOTSUPP;
@@ -704,7 +716,8 @@ static inline bool nbp_vlan_find(struct net_bridge_port *port, u16 vid)
 	return false;
 }
 
-static inline u16 br_vlan_get_tag(const struct sk_buff *skb, u16 *tag)
+static inline u16 br_vlan_get_tag(const struct sk_buff *skb, __be16 proto,
+				  u16 *tag)
 {
 	return 0;
 }
@@ -712,6 +725,11 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
 {
 	return VLAN_N_VID;	/* Returns invalid vid */
 }
+
+static inline __be16 br_get_vlan_proto(const struct net_bridge *br)
+{
+	return 0;
+}
 #endif
 
 /* br_netfilter.c */
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index cda93e2..f05ef6a 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -32,7 +32,7 @@ static void __vlan_add_flags(struct net_port_vlans *v, u16 vid, u16 flags)
 		set_bit(vid, v->untagged_bitmap);
 }
 
-static int __vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
+static int __vlan_add(struct net_port_vlans *v, __be16 proto, u16 vid, u16 flags)
 {
 	struct net_bridge_port *p = NULL;
 	struct net_bridge *br;
@@ -60,7 +60,7 @@ static int __vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
 		 * that ever changes this code will allow tagged
 		 * traffic to enter the bridge.
 		 */
-		err = vlan_vid_add(dev, htons(ETH_P_8021Q), vid);
+		err = vlan_vid_add(dev, proto, vid);
 		if (err)
 			return err;
 	}
@@ -80,11 +80,11 @@ static int __vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
 
 out_filt:
 	if (p)
-		vlan_vid_del(dev, htons(ETH_P_8021Q), vid);
+		vlan_vid_del(dev, proto, vid);
 	return err;
 }
 
-static int __vlan_del(struct net_port_vlans *v, u16 vid)
+static int __vlan_del(struct net_port_vlans *v, __be16 proto, u16 vid)
 {
 	if (!test_bit(vid, v->vlan_bitmap))
 		return -EINVAL;
@@ -93,7 +93,7 @@ static int __vlan_del(struct net_port_vlans *v, u16 vid)
 	clear_bit(vid, v->untagged_bitmap);
 
 	if (v->port_idx)
-		vlan_vid_del(v->parent.port->dev, htons(ETH_P_8021Q), vid);
+		vlan_vid_del(v->parent.port->dev, proto, vid);
 
 	clear_bit(vid, v->vlan_bitmap);
 	v->num_vlans--;
@@ -132,7 +132,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
 	 * a valid vlan id.  If the vlan id is set in the untagged bitmap,
 	 * send untagged; otherwise, send tagged.
 	 */
-	br_vlan_get_tag(skb, &vid);
+	br_vlan_get_tag(skb, br_get_vlan_proto(br), &vid);
 	if (test_bit(vid, pv->untagged_bitmap))
 		skb->vlan_tci = 0;
 
@@ -145,6 +145,7 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 			struct sk_buff *skb, u16 *vid)
 {
 	int err;
+	__be16 proto = br_get_vlan_proto(br);
 
 	/* If VLAN filtering is disabled on the bridge, all packets are
 	 * permitted.
@@ -158,7 +159,7 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 	if (!v)
 		return false;
 
-	err = br_vlan_get_tag(skb, vid);
+	err = br_vlan_get_tag(skb, proto, vid);
 	if (!*vid) {
 		u16 pvid = br_get_pvid(v);
 
@@ -173,16 +174,27 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 		 * ingress frame is considered to belong to this vlan.
 		 */
 		*vid = pvid;
-		if (likely(err))
+		if (likely(err)) {
 			/* Untagged Frame. */
-			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
-		else
+			if (vlan_tx_tag_present(skb)) {
+				/* skb->vlan_proto was different from br->vlan_proto */
+				skb_push(skb, ETH_HLEN);
+				skb = __vlan_put_tag(skb, skb->vlan_proto,
+						     vlan_tx_tag_get(skb));
+				if (unlikely(!skb))
+					return false;
+				skb_pull(skb, ETH_HLEN);
+				skb_reset_mac_len(skb);
+			}
+			__vlan_hwaccel_put_tag(skb, proto, pvid);
+		} else {
 			/* Priority-tagged Frame.
 			 * At this point, We know that skb->vlan_tci had
 			 * VLAN_TAG_PRESENT bit and its VID field was 0x000.
 			 * We update only VID field and preserve PCP field.
 			 */
 			skb->vlan_tci |= pvid;
+		}
 
 		return true;
 	}
@@ -207,7 +219,7 @@ bool br_allowed_egress(struct net_bridge *br,
 	if (!v)
 		return false;
 
-	br_vlan_get_tag(skb, &vid);
+	br_vlan_get_tag(skb, br_get_vlan_proto(br), &vid);
 	if (test_bit(vid, v->vlan_bitmap))
 		return true;
 
@@ -226,7 +238,7 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
 
 	pv = rtnl_dereference(br->vlan_info);
 	if (pv)
-		return __vlan_add(pv, vid, flags);
+		return __vlan_add(pv, br_get_vlan_proto(br), vid, flags);
 
 	/* Create port vlan infomration
 	 */
@@ -235,7 +247,7 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
 		return -ENOMEM;
 
 	pv->parent.br = br;
-	err = __vlan_add(pv, vid, flags);
+	err = __vlan_add(pv, br_get_vlan_proto(br), vid, flags);
 	if (err)
 		goto out;
 
@@ -261,7 +273,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid)
 
 	br_fdb_find_delete_local(br, NULL, br->dev->dev_addr, vid);
 
-	__vlan_del(pv, vid);
+	__vlan_del(pv, br_get_vlan_proto(br), vid);
 	return 0;
 }
 
@@ -311,6 +323,11 @@ unlock:
 	return 0;
 }
 
+void br_vlan_init(struct net_bridge *br)
+{
+	br->vlan_proto = htons(ETH_P_8021Q);
+}
+
 /* Must be protected by RTNL.
  * Must be called with vid in range from 1 to 4094 inclusive.
  */
@@ -323,7 +340,7 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
 
 	pv = rtnl_dereference(port->vlan_info);
 	if (pv)
-		return __vlan_add(pv, vid, flags);
+		return __vlan_add(pv, br_get_vlan_proto(port->br), vid, flags);
 
 	/* Create port vlan infomration
 	 */
@@ -335,7 +352,7 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
 
 	pv->port_idx = port->port_no;
 	pv->parent.port = port;
-	err = __vlan_add(pv, vid, flags);
+	err = __vlan_add(pv, br_get_vlan_proto(port->br), vid, flags);
 	if (err)
 		goto clean_up;
 
@@ -362,7 +379,7 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
 
 	br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid);
 
-	return __vlan_del(pv, vid);
+	return __vlan_del(pv, br_get_vlan_proto(port->br), vid);
 }
 
 void nbp_vlan_flush(struct net_bridge_port *port)
@@ -377,7 +394,7 @@ void nbp_vlan_flush(struct net_bridge_port *port)
 		return;
 
 	for_each_set_bit(vid, pv->vlan_bitmap, VLAN_N_VID)
-		vlan_vid_del(port->dev, htons(ETH_P_8021Q), vid);
+		vlan_vid_del(port->dev, br_get_vlan_proto(port->br), vid);
 
 	__vlan_flush(pv);
 }
-- 
1.8.1.2

[PATCH RFC 3/3] bridge: Support 802.1ad vlan filtering

[Toshiaki Makita]

This enables us to change protocols for vlan_filtering.
We come to be able to filter frames based on 802.1ad vlan tags through
a bridge.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 include/uapi/linux/if_bridge.h |  2 ++
 net/bridge/br_netlink.c        | 67 +++++++++++++++++++++++++++++-------------
 net/bridge/br_private.h        |  6 ++++
 net/bridge/br_vlan.c           | 57 +++++++++++++++++++++++++++++++++++
 4 files changed, 111 insertions(+), 21 deletions(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 39f621a..cb31364 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -110,12 +110,14 @@ struct __fdb_entry {
  *     [IFLA_BRIDGE_FLAGS]
  *     [IFLA_BRIDGE_MODE]
  *     [IFLA_BRIDGE_VLAN_INFO]
+ *     [IFLA_BRIDGE_VLAN_PROTOCOL]
  * }
  */
 enum {
 	IFLA_BRIDGE_FLAGS,
 	IFLA_BRIDGE_MODE,
 	IFLA_BRIDGE_VLAN_INFO,
+	IFLA_BRIDGE_VLAN_PROTOCOL,
 	__IFLA_BRIDGE_MAX,
 };
 #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index e74b6d53..eac82a4 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -119,7 +119,7 @@ static int br_fill_ifinfo(struct sk_buff *skb,
 		nla_nest_end(skb, nest);
 	}
 
-	/* Check if  the VID information is requested */
+	/* Check if the vlan information is requested */
 	if (filter_mask & RTEXT_FILTER_BRVLAN) {
 		struct nlattr *af;
 		const struct net_port_vlans *pv;
@@ -127,17 +127,27 @@ static int br_fill_ifinfo(struct sk_buff *skb,
 		u16 vid;
 		u16 pvid;
 
-		if (port)
+		if (port) {
 			pv = nbp_get_vlan_info(port);
-		else
-			pv = br_get_vlan_info(br);
+			if (!pv || bitmap_empty(pv->vlan_bitmap, VLAN_N_VID))
+				goto done;
 
-		if (!pv || bitmap_empty(pv->vlan_bitmap, VLAN_N_VID))
-			goto done;
+			af = nla_nest_start(skb, IFLA_AF_SPEC);
+			if (!af)
+				goto nla_put_failure;
+		} else {
+			af = nla_nest_start(skb, IFLA_AF_SPEC);
+			if (!af)
+				goto nla_put_failure;
 
-		af = nla_nest_start(skb, IFLA_AF_SPEC);
-		if (!af)
-			goto nla_put_failure;
+			if (nla_put_be16(skb, IFLA_BRIDGE_VLAN_PROTOCOL,
+					 br_get_vlan_proto(br)))
+				goto nla_put_failure;
+
+			pv = br_get_vlan_info(br);
+			if (!pv || bitmap_empty(pv->vlan_bitmap, VLAN_N_VID))
+				goto afspec_nest_end;
+		}
 
 		pvid = br_get_pvid(pv);
 		for_each_set_bit(vid, pv->vlan_bitmap, VLAN_N_VID) {
@@ -153,7 +163,7 @@ static int br_fill_ifinfo(struct sk_buff *skb,
 				    sizeof(vinfo), &vinfo))
 				goto nla_put_failure;
 		}
-
+afspec_nest_end:
 		nla_nest_end(skb, af);
 	}
 
@@ -223,6 +233,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
 	[IFLA_BRIDGE_MODE]	= { .type = NLA_U16 },
 	[IFLA_BRIDGE_VLAN_INFO]	= { .type = NLA_BINARY,
 				    .len = sizeof(struct bridge_vlan_info), },
+	[IFLA_BRIDGE_VLAN_PROTOCOL] = { .type = NLA_U16 },
 };
 
 static int br_afspec(struct net_bridge *br,
@@ -250,7 +261,7 @@ static int br_afspec(struct net_bridge *br,
 			if (p) {
 				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
 				if (err)
-					break;
+					return err;
 
 				if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
 					err = br_vlan_add(p->br, vinfo->vid,
@@ -259,7 +270,7 @@ static int br_afspec(struct net_bridge *br,
 				err = br_vlan_add(br, vinfo->vid, vinfo->flags);
 
 			if (err)
-				break;
+				return err;
 
 			break;
 
@@ -274,6 +285,16 @@ static int br_afspec(struct net_bridge *br,
 		}
 	}
 
+	if (tb[IFLA_BRIDGE_VLAN_PROTOCOL]) {
+		__be16 proto = nla_get_be16(tb[IFLA_BRIDGE_VLAN_PROTOCOL]);
+
+		if (cmd != RTM_SETLINK || p ||
+		    (proto != htons(ETH_P_8021Q) && proto != htons(ETH_P_8021AD)))
+			return -EINVAL;
+
+		err = br_set_vlan_proto(br, proto);
+	}
+
 	return err;
 }
 
@@ -447,20 +468,24 @@ static int br_validate(struct nlattr *tb[], struct nlattr *data[])
 
 static size_t br_get_link_af_size(const struct net_device *dev)
 {
-	struct net_port_vlans *pv;
+	struct net_port_vlans *pv = NULL;
+	size_t tot = 0;
 
-	if (br_port_exists(dev))
+	if (br_port_exists(dev)) {
 		pv = nbp_get_vlan_info(br_port_get_rtnl(dev));
-	else if (dev->priv_flags & IFF_EBRIDGE)
+	} else if (dev->priv_flags & IFF_EBRIDGE) {
+		tot = nla_total_size(2); /* IFLA_BRIDGE_VLAN_PROTOCOL */
 		pv = br_get_vlan_info((struct net_bridge *)netdev_priv(dev));
-	else
-		return 0;
+	}
 
-	if (!pv)
-		return 0;
+	if (pv)
+		/* IFLA_BRIDGE_VLAN_INFO
+		 * Each VLAN is returned in bridge_vlan_info along with flags.
+		 */
+		tot += pv->num_vlans *
+		       nla_total_size(sizeof(struct bridge_vlan_info));
 
-	/* Each VLAN is returned in bridge_vlan_info along with flags */
-	return pv->num_vlans * nla_total_size(sizeof(struct bridge_vlan_info));
+	return tot;
 }
 
 static struct rtnl_af_ops br_af_ops = {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 500cf0e..d83843a 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -591,6 +591,7 @@ void br_vlan_flush(struct net_bridge *br);
 bool br_vlan_find(struct net_bridge *br, u16 vid);
 int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
 void br_vlan_init(struct net_bridge *br);
+int br_set_vlan_proto(struct net_bridge *br, __be16 proto);
 int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags);
 int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
 void nbp_vlan_flush(struct net_bridge_port *port);
@@ -730,6 +731,11 @@ static inline __be16 br_get_vlan_proto(const struct net_bridge *br)
 {
 	return 0;
 }
+
+static inline int br_set_vlan_proto(struct net_bridge *br, __be16 proto)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 /* br_netfilter.c */
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f05ef6a..b465542 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -328,6 +328,63 @@ void br_vlan_init(struct net_bridge *br)
 	br->vlan_proto = htons(ETH_P_8021Q);
 }
 
+int br_set_vlan_proto(struct net_bridge *br, __be16 proto)
+{
+	int err = 0;
+	struct net_bridge_port *p;
+	struct net_port_vlans *pv;
+	__be16 oldproto = br_get_vlan_proto(br);
+	u16 vid, errvid;
+
+	ASSERT_RTNL();
+
+	if (oldproto == proto)
+		return 0;
+
+	/* Add VLANs for the new proto to the device filter. */
+	list_for_each_entry(p, &br->port_list, list) {
+		pv = rtnl_dereference(p->vlan_info);
+		if (!pv)
+			continue;
+
+		for_each_set_bit(vid, pv->vlan_bitmap, VLAN_N_VID) {
+			err = vlan_vid_add(p->dev, proto, vid);
+			if (err)
+				goto out_filt;
+		}
+	}
+
+	br->vlan_proto = proto;
+
+	/* Delete VLANs for the old proto from the device filter. */
+	list_for_each_entry(p, &br->port_list, list) {
+		pv = rtnl_dereference(p->vlan_info);
+		if (!pv)
+			continue;
+
+		for_each_set_bit(vid, pv->vlan_bitmap, VLAN_N_VID)
+			vlan_vid_del(p->dev, oldproto, vid);
+	}
+
+	return 0;
+
+out_filt:
+	errvid = vid;
+	for_each_set_bit(vid, pv->vlan_bitmap, errvid)
+		vlan_vid_del(p->dev, proto, vid);
+
+	list_for_each_entry_continue_reverse(p, &br->port_list, list) {
+		pv = rtnl_dereference(p->vlan_info);
+		if (!pv)
+			continue;
+
+		for_each_set_bit(vid, pv->vlan_bitmap, VLAN_N_VID)
+			vlan_vid_del(p->dev, proto, vid);
+	}
+
+	return err;
+}
+
 /* Must be protected by RTNL.
  * Must be called with vid in range from 1 to 4094 inclusive.
  */
-- 
1.8.1.2

[PATCH RFC iproute2] bridge: Add 802.1ad vlan support

[Toshiaki Makita]

Sample configuration:
  # ip link add br0 type bridge
  # ip link set eth0 master br0
  # ip link set vnet0 master br0
  # bridge vlan set protocol 802.1ad dev br0
  # bridge vlan add vid 10 dev vnet0 pvid untagged
  # bridge vlan add vid 10 dev eth0
  # ip link set br0 up

Now eth0 can communicate with VM behind vnet0 using 802.1ad tag with vid 10.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 bridge/vlan.c             | 156 ++++++++++++++++++++++++++++++++++++++++++----
 include/linux/if_bridge.h |   2 +
 2 files changed, 146 insertions(+), 12 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 83c4088..4b36d8f 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -12,6 +12,7 @@
 #include "libnetlink.h"
 #include "br_common.h"
 #include "utils.h"
+#include "rt_names.h"
 
 int filter_index;
 
@@ -19,7 +20,10 @@ static void usage(void)
 {
 	fprintf(stderr, "Usage: bridge vlan { add | del } vid VLAN_ID dev DEV [ pvid] [ untagged ]\n");
 	fprintf(stderr, "                                                     [ self ] [ master ]\n");
-	fprintf(stderr, "       bridge vlan { show } [ dev DEV ]\n");
+	fprintf(stderr, "       bridge vlan set protocol VLANPROTO dev DEV\n");
+	fprintf(stderr, "       bridge vlan { show } [ dev DEV ] [ protocol ]\n");
+	fprintf(stderr, "\n");
+	fprintf(stderr, "VLANPROTO: { 802.1Q / 802.1ad }\n");
 	exit(-1);
 }
 
@@ -101,6 +105,64 @@ static int vlan_modify(int cmd, int argc, char **argv)
 	return 0;
 }
 
+static int vlan_set(int argc, char **argv)
+{
+	struct {
+		struct nlmsghdr		n;
+		struct ifinfomsg	ifm;
+		char			buf[1024];
+	} req;
+	char *d = NULL;
+	struct rtattr *afspec;
+	__u16 proto = 0;
+
+	memset(&req, 0, sizeof(req));
+
+	req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
+	req.n.nlmsg_flags = NLM_F_REQUEST;
+	req.n.nlmsg_type = RTM_SETLINK;
+	req.ifm.ifi_family = PF_BRIDGE;
+
+	while (argc > 0) {
+		if (strcmp(*argv, "dev") == 0) {
+			NEXT_ARG();
+			d = *argv;
+		} else if (strcmp(*argv, "protocol") == 0) {
+			NEXT_ARG();
+			if (ll_proto_a2n(&proto, *argv))
+				invarg("protocol is invalid", *argv);
+		} else {
+			if (matches(*argv, "help") == 0)
+				NEXT_ARG();
+		}
+		argc--; argv++;
+	}
+
+	if (d == NULL || !proto) {
+		fprintf(stderr, "Device and protocol are required arguments.\n");
+		return -1;
+	}
+
+	req.ifm.ifi_index = ll_name_to_index(d);
+	if (req.ifm.ifi_index == 0) {
+		fprintf(stderr, "Cannot find bridge device \"%s\"\n", d);
+		return -1;
+	}
+
+	afspec = addattr_nest(&req.n, sizeof(req), IFLA_AF_SPEC);
+
+	addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, BRIDGE_FLAGS_SELF);
+
+	addattr16(&req.n, sizeof(req), IFLA_BRIDGE_VLAN_PROTOCOL, proto);
+
+	addattr_nest_end(&req.n, afspec);
+
+	if (rtnl_talk(&rth, &req.n, 0, 0, NULL) < 0)
+		exit(2);
+
+	return 0;
+}
+
 static int print_vlan(const struct sockaddr_nl *who,
 		      struct nlmsghdr *n,
 		      void *arg)
@@ -109,6 +171,7 @@ static int print_vlan(const struct sockaddr_nl *who,
 	struct ifinfomsg *ifm = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	struct rtattr * tb[IFLA_MAX+1];
+	int found = 0;
 
 	if (n->nlmsg_type != RTM_NEWLINK) {
 		fprintf(stderr, "Not RTM_NEWLINK: %08x %08x %08x\n",
@@ -130,21 +193,21 @@ static int print_vlan(const struct sockaddr_nl *who,
 
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifm), len);
 
-	/* if AF_SPEC isn't there, vlan table is not preset for this port */
-	if (!tb[IFLA_AF_SPEC]) {
-		fprintf(fp, "%s\tNone\n", ll_index_to_name(ifm->ifi_index));
-		return 0;
-	} else {
+	if (tb[IFLA_AF_SPEC]) {
 		struct rtattr *i, *list = tb[IFLA_AF_SPEC];
 		int rem = RTA_PAYLOAD(list);
 
-		fprintf(fp, "%s", ll_index_to_name(ifm->ifi_index));
 		for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
 			struct bridge_vlan_info *vinfo;
 
 			if (i->rta_type != IFLA_BRIDGE_VLAN_INFO)
 				continue;
 
+			if (!found) {
+				fprintf(fp, "%s",
+					ll_index_to_name(ifm->ifi_index));
+				found = 1;
+			}
 			vinfo = RTA_DATA(i);
 			fprintf(fp, "\t %hu", vinfo->vid);
 			if (vinfo->flags & BRIDGE_VLAN_INFO_PVID)
@@ -154,7 +217,63 @@ static int print_vlan(const struct sockaddr_nl *who,
 			fprintf(fp, "\n");
 		}
 	}
-	fprintf(fp, "\n");
+
+	/* if IFLA_BRIDGE_VLAN_INFO isn't there, vlan table is not preset for
+	 * this port.
+	 */
+	if (!found)
+		fprintf(fp, "%s\tNone\n", ll_index_to_name(ifm->ifi_index));
+
+	fflush(fp);
+	return 0;
+}
+
+static int print_vlan_proto(const struct sockaddr_nl *who,
+			    struct nlmsghdr *n, void *arg)
+{
+	FILE *fp = arg;
+	struct ifinfomsg *ifm = NLMSG_DATA(n);
+	int len = n->nlmsg_len;
+	struct rtattr *tb[IFLA_MAX+1];
+
+	if (n->nlmsg_type != RTM_NEWLINK) {
+		fprintf(stderr, "Not RTM_NEWLINK: %08x %08x %08x\n",
+			n->nlmsg_len, n->nlmsg_type, n->nlmsg_flags);
+		return 0;
+	}
+
+	len -= NLMSG_LENGTH(sizeof(*ifm));
+	if (len < 0) {
+		fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
+		return -1;
+	}
+
+	if (ifm->ifi_family != AF_BRIDGE)
+		return 0;
+
+	if (filter_index && filter_index != ifm->ifi_index)
+		return 0;
+
+	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifm), len);
+
+	if (tb[IFLA_AF_SPEC]) {
+		struct rtattr *i, *list = tb[IFLA_AF_SPEC];
+		int rem = RTA_PAYLOAD(list);
+
+		for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
+			SPRINT_BUF(b1);
+
+			if (i->rta_type != IFLA_BRIDGE_VLAN_PROTOCOL)
+				continue;
+
+			fprintf(fp, "%s", ll_index_to_name(ifm->ifi_index));
+			fprintf(fp, "\t%s",
+				ll_proto_n2a(rta_getattr_u16(i), b1, sizeof(b1)));
+			fprintf(fp, "\n");
+			break;
+		}
+	}
+
 	fflush(fp);
 	return 0;
 }
@@ -162,6 +281,7 @@ static int print_vlan(const struct sockaddr_nl *who,
 static int vlan_show(int argc, char **argv)
 {
 	char *filter_dev = NULL;
+	int proto = 0;
 
 	while (argc > 0) {
 		if (strcmp(*argv, "dev") == 0) {
@@ -170,6 +290,8 @@ static int vlan_show(int argc, char **argv)
 				duparg("dev", *argv);
 			filter_dev = *argv;
 		}
+		if (strcmp(*argv, "protocol") == 0)
+			proto = 1;
 		argc--; argv++;
 	}
 
@@ -187,10 +309,18 @@ static int vlan_show(int argc, char **argv)
 		exit(1);
 	}
 
-	printf("port\tvlan ids\n");
-	if (rtnl_dump_filter(&rth, print_vlan, stdout) < 0) {
-		fprintf(stderr, "Dump ternminated\n");
-		exit(1);
+	if (proto) {
+		printf("port\tprotocol\n");
+		if (rtnl_dump_filter(&rth, print_vlan_proto, stdout) < 0) {
+			fprintf(stderr, "Dump terminatied\n");
+			exit(1);
+		}
+	} else {
+		printf("port\tvlan ids\n");
+		if (rtnl_dump_filter(&rth, print_vlan, stdout) < 0) {
+			fprintf(stderr, "Dump ternminated\n");
+			exit(1);
+		}
 	}
 
 	return 0;
@@ -206,6 +336,8 @@ int do_vlan(int argc, char **argv)
 			return vlan_modify(RTM_SETLINK, argc-1, argv+1);
 		if (matches(*argv, "delete") == 0)
 			return vlan_modify(RTM_DELLINK, argc-1, argv+1);
+		if (matches(*argv, "set") == 0)
+			return vlan_set(argc-1, argv+1);
 		if (matches(*argv, "show") == 0 ||
 		    matches(*argv, "lst") == 0 ||
 		    matches(*argv, "list") == 0)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index d2de4e6..44ed163 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -110,12 +110,14 @@ struct __fdb_entry {
  *     [IFLA_BRIDGE_FLAGS]
  *     [IFLA_BRIDGE_MODE]
  *     [IFLA_BRIDGE_VLAN_INFO]
+ *     [IFLA_BRIDGE_VLAN_PROTOCOL]
  * }
  */
 enum {
 	IFLA_BRIDGE_FLAGS,
 	IFLA_BRIDGE_MODE,
 	IFLA_BRIDGE_VLAN_INFO,
+	IFLA_BRIDGE_VLAN_PROTOCOL,
 	__IFLA_BRIDGE_MAX,
 };
 #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
-- 
1.8.1.2

Re: [PATCH RFC 2/3] bridge: Prepare for 802.1ad vlan filtering support

[Vlad Yasevich]

On 03/10/2014 04:11 AM, Toshiaki Makita wrote:
> This enables a bridge to have vlan protocol informantion and allows vlan
> filtering code to take vlan protocols into account.
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>  net/bridge/br_device.c  |  1 +
>  net/bridge/br_input.c   |  2 +-
>  net/bridge/br_private.h | 24 +++++++++++++++++++---
>  net/bridge/br_vlan.c    | 53 ++++++++++++++++++++++++++++++++-----------------
>  4 files changed, 58 insertions(+), 22 deletions(-)
> 
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index b063050..6cf2fbc 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -381,4 +381,5 @@ void br_dev_setup(struct net_device *dev)
>  	br_netfilter_rtable_init(br);
>  	br_stp_timer_init(br);
>  	br_multicast_init(br);
> +	br_vlan_init(br);
>  }
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 28d5446..8b69da6 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -146,7 +146,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
>  	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
>  	u16 vid = 0;
>  
> -	br_vlan_get_tag(skb, &vid);
> +	br_vlan_get_tag(skb, br_get_vlan_proto(p->br), &vid);
>  	if (p->flags & BR_LEARNING)
>  		br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, false);
>  	return 0;	 /* process further */
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index e1ca1dc..500cf0e 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -292,6 +292,7 @@ struct net_bridge
>  	struct kobject			*ifobj;
>  #ifdef CONFIG_BRIDGE_VLAN_FILTERING
>  	u8				vlan_enabled;
> +	__be16				vlan_proto;
>  	struct net_port_vlans __rcu	*vlan_info;
>  #endif
>  };
> @@ -589,6 +590,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid);
>  void br_vlan_flush(struct net_bridge *br);
>  bool br_vlan_find(struct net_bridge *br, u16 vid);
>  int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
> +void br_vlan_init(struct net_bridge *br);
>  int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags);
>  int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
>  void nbp_vlan_flush(struct net_bridge_port *port);
> @@ -609,11 +611,12 @@ static inline struct net_port_vlans *nbp_get_vlan_info(
>  /* Since bridge now depends on 8021Q module, but the time bridge sees the
>   * skb, the vlan tag will always be present if the frame was tagged.
>   */
> -static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid)
> +static inline int br_vlan_get_tag(const struct sk_buff *skb, __be16 proto,
> +				  u16 *vid)
>  {
>  	int err = 0;
>  
> -	if (vlan_tx_tag_present(skb))
> +	if (vlan_tx_tag_present(skb) && skb->vlan_proto == proto)
>  		*vid = vlan_tx_tag_get(skb) & VLAN_VID_MASK;
>  	else {
>  		*vid = 0;
> @@ -632,6 +635,11 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
>  	return v->pvid ?: VLAN_N_VID;
>  }
>  
> +static inline __be16 br_get_vlan_proto(const struct net_bridge *br)
> +{
> +	return br->vlan_proto;
> +}
> +
>  #else
>  static inline bool br_allowed_ingress(struct net_bridge *br,
>  				      struct net_port_vlans *v,
> @@ -674,6 +682,10 @@ static inline bool br_vlan_find(struct net_bridge *br, u16 vid)
>  	return false;
>  }
>  
> +static inline void br_vlan_init(struct net_bridge *br)
> +{
> +}
> +
>  static inline int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
>  {
>  	return -EOPNOTSUPP;
> @@ -704,7 +716,8 @@ static inline bool nbp_vlan_find(struct net_bridge_port *port, u16 vid)
>  	return false;
>  }
>  
> -static inline u16 br_vlan_get_tag(const struct sk_buff *skb, u16 *tag)
> +static inline u16 br_vlan_get_tag(const struct sk_buff *skb, __be16 proto,
> +				  u16 *tag)
>  {
>  	return 0;
>  }
> @@ -712,6 +725,11 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
>  {
>  	return VLAN_N_VID;	/* Returns invalid vid */
>  }
> +
> +static inline __be16 br_get_vlan_proto(const struct net_bridge *br)
> +{
> +	return 0;
> +}
>  #endif
>  
>  /* br_netfilter.c */
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index cda93e2..f05ef6a 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -32,7 +32,7 @@ static void __vlan_add_flags(struct net_port_vlans *v, u16 vid, u16 flags)
>  		set_bit(vid, v->untagged_bitmap);
>  }
>  
> -static int __vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
> +static int __vlan_add(struct net_port_vlans *v, __be16 proto, u16 vid, u16 flags)
>  {
>  	struct net_bridge_port *p = NULL;
>  	struct net_bridge *br;
> @@ -60,7 +60,7 @@ static int __vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
>  		 * that ever changes this code will allow tagged
>  		 * traffic to enter the bridge.
>  		 */
> -		err = vlan_vid_add(dev, htons(ETH_P_8021Q), vid);
> +		err = vlan_vid_add(dev, proto, vid);
>  		if (err)
>  			return err;
>  	}
> @@ -80,11 +80,11 @@ static int __vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
>  
>  out_filt:
>  	if (p)
> -		vlan_vid_del(dev, htons(ETH_P_8021Q), vid);
> +		vlan_vid_del(dev, proto, vid);
>  	return err;
>  }
>  
> -static int __vlan_del(struct net_port_vlans *v, u16 vid)
> +static int __vlan_del(struct net_port_vlans *v, __be16 proto, u16 vid)
>  {
>  	if (!test_bit(vid, v->vlan_bitmap))
>  		return -EINVAL;
> @@ -93,7 +93,7 @@ static int __vlan_del(struct net_port_vlans *v, u16 vid)
>  	clear_bit(vid, v->untagged_bitmap);
>  
>  	if (v->port_idx)
> -		vlan_vid_del(v->parent.port->dev, htons(ETH_P_8021Q), vid);
> +		vlan_vid_del(v->parent.port->dev, proto, vid);
>  
>  	clear_bit(vid, v->vlan_bitmap);
>  	v->num_vlans--;
> @@ -132,7 +132,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
>  	 * a valid vlan id.  If the vlan id is set in the untagged bitmap,
>  	 * send untagged; otherwise, send tagged.
>  	 */
> -	br_vlan_get_tag(skb, &vid);
> +	br_vlan_get_tag(skb, br_get_vlan_proto(br), &vid);
>  	if (test_bit(vid, pv->untagged_bitmap))
>  		skb->vlan_tci = 0;
>  
> @@ -145,6 +145,7 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
>  			struct sk_buff *skb, u16 *vid)
>  {
>  	int err;
> +	__be16 proto = br_get_vlan_proto(br);
>  
>  	/* If VLAN filtering is disabled on the bridge, all packets are
>  	 * permitted.
> @@ -158,7 +159,7 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
>  	if (!v)
>  		return false;
>  
> -	err = br_vlan_get_tag(skb, vid);
> +	err = br_vlan_get_tag(skb, proto, vid);
>  	if (!*vid) {
>  		u16 pvid = br_get_pvid(v);
>  
> @@ -173,16 +174,27 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
>  		 * ingress frame is considered to belong to this vlan.
>  		 */
>  		*vid = pvid;
> -		if (likely(err))
> +		if (likely(err)) {
>  			/* Untagged Frame. */
> -			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
> -		else
> +			if (vlan_tx_tag_present(skb)) {
> +				/* skb->vlan_proto was different from br->vlan_proto */
> +				skb_push(skb, ETH_HLEN);
> +				skb = __vlan_put_tag(skb, skb->vlan_proto,
> +						     vlan_tx_tag_get(skb));
> +				if (unlikely(!skb))
> +					return false;
> +				skb_pull(skb, ETH_HLEN);
> +				skb_reset_mac_len(skb);
> +			}
> +			__vlan_hwaccel_put_tag(skb, proto, pvid);

So this seems to be handling the case where we had a protocol mis-match.
My question is why are we hiding this case behind our inability to
fetch the vid from the packet.

I think it might be clearer to make the protocol check explicit
(at least if we were to continue using the approach of defining
 the protocol per bridge).

This code also has a side-effect that it would be permit 802.1ad packets
on an 802.1Q bridge and possibly forward such packets encapsulated yet
again.

-vlad

> +		} else {
>  			/* Priority-tagged Frame.
>  			 * At this point, We know that skb->vlan_tci had
>  			 * VLAN_TAG_PRESENT bit and its VID field was 0x000.
>  			 * We update only VID field and preserve PCP field.
>  			 */
>  			skb->vlan_tci |= pvid;
> +		}
>  
>  		return true;
>  	}
> @@ -207,7 +219,7 @@ bool br_allowed_egress(struct net_bridge *br,
>  	if (!v)
>  		return false;
>  
> -	br_vlan_get_tag(skb, &vid);
> +	br_vlan_get_tag(skb, br_get_vlan_proto(br), &vid);
>  	if (test_bit(vid, v->vlan_bitmap))
>  		return true;
>  
> @@ -226,7 +238,7 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
>  
>  	pv = rtnl_dereference(br->vlan_info);
>  	if (pv)
> -		return __vlan_add(pv, vid, flags);
> +		return __vlan_add(pv, br_get_vlan_proto(br), vid, flags);
>  
>  	/* Create port vlan infomration
>  	 */
> @@ -235,7 +247,7 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
>  		return -ENOMEM;
>  
>  	pv->parent.br = br;
> -	err = __vlan_add(pv, vid, flags);
> +	err = __vlan_add(pv, br_get_vlan_proto(br), vid, flags);
>  	if (err)
>  		goto out;
>  
> @@ -261,7 +273,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid)
>  
>  	br_fdb_find_delete_local(br, NULL, br->dev->dev_addr, vid);
>  
> -	__vlan_del(pv, vid);
> +	__vlan_del(pv, br_get_vlan_proto(br), vid);
>  	return 0;
>  }
>  
> @@ -311,6 +323,11 @@ unlock:
>  	return 0;
>  }
>  
> +void br_vlan_init(struct net_bridge *br)
> +{
> +	br->vlan_proto = htons(ETH_P_8021Q);
> +}
> +
>  /* Must be protected by RTNL.
>   * Must be called with vid in range from 1 to 4094 inclusive.
>   */
> @@ -323,7 +340,7 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
>  
>  	pv = rtnl_dereference(port->vlan_info);
>  	if (pv)
> -		return __vlan_add(pv, vid, flags);
> +		return __vlan_add(pv, br_get_vlan_proto(port->br), vid, flags);
>  
>  	/* Create port vlan infomration
>  	 */
> @@ -335,7 +352,7 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
>  
>  	pv->port_idx = port->port_no;
>  	pv->parent.port = port;
> -	err = __vlan_add(pv, vid, flags);
> +	err = __vlan_add(pv, br_get_vlan_proto(port->br), vid, flags);
>  	if (err)
>  		goto clean_up;
>  
> @@ -362,7 +379,7 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
>  
>  	br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid);
>  
> -	return __vlan_del(pv, vid);
> +	return __vlan_del(pv, br_get_vlan_proto(port->br), vid);
>  }
>  
>  void nbp_vlan_flush(struct net_bridge_port *port)
> @@ -377,7 +394,7 @@ void nbp_vlan_flush(struct net_bridge_port *port)
>  		return;
>  
>  	for_each_set_bit(vid, pv->vlan_bitmap, VLAN_N_VID)
> -		vlan_vid_del(port->dev, htons(ETH_P_8021Q), vid);
> +		vlan_vid_del(port->dev, br_get_vlan_proto(port->br), vid);
>  
>  	__vlan_flush(pv);
>  }
> 

Re: [PATCH RFC 2/3] bridge: Prepare for 802.1ad vlan filtering support

[Toshiaki Makita]

On Wed, 2014-03-12 at 13:26 -0400, Vlad Yasevich wrote:
> On 03/10/2014 04:11 AM, Toshiaki Makita wrote:
> > This enables a bridge to have vlan protocol informantion and allows vlan
> > filtering code to take vlan protocols into account.
...
> > @@ -173,16 +174,27 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
> >  		 * ingress frame is considered to belong to this vlan.
> >  		 */
> >  		*vid = pvid;
> > -		if (likely(err))
> > +		if (likely(err)) {
> >  			/* Untagged Frame. */
> > -			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
> > -		else
> > +			if (vlan_tx_tag_present(skb)) {
> > +				/* skb->vlan_proto was different from br->vlan_proto */
> > +				skb_push(skb, ETH_HLEN);
> > +				skb = __vlan_put_tag(skb, skb->vlan_proto,
> > +						     vlan_tx_tag_get(skb));
> > +				if (unlikely(!skb))
> > +					return false;
> > +				skb_pull(skb, ETH_HLEN);
> > +				skb_reset_mac_len(skb);
> > +			}
> > +			__vlan_hwaccel_put_tag(skb, proto, pvid);
> 
> So this seems to be handling the case where we had a protocol mis-match.
> My question is why are we hiding this case behind our inability to
> fetch the vid from the packet.
> 
> I think it might be clearer to make the protocol check explicit
> (at least if we were to continue using the approach of defining
>  the protocol per bridge).

I didn't intend to handle protocol mismatch, but handle the case where
the vlan_tci we are about to use happens to be already used.
In this function, it can occur only if the frame is originally tagged
with another protocol.

However, indeed, we seem to need the check of skb->vlan_proto only at
ingress.
So it maybe makes sense to check the vid and the protocol separately.

I'm thinking of changing that code like this.

	bool untagged;
...
	err = br_vlan_get_tag(skb, vid);
	if (!err) {
		if (skb->vlan_proto != proto) {
			...
			skb = __vlan_put_tag(...);
			...
			*vid = 0;
			untagged = true;
		} else {
			untagged = false;
		}
	} else {
		untagged = true;
	}

	if (!*vid) {
		...
		if (likely(untagged)) {
			/* Untagged Frame. */
			...
		} else {
			/* Priority-tagged Frame.
			...
		}
	}

> 
> This code also has a side-effect that it would be permit 802.1ad packets
> on an 802.1Q bridge and possibly forward such packets encapsulated yet
> again.

Well, this is an interesting situation.
But I have no reason to restrict it.
Users can configure such an environment if they want.

Thanks,
Toshiaki Makita

Re: [PATCH RFC 2/3] bridge: Prepare for 802.1ad vlan filtering support

[Vlad Yasevich]

On 03/13/2014 08:33 AM, Toshiaki Makita wrote:
> On Wed, 2014-03-12 at 13:26 -0400, Vlad Yasevich wrote:
>> On 03/10/2014 04:11 AM, Toshiaki Makita wrote:
>>> This enables a bridge to have vlan protocol informantion and allows vlan
>>> filtering code to take vlan protocols into account.
> ...
>>> @@ -173,16 +174,27 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
>>>  		 * ingress frame is considered to belong to this vlan.
>>>  		 */
>>>  		*vid = pvid;
>>> -		if (likely(err))
>>> +		if (likely(err)) {
>>>  			/* Untagged Frame. */
>>> -			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
>>> -		else
>>> +			if (vlan_tx_tag_present(skb)) {
>>> +				/* skb->vlan_proto was different from br->vlan_proto */
>>> +				skb_push(skb, ETH_HLEN);
>>> +				skb = __vlan_put_tag(skb, skb->vlan_proto,
>>> +						     vlan_tx_tag_get(skb));
>>> +				if (unlikely(!skb))
>>> +					return false;
>>> +				skb_pull(skb, ETH_HLEN);
>>> +				skb_reset_mac_len(skb);
>>> +			}
>>> +			__vlan_hwaccel_put_tag(skb, proto, pvid);
>>
>> So this seems to be handling the case where we had a protocol mis-match.
>> My question is why are we hiding this case behind our inability to
>> fetch the vid from the packet.
>>
>> I think it might be clearer to make the protocol check explicit
>> (at least if we were to continue using the approach of defining
>>  the protocol per bridge).
> 
> I didn't intend to handle protocol mismatch, but handle the case where
> the vlan_tci we are about to use happens to be already used.
> In this function, it can occur only if the frame is originally tagged
> with another protocol.
> 
> However, indeed, we seem to need the check of skb->vlan_proto only at
> ingress.
> So it maybe makes sense to check the vid and the protocol separately.
> 
> I'm thinking of changing that code like this.
> 
> 	bool untagged;
> ...
> 	err = br_vlan_get_tag(skb, vid);
> 	if (!err) {
> 		if (skb->vlan_proto != proto) {
> 			...
> 			skb = __vlan_put_tag(...);
> 			...
> 			*vid = 0;
> 			untagged = true;
> 		} else {
> 			untagged = false;
> 		}
> 	} else {
> 		untagged = true;
> 	}
> 
> 	if (!*vid) {
> 		...
> 		if (likely(untagged)) {
> 			/* Untagged Frame. */
> 			...
> 		} else {
> 			/* Priority-tagged Frame.
> 			...
> 		}
> 	}
> 
>>
>> This code also has a side-effect that it would be permit 802.1ad packets
>> on an 802.1Q bridge and possibly forward such packets encapsulated yet
>> again.
> 
> Well, this is an interesting situation.
> But I have no reason to restrict it.
> Users can configure such an environment if they want.

This is almost like tunnel mode that is available on some switches.
Does it make sense to explicitly permit/restrict it?

-vlad

> 
> Thanks,
> Toshiaki Makita
> 
> 

Re: [PATCH RFC 2/3] bridge: Prepare for 802.1ad vlan filtering support

[Toshiaki Makita]

On Thu, 2014-03-13 at 09:53 -0400, Vlad Yasevich wrote:
> On 03/13/2014 08:33 AM, Toshiaki Makita wrote:
> > On Wed, 2014-03-12 at 13:26 -0400, Vlad Yasevich wrote:
> >> On 03/10/2014 04:11 AM, Toshiaki Makita wrote:
> >>> This enables a bridge to have vlan protocol informantion and allows vlan
> >>> filtering code to take vlan protocols into account.
> > ...
> >>> @@ -173,16 +174,27 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
> >>>  		 * ingress frame is considered to belong to this vlan.
> >>>  		 */
> >>>  		*vid = pvid;
> >>> -		if (likely(err))
> >>> +		if (likely(err)) {
> >>>  			/* Untagged Frame. */
> >>> -			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
> >>> -		else
> >>> +			if (vlan_tx_tag_present(skb)) {
> >>> +				/* skb->vlan_proto was different from br->vlan_proto */
> >>> +				skb_push(skb, ETH_HLEN);
> >>> +				skb = __vlan_put_tag(skb, skb->vlan_proto,
> >>> +						     vlan_tx_tag_get(skb));
> >>> +				if (unlikely(!skb))
> >>> +					return false;
> >>> +				skb_pull(skb, ETH_HLEN);
> >>> +				skb_reset_mac_len(skb);
> >>> +			}
> >>> +			__vlan_hwaccel_put_tag(skb, proto, pvid);
> >>
> >> So this seems to be handling the case where we had a protocol mis-match.
> >> My question is why are we hiding this case behind our inability to
> >> fetch the vid from the packet.
> >>
> >> I think it might be clearer to make the protocol check explicit
> >> (at least if we were to continue using the approach of defining
> >>  the protocol per bridge).
> > 
> > I didn't intend to handle protocol mismatch, but handle the case where
> > the vlan_tci we are about to use happens to be already used.
> > In this function, it can occur only if the frame is originally tagged
> > with another protocol.
> > 
> > However, indeed, we seem to need the check of skb->vlan_proto only at
> > ingress.
> > So it maybe makes sense to check the vid and the protocol separately.
> > 
> > I'm thinking of changing that code like this.
> > 
> > 	bool untagged;
> > ...
> > 	err = br_vlan_get_tag(skb, vid);
> > 	if (!err) {
> > 		if (skb->vlan_proto != proto) {
> > 			...
> > 			skb = __vlan_put_tag(...);
> > 			...
> > 			*vid = 0;
> > 			untagged = true;
> > 		} else {
> > 			untagged = false;
> > 		}
> > 	} else {
> > 		untagged = true;
> > 	}
> > 
> > 	if (!*vid) {
> > 		...
> > 		if (likely(untagged)) {
> > 			/* Untagged Frame. */
> > 			...
> > 		} else {
> > 			/* Priority-tagged Frame.
> > 			...
> > 		}
> > 	}
> > 
> >>
> >> This code also has a side-effect that it would be permit 802.1ad packets
> >> on an 802.1Q bridge and possibly forward such packets encapsulated yet
> >> again.
> > 
> > Well, this is an interesting situation.
> > But I have no reason to restrict it.
> > Users can configure such an environment if they want.
> 
> This is almost like tunnel mode that is available on some switches.
> Does it make sense to explicitly permit/restrict it?

I haven't found a description to prohibit a 802.1Q bridge from
encapsulating S-tagged frames. So I'm permitting it.
A C-VLAN component shouldn't recognize S-tag according to 802.1Q-2011
5.5. Therefore, a C-VLAN component behaves regardless of existence of
S-tag.

I think some switches support stacked 802.1Q tags (not using 802.1ad).
This can't be realized by current implementation. It maybe needs an
option to insert a new tag even if the received frame is already tagged.

BTW, I happen to have noticed that an S-VLAN bridge uses different
destination MAC address for STP (802.1Q-2011 8.13.5 and 13.2).
I have to consider it and something similar to it (LACPDU, etc).

Thanks,
Toshiaki Makita