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

Vladimir Oltean
2021-03-22T16:20:56Z

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?