mlxsw: Add 802.1X and MAB offload support
Subsystems (9)
Petr Machata
maintainercover mlxsw: Add 802.1X and MAB offload support
Ido Schimmel <[email protected]> writes:
This patchset adds 802.1X [1] and MAB [2] offload support in mlxsw.
Patches #1-#3 add the required switchdev interfaces.
Patches #4-#5 add the required packet traps for 802.1X.
Patches #6-#10 are small preparations in mlxsw.
Patch #11 adds locked bridge port support in mlxsw.
Patches #12-#15 add mlxsw selftests. The patchset was also tested with the generic forwarding selftest ('bridge_locked_port.sh').
[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=a21d9a670d81103db7f788de1a4a4a6e4b891a0b
[2] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=a35ec8e38cdd1766f29924ca391a01de20163931
Hans J. Schultz (1):
bridge: switchdev: Allow device drivers to install locked FDB entries
Ido Schimmel (14):
bridge: switchdev: Let device drivers determine FDB offload indication
bridge: switchdev: Reflect MAB bridge port flag to device drivers
devlink: Add packet traps for 802.1X operation
mlxsw: spectrum_trap: Register 802.1X packet traps with devlink
mlxsw: reg: Add Switch Port FDB Security Register
mlxsw: spectrum: Add an API to configure security checks
mlxsw: spectrum_switchdev: Prepare for locked FDB notifications
mlxsw: spectrum_switchdev: Add support for locked FDB notifications
mlxsw: spectrum_switchdev: Use extack in bridge port flag validation
mlxsw: spectrum_switchdev: Add locked bridge port support
selftests: devlink_lib: Split out helper
selftests: mlxsw: Add a test for EAPOL trap
selftests: mlxsw: Add a test for locked port trap
selftests: mlxsw: Add a test for invalid locked bridge port
configurations
.../networking/devlink/devlink-trap.rst | 13 +++ drivers/net/ethernet/mellanox/mlxsw/reg.h | 35 ++++++ .../net/ethernet/mellanox/mlxsw/spectrum.c | 22 ++++ .../net/ethernet/mellanox/mlxsw/spectrum.h | 5 +- .../mellanox/mlxsw/spectrum_switchdev.c | 64 +++++++++-- .../ethernet/mellanox/mlxsw/spectrum_trap.c | 25 +++++ drivers/net/ethernet/mellanox/mlxsw/trap.h | 2 + include/net/devlink.h | 9 ++ include/net/switchdev.h | 1 + net/bridge/br.c | 5 +- net/bridge/br_fdb.c | 22 +++- net/bridge/br_private.h | 2 +- net/bridge/br_switchdev.c | 6 +- net/core/devlink.c | 3 + .../drivers/net/mlxsw/devlink_trap_control.sh | 22 ++++ .../net/mlxsw/devlink_trap_l2_drops.sh | 105 ++++++++++++++++++ .../selftests/drivers/net/mlxsw/rtnetlink.sh | 31 ++++++ .../selftests/net/forwarding/devlink_lib.sh | 19 ++-- 18 files changed, 366 insertions(+), 25 deletions(-)
Signature · 1 line
2.35.3
Petr Machata
maintainerPATCH 1/15 bridge: switchdev: Let device drivers determine FDB offload indication
From: Ido Schimmel <[email protected]>
Currently, FDB entries that are notified to the bridge via 'SWITCHDEV_FDB_ADD_TO_BRIDGE' are always marked as offloaded. With MAB enabled, this will no longer be universally true. Device drivers will report locked FDB entries to the bridge to let it know that the corresponding hosts required authorization, but it does not mean that these entries are necessarily programmed in the underlying hardware.
Solve this by determining the offload indication based of the 'offloaded' bit in the FDB notification.
--- net/bridge/br.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/bridge/br.c +1 -1
@@ -172,7 +172,7 @@ static int br_switchdev_event(struct notifier_block *unused,break ; }br_fdb_offloaded_set (br, p, fdb_info->addr, - fdb_info->vid, true); + fdb_info->vid, fdb_info->offloaded);break ;case SWITCHDEV_FDB_DEL_TO_BRIDGE : fdb_info = ptr;
Signature · 1 line
2.35.3
Re: [PATCH net-next 01/15] bridge: switchdev: Let device drivers determine FDB offload indication
Quoted 9 lines · Petr Machata
From: Ido Schimmel <[email protected]>
Currently, FDB entries that are notified to the bridge via '
SWITCHDEV_FDB_ADD_TO_BRIDGE' are always marked as offloaded. With MAB enabled, this will no longer be universally true. Device drivers will report locked FDB entries to the bridge to let it know that the corresponding hosts required authorization, but it does not mean that these entries are necessarily programmed in the underlying hardware.
Quoted 7 lines
Solve this by determining the offload indication based of the 'offloaded' bit in the FDB notification.
Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>---
Re: [PATCH net-next 01/15] bridge: switchdev: Let device drivers determine FDB offload indication
In reply to Petr MachataQuoted 12 lines · Petr Machata
From: Ido Schimmel <[email protected]>
Currently, FDB entries that are notified to the bridge via '
SWITCHDEV_FDB_ADD_TO_BRIDGE' are always marked as offloaded. With MAB enabled, this will no longer be universally true. Device drivers will report locked FDB entries to the bridge to let it know that the corresponding hosts required authorization, but it does not mean that these entries are necessarily programmed in the underlying hardware.Solve this by determining the offload indication based of the 'offloaded' bit in the FDB notification.
Quoted 7 lines
Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>---
net/bridge/br.c| 2 +-1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines
net/bridge/br.c +1
-1@@ -172,7 +172,7 @@ static int br_switchdev_event(struct notifier_block *unused,break ; }br_fdb_offloaded_set (br, p, fdb_info->addr, - fdb_info->vid, true); + fdb_info->vid, fdb_info->offloaded);break ;case SWITCHDEV_FDB_DEL_TO_BRIDGE : fdb_info = ptr;
Petr Machata
maintainerPATCH 2/15 bridge: switchdev: Allow device drivers to install locked FDB entries
In reply to Petr MachataFrom: Hans J. Schultz <[email protected]>
When the bridge is offloaded to hardware, FDB entries are learned and aged-out by the hardware. Some device drivers synchronize the hardware and software FDBs by generating switchdev events towards the bridge.
When a port is locked, the hardware must not learn autonomously, as otherwise any host will blindly gain authorization. Instead, the hardware should generate events regarding hosts that are trying to gain authorization and their MAC addresses should be notified by the device driver as locked FDB entries towards the bridge driver.
Allow device drivers to notify the bridge driver about such entries by extending the 'switchdev_notifier_fdb_info' structure with the 'locked' bit. The bit can only be set by device drivers and not by the bridge driver.
Prevent a locked entry from being installed if MAB is not enabled on the bridge port.
If an entry already exists in the bridge driver, reject the locked entry if the current entry does not have the "locked" flag set or if it points to a different port. The same semantics are implemented in the software data path.
---
Notes:
v1:
* Adjust commit message.
* Add a check in br_switchdev_fdb_notify().
* Use 'false' instead of '0' in br_switchdev_fdb_populate().
Changes made by Ido:
* Reword commit message.
* Forbid locked entries when MAB is not enabled.
* Forbid roaming of locked entries.
* Avoid setting 'locked' bit towards device drivers.
include/net/switchdev.h | 1 + net/bridge/br.c | 3 ++- net/bridge/br_fdb.c | 22 ++++++++++++++++++++-- net/bridge/br_private.h | 2 +- net/bridge/br_switchdev.c | 4 ++++ 5 files changed, 28 insertions(+), 4 deletions(-)
include/net/switchdev.h +1 -0
@@ -248,6 +248,7 @@ struct switchdev_notifier_fdb_info {u16 vid; u8 added_by_user:1, is_local:1, + locked:1, offloaded:1; };
net/bridge/br.c +2 -1
@@ -166,7 +166,8 @@ static int br_switchdev_event(struct notifier_block *unused,case SWITCHDEV_FDB_ADD_TO_BRIDGE : fdb_info = ptr; err =br_fdb_external_learn_add (br, p, fdb_info->addr, - fdb_info->vid, false); + fdb_info->vid, + fdb_info->locked, false);if (err) { err =notifier_from_errno (err);break ;
net/bridge/br_fdb.c +20 -2
@@ -1139,7 +1139,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, nfea_tb);
@@ -1377,7 +1377,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;
@@ -1386,6 +1386,9 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,trace_br_fdb_external_learn_add (br, p, addr, vid); +if (locked && (!p || !(p->flags &BR_PORT_MAB ))) +return -EINVAL ; +spin_lock_bh (&br->hash_lock); fdb =br_fdb_find (br, addr, vid);
@@ -1398,6 +1401,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 ;
@@ -1405,6 +1411,13 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, }fdb_notify (br, fdb,RTM_NEWNEIGH , swdev_notify); }else { +if (locked && + (!test_bit (BR_FDB_LOCKED , &fdb->flags) || +READ_ONCE (fdb->dst) != p)) { + err = -EINVAL ; + goto err_unlock; + } + fdb->updated = jiffies;if (READ_ONCE (fdb->dst) != p) {
@@ -1421,6 +1434,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);
net/bridge/br_private.h +1 -1
@@ -811,7 +811,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);
net/bridge/br_switchdev.c +4 -0
@@ -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 = false; item->info.dev = (!p || item->is_local) ? br->dev : p->dev; item->info.ctx = ctx; }
@@ -146,6 +147,9 @@ br_switchdev_fdb_notify(struct net_bridge *br, {struct switchdev_notifier_fdb_info item; +if (test_bit (BR_FDB_LOCKED , &fdb->flags)) +return ; +br_switchdev_fdb_populate (br, &item, fdb,NULL );switch (type) {
Signature · 1 line
2.35.3
Re: [PATCH net-next 02/15] bridge: switchdev: Allow device drivers to install locked FDB entries
Quoted 29 lines · Petr Machata
From: Hans J. Schultz <[email protected]>
When the bridge is offloaded to hardware, FDB entries are learned and aged-out by the hardware. Some device drivers synchronize the hardware and software FDBs by generating switchdev events towards the bridge.
When a port is locked, the hardware must not learn autonomously, as otherwise any host will blindly gain authorization. Instead, the hardware should generate events regarding hosts that are trying to gain authorization and their MAC addresses should be notified by the device driver as locked FDB entries towards the bridge driver.
Allow device drivers to notify the bridge driver about such entries by extending the '
switchdev_notifier_fdb_info' structure with the 'locked' bit. The bit can only be set by device drivers and not by the bridge driver.Prevent a locked entry from being installed if MAB is not enabled on the bridge port.
If an entry already exists in the bridge driver, reject the locked entry if the current entry does not have the "locked" flag set or if it points to a different port. The same semantics are implemented in the software data path.
Signed-off-by Hans J. Schultz <[email protected]>Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>
Quoted 7 lines
---
Notes:
v1:
* Adjust commit message.
* Add a check inbr_switchdev_fdb_notify().
* Use 'false' instead of '0' inbr_switchdev_fdb_populate().
Thanks for making the changes.
(imagine this was my NXP email address, I'm not subscribed to netdev @work)
Re: [PATCH net-next 02/15] bridge: switchdev: Allow device drivers to install locked FDB entries
In reply to Petr MachataQuoted 43 lines · Petr Machata
From: Hans J. Schultz <[email protected]>
When the bridge is offloaded to hardware, FDB entries are learned and aged-out by the hardware. Some device drivers synchronize the hardware and software FDBs by generating switchdev events towards the bridge.
When a port is locked, the hardware must not learn autonomously, as otherwise any host will blindly gain authorization. Instead, the hardware should generate events regarding hosts that are trying to gain authorization and their MAC addresses should be notified by the device driver as locked FDB entries towards the bridge driver.
Allow device drivers to notify the bridge driver about such entries by extending the '
switchdev_notifier_fdb_info' structure with the 'locked' bit. The bit can only be set by device drivers and not by the bridge driver.Prevent a locked entry from being installed if MAB is not enabled on the bridge port.
If an entry already exists in the bridge driver, reject the locked entry if the current entry does not have the "locked" flag set or if it points to a different port. The same semantics are implemented in the software data path.
Signed-off-by Hans J. Schultz <[email protected]>Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>---
Notes:
v1:
* Adjust commit message.
* Add a check inbr_switchdev_fdb_notify().
* Use 'false' instead of '0' inbr_switchdev_fdb_populate().Changes made by Ido:
* Reword commit message.
* Forbid locked entries when MAB is not enabled.
* Forbid roaming of locked entries.
* Avoid setting 'locked' bit towards device drivers.
Quoted 7 lines
include/net/switchdev.h | 1 + net/bridge/br.c | 3 ++- net/bridge/br_fdb.c | 22 ++++++++++++++++++++-- net/bridge/br_private.h | 2 +- net/bridge/br_switchdev.c | 4 ++++ 5 files changed, 28 insertions(+), 4 deletions(-)
Quoted 129 lines
include/net/switchdev.h +1
-0@@ -248,6 +248,7 @@ struct switchdev_notifier_fdb_info {u16 vid; u8 added_by_user:1, is_local:1, + locked:1, offloaded:1; };net/bridge/br.c +2
-1@@ -166,7 +166,8 @@ static int br_switchdev_event(struct notifier_block *unused,case SWITCHDEV_FDB_ADD_TO_BRIDGE : fdb_info = ptr; err =br_fdb_external_learn_add (br, p, fdb_info->addr, - fdb_info->vid, false); + fdb_info->vid, + fdb_info->locked, false);if (err) { err =notifier_from_errno (err);break ;net/bridge/br_fdb.c +20
-2@@ -1139,7 +1139,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, nfea_tb);@@ -1377,7 +1377,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;@@ -1386,6 +1386,9 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,trace_br_fdb_external_learn_add (br, p, addr, vid); +if (locked && (!p || !(p->flags &BR_PORT_MAB ))) +return -EINVAL ; +spin_lock_bh (&br->hash_lock); fdb =br_fdb_find (br, addr, vid);@@ -1398,6 +1401,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 ;@@ -1405,6 +1411,13 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, }fdb_notify (br, fdb,RTM_NEWNEIGH , swdev_notify); }else { +if (locked && + (!test_bit (BR_FDB_LOCKED , &fdb->flags) || +READ_ONCE (fdb->dst) != p)) { + err = -EINVAL ; + goto err_unlock; + } + fdb->updated = jiffies;if (READ_ONCE (fdb->dst) != p) {@@ -1421,6 +1434,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);net/bridge/br_private.h +1
-1@@ -811,7 +811,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);net/bridge/br_switchdev.c +4
-0@@ -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 = false; item->info.dev = (!p || item->is_local) ? br->dev : p->dev; item->info.ctx = ctx; }@@ -146,6 +147,9 @@ br_switchdev_fdb_notify(struct net_bridge *br, {struct switchdev_notifier_fdb_info item; +if (test_bit (BR_FDB_LOCKED , &fdb->flags)) +return ; +br_switchdev_fdb_populate (br, &item, fdb,NULL );switch (type) {
Petr Machata
maintainerPATCH 3/15 bridge: switchdev: Reflect MAB bridge port flag to device drivers
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Reflect the 'BR_PORT_MAB' flag to device drivers so that:
* Drivers that support MAB could act upon the flag being toggled.
* Drivers that do not support MAB will prevent MAB from being enabled.
---
Notes:
v1:
* New patch.
net/bridge/br_switchdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/bridge/br_switchdev.c +1 -1
@@ -71,7 +71,7 @@ bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, }/* Flags that can be offloaded to hardware */ -#define BR_PORT_FLAGS_HW_OFFLOAD (BR_LEARNING | BR_FLOOD | \ +#define BR_PORT_FLAGS_HW_OFFLOAD (BR_LEARNING | BR_FLOOD | BR_PORT_MAB | \ BR_MCAST_FLOOD | BR_BCAST_FLOOD | BR_PORT_LOCKED | \ BR_HAIRPIN_MODE | BR_ISOLATED | BR_MULTICAST_TO_UNICAST)
Signature · 1 line
2.35.3
Re: [PATCH net-next 03/15] bridge: switchdev: Reflect MAB bridge port flag to device drivers
Quoted 11 lines · Petr Machata
From: Ido Schimmel <[email protected]>
Reflect the '
BR_PORT_MAB' flag to device drivers so that:* Drivers that support MAB could act upon the flag being toggled.
* Drivers that do not support MAB will prevent MAB from being enabled.Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>---
Re: [PATCH net-next 03/15] bridge: switchdev: Reflect MAB bridge port flag to device drivers
In reply to Petr MachataQuoted 10 lines · Petr Machata
From: Ido Schimmel <[email protected]>
Reflect the '
BR_PORT_MAB' flag to device drivers so that:* Drivers that support MAB could act upon the flag being toggled.
* Drivers that do not support MAB will prevent MAB from being enabled.Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>
Quoted 9 lines
---
Notes:
v1:
* New patch.
net/bridge/br_switchdev.c| 2 +-1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines
net/bridge/br_switchdev.c +1
-1@@ -71,7 +71,7 @@ bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, }/* Flags that can be offloaded to hardware */ -#define BR_PORT_FLAGS_HW_OFFLOAD (BR_LEARNING | BR_FLOOD | \ +#define BR_PORT_FLAGS_HW_OFFLOAD (BR_LEARNING | BR_FLOOD | BR_PORT_MAB | \ BR_MCAST_FLOOD | BR_BCAST_FLOOD | BR_PORT_LOCKED | \ BR_HAIRPIN_MODE | BR_ISOLATED | BR_MULTICAST_TO_UNICAST)
Petr Machata
maintainerPATCH 4/15 devlink: Add packet traps for 802.1X operation
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Add packet traps for 802.1X operation. The "eapol" control trap is used to trap EAPOL packets and is required for the correct operation of the control plane. The "locked_port" drop trap can be enabled to gain visibility into packets that were dropped by the device due to the locked bridge port check.
--- Documentation/networking/devlink/devlink-trap.rst | 13 +++++++++++++ include/net/devlink.h | 9 +++++++++ net/core/devlink.c | 3 +++ 3 files changed, 25 insertions(+)
Documentation/networking/devlink/devlink-trap.rst +13 -0
@@ -485,6 +485,16 @@ be added to the following table: - Traps incoming packets that the device decided to drop because the destination MAC is not configured in the MAC table and the interface is not in promiscuous mode + * - ``eapol`` + - ``control`` + - Traps "Extensible Authentication Protocol over LAN" (EAPOL) packets + specified in IEEE 802.1X + * - ``locked_port`` + - ``drop`` + - Traps packets that the device decided to drop because they failed the + locked bridge port check. That is, packets that were received via a + locked port and whose {SMAC, VID} does not correspond to an FDB entry + pointing to the port Driver-specific Packet Traps ============================
@@ -589,6 +599,9 @@ narrow. The description of these groups must be added to the following table: * - ``parser_error_drops`` - Contains packet traps for packets that were marked by the device during parsing as erroneous + * - ``eapol`` + - Contains packet traps for "Extensible Authentication Protocol over LAN" + (EAPOL) packets specified in IEEE 802.1X Packet Trap Policers ====================
include/net/devlink.h +9 -0
@@ -894,6 +894,8 @@ enum devlink_trap_generic_id {DEVLINK_TRAP_GENERIC_ID_ESP_PARSING ,DEVLINK_TRAP_GENERIC_ID_BLACKHOLE_NEXTHOP ,DEVLINK_TRAP_GENERIC_ID_DMAC_FILTER , +DEVLINK_TRAP_GENERIC_ID_EAPOL , +DEVLINK_TRAP_GENERIC_ID_LOCKED_PORT ,/* Add new generic trap IDs above */ __DEVLINK_TRAP_GENERIC_ID_MAX,
@@ -930,6 +932,7 @@ enum devlink_trap_group_generic_id {DEVLINK_TRAP_GROUP_GENERIC_ID_ACL_SAMPLE ,DEVLINK_TRAP_GROUP_GENERIC_ID_ACL_TRAP ,DEVLINK_TRAP_GROUP_GENERIC_ID_PARSER_ERROR_DROPS , +DEVLINK_TRAP_GROUP_GENERIC_ID_EAPOL ,/* Add new generic trap group IDs above */ __DEVLINK_TRAP_GROUP_GENERIC_ID_MAX,
@@ -1121,6 +1124,10 @@ enum devlink_trap_group_generic_id { "blackhole_nexthop"#define DEVLINK_TRAP_GENERIC_NAME_DMAC_FILTER \ "dmac_filter" +#define DEVLINK_TRAP_GENERIC_NAME_EAPOL \ + "eapol" +#define DEVLINK_TRAP_GENERIC_NAME_LOCKED_PORT \ + "locked_port"#define DEVLINK_TRAP_GROUP_GENERIC_NAME_L2_DROPS \ "l2_drops"
@@ -1174,6 +1181,8 @@ enum devlink_trap_group_generic_id { "acl_trap"#define DEVLINK_TRAP_GROUP_GENERIC_NAME_PARSER_ERROR_DROPS \ "parser_error_drops" +#define DEVLINK_TRAP_GROUP_GENERIC_NAME_EAPOL \ + "eapol"#define DEVLINK_TRAP_GENERIC (_type, _init_action, _id, _group_id, \ _metadata_cap) \
net/core/devlink.c +3 -0
@@ -11731,6 +11731,8 @@ static const struct devlink_trap devlink_trap_generic[] = {DEVLINK_TRAP (ESP_PARSING ,DROP ),DEVLINK_TRAP (BLACKHOLE_NEXTHOP ,DROP ),DEVLINK_TRAP (DMAC_FILTER ,DROP ), +DEVLINK_TRAP (EAPOL ,CONTROL ), + DEVLINK_TRAP(LOCKED_PORT, DROP), };#define DEVLINK_TRAP_GROUP (_id) \
@@ -11766,6 +11768,7 @@ static const struct devlink_trap_group devlink_trap_group_generic[] = {DEVLINK_TRAP_GROUP (ACL_SAMPLE ),DEVLINK_TRAP_GROUP (ACL_TRAP ),DEVLINK_TRAP_GROUP (PARSER_ERROR_DROPS ), + DEVLINK_TRAP_GROUP(EAPOL), };static int devlink_trap_generic_verify (const struct devlink_trap *trap)
Signature · 1 line
2.35.3
Re: [PATCH net-next 04/15] devlink: Add packet traps for 802.1X operation
Quoted 3 lines · Petr Machata
The "
locked_port" drop trap can be enabled to gain visibility into packets that were dropped by the device due to the locked bridge port check.
Pretty cool.
The action of all devlink DROP traps can be changed to e.g. CONTROL, right? This doesn't functionally affect an offloading driver, because what won't be dropped in hw on a locked port will still be dropped by the bridge, correct?
I'm not familiar with devlink traps. Is there another way to see the dropped packets except to do what I said (change the action type)?
Petr Machata
maintainerPATCH 5/15 mlxsw: spectrum_trap: Register 802.1X packet traps with devlink
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Register the previously added packet traps with devlink. This allows user space to tune their policers and in the case of the locked port trap, user space can set its action to "trap" in order to gain visibility into packets that were discarded by the device due to the locked port check failure.
--- drivers/net/ethernet/mellanox/mlxsw/reg.h | 1 + .../ethernet/mellanox/mlxsw/spectrum_trap.c | 25 +++++++++++++++++++ drivers/net/ethernet/mellanox/mlxsw/trap.h | 2 ++ 3 files changed, 28 insertions(+)
drivers/net/ethernet/mellanox/mlxsw/reg.h +1 -0
@@ -6316,6 +6316,7 @@ enum mlxsw_reg_htgt_trap_group {MLXSW_REG_HTGT_TRAP_GROUP_SP_TUNNEL_DISCARDS ,MLXSW_REG_HTGT_TRAP_GROUP_SP_ACL_DISCARDS ,MLXSW_REG_HTGT_TRAP_GROUP_SP_BUFFER_DISCARDS , +MLXSW_REG_HTGT_TRAP_GROUP_SP_EAPOL , __MLXSW_REG_HTGT_TRAP_GROUP_MAX,MLXSW_REG_HTGT_TRAP_GROUP_MAX = __MLXSW_REG_HTGT_TRAP_GROUP_MAX -1
drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c +25 -0
@@ -510,6 +510,9 @@ mlxsw_sp_trap_policer_items_arr[] = { { .policer = MLXSW_SP_TRAP_POLICER(20, 10240, 4096), }, + { + .policer = MLXSW_SP_TRAP_POLICER(21, 128, 128), + }, }; static const struct mlxsw_sp_trap_group_item mlxsw_sp_trap_group_items_arr[] = {
@@ -628,6 +631,11 @@ static const struct mlxsw_sp_trap_group_item mlxsw_sp_trap_group_items_arr[] = { .hw_group_id = MLXSW_REG_HTGT_TRAP_GROUP_SP_FLOW_LOGGING, .priority = 4, }, + { + .group = DEVLINK_TRAP_GROUP_GENERIC(EAPOL, 21), + .hw_group_id = MLXSW_REG_HTGT_TRAP_GROUP_SP_EAPOL, + .priority = 5, + }, };static const struct mlxsw_sp_trap_item mlxsw_sp_trap_items_arr[] = {
@@ -1160,6 +1168,23 @@ static const struct mlxsw_sp_trap_item mlxsw_sp_trap_items_arr[] = { MLXSW_SP_RXL_DISCARD(ROUTER3, L3_DISCARDS), }, }, + { + .trap = MLXSW_SP_TRAP_CONTROL(EAPOL, EAPOL, TRAP), + .listeners_arr = { + MLXSW_SP_RXL_NO_MARK(EAPOL, EAPOL, TRAP_TO_CPU, true), + }, + }, + { + .trap = MLXSW_SP_TRAP_DROP(LOCKED_PORT, L2_DROPS), + .listeners_arr = { + MLXSW_RXL_DIS(mlxsw_sp_rx_drop_listener, FDB_MISS, + TRAP_EXCEPTION_TO_CPU, false, + SP_L2_DISCARDS, DISCARD, SP_L2_DISCARDS), +MLXSW_RXL_DIS (mlxsw_sp_rx_drop_listener,FDB_MISMATCH , +TRAP_EXCEPTION_TO_CPU , false, + SP_L2_DISCARDS, DISCARD, SP_L2_DISCARDS), + }, + }, }; static struct mlxsw_sp_trap_policer_item *
drivers/net/ethernet/mellanox/mlxsw/trap.h +2 -0
@@ -25,6 +25,8 @@ enum {MLXSW_TRAP_ID_IGMP_V2_LEAVE =0x33 ,MLXSW_TRAP_ID_IGMP_V3_REPORT =0x34 ,MLXSW_TRAP_ID_PKT_SAMPLE =0x38 , +MLXSW_TRAP_ID_FDB_MISS =0x3A , +MLXSW_TRAP_ID_FDB_MISMATCH =0x3B ,MLXSW_TRAP_ID_FID_MISS =0x3D ,MLXSW_TRAP_ID_DECAP_ECN0 =0x40 , MLXSW_TRAP_ID_MTUERROR = 0x52,
Signature · 1 line
2.35.3
Petr Machata
maintainerPATCH 6/15 mlxsw: reg: Add Switch Port FDB Security Register
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Add the Switch Port FDB Security Register (SPFSR) that allows enabling and disabling security checks on a given local port. In Linux terms, it allows locking / unlocking a port.
--- drivers/net/ethernet/mellanox/mlxsw/reg.h | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+)
drivers/net/ethernet/mellanox/mlxsw/reg.h +34 -0
@@ -2046,6 +2046,39 @@ static inline void mlxsw_reg_spvmlr_pack(char *payload, u16 local_port, } } +/* SPFSR - Switch Port FDB Security Register +* ----------------------------------------- +* Configures the security mode per port. +*/ +#define MLXSW_REG_SPFSR_ID 0x2023 +#define MLXSW_REG_SPFSR_LEN 0x08 + +MLXSW_REG_DEFINE (spfsr,MLXSW_REG_SPFSR_ID ,MLXSW_REG_SPFSR_LEN ); + +/* reg_spfsr_local_port +* Local port. +* Access: Index +* +* Note: not supported for CPU port. +*/ +MLXSW_ITEM32_LP (reg, spfsr,0x00 ,16 ,0x00 ,12 ); + +/* reg_spfsr_security +* Security checks. +* 0: disabled (default) +* 1: enabled +* Access: RW +*/ +MLXSW_ITEM32 (reg, spfsr, security,0x04 ,31 ,1 ); + +static inline void mlxsw_reg_spfsr_pack (char *payload,u16 local_port, +bool security) +{ +MLXSW_REG_ZERO (spfsr, payload); +mlxsw_reg_spfsr_local_port_set (payload, local_port); +mlxsw_reg_spfsr_security_set (payload, security); +} +/* SPVC - Switch Port VLAN Classification Register * ----------------------------------------------- * Configures the port to identify packets as untagged / single tagged /
@@ -12762,6 +12795,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {MLXSW_REG (svpe),MLXSW_REG (sfmr),MLXSW_REG (spvmlr), +MLXSW_REG (spfsr),MLXSW_REG (spvc),MLXSW_REG (spevet), MLXSW_REG(smpe),
Signature · 1 line
2.35.3
Petr Machata
maintainerPATCH 7/15 mlxsw: spectrum: Add an API to configure security checks
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Add an API to enable or disable security checks on a local port. It will be used by subsequent patches when the 'BR_PORT_LOCKED' flag is toggled.
--- drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 18 ++++++++++++++++++ drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 5 ++++- 2 files changed, 22 insertions(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum.c +18 -0
@@ -466,6 +466,24 @@ int mlxsw_sp_port_vid_learning_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 vid,return err; } +int mlxsw_sp_port_security_set (struct mlxsw_sp_port *mlxsw_sp_port,bool enable) +{ +struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp; +char spfsr_pl[MLXSW_REG_SPFSR_LEN ]; +int err; + +if (mlxsw_sp_port->security == enable) +return 0 ; + +mlxsw_reg_spfsr_pack (spfsr_pl, mlxsw_sp_port->local_port, enable); + err =mlxsw_reg_write (mlxsw_sp->core,MLXSW_REG (spfsr), spfsr_pl); +if (err) +return err; + + mlxsw_sp_port->security = enable; +return 0 ; +} +int mlxsw_sp_ethtype_to_sver_type (u16 ethtype,u8 *p_sver_type) {switch (ethtype) {
drivers/net/ethernet/mellanox/mlxsw/spectrum.h +4 -1
@@ -321,7 +321,8 @@ struct mlxsw_sp_port {struct mlxsw_sp *mlxsw_sp;u16 local_port; u8 lagged:1, - split:1; + split:1, + security:1;u16 pvid;u16 lag_id;struct {
@@ -687,6 +688,8 @@ int mlxsw_sp_port_vid_stp_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 vid,int mlxsw_sp_port_vp_mode_set (struct mlxsw_sp_port *mlxsw_sp_port,bool enable);int mlxsw_sp_port_vid_learning_set (struct mlxsw_sp_port *mlxsw_sp_port,u16 vid,bool learn_enable); +int mlxsw_sp_port_security_set (struct mlxsw_sp_port *mlxsw_sp_port, +bool enable);int mlxsw_sp_ethtype_to_sver_type (u16 ethtype,u8 *p_sver_type);int mlxsw_sp_port_egress_ethtype_set (struct mlxsw_sp_port *mlxsw_sp_port,u16 ethtype);
Signature · 1 line
2.35.3
Petr Machata
maintainerPATCH 8/15 mlxsw: spectrum_switchdev: Prepare for locked FDB notifications
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Subsequent patches will need to report locked FDB entries to the bridge driver. Prepare for that by adding a 'locked' argument to mlxsw_sp_fdb_call_notifiers() according to which the 'locked' bit is set in the FDB notification info. For now, always pass 'false'.
--- .../mellanox/mlxsw/spectrum_switchdev.c | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +13 -8
@@ -2888,13 +2888,14 @@ static void mlxsw_sp_fdb_nve_call_notifiers(struct net_device *dev,static void mlxsw_sp_fdb_call_notifiers (enum switchdev_notifier_type type,const char *mac,u16 vid, -struct net_device *dev,bool offloaded) +struct net_device *dev,bool offloaded,bool locked) {struct switchdev_notifier_fdb_info info = {}; info.addr = mac; info.vid = vid; info.offloaded = offloaded; + info.locked = locked;call_switchdev_notifiers (type, dev, &info.info,NULL ); }
@@ -2952,7 +2953,8 @@ static void mlxsw_sp_fdb_notify_mac_process(struct mlxsw_sp *mlxsw_sp,if (!do_notification)return ; type = adding ?SWITCHDEV_FDB_ADD_TO_BRIDGE :SWITCHDEV_FDB_DEL_TO_BRIDGE ; -mlxsw_sp_fdb_call_notifiers (type, mac, vid, bridge_port->dev, adding); +mlxsw_sp_fdb_call_notifiers (type, mac, vid, bridge_port->dev, adding, + false);return ;
@@ -3015,7 +3017,8 @@ static void mlxsw_sp_fdb_notify_mac_lag_process(struct mlxsw_sp *mlxsw_sp,if (!do_notification)return ; type = adding ?SWITCHDEV_FDB_ADD_TO_BRIDGE :SWITCHDEV_FDB_DEL_TO_BRIDGE ; -mlxsw_sp_fdb_call_notifiers (type, mac, vid, bridge_port->dev, adding); +mlxsw_sp_fdb_call_notifiers (type, mac, vid, bridge_port->dev, adding, + false);return ;
@@ -3122,7 +3125,7 @@ static void mlxsw_sp_fdb_notify_mac_uc_tunnel_process(struct mlxsw_sp *mlxsw_sp, type = adding ?SWITCHDEV_FDB_ADD_TO_BRIDGE :SWITCHDEV_FDB_DEL_TO_BRIDGE ; -mlxsw_sp_fdb_call_notifiers (type, mac, vid, nve_dev, adding); +mlxsw_sp_fdb_call_notifiers (type, mac, vid, nve_dev, adding, false);mlxsw_sp_fid_put (fid);
@@ -3264,7 +3267,7 @@ mlxsw_sp_switchdev_bridge_vxlan_fdb_event(struct mlxsw_sp *mlxsw_sp, &vxlan_fdb_info.info, NULL);mlxsw_sp_fdb_call_notifiers (SWITCHDEV_FDB_OFFLOADED , vxlan_fdb_info.eth_addr, - fdb_info->vid, dev, true); + fdb_info->vid, dev, true, false);break ;case SWITCHDEV_FDB_DEL_TO_DEVICE : err = mlxsw_sp_port_fdb_tunnel_uc_op(mlxsw_sp,
@@ -3359,7 +3362,7 @@ static void mlxsw_sp_switchdev_bridge_fdb_event_work(struct work_struct *work)break ;mlxsw_sp_fdb_call_notifiers (SWITCHDEV_FDB_OFFLOADED , fdb_info->addr, - fdb_info->vid, dev, true); + fdb_info->vid, dev, true, false);break ;case SWITCHDEV_FDB_DEL_TO_DEVICE : fdb_info = &switchdev_work->fdb_info;
@@ -3443,7 +3446,8 @@ mlxsw_sp_switchdev_vxlan_fdb_add(struct mlxsw_sp *mlxsw_sp,call_switchdev_notifiers (SWITCHDEV_VXLAN_FDB_OFFLOADED , dev, &vxlan_fdb_info->info,NULL );mlxsw_sp_fdb_call_notifiers (SWITCHDEV_FDB_OFFLOADED , - vxlan_fdb_info->eth_addr, vid, dev, true); + vxlan_fdb_info->eth_addr, vid, dev, true, + false);mlxsw_sp_fid_put (fid);
@@ -3493,7 +3497,8 @@ mlxsw_sp_switchdev_vxlan_fdb_del(struct mlxsw_sp *mlxsw_sp, false, false); vid = bridge_device->ops->fid_vid (bridge_device, fid);mlxsw_sp_fdb_call_notifiers (SWITCHDEV_FDB_OFFLOADED , - vxlan_fdb_info->eth_addr, vid, dev, false); + vxlan_fdb_info->eth_addr, vid, dev, false, + false);mlxsw_sp_fid_put (fid); }
Signature · 1 line
2.35.3
Petr Machata
maintainerPATCH 9/15 mlxsw: spectrum_switchdev: Add support for locked FDB notifications
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
In Spectrum, learning happens in parallel to the security checks. Therefore, regardless of the result of the security checks, a learning notification will be generated by the device and polled later on by the driver.
Currently, the driver reacts to learning notifications by programming corresponding FDB entries to the device. When a port is locked (i.e., has security checks enabled), this can no longer happen, as otherwise any host will blindly gain authorization.
Instead, notify the learned entry as a locked entry to the bridge driver that will in turn notify it to user space, in case MAB is enabled. User space can then decide to authorize the host by clearing the "locked" flag, which will cause the entry to be programmed to the device.
--- .../net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +12 -0
@@ -2942,6 +2942,12 @@ static void mlxsw_sp_fdb_notify_mac_process(struct mlxsw_sp *mlxsw_sp, vid = bridge_device->vlan_enabled ? mlxsw_sp_port_vlan->vid :0 ; evid = mlxsw_sp_port_vlan->vid; +if (adding && mlxsw_sp_port->security) { +mlxsw_sp_fdb_call_notifiers (SWITCHDEV_FDB_ADD_TO_BRIDGE , mac, + vid, bridge_port->dev, false, true); +return ; + } + do_fdb_op: err =mlxsw_sp_port_fdb_uc_op (mlxsw_sp, local_port, mac, fid, evid, adding, true);
@@ -3006,6 +3012,12 @@ static void mlxsw_sp_fdb_notify_mac_lag_process(struct mlxsw_sp *mlxsw_sp, vid = bridge_device->vlan_enabled ? mlxsw_sp_port_vlan->vid :0 ; lag_vid = mlxsw_sp_port_vlan->vid; +if (adding && mlxsw_sp_port->security) { +mlxsw_sp_fdb_call_notifiers (SWITCHDEV_FDB_ADD_TO_BRIDGE , mac, + vid, bridge_port->dev, false, true); +return ; + } + do_fdb_op: err =mlxsw_sp_port_fdb_uc_lag_op (mlxsw_sp, lag_id, mac, fid, lag_vid, adding, true);
Signature · 1 line
2.35.3
Petr Machata
maintainerPATCH 10/15 mlxsw: spectrum_switchdev: Use extack in bridge port flag validation
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Propagate extack to mlxsw_sp_port_attr_br_pre_flags_set() in order to communicate error messages related to bridge port flag validation.
Example:
# bridge link set dev swp1 locked on Error: mlxsw_spectrum: Unsupported bridge port flag.
More error messages will be added in subsequent patches.
--- .../net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +7 -3
@@ -782,10 +782,13 @@ mlxsw_sp_bridge_port_learning_set(struct mlxsw_sp_port *mlxsw_sp_port,static int mlxsw_sp_port_attr_br_pre_flags_set (struct mlxsw_sp_port *mlxsw_sp_port, -struct switchdev_brport_flags flags) +struct switchdev_brport_flags flags, +struct netlink_ext_ack *extack) { -if (flags.mask & ~(BR_LEARNING |BR_FLOOD |BR_MCAST_FLOOD )) +if (flags.mask & ~(BR_LEARNING |BR_FLOOD |BR_MCAST_FLOOD )) { +NL_SET_ERR_MSG_MOD (extack,"Unsupported bridge port flag" );return -EINVAL ; + }return 0 ; }
@@ -1186,7 +1189,8 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev, const void *ctx,break ;case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS : err =mlxsw_sp_port_attr_br_pre_flags_set (mlxsw_sp_port, - attr->u.brport_flags); + attr->u.brport_flags, + extack);break ;case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS : err = mlxsw_sp_port_attr_br_flags_set(mlxsw_sp_port,
Signature · 1 line
2.35.3
Petr Machata
maintainerPATCH 11/15 mlxsw: spectrum_switchdev: Add locked bridge port support
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Add locked bridge port support by reacting to changes in the 'BR_PORT_LOCKED' flag. When set, enable security checks on the local port via the previously added SPFSR register.
When security checks are enabled, an incoming packet will trigger an FDB lookup with the packet's source MAC and the FID it was classified to. If an FDB entry was not found or was found to be pointing to a different port, the packet will be dropped. Such packets increment the "discard_ingress_general" ethtool counter. For added visibility, user space can trap such packets to the CPU by enabling the "locked_port" trap. Example:
# devlink trap set pci/0000:06:00.0 trap locked_port action trap
Unlike other configurations done via bridge port flags (e.g., learning, flooding), security checks are enabled in the device on a per-port basis and not on a per-{port, VLAN} basis. As such, scenarios where user space can configure different locking settings for different VLANs configured on a port need to be vetoed. To that end, veto the following scenarios:
1. Locking is set on a bridge port that is a VLAN upper
2. Locking is set on a bridge port that has VLAN uppers
3. VLAN upper is configured on a locked bridge port
Examples:
# bridge link set dev swp1.10 locked on Error: mlxsw_spectrum: Locked flag cannot be set on a VLAN upper.
# ip link add link swp1 name swp1.10 type vlan id 10 # bridge link set dev swp1 locked on Error: mlxsw_spectrum: Locked flag cannot be set on a bridge port that has VLAN uppers.
# bridge link set dev swp1 locked on # ip link add link swp1 name swp1.10 type vlan id 10 Error: mlxsw_spectrum: VLAN uppers are not supported on a locked port.
---
Notes:
v1:
* Add 'BR_PORT_MAB' in mlxsw_sp_port_attr_br_pre_flags_set().
.../net/ethernet/mellanox/mlxsw/spectrum.c | 4 ++++ .../mellanox/mlxsw/spectrum_switchdev.c | 23 ++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlxsw/spectrum.c +4 -0
@@ -4760,6 +4760,10 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev,NL_SET_ERR_MSG_MOD (extack,"VLAN uppers are only supported with 802.1q VLAN protocol" );return -EOPNOTSUPP ; } +if (is_vlan_dev (upper_dev) && mlxsw_sp_port->security) { +NL_SET_ERR_MSG_MOD (extack,"VLAN uppers are not supported on a locked port" ); +return -EOPNOTSUPP ; + }break ;case NETDEV_CHANGEUPPER : upper_dev = info->upper_dev;
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +22 -1
@@ -782,14 +782,26 @@ mlxsw_sp_bridge_port_learning_set(struct mlxsw_sp_port *mlxsw_sp_port,static int mlxsw_sp_port_attr_br_pre_flags_set (struct mlxsw_sp_port *mlxsw_sp_port, +const struct net_device *orig_dev,struct switchdev_brport_flags flags,struct netlink_ext_ack *extack) { -if (flags.mask & ~(BR_LEARNING |BR_FLOOD |BR_MCAST_FLOOD )) { +if (flags.mask & ~(BR_LEARNING |BR_FLOOD |BR_MCAST_FLOOD | +BR_PORT_LOCKED |BR_PORT_MAB )) {NL_SET_ERR_MSG_MOD (extack,"Unsupported bridge port flag" );return -EINVAL ; } +if ((flags.mask &BR_PORT_LOCKED ) &&is_vlan_dev (orig_dev)) { +NL_SET_ERR_MSG_MOD (extack,"Locked flag cannot be set on a VLAN upper" ); +return -EINVAL ; + } + +if ((flags.mask &BR_PORT_LOCKED ) &&vlan_uses_dev (orig_dev)) { +NL_SET_ERR_MSG_MOD (extack,"Locked flag cannot be set on a bridge port that has VLAN uppers" ); +return -EINVAL ; + } +return 0 ; }
@@ -822,6 +834,13 @@ static int mlxsw_sp_port_attr_br_flags_set(struct mlxsw_sp_port *mlxsw_sp_port,return err; } +if (flags.mask &BR_PORT_LOCKED ) { + err =mlxsw_sp_port_security_set (mlxsw_sp_port, + flags.val &BR_PORT_LOCKED ); +if (err) +return err; + } +if (bridge_port->bridge_device->multicast_enabled) goto out;
@@ -1189,6 +1208,7 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev, const void *ctx,break ;case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS : err =mlxsw_sp_port_attr_br_pre_flags_set (mlxsw_sp_port, + attr->orig_dev, attr->u.brport_flags, extack);break ;
@@ -2787,6 +2807,7 @@ void mlxsw_sp_port_bridge_leave(struct mlxsw_sp_port *mlxsw_sp_port, bridge_device->ops->port_leave (bridge_device, bridge_port, mlxsw_sp_port); +mlxsw_sp_port_security_set (mlxsw_sp_port, false);mlxsw_sp_bridge_port_put (mlxsw_sp->bridge, bridge_port); }
Signature · 1 line
2.35.3
Re: [PATCH net-next 11/15] mlxsw: spectrum_switchdev: Add locked bridge port support
Quoted 14 lines · Petr Machata
From: Ido Schimmel <[email protected]>
Add locked bridge port support by reacting to changes in the '
BR_PORT_LOCKED' flag. When set, enable security checks on the local port via the previously added SPFSR register.When security checks are enabled, an incoming packet will trigger an FDB lookup with the packet's source MAC and the FID it was classified to. If an FDB entry was not found or was found to be pointing to a different port, the packet will be dropped. Such packets increment the "
discard_ingress_general" ethtool counter. For added visibility, user space can trap such packets to the CPU by enabling the "locked_port" trap. Example:
Quoted 1 line
# devlink trap set pci/0000:06:00.0 trap locked_port action trap
Got the answer I was looking for.
Quoted 22 lines
Unlike other configurations done via bridge port flags (e.g., learning, flooding), security checks are enabled in the device on a per-port basis and not on a per-{port, VLAN} basis. As such, scenarios where user space can configure different locking settings for different VLANs configured on a port need to be vetoed. To that end, veto the following scenarios:
1. Locking is set on a bridge port that is a VLAN upper
2. Locking is set on a bridge port that has VLAN uppers
3. VLAN upper is configured on a locked bridge port
Examples:
# bridge link set dev swp1.10 locked on Error: mlxsw_spectrum: Locked flag cannot be set on a VLAN upper.# ip link add link swp1 name swp1.10 type vlan id 10 # bridge link set dev swp1 locked on Error: mlxsw_spectrum: Locked flag cannot be set on a bridge port that has VLAN uppers.
Quoted 8 lines
# bridge link set dev swp1 locked on # ip link add link swp1 name swp1.10 type vlan id 10 Error: mlxsw_spectrum: VLAN uppers are not supported on a locked port.Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>---
Can't really figure out from the patch, sorry. Port security works with LAG offload?
Ido Schimmel
maintainerRe: [PATCH net-next 11/15] mlxsw: spectrum_switchdev: Add locked bridge port support
Quoted 2 lines · Vladimir Oltean
Can't really figure out from the patch, sorry. Port security works with LAG offload?
Yes. It's just that port security needs to be enabled on each of the member ports. FDB entries that point to a LAG are programmed with a lag_id. When a packet is received from a LAG the hardware will compare source_lag_id == lag_id instead of rx_local_port == tx_local_port.
Re: [PATCH net-next 11/15] mlxsw: spectrum_switchdev: Add locked bridge port support
Quoted 8 lines · Ido Schimmel
Quoted 2 lines · Vladimir Oltean
Can't really figure out from the patch, sorry. Port security works with LAG offload?
Yes. It's just that port security needs to be enabled on each of the member ports. FDB entries that point to a LAG are programmed with a
lag_id. When a packet is received from a LAG the hardware will comparesource_lag_id==lag_idinstead ofrx_local_port==tx_local_port.
Okay, understood, the concepts are clear.
Petr Machata
maintainerPATCH 12/15 selftests: devlink_lib: Split out helper
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Merely checking whether a trap counter incremented or not without logging a test result is useful on its own. Split this functionality to a helper which will be used by subsequent patches.
--- .../selftests/net/forwarding/devlink_lib.sh | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
tools/testing/selftests/net/forwarding/devlink_lib.sh +12 -7
@@ -503,25 +503,30 @@ devlink_trap_drop_cleanup() tc filter del dev $dev egress protocol $proto pref $pref handle $handle flower } -devlink_trap_stats_test() +devlink_trap_stats_check() { - local test_name=$1; shift local trap_name=$1; shift local send_one="$@" local t0_packets local t1_packets - RET=0 - t0_packets=$(devlink_trap_rx_packets_get $trap_name) $send_one && sleep 1 t1_packets=$(devlink_trap_rx_packets_get $trap_name) - if [[ $t1_packets -eq $t0_packets ]]; then - check_err 1 "Trap stats did not increase" - fi + [[ $t1_packets -ne $t0_packets ]] +} + +devlink_trap_stats_test() +{ + local test_name=$1; shift + + RET=0 + + devlink_trap_stats_check "$@" + check_err $? "Trap stats did not increase" log_test "$test_name" }
Signature · 1 line
2.35.3
Petr Machata
maintainerPATCH 13/15 selftests: mlxsw: Add a test for EAPOL trap
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Test that packets with a destination MAC of 01:80:C2:00:00:03 trigger the "eapol" packet trap.
--- .../drivers/net/mlxsw/devlink_trap_control.sh | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+)
tools/testing/selftests/drivers/net/mlxsw/devlink_trap_control.sh +22 -0
@@ -83,6 +83,7 @@ ALL_TESTS=" ptp_general_test flow_action_sample_test flow_action_trap_test + eapol_test " NUM_NETIFS=4 source $lib_dir/lib.sh
@@ -677,6 +678,27 @@ flow_action_trap_test() tc qdisc del dev $rp1 clsact } +eapol_payload_get() +{ + local source_mac=$1; shift + local p + + p=$(: + )"01:80:C2:00:00:03:"$( : ETH daddr + )"$source_mac:"$( : ETH saddr + )"88:8E:"$( : ETH type + ) + echo $p +} + +eapol_test() +{ + local h1mac=$(mac_get $h1) + + devlink_trap_stats_test "EAPOL" "eapol" $MZ $h1 -c 1 \ + $(eapol_payload_get $h1mac) -p 100 -q +} + trap cleanup EXIT setup_prepare
Signature · 1 line
2.35.3
Re: [PATCH net-next 13/15] selftests: mlxsw: Add a test for EAPOL trap
Quoted 9 lines · Petr Machata
From: Ido Schimmel <[email protected]>
Test that packets with a destination MAC of 01:80:C2:00:00:03 trigger the "eapol" packet trap.
Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>---
Petr Machata
maintainerPATCH 14/15 selftests: mlxsw: Add a test for locked port trap
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Test that packets received via a locked bridge port whose {SMAC, VID} does not appear in the bridge's FDB or appears with a different port, trigger the "locked_port" packet trap.
--- .../net/mlxsw/devlink_trap_l2_drops.sh | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+)
tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh +105 -0
@@ -14,6 +14,7 @@ ALL_TESTS=" ingress_stp_filter_test port_list_is_empty_test port_loopback_filter_test + locked_port_test " NUM_NETIFS=4 source $lib_dir/tc_common.sh
@@ -420,6 +421,110 @@ port_loopback_filter_test() port_loopback_filter_uc_test } +locked_port_miss_test() +{ + local trap_name="locked_port" + local smac=00:11:22:33:44:55 + + bridge link set dev $swp1 learning off + bridge link set dev $swp1 locked on + + RET=0 + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_fail $? "Trap stats increased before setting action to \"trap\"" + + devlink_trap_action_set $trap_name "trap" + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_err $? "Trap stats did not increase when should" + + devlink_trap_action_set $trap_name "drop" + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_fail $? "Trap stats increased after setting action to \"drop\"" + + devlink_trap_action_set $trap_name "trap" + + bridge fdb replace $smac dev $swp1 master static vlan 1 + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_fail $? "Trap stats increased after adding an FDB entry" + + bridge fdb del $smac dev $swp1 master static vlan 1 + bridge link set dev $swp1 locked off + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_fail $? "Trap stats increased after unlocking port" + + log_test "Locked port - FDB miss" + + devlink_trap_action_set $trap_name "drop" + bridge link set dev $swp1 learning on +} + +locked_port_mismatch_test() +{ + local trap_name="locked_port" + local smac=00:11:22:33:44:55 + + bridge link set dev $swp1 learning off + bridge link set dev $swp1 locked on + + RET=0 + + bridge fdb replace $smac dev $swp2 master static vlan 1 + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_fail $? "Trap stats increased before setting action to \"trap\"" + + devlink_trap_action_set $trap_name "trap" + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_err $? "Trap stats did not increase when should" + + devlink_trap_action_set $trap_name "drop" + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_fail $? "Trap stats increased after setting action to \"drop\"" + + devlink_trap_action_set $trap_name "trap" + bridge link set dev $swp1 locked off + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_fail $? "Trap stats increased after unlocking port" + + bridge link set dev $swp1 locked on + bridge fdb replace $smac dev $swp1 master static vlan 1 + + devlink_trap_stats_check $trap_name $MZ $h1 -c 1 \ + -a $smac -b $(mac_get $h2) -A 192.0.2.1 -B 192.0.2.2 -p 100 -q + check_fail $? "Trap stats increased after replacing an FDB entry" + + bridge fdb del $smac dev $swp1 master static vlan 1 + devlink_trap_action_set $trap_name "drop" + + log_test "Locked port - FDB mismatch" + + bridge link set dev $swp1 locked off + bridge link set dev $swp1 learning on +} + +locked_port_test() +{ + locked_port_miss_test + locked_port_mismatch_test +} + trap cleanup EXIT setup_prepare
Signature · 1 line
2.35.3
Re: [PATCH net-next 14/15] selftests: mlxsw: Add a test for locked port trap
Quoted 10 lines · Petr Machata
From: Ido Schimmel <[email protected]>
Test that packets received via a locked bridge port whose {SMAC, VID} does not appear in the bridge's FDB or appears with a different port, trigger the "
locked_port" packet trap.Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>---
Pretty impressively designed. I liked the extensive checks (for example that traps stop counting after port security is disabled).
Petr Machata
maintainerPATCH 15/15 selftests: mlxsw: Add a test for invalid locked bridge port configurations
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
Test that locked bridge port configurations that are not supported by mlxsw are rejected.
--- .../selftests/drivers/net/mlxsw/rtnetlink.sh | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+)
tools/testing/selftests/drivers/net/mlxsw/rtnetlink.sh +31 -0
@@ -34,6 +34,7 @@ ALL_TESTS=" nexthop_obj_bucket_offload_test nexthop_obj_blackhole_offload_test nexthop_obj_route_offload_test + bridge_locked_port_test devlink_reload_test " NUM_NETIFS=2
@@ -917,6 +918,36 @@ nexthop_obj_route_offload_test() simple_if_fini $swp1 192.0.2.1/24 2001:db8:1::1/64 } +bridge_locked_port_test() +{ + RET=0 + + ip link add name br1 up type bridge vlan_filtering 0 + + ip link add link $swp1 name $swp1.10 type vlan id 10 + ip link set dev $swp1.10 master br1 + + bridge link set dev $swp1.10 locked on + check_fail $? "managed to set locked flag on a VLAN upper" + + ip link set dev $swp1.10 nomaster + ip link set dev $swp1 master br1 + + bridge link set dev $swp1 locked on + check_fail $? "managed to set locked flag on a bridge port that has a VLAN upper" + + ip link del dev $swp1.10 + bridge link set dev $swp1 locked on + + ip link add link $swp1 name $swp1.10 type vlan id 10 + check_fail $? "managed to configure a VLAN upper on a locked port" + + log_test "bridge locked port" + + ip link del dev $swp1.10 &> /dev/null + ip link del dev br1 +} + devlink_reload_test() { # Test that after executing all the above configuration tests, a
Signature · 1 line
2.35.3
Re: [PATCH net-next 15/15] selftests: mlxsw: Add a test for invalid locked bridge port configurations
Quoted 9 lines · Petr Machata
From: Ido Schimmel <[email protected]>
Test that locked bridge port configurations that are not supported by mlxsw are rejected.
Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>---
Ido Schimmel
maintainer+ Vladimir
You weren't copied on the patches by mistake. They are available here:
https://lore.kernel.org/netdev/[email protected]/
Quoted 61 lines · Petr Machata
Ido Schimmel <[email protected]> writes:
This patchset adds 802.1X [1] and MAB [2] offload support in mlxsw.
Patches #1-#3 add the required switchdev interfaces.
Patches #4-#5 add the required packet traps for 802.1X.
Patches #6-#10 are small preparations in mlxsw.
Patch #11 adds locked bridge port support in mlxsw.
Patches #12-#15 add mlxsw selftests. The patchset was also tested with the generic forwarding selftest ('
bridge_locked_port.sh').[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=a21d9a670d81103db7f788de1a4a4a6e4b891a0b
[2] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=a35ec8e38cdd1766f29924ca391a01de20163931Hans J. Schultz (1):
bridge: switchdev: Allow device drivers to install locked FDB entriesIdo Schimmel (14):
bridge: switchdev: Let device drivers determine FDB offload indication
bridge: switchdev: Reflect MAB bridge port flag to device drivers
devlink: Add packet traps for 802.1X operation
mlxsw:spectrum_trap: Register 802.1X packet traps with devlink
mlxsw: reg: Add Switch Port FDB Security Register
mlxsw: spectrum: Add an API to configure security checks
mlxsw:spectrum_switchdev: Prepare for locked FDB notifications
mlxsw:spectrum_switchdev: Add support for locked FDB notifications
mlxsw:spectrum_switchdev: Use extack in bridge port flag validation
mlxsw:spectrum_switchdev: Add locked bridge port support
selftests:devlink_lib: Split out helper
selftests: mlxsw: Add a test for EAPOL trap
selftests: mlxsw: Add a test for locked port trap
selftests: mlxsw: Add a test for invalid locked bridge port
configurations.../networking/devlink/devlink-trap.rst | 13 +++ drivers/net/ethernet/mellanox/mlxsw/reg.h | 35 ++++++ .../net/ethernet/mellanox/mlxsw/spectrum.c | 22 ++++ .../net/ethernet/mellanox/mlxsw/spectrum.h | 5 +- .../mellanox/mlxsw/spectrum_switchdev.c | 64 +++++++++-- .../ethernet/mellanox/mlxsw/spectrum_trap.c | 25 +++++ drivers/net/ethernet/mellanox/mlxsw/trap.h | 2 + include/net/devlink.h | 9 ++ include/net/switchdev.h | 1 + net/bridge/br.c | 5 +- net/bridge/br_fdb.c | 22 +++- net/bridge/br_private.h | 2 +- net/bridge/br_switchdev.c | 6 +- net/core/devlink.c | 3 + .../drivers/net/mlxsw/devlink_trap_control.sh | 22 ++++ .../net/mlxsw/devlink_trap_l2_drops.sh | 105 ++++++++++++++++++ .../selftests/drivers/net/mlxsw/rtnetlink.sh | 31 ++++++ .../selftests/net/forwarding/devlink_lib.sh | 19 ++-- 18 files changed, 366 insertions(+), 25 deletions(-)Signature · 1 line
2.35.3
Quoted 4 lines · Ido Schimmel
+ Vladimir
You weren't copied on the patches by mistake. They are available here:
https://lore.kernel.org/netdev/[email protected]/
Thanks for copying me. The patches look great to my eyes. I didn't go into details into the mlxsw details, just because I really have no clue there.
This series was applied to netdev/net-next.git (master) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net-next.git (master) by Jakub Kicinski <[email protected]>:
Quoted 9 lines · On Tue, 8 Nov 2022 11:47:06 +0100 you wrote:
Ido Schimmel <[email protected]> writes:
This patchset adds 802.1X [1] and MAB [2] offload support in mlxsw.
Patches #1-#3 add the required switchdev interfaces.
Patches #4-#5 add the required packet traps for 802.1X.
[...]
Here is the summary with links:
- [net-next,01/15] bridge: switchdev: Let device drivers determine FDB offload indication
https://git.kernel.org/netdev/net-next/c/9baedc3c8780 - [net-next,02/15] bridge: switchdev: Allow device drivers to install locked FDB entries https://git.kernel.org/netdev/net-next/c/27fabd02abf3 - [net-next,03/15] bridge: switchdev: Reflect MAB bridge port flag to device drivers https://git.kernel.org/netdev/net-next/c/9c0ca02bace4 - [net-next,04/15] devlink: Add packet traps for 802.1X operation https://git.kernel.org/netdev/net-next/c/2640a82bbc08 - [net-next,05/15] mlxsw: spectrum_trap: Register 802.1X packet traps with devlink https://git.kernel.org/netdev/net-next/c/d85be0f5fd7c - [net-next,06/15] mlxsw: reg: Add Switch Port FDB Security Register https://git.kernel.org/netdev/net-next/c/0b31fb9ba2b5 - [net-next,07/15] mlxsw: spectrum: Add an API to configure security checks https://git.kernel.org/netdev/net-next/c/dc0d1a8b7f84 - [net-next,08/15] mlxsw: spectrum_switchdev: Prepare for locked FDB notifications https://git.kernel.org/netdev/net-next/c/b72cb660b26b - [net-next,09/15] mlxsw: spectrum_switchdev: Add support for locked FDB notifications https://git.kernel.org/netdev/net-next/c/5a660e43f8b9 - [net-next,10/15] mlxsw: spectrum_switchdev: Use extack in bridge port flag validation https://git.kernel.org/netdev/net-next/c/136b8dfbd784 - [net-next,11/15] mlxsw: spectrum_switchdev: Add locked bridge port support https://git.kernel.org/netdev/net-next/c/25ed80884ce1 - [net-next,12/15] selftests: devlink_lib: Split out helper https://git.kernel.org/netdev/net-next/c/da23a713d1de - [net-next,13/15] selftests: mlxsw: Add a test for EAPOL trap https://git.kernel.org/netdev/net-next/c/25a26f0c2015 - [net-next,14/15] selftests: mlxsw: Add a test for locked port trap https://git.kernel.org/netdev/net-next/c/fb398432db2f
- [net-next,15/15] selftests: mlxsw: Add a test for invalid locked bridge port configurations
https://git.kernel.org/netdev/net-next/c/cdbde7edf0e5
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html