From bb367803a37d7bbbded5e0793f9ae33ef54dde10 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 6 May 2021 10:23:08 +0300 Subject: [PATCH] mlxsw: spectrum_mr: Update egress RIF list before route's action commit cbaf3f6af9c268caf558c8e7ec52bcb35c5455dd upstream. Each multicast route that is forwarding packets (as opposed to trapping them) points to a list of egress router interfaces (RIFs) through which packets are replicated. A route's action can transition from trap to forward when a RIF is created for one of the route's egress virtual interfaces (eVIF). When this happens, the route's action is first updated and only later the list of egress RIFs is committed to the device. This results in the route pointing to an invalid list. In case the list pointer is out of range (due to uninitialized memory), the device will complain: mlxsw_spectrum2 0000:06:00.0: EMAD reg access failed (tid=5733bf490000905c,reg_id=300f(pefa),type=write,status=7(bad parameter)) Fix this by first committing the list of egress RIFs to the device and only later update the route's action. Note that a fix is not needed in the reverse function (i.e., mlxsw_sp_mr_route_evif_unresolve()), as there the route's action is first updated and only later the RIF is removed from the list. Cc: stable@vger.kernel.org Fixes: c011ec1bbfd6 ("mlxsw: spectrum: Add the multicast routing offloading logic") Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Link: https://lore.kernel.org/r/20210506072308.3834303-1-idosch@idosch.org Signed-off-by: Jakub Kicinski Signed-off-by: Greg Kroah-Hartman --- .../net/ethernet/mellanox/mlxsw/spectrum_mr.c | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c index 7846a21555ef85..1f6bc0c7e91dda 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c @@ -535,6 +535,16 @@ mlxsw_sp_mr_route_evif_resolve(struct mlxsw_sp_mr_table *mr_table, u16 erif_index = 0; int err; + /* Add the eRIF */ + if (mlxsw_sp_mr_vif_valid(rve->mr_vif)) { + erif_index = mlxsw_sp_rif_index(rve->mr_vif->rif); + err = mr->mr_ops->route_erif_add(mlxsw_sp, + rve->mr_route->route_priv, + erif_index); + if (err) + return err; + } + /* Update the route action, as the new eVIF can be a tunnel or a pimreg * device which will require updating the action. */ @@ -544,17 +554,7 @@ mlxsw_sp_mr_route_evif_resolve(struct mlxsw_sp_mr_table *mr_table, rve->mr_route->route_priv, route_action); if (err) - return err; - } - - /* Add the eRIF */ - if (mlxsw_sp_mr_vif_valid(rve->mr_vif)) { - erif_index = mlxsw_sp_rif_index(rve->mr_vif->rif); - err = mr->mr_ops->route_erif_add(mlxsw_sp, - rve->mr_route->route_priv, - erif_index); - if (err) - goto err_route_erif_add; + goto err_route_action_update; } /* Update the minimum MTU */ @@ -572,14 +572,14 @@ mlxsw_sp_mr_route_evif_resolve(struct mlxsw_sp_mr_table *mr_table, return 0; err_route_min_mtu_update: - if (mlxsw_sp_mr_vif_valid(rve->mr_vif)) - mr->mr_ops->route_erif_del(mlxsw_sp, rve->mr_route->route_priv, - erif_index); -err_route_erif_add: if (route_action != rve->mr_route->route_action) mr->mr_ops->route_action_update(mlxsw_sp, rve->mr_route->route_priv, rve->mr_route->route_action); +err_route_action_update: + if (mlxsw_sp_mr_vif_valid(rve->mr_vif)) + mr->mr_ops->route_erif_del(mlxsw_sp, rve->mr_route->route_priv, + erif_index); return err; }