Re: [RFC PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier

From: Vladimir Oltean <olteanv@gmail.com>
To: "Hans J. Schultz" <netdev@kapio-technology.com>
Cc: davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Eric Dumazet" <edumazet@google.com>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Kurt Kanzenbach" <kurt@linutronix.de>,
	"Hauke Mehrtens" <hauke@hauke-m.de>,
	"Woojung Huh" <woojung.huh@microchip.com>,
	"maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER"
	<UNGLinuxDriver@microchip.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Landen Chao" <Landen.Chao@mediatek.com>,
	"DENG Qingfang" <dqfext@gmail.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Claudiu Manoil" <claudiu.manoil@nxp.com>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Clément Léger" <clement.leger@bootlin.com>,
	"Jiri Pirko" <jiri@resnulli.us>,
	"Ivan Vecera" <ivecera@redhat.com>,
	"Roopa Prabhu" <roopa@nvidia.com>,
	"Nikolay Aleksandrov" <razor@blackwall.org>,
	"Russell King" <linux@armlinux.org.uk>,
	"Christian Marangi" <ansuelsmth@gmail.com>,
	"open list" <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER"
	<linux-renesas-soc@vger.kernel.org>,
	"moderated list:ETHERNET BRIDGE"
	<bridge@lists.linux-foundation.org>
Subject: Re: [RFC PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
Date: Wed, 18 Jan 2023 01:08:06 +0200	[thread overview]
Message-ID: <20230117230806.ipwcbnq4jcc4qs7z@skbuf> (raw)
In-Reply-To: <20230117185714.3058453-2-netdev@kapio-technology.com>

On Tue, Jan 17, 2023 at 07:57:10PM +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 <netdev@kapio-technology.com>
> ---
>  include/net/switchdev.h   | 1 +
>  net/bridge/br_switchdev.c | 1 +
>  2 files changed, 2 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..60c05a00a1df 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -136,6 +136,7 @@ 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 reverse logic? Why not just name this "is_static" and leave any
further interpretations up to the consumer?

>  	item->locked = false;
>  	item->info.dev = (!p || item->is_local) ? br->dev : p->dev;
>  	item->info.ctx = ctx;
> -- 
> 2.34.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel