Skip to contentSkip to conversation

net: bridge: Notify about bridge VLANs

PATCH In review Petr Machata 4 participants 14 messages May 30, 2018 – May 31, 2018

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

v4 cover net: bridge: Notify about bridge VLANs

In commit 946a11e7408e ("mlxsw: spectrum_span: Allow bridge for gretap mirror"), mlxsw got support for offloading mirror-to-gretap such that the underlay packet path involves a bridge. In that case, the offload is also influenced by PVID setting of said bridge. However, changes to VLAN configuration of the bridge itself do not generate switchdev notifications, so there's no mechanism to prod mlxsw to update the offload when these settings change.

In this patchset, the problem is resolved by distributing the switchdev notification SWITCHDEV_OBJ_ID_PORT_VLAN also for configuration changes on bridge VLANs. Since stacked devices distribute the notification to lower devices, such event eventually reaches the driver, which can determine whether it's a bridge or port VLAN by inspecting orig_dev.

To keep things consistent, the newly-distributed notifications observe the same protocol as the existing ones: dual prepare/commit, with -EOPNOTSUPP indicating lack of support, even though there's currently nothing to prepare for and nothing to support. Correspondingly, all switchdev drivers have been updated to return -EOPNOTSUPP for bridge VLAN notifications.

In patches #1 and #2, the code base is changed to support the following
additions: functions br_switchdev_port_vlan_add() and br_switchdev_port_vlan_del() are introduced to simplify sending notifications; and br_vlan_add_existing() is introduced to later make it simpler to add error-handling code for the case of configuring a preexisting VLAN on bridge CPU port.

In patches #3-#6, respectively for mlxsw, rocker, DSA and DPAA2 ethsw, the new notifications (which are not enabled yet) are ignored to maintain the current behavior.

In patch #7, the notification is actually enabled.

In patch #8, mlxsw is changed to update offloads of mirror-to-gre also for bridge-related notifications.

Changes from v3 to v4:

- In patch #1, separate variable declarations from program logic.
- Add patch #2.
- In patch #7, add error handling around a newly-introduced call to br_switchdev_port_vlan_add().
- Rephrase commit messages of patches #3-#6 to explain motivation for the change.

Changes from v2 to v3:

- Add a fallback definition for br_switchdev_port_obj_add() and br_switchdev_port_obj_del() when !CONFIG_NET_SWITCHDEV.

Changes from v1 to v2:

- Rename br_switchdev_port_obj_add() and br_switchdev_port_obj_del() to br_switchdev_port_vlan_add() and br_switchdev_port_vlan_del(), and move from br_vlan.c to br_switchdev.c.

Petr Machata (8):
net: bridge: Extract boilerplate around switchdev_port_obj_*()
net: bridge: Extract br_vlan_add_existing()
mlxsw: spectrum_switchdev: Ignore bridge VLAN events
rocker: rocker_main: Ignore bridge VLAN events
dsa: port: Ignore bridge VLAN events
staging: fsl-dpaa2: ethsw: Ignore bridge VLAN events
net: bridge: Notify about bridge VLANs
mlxsw: spectrum_switchdev: Schedule respin during trans prepare

 .../ethernet/mellanox/mlxsw/spectrum_switchdev.c   |   8 +-
 drivers/net/ethernet/rocker/rocker_main.c          |   6 ++
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c            |   6 ++
 net/bridge/br_private.h                            |  13 +++
 net/bridge/br_switchdev.c                          |  25 +++++
 net/bridge/br_vlan.c                               | 103 ++++++++++++---------
 net/dsa/port.c                                     |   6 ++
 7 files changed, 121 insertions(+), 46 deletions(-)
Signature · 1 line
2.4.11
#1 lore raw

PATCH v4 1/8 net: bridge: Extract boilerplate around switchdev_port_obj_*()

A call to switchdev_port_obj_add() or switchdev_port_obj_del() involves initializing a struct switchdev_obj_port_vlan, a piece of code that repeats on each call site almost verbatim. While in the current codebase there is just one duplicated add call, the follow-up patches add more of both add and del calls.

Thus to remove the duplication, extract the repetition into named functions and reuse.

