| Message ID | 20221018165619.134535-4-netdev@kapio-technology.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Extend locked port feature with FDB locked flag (MAC-Auth/MAB) | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 51 this patch: 51 |
| netdev/cc_maintainers | success | CCed 13 of 13 maintainers |
| netdev/build_clang | success | Errors and warnings before: 15 this patch: 15 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 66 this patch: 66 |
| netdev/checkpatch | warning | WARNING: line length of 85 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 3 this patch: 3 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, Oct 18, 2022 at 06:56:10PM +0200, Hans J. Schultz wrote: > diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c > index 8f3d76c751dd..c6b938c01a74 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->locked = test_bit(BR_FDB_LOCKED, &fdb->flags); Shouldn't this be set to 0 here, since it is the bridge->driver direction? > item->info.dev = (!p || item->is_local) ? br->dev : p->dev; > item->info.ctx = ctx; > } > -- > 2.34.1 >
On 2022-10-20 14:55, Vladimir Oltean wrote: > On Tue, Oct 18, 2022 at 06:56:10PM +0200, Hans J. Schultz wrote: >> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c >> index 8f3d76c751dd..c6b938c01a74 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->locked = test_bit(BR_FDB_LOCKED, &fdb->flags); > > Shouldn't this be set to 0 here, since it is the bridge->driver > direction? > Wouldn't it be a good idea to allow drivers to add what corresponds to a blackhole entry when using the bridge input chain to activate the MAB feature, or in general to leave the decision of what to do to the driver implementation?
On Thu, Oct 20, 2022 at 09:29:06PM +0200, netdev@kapio-technology.com wrote: > On 2022-10-20 14:55, Vladimir Oltean wrote: > > On Tue, Oct 18, 2022 at 06:56:10PM +0200, Hans J. Schultz wrote: > > > diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c > > > index 8f3d76c751dd..c6b938c01a74 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->locked = test_bit(BR_FDB_LOCKED, &fdb->flags); > > > > Shouldn't this be set to 0 here, since it is the bridge->driver > > direction? > > Wouldn't it be a good idea to allow drivers to add what corresponds to a blackhole > entry when using the bridge input chain to activate the MAB feature, or in general > to leave the decision of what to do to the driver implementation? The patch doesn't propose that. It proposes: | net: bridge: enable bridge to install locked fdb entries from drivers | | The bridge will be able to install locked entries when receiving | SWITCHDEV_FDB_ADD_TO_BRIDGE notifications from drivers. Please write patches which make just one logical change, and explain the justification for that change and precisely that change in the commit message.
diff --git a/include/net/switchdev.h b/include/net/switchdev.h index 7dcdc97c0bc3..ca0312b78294 100644 --- a/include/net/switchdev.h +++ b/include/net/switchdev.h @@ -248,6 +248,7 @@ struct switchdev_notifier_fdb_info { u16 vid; u8 added_by_user:1, is_local:1, + locked:1, offloaded:1; }; diff --git a/net/bridge/br.c b/net/bridge/br.c index 96e91d69a9a8..e0e2df2fa278 100644 --- a/net/bridge/br.c +++ b/net/bridge/br.c @@ -165,8 +165,8 @@ static int br_switchdev_event(struct notifier_block *unused, switch (event) { case SWITCHDEV_FDB_ADD_TO_BRIDGE: fdb_info = ptr; - err = br_fdb_external_learn_add(br, p, fdb_info->addr, - fdb_info->vid, false); + err = br_fdb_external_learn_add(br, p, fdb_info->addr, fdb_info->vid, + fdb_info->locked, false); if (err) { err = notifier_from_errno(err); break; diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index 15ead4dc6190..8d207b1416f7 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c @@ -1145,7 +1145,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br, "FDB entry towards bridge must be permanent"); return -EINVAL; } - err = br_fdb_external_learn_add(br, p, addr, vid, true); + err = br_fdb_external_learn_add(br, p, addr, vid, false, true); } else { spin_lock_bh(&br->hash_lock); err = fdb_add_entry(br, p, addr, ndm, nlh_flags, vid, ext_flags, nfea_tb); @@ -1400,7 +1400,7 @@ void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p) } int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, - const unsigned char *addr, u16 vid, + const unsigned char *addr, u16 vid, bool locked, bool swdev_notify) { struct net_bridge_fdb_entry *fdb; @@ -1421,6 +1421,9 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, if (!p) flags |= BIT(BR_FDB_LOCAL); + if (locked) + flags |= BIT(BR_FDB_LOCKED); + fdb = fdb_create(br, p, addr, vid, flags); if (!fdb) { err = -ENOMEM; @@ -1444,6 +1447,11 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, modified = true; } + if (locked != test_bit(BR_FDB_LOCKED, &fdb->flags)) { + change_bit(BR_FDB_LOCKED, &fdb->flags); + modified = true; + } + if (swdev_notify) set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index e7a08657c7ed..3e9f4d1fbd60 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -812,7 +812,7 @@ int br_fdb_sync_static(struct net_bridge *br, struct net_bridge_port *p); void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p); int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, const unsigned char *addr, u16 vid, - bool swdev_notify); + bool locked, bool swdev_notify); int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p, const unsigned char *addr, u16 vid, bool swdev_notify); diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index 8f3d76c751dd..c6b938c01a74 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->locked = test_bit(BR_FDB_LOCKED, &fdb->flags); item->info.dev = (!p || item->is_local) ? br->dev : p->dev; item->info.ctx = ctx; }
The bridge will be able to install locked entries when receiving SWITCHDEV_FDB_ADD_TO_BRIDGE notifications from drivers. Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com> --- include/net/switchdev.h | 1 + net/bridge/br.c | 4 ++-- net/bridge/br_fdb.c | 12 ++++++++++-- net/bridge/br_private.h | 2 +- net/bridge/br_switchdev.c | 1 + 5 files changed, 15 insertions(+), 5 deletions(-)