| Message ID | 948a334a17b35e6ec03de5513fc8bd9dc27a9bb2.1524610426.git.petrm@mellanox.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Ido Schimmel |
| Headers | show |
On Wed, Apr 25, 2018 at 01:08:17AM +0200, Petr Machata wrote: > @@ -2236,8 +2244,13 @@ static void mlxsw_sp_switchdev_event_work(struct work_struct *work) > fdb_info = &switchdev_work->fdb_info; > mlxsw_sp_port_fdb_set(mlxsw_sp_port, fdb_info, false); > break; > + case SWITCHDEV_FDB_ADD_TO_BRIDGE: /* fall through */ > + case SWITCHDEV_FDB_DEL_TO_BRIDGE: To make it even more explicit: /* These events are only used to potentially update an existing SPAN mirror */ > + break; > }
On Wed, Apr 25, 2018 at 01:08:17AM +0200, Petr Machata wrote: > switch (obj->id) { > case SWITCHDEV_OBJ_ID_PORT_VLAN: > - err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, > - SWITCHDEV_OBJ_PORT_VLAN(obj), > - trans); > + vlan = SWITCHDEV_OBJ_PORT_VLAN(obj); > + err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, trans); > + if (switchdev_trans_ph_commit(trans)) > + mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp, vlan); I thought about it again. I think that instead of propagating the VLAN everywhere, we can keep the code generic and simply schedule the respin using a workqueue.
Ido Schimmel <idosch@mellanox.com> writes: > On Wed, Apr 25, 2018 at 01:08:17AM +0200, Petr Machata wrote: >> switch (obj->id) { >> case SWITCHDEV_OBJ_ID_PORT_VLAN: >> - err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, >> - SWITCHDEV_OBJ_PORT_VLAN(obj), >> - trans); >> + vlan = SWITCHDEV_OBJ_PORT_VLAN(obj); >> + err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, trans); >> + if (switchdev_trans_ph_commit(trans)) >> + mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp, vlan); > > I thought about it again. I think that instead of propagating the VLAN > everywhere, we can keep the code generic and simply schedule the respin > using a workqueue. OK, I'll do it like that.
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c index db4aea0..a6bfbfe 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c @@ -49,6 +49,7 @@ #include <linux/netlink.h> #include <net/switchdev.h> +#include "spectrum_span.h" #include "spectrum_router.h" #include "spectrum_switchdev.h" #include "spectrum.h" @@ -923,6 +924,9 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev, break; } + if (switchdev_trans_ph_commit(trans)) + mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp, NULL); + return err; } @@ -1652,13 +1656,15 @@ static int mlxsw_sp_port_obj_add(struct net_device *dev, struct switchdev_trans *trans) { struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev); + const struct switchdev_obj_port_vlan *vlan; int err = 0; switch (obj->id) { case SWITCHDEV_OBJ_ID_PORT_VLAN: - err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, - SWITCHDEV_OBJ_PORT_VLAN(obj), - trans); + vlan = SWITCHDEV_OBJ_PORT_VLAN(obj); + err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, trans); + if (switchdev_trans_ph_commit(trans)) + mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp, vlan); break; case SWITCHDEV_OBJ_ID_PORT_MDB: err = mlxsw_sp_port_mdb_add(mlxsw_sp_port, @@ -1809,6 +1815,8 @@ static int mlxsw_sp_port_obj_del(struct net_device *dev, break; } + mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp, NULL); + return err; } @@ -2236,8 +2244,13 @@ static void mlxsw_sp_switchdev_event_work(struct work_struct *work) fdb_info = &switchdev_work->fdb_info; mlxsw_sp_port_fdb_set(mlxsw_sp_port, fdb_info, false); break; + case SWITCHDEV_FDB_ADD_TO_BRIDGE: /* fall through */ + case SWITCHDEV_FDB_DEL_TO_BRIDGE: + break; } + mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp, NULL); + out: rtnl_unlock(); kfree(switchdev_work->fdb_info.addr); @@ -2266,7 +2279,9 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused, switch (event) { case SWITCHDEV_FDB_ADD_TO_DEVICE: /* fall through */ - case SWITCHDEV_FDB_DEL_TO_DEVICE: + case SWITCHDEV_FDB_DEL_TO_DEVICE: /* fall through */ + case SWITCHDEV_FDB_ADD_TO_BRIDGE: /* fall through */ + case SWITCHDEV_FDB_DEL_TO_BRIDGE: memcpy(&switchdev_work->fdb_info, ptr, sizeof(switchdev_work->fdb_info)); switchdev_work->fdb_info.addr = kzalloc(ETH_ALEN, GFP_ATOMIC);
Changes to switchdev artifact can make a SPAN entry offloadable or unoffloadable. To that end: - Listen to SWITCHDEV_FDB_*_TO_BRIDGE notifications in addition to the *_TO_DEVICE ones, to catch whatever activity is sent to the bridge (likely by mlxsw itself). On each FDB notification, respin SPAN to reconcile it with the FDB changes. - Also respin on switchdev port attribute changes (which currently covers changes to STP state of ports) and port object additions and removals. Signed-off-by: Petr Machata <petrm@mellanox.com> --- Notes: Changes from v1 to v2: - In mlxsw_sp_switchdev_event_work(), add SWITCHDEV_FDB_*_TO_BRIDGE with a do-nothing handler to make it clear that these events expected. - Don't respin unless the switchdev event in question is a commit (as opposed to prepare). - Respin in mlxsw_sp_port_obj_add() and mlxsw_sp_port_obj_del(). .../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 23 ++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)