All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] bridge: 802.1ad vlan protocol support
@ 2014-06-09 11:34 ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger
  Cc: Toshiaki Makita, Vlad Yasevich, netdev, bridge

Currently bridge vlan filtering doesn't work fine with 802.1ad protocol.
Only if a bridge is configured without pvid, the bridge receives only
802.1ad tagged frames and no STP is used, 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.
- While an 802.1ad bridge should use another mac address for STP BPDU and
  should forward customer's BPDU frames, it can't.
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 of an 802.1ad bridge 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.

- 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 the other is an 802.1ad filtering bridge) in one bridge.
  The former usually needs additional features such as vlan id mapping, and
  is likely to make bridge's code 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 the other 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.

Changes:
RFC -> v1:
- Add S-TAG tx offload.
- Remove a fix around stacked vlan which has already been fixed.
- Take into account Bridge Group Addresses.
- Separate handling of protocol-mismatch from br_vlan_get_tag().
- Change the way to set vlan_proto from netlink to sysfs because no other
  existing configuration per bridge can be set by netlink.

Toshiaki Makita (4):
  bridge: Add 802.1ad tx vlan acceleration
  bridge: Prepare for 802.1ad vlan filtering support
  bridge: Consider the Nearest Customer Bridge group addresses
  bridge: Support 802.1ad vlan filtering

 net/bridge/br_device.c   |   6 ++-
 net/bridge/br_input.c    |  15 +++++-
 net/bridge/br_private.h  |  18 +++++++
 net/bridge/br_sysfs_br.c |  18 +++++++
 net/bridge/br_vlan.c     | 137 ++++++++++++++++++++++++++++++++++++++++++-----
 5 files changed, 179 insertions(+), 15 deletions(-)

-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [Bridge] [PATCH net-next 0/4] bridge: 802.1ad vlan protocol support
@ 2014-06-09 11:34 ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger; +Cc: Vlad Yasevich, netdev, bridge

Currently bridge vlan filtering doesn't work fine with 802.1ad protocol.
Only if a bridge is configured without pvid, the bridge receives only
802.1ad tagged frames and no STP is used, 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.
- While an 802.1ad bridge should use another mac address for STP BPDU and
  should forward customer's BPDU frames, it can't.
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 of an 802.1ad bridge 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.

- 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 the other is an 802.1ad filtering bridge) in one bridge.
  The former usually needs additional features such as vlan id mapping, and
  is likely to make bridge's code 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 the other 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.

Changes:
RFC -> v1:
- Add S-TAG tx offload.
- Remove a fix around stacked vlan which has already been fixed.
- Take into account Bridge Group Addresses.
- Separate handling of protocol-mismatch from br_vlan_get_tag().
- Change the way to set vlan_proto from netlink to sysfs because no other
  existing configuration per bridge can be set by netlink.

Toshiaki Makita (4):
  bridge: Add 802.1ad tx vlan acceleration
  bridge: Prepare for 802.1ad vlan filtering support
  bridge: Consider the Nearest Customer Bridge group addresses
  bridge: Support 802.1ad vlan filtering

 net/bridge/br_device.c   |   6 ++-
 net/bridge/br_input.c    |  15 +++++-
 net/bridge/br_private.h  |  18 +++++++
 net/bridge/br_sysfs_br.c |  18 +++++++
 net/bridge/br_vlan.c     | 137 ++++++++++++++++++++++++++++++++++++++++++-----
 5 files changed, 179 insertions(+), 15 deletions(-)

-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH net-next 1/4] bridge: Add 802.1ad tx vlan acceleration
  2014-06-09 11:34 ` [Bridge] " Toshiaki Makita
@ 2014-06-09 11:34   ` Toshiaki Makita
  -1 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger; +Cc: Vlad Yasevich, netdev, bridge

Bridge device doesn't need to embed S-tag into skb->data.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_device.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index d77e2f0..82a410a 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -361,8 +361,9 @@ void br_dev_setup(struct net_device *dev)
 	dev->priv_flags = IFF_EBRIDGE;
 
 	dev->features = COMMON_FEATURES | NETIF_F_LLTX | NETIF_F_NETNS_LOCAL |
-			NETIF_F_HW_VLAN_CTAG_TX;
-	dev->hw_features = COMMON_FEATURES | NETIF_F_HW_VLAN_CTAG_TX;
+			NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
+	dev->hw_features = COMMON_FEATURES | NETIF_F_HW_VLAN_CTAG_TX |
+			   NETIF_F_HW_VLAN_STAG_TX;
 	dev->vlan_features = COMMON_FEATURES;
 
 	br->dev = dev;
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [Bridge] [PATCH net-next 1/4] bridge: Add 802.1ad tx vlan acceleration
@ 2014-06-09 11:34   ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger; +Cc: Vlad Yasevich, netdev, bridge

Bridge device doesn't need to embed S-tag into skb->data.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_device.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index d77e2f0..82a410a 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -361,8 +361,9 @@ void br_dev_setup(struct net_device *dev)
 	dev->priv_flags = IFF_EBRIDGE;
 
 	dev->features = COMMON_FEATURES | NETIF_F_LLTX | NETIF_F_NETNS_LOCAL |
-			NETIF_F_HW_VLAN_CTAG_TX;
-	dev->hw_features = COMMON_FEATURES | NETIF_F_HW_VLAN_CTAG_TX;
+			NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
+	dev->hw_features = COMMON_FEATURES | NETIF_F_HW_VLAN_CTAG_TX |
+			   NETIF_F_HW_VLAN_STAG_TX;
 	dev->vlan_features = COMMON_FEATURES;
 
 	br->dev = dev;
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH net-next 2/4] bridge: Prepare for 802.1ad vlan filtering support
  2014-06-09 11:34 ` [Bridge] " Toshiaki Makita
@ 2014-06-09 11:34   ` Toshiaki Makita
  -1 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger; +Cc: Vlad Yasevich, netdev, bridge

This enables a bridge to have vlan protocol informantion and allows vlan
tag manipulation (retrieve, insert and remove tags) according to vlan
protocols.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_device.c  |  1 +
 net/bridge/br_private.h |  6 ++++++
 net/bridge/br_vlan.c    | 56 ++++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 82a410a..1b797c4 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -388,4 +388,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_private.h b/net/bridge/br_private.h
index bc17210..b65fee9 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -298,6 +298,7 @@ struct net_bridge
 	u32				auto_cnt;
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	u8				vlan_enabled;
+	__be16				vlan_proto;
 	struct net_port_vlans __rcu	*vlan_info;
 #endif
 };
@@ -598,6 +599,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);
@@ -693,6 +695,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;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index fcc9539..63bd981 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -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, br->vlan_proto, vid);
 		if (err)
 			return err;
 	}
@@ -80,7 +80,7 @@ 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, br->vlan_proto, vid);
 	return err;
 }
 
@@ -92,8 +92,10 @@ static int __vlan_del(struct net_port_vlans *v, u16 vid)
 	__vlan_delete_pvid(v, vid);
 	clear_bit(vid, v->untagged_bitmap);
 
-	if (v->port_idx)
-		vlan_vid_del(v->parent.port->dev, htons(ETH_P_8021Q), vid);
+	if (v->port_idx) {
+		struct net_bridge_port *p = v->parent.port;
+		vlan_vid_del(p->dev, p->br->vlan_proto, vid);
+	}
 
 	clear_bit(vid, v->vlan_bitmap);
 	v->num_vlans--;
@@ -158,7 +160,8 @@ out:
 bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 			struct sk_buff *skb, u16 *vid)
 {
-	int err;
+	bool tagged;
+	__be16 proto;
 
 	/* If VLAN filtering is disabled on the bridge, all packets are
 	 * permitted.
@@ -172,19 +175,41 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 	if (!v)
 		goto drop;
 
+	proto = br->vlan_proto;
+
 	/* If vlan tx offload is disabled on bridge device and frame was
 	 * sent from vlan device on the bridge device, it does not have
 	 * HW accelerated vlan tag.
 	 */
 	if (unlikely(!vlan_tx_tag_present(skb) &&
-		     (skb->protocol == htons(ETH_P_8021Q) ||
-		      skb->protocol == htons(ETH_P_8021AD)))) {
+		     skb->protocol == proto)) {
 		skb = vlan_untag(skb);
 		if (unlikely(!skb))
 			return false;
 	}
 
-	err = br_vlan_get_tag(skb, vid);
+	if (!br_vlan_get_tag(skb, vid)) {
+		/* Tagged frame */
+		if (skb->vlan_proto != proto) {
+			/* Protocol-mismatch, empty out vlan_tci for new tag */
+			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);
+			*vid = 0;
+			tagged = false;
+		} else {
+			tagged = true;
+		}
+	} else {
+		/* Untagged frame */
+		tagged = false;
+	}
+
 	if (!*vid) {
 		u16 pvid = br_get_pvid(v);
 
@@ -199,9 +224,9 @@ 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(!tagged))
 			/* Untagged Frame. */
