On 2023-01-19 14:40, Vladimir Oltean wrote:

On Thu, Jan 19, 2023 at 11:33:58AM +0200, Vladimir Oltean wrote:

quoted

On Wed, Jan 18, 2023 at 11:14:00PM +0100, netdev@kapio-technology.com 
wrote:

quoted

quoted

quoted

+	item->is_dyn = !test_bit(BR_FDB_STATIC, &fdb->flags);

Why reverse logic? Why not just name this "is_static" and leave any
further interpretations up to the consumer?

My reasoning for this is that the common case is to have static entries,
thus is_dyn=false, so whenever someone uses a switchdev_notifier_fdb_info
struct the common case does not need to be entered.
Otherwise it might also break something when someone uses this struct and if
it was 'is_static' and they forget to code is_static=true they will get
dynamic entries without wanting it and it can be hard to find such an error.

I'll leave it up to bridge maintainers if this is preferable to 
patching
all callers of SWITCHDEV_FDB_ADD_TO_BRIDGE such that they set 
is_static=true.

Actually, why would you assume that all users of 
SWITCHDEV_FDB_ADD_TO_BRIDGE
want to add static FDB entries? You can't avoid inspecting the code and
making sure that the is_dyn/is_static flag is set correctly either way.

Well, up until this patch set there is no option, besides entries from 
SWITCHDEV_FDB_ADD_TO_BRIDGE events will get the external learned flag 
set, so they will not be aged by the bridge, and so dynamic entries that 
way don't make much sense I think. Is that not right?