netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA
@ 2021-02-13 20:43 Vladimir Oltean
  2021-02-13 20:43 ` [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle Vladimir Oltean
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-02-13 20:43 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang,
	Ido Schimmel

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This series moves the restriction messages printed by the DSA core, and
by some individual device drivers, into the netlink extended ack
structure, to be communicated to user space where possible, or still
printed to the kernel log from the bridge layer.

Vladimir Oltean (5):
  net: bridge: remove __br_vlan_filter_toggle
  net: bridge: propagate extack through store_bridge_parm
  net: bridge: propagate extack through switchdev_port_attr_set
  net: dsa: propagate extack to .port_vlan_add
  net: dsa: propagate extack to .port_vlan_filtering

 drivers/net/dsa/b53/b53_common.c          |   6 +-
 drivers/net/dsa/b53/b53_priv.h            |   6 +-
 drivers/net/dsa/bcm_sf2_cfp.c             |   2 +-
 drivers/net/dsa/dsa_loop.c                |   6 +-
 drivers/net/dsa/hirschmann/hellcreek.c    |  15 +-
 drivers/net/dsa/lantiq_gswip.c            |  22 ++-
 drivers/net/dsa/microchip/ksz8795.c       |   6 +-
 drivers/net/dsa/microchip/ksz9477.c       |  10 +-
 drivers/net/dsa/mt7530.c                  |   7 +-
 drivers/net/dsa/mv88e6xxx/chip.c          |   6 +-
 drivers/net/dsa/ocelot/felix.c            |   6 +-
 drivers/net/dsa/qca8k.c                   |   6 +-
 drivers/net/dsa/realtek-smi-core.h        |   7 +-
 drivers/net/dsa/rtl8366.c                 |  14 +-
 drivers/net/dsa/sja1105/sja1105.h         |   3 +-
 drivers/net/dsa/sja1105/sja1105_devlink.c |   2 +-
 drivers/net/dsa/sja1105/sja1105_main.c    |  15 +-
 include/net/dsa.h                         |   6 +-
 include/net/switchdev.h                   |   3 +-
 net/bridge/br_mrp_switchdev.c             |   4 +-
 net/bridge/br_multicast.c                 |   6 +-
 net/bridge/br_netlink.c                   |   4 +-
 net/bridge/br_private.h                   |  17 ++-
 net/bridge/br_stp.c                       |   4 +-
 net/bridge/br_switchdev.c                 |   6 +-
 net/bridge/br_sysfs_br.c                  | 166 +++++++++++++++++-----
 net/bridge/br_vlan.c                      |  29 ++--
 net/dsa/dsa_priv.h                        |   7 +-
 net/dsa/port.c                            |  22 +--
 net/dsa/slave.c                           |  28 ++--
 net/dsa/switch.c                          |   9 +-
 net/switchdev/switchdev.c                 |  19 ++-
 32 files changed, 319 insertions(+), 150 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle
  2021-02-13 20:43 [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA Vladimir Oltean
@ 2021-02-13 20:43 ` Vladimir Oltean
  2021-02-14  1:04   ` Florian Fainelli
  2021-02-14 10:20   ` Nikolay Aleksandrov
  2021-02-13 20:43 ` [PATCH net-next 2/5] net: bridge: propagate extack through store_bridge_parm Vladimir Oltean
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-02-13 20:43 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang,
	Ido Schimmel

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This function is identical with br_vlan_filter_toggle.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_netlink.c | 2 +-
 net/bridge/br_private.h | 5 ++---
 net/bridge/br_vlan.c    | 7 +------
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 7b513c5d347f..a12bbbdacb9b 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1212,7 +1212,7 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
 	if (data[IFLA_BR_VLAN_FILTERING]) {
 		u8 vlan_filter = nla_get_u8(data[IFLA_BR_VLAN_FILTERING]);
 
-		err = __br_vlan_filter_toggle(br, vlan_filter);
+		err = br_vlan_filter_toggle(br, vlan_filter);
 		if (err)
 			return err;
 	}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a1639d41188b..0281de20212e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1085,7 +1085,6 @@ int br_vlan_delete(struct net_bridge *br, u16 vid);
 void br_vlan_flush(struct net_bridge *br);
 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);
 int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
 int __br_vlan_set_proto(struct net_bridge *br, __be16 proto);
 int br_vlan_set_proto(struct net_bridge *br, unsigned long val);
@@ -1261,8 +1260,8 @@ static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
 	return 0;
 }
 
-static inline int __br_vlan_filter_toggle(struct net_bridge *br,
-					  unsigned long val)
+static inline int br_vlan_filter_toggle(struct net_bridge *br,
+					unsigned long val)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index bb2909738518..26e7e06b6a0d 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -806,7 +806,7 @@ void br_recalculate_fwd_mask(struct net_bridge *br)
 					      ~(1u << br->group_addr[5]);
 }
 
-int __br_vlan_filter_toggle(struct net_bridge *br, unsigned long val)
+int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val)
 {
 	struct switchdev_attr attr = {
 		.orig_dev = br->dev,
@@ -831,11 +831,6 @@ int __br_vlan_filter_toggle(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val)
-{
-	return __br_vlan_filter_toggle(br, val);
-}
-
 bool br_vlan_enabled(const struct net_device *dev)
 {
 	struct net_bridge *br = netdev_priv(dev);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 2/5] net: bridge: propagate extack through store_bridge_parm
  2021-02-13 20:43 [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA Vladimir Oltean
  2021-02-13 20:43 ` [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle Vladimir Oltean
@ 2021-02-13 20:43 ` Vladimir Oltean
  2021-02-14  1:06   ` Florian Fainelli
  2021-02-14 10:43   ` Nikolay Aleksandrov
  2021-02-13 20:43 ` [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set Vladimir Oltean
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-02-13 20:43 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang,
	Ido Schimmel

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The bridge sysfs interface stores parameters for the STP, VLAN,
multicast etc subsystems using a predefined function prototype.
Sometimes the underlying function being called supports a netlink
extended ack message, and we ignore it.

Let's expand the store_bridge_parm function prototype to include the
extack, and just print it to console, but at least propagate it where
applicable. Where not applicable, create a shim function in the
br_sysfs_br.c file that discards the extra function argument.

This patch allows us to propagate the extack argument to
br_vlan_set_default_pvid, br_vlan_set_proto and br_vlan_filter_toggle,
and from there, further up in br_changelink from br_netlink.c.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_netlink.c  |   2 +-
 net/bridge/br_private.h  |   9 ++-
 net/bridge/br_sysfs_br.c | 166 ++++++++++++++++++++++++++++++---------
 net/bridge/br_vlan.c     |  11 ++-
 4 files changed, 142 insertions(+), 46 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index a12bbbdacb9b..3f5dc6fcc980 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1212,7 +1212,7 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
 	if (data[IFLA_BR_VLAN_FILTERING]) {
 		u8 vlan_filter = nla_get_u8(data[IFLA_BR_VLAN_FILTERING]);
 
-		err = br_vlan_filter_toggle(br, vlan_filter);
+		err = br_vlan_filter_toggle(br, vlan_filter, extack);
 		if (err)
 			return err;
 	}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 0281de20212e..a8d483325476 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1085,13 +1085,16 @@ int br_vlan_delete(struct net_bridge *br, u16 vid);
 void br_vlan_flush(struct net_bridge *br);
 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);
+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, unsigned long val);
+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);
 int br_vlan_set_stats_per_port(struct net_bridge *br, unsigned long val);
 int br_vlan_init(struct net_bridge *br);
-int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val);
+int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val,
+			     struct netlink_ext_ack *extack);
 int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid,
 			       struct netlink_ext_ack *extack);
 int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags,
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 71f0f671c4ef..072e29840082 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -30,11 +30,13 @@
  */
 static ssize_t store_bridge_parm(struct device *d,
 				 const char *buf, size_t len,
-				 int (*set)(struct net_bridge *, unsigned long))
+				 int (*set)(struct net_bridge *br, unsigned long val,
+					    struct netlink_ext_ack *extack))
 {
 	struct net_bridge *br = to_bridge(d);
-	char *endp;
+	struct netlink_ext_ack extack = {0};
 	unsigned long val;
+	char *endp;
 	int err;
 
 	if (!ns_capable(dev_net(br->dev)->user_ns, CAP_NET_ADMIN))
@@ -47,9 +49,15 @@ static ssize_t store_bridge_parm(struct device *d,
 	if (!rtnl_trylock())
 		return restart_syscall();
 
-	err = (*set)(br, val);
+	err = (*set)(br, val, &extack);
 	if (!err)
 		netdev_state_change(br->dev);
+	if (extack._msg) {
+		if (err)
+			br_err(br, "%s\n", extack._msg);
+		else
+			br_warn(br, "%s\n", extack._msg);
+	}
 	rtnl_unlock();
 
 	return err ? err : len;
@@ -63,11 +71,17 @@ static ssize_t forward_delay_show(struct device *d,
 	return sprintf(buf, "%lu\n", jiffies_to_clock_t(br->forward_delay));
 }
 
+static int set_forward_delay(struct net_bridge *br, unsigned long val,
+			     struct netlink_ext_ack *extack)
+{
+	return br_set_forward_delay(br, val);
+}
+
 static ssize_t forward_delay_store(struct device *d,
 				   struct device_attribute *attr,
 				   const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_set_forward_delay);
+	return store_bridge_parm(d, buf, len, set_forward_delay);
 }
 static DEVICE_ATTR_RW(forward_delay);
 
@@ -78,11 +92,17 @@ static ssize_t hello_time_show(struct device *d, struct device_attribute *attr,
 		       jiffies_to_clock_t(to_bridge(d)->hello_time));
 }
 
