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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 07938C636D4 for ; Thu, 2 Feb 2023 16:39:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:Cc:To:From :Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=zP7Qhw27TMPVquLsGwtKKhLRbJBkmrhd7k+T6g+Uk5E=; b=jchnTnzO4NcvGHc6vvML5IFv5n RR8LT7RRKCzKSSCet9DdkheBH0RbK+1ukn3Q9+WVsAu+eeUOd5MxMx8vu8glNtO/qHQnMfaRaa8W8 lNNa/zGp2oWQteR350/g/b/ge+/LejqlbBG/Z715R7eF/7JGo0khKUDHCf85KJvqq1xax2q2OVjeA nC9zR9sbkYrbOyX7u44THxMyHZyvudvkL09Iul7Sfh+8kI3a5Y7O669YwqP6KNcAcEeVWhd7PINS9 QxwIXfDSlMwHYFl3UjLaNeN7ZvutzZTyMFGWjqy7xk8U6Z2q0Kmaelr9/H0EUDqNhSXy7RI5DJDAY 5+zbXZaA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pNcbD-00GgIe-WA; Thu, 02 Feb 2023 16:38:12 +0000 Received: from mailout-taastrup.gigahost.dk ([46.183.139.199]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pNcbA-00GgI2-6m; Thu, 02 Feb 2023 16:38:10 +0000 Received: from mailout.gigahost.dk (mailout.gigahost.dk [89.186.169.112]) by mailout-taastrup.gigahost.dk (Postfix) with ESMTP id 965C318843AC; Thu, 2 Feb 2023 16:38:06 +0000 (UTC) Received: from smtp.gigahost.dk (smtp.gigahost.dk [89.186.169.109]) by mailout.gigahost.dk (Postfix) with ESMTP id 8D99F250007B; Thu, 2 Feb 2023 16:38:06 +0000 (UTC) Received: by smtp.gigahost.dk (Postfix, from userid 1000) id 743E591201E4; Thu, 2 Feb 2023 16:38:06 +0000 (UTC) X-Screener-Id: 413d8c6ce5bf6eab4824d0abaab02863e8e3f662 MIME-Version: 1.0 Date: Thu, 02 Feb 2023 17:38:06 +0100 From: netdev@kapio-technology.com To: Ido Schimmel Cc: davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org, Florian Fainelli , Andrew Lunn , Vladimir Oltean , Eric Dumazet , Paolo Abeni , Kurt Kanzenbach , Hauke Mehrtens , Woojung Huh , "maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER" , Sean Wang , Landen Chao , DENG Qingfang , Matthias Brugger , Claudiu Manoil , Alexandre Belloni , =?UTF-8?Q?Cl=C3=A9m?= =?UTF-8?Q?ent_L=C3=A9ger?= , Jiri Pirko , Ivan Vecera , Roopa Prabhu , Nikolay Aleksandrov , Russell King , Christian Marangi , open list , "moderated list:ARM/Mediatek SoC support" , "moderated list:ARM/Mediatek SoC support" , "open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER" , "moderated list:ETHERNET BRIDGE" Subject: Re: [PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier In-Reply-To: References: <20230130173429.3577450-1-netdev@kapio-technology.com> <20230130173429.3577450-2-netdev@kapio-technology.com> User-Agent: Gigahost Webmail Message-ID: <766efaf94fcb6362c5ceb176ad7955f1@kapio-technology.com> X-Sender: netdev@kapio-technology.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230202_083808_570438_ED87A9CF X-CRM114-Status: GOOD ( 33.98 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2023-02-02 17:11, Ido Schimmel wrote: > On Thu, Feb 02, 2023 at 08:28:36AM +0100, netdev@kapio-technology.com > wrote: >> On 2023-02-01 19:10, Ido Schimmel wrote: >> > On Mon, Jan 30, 2023 at 06:34:25PM +0100, Hans J. Schultz wrote: >> > > To be able to add dynamic FDB entries to drivers from userspace, the >> > > dynamic flag must be added when sending RTM_NEWNEIGH events down. >> > > >> > > Signed-off-by: Hans J. Schultz >> > > --- >> > > include/net/switchdev.h | 1 + >> > > net/bridge/br_switchdev.c | 2 ++ >> > > 2 files changed, 3 insertions(+) >> > > >> > > diff --git a/include/net/switchdev.h b/include/net/switchdev.h >> > > index ca0312b78294..aaf918d4ba67 100644 >> > > --- a/include/net/switchdev.h >> > > +++ b/include/net/switchdev.h >> > > @@ -249,6 +249,7 @@ struct switchdev_notifier_fdb_info { >> > > u8 added_by_user:1, >> > > is_local:1, >> > > locked:1, >> > > + is_dyn:1, >> > > offloaded:1; >> > > }; >> > > >> > > diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c >> > > index 7eb6fd5bb917..4420fcbbfdb2 100644 >> > > --- a/net/bridge/br_switchdev.c >> > > +++ b/net/bridge/br_switchdev.c >> > > @@ -136,6 +136,8 @@ static void br_switchdev_fdb_populate(struct >> > > net_bridge *br, >> > > item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); >> > > item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags); >> > > item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags); >> > > + item->is_dyn = !test_bit(BR_FDB_STATIC, &fdb->flags) && >> > >> > Why not 'is_static' and be consistent with the bridge flag like all the >> > other fields? >> > >> > Regardless of how you name this field, it is irrelevant for >> > 'SWITCHDEV_FDB_ADD_TO_BRIDGE' notifications that all add FDB entries >> > with the 'BR_FDB_ADDED_BY_EXT_LEARN' flag set, which makes >> > 'BR_FDB_STATIC' irrelevant. >> > >> > > + item->added_by_user; >> > >> > Unclear why this is needed... >> > >> >> The answer to those two questions lies in my earlier correspondences >> (with >> Oltean) on the RFC version. > > It is not up to me as a reviewer to dig up old versions of the patch > and > find out what was changed and why. It is up to you as the submitter of > the patch to provide all this information in the patch posting. Please > read: > https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > Specifically: > > "Review comments or questions that do not lead to a code change should > almost certainly bring about a comment or changelog entry so that the > next reviewer better understands what is going on." > > And: > > "Other comments relevant only to the moment or the maintainer, not > suitable for the permanent changelog, should also go here. A good > example of such comments might be patch changelogs which describe what > has changed between the v1 and v2 version of the patch. > > Please put this information after the --- line which separates the > changelog from the rest of the patch. The version information is not > part of the changelog which gets committed to the git tree. It is > additional information for the reviewers." > > Thanks Sorry about that. I thought it would be easily found... On the first question please look here: https://lore.kernel.org/netdev/20230119134045.fqdt6zrna5x3iavt@skbuf/ On the second question it is what Oltean pointed out to me here... https://lore.kernel.org/netdev/20230118230135.szu6a7kvt2mjb3i5@skbuf/ Oltean says there: "This is not true, because it assumes that DSA never called port_fdb_add() up until now for bridge FDB entries with the BR_FDB_STATIC flag unset, which is incorrect (it did)." Though as I see it, if it is only from the DSA layer on, the new is_dynamic flag would not be set anyway in the case he references. And as can be seen the change is in the bridge layer, as the rest is just propagating the flag, but it ensures that to set this flag that it comes from the user adding an FDB entry. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel