From: Ido Schimmel <idosch@idosch.org> To: Vladimir Oltean <olteanv@gmail.com> Cc: Tobias Waldekranz <tobias@waldekranz.com>, davem@davemloft.net, kuba@kernel.org, Nikolay Aleksandrov <razor@blackwall.org>, Andrew Lunn <andrew@lunn.ch>, Vivien Didelot <vivien.didelot@gmail.com>, Florian Fainelli <f.fainelli@gmail.com>, Jiri Pirko <jiri@resnulli.us>, Ivan Vecera <ivecera@redhat.com>, Roopa Prabhu <roopa@nvidia.com>, Russell King <linux@armlinux.org.uk>, Petr Machata <petrm@nvidia.com>, Ido Schimmel <idosch@nvidia.com>, Matt Johnston <matt@codeconstruct.com.au>, Cooper Lees <me@cooperlees.com>, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, bridge@lists.linux-foundation.org Subject: Re: [PATCH v5 net-next 01/15] net: bridge: mst: Multiple Spanning Tree (MST) mode Date: Mon, 9 Jan 2023 13:43:46 +0200 [thread overview] Message-ID: <Y7v98s8lC1WUvsSO@shredder> (raw) In-Reply-To: <20230109100236.euq7iaaorqxrun7u@skbuf> On Mon, Jan 09, 2023 at 12:02:36PM +0200, Vladimir Oltean wrote: > On Mon, Jan 09, 2023 at 10:05:53AM +0200, Ido Schimmel wrote: > > > + if (on) > > > + static_branch_enable(&br_mst_used); > > > + else > > > + static_branch_disable(&br_mst_used); > > > > Hi, > > > > I'm not actually using MST, but I ran into this code and was wondering > > if the static key usage is correct. The static key is global (not > > per-bridge), so what happens when two bridges have MST enabled and then > > it is disabled on one? I believe it would be disabled for both. If so, > > maybe use static_branch_inc() / static_branch_dec() instead? > > Sounds about right. FWIW, br_switchdev_tx_fwd_offload does use > static_branch_inc() / static_branch_dec(). OK, thanks for confirming. Will send a patch later this week if Tobias won't take care of it by then. First patch will probably be [1] to make sure we dump the correct MST state to user space. It will also make it easier to show the problem and validate the fix. [1] diff --git a/net/bridge/br.c b/net/bridge/br.c index 4f5098d33a46..f02a1ad589de 100644 --- a/net/bridge/br.c +++ b/net/bridge/br.c @@ -286,7 +286,7 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt) case BR_BOOLOPT_MCAST_VLAN_SNOOPING: return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED); case BR_BOOLOPT_MST_ENABLE: - return br_opt_get(br, BROPT_MST_ENABLED); + return br_mst_is_enabled(br); default: /* shouldn't be called with unsupported options */ WARN_ON(1); diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 75aff9bbf17e..7f0475f62d45 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -1827,7 +1827,7 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow) /* br_mst.c */ #ifdef CONFIG_BRIDGE_VLAN_FILTERING DECLARE_STATIC_KEY_FALSE(br_mst_used); -static inline bool br_mst_is_enabled(struct net_bridge *br) +static inline bool br_mst_is_enabled(const struct net_bridge *br) { return static_branch_unlikely(&br_mst_used) && br_opt_get(br, BROPT_MST_ENABLED); @@ -1845,7 +1845,7 @@ int br_mst_fill_info(struct sk_buff *skb, int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr, struct netlink_ext_ack *extack); #else -static inline bool br_mst_is_enabled(struct net_bridge *br) +static inline bool br_mst_is_enabled(const struct net_bridge *br) { return false; }
WARNING: multiple messages have this Message-ID (diff)
From: Ido Schimmel <idosch@idosch.org> To: Vladimir Oltean <olteanv@gmail.com> Cc: Ivan Vecera <ivecera@redhat.com>, Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>, Jiri Pirko <jiri@resnulli.us>, Petr Machata <petrm@nvidia.com>, Nikolay Aleksandrov <razor@blackwall.org>, bridge@lists.linux-foundation.org, Russell King <linux@armlinux.org.uk>, Vivien Didelot <vivien.didelot@gmail.com>, Ido Schimmel <idosch@nvidia.com>, netdev@vger.kernel.org, Cooper Lees <me@cooperlees.com>, Roopa Prabhu <roopa@nvidia.com>, kuba@kernel.org, Matt Johnston <matt@codeconstruct.com.au>, davem@davemloft.net, linux-kernel@vger.kernel.org, Tobias Waldekranz <tobias@waldekranz.com> Subject: Re: [Bridge] [PATCH v5 net-next 01/15] net: bridge: mst: Multiple Spanning Tree (MST) mode Date: Mon, 9 Jan 2023 13:43:46 +0200 [thread overview] Message-ID: <Y7v98s8lC1WUvsSO@shredder> (raw) In-Reply-To: <20230109100236.euq7iaaorqxrun7u@skbuf> On Mon, Jan 09, 2023 at 12:02:36PM +0200, Vladimir Oltean wrote: > On Mon, Jan 09, 2023 at 10:05:53AM +0200, Ido Schimmel wrote: > > > + if (on) > > > + static_branch_enable(&br_mst_used); > > > + else > > > + static_branch_disable(&br_mst_used); > > > > Hi, > > > > I'm not actually using MST, but I ran into this code and was wondering > > if the static key usage is correct. The static key is global (not > > per-bridge), so what happens when two bridges have MST enabled and then > > it is disabled on one? I believe it would be disabled for both. If so, > > maybe use static_branch_inc() / static_branch_dec() instead? > > Sounds about right. FWIW, br_switchdev_tx_fwd_offload does use > static_branch_inc() / static_branch_dec(). OK, thanks for confirming. Will send a patch later this week if Tobias won't take care of it by then. First patch will probably be [1] to make sure we dump the correct MST state to user space. It will also make it easier to show the problem and validate the fix. [1] diff --git a/net/bridge/br.c b/net/bridge/br.c index 4f5098d33a46..f02a1ad589de 100644 --- a/net/bridge/br.c +++ b/net/bridge/br.c @@ -286,7 +286,7 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt) case BR_BOOLOPT_MCAST_VLAN_SNOOPING: return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED); case BR_BOOLOPT_MST_ENABLE: - return br_opt_get(br, BROPT_MST_ENABLED); + return br_mst_is_enabled(br); default: /* shouldn't be called with unsupported options */ WARN_ON(1); diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 75aff9bbf17e..7f0475f62d45 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -1827,7 +1827,7 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow) /* br_mst.c */ #ifdef CONFIG_BRIDGE_VLAN_FILTERING DECLARE_STATIC_KEY_FALSE(br_mst_used); -static inline bool br_mst_is_enabled(struct net_bridge *br) +static inline bool br_mst_is_enabled(const struct net_bridge *br) { return static_branch_unlikely(&br_mst_used) && br_opt_get(br, BROPT_MST_ENABLED); @@ -1845,7 +1845,7 @@ int br_mst_fill_info(struct sk_buff *skb, int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr, struct netlink_ext_ack *extack); #else -static inline bool br_mst_is_enabled(struct net_bridge *br) +static inline bool br_mst_is_enabled(const struct net_bridge *br) { return false; }
next prev parent reply other threads:[~2023-01-09 11:44 UTC|newest] Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-16 15:08 [PATCH v5 net-next 00/15] net: bridge: Multiple Spanning Trees Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-16 15:08 ` [PATCH v5 net-next 01/15] net: bridge: mst: Multiple Spanning Tree (MST) mode Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2023-01-09 8:05 ` Ido Schimmel 2023-01-09 8:05 ` [Bridge] " Ido Schimmel 2023-01-09 10:02 ` Vladimir Oltean 2023-01-09 10:02 ` [Bridge] " Vladimir Oltean 2023-01-09 11:43 ` Ido Schimmel [this message] 2023-01-09 11:43 ` Ido Schimmel 2023-01-09 11:51 ` Nikolay Aleksandrov 2023-01-09 11:51 ` [Bridge] " Nikolay Aleksandrov 2023-01-09 11:56 ` Vladimir Oltean 2023-01-09 11:56 ` [Bridge] " Vladimir Oltean 2023-01-09 12:20 ` Ido Schimmel 2023-01-09 12:20 ` [Bridge] " Ido Schimmel 2023-01-09 12:29 ` Vladimir Oltean 2023-01-09 12:29 ` [Bridge] " Vladimir Oltean 2022-03-16 15:08 ` [PATCH v5 net-next 02/15] net: bridge: mst: Allow changing a VLAN's MSTI Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-16 15:08 ` [PATCH v5 net-next 03/15] net: bridge: mst: Support setting and reporting MST port states Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-17 8:55 ` Nikolay Aleksandrov 2022-03-17 8:55 ` [Bridge] " Nikolay Aleksandrov 2022-03-16 15:08 ` [PATCH v5 net-next 04/15] net: bridge: mst: Notify switchdev drivers of MST mode changes Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-17 8:56 ` Nikolay Aleksandrov 2022-03-17 8:56 ` [Bridge] " Nikolay Aleksandrov 2022-03-16 15:08 ` [PATCH v5 net-next 05/15] net: bridge: mst: Notify switchdev drivers of VLAN MSTI migrations Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-17 8:57 ` Nikolay Aleksandrov 2022-03-17 8:57 ` [Bridge] " Nikolay Aleksandrov 2022-03-16 15:08 ` [PATCH v5 net-next 06/15] net: bridge: mst: Notify switchdev drivers of MST state changes Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-17 8:57 ` Nikolay Aleksandrov 2022-03-17 8:57 ` [Bridge] " Nikolay Aleksandrov 2022-03-16 15:08 ` [PATCH v5 net-next 07/15] net: bridge: mst: Add helper to map an MSTI to a VID set Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-17 0:43 ` Vladimir Oltean 2022-03-17 0:43 ` [Bridge] " Vladimir Oltean 2022-03-17 9:01 ` Nikolay Aleksandrov 2022-03-17 9:01 ` [Bridge] " Nikolay Aleksandrov 2022-03-16 15:08 ` [PATCH v5 net-next 08/15] net: bridge: mst: Add helper to check if MST is enabled Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-17 0:42 ` Vladimir Oltean 2022-03-17 0:42 ` [Bridge] " Vladimir Oltean 2022-03-17 9:01 ` Nikolay Aleksandrov 2022-03-17 9:01 ` [Bridge] " Nikolay Aleksandrov 2022-03-16 15:08 ` [PATCH v5 net-next 09/15] net: bridge: mst: Add helper to query a port's MST state Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-17 0:41 ` Vladimir Oltean 2022-03-17 0:41 ` [Bridge] " Vladimir Oltean 2022-03-17 9:01 ` Nikolay Aleksandrov 2022-03-17 9:01 ` [Bridge] " Nikolay Aleksandrov 2022-03-16 15:08 ` [PATCH v5 net-next 10/15] net: dsa: Validate hardware support for MST Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-16 15:59 ` Vladimir Oltean 2022-03-16 15:59 ` [Bridge] " Vladimir Oltean 2022-03-16 15:08 ` [PATCH v5 net-next 11/15] net: dsa: Pass VLAN MSTI migration notifications to driver Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-16 15:08 ` [PATCH v5 net-next 12/15] net: dsa: Handle MST state changes Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-16 15:56 ` Vladimir Oltean 2022-03-16 15:56 ` [Bridge] " Vladimir Oltean 2022-03-16 15:08 ` [PATCH v5 net-next 13/15] net: dsa: mv88e6xxx: Disentangle STU from VTU Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-16 15:08 ` [PATCH v5 net-next 14/15] net: dsa: mv88e6xxx: Export STU as devlink region Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-16 15:08 ` [PATCH v5 net-next 15/15] net: dsa: mv88e6xxx: MST Offloading Tobias Waldekranz 2022-03-16 15:08 ` [Bridge] " Tobias Waldekranz 2022-03-17 9:00 ` [PATCH v5 net-next 00/15] net: bridge: Multiple Spanning Trees Nikolay Aleksandrov 2022-03-17 9:00 ` [Bridge] " Nikolay Aleksandrov 2022-03-17 9:50 ` Tobias Waldekranz 2022-03-17 9:50 ` [Bridge] " Tobias Waldekranz 2022-03-17 9:56 ` Nikolay Aleksandrov 2022-03-17 9:56 ` [Bridge] " Nikolay Aleksandrov 2022-03-18 0:20 ` patchwork-bot+netdevbpf 2022-03-18 0:20 ` [Bridge] " patchwork-bot+netdevbpf
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=Y7v98s8lC1WUvsSO@shredder \ --to=idosch@idosch.org \ --cc=andrew@lunn.ch \ --cc=bridge@lists.linux-foundation.org \ --cc=davem@davemloft.net \ --cc=f.fainelli@gmail.com \ --cc=idosch@nvidia.com \ --cc=ivecera@redhat.com \ --cc=jiri@resnulli.us \ --cc=kuba@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=matt@codeconstruct.com.au \ --cc=me@cooperlees.com \ --cc=netdev@vger.kernel.org \ --cc=olteanv@gmail.com \ --cc=petrm@nvidia.com \ --cc=razor@blackwall.org \ --cc=roopa@nvidia.com \ --cc=tobias@waldekranz.com \ --cc=vivien.didelot@gmail.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.