Re: [PATCH net-next v1 5/9] net: dsa: qca: ar9331: add forwarding database support
From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-04-04 00:46:27
Also in:
linux-mips, lkml
quoted
Plus, i'm not actually sure we should be issuing warnings here. What does the bridge code do in this case? Is it silent and just does it, or does it issue a warning?:D What Oleksij doesn't know, I bet, is that he's using the bridge bypass commands: bridge fdb add dev lan0 00:01:02:03:04:05 That is the deprecated way of managing FDB entries, and has several disadvantages such as: - the bridge software FDB never gets updated with this entry, so other drivers which might be subscribed to DSA's FDB (imagine a non-DSA driver having the same logic as our ds->assisted_learning_on_cpu_port) will never see these FDB entries - you have to manage duplicates yourself
I was actually meaning a pure software bridge, with unaccelerated
interfaces. It has a dynamic MAC address in its tables, and the user
adds a static. Ideally, we want the same behaviour.
And i think the answer is:
static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid)
{
struct net_bridge_fdb_entry *fdb;
if (!is_valid_ether_addr(addr))
return -EINVAL;
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));
if (!fdb)
return -ENOMEM;
fdb_add_hw_addr(br, addr);
fdb_notify(br, fdb, RTM_NEWNEIGH, true);
return 0;
}
So it looks like it warns and then replaces the dynamic entry.
So having the DSA driver also warn is maybe O.K. Having said that, i
don't think any other DSA driver does.
Andrew