---
 net/bridge/br_private.h   | 13 +++++++++++++
 net/bridge/br_switchdev.c | 25 +++++++++++++++++++++++++
 net/bridge/br_vlan.c      | 26 +++-----------------------
 3 files changed, 41 insertions(+), 23 deletions(-)
net/bridge/br_private.h +13 -0
index 11520ed..5216a52 100644
@@ -1139,6 +1139,8 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
 			       unsigned long mask);
 void br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb,
 			     int type);
+int br_switchdev_port_vlan_add(struct net_device *dev, u16 vid, u16 flags);
+int br_switchdev_port_vlan_del(struct net_device *dev, u16 vid);
 
 static inline void br_switchdev_frame_unmark(struct sk_buff *skb)
 {
@@ -1168,6 +1170,17 @@ static inline int br_switchdev_set_port_flag(struct net_bridge_port *p,
 	return 0;
 }
 
+static inline int br_switchdev_port_vlan_add(struct net_device *dev,
+					     u16 vid, u16 flags)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int br_switchdev_port_vlan_del(struct net_device *dev, u16 vid)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline void
 br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type)
 {
net/bridge/br_switchdev.c +25 -0
index 35474d4..d77f807 100644
@@ -136,3 +136,28 @@ br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type)
 		break;
 	}
 }
