[PATCH net-next v3 0/7] net: bridge: Notify about bridge VLANs

[PATCH net-next v3 0/7] net: bridge: Notify about bridge VLANs

[Petr Machata]

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 patch #1, the code to send notifications for adding and deleting is
factored out into two named functions.

In patches #2-#5, 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 #6, the notification is actually enabled.

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

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 (7):
  net: bridge: Extract boilerplate around switchdev_port_obj_*()
  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                               | 45 +++++++++-------------
 net/dsa/port.c                                     |  6 +++
 7 files changed, 81 insertions(+), 28 deletions(-)

-- 
2.4.11

[PATCH net-next v3 1/7] net: bridge: Extract boilerplate around switchdev_port_obj_*()

[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.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 net/bridge/br_private.h   | 13 +++++++++++++
 net/bridge/br_switchdev.c | 25 +++++++++++++++++++++++++
 net/bridge/br_vlan.c      | 31 ++++---------------------------
 3 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 11520ed..5216a52 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -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)
 {
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 35474d4..d77f807 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -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);
+}
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index dc832c09..8ad5756 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -82,19 +82,10 @@ 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);
+	int 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 +121,11 @@ 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);
+	int err = br_switchdev_port_vlan_del(dev, vid);
+
 	if (err == -EOPNOTSUPP) {
 		vlan_vid_del(dev, br->vlan_proto, vid);
 		return 0;
@@ -1053,13 +1037,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 +1046,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);
-- 
2.4.11

[PATCH net-next v3 2/7] mlxsw: spectrum_switchdev: Ignore bridge VLAN events

[Petr Machata]

Ignore VLAN events where the orig_dev is the bridge device itself.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 8c9cf8e..cbc8fab 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -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;
-- 
2.4.11

[PATCH net-next v3 3/7] rocker: rocker_main: Ignore bridge VLAN events

[Petr Machata]

Ignore VLAN events where the orig_dev is the bridge device itself.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 drivers/net/ethernet/rocker/rocker_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index e73e4fe..aeafdb9 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -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);
-- 
2.4.11

[PATCH net-next v3 4/7] dsa: port: Ignore bridge VLAN events

[Petr Machata]

Ignore VLAN events where the orig_dev is the bridge device itself.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/port.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2413beb..ed05954 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -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);
 
-- 
2.4.11

[PATCH net-next v3 5/7] staging: fsl-dpaa2: ethsw: Ignore bridge VLAN events

[Petr Machata]

Ignore VLAN events where the orig_dev is the bridge device itself.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index c723a04..a17dd29 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -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)
-- 
2.4.11

[PATCH net-next v3 6/7] net: bridge: Notify about bridge VLANs

[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.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/bridge/br_vlan.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 8ad5756..b15ef3f 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -243,6 +243,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 */
@@ -278,6 +282,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;
@@ -303,6 +309,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)) {
@@ -580,6 +591,9 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed)
 			vg->num_vlans++;
 			*changed = true;
 		}
+		ret = br_switchdev_port_vlan_add(br->dev, vid, flags);
+		if (ret && ret != -EOPNOTSUPP)
+			return ret;
 		if (__vlan_add_flags(vlan, flags))
 			*changed = true;
 
-- 
2.4.11

[PATCH net-next v3 7/7] mlxsw: spectrum_switchdev: Schedule respin during trans prepare

