Vladimir Oltean
2023-Jan-17 23:08 UTC
[Bridge] [RFC PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
On Tue, Jan 17, 2023 at 07:57:10PM +0100, Hans J. Schultz wrote:> To be able to add dynamic FDB entries to drivers from userspace, the > dynamic flag must be added when sending RTM_NEWNEIGH events down. > > Signed-off-by: Hans J. Schultz <netdev at kapio-technology.com> > --- > include/net/switchdev.h | 1 + > net/bridge/br_switchdev.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/include/net/switchdev.h b/include/net/switchdev.h > index ca0312b78294..aaf918d4ba67 100644 > --- a/include/net/switchdev.h > +++ b/include/net/switchdev.h > @@ -249,6 +249,7 @@ struct switchdev_notifier_fdb_info { > u8 added_by_user:1, > is_local:1, > locked:1, > + is_dyn:1, > offloaded:1; > }; > > diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c > index 7eb6fd5bb917..60c05a00a1df 100644 > --- a/net/bridge/br_switchdev.c > +++ b/net/bridge/br_switchdev.c > @@ -136,6 +136,7 @@ static void br_switchdev_fdb_populate(struct net_bridge *br, > item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); > item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags); > item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags); > + 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?> item->locked = false; > item->info.dev = (!p || item->is_local) ? br->dev : p->dev; > item->info.ctx = ctx; > -- > 2.34.1 >
netdev at kapio-technology.com
2023-Jan-18 22:14 UTC
[Bridge] [RFC PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
On 2023-01-18 00:08, Vladimir Oltean wrote:> On Tue, Jan 17, 2023 at 07:57:10PM +0100, Hans J. Schultz wrote: >> To be able to add dynamic FDB entries to drivers from userspace, the >> dynamic flag must be added when sending RTM_NEWNEIGH events down. >> >> Signed-off-by: Hans J. Schultz <netdev at kapio-technology.com> >> --- >> include/net/switchdev.h | 1 + >> net/bridge/br_switchdev.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/include/net/switchdev.h b/include/net/switchdev.h >> index ca0312b78294..aaf918d4ba67 100644 >> --- a/include/net/switchdev.h >> +++ b/include/net/switchdev.h >> @@ -249,6 +249,7 @@ struct switchdev_notifier_fdb_info { >> u8 added_by_user:1, >> is_local:1, >> locked:1, >> + is_dyn:1, >> offloaded:1; >> }; >> >> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c >> index 7eb6fd5bb917..60c05a00a1df 100644 >> --- a/net/bridge/br_switchdev.c >> +++ b/net/bridge/br_switchdev.c >> @@ -136,6 +136,7 @@ static void br_switchdev_fdb_populate(struct >> net_bridge *br, >> item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); >> item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags); >> item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags); >> + 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.>> item->locked = false; >> item->info.dev = (!p || item->is_local) ? br->dev : p->dev; >> item->info.ctx = ctx; >> -- >> 2.34.1 >>
Possibly Parallel Threads
- [Bridge] [RFC PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
- [Bridge] [PATCH v2 net-next 1/6] net: bridge: add dynamic flag to switchdev notifier
- [Bridge] [RFC PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
- [Bridge] [RFC PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
- [Bridge] [PATCH net] net: bridge: switchdev: don't notify FDB entries with "master dynamic"