+
+int br_switchdev_port_vlan_add(struct net_device *dev, u16 vid, u16 flags)
+{
+	struct switchdev_obj_port_vlan v = {
+		.obj.orig_dev = dev,
+		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
+		.flags = flags,
+		.vid_begin = vid,
+		.vid_end = vid,
+	};
+
+	return switchdev_port_obj_add(dev, &v.obj);
+}
+
+int br_switchdev_port_vlan_del(struct net_device *dev, u16 vid)
+{
+	struct switchdev_obj_port_vlan v = {
+		.obj.orig_dev = dev,
+		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
+		.vid_begin = vid,
+		.vid_end = vid,
+	};
+
+	return switchdev_port_obj_del(dev, &v.obj);
+}
net/bridge/br_vlan.c +3 -23
index dc832c09..e3b14c0 100644
@@ -82,19 +82,12 @@ static bool __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
 static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
 			  u16 vid, u16 flags)
 {
-	struct switchdev_obj_port_vlan v = {
-		.obj.orig_dev = dev,
-		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
-		.flags = flags,
-		.vid_begin = vid,
-		.vid_end = vid,
-	};
 	int err;
 
 	/* Try switchdev op first. In case it is not supported, fallback to
 	 * 8021q add.
 	 */
-	err = switchdev_port_obj_add(dev, &v.obj);
+	err = br_switchdev_port_vlan_add(dev, vid, flags);
 	if (err == -EOPNOTSUPP)
 		return vlan_vid_add(dev, br->vlan_proto, vid);
 	return err;
@@ -130,18 +123,12 @@ static void __vlan_del_list(struct net_bridge_vlan *v)
 static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br,
 			  u16 vid)
 {
-	struct switchdev_obj_port_vlan v = {
-		.obj.orig_dev = dev,
-		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
-		.vid_begin = vid,
-		.vid_end = vid,
-	};
 	int err;
 
 	/* Try switchdev op first. In case it is not supported, fallback to
 	 * 8021q del.
 	 */
-	err = switchdev_port_obj_del(dev, &v.obj);
+	err = br_switchdev_port_vlan_del(dev, vid);
 	if (err == -EOPNOTSUPP) {
 		vlan_vid_del(dev, br->vlan_proto, vid);
 		return 0;
@@ -1053,13 +1040,6 @@ int nbp_vlan_init(struct net_bridge_port *p)
 int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags,
 		 bool *changed)
 {
-	struct switchdev_obj_port_vlan v = {
-		.obj.orig_dev = port->dev,
-		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
-		.flags = flags,
-		.vid_begin = vid,
-		.vid_end = vid,
-	};
 	struct net_bridge_vlan *vlan;
 	int ret;
 
@@ -1069,7 +1049,7 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags,
 	vlan = br_vlan_find(nbp_vlan_group(port), vid);
 	if (vlan) {
 		/* Pass the flags to the hardware bridge */
-		ret = switchdev_port_obj_add(port->dev, &v.obj);
+		ret = br_switchdev_port_vlan_add(port->dev, vid, flags);
 		if (ret && ret != -EOPNOTSUPP)
 			return ret;
 		*changed = __vlan_add_flags(vlan, flags);
Signature · 1 line
2.4.11
Reviewed-by on v4 1/8
#2 lore raw

Re: [PATCH net-next v4 1/8] net: bridge: Extract boilerplate around switchdev_port_obj_*()

Quoted 9 lines · Petr Machata

A call to switchdev_port_obj_add() or switchdev_port_obj_del() involves initializing a struct switchdev_obj_port_vlan, a piece of code that repeats on each call site almost verbatim. While in the current codebase there is just one duplicated add call, the follow-up patches add more of both add and del calls.

Thus to remove the duplication, extract the repetition into named functions and reuse.

Quoted 8 lines
---
 net/bridge/br_private.h   | 13 +++++++++++++
 net/bridge/br_switchdev.c | 25 +++++++++++++++++++++++++
 net/bridge/br_vlan.c      | 26 +++-----------------------
 3 files changed, 41 insertions(+), 23 deletions(-)
#3 lore raw

PATCH v4 2/8 net: bridge: Extract br_vlan_add_existing()

In reply to Petr Machata

Extract the code that deals with adding a preexisting VLAN to bridge CPU port to a separate function. A follow-up patch introduces a need to roll back operations in this block due to an error, and this split will make the error-handling code clearer.

---
 net/bridge/br_vlan.c | 55 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 22 deletions(-)
net/bridge/br_vlan.c +33 -22
index e3b14c0..602c869 100644
@@ -551,6 +551,37 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
 	return false;
 }
 
+static int br_vlan_add_existing(struct net_bridge *br,
+				struct net_bridge_vlan_group *vg,
+				struct net_bridge_vlan *vlan,
+				u16 flags, bool *changed)
+{
+	int err;
+
+	if (!br_vlan_is_brentry(vlan)) {
+		/* Trying to change flags of non-existent bridge vlan */
+		if (!(flags & BRIDGE_VLAN_INFO_BRENTRY))
+			return -EINVAL;
+		/* It was only kept for port vlans, now make it real */
+		err = br_fdb_insert(br, NULL, br->dev->dev_addr,
+				    vlan->vid);
+		if (err) {
+			br_err(br, "failed to insert local address into bridge forwarding table\n");
+			return err;
+		}
+
+		refcount_inc(&vlan->refcnt);
+		vlan->flags |= BRIDGE_VLAN_INFO_BRENTRY;
+		vg->num_vlans++;
+		*changed = true;
+	}
+
+	if (__vlan_add_flags(vlan, flags))
+		*changed = true;
+
+	return 0;
+}
+
 /* Must be protected by RTNL.
  * Must be called with vid in range from 1 to 4094 inclusive.
  * changed must be true only if the vlan was created or updated
@@ -566,28 +597,8 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed)
 	*changed = false;
 	vg = br_vlan_group(br);
 	vlan = br_vlan_find(vg, vid);
-	if (vlan) {
-		if (!br_vlan_is_brentry(vlan)) {
-			/* Trying to change flags of non-existent bridge vlan */
-			if (!(flags & BRIDGE_VLAN_INFO_BRENTRY))
-				return -EINVAL;
-			/* It was only kept for port vlans, now make it real */
-			ret = br_fdb_insert(br, NULL, br->dev->dev_addr,
-					    vlan->vid);
-			if (ret) {
-				br_err(br, "failed insert local address into bridge forwarding table\n");
-				return ret;
-			}
-			refcount_inc(&vlan->refcnt);
-			vlan->flags |= BRIDGE_VLAN_INFO_BRENTRY;
-			vg->num_vlans++;
-			*changed = true;
-		}
-		if (__vlan_add_flags(vlan, flags))
-			*changed = true;
-
-		return 0;
-	}
+	if (vlan)
+		return br_vlan_add_existing(br, vg, vlan, flags, changed);
 
 	vlan = kzalloc(sizeof(*vlan), GFP_KERNEL);
 	if (!vlan)