+static int set_hello_time(struct net_bridge *br, unsigned long val,
+			  struct netlink_ext_ack *extack)
+{
+	return br_set_hello_time(br, val);
+}
+
 static ssize_t hello_time_store(struct device *d,
 				struct device_attribute *attr, const char *buf,
 				size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_set_hello_time);
+	return store_bridge_parm(d, buf, len, set_hello_time);
 }
 static DEVICE_ATTR_RW(hello_time);
 
@@ -93,10 +113,16 @@ static ssize_t max_age_show(struct device *d, struct device_attribute *attr,
 		       jiffies_to_clock_t(to_bridge(d)->max_age));
 }
 
+static int set_max_age(struct net_bridge *br, unsigned long val,
+		       struct netlink_ext_ack *extack)
+{
+	return br_set_max_age(br, val);
+}
+
 static ssize_t max_age_store(struct device *d, struct device_attribute *attr,
 			     const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_set_max_age);
+	return store_bridge_parm(d, buf, len, set_max_age);
 }
 static DEVICE_ATTR_RW(max_age);
 
@@ -107,7 +133,8 @@ static ssize_t ageing_time_show(struct device *d,
 	return sprintf(buf, "%lu\n", jiffies_to_clock_t(br->ageing_time));
 }
 
-static int set_ageing_time(struct net_bridge *br, unsigned long val)
+static int set_ageing_time(struct net_bridge *br, unsigned long val,
+			   struct netlink_ext_ack *extack)
 {
 	return br_set_ageing_time(br, val);
 }
@@ -128,9 +155,10 @@ static ssize_t stp_state_show(struct device *d,
 }
 
 