-			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
+			__vlan_hwaccel_put_tag(skb, proto, pvid);
 		else
 			/* Priority-tagged Frame.
 			 * At this point, We know that skb->vlan_tci had
@@ -254,7 +279,9 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
 	if (!v)
 		return false;
 
-	br_vlan_get_tag(skb, vid);
+	if (!br_vlan_get_tag(skb, vid) && skb->vlan_proto != br->vlan_proto)
+		*vid = 0;
+
 	if (!*vid) {
 		*vid = br_get_pvid(v);
 		if (*vid == VLAN_N_VID)
@@ -367,6 +394,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.
  */
@@ -433,7 +465,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, port->br->vlan_proto, vid);
 
 	__vlan_flush(pv);
 }
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [Bridge] [PATCH net-next 2/4] bridge: Prepare for 802.1ad vlan filtering support
@ 2014-06-09 11:34   ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger; +Cc: Vlad Yasevich, netdev, bridge

This enables a bridge to have vlan protocol informantion and allows vlan
tag manipulation (retrieve, insert and remove tags) according to vlan
protocols.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_device.c  |  1 +
 net/bridge/br_private.h |  6 ++++++
 net/bridge/br_vlan.c    | 56 ++++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 82a410a..1b797c4 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -388,4 +388,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_private.h b/net/bridge/br_private.h
index bc17210..b65fee9 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -298,6 +298,7 @@ struct net_bridge
 	u32				auto_cnt;
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	u8				vlan_enabled;
+	__be16				vlan_proto;
 	struct net_port_vlans __rcu	*vlan_info;
 #endif
 };
@@ -598,6 +599,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);
@@ -693,6 +695,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;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index fcc9539..63bd981 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -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, br->vlan_proto, vid);
 		if (err)
 			return err;
 	}
@@ -80,7 +80,7 @@ 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, br->vlan_proto, vid);
 	return err;
 }
 
@@ -92,8 +92,10 @@ static int __vlan_del(struct net_port_vlans *v, u16 vid)
 	__vlan_delete_pvid(v, vid);
 	clear_bit(vid, v->untagged_bitmap);
 
-	if (v->port_idx)
-		vlan_vid_del(v->parent.port->dev, htons(ETH_P_8021Q), vid);
+	if (v->port_idx) {
+		struct net_bridge_port *p = v->parent.port;
+		vlan_vid_del(p->dev, p->br->vlan_proto, vid);
+	}
 
 	clear_bit(vid, v->vlan_bitmap);
 	v->num_vlans--;
@@ -158,7 +160,8 @@ out:
 bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 			struct sk_buff *skb, u16 *vid)
 {
-	int err;
+	bool tagged;
+	__be16 proto;
 
 	/* If VLAN filtering is disabled on the bridge, all packets are
 	 * permitted.
@@ -172,19 +175,41 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 	if (!v)
 		goto drop;
 
+	proto = br->vlan_proto;
+
 	/* If vlan tx offload is disabled on bridge device and frame was
 	 * sent from vlan device on the bridge device, it does not have
 	 * HW accelerated vlan tag.
 	 */
 	if (unlikely(!vlan_tx_tag_present(skb) &&
-		     (skb->protocol == htons(ETH_P_8021Q) ||
-		      skb->protocol == htons(ETH_P_8021AD)))) {
+		     skb->protocol == proto)) {
 		skb = vlan_untag(skb);
 		if (unlikely(!skb))
 			return false;
 	}
 
-	err = br_vlan_get_tag(skb, vid);
+	if (!br_vlan_get_tag(skb, vid)) {
+		/* Tagged frame */
+		if (skb->vlan_proto != proto) {
+			/* Protocol-mismatch, empty out vlan_tci for new tag */
+			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);
+			*vid = 0;
+			tagged = false;
+		} else {
+			tagged = true;
+		}
+	} else {
+		/* Untagged frame */
+		tagged = false;
+	}
+
 	if (!*vid) {
 		u16 pvid = br_get_pvid(v);
 
@@ -199,9 +224,9 @@ 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(!tagged))
 			/* Untagged Frame. */
-			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
+			__vlan_hwaccel_put_tag(skb, proto, pvid);
 		else
 			/* Priority-tagged Frame.
 			 * At this point, We know that skb->vlan_tci had
@@ -254,7 +279,9 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
 	if (!v)
 		return false;
 
-	br_vlan_get_tag(skb, vid);
+	if (!br_vlan_get_tag(skb, vid) && skb->vlan_proto != br->vlan_proto)
+		*vid = 0;
+
 	if (!*vid) {
 		*vid = br_get_pvid(v);
 		if (*vid == VLAN_N_VID)
@@ -367,6 +394,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.
  */
@@ -433,7 +465,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, port->br->vlan_proto, vid);
 
 	__vlan_flush(pv);
 }
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
  2014-06-09 11:34 ` [Bridge] " Toshiaki Makita
@ 2014-06-09 11:34   ` Toshiaki Makita
  -1 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger
  Cc: Toshiaki Makita, Vlad Yasevich, netdev, bridge

An 802.1ad bridge must forward the Nearest Customer Bridge group addresses.
 01-80-C2-00-00-00
 01-80-C2-00-00-0B
 01-80-C2-00-00-0C
 01-80-C2-00-00-0D
 01-80-C2-00-00-0F
(For details, see IEEE 802.1Q-2011 8.6.3.)

An exception is the br->group_addr, which needs to be passed to the higher
layer entity so that STP works.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_input.c   | 15 ++++++++++++++-
 net/bridge/br_private.h | 10 ++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 04d6348..b05d419 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -194,13 +194,26 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 		case 0x00:	/* Bridge Group Address */
 			/* If STP is turned off,
 			   then must forward to keep loop detection */
-			if (p->br->stp_enabled == BR_NO_STP)
+			if (p->br->stp_enabled == BR_NO_STP ||
+			    (br_vlan_enabled(p->br) &&
+			     br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
+			     p->br->group_addr[5] != dest[5]))
 				goto forward;
 			break;
 
 		case 0x01:	/* IEEE MAC (Pause) */
 			goto drop;
 
+		case 0x0B:
+		case 0x0C:
+		case 0x0D:
+		case 0x0F:
+			/* The Nearest Customer Bridge group address */
+			if (br_vlan_enabled(p->br) &&
+			    br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
+			    p->br->group_addr[5] != dest[5])
+				goto forward;
+			/* fall through */
 		default:
 			/* Allow selective forwarding for most other protocols */
 			if (p->br->group_fwd_mask & (1u << dest[5]))
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b65fee9..65204c2 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -647,6 +647,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
 {
 	return br->vlan_enabled;
 }