Signature · 1 line
2.4.11
Reviewed-by on v4 2/8
#4 lore raw

Re: [PATCH net-next v4 2/8] net: bridge: Extract br_vlan_add_existing()

Quoted 10 lines · Petr Machata

Extract the code that deals with adding a preexisting VLAN to bridge CPU port to a separate function. A follow-up patch introduces a need to roll back operations in this block due to an error, and this split will make the error-handling code clearer.

---
 net/bridge/br_vlan.c | 55 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 22 deletions(-)
#5 lore raw

PATCH v4 3/8 mlxsw: spectrum_switchdev: Ignore bridge VLAN events

In reply to Petr Machata

A follow-up patch enables emitting VLAN notifications for the bridge CPU port in addition to the existing slave port notifications. These notifications have orig_dev set to the bridge in question.

Because there's no specific support for these VLANs, just ignore the notifications to maintain the current behavior.

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +6 -0
index 8c9cf8e..cbc8fab 100644
@@ -1144,6 +1144,9 @@ static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,
 	struct mlxsw_sp_bridge_port *bridge_port;
 	u16 vid;
 
+	if (netif_is_bridge_master(orig_dev))
+		return -EOPNOTSUPP;
+
 	if (switchdev_trans_ph_prepare(trans))
 		return 0;
 
@@ -1741,6 +1744,9 @@ static int mlxsw_sp_port_vlans_del(struct mlxsw_sp_port *mlxsw_sp_port,
 	struct mlxsw_sp_bridge_port *bridge_port;
 	u16 vid;
 
+	if (netif_is_bridge_master(orig_dev))
+		return -EOPNOTSUPP;
+
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp->bridge, orig_dev);
 	if (WARN_ON(!bridge_port))
 		return -EINVAL;
Signature · 1 line
2.4.11
#6 lore raw

PATCH v4 4/8 rocker: rocker_main: Ignore bridge VLAN events

In reply to Petr Machata

A follow-up patch enables emitting VLAN notifications for the bridge CPU port in addition to the existing slave port notifications. These notifications have orig_dev set to the bridge in question.

Because there's no specific support for these VLANs, just ignore the notifications to maintain the current behavior.

---
 drivers/net/ethernet/rocker/rocker_main.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/ethernet/rocker/rocker_main.c +6 -0
