From patchwork Fri Feb 12 01:05:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 382281 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9826AC4332E for ; Fri, 12 Feb 2021 01:07:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 58B4664E39 for ; Fri, 12 Feb 2021 01:07:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230101AbhBLBGn (ORCPT ); Thu, 11 Feb 2021 20:06:43 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35572 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229940AbhBLBGa (ORCPT ); Thu, 11 Feb 2021 20:06:30 -0500 Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com [IPv6:2a00:1450:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 98D33C061788; Thu, 11 Feb 2021 17:05:50 -0800 (PST) Received: by mail-ej1-x630.google.com with SMTP id a9so13032473ejr.2; Thu, 11 Feb 2021 17:05:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=VU/Ikjjp9YKqSORV3tobt3dZbVwKnijcAhFtaSftkOM=; b=qonI8wT2dff7Mgsx31B+JOmlvExNq+3TpV5Yz8M7nlv4XGgSFrCxsRmEM7/a5q75+f fepRIKLIGyqAbSW+WEGSqaHHEiMzZsxOtmHEzn+fTkj4lj801GQWwfzgZIOQoI3DNLmm i96R1+mB5/C8rZTZ77FYFQuncEK10Gx6tOK15nqaEvQ4LfbXBrCy1GOEYuf9y7zLRV0C AkAvEsJF1Utgo5JzfjjkGuOfzn5yFTnsfptqd2hX+yvMmfKgh9Ls8MuvZ8NRhbDMpFTJ HG30AhPayp5i9X6pTcFds/L1V/8UP8RbSrVErEcgH5IVGWyUhNMT/VWJ2Uf4bkE0r3Wj RzqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=VU/Ikjjp9YKqSORV3tobt3dZbVwKnijcAhFtaSftkOM=; b=R4dUYUo3p0Tg1FW5KZXYpTJerGMZzjn0SjIbC5dV2mFFBmEpBm0yYuUBEnBWYWJvS0 IZTpwcD7uQRNk5/3SC4jW4g0K+bZmy00P4VgvZQqZ/iL7LURzzFp7KLYpZRPaXbJK2pY VvrC7n8QFRIzk0Q4MXZud6+SCJ7x6i/eal5ihmyVFPdY9eYm3sE6nsTqg7PVBoQ3a7y3 PQXxSz27z5Uj3OPz4Y7I5SN0aqNXwDfdnG0MmPmlXJtX3dGy3LKX+BgLiiPQ9Tiux/DH QP63pcAfVddhne/xlT5wHuom4aOy9QfogiaBLACGxSciueC3GUipk1sspn4i5dCpBnBD RyjA== X-Gm-Message-State: AOAM5306mlY7mWCL2W+fPc9H8aVNc1kmVez9jA1XJ/0RfJpiz8eKD5Yq 2owaIadCSgYUyoMshC38eiw= X-Google-Smtp-Source: ABdhPJztOKxP6QlHyjfAX6gy8Mxma2MIQRJSXYZtm/pEL/KXLeQmNrUN84DOZf/rfS3UpGlDuSJvkg== X-Received: by 2002:a17:906:d19b:: with SMTP id c27mr480240ejz.234.1613091949335; Thu, 11 Feb 2021 17:05:49 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id z13sm5019580edc.73.2021.02.11.17.05.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Feb 2021 17:05:48 -0800 (PST) From: Vladimir Oltean To: Jakub Kicinski , "David S. Miller" Cc: Andrew Lunn , Vivien Didelot , Florian Fainelli , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, bridge@lists.linux-foundation.org, Roopa Prabhu , Nikolay Aleksandrov , Jiri Pirko , Ido Schimmel , Claudiu Manoil , Alexandre Belloni , UNGLinuxDriver@microchip.com, Vadym Kochan , Taras Chornyi , Grygorii Strashko , Ioana Ciornei , Ivan Vecera , linux-omap@vger.kernel.org Subject: [PATCH v4 net-next 4/9] net: dsa: configure better brport flags when ports leave the bridge Date: Fri, 12 Feb 2021 03:05:26 +0200 Message-Id: <20210212010531.2722925-5-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210212010531.2722925-1-olteanv@gmail.com> References: <20210212010531.2722925-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean 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 --- Changes in v4: None. Changes in v3: Patch is new, logically it was moved from the bridge layer to the DSA layer. net/dsa/port.c | 45 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/net/dsa/port.c b/net/dsa/port.c index 5e079a61528e..9f65ba11ad00 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -122,6 +122,27 @@ void dsa_port_disable(struct dsa_port *dp) 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)); + if (err) + continue; + + dsa_port_bridge_flags(dp, flags & BIT(flag)); + } +} + int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br) { struct dsa_notifier_bridge_info info = { @@ -132,10 +153,10 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br) }; int err; - /* Set the flooding mode before joining the port in the switch */ - err = dsa_port_bridge_flags(dp, BR_FLOOD | BR_MCAST_FLOOD); - 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. @@ -146,7 +167,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br) /* The bridging is rolled back on error */ if (err) { - dsa_port_bridge_flags(dp, 0); + dsa_port_change_brport_flags(dp, false); dp->bridge_dev = NULL; } @@ -172,8 +193,18 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br) 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); + /* 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