Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 12 Apr 2023 19:24:07 +0300
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Ido Schimmel <idosch@nvidia.com>
Cc:     netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        "Hans J. Schultz" <netdev@kapio-technology.com>,
        Roopa Prabhu <roopa@nvidia.com>,
        Nikolay Aleksandrov <razor@blackwall.org>,
        Ivan Vecera <ivecera@redhat.com>,
        Jiri Pirko <jiri@resnulli.us>,
        Arkadi Sharshevsky <arkadis@mellanox.com>,
        Ido Schimmel <idosch@mellanox.com>,
        bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net] net: bridge: switchdev: don't notify FDB entries
 with "master dynamic"
Message-ID: <20230412162407.xk3okeiedylv6sqp@skbuf>
References: <20230410204951.1359485-1-vladimir.oltean@nxp.com>
 <ZDa856x2rhzNrrXa@shredder>
 <20230412142733.6jhxt7kjf3nwqzsy@skbuf>
 <ZDbVgqV9JT7Ru96j@shredder>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZDbVgqV9JT7Ru96j@shredder>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MZe5W+/SCjUu5wpzJOswxkj7pcb9sNVAzuwZKgjNszBtM86Px+hx1tffjmfa?=
 =?us-ascii?Q?O6vLWXXCPfXH20jWexfFAkB+12UQaUxJsrp02Jbl7Mr4zKl2fgjsARoE1TtX?=
 =?us-ascii?Q?eMPHuvbbsFmYS8YFS3rvDeLYH9NeIl8BRSAbt9I4ID+Q46RJkKra3ZDw3Uu5?=
 =?us-ascii?Q?kHrgvKkswiEcBQna3nmeL7DP/EgrBEZKCjgxeSvlQNCOajzo8VrCZiAjceI+?=
 =?us-ascii?Q?jvllT0T1NaV5c4WTDXDGA8GM9lMnByvYdbSzY5CDCOgzrhwsXk8m3peq/Ksc?=
 =?us-ascii?Q?iL09lamgLPaJK62F9BdLMfu+5wDiDOWH7DPjnvnOqtpvVLuIG5kz6ijpshY/?=
 =?us-ascii?Q?gMrUsntLE3roymSYYP27fPNbzD19eD63MG1wJJGw8EOucDICcelgOtDr9DDM?=
 =?us-ascii?Q?gPV8ZuPuydtKBNIqMGPYDRO1zCydj9fa7+tTAy4IhYojTC9+87K2KpQAYkTk?=
 =?us-ascii?Q?qHXztwioz3ZP+mw2tlGzmc4kIYY1JRw/+AGBtPAWQMDA71l6WLHPixUYFUpp?=
 =?us-ascii?Q?5azgyxZ7x9E0EW/zKfh0flbjtyzdtzz23U8G25KZnfs6owxu56raCHQfTyzh?=
 =?us-ascii?Q?7Zkxlu4disYKxKggkJDWXAZWkueHhb4F2B3HeXJGlRRdoZYeWyze/ZmtpZvA?=
 =?us-ascii?Q?UR9cH8U211ikT3mMiHtDPtEzKzIJGA2Wuzwvmw1zIyTH5QbGDyXOIjkXqLBG?=
 =?us-ascii?Q?lb0w6lPkR6+damBsamSoPptlXX5TJ1B6QpS17CK48fkRIUSeho70DtClKiRH?=
 =?us-ascii?Q?i+dcSUK6cUWJrrvpidrRDBPOhUDUkAuPUlSQ/hp4+GbN1W+Xvu/ACTfaS14n?=
 =?us-ascii?Q?u7tv/f3ACKKwtTZDZim0oJOvaufvijaiS2Z4kMLK4X7NbTSYRtPwU/OZSFJP?=
 =?us-ascii?Q?2+ZlocgHVFGus/v9mtLbw/2AyBXKoSBFZZCPFZbvXxjeO3BUGxF2w9jd7TSZ?=
 =?us-ascii?Q?RZHkOIcNgAMIfO58mDPXIbXggwmrMDJkg54DyYS/msmJ5+b71CwayQO7O93U?=
 =?us-ascii?Q?KCtCNxvHX+O8qtThiKW18RmfxVjxbE7wzw5LuL+TUjXjNTukrmJP00X8h9/k?=
 =?us-ascii?Q?h4P1RUA+ifHjQMKTSqshNU17yizuOrRewgXSKu7bhYTcTmmGSzpWRbqWLcNL?=
 =?us-ascii?Q?nGPLiIqHNWtdA5OO2eQgTRL8mkNRwJkMGRe4e1Slfdxwqkb+o6WOptVOM5k2?=
 =?us-ascii?Q?ISk+Zx2rH5s2XfEOoy7qicDkWZYRJ1gWYNIVfbakUCb5PSVZM5a7cVgxi5Lr?=
 =?us-ascii?Q?9YnWt5HiV9Zd7yqWgPqw3UDpsfy/ojf773D76G0CV+FZAwphYKrL0+k0GEvF?=
 =?us-ascii?Q?PXJ0jSdlj8dLPJKq/b7qVHN7HrenOaee8VX52+lqHv/YodS1969S+CK+T4kh?=
 =?us-ascii?Q?icr1CumC4zSkLPlG2gsiJ238PeJd2o6ctrD9W/ptdyn1zJloT5ZZF+4sVBuG?=
 =?us-ascii?Q?CA2ymDA242mRr6/0ITUhxVssNfHJSG6fkWqc40xdSwyegas3nYrS3pOG096L?=
 =?us-ascii?Q?+yfC/Dzmy+C0P0sXtW3b7OYUV3WJ1aSkYQ2mcOIY25VTu1a9LtInbT8wXMaP?=
 =?us-ascii?Q?NuGqX1hRlYkiCZXrrvo1pK2KLzgh+0reIVcHOlf5+2LeQgJwrN7ubiKyPvi9?=
 =?us-ascii?Q?CA=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bfc43276-0e5d-4dc5-fecd-08db3b725959
