diff mbox series

[net-next,3/5] net: bridge: propagate extack through switchdev_port_attr_set

Message ID 20210213204319.1226170-4-olteanv@gmail.com (mailing list archive)
State Accepted
Commit dcbdf1350e3312c199dbc6a76f41cf8f67e8c09c
Delegated to: Netdev Maintainers
Headers show
Series Propagate extack for switchdev VLANs from DSA | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 1 maintainers not CCed: bridge@lists.linux-foundation.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 158 this patch: 158
netdev/kdoc success Errors and warnings before: 3 this patch: 3
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 203 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 184 this patch: 184
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Vladimir Oltean Feb. 13, 2021, 8:43 p.m. UTC 
From: Vladimir Oltean <vladimir.oltean@nxp.com>

The benefit is the ability to propagate errors from switchdev drivers
for the SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING and
SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL attributes.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/net/switchdev.h       |  3 ++-
 net/bridge/br_mrp_switchdev.c |  4 ++--
 net/bridge/br_multicast.c     |  6 +++---
 net/bridge/br_netlink.c       |  2 +-
 net/bridge/br_private.h       |  3 ++-
 net/bridge/br_stp.c           |  4 ++--
 net/bridge/br_switchdev.c     |  6 ++++--
 net/bridge/br_vlan.c          | 13 +++++++------
 net/switchdev/switchdev.c     | 19 ++++++++++++-------
 9 files changed, 35 insertions(+), 25 deletions(-)

Comments

Florian Fainelli Feb. 14, 2021, 1:14 a.m. UTC | #1 
On 2/13/2021 12:43, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The benefit is the ability to propagate errors from switchdev drivers
> for the SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING and
> SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL attributes.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Nikolay Aleksandrov Feb. 14, 2021, 10:45 a.m. UTC | #2 
On 13/02/2021 22:43, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The benefit is the ability to propagate errors from switchdev drivers
> for the SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING and
> SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL attributes.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  include/net/switchdev.h       |  3 ++-
>  net/bridge/br_mrp_switchdev.c |  4 ++--
>  net/bridge/br_multicast.c     |  6 +++---
>  net/bridge/br_netlink.c       |  2 +-
>  net/bridge/br_private.h       |  3 ++-
>  net/bridge/br_stp.c           |  4 ++--
>  net/bridge/br_switchdev.c     |  6 ++++--
>  net/bridge/br_vlan.c          | 13 +++++++------
>  net/switchdev/switchdev.c     | 19 ++++++++++++-------
>  9 files changed, 35 insertions(+), 25 deletions(-)
> 

You have to update the !CONFIG_NET_SWITCHDEV switchdev_port_attr_set() stub as well.
Vladimir Oltean Feb. 14, 2021, 4:39 p.m. UTC | #3 
Hi Nikolay,

On Sun, Feb 14, 2021 at 12:45:11PM +0200, Nikolay Aleksandrov wrote:
> On 13/02/2021 22:43, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > The benefit is the ability to propagate errors from switchdev drivers
> > for the SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING and
> > SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL attributes.
> >
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> >  include/net/switchdev.h       |  3 ++-
> >  net/bridge/br_mrp_switchdev.c |  4 ++--
> >  net/bridge/br_multicast.c     |  6 +++---
> >  net/bridge/br_netlink.c       |  2 +-
> >  net/bridge/br_private.h       |  3 ++-
> >  net/bridge/br_stp.c           |  4 ++--
> >  net/bridge/br_switchdev.c     |  6 ++++--
> >  net/bridge/br_vlan.c          | 13 +++++++------
> >  net/switchdev/switchdev.c     | 19 ++++++++++++-------
> >  9 files changed, 35 insertions(+), 25 deletions(-)
> >
>
> You have to update the !CONFIG_NET_SWITCHDEV switchdev_port_attr_set() stub as well.

Thanks for pointing this out, you are right, the build fails.
What is really curious is that I had this issue on my radar but I forgot
to address it nonetheless.
I will take a step back and defer this series for after the break.
diff mbox series

Patch

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 25d9e4570934..195f62672cc4 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -247,7 +247,8 @@  switchdev_notifier_info_to_extack(const struct switchdev_notifier_info *info)
 
 void switchdev_deferred_process(void);
 int switchdev_port_attr_set(struct net_device *dev,
-			    const struct switchdev_attr *attr);
+			    const struct switchdev_attr *attr,
+			    struct netlink_ext_ack *extack);
 int switchdev_port_obj_add(struct net_device *dev,
 			   const struct switchdev_obj *obj,
 			   struct netlink_ext_ack *extack);
