Thread (30 messages) 30 messages, 5 authors, 2021-02-03

Re: [RFC net-next 2/7] net: bridge: switchdev: Include local flag in FDB notifications

From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-01-18 21:05:38

On Mon, Jan 18, 2021 at 09:19:11PM +0100, Tobias Waldekranz wrote:
quoted
My only hope is that we could just offload the entries pointing towards
br0, and ignore the local ones.
That was my initial approach. Unfortunately that breaks down when the
bridge inherits its address from a port, i.e. the default case.

When the address is added to the bridge (fdb->dst == NULL), fdb_insert
will find the previous local entry that is set on the port and bail out
before sending a notification:

	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);
	}

You could change this so that a notification always is sent out. Or you
could give precedence to !fdb->dst and update the existing entry.
I'm afraid my competence ends here.
IMO the problem is really the struct net_bridge_port *source argument of
fdb_insert. The behavior we want is that all is_local FDB entries are
coming from br0, and none from the brports (aka source == NULL, so the
callers that had something non-NULL for source should be deleted).
"You can't always get what you want" though.
quoted
But for that I would need the bridge maintainers to clarify what is
the difference between then, as I asked in your other patch.
I am pretty sure they mean the same thing, I believe that !fdb->dst
implies is_local. It is just that "bridge fdb add ADDR dev br0 self" is
a new(er) thing, and before that there was "local" entries on ports.
Maybe I should try to get rid of the local flag in the bridge first, and
then come back to this problem once that is done? Either way, I agree
that 5/7 is all we want to add to DSA to get this working.
Please expand on what you plan to do. The is_local bit is part of the
bridge UAPI, how do you plan to get rid of it?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help