index e73e4fe..aeafdb9 100644
@@ -1632,6 +1632,9 @@ rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port,
 {
 	struct rocker_world_ops *wops = rocker_port->rocker->wops;
 
+	if (netif_is_bridge_master(vlan->obj.orig_dev))
+		return -EOPNOTSUPP;
+
 	if (!wops->port_obj_vlan_add)
 		return -EOPNOTSUPP;
 
@@ -1647,6 +1650,9 @@ rocker_world_port_obj_vlan_del(struct rocker_port *rocker_port,
 {
 	struct rocker_world_ops *wops = rocker_port->rocker->wops;
 
+	if (netif_is_bridge_master(vlan->obj.orig_dev))
+		return -EOPNOTSUPP;
+
 	if (!wops->port_obj_vlan_del)
 		return -EOPNOTSUPP;
 	return wops->port_obj_vlan_del(rocker_port, vlan);
Signature · 1 line
2.4.11
#7 lore raw

PATCH v4 5/8 dsa: port: Ignore bridge VLAN events

In reply to Petr Machata

A follow-up patch enables emitting VLAN notifications for the bridge CPU port in addition to the existing slave port notifications. These notifications have orig_dev set to the bridge in question.

Because there's no specific support for these VLANs, just ignore the notifications to maintain the current behavior.

---
 net/dsa/port.c | 6 ++++++
 1 file changed, 6 insertions(+)
net/dsa/port.c +6 -0
index 2413beb..ed05954 100644
@@ -252,6 +252,9 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 		.vlan = vlan,
 	};
 
+	if (netif_is_bridge_master(vlan->obj.orig_dev))
+		return -EOPNOTSUPP;
+
 	if (br_vlan_enabled(dp->bridge_dev))
 		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
 
@@ -267,6 +270,9 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 		.vlan = vlan,
 	};
 
+	if (netif_is_bridge_master(vlan->obj.orig_dev))
+		return -EOPNOTSUPP;
+
 	if (br_vlan_enabled(dp->bridge_dev))
 		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
 
Signature · 1 line
2.4.11
Reviewed-by on v4 5/8
#8 lore raw

Re: [PATCH net-next v4 5/8] dsa: port: Ignore bridge VLAN events

Quoted 9 lines · Petr Machata

A follow-up patch enables emitting VLAN notifications for the bridge CPU port in addition to the existing slave port notifications. These notifications have orig_dev set to the bridge in question.

Because there's no specific support for these VLANs, just ignore the notifications to maintain the current behavior.

Signature · 1 line
Florian
#9 lore raw

PATCH v4 6/8 staging: fsl-dpaa2: ethsw: Ignore bridge VLAN events

In reply to Petr Machata

A follow-up patch enables emitting VLAN notifications for the bridge CPU port in addition to the existing slave port notifications. These notifications have orig_dev set to the bridge in question.

Because there's no specific support for these VLANs, just ignore the notifications to maintain the current behavior.

---
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/staging/fsl-dpaa2/ethsw/ethsw.c +6 -0
index c723a04..a17dd29 100644
@@ -719,6 +719,9 @@ static int port_vlans_add(struct net_device *netdev,
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
 	int vid, err;
 
+	if (netif_is_bridge_master(vlan->obj.orig_dev))
+		return -EOPNOTSUPP;
+
 	if (switchdev_trans_ph_prepare(trans))
 		return 0;
 
@@ -873,6 +876,9 @@ static int port_vlans_del(struct net_device *netdev,
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
 	int vid, err;
 
+	if (netif_is_bridge_master(vlan->obj.orig_dev))
+		return -EOPNOTSUPP;
+
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
 		err = ethsw_port_del_vlan(port_priv, vid);
 		if (err)
Signature · 1 line
2.4.11
#10 lore raw

PATCH v4 7/8 net: bridge: Notify about bridge VLANs

In reply to Petr Machata

A driver might need to react to changes in settings of brentry VLANs. Therefore send switchdev port notifications for these as well. Reuse SWITCHDEV_OBJ_ID_PORT_VLAN for this purpose. Listeners should use netif_is_bridge_master() on orig_dev to determine whether the notification is about a bridge port or a bridge.

---
 net/bridge/br_vlan.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)
net/bridge/br_vlan.c +25 -3
index 602c869..7df2690 100644
@@ -246,6 +246,10 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
 			goto out_filt;
 		v->brvlan = masterv;
 		v->stats = masterv->stats;
+	} else {
+		err = br_switchdev_port_vlan_add(dev, v->vid, flags);
+		if (err && err != -EOPNOTSUPP)
+			goto out;
 	}
 
 	/* Add the dev mac and count the vlan only if it's usable */
@@ -281,6 +285,8 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
 			br_vlan_put_master(masterv);
 			v->brvlan = NULL;
 		}
+	} else {
+		br_switchdev_port_vlan_del(dev, v->vid);
 	}
 
 	goto out;
@@ -306,6 +312,11 @@ static int __vlan_del(struct net_bridge_vlan *v)
 		err = __vlan_vid_del(p->dev, p->br, v->vid);
 		if (err)
 			goto out;
