Linux Ethernet Bridging - Oct 2022 - [Bridge] [RFC PATCH net-next 10/16] mlxsw: spectrum_switchdev: Add support for locked FDB notifications

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.

Signed-off-by: Ido Schimmel <idosch at nvidia.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 0fbefa43f9b1..f336be77019f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -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);
-- 
2.37.3

On Tue, Oct 25, 2022 at 01:00:18PM +0300, Ido Schimmel
wrote:
> 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.
> 
> Signed-off-by: Ido Schimmel <idosch at nvidia.com>
> ---
So for mlxsw, the hardware/driver always gets learning notifications
if learning is enabled (and regardless of MAB being enabled; with the
mention that BR_PORT_MAB implies BR_LEARNING and so, with MAB, these
notifications always come), and the driver always calls
SWITCHDEV_FDB_ADD_TO_BRIDGE,
letting the bridge figure out if it should create a BR_FDB_LOCKED entry
or to throw the notification away?

Hans' case is different; he needs to configure the HW differently
(MAB is more resource intensive). I suppose at some point, in his patch
series, he will need to also offload BR_PORT_MAB, something which you
didn't need. Ok.

The thing is that it will become tricky to know, when adding BR_PORT_MAB
to BR_PORT_FLAGS_HW_OFFLOAD, which drivers can offload MAB and which
can't, without some prior knowledge. For example, Hans will need to
patch mlxsw_sp_port_attr_br_pre_flags_set() to not reject BR_PORT_MAB,
even if mlxsw will need to do nothing based on the flag, right?