Linux Ethernet Bridging - Feb 2023 - [Bridge] [PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier

On Thu, Feb 02, 2023 at 05:38:06PM +0100, netdev at kapio-technology.com
wrote:
> On the first question please look here:
> https://lore.kernel.org/netdev/20230119134045.fqdt6zrna5x3iavt at skbuf/
It seems Vladimir also wants the new field to be named 'is_static'
instead of 'is_dyn'. In your reason you mention
'SWITCHDEV_FDB_ADD_TO_BRIDGE', but this is not the interesting case for
the field. This event is used for devices to notify the bridge on new
learned entries. The bridge marks them as "extern_learn" which means
that "dynamic" / "static" flags are irrelevant.

The interesting case for the new field is the bridge to device direction
('SWITCHDEV_FDB_ADD_TO_DEVICE'). Drivers need to be patched to take the
new field into account when deciding the policy to program the entry
with. They can do it just as well if you name the new field 'is_static'
instead of 'is_dyn'.
> On the second question it is what Oltean pointed out to me here...
> https://lore.kernel.org/netdev/20230118230135.szu6a7kvt2mjb3i5 at skbuf/
> 
> Oltean says there:
> "This is not true, because it assumes that DSA never called
port_fdb_add()
> up until now for bridge FDB entries with the BR_FDB_STATIC flag unset,
> which is incorrect (it did)."
> 
> Though as I see it, if it is only from the DSA layer on, the new is_dynamic
> flag would not be set anyway in the case he references. And as can be seen
> the change is in the bridge layer, as the rest is just propagating the
flag,
> but it ensures that to set this flag that it comes from the user adding an
> FDB entry.
OK, so can't this hunk:

```
	if (fdb_info->is_dyn)
		fdb_flags |= DSA_FDB_FLAG_DYNAMIC;
```

Become:

```
	if (fdb_info->is_dyn && !fdb_info->added_by_user)
		fdb_flags |= DSA_FDB_FLAG_DYNAMIC;
```

?

Then there is no need to fold 'added_by_user' into 'is_dyn' in
the
bridge driver. I *think* this is the change Vladimir asked you to do.

On Fri, 3 Feb 2023 at 18:14, Ido Schimmel <idosch at idosch.org>
wrote:
> I *think* this is the change Vladimir asked you to do.
Yup, although instead of "is_dyn", I would still prefer
"!is_static",
but again, that's a preference for bridge/switchdev maintainers to
override.

On 2023-02-03 17:14, Ido Schimmel wrote:
> 
> OK, so can't this hunk:
> 
> ```
> 	if (fdb_info->is_dyn)
> 		fdb_flags |= DSA_FDB_FLAG_DYNAMIC;
> ```
> 
> Become:
> 
> ```
> 	if (fdb_info->is_dyn && !fdb_info->added_by_user)
> 		fdb_flags |= DSA_FDB_FLAG_DYNAMIC;
> ```
> 
> ?
> 
> Then there is no need to fold 'added_by_user' into 'is_dyn'
in the
> bridge driver. I *think* this is the change Vladimir asked you to do.
I suppose you mean?:
  	if (fdb_info->is_dyn && fdb_info->added_by_user)
  		fdb_flags |= DSA_FDB_FLAG_DYNAMIC;