-static int set_stp_state(struct net_bridge *br, unsigned long val)
+static int set_stp_state(struct net_bridge *br, unsigned long val,
+			 struct netlink_ext_ack *extack)
 {
-	return br_stp_set_enabled(br, val, NULL);
+	return br_stp_set_enabled(br, val, extack);
 }
 
 static ssize_t stp_state_store(struct device *d,
@@ -149,7 +177,8 @@ static ssize_t group_fwd_mask_show(struct device *d,
 	return sprintf(buf, "%#x\n", br->group_fwd_mask);
 }
 
-static int set_group_fwd_mask(struct net_bridge *br, unsigned long val)
+static int set_group_fwd_mask(struct net_bridge *br, unsigned long val,
+			      struct netlink_ext_ack *extack)
 {
 	if (val & BR_GROUPFWD_RESTRICTED)
 		return -EINVAL;
@@ -176,7 +205,8 @@ static ssize_t priority_show(struct device *d, struct device_attribute *attr,
 		       (br->bridge_id.prio[0] << 8) | br->bridge_id.prio[1]);
 }
 
-static int set_priority(struct net_bridge *br, unsigned long val)
+static int set_priority(struct net_bridge *br, unsigned long val,
+			struct netlink_ext_ack *extack)
 {
 	br_stp_set_bridge_priority(br, (u16) val);
 	return 0;
@@ -312,7 +342,8 @@ static ssize_t group_addr_store(struct device *d,
 
 static DEVICE_ATTR_RW(group_addr);
 
-static int set_flush(struct net_bridge *br, unsigned long val)
+static int set_flush(struct net_bridge *br, unsigned long val,
+		     struct netlink_ext_ack *extack)
 {
 	br_fdb_flush(br);
 	return 0;
@@ -334,9 +365,10 @@ static ssize_t no_linklocal_learn_show(struct device *d,
 	return sprintf(buf, "%d\n", br_boolopt_get(br, BR_BOOLOPT_NO_LL_LEARN));
 }
 
-static int set_no_linklocal_learn(struct net_bridge *br, unsigned long val)
+static int set_no_linklocal_learn(struct net_bridge *br, unsigned long val,
+				  struct netlink_ext_ack *extack)
 {
-	return br_boolopt_toggle(br, BR_BOOLOPT_NO_LL_LEARN, !!val, NULL);
+	return br_boolopt_toggle(br, BR_BOOLOPT_NO_LL_LEARN, !!val, extack);
 }
 
 static ssize_t no_linklocal_learn_store(struct device *d,
@@ -355,11 +387,17 @@ static ssize_t multicast_router_show(struct device *d,
 	return sprintf(buf, "%d\n", br->multicast_router);
 }
 
+static int set_multicast_router(struct net_bridge *br, unsigned long val,
+				struct netlink_ext_ack *extack)
+{
+	return br_multicast_set_router(br, val);
+}
+
 static ssize_t multicast_router_store(struct device *d,
 				      struct device_attribute *attr,
 				      const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_multicast_set_router);
+	return store_bridge_parm(d, buf, len, set_multicast_router);
 }
 static DEVICE_ATTR_RW(multicast_router);
 
@@ -371,11 +409,17 @@ static ssize_t multicast_snooping_show(struct device *d,
 	return sprintf(buf, "%d\n", br_opt_get(br, BROPT_MULTICAST_ENABLED));
 }
 
+static int toggle_multicast(struct net_bridge *br, unsigned long val,
+			    struct netlink_ext_ack *extack)
+{
+	return br_multicast_toggle(br, val);
+}
+
 static ssize_t multicast_snooping_store(struct device *d,
 					struct device_attribute *attr,
 					const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_multicast_toggle);
+	return store_bridge_parm(d, buf, len, toggle_multicast);
 }
 static DEVICE_ATTR_RW(multicast_snooping);
 
@@ -388,7 +432,8 @@ static ssize_t multicast_query_use_ifaddr_show(struct device *d,
 		       br_opt_get(br, BROPT_MULTICAST_QUERY_USE_IFADDR));
 }
 
-static int set_query_use_ifaddr(struct net_bridge *br, unsigned long val)
+static int set_query_use_ifaddr(struct net_bridge *br, unsigned long val,
+				struct netlink_ext_ack *extack)
 {
 	br_opt_toggle(br, BROPT_MULTICAST_QUERY_USE_IFADDR, !!val);
 	return 0;
@@ -411,11 +456,17 @@ static ssize_t multicast_querier_show(struct device *d,
 	return sprintf(buf, "%d\n", br_opt_get(br, BROPT_MULTICAST_QUERIER));
 }
 
+static int set_multicast_querier(struct net_bridge *br, unsigned long val,
+				 struct netlink_ext_ack *extack)
+{
+	return br_multicast_set_querier(br, val);
+}
+
 static ssize_t multicast_querier_store(struct device *d,
 				       struct device_attribute *attr,
 				       const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_multicast_set_querier);
+	return store_bridge_parm(d, buf, len, set_multicast_querier);
 }
 static DEVICE_ATTR_RW(multicast_querier);
 
@@ -425,10 +476,12 @@ static ssize_t hash_elasticity_show(struct device *d,
 	return sprintf(buf, "%u\n", RHT_ELASTICITY);
 }
 
-static int set_elasticity(struct net_bridge *br, unsigned long val)
+static int set_elasticity(struct net_bridge *br, unsigned long val,
+			  struct netlink_ext_ack *extack)
 {
-	br_warn(br, "the hash_elasticity option has been deprecated and is always %u\n",
-		RHT_ELASTICITY);
+	/* 16 is RHT_ELASTICITY */
+	NL_SET_ERR_MSG_MOD(extack,
+			   "the hash_elasticity option has been deprecated and is always 16");
 	return 0;
 }
 
@@ -447,7 +500,8 @@ static ssize_t hash_max_show(struct device *d, struct device_attribute *attr,
 	return sprintf(buf, "%u\n", br->hash_max);
 }
 
-static int set_hash_max(struct net_bridge *br, unsigned long val)
+static int set_hash_max(struct net_bridge *br, unsigned long val,
+			struct netlink_ext_ack *extack)
 {
 	br->hash_max = val;
 	return 0;
@@ -469,11 +523,17 @@ static ssize_t multicast_igmp_version_show(struct device *d,
 	return sprintf(buf, "%u\n", br->multicast_igmp_version);
 }
 
+static int set_multicast_igmp_version(struct net_bridge *br, unsigned long val,
+				      struct netlink_ext_ack *extack)
+{
+	return br_multicast_set_igmp_version(br, val);
+}
+
 static ssize_t multicast_igmp_version_store(struct device *d,
 					    struct device_attribute *attr,
 					    const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_multicast_set_igmp_version);
+	return store_bridge_parm(d, buf, len, set_multicast_igmp_version);
 }
 static DEVICE_ATTR_RW(multicast_igmp_version);
 
@@ -485,7 +545,8 @@ static ssize_t multicast_last_member_count_show(struct device *d,
 	return sprintf(buf, "%u\n", br->multicast_last_member_count);
 }
 
-static int set_last_member_count(struct net_bridge *br, unsigned long val)
+static int set_last_member_count(struct net_bridge *br, unsigned long val,
+				 struct netlink_ext_ack *extack)
 {
 	br->multicast_last_member_count = val;
 	return 0;
@@ -506,7 +567,8 @@ static ssize_t multicast_startup_query_count_show(
 	return sprintf(buf, "%u\n", br->multicast_startup_query_count);
 }
 
-static int set_startup_query_count(struct net_bridge *br, unsigned long val)
+static int set_startup_query_count(struct net_bridge *br, unsigned long val,
+				   struct netlink_ext_ack *extack)
 {
 	br->multicast_startup_query_count = val;
 	return 0;
@@ -528,7 +590,8 @@ static ssize_t multicast_last_member_interval_show(
 		       jiffies_to_clock_t(br->multicast_last_member_interval));
 }
 
-static int set_last_member_interval(struct net_bridge *br, unsigned long val)
+static int set_last_member_interval(struct net_bridge *br, unsigned long val,
+				    struct netlink_ext_ack *extack)
 {
 	br->multicast_last_member_interval = clock_t_to_jiffies(val);
 	return 0;
@@ -550,7 +613,8 @@ static ssize_t multicast_membership_interval_show(
 		       jiffies_to_clock_t(br->multicast_membership_interval));
 }
 
-static int set_membership_interval(struct net_bridge *br, unsigned long val)
+static int set_membership_interval(struct net_bridge *br, unsigned long val,
+				   struct netlink_ext_ack *extack)
 {
 	br->multicast_membership_interval = clock_t_to_jiffies(val);
 	return 0;
@@ -573,7 +637,8 @@ static ssize_t multicast_querier_interval_show(struct device *d,
 		       jiffies_to_clock_t(br->multicast_querier_interval));
 }
 
-static int set_querier_interval(struct net_bridge *br, unsigned long val)
+static int set_querier_interval(struct net_bridge *br, unsigned long val,
+				struct netlink_ext_ack *extack)
 {
 	br->multicast_querier_interval = clock_t_to_jiffies(val);
 	return 0;
@@ -596,7 +661,8 @@ static ssize_t multicast_query_interval_show(struct device *d,
 		       jiffies_to_clock_t(br->multicast_query_interval));
 }
 
-static int set_query_interval(struct net_bridge *br, unsigned long val)
+static int set_query_interval(struct net_bridge *br, unsigned long val,
+			      struct netlink_ext_ack *extack)
 {
 	br->multicast_query_interval = clock_t_to_jiffies(val);
 	return 0;
@@ -619,7 +685,8 @@ static ssize_t multicast_query_response_interval_show(
 		jiffies_to_clock_t(br->multicast_query_response_interval));
 }
 
-static int set_query_response_interval(struct net_bridge *br, unsigned long val)
+static int set_query_response_interval(struct net_bridge *br, unsigned long val,
+				       struct netlink_ext_ack *extack)
 {
 	br->multicast_query_response_interval = clock_t_to_jiffies(val);
 	return 0;
@@ -642,7 +709,8 @@ static ssize_t multicast_startup_query_interval_show(
 		jiffies_to_clock_t(br->multicast_startup_query_interval));
 }
 
-static int set_startup_query_interval(struct net_bridge *br, unsigned long val)
+static int set_startup_query_interval(struct net_bridge *br, unsigned long val,
+				      struct netlink_ext_ack *extack)
 {
 	br->multicast_startup_query_interval = clock_t_to_jiffies(val);
 	return 0;
@@ -666,7 +734,8 @@ static ssize_t multicast_stats_enabled_show(struct device *d,
 		       br_opt_get(br, BROPT_MULTICAST_STATS_ENABLED));
 }
 
-static int set_stats_enabled(struct net_bridge *br, unsigned long val)
+static int set_stats_enabled(struct net_bridge *br, unsigned long val,
+			     struct netlink_ext_ack *extack)
 {
 	br_opt_toggle(br, BROPT_MULTICAST_STATS_ENABLED, !!val);
 	return 0;
@@ -691,11 +760,17 @@ static ssize_t multicast_mld_version_show(struct device *d,
 	return sprintf(buf, "%u\n", br->multicast_mld_version);
 }
 
+static int set_multicast_mld_version(struct net_bridge *br, unsigned long val,
+				     struct netlink_ext_ack *extack)
+{
+	return br_multicast_set_mld_version(br, val);
+}
+
 static ssize_t multicast_mld_version_store(struct device *d,
 					   struct device_attribute *attr,
 					   const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_multicast_set_mld_version);
+	return store_bridge_parm(d, buf, len, set_multicast_mld_version);
 }
 static DEVICE_ATTR_RW(multicast_mld_version);
 #endif
@@ -708,7 +783,8 @@ static ssize_t nf_call_iptables_show(
 	return sprintf(buf, "%u\n", br_opt_get(br, BROPT_NF_CALL_IPTABLES));
 }
 
-static int set_nf_call_iptables(struct net_bridge *br, unsigned long val)
+static int set_nf_call_iptables(struct net_bridge *br, unsigned long val,
+				struct netlink_ext_ack *extack)
 {
 	br_opt_toggle(br, BROPT_NF_CALL_IPTABLES, !!val);
 	return 0;
@@ -729,7 +805,8 @@ static ssize_t nf_call_ip6tables_show(
 	return sprintf(buf, "%u\n", br_opt_get(br, BROPT_NF_CALL_IP6TABLES));
 }
 
-static int set_nf_call_ip6tables(struct net_bridge *br, unsigned long val)
+static int set_nf_call_ip6tables(struct net_bridge *br, unsigned long val,
+				 struct netlink_ext_ack *extack)
 {
 	br_opt_toggle(br, BROPT_NF_CALL_IP6TABLES, !!val);
 	return 0;
@@ -750,7 +827,8 @@ static ssize_t nf_call_arptables_show(
 	return sprintf(buf, "%u\n", br_opt_get(br, BROPT_NF_CALL_ARPTABLES));
 }
 
-static int set_nf_call_arptables(struct net_bridge *br, unsigned long val)
+static int set_nf_call_arptables(struct net_bridge *br, unsigned long val,
+				 struct netlink_ext_ack *extack)
 {
 	br_opt_toggle(br, BROPT_NF_CALL_ARPTABLES, !!val);
 	return 0;
@@ -821,11 +899,17 @@ static ssize_t vlan_stats_enabled_show(struct device *d,
 	return sprintf(buf, "%u\n", br_opt_get(br, BROPT_VLAN_STATS_ENABLED));
 }
 
+static int set_vlan_stats_enabled(struct net_bridge *br, unsigned long val,
+				  struct netlink_ext_ack *extack)
+{
+	return br_vlan_set_stats(br, val);
+}
+
 static ssize_t vlan_stats_enabled_store(struct device *d,
 					struct device_attribute *attr,
 					const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_vlan_set_stats);
+	return store_bridge_parm(d, buf, len, set_vlan_stats_enabled);
 }
 static DEVICE_ATTR_RW(vlan_stats_enabled);
 
@@ -837,11 +921,17 @@ static ssize_t vlan_stats_per_port_show(struct device *d,
 	return sprintf(buf, "%u\n", br_opt_get(br, BROPT_VLAN_STATS_PER_PORT));
 }
 
+static int set_vlan_stats_per_port(struct net_bridge *br, unsigned long val,
+				   struct netlink_ext_ack *extack)
+{
+	return br_vlan_set_stats_per_port(br, val);
+}
+
 static ssize_t vlan_stats_per_port_store(struct device *d,
 					 struct device_attribute *attr,
 					 const char *buf, size_t len)
 {
-	return store_bridge_parm(d, buf, len, br_vlan_set_stats_per_port);
+	return store_bridge_parm(d, buf, len, set_vlan_stats_per_port);
 }
 static DEVICE_ATTR_RW(vlan_stats_per_port);
 #endif
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 26e7e06b6a0d..c4a51095850a 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -806,7 +806,8 @@ void br_recalculate_fwd_mask(struct net_bridge *br)
 					      ~(1u << br->group_addr[5]);
 }
 
-int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val)
+int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val,
+			  struct netlink_ext_ack *extack)
 {
 	struct switchdev_attr attr = {
 		.orig_dev = br->dev,
@@ -910,7 +911,8 @@ int __br_vlan_set_proto(struct net_bridge *br, __be16 proto)
 	return err;
 }
 
-int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
+int br_vlan_set_proto(struct net_bridge *br, unsigned long val,
+		      struct netlink_ext_ack *extack)
 {
 	if (!eth_type_vlan(htons(val)))
 		return -EPROTONOSUPPORT;
@@ -1095,7 +1097,8 @@ int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid,
 	goto out;
 }
 
-int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
+int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val,
+			     struct netlink_ext_ack *extack)
 {
 	u16 pvid = val;
 	int err = 0;
@@ -1112,7 +1115,7 @@ int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
 		err = -EPERM;
 		goto out;
 	}
-	err = __br_vlan_set_default_pvid(br, pvid, NULL);
+	err = __br_vlan_set_default_pvid(br, pvid, extack);
 out:
 	return err;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set
  2021-02-13 20:43 [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA Vladimir Oltean
  2021-02-13 20:43 ` [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle Vladimir Oltean
  2021-02-13 20:43 ` [PATCH net-next 2/5] net: bridge: propagate extack through store_bridge_parm Vladimir Oltean
@ 2021-02-13 20:43 ` Vladimir Oltean
  2021-02-14  1:14   ` Florian Fainelli
  2021-02-14 10:45   ` Nikolay Aleksandrov
  2021-02-13 20:43 ` [PATCH net-next 4/5] net: dsa: propagate extack to .port_vlan_add Vladimir Oltean
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-02-13 20:43 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang,
	Ido Schimmel

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(-)

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);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 4/5] net: dsa: propagate extack to .port_vlan_add
  2021-02-13 20:43 [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA Vladimir Oltean
                   ` (2 preceding siblings ...)
  2021-02-13 20:43 ` [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set Vladimir Oltean
@ 2021-02-13 20:43 ` Vladimir Oltean
  2021-02-14  1:16   ` Florian Fainelli
  2021-02-13 20:43 ` [PATCH net-next 5/5] net: dsa: propagate extack to .port_vlan_filtering Vladimir Oltean
  2021-02-15 20:40 ` [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA patchwork-bot+netdevbpf
  5 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2021-02-13 20:43 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang,
	Ido Schimmel

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Allow drivers to communicate their restrictions to user space directly,
instead of printing to the kernel log. Where the conversion would have
been lossy and things like VLAN ID could no longer be conveyed (due to
the lack of support for printf format specifier in netlink extack), I
chose to keep the messages in full form to the kernel log only, and
leave it up to individual driver maintainers to move more messages to
extack.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c       |  3 ++-
 drivers/net/dsa/b53/b53_priv.h         |  3 ++-
 drivers/net/dsa/bcm_sf2_cfp.c          |  2 +-
 drivers/net/dsa/dsa_loop.c             |  3 ++-
 drivers/net/dsa/hirschmann/hellcreek.c | 12 ++++++++----
 drivers/net/dsa/lantiq_gswip.c         | 12 ++++++++----
 drivers/net/dsa/microchip/ksz8795.c    |  3 ++-
 drivers/net/dsa/microchip/ksz9477.c    |  7 ++++---
 drivers/net/dsa/mt7530.c               |  3 ++-
 drivers/net/dsa/mv88e6xxx/chip.c       |  3 ++-
 drivers/net/dsa/ocelot/felix.c         |  3 ++-
 drivers/net/dsa/qca8k.c                |  3 ++-
 drivers/net/dsa/realtek-smi-core.h     |  3 ++-
 drivers/net/dsa/rtl8366.c              | 11 ++++++++---
 drivers/net/dsa/sja1105/sja1105_main.c |  6 ++++--
 include/net/dsa.h                      |  3 ++-
 net/dsa/dsa_priv.h                     |  4 +++-
 net/dsa/port.c                         |  4 +++-
 net/dsa/slave.c                        | 25 ++++++++++++++++++-------
 net/dsa/switch.c                       |  3 ++-
 20 files changed, 79 insertions(+), 37 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 72c75c7bdb65..98cc051e513e 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1444,7 +1444,8 @@ static int b53_vlan_prepare(struct dsa_switch *ds, int port,
 }
 
 int b53_vlan_add(struct dsa_switch *ds, int port,
-		 const struct switchdev_obj_port_vlan *vlan)
+		 const struct switchdev_obj_port_vlan *vlan,
+		 struct netlink_ext_ack *extack)
 {
 	struct b53_device *dev = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index ae72ef46b0b6..fc5d6fddb3fe 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -348,7 +348,8 @@ void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			     bool tx_pause, bool rx_pause);
 int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering);
 int b53_vlan_add(struct dsa_switch *ds, int port,
-		 const struct switchdev_obj_port_vlan *vlan);
+		 const struct switchdev_obj_port_vlan *vlan,
+		 struct netlink_ext_ack *extack);
 int b53_vlan_del(struct dsa_switch *ds, int port,
 		 const struct switchdev_obj_port_vlan *vlan);
 int b53_fdb_add(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index 178218cf73a3..a7e2fcf2df2c 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -891,7 +891,7 @@ static int bcm_sf2_cfp_rule_insert(struct dsa_switch *ds, int port,
 		else
 			vlan.flags = 0;
 
-		ret = ds->ops->port_vlan_add(ds, port_num, &vlan);
+		ret = ds->ops->port_vlan_add(ds, port_num, &vlan, NULL);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index 8c283f59158b..e55b63a7e907 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -199,7 +199,8 @@ static int dsa_loop_port_vlan_filtering(struct dsa_switch *ds, int port,
 }
 
 static int dsa_loop_port_vlan_add(struct dsa_switch *ds, int port,
-				  const struct switchdev_obj_port_vlan *vlan)
+				  const struct switchdev_obj_port_vlan *vlan,
+				  struct netlink_ext_ack *extack)
 {
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index f984ca75a71f..5816ef922e55 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -341,7 +341,8 @@ static u16 hellcreek_private_vid(int port)
 }
 
 static int hellcreek_vlan_prepare(struct dsa_switch *ds, int port,
-				  const struct switchdev_obj_port_vlan *vlan)
+				  const struct switchdev_obj_port_vlan *vlan,
+				  struct netlink_ext_ack *extack)
 {
 	struct hellcreek *hellcreek = ds->priv;
 	int i;
@@ -358,8 +359,10 @@ static int hellcreek_vlan_prepare(struct dsa_switch *ds, int port,
 		if (!dsa_is_user_port(ds, i))
 			continue;
 
-		if (vlan->vid == restricted_vid)
+		if (vlan->vid == restricted_vid) {
+			NL_SET_ERR_MSG_MOD(extack, "VID restricted by driver");
 			return -EBUSY;
+		}
 	}
 
 	return 0;
@@ -445,14 +448,15 @@ static void hellcreek_unapply_vlan(struct hellcreek *hellcreek, int port,
 }
 
 static int hellcreek_vlan_add(struct dsa_switch *ds, int port,
-			      const struct switchdev_obj_port_vlan *vlan)
+			      const struct switchdev_obj_port_vlan *vlan,
+			      struct netlink_ext_ack *extack)
 {
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 	struct hellcreek *hellcreek = ds->priv;
 	int err;
 
-	err = hellcreek_vlan_prepare(ds, port, vlan);
+	err = hellcreek_vlan_prepare(ds, port, vlan, extack);
 	if (err)
 		return err;
 
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 9fec97773a15..174ca3a484a0 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1128,7 +1128,8 @@ static void gswip_port_bridge_leave(struct dsa_switch *ds, int port,
 }
 
 static int gswip_port_vlan_prepare(struct dsa_switch *ds, int port,
-				   const struct switchdev_obj_port_vlan *vlan)
+				   const struct switchdev_obj_port_vlan *vlan,
+				   struct netlink_ext_ack *extack)
 {
 	struct gswip_priv *priv = ds->priv;
 	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
@@ -1163,15 +1164,18 @@ static int gswip_port_vlan_prepare(struct dsa_switch *ds, int port,
 			}
 		}
 
-		if (idx == -1)
+		if (idx == -1) {
+			NL_SET_ERR_MSG_MOD(extack, "No slot in VLAN table");
 			return -ENOSPC;
+		}
 	}
 
 	return 0;
 }
 
 static int gswip_port_vlan_add(struct dsa_switch *ds, int port,
-			       const struct switchdev_obj_port_vlan *vlan)
+			       const struct switchdev_obj_port_vlan *vlan,
+			       struct netlink_ext_ack *extack)
 {
 	struct gswip_priv *priv = ds->priv;
 	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
@@ -1179,7 +1183,7 @@ static int gswip_port_vlan_add(struct dsa_switch *ds, int port,
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 	int err;
 
-	err = gswip_port_vlan_prepare(ds, port, vlan);
+	err = gswip_port_vlan_prepare(ds, port, vlan, extack);
 	if (err)
 		return err;
 
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index c87d445b30fd..1e27a3e58141 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -793,7 +793,8 @@ static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
 }
 
 static int ksz8795_port_vlan_add(struct dsa_switch *ds, int port,
-				 const struct switchdev_obj_port_vlan *vlan)
+				 const struct switchdev_obj_port_vlan *vlan,
+				 struct netlink_ext_ack *extack)
 {
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	struct ksz_device *dev = ds->priv;
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 00e38c8e0d01..772e34d5b6b8 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -511,7 +511,8 @@ static int ksz9477_port_vlan_filtering(struct dsa_switch *ds, int port,
 }
 
 static int ksz9477_port_vlan_add(struct dsa_switch *ds, int port,
-				 const struct switchdev_obj_port_vlan *vlan)
+				 const struct switchdev_obj_port_vlan *vlan,
+				 struct netlink_ext_ack *extack)
 {
 	struct ksz_device *dev = ds->priv;
 	u32 vlan_table[3];
@@ -520,7 +521,7 @@ static int ksz9477_port_vlan_add(struct dsa_switch *ds, int port,
 
 	err = ksz9477_get_vlan_table(dev, vlan->vid, vlan_table);
 	if (err) {
-		dev_dbg(dev->dev, "Failed to get vlan table\n");
+		NL_SET_ERR_MSG_MOD(extack, "Failed to get vlan table");
 		return err;
 	}
 
@@ -535,7 +536,7 @@ static int ksz9477_port_vlan_add(struct dsa_switch *ds, int port,
 
 	err = ksz9477_set_vlan_table(dev, vlan->vid, vlan_table);
 	if (err) {
-		dev_dbg(dev->dev, "Failed to set vlan table\n");
+		NL_SET_ERR_MSG_MOD(extack, "Failed to set vlan table");
 		return err;
 	}
 
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index eb13ba79dd01..c089cd48e65d 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1483,7 +1483,8 @@ mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid,
 
 static int
 mt7530_port_vlan_add(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_vlan *vlan)
+		     const struct switchdev_obj_port_vlan *vlan,
+		     struct netlink_ext_ack *extack)
 {
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 0ef1fadfec68..d46f0c096c97 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1982,7 +1982,8 @@ static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port,
 }
 
 static int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
-				   const struct switchdev_obj_port_vlan *vlan)
+				   const struct switchdev_obj_port_vlan *vlan,
+				   struct netlink_ext_ack *extack)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index d3180b0f2307..4db54d91eae2 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -651,7 +651,8 @@ static int felix_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
 }
 
 static int felix_vlan_add(struct dsa_switch *ds, int port,
-			  const struct switchdev_obj_port_vlan *vlan)
+			  const struct switchdev_obj_port_vlan *vlan,
+			  struct netlink_ext_ack *extack)
 {
 	struct ocelot *ocelot = ds->priv;
 	u16 flags = vlan->flags;
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 6127823d6c2e..73978e7e85cd 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1313,7 +1313,8 @@ qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 
 static int
 qca8k_port_vlan_add(struct dsa_switch *ds, int port,
-		    const struct switchdev_obj_port_vlan *vlan)
+		    const struct switchdev_obj_port_vlan *vlan,
+		    struct netlink_ext_ack *extack)
 {
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
diff --git a/drivers/net/dsa/realtek-smi-core.h b/drivers/net/dsa/realtek-smi-core.h
index 26376b052594..93a3e05a6f71 100644
--- a/drivers/net/dsa/realtek-smi-core.h
+++ b/drivers/net/dsa/realtek-smi-core.h
@@ -133,7 +133,8 @@ int rtl8366_init_vlan(struct realtek_smi *smi);
 int rtl8366_vlan_filtering(struct dsa_switch *ds, int port,
 			   bool vlan_filtering);
 int rtl8366_vlan_add(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_vlan *vlan);
+		     const struct switchdev_obj_port_vlan *vlan,
+		     struct netlink_ext_ack *extack);
 int rtl8366_vlan_del(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan);
 void rtl8366_get_strings(struct dsa_switch *ds, int port, u32 stringset,
diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index 3b24f2e13200..76303a77aa82 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -375,7 +375,8 @@ int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 EXPORT_SYMBOL_GPL(rtl8366_vlan_filtering);
 
 int rtl8366_vlan_add(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_vlan *vlan)
+		     const struct switchdev_obj_port_vlan *vlan,
+		     struct netlink_ext_ack *extack)
 {
 	bool untagged = !!(vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED);
 	bool pvid = !!(vlan->flags & BRIDGE_VLAN_INFO_PVID);
@@ -384,16 +385,20 @@ int rtl8366_vlan_add(struct dsa_switch *ds, int port,
 	u32 untag = 0;
 	int ret;
 
-	if (!smi->ops->is_vlan_valid(smi, vlan->vid))
+	if (!smi->ops->is_vlan_valid(smi, vlan->vid)) {
+		NL_SET_ERR_MSG_MOD(extack, "VLAN ID not valid");
 		return -EINVAL;
+	}
 
 	/* Enable VLAN in the hardware
 	 * FIXME: what's with this 4k business?
 	 * Just rtl8366_enable_vlan() seems inconclusive.
 	 */
 	ret = rtl8366_enable_vlan4k(smi, true);
-	if (ret)
+	if (ret) {
+		NL_SET_ERR_MSG_MOD(extack, "Failed to enable VLAN 4K");
 		return ret;
+	}
 
 	dev_info(smi->dev, "add VLAN %d on port %d, %s, %s\n",
 		 vlan->vid, port, untagged ? "untagged" : "tagged",
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 260073e830c7..6aea8034c32b 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2795,7 +2795,8 @@ static int sja1105_vlan_del_one(struct dsa_switch *ds, int port, u16 vid,
 }
 
 static int sja1105_vlan_add(struct dsa_switch *ds, int port,
-			    const struct switchdev_obj_port_vlan *vlan)
+			    const struct switchdev_obj_port_vlan *vlan,
+			    struct netlink_ext_ack *extack)
 {
 	struct sja1105_private *priv = ds->priv;
 	bool vlan_table_changed = false;
@@ -2807,7 +2808,8 @@ static int sja1105_vlan_add(struct dsa_switch *ds, int port,
 	 */
 	if (priv->vlan_state != SJA1105_VLAN_FILTERING_FULL &&
 	    vid_is_dsa_8021q(vlan->vid)) {
-		dev_err(ds->dev, "Range 1024-3071 reserved for dsa_8021q operation\n");
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Range 1024-3071 reserved for dsa_8021q operation");
 		return -EBUSY;
 	}
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 74457aaffec7..94dfe96df39a 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -641,7 +641,8 @@ struct dsa_switch_ops {
 	int	(*port_vlan_filtering)(struct dsa_switch *ds, int port,
 				       bool vlan_filtering);
 	int	(*port_vlan_add)(struct dsa_switch *ds, int port,
-				 const struct switchdev_obj_port_vlan *vlan);
+				 const struct switchdev_obj_port_vlan *vlan,
+				 struct netlink_ext_ack *extack);
 	int	(*port_vlan_del)(struct dsa_switch *ds, int port,
 				 const struct switchdev_obj_port_vlan *vlan);
 	/*
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index f5949b39f6f7..17a9f82db937 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -75,6 +75,7 @@ struct dsa_notifier_vlan_info {
 	const struct switchdev_obj_port_vlan *vlan;
 	int sw_index;
 	int port;
+	struct netlink_ext_ack *extack;
 };
 
 /* DSA_NOTIFIER_MTU */
@@ -192,7 +193,8 @@ int dsa_port_bridge_flags(const struct dsa_port *dp,
 int dsa_port_mrouter(struct dsa_port *dp, bool mrouter,
 		     struct netlink_ext_ack *extack);
 int dsa_port_vlan_add(struct dsa_port *dp,
-		      const struct switchdev_obj_port_vlan *vlan);
+		      const struct switchdev_obj_port_vlan *vlan,
+		      struct netlink_ext_ack *extack);
 int dsa_port_vlan_del(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan);
 int dsa_port_link_register_of(struct dsa_port *dp);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 80e6471a7a5c..03ecefe1064a 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -535,12 +535,14 @@ int dsa_port_mdb_del(const struct dsa_port *dp,
 }
 
 int dsa_port_vlan_add(struct dsa_port *dp,
-		      const struct switchdev_obj_port_vlan *vlan)
+		      const struct switchdev_obj_port_vlan *vlan,
+		      struct netlink_ext_ack *extack)
 {
 	struct dsa_notifier_vlan_info info = {
 		.sw_index = dp->ds->index,
 		.port = dp->index,
 		.vlan = vlan,
+		.extack = extack,
 	};
 
 	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 8c9a41a7209a..9ec487b63e13 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -357,11 +357,14 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 		rcu_read_lock();
 		err = dsa_slave_vlan_check_for_8021q_uppers(dev, &vlan);
 		rcu_read_unlock();
-		if (err)
+		if (err) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Port already has a VLAN upper with this VID");
 			return err;
+		}
 	}
 
-	err = dsa_port_vlan_add(dp, &vlan);
+	err = dsa_port_vlan_add(dp, &vlan, extack);
 	if (err)
 		return err;
 
@@ -371,7 +374,7 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 	 */
 	vlan.flags &= ~BRIDGE_VLAN_INFO_PVID;
 
-	err = dsa_port_vlan_add(dp->cpu_dp, &vlan);
+	err = dsa_port_vlan_add(dp->cpu_dp, &vlan, extack);
 	if (err)
 		return err;
 
@@ -1287,17 +1290,25 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 		/* This API only allows programming tagged, non-PVID VIDs */
 		.flags = 0,
 	};
+	struct netlink_ext_ack extack = {0};
 	int ret;
 
 	/* User port... */
-	ret = dsa_port_vlan_add(dp, &vlan);
-	if (ret)
+	ret = dsa_port_vlan_add(dp, &vlan, &extack);
+	if (ret) {
+		if (extack._msg)
+			netdev_err(dev, "%s\n", extack._msg);
 		return ret;
+	}
 
 	/* And CPU port... */
-	ret = dsa_port_vlan_add(dp->cpu_dp, &vlan);
-	if (ret)
+	ret = dsa_port_vlan_add(dp->cpu_dp, &vlan, &extack);
+	if (ret) {
+		if (extack._msg)
+			netdev_err(dev, "CPU port %d: %s\n", dp->cpu_dp->index,
+				   extack._msg);
 		return ret;
+	}
 
 	return vlan_vid_add(master, proto, vid);
 }
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 1906179e59f7..c82d201181a5 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -291,7 +291,8 @@ static int dsa_switch_vlan_add(struct dsa_switch *ds,
 
 	for (port = 0; port < ds->num_ports; port++) {
 		if (dsa_switch_vlan_match(ds, port, info)) {
-			err = ds->ops->port_vlan_add(ds, port, info->vlan);
+			err = ds->ops->port_vlan_add(ds, port, info->vlan,
+						     info->extack);
 			if (err)
 				return err;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 5/5] net: dsa: propagate extack to .port_vlan_filtering
  2021-02-13 20:43 [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA Vladimir Oltean
                   ` (3 preceding siblings ...)
  2021-02-13 20:43 ` [PATCH net-next 4/5] net: dsa: propagate extack to .port_vlan_add Vladimir Oltean
@ 2021-02-13 20:43 ` Vladimir Oltean
  2021-02-14  1:17   ` Florian Fainelli
  2021-02-15 20:40 ` [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA patchwork-bot+netdevbpf
  5 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2021-02-13 20:43 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang,
	Ido Schimmel

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Some drivers can't dynamically change the VLAN filtering option, or
impose some restrictions, it would be nice to propagate this info
through netlink instead of printing it to a kernel log that might never
be read. Also netlink extack includes the module that emitted the
message, which means that it's easier to figure out which ones are
driver-generated errors as opposed to command misuse.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c          |  3 ++-
 drivers/net/dsa/b53/b53_priv.h            |  3 ++-
 drivers/net/dsa/dsa_loop.c                |  3 ++-
 drivers/net/dsa/hirschmann/hellcreek.c    |  3 ++-
 drivers/net/dsa/lantiq_gswip.c            | 10 +++++++---
 drivers/net/dsa/microchip/ksz8795.c       |  3 ++-
 drivers/net/dsa/microchip/ksz9477.c       |  3 ++-
 drivers/net/dsa/mt7530.c                  |  4 ++--
 drivers/net/dsa/mv88e6xxx/chip.c          |  3 ++-
 drivers/net/dsa/ocelot/felix.c            |  3 ++-
 drivers/net/dsa/qca8k.c                   |  3 ++-
 drivers/net/dsa/realtek-smi-core.h        |  4 ++--
 drivers/net/dsa/rtl8366.c                 |  3 ++-
 drivers/net/dsa/sja1105/sja1105.h         |  3 ++-
 drivers/net/dsa/sja1105/sja1105_devlink.c |  2 +-
 drivers/net/dsa/sja1105/sja1105_main.c    |  9 +++++----
 include/net/dsa.h                         |  3 ++-
 net/dsa/dsa_priv.h                        |  3 ++-
 net/dsa/port.c                            | 18 +++++++++++-------
 net/dsa/slave.c                           |  3 ++-
 net/dsa/switch.c                          |  6 +++++-
 21 files changed, 61 insertions(+), 34 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 98cc051e513e..ae86ded1e2a1 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1409,7 +1409,8 @@ void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL(b53_phylink_mac_link_up);
 
-int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
+int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
+		       struct netlink_ext_ack *extack)
 {
 	struct b53_device *dev = ds->priv;
 
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index fc5d6fddb3fe..faf983fbca82 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -346,7 +346,8 @@ void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			     struct phy_device *phydev,
 			     int speed, int duplex,
 			     bool tx_pause, bool rx_pause);
-int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering);
+int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
+		       struct netlink_ext_ack *extack);
 int b53_vlan_add(struct dsa_switch *ds, int port,
 		 const struct switchdev_obj_port_vlan *vlan,
 		 struct netlink_ext_ack *extack);
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index e55b63a7e907..bfdf3324aac3 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -190,7 +190,8 @@ static void dsa_loop_port_stp_state_set(struct dsa_switch *ds, int port,
 }
 
 static int dsa_loop_port_vlan_filtering(struct dsa_switch *ds, int port,
-					bool vlan_filtering)
+					bool vlan_filtering,
+					struct netlink_ext_ack *extack)
 {
 	dev_dbg(ds->dev, "%s: port: %d, vlan_filtering: %d\n",
 		__func__, port, vlan_filtering);
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 5816ef922e55..463137c39db2 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -875,7 +875,8 @@ static int hellcreek_fdb_dump(struct dsa_switch *ds, int port,
 }
 
 static int hellcreek_vlan_filtering(struct dsa_switch *ds, int port,
-				    bool vlan_filtering)
+				    bool vlan_filtering,
+				    struct netlink_ext_ack *extack)
 {
 	struct hellcreek *hellcreek = ds->priv;
 
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 174ca3a484a0..52e865a3912c 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -727,14 +727,18 @@ static int gswip_pce_load_microcode(struct gswip_priv *priv)
 }
 
 static int gswip_port_vlan_filtering(struct dsa_switch *ds, int port,
-				     bool vlan_filtering)
+				     bool vlan_filtering,
+				     struct netlink_ext_ack *extack)
 {
 	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
 	struct gswip_priv *priv = ds->priv;
 
 	/* Do not allow changing the VLAN filtering options while in bridge */
-	if (bridge && !!(priv->port_vlan_filter & BIT(port)) != vlan_filtering)
+	if (bridge && !!(priv->port_vlan_filter & BIT(port)) != vlan_filtering) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Dynamic toggling of vlan_filtering not supported");
 		return -EIO;
+	}
 
 	if (vlan_filtering) {
 		/* Use port based VLAN tag */
@@ -773,7 +777,7 @@ static int gswip_setup(struct dsa_switch *ds)
 	/* disable port fetch/store dma on all ports */
 	for (i = 0; i < priv->hw_info->max_ports; i++) {
 		gswip_port_disable(ds, i);
-		gswip_port_vlan_filtering(ds, i, false);
+		gswip_port_vlan_filtering(ds, i, false, NULL);
 	}
 
 	/* enable Switch */
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 1e27a3e58141..b4b7de63ca79 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -783,7 +783,8 @@ static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
 }
 
 static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
-				       bool flag)
+				       bool flag,
+				       struct netlink_ext_ack *extack)
 {
 	struct ksz_device *dev = ds->priv;
 
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 772e34d5b6b8..55e5d479acce 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -493,7 +493,8 @@ static void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port)
 }
 
 static int ksz9477_port_vlan_filtering(struct dsa_switch *ds, int port,
-				       bool flag)
+				       bool flag,
+				       struct netlink_ext_ack *extack)
 {
 	struct ksz_device *dev = ds->priv;
 
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c089cd48e65d..c17de2bcf2fe 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1376,8 +1376,8 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
 }
 
 static int
-mt7530_port_vlan_filtering(struct dsa_switch *ds, int port,
-			   bool vlan_filtering)
+mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
+			   struct netlink_ext_ack *extack)
 {
 	if (vlan_filtering) {
 		/* The port is being kept as VLAN-unaware port when bridge is
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d46f0c096c97..903d619e08ed 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1600,7 +1600,8 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 }
 
 static int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
-					 bool vlan_filtering)
+					 bool vlan_filtering,
+					 struct netlink_ext_ack *extack)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	u16 mode = vlan_filtering ? MV88E6XXX_PORT_CTL2_8021Q_MODE_SECURE :
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 4db54d91eae2..c1e02d56dd45 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -643,7 +643,8 @@ static int felix_vlan_prepare(struct dsa_switch *ds, int port,
 				   flags & BRIDGE_VLAN_INFO_UNTAGGED);
 }
 
-static int felix_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
+static int felix_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
+				struct netlink_ext_ack *extack)
 {
 	struct ocelot *ocelot = ds->priv;
 
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 73978e7e85cd..cdaf9f85a2cb 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1294,7 +1294,8 @@ qca8k_port_fdb_dump(struct dsa_switch *ds, int port,
 }
 
 static int
-qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
+qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
+			  struct netlink_ext_ack *extack)
 {
 	struct qca8k_priv *priv = ds->priv;
 
diff --git a/drivers/net/dsa/realtek-smi-core.h b/drivers/net/dsa/realtek-smi-core.h
index 93a3e05a6f71..fcf465f7f922 100644
--- a/drivers/net/dsa/realtek-smi-core.h
+++ b/drivers/net/dsa/realtek-smi-core.h
@@ -130,8 +130,8 @@ int rtl8366_enable_vlan4k(struct realtek_smi *smi, bool enable);
 int rtl8366_enable_vlan(struct realtek_smi *smi, bool enable);
 int rtl8366_reset_vlan(struct realtek_smi *smi);
 int rtl8366_init_vlan(struct realtek_smi *smi);
-int rtl8366_vlan_filtering(struct dsa_switch *ds, int port,
-			   bool vlan_filtering);
+int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
+			   struct netlink_ext_ack *extack);
 int rtl8366_vlan_add(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan,
 		     struct netlink_ext_ack *extack);
diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index 76303a77aa82..75897a369096 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -340,7 +340,8 @@ int rtl8366_init_vlan(struct realtek_smi *smi)
 }
 EXPORT_SYMBOL_GPL(rtl8366_init_vlan);
 
-int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
+int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
+			   struct netlink_ext_ack *extack)
 {
 	struct realtek_smi *smi = ds->priv;
 	struct rtl8366_vlan_4k vlan4k;
diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index 15a0893d0ff1..90f0f6f3124e 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -247,7 +247,8 @@ enum sja1105_reset_reason {
 
 int sja1105_static_config_reload(struct sja1105_private *priv,
 				 enum sja1105_reset_reason reason);
-int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled);
+int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
+			   struct netlink_ext_ack *extack);
 void sja1105_frame_memory_partitioning(struct sja1105_private *priv);
 
 /* From sja1105_devlink.c */
diff --git a/drivers/net/dsa/sja1105/sja1105_devlink.c b/drivers/net/dsa/sja1105/sja1105_devlink.c
index b4bf1b10e66c..b6a4a16b8c7e 100644
--- a/drivers/net/dsa/sja1105/sja1105_devlink.c
+++ b/drivers/net/dsa/sja1105/sja1105_devlink.c
@@ -143,7 +143,7 @@ static int sja1105_best_effort_vlan_filtering_set(struct sja1105_private *priv,
 		dp = dsa_to_port(ds, port);
 		vlan_filtering = dsa_port_is_vlan_filtering(dp);
 
-		rc = sja1105_vlan_filtering(ds, port, vlan_filtering);
+		rc = sja1105_vlan_filtering(ds, port, vlan_filtering, NULL);
 		if (rc)
 			break;
 	}
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 6aea8034c32b..85a39d599ff3 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2639,7 +2639,8 @@ static int sja1105_build_vlan_table(struct sja1105_private *priv, bool notify)
  * which can only be partially reconfigured at runtime (and not the TPID).
  * So a switch reset is required.
  */
-int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
+int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
+			   struct netlink_ext_ack *extack)
 {
 	struct sja1105_l2_lookup_params_entry *l2_lookup_params;
 	struct sja1105_general_params_entry *general_params;
@@ -2653,8 +2654,8 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
 
 	list_for_each_entry(rule, &priv->flow_block.rules, list) {
 		if (rule->type == SJA1105_RULE_VL) {
-			dev_err(ds->dev,
-				"Cannot change VLAN filtering with active VL rules\n");
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Cannot change VLAN filtering with active VL rules");
 			return -EBUSY;
 		}
 	}
@@ -2736,7 +2737,7 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
 
 	rc = sja1105_static_config_reload(priv, SJA1105_VLAN_FILTERING);
 	if (rc)
-		dev_err(ds->dev, "Failed to change VLAN Ethertype\n");
+		NL_SET_ERR_MSG_MOD(extack, "Failed to change VLAN Ethertype");
 
 	/* Switch port identification based on 802.1Q is only passable
 	 * if we are not under a vlan_filtering bridge. So make sure
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 94dfe96df39a..f9e9c149322f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -639,7 +639,8 @@ struct dsa_switch_ops {
 	 * VLAN support
 	 */
 	int	(*port_vlan_filtering)(struct dsa_switch *ds, int port,
-				       bool vlan_filtering);
+				       bool vlan_filtering,
+				       struct netlink_ext_ack *extack);
 	int	(*port_vlan_add)(struct dsa_switch *ds, int port,
 				 const struct switchdev_obj_port_vlan *vlan,
 				 struct netlink_ext_ack *extack);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 17a9f82db937..e9d1e76c42ba 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -170,7 +170,8 @@ int dsa_port_lag_change(struct dsa_port *dp,
 int dsa_port_lag_join(struct dsa_port *dp, struct net_device *lag_dev,
 		      struct netdev_lag_upper_info *uinfo);
 void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag_dev);
-int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering);
+int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
+			    struct netlink_ext_ack *extack);
 bool dsa_port_skip_vlan_configuration(struct dsa_port *dp);
 int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock);
 int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 03ecefe1064a..14a1d0d77657 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -294,7 +294,8 @@ void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag)
 
 /* Must be called under rcu_read_lock() */
 static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
-					      bool vlan_filtering)
+					      bool vlan_filtering,
+					      struct netlink_ext_ack *extack)
 {
 	struct dsa_switch *ds = dp->ds;
 	int err, i;
@@ -324,8 +325,8 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 			 */
 			err = br_vlan_get_info(br, vid, &br_info);
 			if (err == 0) {
-				dev_err(ds->dev, "Must remove upper %s first\n",
-					upper_dev->name);
+				NL_SET_ERR_MSG_MOD(extack,
+						   "Must first remove VLAN uppers having VIDs also present in bridge");
 				return false;
 			}
 		}
@@ -351,14 +352,16 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 		if (other_bridge == dp->bridge_dev)
 			continue;
 		if (br_vlan_enabled(other_bridge) != vlan_filtering) {
-			dev_err(ds->dev, "VLAN filtering is a global setting\n");
+			NL_SET_ERR_MSG_MOD(extack,
+					   "VLAN filtering is a global setting");
 			return false;
 		}
 	}
 	return true;
 }
 
-int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering)
+int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
+			    struct netlink_ext_ack *extack)
 {
 	struct dsa_switch *ds = dp->ds;
 	bool apply;
@@ -372,7 +375,7 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering)
 	 * dsa_slave_switchdev_event().
 	 */
 	rcu_read_lock();
-	apply = dsa_port_can_apply_vlan_filtering(dp, vlan_filtering);
+	apply = dsa_port_can_apply_vlan_filtering(dp, vlan_filtering, extack);
 	rcu_read_unlock();
 	if (!apply)
 		return -EINVAL;
@@ -380,7 +383,8 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering)
 	if (dsa_port_is_vlan_filtering(dp) == vlan_filtering)
 		return 0;
 
-	err = ds->ops->port_vlan_filtering(ds, dp->index, vlan_filtering);
+	err = ds->ops->port_vlan_filtering(ds, dp->index, vlan_filtering,
+					   extack);
 	if (err)
 		return err;
 
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9ec487b63e13..5ecb43a1b6e0 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -286,7 +286,8 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
 		ret = dsa_port_set_state(dp, attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
-		ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering);
+		ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering,
+					      extack);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
 		ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index c82d201181a5..db2a9b221988 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -106,6 +106,7 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 {
 	bool unset_vlan_filtering = br_vlan_enabled(info->br);
 	struct dsa_switch_tree *dst = ds->dst;
+	struct netlink_ext_ack extack = {0};
 	int err, i;
 
 	if (dst->index == info->tree_index && ds->index == info->sw_index &&
@@ -137,7 +138,10 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 	}
 	if (unset_vlan_filtering) {
 		err = dsa_port_vlan_filtering(dsa_to_port(ds, info->port),
-					      false);
+					      false, &extack);
+		if (extack._msg)
+			dev_err(ds->dev, "port %d: %s\n", info->port,
+				extack._msg);
 		if (err && err != EOPNOTSUPP)
 			return err;
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle
  2021-02-13 20:43 ` [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle Vladimir Oltean
@ 2021-02-14  1:04   ` Florian Fainelli
  2021-02-14 10:20   ` Nikolay Aleksandrov
  1 sibling, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:04 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Kurt Kanzenbach, Woojung Huh,
	Linus Walleij, Hauke Mehrtens, Jiri Pirko, Ivan Vecera,
	Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang, Ido Schimmel



On 2/13/2021 12:43, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This function is identical with br_vlan_filter_toggle.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 2/5] net: bridge: propagate extack through store_bridge_parm
  2021-02-13 20:43 ` [PATCH net-next 2/5] net: bridge: propagate extack through store_bridge_parm Vladimir Oltean
@ 2021-02-14  1:06   ` Florian Fainelli
  2021-02-14 10:43   ` Nikolay Aleksandrov
  1 sibling, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:06 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Kurt Kanzenbach, Woojung Huh,
	Linus Walleij, Hauke Mehrtens, Jiri Pirko, Ivan Vecera,
	Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang, Ido Schimmel



On 2/13/2021 12:43, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The bridge sysfs interface stores parameters for the STP, VLAN,
> multicast etc subsystems using a predefined function prototype.
> Sometimes the underlying function being called supports a netlink
> extended ack message, and we ignore it.
> 
> Let's expand the store_bridge_parm function prototype to include the
> extack, and just print it to console, but at least propagate it where
> applicable. Where not applicable, create a shim function in the
> br_sysfs_br.c file that discards the extra function argument.
> 
> This patch allows us to propagate the extack argument to
> br_vlan_set_default_pvid, br_vlan_set_proto and br_vlan_filter_toggle,
> and from there, further up in br_changelink from br_netlink.c.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set
  2021-02-13 20:43 ` [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set Vladimir Oltean
@ 2021-02-14  1:14   ` Florian Fainelli
  2021-02-14 10:45   ` Nikolay Aleksandrov
  1 sibling, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:14 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Kurt Kanzenbach, Woojung Huh,
	Linus Walleij, Hauke Mehrtens, Jiri Pirko, Ivan Vecera,
	Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang, Ido Schimmel



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>
-- 
Florian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 4/5] net: dsa: propagate extack to .port_vlan_add
  2021-02-13 20:43 ` [PATCH net-next 4/5] net: dsa: propagate extack to .port_vlan_add Vladimir Oltean
@ 2021-02-14  1:16   ` Florian Fainelli
  0 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:16 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Kurt Kanzenbach, Woojung Huh,
	Linus Walleij, Hauke Mehrtens, Jiri Pirko, Ivan Vecera,
	Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang, Ido Schimmel



On 2/13/2021 12:43, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Allow drivers to communicate their restrictions to user space directly,
> instead of printing to the kernel log. Where the conversion would have
> been lossy and things like VLAN ID could no longer be conveyed (due to
> the lack of support for printf format specifier in netlink extack), I
> chose to keep the messages in full form to the kernel log only, and
> leave it up to individual driver maintainers to move more messages to
> extack.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 5/5] net: dsa: propagate extack to .port_vlan_filtering
  2021-02-13 20:43 ` [PATCH net-next 5/5] net: dsa: propagate extack to .port_vlan_filtering Vladimir Oltean
@ 2021-02-14  1:17   ` Florian Fainelli
  0 siblings, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:17 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Kurt Kanzenbach, Woojung Huh,
	Linus Walleij, Hauke Mehrtens, Jiri Pirko, Ivan Vecera,
	Roopa Prabhu, Nikolay Aleksandrov, DENG Qingfang, Ido Schimmel



On 2/13/2021 12:43, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Some drivers can't dynamically change the VLAN filtering option, or
> impose some restrictions, it would be nice to propagate this info
> through netlink instead of printing it to a kernel log that might never
> be read. Also netlink extack includes the module that emitted the
> message, which means that it's easier to figure out which ones are
> driver-generated errors as opposed to command misuse.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle
  2021-02-13 20:43 ` [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle Vladimir Oltean
  2021-02-14  1:04   ` Florian Fainelli
@ 2021-02-14 10:20   ` Nikolay Aleksandrov
  1 sibling, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2021-02-14 10:20 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, DENG Qingfang, Ido Schimmel

On 13/02/2021 22:43, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This function is identical with br_vlan_filter_toggle.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/bridge/br_netlink.c | 2 +-
>  net/bridge/br_private.h | 5 ++---
>  net/bridge/br_vlan.c    | 7 +------
>  3 files changed, 4 insertions(+), 10 deletions(-)
> 

Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 2/5] net: bridge: propagate extack through store_bridge_parm
  2021-02-13 20:43 ` [PATCH net-next 2/5] net: bridge: propagate extack through store_bridge_parm Vladimir Oltean
  2021-02-14  1:06   ` Florian Fainelli
@ 2021-02-14 10:43   ` Nikolay Aleksandrov
  1 sibling, 0 replies; 17+ messages in thread
From: Nikolay Aleksandrov @ 2021-02-14 10:43 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, DENG Qingfang, Ido Schimmel

On 13/02/2021 22:43, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The bridge sysfs interface stores parameters for the STP, VLAN,
> multicast etc subsystems using a predefined function prototype.
> Sometimes the underlying function being called supports a netlink
> extended ack message, and we ignore it.
> 
> Let's expand the store_bridge_parm function prototype to include the
> extack, and just print it to console, but at least propagate it where
> applicable. Where not applicable, create a shim function in the
> br_sysfs_br.c file that discards the extra function argument.
> 
> This patch allows us to propagate the extack argument to
> br_vlan_set_default_pvid, br_vlan_set_proto and br_vlan_filter_toggle,
> and from there, further up in br_changelink from br_netlink.c.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/bridge/br_netlink.c  |   2 +-
>  net/bridge/br_private.h  |   9 ++-
>  net/bridge/br_sysfs_br.c | 166 ++++++++++++++++++++++++++++++---------
>  net/bridge/br_vlan.c     |  11 ++-
>  4 files changed, 142 insertions(+), 46 deletions(-)
> 

Hi,
You have to update the !CONFIG_BRIDGE_VLAN_FILTERING br_vlan_filter_toggle stub as well
otherwise compilation will fail. 

Thanks,
 Nik


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set
  2021-02-13 20:43 ` [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set Vladimir Oltean
  2021-02-14  1:14   ` Florian Fainelli
@ 2021-02-14 10:45   ` Nikolay Aleksandrov
  2021-02-14 16:39     ` Vladimir Oltean
  1 sibling, 1 reply; 17+ messages in thread
From: Nikolay Aleksandrov @ 2021-02-14 10:45 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Kurt Kanzenbach,
	Woojung Huh, Linus Walleij, Hauke Mehrtens, Jiri Pirko,
	Ivan Vecera, Roopa Prabhu, DENG Qingfang, Ido Schimmel

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.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set
  2021-02-14 10:45   ` Nikolay Aleksandrov
@ 2021-02-14 16:39     ` Vladimir Oltean
  0 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-02-14 16:39 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: David S . Miller, Jakub Kicinski, netdev, Florian Fainelli,
	Andrew Lunn, Vivien Didelot, Kurt Kanzenbach, Woojung Huh,
	Linus Walleij, Hauke Mehrtens, Jiri Pirko, Ivan Vecera,
	Roopa Prabhu, DENG Qingfang, Ido Schimmel

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA
  2021-02-13 20:43 [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA Vladimir Oltean
                   ` (4 preceding siblings ...)
  2021-02-13 20:43 ` [PATCH net-next 5/5] net: dsa: propagate extack to .port_vlan_filtering Vladimir Oltean
@ 2021-02-15 20:40 ` patchwork-bot+netdevbpf
  2021-02-15 20:49   ` Vladimir Oltean
  5 siblings, 1 reply; 17+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-15 20:40 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, netdev, f.fainelli, andrew, vivien.didelot, kurt,
	woojung.huh, linus.walleij, hauke, jiri, ivecera, roopa, nikolay,
	dqfext, idosch

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Sat, 13 Feb 2021 22:43:14 +0200 you wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This series moves the restriction messages printed by the DSA core, and
> by some individual device drivers, into the netlink extended ack
> structure, to be communicated to user space where possible, or still
> printed to the kernel log from the bridge layer.
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] net: bridge: remove __br_vlan_filter_toggle
    https://git.kernel.org/netdev/net-next/c/7a572964e0c4
  - [net-next,2/5] net: bridge: propagate extack through store_bridge_parm
    https://git.kernel.org/netdev/net-next/c/9e781401cbfc
  - [net-next,3/5] net: bridge: propagate extack through switchdev_port_attr_set
    https://git.kernel.org/netdev/net-next/c/dcbdf1350e33
  - [net-next,4/5] net: dsa: propagate extack to .port_vlan_add
    https://git.kernel.org/netdev/net-next/c/31046a5fd92c
  - [net-next,5/5] net: dsa: propagate extack to .port_vlan_filtering
    https://git.kernel.org/netdev/net-next/c/89153ed6ebc1

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA
  2021-02-15 20:40 ` [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA patchwork-bot+netdevbpf
@ 2021-02-15 20:49   ` Vladimir Oltean
  0 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2021-02-15 20:49 UTC (permalink / raw)
  To: patchwork-bot+netdevbpf, davem
  Cc: kuba, netdev, f.fainelli, andrew, vivien.didelot, kurt,
	woojung.huh, linus.walleij, hauke, jiri, ivecera, roopa, nikolay,
	dqfext, idosch

On Mon, Feb 15, 2021 at 08:40:08PM +0000, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This series was applied to netdev/net-next.git (refs/heads/master):
> 
> On Sat, 13 Feb 2021 22:43:14 +0200 you wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > This series moves the restriction messages printed by the DSA core, and
> > by some individual device drivers, into the netlink extended ack
> > structure, to be communicated to user space where possible, or still
> > printed to the kernel log from the bridge layer.
> > 
> > [...]
> 
> Here is the summary with links:
>   - [net-next,1/5] net: bridge: remove __br_vlan_filter_toggle
>     https://git.kernel.org/netdev/net-next/c/7a572964e0c4
>   - [net-next,2/5] net: bridge: propagate extack through store_bridge_parm
>     https://git.kernel.org/netdev/net-next/c/9e781401cbfc
>   - [net-next,3/5] net: bridge: propagate extack through switchdev_port_attr_set
>     https://git.kernel.org/netdev/net-next/c/dcbdf1350e33
>   - [net-next,4/5] net: dsa: propagate extack to .port_vlan_add
>     https://git.kernel.org/netdev/net-next/c/31046a5fd92c
>   - [net-next,5/5] net: dsa: propagate extack to .port_vlan_filtering
>     https://git.kernel.org/netdev/net-next/c/89153ed6ebc1
> 
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html

Ouch, I wasn't expecting you to merge these patches.
I had told Nikolay in patch 3 that I was going to resend after the merge window:
https://patchwork.kernel.org/project/netdevbpf/patch/20210213204319.1226170-4-olteanv@gmail.com/

Nonetheless, since there's going to be a short window of build breakage
in net-next when CONFIG_SWITCHDEV=n and/or CONFIG_BRIDGE_VLAN_FILTERING=n
regardless of whether you revert the series now or wait a bit, can I
just send the fixup patch for the function prototypes? Shouldn't take me
more than 15 minutes or so.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-02-15 20:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-13 20:43 [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA Vladimir Oltean
2021-02-13 20:43 ` [PATCH net-next 1/5] net: bridge: remove __br_vlan_filter_toggle Vladimir Oltean
2021-02-14  1:04   ` Florian Fainelli
2021-02-14 10:20   ` Nikolay Aleksandrov
2021-02-13 20:43 ` [PATCH net-next 2/5] net: bridge: propagate extack through store_bridge_parm Vladimir Oltean
2021-02-14  1:06   ` Florian Fainelli
2021-02-14 10:43   ` Nikolay Aleksandrov
2021-02-13 20:43 ` [PATCH net-next 3/5] net: bridge: propagate extack through switchdev_port_attr_set Vladimir Oltean
2021-02-14  1:14   ` Florian Fainelli
2021-02-14 10:45   ` Nikolay Aleksandrov
2021-02-14 16:39     ` Vladimir Oltean
2021-02-13 20:43 ` [PATCH net-next 4/5] net: dsa: propagate extack to .port_vlan_add Vladimir Oltean
2021-02-14  1:16   ` Florian Fainelli
2021-02-13 20:43 ` [PATCH net-next 5/5] net: dsa: propagate extack to .port_vlan_filtering Vladimir Oltean
2021-02-14  1:17   ` Florian Fainelli
2021-02-15 20:40 ` [PATCH net-next 0/5] Propagate extack for switchdev VLANs from DSA patchwork-bot+netdevbpf
2021-02-15 20:49   ` Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).