Re: [RFC PATCH v2 net-next 09/16] net: dsa: replay port and local fdb entries when joining the bridge

On Mon, Mar 22, 2021 at 04:44:41PM +0100, Tobias Waldekranz wrote:

I do not know if it is a problem or not, more of an observation: This is
not guaranteed to be an exact replay of the events that the bridge port
(i.e. bond0 or whatever) has received since, in fdb_insert, we exit
early when adding local entries if that address is already in the
database.

Do we have to guard against this somehow? Or maybe we should consider
the current behavior a bug and make sure to always send the event in the
first place?

I don't really understand what you're saying.
fdb_insert has:

	fdb = br_fdb_find(br, addr, vid);
	if (fdb) {
		/* it is okay to have multiple ports with same
		 * address, just use the first one.
		 */
		if (test_bit(BR_FDB_LOCAL, &fdb->flags))
			return 0;
		br_warn(br, "adding interface %s with same address as a received packet (addr:%pM, vlan:%u)\n",
		       source ? source->dev->name : br->dev->name, addr, vid);
		fdb_delete(br, fdb, true);
	}

	fdb = fdb_create(br, source, addr, vid,
			 BIT(BR_FDB_LOCAL) | BIT(BR_FDB_STATIC));

Basically, if the {addr, vid} pair already exists in the fdb, and it
points to a local entry, fdb_create is bypassed.

Whereas my br_fdb_replay() function iterates over br->fdb_list, which is
exactly where fdb_create() also lays its eggs. That is to say, unless
I'm missing something, that duplicate local FDB entries that skipped the
fdb_create() call in fdb_insert() because they were for already-existing
local FDB entries will also be skipped by br_fdb_replay(), because it
iterates over a br->fdb_list which contains unique local addresses.
Where am I wrong?