+
+static inline __be16 br_vlan_get_proto(struct net_bridge *br)
+{
+	return br->vlan_proto;
+}
 #else
 static inline bool br_allowed_ingress(struct net_bridge *br,
 				      struct net_port_vlans *v,
@@ -742,6 +747,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
 {
 	return 0;
 }
+
+static inline __be16 br_vlan_get_proto(struct net_bridge *br)
+{
+	return 0;
+}
 #endif
 
 /* br_netfilter.c */
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [Bridge] [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
@ 2014-06-09 11:34   ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger; +Cc: Vlad Yasevich, netdev, bridge

An 802.1ad bridge must forward the Nearest Customer Bridge group addresses.
 01-80-C2-00-00-00
 01-80-C2-00-00-0B
 01-80-C2-00-00-0C
 01-80-C2-00-00-0D
 01-80-C2-00-00-0F
(For details, see IEEE 802.1Q-2011 8.6.3.)

An exception is the br->group_addr, which needs to be passed to the higher
layer entity so that STP works.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_input.c   | 15 ++++++++++++++-
 net/bridge/br_private.h | 10 ++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 04d6348..b05d419 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -194,13 +194,26 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 		case 0x00:	/* Bridge Group Address */
 			/* If STP is turned off,
 			   then must forward to keep loop detection */
-			if (p->br->stp_enabled == BR_NO_STP)
+			if (p->br->stp_enabled == BR_NO_STP ||
+			    (br_vlan_enabled(p->br) &&
+			     br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
+			     p->br->group_addr[5] != dest[5]))
 				goto forward;
 			break;
 
 		case 0x01:	/* IEEE MAC (Pause) */
 			goto drop;
 
+		case 0x0B:
+		case 0x0C:
+		case 0x0D:
+		case 0x0F:
+			/* The Nearest Customer Bridge group address */
+			if (br_vlan_enabled(p->br) &&
+			    br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
+			    p->br->group_addr[5] != dest[5])
+				goto forward;
+			/* fall through */
 		default:
 			/* Allow selective forwarding for most other protocols */
 			if (p->br->group_fwd_mask & (1u << dest[5]))
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b65fee9..65204c2 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -647,6 +647,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
 {
 	return br->vlan_enabled;
 }
+
+static inline __be16 br_vlan_get_proto(struct net_bridge *br)
+{
+	return br->vlan_proto;
+}
 #else
 static inline bool br_allowed_ingress(struct net_bridge *br,
 				      struct net_port_vlans *v,
@@ -742,6 +747,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
 {
 	return 0;
 }
+
+static inline __be16 br_vlan_get_proto(struct net_bridge *br)
+{
+	return 0;
+}
 #endif
 
 /* br_netfilter.c */
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH net-next 4/4] bridge: Support 802.1ad vlan filtering
  2014-06-09 11:34 ` [Bridge] " Toshiaki Makita
@ 2014-06-09 11:34   ` Toshiaki Makita
  -1 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger
  Cc: Toshiaki Makita, Vlad Yasevich, netdev, bridge

This enables us to change the vlan protocol for vlan filtering.
We come to be able to filter frames on the basis of 802.1ad vlan tags
through a bridge.

This also changes br->group_addr if it has not been set by user.
This is needed for an 802.1ad bridge.
(See IEEE 802.1Q-2011 8.13.5.)

To change the vlan protocol, write a protocol in sysfs:
# echo 0x88a8 > /sys/class/net/br0/bridge/vlan_protocol

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_private.h  |  2 ++
 net/bridge/br_sysfs_br.c | 18 +++++++++++
 net/bridge/br_vlan.c     | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 65204c2..3c5b23b 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -246,6 +246,7 @@ struct net_bridge
 	unsigned long			bridge_forward_delay;
 
 	u8				group_addr[ETH_ALEN];
+	unsigned char			group_addr_set;
 	u16				root_port;
 
 	enum {
@@ -599,6 +600,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);
+int br_vlan_set_proto(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);
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 8dac6555..1831018 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -315,6 +315,7 @@ static ssize_t group_addr_store(struct device *d,
 	spin_lock_bh(&br->lock);
 	for (i = 0; i < 6; i++)
 		br->group_addr[i] = new_addr[i];
+	br->group_addr_set = 1;
 	spin_unlock_bh(&br->lock);
 	return len;
 }
@@ -700,6 +701,22 @@ static ssize_t vlan_filtering_store(struct device *d,
 	return store_bridge_parm(d, buf, len, br_vlan_filter_toggle);
 }
 static DEVICE_ATTR_RW(vlan_filtering);
+
+static ssize_t vlan_protocol_show(struct device *d,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct net_bridge *br = to_bridge(d);
+	return sprintf(buf, "%#06x\n", ntohs(br->vlan_proto));
+}
+
+static ssize_t vlan_protocol_store(struct device *d,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	return store_bridge_parm(d, buf, len, br_vlan_set_proto);
+}
+static DEVICE_ATTR_RW(vlan_protocol);
 #endif
 
 static struct attribute *bridge_attrs[] = {
@@ -745,6 +762,7 @@ static struct attribute *bridge_attrs[] = {
 #endif
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	&dev_attr_vlan_filtering.attr,
+	&dev_attr_vlan_protocol.attr,
 #endif
 	NULL
 };
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 63bd981..c86a7a6 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -394,6 +394,87 @@ unlock:
 	return 0;
 }
 
+int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
+{
+	int err = 0;
+	struct net_bridge_port *p;
+	struct net_port_vlans *pv;
+	__be16 proto, oldproto;
+	u16 vid, errvid;
+
+	if (val != ETH_P_8021Q && val != ETH_P_8021AD)
+		return -EPROTONOSUPPORT;
+
+	if (!rtnl_trylock())
+		return restart_syscall();
+
+	proto = htons(val);
+	if (br->vlan_proto == proto)
+		goto unlock;
+
+	/* 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 err_filt;
+		}
+	}
+
+	spin_lock_bh(&br->lock);
+	if (!br->group_addr_set) {
+		switch (val) {
+		case ETH_P_8021Q:
+			/* Bridge Group Address */
+			br->group_addr[5] = 0x00;
+			break;
+
+		case ETH_P_8021AD:
+			/* Provider Bridge Group Address */
+			br->group_addr[5] = 0x08;
+			break;
+		}
+	}
+	spin_unlock_bh(&br->lock);
+
+	oldproto = br->vlan_proto;
+	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);
+	}
+
+unlock:
+	rtnl_unlock();
+	return err;
+
+err_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);
+	}
+
+	goto unlock;
+}
+
 void br_vlan_init(struct net_bridge *br)
 {
 	br->vlan_proto = htons(ETH_P_8021Q);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [Bridge] [PATCH net-next 4/4] bridge: Support 802.1ad vlan filtering
@ 2014-06-09 11:34   ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 11:34 UTC (permalink / raw)
  To: David S . Miller, Stephen Hemminger; +Cc: Vlad Yasevich, netdev, bridge

This enables us to change the vlan protocol for vlan filtering.
We come to be able to filter frames on the basis of 802.1ad vlan tags
through a bridge.

This also changes br->group_addr if it has not been set by user.
This is needed for an 802.1ad bridge.
(See IEEE 802.1Q-2011 8.13.5.)

To change the vlan protocol, write a protocol in sysfs:
# echo 0x88a8 > /sys/class/net/br0/bridge/vlan_protocol

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 net/bridge/br_private.h  |  2 ++
 net/bridge/br_sysfs_br.c | 18 +++++++++++
 net/bridge/br_vlan.c     | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 65204c2..3c5b23b 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -246,6 +246,7 @@ struct net_bridge
 	unsigned long			bridge_forward_delay;
 
 	u8				group_addr[ETH_ALEN];
+	unsigned char			group_addr_set;
 	u16				root_port;
 
 	enum {
@@ -599,6 +600,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);
+int br_vlan_set_proto(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);
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 8dac6555..1831018 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -315,6 +315,7 @@ static ssize_t group_addr_store(struct device *d,
 	spin_lock_bh(&br->lock);
 	for (i = 0; i < 6; i++)
 		br->group_addr[i] = new_addr[i];
+	br->group_addr_set = 1;
 	spin_unlock_bh(&br->lock);
 	return len;
 }
@@ -700,6 +701,22 @@ static ssize_t vlan_filtering_store(struct device *d,
 	return store_bridge_parm(d, buf, len, br_vlan_filter_toggle);
 }
 static DEVICE_ATTR_RW(vlan_filtering);
+
+static ssize_t vlan_protocol_show(struct device *d,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct net_bridge *br = to_bridge(d);
+	return sprintf(buf, "%#06x\n", ntohs(br->vlan_proto));
+}
+
+static ssize_t vlan_protocol_store(struct device *d,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	return store_bridge_parm(d, buf, len, br_vlan_set_proto);
+}
+static DEVICE_ATTR_RW(vlan_protocol);
 #endif
 
 static struct attribute *bridge_attrs[] = {
@@ -745,6 +762,7 @@ static struct attribute *bridge_attrs[] = {
 #endif
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	&dev_attr_vlan_filtering.attr,
+	&dev_attr_vlan_protocol.attr,
 #endif
 	NULL
 };
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 63bd981..c86a7a6 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -394,6 +394,87 @@ unlock:
 	return 0;
 }
 
+int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
+{
+	int err = 0;
+	struct net_bridge_port *p;
+	struct net_port_vlans *pv;
+	__be16 proto, oldproto;
+	u16 vid, errvid;
+
+	if (val != ETH_P_8021Q && val != ETH_P_8021AD)
+		return -EPROTONOSUPPORT;
+
+	if (!rtnl_trylock())
+		return restart_syscall();
+
+	proto = htons(val);
+	if (br->vlan_proto == proto)
+		goto unlock;
+
+	/* 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 err_filt;
+		}
+	}
+
+	spin_lock_bh(&br->lock);
+	if (!br->group_addr_set) {
+		switch (val) {
+		case ETH_P_8021Q:
+			/* Bridge Group Address */
+			br->group_addr[5] = 0x00;
+			break;
+
+		case ETH_P_8021AD:
+			/* Provider Bridge Group Address */
+			br->group_addr[5] = 0x08;
+			break;
+		}
+	}
+	spin_unlock_bh(&br->lock);
+
+	oldproto = br->vlan_proto;
+	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);
+	}
+
+unlock:
+	rtnl_unlock();
+	return err;
+
+err_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);
+	}
+
+	goto unlock;
+}
+
 void br_vlan_init(struct net_bridge *br)
 {
 	br->vlan_proto = htons(ETH_P_8021Q);
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
  2014-06-09 11:34   ` [Bridge] " Toshiaki Makita
@ 2014-06-09 15:52     ` Stephen Hemminger
  -1 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2014-06-09 15:52 UTC (permalink / raw)
  To: Toshiaki Makita; +Cc: Vlad Yasevich, netdev, bridge, David S . Miller

On Mon,  9 Jun 2014 20:34:46 +0900
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:

> An 802.1ad bridge must forward the Nearest Customer Bridge group addresses.
>  01-80-C2-00-00-00
>  01-80-C2-00-00-0B
>  01-80-C2-00-00-0C
>  01-80-C2-00-00-0D
>  01-80-C2-00-00-0F
> (For details, see IEEE 802.1Q-2011 8.6.3.)
> 
> An exception is the br->group_addr, which needs to be passed to the higher
> layer entity so that STP works.
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>  net/bridge/br_input.c   | 15 ++++++++++++++-
>  net/bridge/br_private.h | 10 ++++++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 04d6348..b05d419 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -194,13 +194,26 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
>  		case 0x00:	/* Bridge Group Address */
>  			/* If STP is turned off,
>  			   then must forward to keep loop detection */
> -			if (p->br->stp_enabled == BR_NO_STP)
> +			if (p->br->stp_enabled == BR_NO_STP ||
> +			    (br_vlan_enabled(p->br) &&
> +			     br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
> +			     p->br->group_addr[5] != dest[5]))
>  				goto forward;
>  			break;
>  
>  		case 0x01:	/* IEEE MAC (Pause) */
>  			goto drop;
>  
> +		case 0x0B:
> +		case 0x0C:
> +		case 0x0D:
> +		case 0x0F:
> +			/* The Nearest Customer Bridge group address */
> +			if (br_vlan_enabled(p->br) &&
> +			    br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
> +			    p->br->group_addr[5] != dest[5])
> +				goto forward;
> +			/* fall through */
>  		default:
>  			/* Allow selective forwarding for most other protocols */
>  			if (p->br->group_fwd_mask & (1u << dest[5]))
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index b65fee9..65204c2 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -647,6 +647,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
>  {
>  	return br->vlan_enabled;
>  }
> +
> +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
> +{
> +	return br->vlan_proto;
> +}
>  #else
>  static inline bool br_allowed_ingress(struct net_bridge *br,
>  				      struct net_port_vlans *v,
> @@ -742,6 +747,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
>  {
>  	return 0;
>  }
> +
> +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
> +{
> +	return 0;
> +}
>  #endif
>  
>  /* br_netfilter.c */

Rather than special casing this around vlan filtering, I would prefer
the code always forward these packets, or manipulate group_fwd_mask
to allow it that way.

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [Bridge] [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
@ 2014-06-09 15:52     ` Stephen Hemminger
  0 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2014-06-09 15:52 UTC (permalink / raw)
  To: Toshiaki Makita; +Cc: Vlad Yasevich, netdev, bridge, David S . Miller

On Mon,  9 Jun 2014 20:34:46 +0900
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:

> An 802.1ad bridge must forward the Nearest Customer Bridge group addresses.
>  01-80-C2-00-00-00
>  01-80-C2-00-00-0B
>  01-80-C2-00-00-0C
>  01-80-C2-00-00-0D
>  01-80-C2-00-00-0F
> (For details, see IEEE 802.1Q-2011 8.6.3.)
> 
> An exception is the br->group_addr, which needs to be passed to the higher
> layer entity so that STP works.
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>  net/bridge/br_input.c   | 15 ++++++++++++++-
>  net/bridge/br_private.h | 10 ++++++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 04d6348..b05d419 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -194,13 +194,26 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
>  		case 0x00:	/* Bridge Group Address */
>  			/* If STP is turned off,
>  			   then must forward to keep loop detection */
> -			if (p->br->stp_enabled == BR_NO_STP)
> +			if (p->br->stp_enabled == BR_NO_STP ||
> +			    (br_vlan_enabled(p->br) &&
> +			     br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
> +			     p->br->group_addr[5] != dest[5]))
>  				goto forward;
>  			break;
>  
>  		case 0x01:	/* IEEE MAC (Pause) */
>  			goto drop;
>  
> +		case 0x0B:
> +		case 0x0C:
> +		case 0x0D:
> +		case 0x0F:
> +			/* The Nearest Customer Bridge group address */
> +			if (br_vlan_enabled(p->br) &&
> +			    br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
> +			    p->br->group_addr[5] != dest[5])
> +				goto forward;
> +			/* fall through */
>  		default:
>  			/* Allow selective forwarding for most other protocols */
>  			if (p->br->group_fwd_mask & (1u << dest[5]))
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index b65fee9..65204c2 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -647,6 +647,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
>  {
>  	return br->vlan_enabled;
>  }
> +
> +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
> +{
> +	return br->vlan_proto;
> +}
>  #else
>  static inline bool br_allowed_ingress(struct net_bridge *br,
>  				      struct net_port_vlans *v,
> @@ -742,6 +747,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
>  {
>  	return 0;
>  }
> +
> +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
> +{
> +	return 0;
> +}
>  #endif
>  
>  /* br_netfilter.c */

Rather than special casing this around vlan filtering, I would prefer
the code always forward these packets, or manipulate group_fwd_mask
to allow it that way.

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
  2014-06-09 15:52     ` [Bridge] " Stephen Hemminger
@ 2014-06-09 16:45       ` Toshiaki Makita
  -1 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 16:45 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Toshiaki Makita, Vlad Yasevich, netdev, bridge, David S . Miller

On Mon, 2014-06-09 at 08:52 -0700, Stephen Hemminger wrote:
> On Mon,  9 Jun 2014 20:34:46 +0900
> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> 
> > An 802.1ad bridge must forward the Nearest Customer Bridge group addresses.
> >  01-80-C2-00-00-00
> >  01-80-C2-00-00-0B
> >  01-80-C2-00-00-0C
> >  01-80-C2-00-00-0D
> >  01-80-C2-00-00-0F
> > (For details, see IEEE 802.1Q-2011 8.6.3.)
> > 
> > An exception is the br->group_addr, which needs to be passed to the higher
> > layer entity so that STP works.
> > 
> > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> > ---
> >  net/bridge/br_input.c   | 15 ++++++++++++++-
> >  net/bridge/br_private.h | 10 ++++++++++
> >  2 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> > index 04d6348..b05d419 100644
> > --- a/net/bridge/br_input.c
> > +++ b/net/bridge/br_input.c
> > @@ -194,13 +194,26 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
> >  		case 0x00:	/* Bridge Group Address */
> >  			/* If STP is turned off,
> >  			   then must forward to keep loop detection */
> > -			if (p->br->stp_enabled == BR_NO_STP)
> > +			if (p->br->stp_enabled == BR_NO_STP ||
> > +			    (br_vlan_enabled(p->br) &&
> > +			     br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
> > +			     p->br->group_addr[5] != dest[5]))
> >  				goto forward;
> >  			break;
> >  
> >  		case 0x01:	/* IEEE MAC (Pause) */
> >  			goto drop;
> >  
> > +		case 0x0B:
> > +		case 0x0C:
> > +		case 0x0D:
> > +		case 0x0F:
> > +			/* The Nearest Customer Bridge group address */
> > +			if (br_vlan_enabled(p->br) &&
> > +			    br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
> > +			    p->br->group_addr[5] != dest[5])
> > +				goto forward;
> > +			/* fall through */
> >  		default:
> >  			/* Allow selective forwarding for most other protocols */
> >  			if (p->br->group_fwd_mask & (1u << dest[5]))
> > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > index b65fee9..65204c2 100644
> > --- a/net/bridge/br_private.h
> > +++ b/net/bridge/br_private.h
> > @@ -647,6 +647,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
> >  {
> >  	return br->vlan_enabled;
> >  }
> > +
> > +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
> > +{
> > +	return br->vlan_proto;
> > +}
> >  #else
> >  static inline bool br_allowed_ingress(struct net_bridge *br,
> >  				      struct net_port_vlans *v,
> > @@ -742,6 +747,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
> >  {
> >  	return 0;
> >  }
> > +
> > +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
> > +{
> > +	return 0;
> > +}
> >  #endif
> >  
> >  /* br_netfilter.c */
> 
> Rather than special casing this around vlan filtering, I would prefer
> the code always forward these packets, or manipulate group_fwd_mask
> to allow it that way.

These addresses must be forwarded only if the bridge is an S-VLAN
bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
forwarded. So, I don't think we can forward them always.

Using group_fwd_mask is a bit complicated. If we use it to forward them,
user can optionally turn off forwarding ability of those addresses...
but we maybe need another information (named like group_fwd_mask_set)
that indicates which bit is set by user. (We have to set group_fwd_mask
automatically when we set vlan_proto to 88a8.)
Is this way acceptable?

Thanks,
Toshiaki Makita

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [Bridge] [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
@ 2014-06-09 16:45       ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-09 16:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Vlad Yasevich, netdev, bridge, David S . Miller

On Mon, 2014-06-09 at 08:52 -0700, Stephen Hemminger wrote:
> On Mon,  9 Jun 2014 20:34:46 +0900
> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> 
> > An 802.1ad bridge must forward the Nearest Customer Bridge group addresses.
> >  01-80-C2-00-00-00
> >  01-80-C2-00-00-0B
> >  01-80-C2-00-00-0C
> >  01-80-C2-00-00-0D
> >  01-80-C2-00-00-0F
> > (For details, see IEEE 802.1Q-2011 8.6.3.)
> > 
> > An exception is the br->group_addr, which needs to be passed to the higher
> > layer entity so that STP works.
> > 
> > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> > ---
> >  net/bridge/br_input.c   | 15 ++++++++++++++-
> >  net/bridge/br_private.h | 10 ++++++++++
> >  2 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> > index 04d6348..b05d419 100644
> > --- a/net/bridge/br_input.c
> > +++ b/net/bridge/br_input.c
> > @@ -194,13 +194,26 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
> >  		case 0x00:	/* Bridge Group Address */
> >  			/* If STP is turned off,
> >  			   then must forward to keep loop detection */
> > -			if (p->br->stp_enabled == BR_NO_STP)
> > +			if (p->br->stp_enabled == BR_NO_STP ||
> > +			    (br_vlan_enabled(p->br) &&
> > +			     br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
> > +			     p->br->group_addr[5] != dest[5]))
> >  				goto forward;
> >  			break;
> >  
> >  		case 0x01:	/* IEEE MAC (Pause) */
> >  			goto drop;
> >  
> > +		case 0x0B:
> > +		case 0x0C:
> > +		case 0x0D:
> > +		case 0x0F:
> > +			/* The Nearest Customer Bridge group address */
> > +			if (br_vlan_enabled(p->br) &&
> > +			    br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
> > +			    p->br->group_addr[5] != dest[5])
> > +				goto forward;
> > +			/* fall through */
> >  		default:
> >  			/* Allow selective forwarding for most other protocols */
> >  			if (p->br->group_fwd_mask & (1u << dest[5]))
> > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > index b65fee9..65204c2 100644
> > --- a/net/bridge/br_private.h
> > +++ b/net/bridge/br_private.h
> > @@ -647,6 +647,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
> >  {
> >  	return br->vlan_enabled;
> >  }
> > +
> > +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
> > +{
> > +	return br->vlan_proto;
> > +}
> >  #else
> >  static inline bool br_allowed_ingress(struct net_bridge *br,
> >  				      struct net_port_vlans *v,
> > @@ -742,6 +747,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
> >  {
> >  	return 0;
> >  }
> > +
> > +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
> > +{
> > +	return 0;
> > +}
> >  #endif
> >  
> >  /* br_netfilter.c */
> 
> Rather than special casing this around vlan filtering, I would prefer
> the code always forward these packets, or manipulate group_fwd_mask
> to allow it that way.

These addresses must be forwarded only if the bridge is an S-VLAN
bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
forwarded. So, I don't think we can forward them always.

Using group_fwd_mask is a bit complicated. If we use it to forward them,
user can optionally turn off forwarding ability of those addresses...
but we maybe need another information (named like group_fwd_mask_set)
that indicates which bit is set by user. (We have to set group_fwd_mask
automatically when we set vlan_proto to 88a8.)
Is this way acceptable?

Thanks,
Toshiaki Makita


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
  2014-06-09 16:45       ` [Bridge] " Toshiaki Makita
@ 2014-06-09 22:33         ` Vlad Yasevich
  -1 siblings, 0 replies; 26+ messages in thread
From: Vlad Yasevich @ 2014-06-09 22:33 UTC (permalink / raw)
  To: Toshiaki Makita, Stephen Hemminger; +Cc: netdev, bridge, David S . Miller

On 06/09/2014 12:45 PM, Toshiaki Makita wrote:
> On Mon, 2014-06-09 at 08:52 -0700, Stephen Hemminger wrote:
>> On Mon,  9 Jun 2014 20:34:46 +0900
>> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
>>
>>> An 802.1ad bridge must forward the Nearest Customer Bridge group addresses.
>>>  01-80-C2-00-00-00
>>>  01-80-C2-00-00-0B
>>>  01-80-C2-00-00-0C
>>>  01-80-C2-00-00-0D
>>>  01-80-C2-00-00-0F
>>> (For details, see IEEE 802.1Q-2011 8.6.3.)
>>>
>>> An exception is the br->group_addr, which needs to be passed to the higher
>>> layer entity so that STP works.
>>>
>>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>> ---
>>>  net/bridge/br_input.c   | 15 ++++++++++++++-
>>>  net/bridge/br_private.h | 10 ++++++++++
>>>  2 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>> index 04d6348..b05d419 100644
>>> --- a/net/bridge/br_input.c
>>> +++ b/net/bridge/br_input.c
>>> @@ -194,13 +194,26 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
>>>  		case 0x00:	/* Bridge Group Address */
>>>  			/* If STP is turned off,
>>>  			   then must forward to keep loop detection */
>>> -			if (p->br->stp_enabled == BR_NO_STP)
>>> +			if (p->br->stp_enabled == BR_NO_STP ||
>>> +			    (br_vlan_enabled(p->br) &&
>>> +			     br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
>>> +			     p->br->group_addr[5] != dest[5]))
>>>  				goto forward;
>>>  			break;
>>>  
>>>  		case 0x01:	/* IEEE MAC (Pause) */
>>>  			goto drop;
>>>  
>>> +		case 0x0B:
>>> +		case 0x0C:
>>> +		case 0x0D:
>>> +		case 0x0F:
>>> +			/* The Nearest Customer Bridge group address */
>>> +			if (br_vlan_enabled(p->br) &&
>>> +			    br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
>>> +			    p->br->group_addr[5] != dest[5])
>>> +				goto forward;
>>> +			/* fall through */
>>>  		default:
>>>  			/* Allow selective forwarding for most other protocols */
>>>  			if (p->br->group_fwd_mask & (1u << dest[5]))
>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>> index b65fee9..65204c2 100644
>>> --- a/net/bridge/br_private.h
>>> +++ b/net/bridge/br_private.h
>>> @@ -647,6 +647,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
>>>  {
>>>  	return br->vlan_enabled;
>>>  }
>>> +
>>> +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
>>> +{
>>> +	return br->vlan_proto;
>>> +}
>>>  #else
>>>  static inline bool br_allowed_ingress(struct net_bridge *br,
>>>  				      struct net_port_vlans *v,
>>> @@ -742,6 +747,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
>>>  {
>>>  	return 0;
>>>  }
>>> +
>>> +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
>>> +{
>>> +	return 0;
>>> +}
>>>  #endif
>>>  
>>>  /* br_netfilter.c */
>>
>> Rather than special casing this around vlan filtering, I would prefer
>> the code always forward these packets, or manipulate group_fwd_mask
>> to allow it that way.
> 
> These addresses must be forwarded only if the bridge is an S-VLAN
> bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
> forwarded. So, I don't think we can forward them always.
> 
> Using group_fwd_mask is a bit complicated. If we use it to forward them,
> user can optionally turn off forwarding ability of those addresses...
> but we maybe need another information (named like group_fwd_mask_set)
> that indicates which bit is set by user. (We have to set group_fwd_mask
> automatically when we set vlan_proto to 88a8.)
> Is this way acceptable?

May be separate it into required mask and user mask.  Set required
mask when this is an S-VLAN bridge.

-vlad

> 
> Thanks,
> Toshiaki Makita
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [Bridge] [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
@ 2014-06-09 22:33         ` Vlad Yasevich
  0 siblings, 0 replies; 26+ messages in thread
From: Vlad Yasevich @ 2014-06-09 22:33 UTC (permalink / raw)
  To: Toshiaki Makita, Stephen Hemminger; +Cc: netdev, bridge, David S . Miller

On 06/09/2014 12:45 PM, Toshiaki Makita wrote:
> On Mon, 2014-06-09 at 08:52 -0700, Stephen Hemminger wrote:
>> On Mon,  9 Jun 2014 20:34:46 +0900
>> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
>>
>>> An 802.1ad bridge must forward the Nearest Customer Bridge group addresses.
>>>  01-80-C2-00-00-00
>>>  01-80-C2-00-00-0B
>>>  01-80-C2-00-00-0C
>>>  01-80-C2-00-00-0D
>>>  01-80-C2-00-00-0F
>>> (For details, see IEEE 802.1Q-2011 8.6.3.)
>>>
>>> An exception is the br->group_addr, which needs to be passed to the higher
>>> layer entity so that STP works.
>>>
>>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>> ---
>>>  net/bridge/br_input.c   | 15 ++++++++++++++-
>>>  net/bridge/br_private.h | 10 ++++++++++
>>>  2 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>> index 04d6348..b05d419 100644
>>> --- a/net/bridge/br_input.c
>>> +++ b/net/bridge/br_input.c
>>> @@ -194,13 +194,26 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
>>>  		case 0x00:	/* Bridge Group Address */
>>>  			/* If STP is turned off,
>>>  			   then must forward to keep loop detection */
>>> -			if (p->br->stp_enabled == BR_NO_STP)
>>> +			if (p->br->stp_enabled == BR_NO_STP ||
>>> +			    (br_vlan_enabled(p->br) &&
>>> +			     br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
>>> +			     p->br->group_addr[5] != dest[5]))
>>>  				goto forward;
>>>  			break;
>>>  
>>>  		case 0x01:	/* IEEE MAC (Pause) */
>>>  			goto drop;
>>>  
>>> +		case 0x0B:
>>> +		case 0x0C:
>>> +		case 0x0D:
>>> +		case 0x0F:
>>> +			/* The Nearest Customer Bridge group address */
>>> +			if (br_vlan_enabled(p->br) &&
>>> +			    br_vlan_get_proto(p->br) == htons(ETH_P_8021AD) &&
>>> +			    p->br->group_addr[5] != dest[5])
>>> +				goto forward;
>>> +			/* fall through */
>>>  		default:
>>>  			/* Allow selective forwarding for most other protocols */
>>>  			if (p->br->group_fwd_mask & (1u << dest[5]))
>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>> index b65fee9..65204c2 100644
>>> --- a/net/bridge/br_private.h
>>> +++ b/net/bridge/br_private.h
>>> @@ -647,6 +647,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
>>>  {
>>>  	return br->vlan_enabled;
>>>  }
>>> +
>>> +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
>>> +{
>>> +	return br->vlan_proto;
>>> +}
>>>  #else
>>>  static inline bool br_allowed_ingress(struct net_bridge *br,
>>>  				      struct net_port_vlans *v,
>>> @@ -742,6 +747,11 @@ static inline int br_vlan_enabled(struct net_bridge *br)
>>>  {
>>>  	return 0;
>>>  }
>>> +
>>> +static inline __be16 br_vlan_get_proto(struct net_bridge *br)
>>> +{
>>> +	return 0;
>>> +}
>>>  #endif
>>>  
>>>  /* br_netfilter.c */
>>
>> Rather than special casing this around vlan filtering, I would prefer
>> the code always forward these packets, or manipulate group_fwd_mask
>> to allow it that way.
> 
> These addresses must be forwarded only if the bridge is an S-VLAN
> bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
> forwarded. So, I don't think we can forward them always.
> 
> Using group_fwd_mask is a bit complicated. If we use it to forward them,
> user can optionally turn off forwarding ability of those addresses...
> but we maybe need another information (named like group_fwd_mask_set)
> that indicates which bit is set by user. (We have to set group_fwd_mask
> automatically when we set vlan_proto to 88a8.)
> Is this way acceptable?

May be separate it into required mask and user mask.  Set required
mask when this is an S-VLAN bridge.

-vlad

> 
> Thanks,
> Toshiaki Makita
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH net-next 4/4] bridge: Support 802.1ad vlan filtering
  2014-06-09 11:34   ` [Bridge] " Toshiaki Makita
@ 2014-06-10  0:50     ` Vlad Yasevich
  -1 siblings, 0 replies; 26+ messages in thread
From: Vlad Yasevich @ 2014-06-10  0:50 UTC (permalink / raw)
  To: Toshiaki Makita, David S . Miller, Stephen Hemminger; +Cc: netdev, bridge

On 06/09/2014 07:34 AM, Toshiaki Makita wrote:
> This enables us to change the vlan protocol for vlan filtering.
> We come to be able to filter frames on the basis of 802.1ad vlan tags
> through a bridge.
> 
> This also changes br->group_addr if it has not been set by user.
> This is needed for an 802.1ad bridge.
> (See IEEE 802.1Q-2011 8.13.5.)
> 
> To change the vlan protocol, write a protocol in sysfs:
> # echo 0x88a8 > /sys/class/net/br0/bridge/vlan_protocol
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>  net/bridge/br_private.h  |  2 ++
>  net/bridge/br_sysfs_br.c | 18 +++++++++++
>  net/bridge/br_vlan.c     | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 101 insertions(+)
> 
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 65204c2..3c5b23b 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -246,6 +246,7 @@ struct net_bridge
>  	unsigned long			bridge_forward_delay;
>  
>  	u8				group_addr[ETH_ALEN];
> +	unsigned char			group_addr_set;

nit:  can be bool since you just use true/false.

-vlad

>  	u16				root_port;
>  
>  	enum {
> @@ -599,6 +600,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);
> +int br_vlan_set_proto(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);
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index 8dac6555..1831018 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -315,6 +315,7 @@ static ssize_t group_addr_store(struct device *d,
>  	spin_lock_bh(&br->lock);
>  	for (i = 0; i < 6; i++)
>  		br->group_addr[i] = new_addr[i];
> +	br->group_addr_set = 1;
>  	spin_unlock_bh(&br->lock);
>  	return len;
>  }
> @@ -700,6 +701,22 @@ static ssize_t vlan_filtering_store(struct device *d,
>  	return store_bridge_parm(d, buf, len, br_vlan_filter_toggle);
>  }
>  static DEVICE_ATTR_RW(vlan_filtering);
> +
> +static ssize_t vlan_protocol_show(struct device *d,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct net_bridge *br = to_bridge(d);
> +	return sprintf(buf, "%#06x\n", ntohs(br->vlan_proto));
> +}
> +
> +static ssize_t vlan_protocol_store(struct device *d,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t len)
> +{
> +	return store_bridge_parm(d, buf, len, br_vlan_set_proto);
> +}
> +static DEVICE_ATTR_RW(vlan_protocol);
>  #endif
>  
>  static struct attribute *bridge_attrs[] = {
> @@ -745,6 +762,7 @@ static struct attribute *bridge_attrs[] = {
>  #endif
>  #ifdef CONFIG_BRIDGE_VLAN_FILTERING
>  	&dev_attr_vlan_filtering.attr,
> +	&dev_attr_vlan_protocol.attr,
>  #endif
>  	NULL
>  };
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 63bd981..c86a7a6 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -394,6 +394,87 @@ unlock:
>  	return 0;
>  }
>  
> +int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
> +{
> +	int err = 0;
> +	struct net_bridge_port *p;
> +	struct net_port_vlans *pv;
> +	__be16 proto, oldproto;
> +	u16 vid, errvid;
> +
> +	if (val != ETH_P_8021Q && val != ETH_P_8021AD)
> +		return -EPROTONOSUPPORT;
> +
> +	if (!rtnl_trylock())
> +		return restart_syscall();
> +
> +	proto = htons(val);
> +	if (br->vlan_proto == proto)
> +		goto unlock;
> +
> +	/* 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 err_filt;
> +		}
> +	}
> +
> +	spin_lock_bh(&br->lock);
> +	if (!br->group_addr_set) {
> +		switch (val) {
> +		case ETH_P_8021Q:
> +			/* Bridge Group Address */
> +			br->group_addr[5] = 0x00;
> +			break;
> +
> +		case ETH_P_8021AD:
> +			/* Provider Bridge Group Address */
> +			br->group_addr[5] = 0x08;
> +			break;
> +		}
> +	}
> +	spin_unlock_bh(&br->lock);
> +
> +	oldproto = br->vlan_proto;
> +	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);
> +	}
> +
> +unlock:
> +	rtnl_unlock();
> +	return err;
> +
> +err_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);
> +	}
> +
> +	goto unlock;
> +}
> +
>  void br_vlan_init(struct net_bridge *br)
>  {
>  	br->vlan_proto = htons(ETH_P_8021Q);
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [Bridge] [PATCH net-next 4/4] bridge: Support 802.1ad vlan filtering
@ 2014-06-10  0:50     ` Vlad Yasevich
  0 siblings, 0 replies; 26+ messages in thread
From: Vlad Yasevich @ 2014-06-10  0:50 UTC (permalink / raw)
  To: Toshiaki Makita, David S . Miller, Stephen Hemminger; +Cc: netdev, bridge

On 06/09/2014 07:34 AM, Toshiaki Makita wrote:
> This enables us to change the vlan protocol for vlan filtering.
> We come to be able to filter frames on the basis of 802.1ad vlan tags
> through a bridge.
> 
> This also changes br->group_addr if it has not been set by user.
> This is needed for an 802.1ad bridge.
> (See IEEE 802.1Q-2011 8.13.5.)
> 
> To change the vlan protocol, write a protocol in sysfs:
> # echo 0x88a8 > /sys/class/net/br0/bridge/vlan_protocol
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>  net/bridge/br_private.h  |  2 ++
>  net/bridge/br_sysfs_br.c | 18 +++++++++++
>  net/bridge/br_vlan.c     | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 101 insertions(+)
> 
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 65204c2..3c5b23b 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -246,6 +246,7 @@ struct net_bridge
>  	unsigned long			bridge_forward_delay;
>  
>  	u8				group_addr[ETH_ALEN];
> +	unsigned char			group_addr_set;

nit:  can be bool since you just use true/false.

-vlad

>  	u16				root_port;
>  
>  	enum {
> @@ -599,6 +600,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);
> +int br_vlan_set_proto(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);
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index 8dac6555..1831018 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -315,6 +315,7 @@ static ssize_t group_addr_store(struct device *d,
>  	spin_lock_bh(&br->lock);
>  	for (i = 0; i < 6; i++)
>  		br->group_addr[i] = new_addr[i];
> +	br->group_addr_set = 1;
>  	spin_unlock_bh(&br->lock);
>  	return len;
>  }
> @@ -700,6 +701,22 @@ static ssize_t vlan_filtering_store(struct device *d,
>  	return store_bridge_parm(d, buf, len, br_vlan_filter_toggle);
>  }
>  static DEVICE_ATTR_RW(vlan_filtering);
> +
> +static ssize_t vlan_protocol_show(struct device *d,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct net_bridge *br = to_bridge(d);
> +	return sprintf(buf, "%#06x\n", ntohs(br->vlan_proto));
> +}
> +
> +static ssize_t vlan_protocol_store(struct device *d,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t len)
> +{
> +	return store_bridge_parm(d, buf, len, br_vlan_set_proto);
> +}
> +static DEVICE_ATTR_RW(vlan_protocol);
>  #endif
>  
>  static struct attribute *bridge_attrs[] = {
> @@ -745,6 +762,7 @@ static struct attribute *bridge_attrs[] = {
>  #endif
>  #ifdef CONFIG_BRIDGE_VLAN_FILTERING
>  	&dev_attr_vlan_filtering.attr,
> +	&dev_attr_vlan_protocol.attr,
>  #endif
>  	NULL
>  };
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 63bd981..c86a7a6 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -394,6 +394,87 @@ unlock:
>  	return 0;
>  }
>  
> +int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
> +{
> +	int err = 0;
> +	struct net_bridge_port *p;
> +	struct net_port_vlans *pv;
> +	__be16 proto, oldproto;
> +	u16 vid, errvid;
> +
> +	if (val != ETH_P_8021Q && val != ETH_P_8021AD)
> +		return -EPROTONOSUPPORT;
> +
> +	if (!rtnl_trylock())
> +		return restart_syscall();
> +
> +	proto = htons(val);
> +	if (br->vlan_proto == proto)
> +		goto unlock;
> +
> +	/* 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 err_filt;
> +		}
> +	}
> +
> +	spin_lock_bh(&br->lock);
> +	if (!br->group_addr_set) {
> +		switch (val) {
> +		case ETH_P_8021Q:
> +			/* Bridge Group Address */
> +			br->group_addr[5] = 0x00;
> +			break;
> +
> +		case ETH_P_8021AD:
> +			/* Provider Bridge Group Address */
> +			br->group_addr[5] = 0x08;
> +			break;
> +		}
> +	}
> +	spin_unlock_bh(&br->lock);
> +
> +	oldproto = br->vlan_proto;
> +	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);
> +	}
> +
> +unlock:
> +	rtnl_unlock();
> +	return err;
> +
> +err_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);
> +	}
> +
> +	goto unlock;
> +}
> +
>  void br_vlan_init(struct net_bridge *br)
>  {
>  	br->vlan_proto = htons(ETH_P_8021Q);
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
  2014-06-09 22:33         ` [Bridge] " Vlad Yasevich
@ 2014-06-10  7:05           ` Toshiaki Makita
  -1 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-10  7:05 UTC (permalink / raw)
  To: vyasevic, Toshiaki Makita, Stephen Hemminger
  Cc: netdev, bridge, David S . Miller

(2014/06/10 7:33), Vlad Yasevich wrote:
...
>>> Rather than special casing this around vlan filtering, I would prefer
>>> the code always forward these packets, or manipulate group_fwd_mask
>>> to allow it that way.
>>
>> These addresses must be forwarded only if the bridge is an S-VLAN
>> bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
>> forwarded. So, I don't think we can forward them always.
>>
>> Using group_fwd_mask is a bit complicated. If we use it to forward them,
>> user can optionally turn off forwarding ability of those addresses...
>> but we maybe need another information (named like group_fwd_mask_set)
>> that indicates which bit is set by user. (We have to set group_fwd_mask
>> automatically when we set vlan_proto to 88a8.)
>> Is this way acceptable?
> 
> May be separate it into required mask and user mask.  Set required
> mask when this is an S-VLAN bridge.

Sounds like a good idea.
I'll give it a try, thank you for your suggestion.

Thanks,
Toshiaki Makita

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [Bridge] [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
@ 2014-06-10  7:05           ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-10  7:05 UTC (permalink / raw)
  To: vyasevic, Toshiaki Makita, Stephen Hemminger
  Cc: netdev, bridge, David S . Miller

(2014/06/10 7:33), Vlad Yasevich wrote:
...
>>> Rather than special casing this around vlan filtering, I would prefer
>>> the code always forward these packets, or manipulate group_fwd_mask
>>> to allow it that way.
>>
>> These addresses must be forwarded only if the bridge is an S-VLAN
>> bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
>> forwarded. So, I don't think we can forward them always.
>>
>> Using group_fwd_mask is a bit complicated. If we use it to forward them,
>> user can optionally turn off forwarding ability of those addresses...
>> but we maybe need another information (named like group_fwd_mask_set)
>> that indicates which bit is set by user. (We have to set group_fwd_mask
>> automatically when we set vlan_proto to 88a8.)
>> Is this way acceptable?
> 
> May be separate it into required mask and user mask.  Set required
> mask when this is an S-VLAN bridge.

Sounds like a good idea.
I'll give it a try, thank you for your suggestion.

Thanks,
Toshiaki Makita


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH net-next 4/4] bridge: Support 802.1ad vlan filtering
  2014-06-10  0:50     ` [Bridge] " Vlad Yasevich
@ 2014-06-10  7:15       ` Toshiaki Makita
  -1 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-10  7:15 UTC (permalink / raw)
  To: Vlad Yasevich, David S . Miller, Stephen Hemminger; +Cc: netdev, bridge

(2014/06/10 9:50), Vlad Yasevich wrote:
> On 06/09/2014 07:34 AM, Toshiaki Makita wrote:
>> This enables us to change the vlan protocol for vlan filtering.
>> We come to be able to filter frames on the basis of 802.1ad vlan tags
>> through a bridge.
>>
>> This also changes br->group_addr if it has not been set by user.
>> This is needed for an 802.1ad bridge.
>> (See IEEE 802.1Q-2011 8.13.5.)
>>
>> To change the vlan protocol, write a protocol in sysfs:
>> # echo 0x88a8 > /sys/class/net/br0/bridge/vlan_protocol
>>
>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> ---
>>  net/bridge/br_private.h  |  2 ++
>>  net/bridge/br_sysfs_br.c | 18 +++++++++++
>>  net/bridge/br_vlan.c     | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 101 insertions(+)
>>
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 65204c2..3c5b23b 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -246,6 +246,7 @@ struct net_bridge
>>  	unsigned long			bridge_forward_delay;
>>  
>>  	u8				group_addr[ETH_ALEN];
>> +	unsigned char			group_addr_set;
> 
> nit:  can be bool since you just use true/false.

I'm not sure which is better in struct... but maybe it's more explicit.
will change it in v2.

Thanks,
Toshiaki Makita

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [Bridge] [PATCH net-next 4/4] bridge: Support 802.1ad vlan filtering
@ 2014-06-10  7:15       ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-10  7:15 UTC (permalink / raw)
  To: Vlad Yasevich, David S . Miller, Stephen Hemminger; +Cc: netdev, bridge

(2014/06/10 9:50), Vlad Yasevich wrote:
> On 06/09/2014 07:34 AM, Toshiaki Makita wrote:
>> This enables us to change the vlan protocol for vlan filtering.
>> We come to be able to filter frames on the basis of 802.1ad vlan tags
>> through a bridge.
>>
>> This also changes br->group_addr if it has not been set by user.
>> This is needed for an 802.1ad bridge.
>> (See IEEE 802.1Q-2011 8.13.5.)
>>
>> To change the vlan protocol, write a protocol in sysfs:
>> # echo 0x88a8 > /sys/class/net/br0/bridge/vlan_protocol
>>
>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> ---
>>  net/bridge/br_private.h  |  2 ++
>>  net/bridge/br_sysfs_br.c | 18 +++++++++++
>>  net/bridge/br_vlan.c     | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 101 insertions(+)
>>
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 65204c2..3c5b23b 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -246,6 +246,7 @@ struct net_bridge
>>  	unsigned long			bridge_forward_delay;
>>  
>>  	u8				group_addr[ETH_ALEN];
>> +	unsigned char			group_addr_set;
> 
> nit:  can be bool since you just use true/false.

I'm not sure which is better in struct... but maybe it's more explicit.
will change it in v2.

Thanks,
Toshiaki Makita

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
  2014-06-10  7:05           ` [Bridge] " Toshiaki Makita
@ 2014-06-10 16:21             ` Stephen Hemminger
  -1 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2014-06-10 16:21 UTC (permalink / raw)
  To: Toshiaki Makita; +Cc: vyasevic, netdev, David S . Miller, bridge

On Tue, 10 Jun 2014 16:05:07 +0900
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:

> (2014/06/10 7:33), Vlad Yasevich wrote:
> ...
> >>> Rather than special casing this around vlan filtering, I would prefer
> >>> the code always forward these packets, or manipulate group_fwd_mask
> >>> to allow it that way.
> >>
> >> These addresses must be forwarded only if the bridge is an S-VLAN
> >> bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
> >> forwarded. So, I don't think we can forward them always.
> >>
> >> Using group_fwd_mask is a bit complicated. If we use it to forward them,
> >> user can optionally turn off forwarding ability of those addresses...
> >> but we maybe need another information (named like group_fwd_mask_set)
> >> that indicates which bit is set by user. (We have to set group_fwd_mask
> >> automatically when we set vlan_proto to 88a8.)
> >> Is this way acceptable?
> > 
> > May be separate it into required mask and user mask.  Set required
> > mask when this is an S-VLAN bridge.
> 
> Sounds like a good idea.
> I'll give it a try, thank you for your suggestion.
> 
> Thanks,
> Toshiaki Makita
> 

Looking again at the code.

1. If doing vlan then it should forward frames as defined in standard
   by default.

2. For compatiability, and for those users doing bump-on-wire, allow
   forwarding via group mask.

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [Bridge] [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
@ 2014-06-10 16:21             ` Stephen Hemminger
  0 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2014-06-10 16:21 UTC (permalink / raw)
  To: Toshiaki Makita; +Cc: vyasevic, netdev, David S . Miller, bridge

On Tue, 10 Jun 2014 16:05:07 +0900
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:

> (2014/06/10 7:33), Vlad Yasevich wrote:
> ...
> >>> Rather than special casing this around vlan filtering, I would prefer
> >>> the code always forward these packets, or manipulate group_fwd_mask
> >>> to allow it that way.
> >>
> >> These addresses must be forwarded only if the bridge is an S-VLAN
> >> bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
> >> forwarded. So, I don't think we can forward them always.
> >>
> >> Using group_fwd_mask is a bit complicated. If we use it to forward them,
> >> user can optionally turn off forwarding ability of those addresses...
> >> but we maybe need another information (named like group_fwd_mask_set)
> >> that indicates which bit is set by user. (We have to set group_fwd_mask
> >> automatically when we set vlan_proto to 88a8.)
> >> Is this way acceptable?
> > 
> > May be separate it into required mask and user mask.  Set required
> > mask when this is an S-VLAN bridge.
> 
> Sounds like a good idea.
> I'll give it a try, thank you for your suggestion.
> 
> Thanks,
> Toshiaki Makita
> 

Looking again at the code.

1. If doing vlan then it should forward frames as defined in standard
   by default.

2. For compatiability, and for those users doing bump-on-wire, allow
   forwarding via group mask.


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
  2014-06-10 16:21             ` [Bridge] " Stephen Hemminger
@ 2014-06-11  6:12               ` Toshiaki Makita
  -1 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-11  6:12 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: vyasevic, netdev, David S . Miller, bridge

(2014/06/11 1:21), Stephen Hemminger wrote:
> On Tue, 10 Jun 2014 16:05:07 +0900
> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> 
>> (2014/06/10 7:33), Vlad Yasevich wrote:
>> ...
>>>>> Rather than special casing this around vlan filtering, I would prefer
>>>>> the code always forward these packets, or manipulate group_fwd_mask
>>>>> to allow it that way.
>>>>
>>>> These addresses must be forwarded only if the bridge is an S-VLAN
>>>> bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
>>>> forwarded. So, I don't think we can forward them always.
>>>>
>>>> Using group_fwd_mask is a bit complicated. If we use it to forward them,
>>>> user can optionally turn off forwarding ability of those addresses...
>>>> but we maybe need another information (named like group_fwd_mask_set)
>>>> that indicates which bit is set by user. (We have to set group_fwd_mask
>>>> automatically when we set vlan_proto to 88a8.)
>>>> Is this way acceptable?
>>>
>>> May be separate it into required mask and user mask.  Set required
>>> mask when this is an S-VLAN bridge.
>>
>> Sounds like a good idea.
>> I'll give it a try, thank you for your suggestion.
>>
>> Thanks,
>> Toshiaki Makita
>>
> 
> Looking again at the code.
> 
> 1. If doing vlan then it should forward frames as defined in standard
>    by default.
> 
> 2. For compatiability, and for those users doing bump-on-wire, allow
>    forwarding via group mask.
> 

Thank you for reviewing. I sent v2 and I'm thinking it can satisfy these
two.

Thanks,
Toshiaki Makita

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [Bridge] [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses
@ 2014-06-11  6:12               ` Toshiaki Makita
  0 siblings, 0 replies; 26+ messages in thread
From: Toshiaki Makita @ 2014-06-11  6:12 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: vyasevic, netdev, David S . Miller, bridge

(2014/06/11 1:21), Stephen Hemminger wrote:
> On Tue, 10 Jun 2014 16:05:07 +0900
> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> 
>> (2014/06/10 7:33), Vlad Yasevich wrote:
>> ...
>>>>> Rather than special casing this around vlan filtering, I would prefer
>>>>> the code always forward these packets, or manipulate group_fwd_mask
>>>>> to allow it that way.
>>>>
>>>> These addresses must be forwarded only if the bridge is an S-VLAN
>>>> bridge. When it is a C-VLAN bridge or a .1D bridge, they may not be
>>>> forwarded. So, I don't think we can forward them always.
>>>>
>>>> Using group_fwd_mask is a bit complicated. If we use it to forward them,
>>>> user can optionally turn off forwarding ability of those addresses...
>>>> but we maybe need another information (named like group_fwd_mask_set)
>>>> that indicates which bit is set by user. (We have to set group_fwd_mask
>>>> automatically when we set vlan_proto to 88a8.)
>>>> Is this way acceptable?
>>>
>>> May be separate it into required mask and user mask.  Set required
>>> mask when this is an S-VLAN bridge.
>>
>> Sounds like a good idea.
>> I'll give it a try, thank you for your suggestion.
>>
>> Thanks,
>> Toshiaki Makita
>>
> 
> Looking again at the code.
> 
> 1. If doing vlan then it should forward frames as defined in standard
>    by default.
> 
> 2. For compatiability, and for those users doing bump-on-wire, allow
>    forwarding via group mask.
> 

Thank you for reviewing. I sent v2 and I'm thinking it can satisfy these
two.

Thanks,
Toshiaki Makita

^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2014-06-11  6:12 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-09 11:34 [PATCH net-next 0/4] bridge: 802.1ad vlan protocol support Toshiaki Makita
2014-06-09 11:34 ` [Bridge] " Toshiaki Makita
2014-06-09 11:34 ` [PATCH net-next 1/4] bridge: Add 802.1ad tx vlan acceleration Toshiaki Makita
2014-06-09 11:34   ` [Bridge] " Toshiaki Makita
2014-06-09 11:34 ` [PATCH net-next 2/4] bridge: Prepare for 802.1ad vlan filtering support Toshiaki Makita
2014-06-09 11:34   ` [Bridge] " Toshiaki Makita
2014-06-09 11:34 ` [PATCH net-next 3/4] bridge: Consider the Nearest Customer Bridge group addresses Toshiaki Makita
2014-06-09 11:34   ` [Bridge] " Toshiaki Makita
2014-06-09 15:52   ` Stephen Hemminger
2014-06-09 15:52     ` [Bridge] " Stephen Hemminger
2014-06-09 16:45     ` Toshiaki Makita
2014-06-09 16:45       ` [Bridge] " Toshiaki Makita
2014-06-09 22:33       ` Vlad Yasevich
2014-06-09 22:33         ` [Bridge] " Vlad Yasevich
2014-06-10  7:05         ` Toshiaki Makita
2014-06-10  7:05           ` [Bridge] " Toshiaki Makita
2014-06-10 16:21           ` Stephen Hemminger
2014-06-10 16:21             ` [Bridge] " Stephen Hemminger
2014-06-11  6:12             ` Toshiaki Makita
2014-06-11  6:12               ` [Bridge] " Toshiaki Makita
2014-06-09 11:34 ` [PATCH net-next 4/4] bridge: Support 802.1ad vlan filtering Toshiaki Makita
2014-06-09 11:34   ` [Bridge] " Toshiaki Makita
2014-06-10  0:50   ` Vlad Yasevich
2014-06-10  0:50     ` [Bridge] " Vlad Yasevich
2014-06-10  7:15     ` Toshiaki Makita
2014-06-10  7:15       ` [Bridge] " Toshiaki Makita

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.