From patchwork Sat Mar 5 09:36:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Bj=C3=B8rn_Mork?= X-Patchwork-Id: 1601511 X-Patchwork-Delegate: stijn@linux-ipv6.be Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=zrLPO5zh; dkim=fail reason="signature verification failed" (1024-bit key; secure) header.d=mork.no header.i=@mork.no header.a=rsa-sha256 header.s=b header.b=BMOzRfhw; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4K9fsL0bM6z9sGF for ; Sat, 5 Mar 2022 20:42:45 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=WFxSqPu4tV4Vy8IqVaAsS5wPOKasfgV6kFhbAHOkaSg=; b=zrLPO5zhhE42Yr KLb1P/vm9gyFnDiNWFk/6GMtNiRT/vP3i+kEm1Tdn1xns9Ws9gdla+Jt0EErIN1HG0puVoHnZ5y4x FIhEnmnn9Hi+OOJyFNkEgm1S5jh04TzQkDCG57nr72s8ENw3DU/shaAXp2esDXV+qkWMvGbwC91VU 402UZOyHvK/uiif5qC8m88er/zx0cJ+1sytUCmlTG51aOqnuCMdeZSDdI9MQLnWvUhLjzCsXGHQLJ PNXpaGvbaRNJzp7pitfKkoeVR90wEMfvVkU71ikLVbeJDb/Sj2DsBhqrvR3JF9oU0dxS4Zl6QgaGS K99dhMHTDXCb/+deWK1Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nQQq3-00D3ks-FZ; Sat, 05 Mar 2022 09:36:35 +0000 Received: from louie.mork.no ([2001:41c8:51:8a:feff:ff:fe00:e5]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nQQpw-00D3kP-T6 for openwrt-devel@lists.openwrt.org; Sat, 05 Mar 2022 09:36:31 +0000 Received: from canardo.dyn.mork.no ([IPv6:2a01:799:c9f:8600:0:0:0:1]) (authenticated bits=0) by louie.mork.no (8.15.2/8.15.2) with ESMTPSA id 2259aG7a1604175 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=OK); Sat, 5 Mar 2022 09:36:18 GMT Received: from canardo.dyn.mork.no (ip6-localhost [IPv6:0:0:0:0:0:0:0:1]) by canardo.dyn.mork.no (8.15.2/8.15.2) with ESMTPS id 2259aGjR499667 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=FAIL); Sat, 5 Mar 2022 10:36:16 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mork.no; s=b; t=1646472976; bh=M08nT7hqCmlpUhQjJV8a+msMVWxLCGcnq5ulBgPJsRM=; h=From:To:Cc:Subject:Date:Message-Id:From; b=BMOzRfhwrhBeGTTzT1PZLakh7teRIGNxiNoKwKBBbHImIqr3BMntPgoC+a8xj/P9h SCRbPzc9m9gOf8ae2T52J9KOh3pfbatDz81lWufrF03XVCQb2iF/RPDsUa7R0dHFb2 MOtVADW5utqVwhzzMEmZyq9iJSJ1lPcmIC4z85+I= Received: (from bjorn@localhost) by canardo.dyn.mork.no (8.15.2/8.15.2/Submit) id 2259aGej499666; Sat, 5 Mar 2022 10:36:16 +0100 From: =?utf-8?q?Bj=C3=B8rn_Mork?= To: openwrt-devel@lists.openwrt.org Cc: openwrt/openwrt , Birger Koblitz , =?utf-8?q?Bj=C3=B8rn_Mork?= Subject: [PATCH] realtek: net: dsa: configure better brport flags when ports leave the bridge Date: Sat, 5 Mar 2022 10:36:07 +0100 Message-Id: <20220305093607.499617-1-bjorn@mork.no> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT shortcircuit=ham autolearn=disabled version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on canardo.dyn.mork.no X-Virus-Scanned: clamav-milter 0.103.3 at canardo X-Virus-Status: Clean X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220305_013629_435647_348BC442 X-CRM114-Status: GOOD ( 32.69 ) X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Ensures that the DSA driver set exactly the same default flags as the bridge when a port joins or leaves. Without this we end up with a confusing flag mismatch, where DSA and bridge ports use differen [...] Content analysis details: (-0.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 SPF_NONE SPF: sender does not publish an SPF Record -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org Ensures that the DSA driver set exactly the same default flags as the bridge when a port joins or leaves. Without this we end up with a confusing flag mismatch, where DSA and bridge ports use different sets of flags. This is critical as the "learning" mismatch will be harmful to the network, causing all traffic to be flooded on all ports. The original commit was buggy, trying to set the flags one-by-one in a loop. This was not supported by the API and the end result was that all but the last flag were cleared. This bug was implicitly fixed upstream by commit e18f4c18ab5b ("net: switchdev: pass flags and mask to both {PRE_,}BRIDGE_FLAGS attributes"). This is a minimum temporary stop meaure fix for the critical lack of "learning" only. The major API change associated with a full v5.12+ backport is neither required nor wanted. A simpler fix, moving the call to dsa_port_bridge_flags() out of the loop, has therefore been merged into this modified backport. Fixes: afa3ab54c03d ("realtek: Backport bridge configuration for DSA") Signed-off-by: Bjørn Mork Acked-by: Daniel Golle --- This is a critical bugfix for OpenWrt 22.0x. Commit afa3ab54c03d is a partial backport of DSA/switchdev API changes between v5.10 and v5.12. It is incomplete and not linked to the original commits. It will be irrelevant with the next major kernel update, where all this is properly fixed - syncronizing the bridge and DSA flags. Backporting all these API changes is not considered an option for v5.10. This is the minimum fix required to unbreak the realtek target without reverting commit afa3ab54c03d and all the mess that would cause. I have tested and verified this fix on a Netgear GS108Tv3 by simply pinging betweeen clients on two ports and observing that no unexpected packets leak out on the CPU port. Without the fix, all paackets are flooded also to the CPU port - causing issues related to the limited bandwidth of the ethernet driver in any real network. Bjørn ...e-better-brport-flags-when-ports-lea.patch | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 target/linux/realtek/patches-5.10/713-v5.12-net-dsa-configure-better-brport-flags-when-ports-lea.patch diff --git a/target/linux/realtek/patches-5.10/713-v5.12-net-dsa-configure-better-brport-flags-when-ports-lea.patch b/target/linux/realtek/patches-5.10/713-v5.12-net-dsa-configure-better-brport-flags-when-ports-lea.patch new file mode 100644 index 000000000000..a3bfec59ab7d --- /dev/null +++ b/target/linux/realtek/patches-5.10/713-v5.12-net-dsa-configure-better-brport-flags-when-ports-lea.patch @@ -0,0 +1,148 @@ +From: Vladimir Oltean +Date: Fri, 12 Feb 2021 17:15:54 +0200 +Subject: [PATCH] net: dsa: configure better brport flags when ports leave the + bridge + +Bugfixed version of upstream commit 5e38c15856e9 ("net: dsa: configure +better brport flags when ports leave the bridge") + +For a DSA switch port operating in standalone mode, address learning +doesn't make much sense since that is a bridge function. In fact, +address learning even breaks setups such as this one: + + +---------------------------------------------+ + | | + | +-------------------+ | + | | br0 | send receive | + | +--------+-+--------+ +--------+ +--------+ | + | | | | | | | | | | + | | swp0 | | swp1 | | swp2 | | swp3 | | + | | | | | | | | | | + +-+--------+-+--------+-+--------+-+--------+-+ + | ^ | ^ + | | | | + | +-----------+ | + | | + +--------------------------------+ + +because if the switch has a single FDB (can offload a single bridge) +then source address learning on swp3 can "steal" the source MAC address +of swp2 from br0's FDB, because learning frames coming from swp2 will be +done twice: first on the swp1 ingress port, second on the swp3 ingress +port. So the hardware FDB will become out of sync with the software +bridge, and when swp2 tries to send one more packet towards swp1, the +ASIC will attempt to short-circuit the forwarding path and send it +directly to swp3 (since that's the last port it learned that address on), +which it obviously can't, because swp3 operates in standalone mode. + +So DSA drivers operating in standalone mode should still configure a +list of bridge port flags even when they are standalone. Currently DSA +attempts to call dsa_port_bridge_flags with 0, which disables egress +flooding of unknown unicast and multicast, something which doesn't make +much sense. For the switches that implement .port_egress_floods - b53 +and mv88e6xxx, it probably doesn't matter too much either, since they +can possibly inject traffic from the CPU into a standalone port, +regardless of MAC DA, even if egress flooding is turned off for that +port, but certainly not all DSA switches can do that - sja1105, for +example, can't. So it makes sense to use a better common default there, +such as "flood everything". + +It should also be noted that what DSA calls "dsa_port_bridge_flags()" +is a degenerate name for just calling .port_egress_floods(), since +nothing else is implemented - not learning, in particular. But disabling +address learning, something that this driver is also coding up for, will +be supported by individual drivers once .port_egress_floods is replaced +with a more generic .port_bridge_flags. + +Previous attempts to code up this logic have been in the common bridge +layer, but as pointed out by Ido Schimmel, there are corner cases that +are missed when doing that: +https://patchwork.kernel.org/project/netdevbpf/patch/20210209151936.97382-5-olteanv@gmail.com/ + +So, at least for now, let's leave DSA in charge of setting port flags +before and after the bridge join and leave. + +Signed-off-by: Vladimir Oltean +Reviewed-by: Florian Fainelli +Signed-off-by: David S. Miller +[ backport and bugfix: break dsa_port_bridge_flags() out of loop ] +Signed-off-by: Bjørn Mork +--- + net/dsa/port.c | 45 ++++++++++++++++++++++++++++++++++++++------- + 1 file changed, 38 insertions(+), 7 deletions(-) + +--- a/net/dsa/port.c ++++ b/net/dsa/port.c +@@ -134,6 +134,27 @@ void dsa_port_disable(struct dsa_port *d + rtnl_unlock(); + } + ++static void dsa_port_change_brport_flags(struct dsa_port *dp, ++ bool bridge_offload) ++{ ++ unsigned long mask, flags; ++ int flag, err; ++ ++ mask = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD; ++ if (bridge_offload) ++ flags = mask; ++ else ++ flags = mask & ~BR_LEARNING; ++ ++ for_each_set_bit(flag, &mask, 32) { ++ err = dsa_port_pre_bridge_flags(dp, BIT(flag), NULL, NULL); ++ if (err) ++ flags &= ~BIT(flag); ++ } ++ ++ dsa_port_bridge_flags(dp, flags, NULL, NULL); ++} ++ + int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br) + { + struct dsa_notifier_bridge_info info = { +@@ -144,10 +165,10 @@ int dsa_port_bridge_join(struct dsa_port + }; + int err; + +- /* Set the flooding mode before joining the port in the switch */ +- err = dsa_port_bridge_flags(dp, BR_FLOOD | BR_MCAST_FLOOD, NULL, NULL); +- if (err) +- return err; ++ /* Notify the port driver to set its configurable flags in a way that ++ * matches the initial settings of a bridge port. ++ */ ++ dsa_port_change_brport_flags(dp, true); + + /* Here the interface is already bridged. Reflect the current + * configuration so that drivers can program their chips accordingly. +@@ -158,7 +179,7 @@ int dsa_port_bridge_join(struct dsa_port + + /* The bridging is rolled back on error */ + if (err) { +- dsa_port_bridge_flags(dp, 0, NULL, NULL); ++ dsa_port_change_brport_flags(dp, false); + dp->bridge_dev = NULL; + } + +@@ -184,8 +205,18 @@ void dsa_port_bridge_leave(struct dsa_po + if (err) + pr_err("DSA: failed to notify DSA_NOTIFIER_BRIDGE_LEAVE\n"); + +- /* Port is leaving the bridge, disable flooding */ +- dsa_port_bridge_flags(dp, 0, NULL, NULL); ++ /* Configure the port for standalone mode (no address learning, ++ * flood everything). ++ * The bridge only emits SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS events ++ * when the user requests it through netlink or sysfs, but not ++ * automatically at port join or leave, so we need to handle resetting ++ * the brport flags ourselves. But we even prefer it that way, because ++ * otherwise, some setups might never get the notification they need, ++ * for example, when a port leaves a LAG that offloads the bridge, ++ * it becomes standalone, but as far as the bridge is concerned, no ++ * port ever left. ++ */ ++ dsa_port_change_brport_flags(dp, false); + + /* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer, + * so allow it to be in BR_STATE_FORWARDING to be kept functional