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

On Wed, Jan 18, 2023 at 11:14:00PM +0100, netdev at kapio-technology.com
wrote:
> > > +	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.

On Thu, Jan 19, 2023 at 11:33:58AM +0200, Vladimir Oltean
wrote:
> On Wed, Jan 18, 2023 at 11:14:00PM +0100, netdev at kapio-technology.com
wrote:
> > > > +	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.