+	} else {
+		err = br_switchdev_port_vlan_del(v->br->dev, v->vid);
+		if (err && err != -EOPNOTSUPP)
+			goto out;
+		err = 0;
 	}
 
 	if (br_vlan_should_use(v)) {
@@ -558,16 +569,22 @@ static int br_vlan_add_existing(struct net_bridge *br,
 {
 	int err;
 
+	err = br_switchdev_port_vlan_add(br->dev, vlan->vid, flags);
+	if (err && err != -EOPNOTSUPP)
+		return err;
+
 	if (!br_vlan_is_brentry(vlan)) {
 		/* Trying to change flags of non-existent bridge vlan */
-		if (!(flags & BRIDGE_VLAN_INFO_BRENTRY))
-			return -EINVAL;
+		if (!(flags & BRIDGE_VLAN_INFO_BRENTRY)) {
+			err = -EINVAL;
+			goto err_flags;
+		}
 		/* It was only kept for port vlans, now make it real */
 		err = br_fdb_insert(br, NULL, br->dev->dev_addr,
 				    vlan->vid);
 		if (err) {
 			br_err(br, "failed to insert local address into bridge forwarding table\n");
-			return err;
+			goto err_fdb_insert;
 		}
 
 		refcount_inc(&vlan->refcnt);
@@ -580,6 +597,11 @@ static int br_vlan_add_existing(struct net_bridge *br,
 		*changed = true;
 
 	return 0;
+
+err_fdb_insert:
+err_flags:
+	br_switchdev_port_vlan_del(br->dev, vlan->vid);
+	return err;
 }
 
 /* Must be protected by RTNL.
Signature · 1 line
2.4.11
Reviewed-by on v4 7/8
#11 lore raw
Quoted 11 lines · Petr Machata

A driver might need to react to changes in settings of brentry VLANs. Therefore send switchdev port notifications for these as well. Reuse SWITCHDEV_OBJ_ID_PORT_VLAN for this purpose. Listeners should use netif_is_bridge_master() on orig_dev to determine whether the notification is about a bridge port or a bridge.

---
 net/bridge/br_vlan.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

LGTM,

#12 lore raw

PATCH v4 8/8 mlxsw: spectrum_switchdev: Schedule respin during trans prepare

In reply to Petr Machata

Since there's no special support for the bridge events, the driver returns -EOPNOTSUPP, and thus the commit never happens. Therefore schedule respin during the prepare stage: there's no real difference one way or another.

This fixes the problem that mirror-to-gretap offload wouldn't adapt to changes in bridge vlan configuration right away and another notification would have to arrive for mlxsw to catch up.

---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +1 -1
index cbc8fab..8a15ac4 100644
@@ -1697,7 +1697,7 @@ static int mlxsw_sp_port_obj_add(struct net_device *dev,
 		vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
 		err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, trans);
 
-		if (switchdev_trans_ph_commit(trans)) {
+		if (switchdev_trans_ph_prepare(trans)) {
 			/* The event is emitted before the changes are actually
 			 * applied to the bridge. Therefore schedule the respin
 			 * call for later, so that the respin logic sees the
Signature · 1 line
2.4.11
maintainer
#13 lore raw
In reply to Petr Machata

From: Petr Machata <[email protected]>
Date: Wed, 30 May 2018 02:55:34 +0200

Quoted 28 lines

In commit 946a11e7408e ("mlxsw: spectrum_span: Allow bridge for gretap mirror"), mlxsw got support for offloading mirror-to-gretap such that the underlay packet path involves a bridge. In that case, the offload is also influenced by PVID setting of said bridge. However, changes to VLAN configuration of the bridge itself do not generate switchdev notifications, so there's no mechanism to prod mlxsw to update the offload when these settings change.

In this patchset, the problem is resolved by distributing the switchdev notification SWITCHDEV_OBJ_ID_PORT_VLAN also for configuration changes on bridge VLANs. Since stacked devices distribute the notification to lower devices, such event eventually reaches the driver, which can determine whether it's a bridge or port VLAN by inspecting orig_dev.

To keep things consistent, the newly-distributed notifications observe the same protocol as the existing ones: dual prepare/commit, with -EOPNOTSUPP indicating lack of support, even though there's currently nothing to prepare for and nothing to support. Correspondingly, all switchdev drivers have been updated to return -EOPNOTSUPP for bridge VLAN notifications.

In patches #1 and #2, the code base is changed to support the following
additions: functions br_switchdev_port_vlan_add() and br_switchdev_port_vlan_del() are introduced to simplify sending notifications; and br_vlan_add_existing() is introduced to later make it simpler to add error-handling code for the case of configuring a preexisting VLAN on bridge CPU port.

Quoted 8 lines

In patches #3-#6, respectively for mlxsw, rocker, DSA and DPAA2 ethsw, the new notifications (which are not enabled yet) are ignored to maintain the current behavior.

In patch #7, the notification is actually enabled.

In patch #8, mlxsw is changed to update offloads of mirror-to-gre also for bridge-related notifications.

...

Series applied, thanks.