Ido Schimmel
2023-Feb-01 18:10 UTC
[Bridge] [PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
On Mon, Jan 30, 2023 at 06:34:25PM +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 | 2 ++ > 2 files changed, 3 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..4420fcbbfdb2 100644 > --- a/net/bridge/br_switchdev.c > +++ b/net/bridge/br_switchdev.c > @@ -136,6 +136,8 @@ 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 not 'is_static' and be consistent with the bridge flag like all the other fields? Regardless of how you name this field, it is irrelevant for 'SWITCHDEV_FDB_ADD_TO_BRIDGE' notifications that all add FDB entries with the 'BR_FDB_ADDED_BY_EXT_LEARN' flag set, which makes 'BR_FDB_STATIC' irrelevant.> + item->added_by_user;Unclear why this is needed...> 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-Feb-02 07:28 UTC
[Bridge] [PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
On 2023-02-01 19:10, Ido Schimmel wrote:> On Mon, Jan 30, 2023 at 06:34:25PM +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 | 2 ++ >> 2 files changed, 3 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..4420fcbbfdb2 100644 >> --- a/net/bridge/br_switchdev.c >> +++ b/net/bridge/br_switchdev.c >> @@ -136,6 +136,8 @@ 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 not 'is_static' and be consistent with the bridge flag like all the > other fields? > > Regardless of how you name this field, it is irrelevant for > 'SWITCHDEV_FDB_ADD_TO_BRIDGE' notifications that all add FDB entries > with the 'BR_FDB_ADDED_BY_EXT_LEARN' flag set, which makes > 'BR_FDB_STATIC' irrelevant. > >> + item->added_by_user; > > Unclear why this is needed... >The answer to those two questions lies in my earlier correspondences (with Oltean) on the RFC version.