diff --git a/net/bridge/br_mrp_switchdev.c b/net/bridge/br_mrp_switchdev.c
index 75a7e8d0a268..3c9a4abcf4ee 100644
--- a/net/bridge/br_mrp_switchdev.c
+++ b/net/bridge/br_mrp_switchdev.c
@@ -178,7 +178,7 @@  int br_mrp_port_switchdev_set_state(struct net_bridge_port *p, u32 state)
 	};
 	int err;
 
-	err = switchdev_port_attr_set(p->dev, &attr);
+	err = switchdev_port_attr_set(p->dev, &attr, NULL);
 	if (err && err != -EOPNOTSUPP)
 		br_warn(p->br, "error setting offload MRP state on port %u(%s)\n",
 			(unsigned int)p->port_no, p->dev->name);
@@ -196,7 +196,7 @@  int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
 	};
 	int err;
 
-	err = switchdev_port_attr_set(p->dev, &attr);
+	err = switchdev_port_attr_set(p->dev, &attr, NULL);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index bf10ef5bbcd9..9d265447d654 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1381,7 +1381,7 @@  static void br_mc_router_state_change(struct net_bridge *p,
 		.u.mrouter = is_mc_router,
 	};
 
-	switchdev_port_attr_set(p->dev, &attr);
+	switchdev_port_attr_set(p->dev, &attr, NULL);
 }
 
 static void br_multicast_local_router_expired(struct timer_list *t)
@@ -1602,7 +1602,7 @@  static void br_mc_disabled_update(struct net_device *dev, bool value)
 		.u.mc_disabled = !value,
 	};
 
-	switchdev_port_attr_set(dev, &attr);
+	switchdev_port_attr_set(dev, &attr, NULL);
 }
 
 int br_multicast_add_port(struct net_bridge_port *port)
@@ -2645,7 +2645,7 @@  static void br_port_mc_router_state_change(struct net_bridge_port *p,
 		.u.mrouter = is_mc_router,
 	};
 
