From 53f800e3baf980827c197a9332f63effe80d4809 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Fri, 23 Dec 2016 09:32:48 +0100 Subject: [PATCH 1/3] neigh: Send netevent after marking neigh as dead neigh_cleanup_and_release() is always called after marking a neighbour as dead, but it only notifies user space and not in-kernel listeners of the netevent notification chain. This can cause multiple problems. In my specific use case, it causes the listener (a switch driver capable of L3 offloads) to believe a neighbour entry is still valid, and is thus erroneously kept in the device's table. Fix that by sending a netevent after marking the neighbour as dead. Fixes: a6bf9e933daf ("mlxsw: spectrum_router: Offload neighbours based on NUD state change") Signed-off-by: Ido Schimmel Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/core/neighbour.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 782dd8663665..7bb12e07ffef 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -100,6 +100,7 @@ static void neigh_cleanup_and_release(struct neighbour *neigh) neigh->parms->neigh_cleanup(neigh); __neigh_notify(neigh, RTM_DELNEIGH, 0); + call_netevent_notifiers(NETEVENT_NEIGH_UPDATE, neigh); neigh_release(neigh); } From 93a87e5e794fb71a51f97fbde6c0010680b62d70 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Fri, 23 Dec 2016 09:32:49 +0100 Subject: [PATCH 2/3] mlxsw: spectrum_router: Don't reflect dead neighs When a neighbour is considered to be dead, we should remove it from the device's table regardless of its NUD state. Without this patch, after setting a port to be administratively down we get the following errors when we periodically try to update the kernel about neighbours activity: [ 461.947268] mlxsw_spectrum 0000:03:00.0 sw1p3: Failed to find matching neighbour for IP=192.168.100.2 Fixes: a6bf9e933daf ("mlxsw: spectrum_router: Offload neighbours based on NUD state change") Signed-off-by: Ido Schimmel Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c index 53126bf68ea9..a0f9742f62df 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c @@ -942,7 +942,7 @@ static void mlxsw_sp_router_neigh_update_hw(struct work_struct *work) char rauht_pl[MLXSW_REG_RAUHT_LEN]; struct net_device *dev; bool entry_connected; - u8 nud_state; + u8 nud_state, dead; bool updating; bool removing; bool adding; @@ -953,10 +953,11 @@ static void mlxsw_sp_router_neigh_update_hw(struct work_struct *work) dip = ntohl(*((__be32 *) n->primary_key)); memcpy(neigh_entry->ha, n->ha, sizeof(neigh_entry->ha)); nud_state = n->nud_state; + dead = n->dead; dev = n->dev; read_unlock_bh(&n->lock); - entry_connected = nud_state & NUD_VALID; + entry_connected = nud_state & NUD_VALID && !dead; adding = (!neigh_entry->offloaded) && entry_connected; updating = neigh_entry->offloaded && entry_connected; removing = neigh_entry->offloaded && !entry_connected; @@ -1351,7 +1352,7 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp, struct mlxsw_sp_neigh_entry *neigh_entry; struct net_device *dev = fib_nh->nh_dev; struct neighbour *n; - u8 nud_state; + u8 nud_state, dead; /* Take a reference of neigh here ensuring that neigh would * not be detructed before the nexthop entry is finished. @@ -1383,8 +1384,9 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp, list_add_tail(&nh->neigh_list_node, &neigh_entry->nexthop_list); read_lock_bh(&n->lock); nud_state = n->nud_state; + dead = n->dead; read_unlock_bh(&n->lock); - __mlxsw_sp_nexthop_neigh_update(nh, !(nud_state & NUD_VALID)); + __mlxsw_sp_nexthop_neigh_update(nh, !(nud_state & NUD_VALID && !dead)); return 0; } From 58312125da5806308bd69e075fedae30f8cf7794 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Fri, 23 Dec 2016 09:32:50 +0100 Subject: [PATCH 3/3] mlxsw: spectrum_router: Correctly remove nexthop groups At the end of the nexthop initialization process we determine whether the nexthop should be offloaded or not based on the NUD state of the neighbour representing it. After all the nexthops were initialized we refresh the nexthop group and potentially offload it to the device, in case some of the nexthops were resolved. Make the destruction of a nexthop group symmetric with its creation by marking all nexthops as invalid and then refresh the nexthop group to make sure it was removed from the device's tables. Fixes: b2157149b0b0 ("mlxsw: spectrum_router: Add the nexthop neigh activity update") Signed-off-by: Ido Schimmel Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c index a0f9742f62df..01d0efa9c5c7 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c @@ -1396,6 +1396,7 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp, { struct mlxsw_sp_neigh_entry *neigh_entry = nh->neigh_entry; + __mlxsw_sp_nexthop_neigh_update(nh, true); list_del(&nh->neigh_list_node); /* If that is the last nexthop connected to that neigh, remove from @@ -1454,6 +1455,8 @@ mlxsw_sp_nexthop_group_destroy(struct mlxsw_sp *mlxsw_sp, nh = &nh_grp->nexthops[i]; mlxsw_sp_nexthop_fini(mlxsw_sp, nh); } + mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh_grp); + WARN_ON_ONCE(nh_grp->adj_index_valid); kfree(nh_grp); }