From: Ido Schimmel <idosch@idosch.org> To: Tobias Waldekranz <tobias@waldekranz.com> Cc: 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>, Vladimir Oltean <olteanv@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 10:05:53 +0200 [thread overview] Message-ID: <Y7vK4T18pOZ9KAKE@shredder> (raw) In-Reply-To: <20220316150857.2442916-2-tobias@waldekranz.com> On Wed, Mar 16, 2022 at 04:08:43PM +0100, Tobias Waldekranz wrote: > +DEFINE_STATIC_KEY_FALSE(br_mst_used); [...] > +int br_mst_set_enabled(struct net_bridge *br, bool on, > + struct netlink_ext_ack *extack) > +{ > + struct net_bridge_vlan_group *vg; > + struct net_bridge_port *p; > + > + list_for_each_entry(p, &br->port_list, list) { > + vg = nbp_vlan_group(p); > + > + if (!vg->num_vlans) > + continue; > + > + NL_SET_ERR_MSG(extack, > + "MST mode can't be changed while VLANs exist"); > + return -EBUSY; > + } > + > + if (br_opt_get(br, BROPT_MST_ENABLED) == on) > + return 0; > + > + if (on) > + static_branch_enable(&br_mst_used); > + else > + static_branch_disable(&br_mst_used); > + > + br_opt_toggle(br, BROPT_MST_ENABLED, on); > + return 0; > +} 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? Thanks
WARNING: multiple messages have this Message-ID (diff)
From: Ido Schimmel <idosch@idosch.org> To: Tobias Waldekranz <tobias@waldekranz.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>, Vladimir Oltean <olteanv@gmail.com>, davem@davemloft.net, linux-kernel@vger.kernel.org Subject: Re: [Bridge] [PATCH v5 net-next 01/15] net: bridge: mst: Multiple Spanning Tree (MST) mode Date: Mon, 9 Jan 2023 10:05:53 +0200 [thread overview] Message-ID: <Y7vK4T18pOZ9KAKE@shredder> (raw) In-Reply-To: <20220316150857.2442916-2-tobias@waldekranz.com> On Wed, Mar 16, 2022 at 04:08:43PM +0100, Tobias Waldekranz wrote: > +DEFINE_STATIC_KEY_FALSE(br_mst_used); [...] > +int br_mst_set_enabled(struct net_bridge *br, bool on, > + struct netlink_ext_ack *extack) > +{ > + struct net_bridge_vlan_group *vg; > + struct net_bridge_port *p; > + > + list_for_each_entry(p, &br->port_list, list) { > + vg = nbp_vlan_group(p); > + > + if (!vg->num_vlans) > + continue; > + > + NL_SET_ERR_MSG(extack, > + "MST mode can't be changed while VLANs exist"); > + return -EBUSY; > + } > + > + if (br_opt_get(br, BROPT_MST_ENABLED) == on) > + return 0; > + > + if (on) > + static_branch_enable(&br_mst_used); > + else > + static_branch_disable(&br_mst_used); > + > + br_opt_toggle(br, BROPT_MST_ENABLED, on); > + return 0; > +} 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? Thanks
next prev parent reply other threads:[~2023-01-09 8:06 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 [this message] 2023-01-09 8:05 ` Ido Schimmel 2023-01-09 10:02 ` Vladimir Oltean 2023-01-09 10:02 ` [Bridge] " Vladimir Oltean 2023-01-09 11:43 ` Ido Schimmel 2023-01-09 11:43 ` [Bridge] " 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=Y7vK4T18pOZ9KAKE@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.