X-MS-Exchange-CrossTenant-AuthSource: AM0PR04MB6452.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Apr 2023 16:24:11.6013
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: rexKwyRL+8/L1D+X6IR2YWLP8Euk5hi1L+murmLDFXw97QzAH0hf1E5Ogv2gZbvGkoKHEg9J931lgkKbqh7T6Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB8PR04MB7147
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Apr 12, 2023 at 07:00:02PM +0300, Ido Schimmel wrote:
> On Wed, Apr 12, 2023 at 05:27:33PM +0300, Vladimir Oltean wrote:
> > How are extern_learn FDB entries processed by spectrum's
> > SWITCHDEV_FDB_ADD_TO_DEVICE handler?
> 
> No different than "BR_FDB_STATIC", which is a bug I'm aware of and
> intend to fix in net-next when I get the time (together with all the
> other combinations enabled by the bridge). Entry has ageing disabled,
> but can roam in which case it becomes age-able.
> 
> TBH, I think most devices don't handle "BR_FDB_STATIC" correctly. In the
> Linux bridge, "BR_FDB_STATIC" only means ageing disabled. The entry can
> still roam, but remains "static". I believe that in most devices out
> there "static" means no roaming and no ageing which is equivalent to
> "BR_FDB_STATIC | BR_FDB_STICKY". Mentioned in your commit message as
> well: "As for the hardware FDB entry, that's static, it doesn't move
> when the station roams."
> 
> As it stands, the situation is far from perfect, but the patch doesn't
> solve a regression (always broken) and will introduce one. My suggestion
> allows you to move forward and solve the "dynamic" case, so let's
> proceed with that unless there's a better alternative.

I'm not trying to accuse anybody, I just wanted to make sure I'm not
missing something (and surprise, I was).

The comment regarding BR_FDB_STATIC vs BR_FDB_STATIC | BR_FDB_STICKY
is interesting. This whole "hey, did you know you were never using the
bridge fdb flags correctly?" is starting to become a bit of a meme.

I'll send v2 with BR_FDB_ADDED_BY_EXT_LEARN not prevented from being
notified from switchdev.

Unless you have any objection, I won't send v2 like this:

	if (test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags) &&
	    !test_bit(BR_FDB_STATIC, &fdb->flags) &&
	    !test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags))
		return;

but like this:

	/* Entries with just the BR_FDB_ADDED_BY_USER flag set were created
	 * using 'bridge fdb add ... master dynamic'
	 */
	if (fdb->flags == BIT(BR_FDB_ADDED_BY_USER))
		return;

Thanks for the review and for pointing out the regression early.