From: Vladimir Oltean <olteanv@gmail.com> To: Jakub Kicinski <kuba@kernel.org>, "David S. Miller" <davem@davemloft.net> Cc: Andrew Lunn <andrew@lunn.ch>, Vivien Didelot <vivien.didelot@gmail.com>, Florian Fainelli <f.fainelli@gmail.com>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, bridge@lists.linux-foundation.org, Roopa Prabhu <roopa@nvidia.com>, Nikolay Aleksandrov <nikolay@nvidia.com>, Jiri Pirko <jiri@resnulli.us>, Ido Schimmel <idosch@idosch.org>, Claudiu Manoil <claudiu.manoil@nxp.com>, Alexandre Belloni <alexandre.belloni@bootlin.com>, UNGLinuxDriver@microchip.com, Vadym Kochan <vkochan@marvell.com>, Taras Chornyi <tchornyi@marvell.com>, Grygorii Strashko <grygorii.strashko@ti.com>, Ioana Ciornei <ioana.ciornei@nxp.com>, Ivan Vecera <ivecera@redhat.com>, linux-omap@vger.kernel.org Subject: [PATCH v3 net-next 03/11] net: bridge: don't print in br_switchdev_set_port_flag Date: Wed, 10 Feb 2021 11:14:37 +0200 [thread overview] Message-ID: <20210210091445.741269-4-olteanv@gmail.com> (raw) In-Reply-To: <20210210091445.741269-1-olteanv@gmail.com> From: Vladimir Oltean <vladimir.oltean@nxp.com> Currently br_switchdev_set_port_flag has two options for error handling and neither is good: - The driver returns -EOPNOTSUPP in PRE_BRIDGE_FLAGS if it doesn't support offloading that flag, and this gets silently ignored and converted to an errno of 0. Nobody does this. - The driver returns some other error code, like -EINVAL, in PRE_BRIDGE_FLAGS, and br_switchdev_set_port_flag shouts loudly. The problem is that we'd like to offload some port flags during bridge join and leave, but also not have the bridge shout at us if those fail. But on the other hand we'd like the user to know that we can't offload something when they set that through netlink. And since we can't have the driver return -EOPNOTSUPP or -EINVAL depending on whether it's called by the user or internally by the bridge, let's just add an extack argument to br_switchdev_set_port_flag and propagate it to its callers. Then, when we need offloading to really fail silently, this can simply be passed a NULL argument. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- Changes in v3: - Deal with the br_switchdev_set_port_flag call from sysfs too. Changes in v2: - br_set_port_flag now returns void, so no extack there. - don't overwrite extack in br_switchdev_set_port_flag if already populated. net/bridge/br_netlink.c | 9 +++++---- net/bridge/br_private.h | 6 ++++-- net/bridge/br_switchdev.c | 13 +++++++------ net/bridge/br_sysfs_if.c | 7 +++++-- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 4e64775bd8fb..b7731614c036 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -866,7 +866,8 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[], } /* Process bridge protocol info on port */ -static int br_setport(struct net_bridge_port *p, struct nlattr *tb[]) +static int br_setport(struct net_bridge_port *p, struct nlattr *tb[], + struct netlink_ext_ack *extack) { unsigned long old_flags, changed_mask; bool br_vlan_tunnel_old; @@ -896,7 +897,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[]) changed_mask = old_flags ^ p->flags; - err = br_switchdev_set_port_flag(p, p->flags, changed_mask); + err = br_switchdev_set_port_flag(p, p->flags, changed_mask, extack); if (err) { p->flags = old_flags; goto out; @@ -1015,7 +1016,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags, if (err) return err; - err = br_setport(p, tb); + err = br_setport(p, tb, extack); } else { /* Binary compatibility with old RSTP */ if (nla_len(protinfo) < sizeof(u8)) @@ -1105,7 +1106,7 @@ static int br_port_slave_changelink(struct net_device *brdev, if (!data) return 0; - return br_setport(br_port_get_rtnl(dev), data); + return br_setport(br_port_get_rtnl(dev), data, extack); } static int br_port_fill_slave_info(struct sk_buff *skb, diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index d242ba668e47..a1639d41188b 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -1575,7 +1575,8 @@ bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, const struct sk_buff *skb); int br_switchdev_set_port_flag(struct net_bridge_port *p, unsigned long flags, - unsigned long mask); + unsigned long mask, + struct netlink_ext_ack *extack); void br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type); int br_switchdev_port_vlan_add(struct net_device *dev, u16 vid, u16 flags, @@ -1605,7 +1606,8 @@ static inline bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, static inline int br_switchdev_set_port_flag(struct net_bridge_port *p, unsigned long flags, - unsigned long mask) + unsigned long mask, + struct netlink_ext_ack *extack) { return 0; } diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index c004ade25ac0..ac8dead86bf2 100644 --- a/net/bridge/br_switchdev.c +++ b/net/bridge/br_switchdev.c @@ -60,7 +60,8 @@ bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, int br_switchdev_set_port_flag(struct net_bridge_port *p, unsigned long flags, - unsigned long mask) + unsigned long mask, + struct netlink_ext_ack *extack) { struct switchdev_attr attr = { .orig_dev = p->dev, @@ -80,14 +81,15 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p, /* We run from atomic context here */ err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev, - &info.info, NULL); + &info.info, extack); err = notifier_to_errno(err); if (err == -EOPNOTSUPP) return 0; if (err) { - br_warn(p->br, "bridge flag offload is not supported %u(%s)\n", - (unsigned int)p->port_no, p->dev->name); + if (extack && !extack->_msg) + NL_SET_ERR_MSG_MOD(extack, + "bridge flag offload is not supported"); return -EOPNOTSUPP; } @@ -97,8 +99,7 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p, err = switchdev_port_attr_set(p->dev, &attr); if (err) { - br_warn(p->br, "error setting offload flag on port %u(%s)\n", - (unsigned int)p->port_no, p->dev->name); + NL_SET_ERR_MSG_MOD(extack, "error setting offload flag on port"); return err; } diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c index 5aea9427ffe1..72e92376eef1 100644 --- a/net/bridge/br_sysfs_if.c +++ b/net/bridge/br_sysfs_if.c @@ -59,6 +59,7 @@ static BRPORT_ATTR(_name, 0644, \ static int store_flag(struct net_bridge_port *p, unsigned long v, unsigned long mask) { + struct netlink_ext_ack extack = {0}; unsigned long flags = p->flags; int err; @@ -68,9 +69,11 @@ static int store_flag(struct net_bridge_port *p, unsigned long v, flags &= ~mask; if (flags != p->flags) { - err = br_switchdev_set_port_flag(p, flags, mask); - if (err) + err = br_switchdev_set_port_flag(p, flags, mask, &extack); + if (err) { + netdev_err(p->dev, "%s\n", extack._msg); return err; + } p->flags = flags; br_port_flags_change(p, mask); -- 2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: Vladimir Oltean <olteanv@gmail.com> To: Jakub Kicinski <kuba@kernel.org>, "David S. Miller" <davem@davemloft.net> Cc: Ivan Vecera <ivecera@redhat.com>, Andrew Lunn <andrew@lunn.ch>, Alexandre Belloni <alexandre.belloni@bootlin.com>, Florian Fainelli <f.fainelli@gmail.com>, Jiri Pirko <jiri@resnulli.us>, Vadym Kochan <vkochan@marvell.com>, netdev@vger.kernel.org, bridge@lists.linux-foundation.org, Ioana Ciornei <ioana.ciornei@nxp.com>, linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com, Taras Chornyi <tchornyi@marvell.com>, Ido Schimmel <idosch@idosch.org>, Claudiu Manoil <claudiu.manoil@nxp.com>, Grygorii Strashko <grygorii.strashko@ti.com>, Nikolay Aleksandrov <nikolay@nvidia.com>, Roopa Prabhu <roopa@nvidia.com>, linux-omap@vger.kernel.org, Vivien Didelot <vivien.didelot@gmail.com> Subject: [Bridge] [PATCH v3 net-next 03/11] net: bridge: don't print in br_switchdev_set_port_flag Date: Wed, 10 Feb 2021 11:14:37 +0200 [thread overview] Message-ID: <20210210091445.741269-4-olteanv@gmail.com> (raw) In-Reply-To: <20210210091445.741269-1-olteanv@gmail.com> From: Vladimir Oltean <vladimir.oltean@nxp.com> Currently br_switchdev_set_port_flag has two options for error handling and neither is good: - The driver returns -EOPNOTSUPP in PRE_BRIDGE_FLAGS if it doesn't support offloading that flag, and this gets silently ignored and converted to an errno of 0. Nobody does this. - The driver returns some other error code, like -EINVAL, in PRE_BRIDGE_FLAGS, and br_switchdev_set_port_flag shouts loudly. The problem is that we'd like to offload some port flags during bridge join and leave, but also not have the bridge shout at us if those fail. But on the other hand we'd like the user to know that we can't offload something when they set that through netlink. And since we can't have the driver return -EOPNOTSUPP or -EINVAL depending on whether it's called by the user or internally by the bridge, let's just add an extack argument to br_switchdev_set_port_flag and propagate it to its callers. Then, when we need offloading to really fail silently, this can simply be passed a NULL argument. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- Changes in v3: - Deal with the br_switchdev_set_port_flag call from sysfs too. Changes in v2: - br_set_port_flag now returns void, so no extack there. - don't overwrite extack in br_switchdev_set_port_flag if already populated. net/bridge/br_netlink.c | 9 +++++---- net/bridge/br_private.h | 6 ++++-- net/bridge/br_switchdev.c | 13 +++++++------ net/bridge/br_sysfs_if.c | 7 +++++-- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 4e64775bd8fb..b7731614c036 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -866,7 +866,8 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[], } /* Process bridge protocol info on port */ -static int br_setport(struct net_bridge_port *p, struct nlattr *tb[]) +static int br_setport(struct net_bridge_port *p, struct nlattr *tb[], + struct netlink_ext_ack *extack) { unsigned long old_flags, changed_mask; bool br_vlan_tunnel_old; @@ -896,7 +897,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[]) changed_mask = old_flags ^ p->flags; - err = br_switchdev_set_port_flag(p, p->flags, changed_mask); + err = br_switchdev_set_port_flag(p, p->flags, changed_mask, extack); if (err) { p->flags = old_flags; goto out; @@ -1015,7 +1016,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags, if (err) return err; - err = br_setport(p, tb); + err = br_setport(p, tb, extack); } else { /* Binary compatibility with old RSTP */ if (nla_len(protinfo) < sizeof(u8)) @@ -1105,7 +1106,7 @@ static int br_port_slave_changelink(struct net_device *brdev, if (!data) return 0; - return br_setport(br_port_get_rtnl(dev), data); + return br_setport(br_port_get_rtnl(dev), data, extack); } static int br_port_fill_slave_info(struct sk_buff *skb, diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index d242ba668e47..a1639d41188b 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -1575,7 +1575,8 @@ bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, const struct sk_buff *skb); int br_switchdev_set_port_flag(struct net_bridge_port *p, unsigned long flags, - unsigned long mask); + unsigned long mask, + struct netlink_ext_ack *extack); void br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type); int br_switchdev_port_vlan_add(struct net_device *dev, u16 vid, u16 flags, @@ -1605,7 +1606,8 @@ static inline bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, static inline int br_switchdev_set_port_flag(struct net_bridge_port *p, unsigned long flags, - unsigned long mask) + unsigned long mask, + struct netlink_ext_ack *extack) { return 0; } diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index c004ade25ac0..ac8dead86bf2 100644 --- a/net/bridge/br_switchdev.c +++ b/net/bridge/br_switchdev.c @@ -60,7 +60,8 @@ bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, int br_switchdev_set_port_flag(struct net_bridge_port *p, unsigned long flags, - unsigned long mask) + unsigned long mask, + struct netlink_ext_ack *extack) { struct switchdev_attr attr = { .orig_dev = p->dev, @@ -80,14 +81,15 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p, /* We run from atomic context here */ err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev, - &info.info, NULL); + &info.info, extack); err = notifier_to_errno(err); if (err == -EOPNOTSUPP) return 0; if (err) { - br_warn(p->br, "bridge flag offload is not supported %u(%s)\n", - (unsigned int)p->port_no, p->dev->name); + if (extack && !extack->_msg) + NL_SET_ERR_MSG_MOD(extack, + "bridge flag offload is not supported"); return -EOPNOTSUPP; } @@ -97,8 +99,7 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p, err = switchdev_port_attr_set(p->dev, &attr); if (err) { - br_warn(p->br, "error setting offload flag on port %u(%s)\n", - (unsigned int)p->port_no, p->dev->name); + NL_SET_ERR_MSG_MOD(extack, "error setting offload flag on port"); return err; } diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c index 5aea9427ffe1..72e92376eef1 100644 --- a/net/bridge/br_sysfs_if.c +++ b/net/bridge/br_sysfs_if.c @@ -59,6 +59,7 @@ static BRPORT_ATTR(_name, 0644, \ static int store_flag(struct net_bridge_port *p, unsigned long v, unsigned long mask) { + struct netlink_ext_ack extack = {0}; unsigned long flags = p->flags; int err; @@ -68,9 +69,11 @@ static int store_flag(struct net_bridge_port *p, unsigned long v, flags &= ~mask; if (flags != p->flags) { - err = br_switchdev_set_port_flag(p, flags, mask); - if (err) + err = br_switchdev_set_port_flag(p, flags, mask, &extack); + if (err) { + netdev_err(p->dev, "%s\n", extack._msg); return err; + } p->flags = flags; br_port_flags_change(p, mask); -- 2.25.1
next prev parent reply other threads:[~2021-02-10 9:26 UTC|newest] Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-10 9:14 [PATCH v3 net-next 00/11] Cleanup in brport flags switchdev offload for DSA Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-10 9:14 ` [PATCH v3 net-next 01/11] net: switchdev: propagate extack to port attributes Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-11 4:12 ` Florian Fainelli 2021-02-11 4:12 ` [Bridge] " Florian Fainelli 2021-02-10 9:14 ` [PATCH v3 net-next 02/11] net: bridge: offload all port flags at once in br_setport Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-10 9:14 ` Vladimir Oltean [this message] 2021-02-10 9:14 ` [Bridge] [PATCH v3 net-next 03/11] net: bridge: don't print in br_switchdev_set_port_flag Vladimir Oltean 2021-02-10 9:14 ` [PATCH v3 net-next 04/11] net: dsa: configure proper brport flags when ports leave the bridge Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-11 4:16 ` Florian Fainelli 2021-02-11 4:16 ` [Bridge] " Florian Fainelli 2021-02-10 9:14 ` [PATCH v3 net-next 05/11] net: squash switchdev attributes PRE_BRIDGE_FLAGS and BRIDGE_FLAGS Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-10 9:14 ` [PATCH v3 net-next 06/11] net: dsa: kill .port_egress_floods overengineering Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-11 4:18 ` Florian Fainelli 2021-02-11 4:18 ` [Bridge] " Florian Fainelli 2021-02-10 9:14 ` [PATCH v3 net-next 07/11] net: prep switchdev drivers for concurrent SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-10 10:12 ` Ido Schimmel 2021-02-10 10:12 ` [Bridge] " Ido Schimmel 2021-02-10 10:23 ` Vladimir Oltean 2021-02-10 10:23 ` [Bridge] " Vladimir Oltean 2021-02-10 23:34 ` David Miller 2021-02-10 23:34 ` [Bridge] " David Miller 2021-02-10 9:14 ` [PATCH v3 net-next 08/11] net: bridge: put SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS on the blocking call chain Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-10 10:14 ` Nikolay Aleksandrov 2021-02-10 10:14 ` [Bridge] " Nikolay Aleksandrov 2021-02-10 9:14 ` [PATCH v3 net-next 09/11] net: mscc: ocelot: use separate flooding PGID for broadcast Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-11 4:19 ` Florian Fainelli 2021-02-11 4:19 ` [Bridge] " Florian Fainelli 2021-02-10 9:14 ` [PATCH v3 net-next 10/11] net: mscc: ocelot: offload bridge port flags to device Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-11 4:20 ` Florian Fainelli 2021-02-11 4:20 ` [Bridge] " Florian Fainelli 2021-02-10 9:14 ` [PATCH v3 net-next 11/11] net: dsa: sja1105: " Vladimir Oltean 2021-02-10 9:14 ` [Bridge] " Vladimir Oltean 2021-02-10 10:31 ` [PATCH v3 net-next 00/11] Cleanup in brport flags switchdev offload for DSA Nikolay Aleksandrov 2021-02-10 10:31 ` [Bridge] " Nikolay Aleksandrov 2021-02-10 10:45 ` Vladimir Oltean 2021-02-10 10:45 ` [Bridge] " Vladimir Oltean 2021-02-10 10:52 ` Nikolay Aleksandrov 2021-02-10 10:52 ` [Bridge] " Nikolay Aleksandrov 2021-02-10 11:01 ` Vladimir Oltean 2021-02-10 11:01 ` [Bridge] " Vladimir Oltean 2021-02-10 11:05 ` Nikolay Aleksandrov 2021-02-10 11:05 ` [Bridge] " Nikolay Aleksandrov 2021-02-10 12:01 ` Vladimir Oltean 2021-02-10 12:01 ` [Bridge] " Vladimir Oltean 2021-02-10 12:10 ` Nikolay Aleksandrov 2021-02-10 12:10 ` [Bridge] " Nikolay Aleksandrov 2021-02-10 12:21 ` Ido Schimmel 2021-02-10 12:21 ` [Bridge] " Ido Schimmel 2021-02-10 12:29 ` Vladimir Oltean 2021-02-10 12:29 ` [Bridge] " Vladimir Oltean 2021-02-10 12:38 ` Ido Schimmel 2021-02-10 12:38 ` [Bridge] " Ido Schimmel 2021-02-10 12:55 ` Vladimir Oltean 2021-02-10 12:55 ` [Bridge] " Vladimir Oltean 2021-02-10 12:59 ` Ido Schimmel 2021-02-10 12:59 ` [Bridge] " Ido Schimmel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210210091445.741269-4-olteanv@gmail.com \ --to=olteanv@gmail.com \ --cc=UNGLinuxDriver@microchip.com \ --cc=alexandre.belloni@bootlin.com \ --cc=andrew@lunn.ch \ --cc=bridge@lists.linux-foundation.org \ --cc=claudiu.manoil@nxp.com \ --cc=davem@davemloft.net \ --cc=f.fainelli@gmail.com \ --cc=grygorii.strashko@ti.com \ --cc=idosch@idosch.org \ --cc=ioana.ciornei@nxp.com \ --cc=ivecera@redhat.com \ --cc=jiri@resnulli.us \ --cc=kuba@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=nikolay@nvidia.com \ --cc=roopa@nvidia.com \ --cc=tchornyi@marvell.com \ --cc=vivien.didelot@gmail.com \ --cc=vkochan@marvell.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.