From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E314C54EBD for ; Mon, 9 Jan 2023 08:06:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236333AbjAIIGv (ORCPT ); Mon, 9 Jan 2023 03:06:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60142 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236497AbjAIIGb (ORCPT ); Mon, 9 Jan 2023 03:06:31 -0500 Received: from wout4-smtp.messagingengine.com (wout4-smtp.messagingengine.com [64.147.123.20]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 943ABBA4; Mon, 9 Jan 2023 00:06:03 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id CD49832006F5; Mon, 9 Jan 2023 03:05:58 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Mon, 09 Jan 2023 03:06:00 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1673251558; x=1673337958; bh=hObozgfzpCa4NJkSvRz0TIjGKTPe jFUvHPU4Z0aahqk=; b=mzxOVgnYUmn1wqkvAoWZNnNN7BUjMblm39y8OWSvfmWq v3yc/JIaIn8ehELmCz5BW/SbUKx2+l3nl8SWKpCptAqZMqDNQOm3f7oWtNk0fnIf cECuWF/EThU6YipnukTdmxsSz4VKplC3izMcBUceGP/Y+gmYAb0yTWaYYCRDi2eL CBxayCahGbY3xW9Sc9ZabOZ/mg4/DS3OoljyhOOBV6f+mqAvs5BXAtWy8Y2X4Xtc QlKNITAK0OyTqIPwNTH9e8rLr/t+Drpov8608l6NgFpFHyn4X885BThpWUDNMXDU izE8b3OI056CdOZBeSGeMDY5JT72dwD9sWtT/qp7/Q== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrkeehgdduudelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvdenucfhrhhomhepkfguohcu ufgthhhimhhmvghluceoihguohhstghhsehiughoshgthhdrohhrgheqnecuggftrfgrth htvghrnhepvddufeevkeehueegfedtvdevfefgudeifeduieefgfelkeehgeelgeejjeeg gefhnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepih guohhstghhsehiughoshgthhdrohhrgh X-ME-Proxy: Feedback-ID: i494840e7:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 9 Jan 2023 03:05:56 -0500 (EST) Date: Mon, 9 Jan 2023 10:05:53 +0200 From: Ido Schimmel To: Tobias Waldekranz Cc: davem@davemloft.net, kuba@kernel.org, Nikolay Aleksandrov , Andrew Lunn , Vivien Didelot , Florian Fainelli , Vladimir Oltean , Jiri Pirko , Ivan Vecera , Roopa Prabhu , Russell King , Petr Machata , Ido Schimmel , Matt Johnston , Cooper Lees , 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 Message-ID: References: <20220316150857.2442916-1-tobias@waldekranz.com> <20220316150857.2442916-2-tobias@waldekranz.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220316150857.2442916-2-tobias@waldekranz.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 7E63381B7B DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 085F681BB2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1673251558; x=1673337958; bh=hObozgfzpCa4NJkSvRz0TIjGKTPe jFUvHPU4Z0aahqk=; b=mzxOVgnYUmn1wqkvAoWZNnNN7BUjMblm39y8OWSvfmWq v3yc/JIaIn8ehELmCz5BW/SbUKx2+l3nl8SWKpCptAqZMqDNQOm3f7oWtNk0fnIf cECuWF/EThU6YipnukTdmxsSz4VKplC3izMcBUceGP/Y+gmYAb0yTWaYYCRDi2eL CBxayCahGbY3xW9Sc9ZabOZ/mg4/DS3OoljyhOOBV6f+mqAvs5BXAtWy8Y2X4Xtc QlKNITAK0OyTqIPwNTH9e8rLr/t+Drpov8608l6NgFpFHyn4X885BThpWUDNMXDU izE8b3OI056CdOZBeSGeMDY5JT72dwD9sWtT/qp7/Q== Date: Mon, 9 Jan 2023 10:05:53 +0200 From: Ido Schimmel Message-ID: References: <20220316150857.2442916-1-tobias@waldekranz.com> <20220316150857.2442916-2-tobias@waldekranz.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220316150857.2442916-2-tobias@waldekranz.com> Subject: Re: [Bridge] [PATCH v5 net-next 01/15] net: bridge: mst: Multiple Spanning Tree (MST) mode List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tobias Waldekranz Cc: Ivan Vecera , Andrew Lunn , Florian Fainelli , Jiri Pirko , Petr Machata , Nikolay Aleksandrov , bridge@lists.linux-foundation.org, Russell King , Vivien Didelot , Ido Schimmel , netdev@vger.kernel.org, Cooper Lees , Roopa Prabhu , kuba@kernel.org, Matt Johnston , Vladimir Oltean , davem@davemloft.net, linux-kernel@vger.kernel.org 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