| Message ID | f433543efdb610ef5a6aba9ac52b4783ff137a13.1667902754.git.petrm@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 25ed80884ce11e38d1d611c49f1d1aa3e49a1e08 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mlxsw: Add 802.1X and MAB offload support | expand |
On Tue, Nov 08, 2022 at 11:47:17AM +0100, Petr Machata wrote: > From: Ido Schimmel <idosch@nvidia.com> > > 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 Got the answer I was looking for. > > 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. > > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > Reviewed-by: Petr Machata <petrm@nvidia.com> > Signed-off-by: Petr Machata <petrm@nvidia.com> > --- Can't really figure out from the patch, sorry. Port security works with LAG offload?
On Tue, Nov 08, 2022 at 04:59:29PM +0200, Vladimir Oltean wrote: > 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.
On Wed, Nov 09, 2022 at 10:26:29AM +0200, Ido Schimmel wrote: > On Tue, Nov 08, 2022 at 04:59:29PM +0200, Vladimir Oltean wrote: > > 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. Okay, understood, the concepts are clear.
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index b34366521914..f5b2d965d476 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -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; diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c index db149af7c888..accea95cae5d 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c @@ -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); }