-	switchdev_port_attr_set(p->dev, &attr);
+	switchdev_port_attr_set(p->dev, &attr, NULL);
 }
 
 /*
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 3f5dc6fcc980..f2b1343f8332 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1221,7 +1221,7 @@  static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
 	if (data[IFLA_BR_VLAN_PROTOCOL]) {
 		__be16 vlan_proto = nla_get_be16(data[IFLA_BR_VLAN_PROTOCOL]);
 
-		err = __br_vlan_set_proto(br, vlan_proto);
+		err = __br_vlan_set_proto(br, vlan_proto, extack);
 		if (err)
 			return err;
 	}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a8d483325476..da71e71fcddc 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1087,7 +1087,8 @@  struct net_bridge_vlan *br_vlan_find(struct net_bridge_vlan_group *vg, u16 vid);
 void br_recalculate_fwd_mask(struct net_bridge *br);
 int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val,
 			  struct netlink_ext_ack *extack);
-int __br_vlan_set_proto(struct net_bridge *br, __be16 proto);
+int __br_vlan_set_proto(struct net_bridge *br, __be16 proto,
+			struct netlink_ext_ack *extack);
 int br_vlan_set_proto(struct net_bridge *br, unsigned long val,
 		      struct netlink_ext_ack *extack);
 int br_vlan_set_stats(struct net_bridge *br, unsigned long val);
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index a3a5745660dd..21c6781906aa 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -43,7 +43,7 @@  void br_set_state(struct net_bridge_port *p, unsigned int state)
 		return;
 
 	p->state = state;
-	err = switchdev_port_attr_set(p->dev, &attr);
+	err = switchdev_port_attr_set(p->dev, &attr, NULL);
 	if (err && err != -EOPNOTSUPP)
 		br_warn(p->br, "error setting offload STP state on port %u(%s)\n",
 				(unsigned int) p->port_no, p->dev->name);
@@ -591,7 +591,7 @@  int __set_ageing_time(struct net_device *dev, unsigned long t)
 	};
 	int err;
 
-	err = switchdev_port_attr_set(dev, &attr);
+	err = switchdev_port_attr_set(dev, &attr, NULL);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 184cf4c8b06d..b89503832fcc 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -96,9 +96,11 @@  int br_switchdev_set_port_flag(struct net_bridge_port *p,
 	attr.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS;
 	attr.flags = SWITCHDEV_F_DEFER;
 
-	err = switchdev_port_attr_set(p->dev, &attr);
+	err = switchdev_port_attr_set(p->dev, &attr, extack);
 	if (err) {
-		NL_SET_ERR_MSG_MOD(extack, "error setting offload flag on port");
+		if (extack && !extack->_msg)
+			NL_SET_ERR_MSG_MOD(extack,
+					   "error setting offload flag on port");
 		return err;
 	}
 
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index c4a51095850a..8829f621b8ec 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -820,7 +820,7 @@  int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val,
 	if (br_opt_get(br, BROPT_VLAN_ENABLED) == !!val)
 		return 0;
 
-	err = switchdev_port_attr_set(br->dev, &attr);
+	err = switchdev_port_attr_set(br->dev, &attr, extack);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
@@ -850,7 +850,8 @@  int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto)
 }
 EXPORT_SYMBOL_GPL(br_vlan_get_proto);
 
-int __br_vlan_set_proto(struct net_bridge *br, __be16 proto)
+int __br_vlan_set_proto(struct net_bridge *br, __be16 proto,
+			struct netlink_ext_ack *extack)
 {
 	struct switchdev_attr attr = {
 		.orig_dev = br->dev,
@@ -867,7 +868,7 @@  int __br_vlan_set_proto(struct net_bridge *br, __be16 proto)
 	if (br->vlan_proto == proto)
 		return 0;
 
-	err = switchdev_port_attr_set(br->dev, &attr);
+	err = switchdev_port_attr_set(br->dev, &attr, extack);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
@@ -897,7 +898,7 @@  int __br_vlan_set_proto(struct net_bridge *br, __be16 proto)
 
 err_filt:
 	attr.u.vlan_protocol = ntohs(oldproto);
-	switchdev_port_attr_set(br->dev, &attr);
+	switchdev_port_attr_set(br->dev, &attr, NULL);
 
 	list_for_each_entry_continue_reverse(vlan, &vg->vlan_list, vlist)
 		vlan_vid_del(p->dev, proto, vlan->vid);
@@ -917,7 +918,7 @@  int br_vlan_set_proto(struct net_bridge *br, unsigned long val,
 	if (!eth_type_vlan(htons(val)))
 		return -EPROTONOSUPPORT;
 
-	return __br_vlan_set_proto(br, htons(val));
+	return __br_vlan_set_proto(br, htons(val), extack);
 }
 
 int br_vlan_set_stats(struct net_bridge *br, unsigned long val)
@@ -1165,7 +1166,7 @@  int nbp_vlan_init(struct net_bridge_port *p, struct netlink_ext_ack *extack)
 	if (!vg)
 		goto out;
 
-	ret = switchdev_port_attr_set(p->dev, &attr);
+	ret = switchdev_port_attr_set(p->dev, &attr, extack);
 	if (ret && ret != -EOPNOTSUPP)
 		goto err_vlan_enabled;
 
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 0b84f076591e..89a36db47ab4 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -100,7 +100,8 @@  static int switchdev_deferred_enqueue(struct net_device *dev,
 
 static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
 				      struct net_device *dev,
-				      const struct switchdev_attr *attr)
+				      const struct switchdev_attr *attr,
+				      struct netlink_ext_ack *extack)
 {
 	int err;
 	int rc;
@@ -111,7 +112,7 @@  static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
 	};
 
 	rc = call_switchdev_blocking_notifiers(nt, dev,
-					       &attr_info.info, NULL);
+					       &attr_info.info, extack);
 	err = notifier_to_errno(rc);
 	if (err) {
 		WARN_ON(!attr_info.handled);
@@ -125,9 +126,11 @@  static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
 }
 
 static int switchdev_port_attr_set_now(struct net_device *dev,
-				       const struct switchdev_attr *attr)
+				       const struct switchdev_attr *attr,
+				       struct netlink_ext_ack *extack)
 {
-	return switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr);
+	return switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
+					  extack);
 }
 
 static void switchdev_port_attr_set_deferred(struct net_device *dev,
@@ -136,7 +139,7 @@  static void switchdev_port_attr_set_deferred(struct net_device *dev,
 	const struct switchdev_attr *attr = data;
 	int err;
 
-	err = switchdev_port_attr_set_now(dev, attr);
+	err = switchdev_port_attr_set_now(dev, attr, NULL);
 	if (err && err != -EOPNOTSUPP)
 		netdev_err(dev, "failed (err=%d) to set attribute (id=%d)\n",
 			   err, attr->id);
@@ -156,17 +159,19 @@  static int switchdev_port_attr_set_defer(struct net_device *dev,
  *
  *	@dev: port device
  *	@attr: attribute to set
+ *	@extack: netlink extended ack, for error message propagation
  *
  *	rtnl_lock must be held and must not be in atomic section,
  *	in case SWITCHDEV_F_DEFER flag is not set.
  */
 int switchdev_port_attr_set(struct net_device *dev,
-			    const struct switchdev_attr *attr)
+			    const struct switchdev_attr *attr,
+			    struct netlink_ext_ack *extack)
 {
 	if (attr->flags & SWITCHDEV_F_DEFER)
 		return switchdev_port_attr_set_defer(dev, attr);
 	ASSERT_RTNL();
-	return switchdev_port_attr_set_now(dev, attr);
+	return switchdev_port_attr_set_now(dev, attr, extack);
 }
 EXPORT_SYMBOL_GPL(switchdev_port_attr_set);