[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.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index cbc8fab..8a15ac4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -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
-- 
2.4.11

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

[Dan Carpenter]

On Mon, May 28, 2018 at 05:10:28PM +0200, Petr Machata wrote:
>  	/* Try switchdev op first. In case it is not supported, fallback to
>  	 * 8021q add.
>  	 */
> -	err = switchdev_port_obj_add(dev, &v.obj);
> +	int err = br_switchdev_port_vlan_add(dev, vid, flags);
>  	if (err == -EOPNOTSUPP)

This will introduce a checkpatch warning if you re-run with the --file
option.  (I haven't tested).  It's sort of ugly to put function logic in
the declaration block.  That's really just for initializing variables
like "int start = 0; struct foo *p = parent_of_bar();"  Do it like this
instead:

	int err;

	err = br_switchdev_port_vlan_add(dev, vid, flags);
	if (err == -EOPNOTSUPP)

regards,
dan carpenter

Re: [PATCH net-next v3 2/7] mlxsw: spectrum_switchdev: Ignore bridge VLAN events

[Dan Carpenter]

On Mon, May 28, 2018 at 05:10:40PM +0200, Petr Machata wrote:
> Ignore VLAN events where the orig_dev is the bridge device itself.
> 

I don't know this code, and I have not looked at it either...  But this
changelog just says what you are doing and not why.  Is this a bug fix?
What are the user visible effects of this patch?  Perhaps it is
something which will be required in the future but has no effect now?

You know how the New York Times is written for people with at least a
10th grade education?  I feel like maybe if I knew this code I would
know the answers to my question, but kernel commit descriptions should
generally be written to Dan Carpenter level of education.  Which is
pretty darn ignorant...  Can you please resend?

regards,
dan carpenter

Re: [PATCH net-next v3 0/7] net: bridge: Notify about bridge VLANs

[Dan Carpenter]

On Mon, May 28, 2018 at 05:10:22PM +0200, Petr Machata wrote:
> 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 patch #1, the code to send notifications for adding and deleting is
> factored out into two named functions.
> 
> In patches #2-#5, respectively for mlxsw, rocker, DSA and DPAA2 ethsw,
> the new notifications (which are not enabled yet) are ignored to
> maintain the current behavior.
> 

It occured to me that I should read the cover letter and here are the
answers I was looking for.  But the cover letter isn't saved after the
commits are merged.  This should really be in the commit messages itself
so that we can look it up in the git history.

> In patch #6, the notification is actually enabled.
> 
> In patch #7, mlxsw is changed to update offloads of mirror-to-gre also
> for bridge-related notifications.

regards,
dan carpenter

Re: [PATCH net-next v3 6/7] net: bridge: Notify about bridge VLANs

[Dan Carpenter]

On Mon, May 28, 2018 at 05:11:04PM +0200, Petr Machata wrote:
> @@ -580,6 +591,9 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed)
>  			vg->num_vlans++;
>  			*changed = true;
>  		}
> +		ret = br_switchdev_port_vlan_add(br->dev, vid, flags);
> +		if (ret && ret != -EOPNOTSUPP)
> +			return ret;

We should probably do some error handling instead of returning directly?

>  		if (__vlan_add_flags(vlan, flags))
>  			*changed = true;
>  

regards,
dan caprenter

Re: [PATCH net-next v3 0/7] net: bridge: Notify about bridge VLANs

[Ido Schimmel]

On Tue, May 29, 2018 at 01:46:09PM +0300, Dan Carpenter wrote:
> It occured to me that I should read the cover letter and here are the
> answers I was looking for.  But the cover letter isn't saved after the
> commits are merged.

DaveM adds the cover letter to the merge commit.

Re: [PATCH net-next v3 0/7] net: bridge: Notify about bridge VLANs

[Dan Carpenter]

On Tue, May 29, 2018 at 01:58:53PM +0300, Ido Schimmel wrote:
> On Tue, May 29, 2018 at 01:46:09PM +0300, Dan Carpenter wrote:
> > It occured to me that I should read the cover letter and here are the
> > answers I was looking for.  But the cover letter isn't saved after the
> > commits are merged.
> 
> DaveM adds the cover letter to the merge commit.

That's true but it doesn't help.  When I want to find why line of code
is there I look up the commit message, not the merge commit.

regards,
dan carpenter

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

[Vivien Didelot]

Hi Petr,

Petr Machata <petrm@mellanox.com> writes:

> 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.
>
> Signed-off-by: Petr Machata <petrm@mellanox.com>

Considering Dan's comment as well:

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Thanks,

        Vivien

Re: [PATCH net-next v3 6/7] net: bridge: Notify about bridge VLANs

[Petr Machata]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> On Mon, May 28, 2018 at 05:11:04PM +0200, Petr Machata wrote:
>> @@ -580,6 +591,9 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed)
>>  			vg->num_vlans++;
>>  			*changed = true;
>>  		}
>> +		ret = br_switchdev_port_vlan_add(br->dev, vid, flags);
>> +		if (ret && ret != -EOPNOTSUPP)
>> +			return ret;
>
> We should probably do some error handling instead of returning directly?

I missed that, you are right. There's a bunch of mutations in the block
above.

Thanks,
Petr