[patch net-next 00/15] mlxsw: Reflect nexthop status changes

[patch net-next 00/15] mlxsw: Reflect nexthop status changes

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Ido says:

When the kernel forwards IPv4 packets via multipath routes it doesn't
consider nexthops that are dead or linkdown. For example, if the nexthop
netdev is administratively down or doesn't have a carrier.

Devices capable of offloading such multipath routes need to be made
aware of changes in the reflected nexthops' status. Otherwise, the
device might forward packets via non-functional nexthops, resulting in
packet loss. This patchset aims to fix that.

The first 11 patches deal with the necessary restructuring in the
mlxsw driver, so that it's able to correctly add and remove nexthops
from the device's adjacency table.

The 12th patch adds the NH_{ADD,DEL} events to the FIB notification
chain. These notifications are sent whenever the kernel decides to add
or remove a nexthop from the forwarding plane.

Finally, the last three patches add support for these events in the
mlxsw driver, which is currently the only driver capable of offloading
multipath routes.

Ido Schimmel (15):
  mlxsw: spectrum_router: Nullify nexthop's neigh pointer
  mlxsw: spectrum_router: Store nexthop groups in a hash table
  mlxsw: spectrum_router: Store nexthops in a hash table
  mlxsw: spectrum_router: Use nexthop's scope to set action type
  mlxsw: spectrum_router: Add gateway indication to nexthop group
  mlxsw: spectrum_router: Store routes in a more generic way
  mlxsw: spectrum_router: Remove FIB info from FIB entry struct
  mlxsw: spectrum_router: Refactor nexthop init routine
  mlxsw: spectrum_router: More accurately set offload flag
  mlxsw: spectrum_router: Determine offload status using generic
    function
  mlxsw: spectrum_router: Use trap action only for some route types
  ipv4: fib: Notify about nexthop status changes
  mlxsw: spectrum_router: Reflect nexthop status changes
  mlxsw: spectrum_router: Don't reflect LINKDOWN nexthops
  mlxsw: spectrum_router: Flush resources when RIF is deleted

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |   6 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |   7 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 534 ++++++++++++++++-----
 include/net/ip_fib.h                               |   7 +
 net/ipv4/fib_semantics.c                           |  33 ++
 5 files changed, 457 insertions(+), 130 deletions(-)

-- 
2.7.4

[patch net-next 01/15] mlxsw: spectrum_router: Nullify nexthop's neigh pointer

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

When we invalidate a nexthop we should also invalidate its neighbour
entry pointer as it might be destroyed later on. This makes the nexthop
de-init function symmetric with its init and also ensures nobody will
try to access the neighbour entry.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index b19f69f..31680de 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1398,12 +1398,13 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
 
 	__mlxsw_sp_nexthop_neigh_update(nh, true);
 	list_del(&nh->neigh_list_node);
+	nh->neigh_entry = NULL;
 
 	/* If that is the last nexthop connected to that neigh, remove from
 	 * nexthop_neighs_list
 	 */
-	if (list_empty(&nh->neigh_entry->nexthop_list))
-		list_del(&nh->neigh_entry->nexthop_neighs_list_node);
+	if (list_empty(&neigh_entry->nexthop_list))
+		list_del(&neigh_entry->nexthop_neighs_list_node);
 
 	if (!neigh_entry->connected && list_empty(&neigh_entry->nexthop_list))
 		mlxsw_sp_neigh_entry_destroy(mlxsw_sp, neigh_entry);
-- 
2.7.4

[patch net-next 02/15] mlxsw: spectrum_router: Store nexthop groups in a hash table

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

Currently, when we're notified about a new RTN_UNICAST route we perform
a lookup on the nexthop group list looking for a group with a matching
configuration to that found in the FIB info. This is quite inefficient.

Instead, we can simply rely on the kernel to consolidate several FIB
configurations into the same FIB info and use the FIB info as the key
for our private nexthop group struct.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |   2 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 104 +++++++++++----------
 2 files changed, 54 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index dcd641a..8014d15 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -254,13 +254,13 @@ struct mlxsw_sp_router {
 	struct mlxsw_sp_lpm_tree lpm_trees[MLXSW_SP_LPM_TREE_COUNT];
 	struct mlxsw_sp_vr *vrs;
 	struct rhashtable neigh_ht;
+	struct rhashtable nexthop_group_ht;
 	struct {
 		struct delayed_work dw;
 		unsigned long interval;	/* ms */
 	} neighs_update;
 	struct delayed_work nexthop_probe_dw;
 #define MLXSW_SP_UNRESOLVED_NH_PROBE_INTERVAL 5000 /* ms */
-	struct list_head nexthop_group_list;
 	struct list_head nexthop_neighs_list;
 	bool aborted;
 };
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 31680de..a511b95 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1107,9 +1107,14 @@ struct mlxsw_sp_nexthop {
 	struct mlxsw_sp_neigh_entry *neigh_entry;
 };
 
+struct mlxsw_sp_nexthop_group_key {
+	struct fib_info *fi;
+};
+
 struct mlxsw_sp_nexthop_group {
-	struct list_head list; /* node in mlxsw->router.nexthop_group_list */
+	struct rhash_head ht_node;
 	struct list_head fib_list; /* list of fib entries that use this group */
+	struct mlxsw_sp_nexthop_group_key key;
 	u8 adj_index_valid:1;
 	u32 adj_index;
 	u16 ecmp_size;
@@ -1117,6 +1122,36 @@ struct mlxsw_sp_nexthop_group {
 	struct mlxsw_sp_nexthop nexthops[0];
 };
 
+static const struct rhashtable_params mlxsw_sp_nexthop_group_ht_params = {
+	.key_offset = offsetof(struct mlxsw_sp_nexthop_group, key),
+	.head_offset = offsetof(struct mlxsw_sp_nexthop_group, ht_node),
+	.key_len = sizeof(struct mlxsw_sp_nexthop_group_key),
+};
+
+static int mlxsw_sp_nexthop_group_insert(struct mlxsw_sp *mlxsw_sp,
+					 struct mlxsw_sp_nexthop_group *nh_grp)
+{
+	return rhashtable_insert_fast(&mlxsw_sp->router.nexthop_group_ht,
+				      &nh_grp->ht_node,
+				      mlxsw_sp_nexthop_group_ht_params);
+}
+
+static void mlxsw_sp_nexthop_group_remove(struct mlxsw_sp *mlxsw_sp,
+					  struct mlxsw_sp_nexthop_group *nh_grp)
+{
+	rhashtable_remove_fast(&mlxsw_sp->router.nexthop_group_ht,
+			       &nh_grp->ht_node,
+			       mlxsw_sp_nexthop_group_ht_params);
+}
+
+static struct mlxsw_sp_nexthop_group *
+mlxsw_sp_nexthop_group_lookup(struct mlxsw_sp *mlxsw_sp,
+			      struct mlxsw_sp_nexthop_group_key key)
+{
+	return rhashtable_lookup_fast(&mlxsw_sp->router.nexthop_group_ht, &key,
+				      mlxsw_sp_nexthop_group_ht_params);
+}
+
 static int mlxsw_sp_adj_index_mass_update_vr(struct mlxsw_sp *mlxsw_sp,
 					     struct mlxsw_sp_vr *vr,
 					     u32 adj_index, u16 ecmp_size,
@@ -1429,6 +1464,7 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
 		return ERR_PTR(-ENOMEM);
 	INIT_LIST_HEAD(&nh_grp->fib_list);
 	nh_grp->count = fi->fib_nhs;
+	nh_grp->key.fi = fi;
 	for (i = 0; i < nh_grp->count; i++) {
 		nh = &nh_grp->nexthops[i];
 		fib_nh = &fi->fib_nh[i];
@@ -1436,10 +1472,13 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
 		if (err)
 			goto err_nexthop_init;
 	}
-	list_add_tail(&nh_grp->list, &mlxsw_sp->router.nexthop_group_list);
+	err = mlxsw_sp_nexthop_group_insert(mlxsw_sp, nh_grp);
+	if (err)
+		goto err_nexthop_group_insert;
 	mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh_grp);
 	return nh_grp;
 
+err_nexthop_group_insert:
 err_nexthop_init:
 	for (i--; i >= 0; i--)
 		mlxsw_sp_nexthop_fini(mlxsw_sp, nh);
@@ -1454,7 +1493,7 @@ mlxsw_sp_nexthop_group_destroy(struct mlxsw_sp *mlxsw_sp,
 	struct mlxsw_sp_nexthop *nh;
 	int i;
 
-	list_del(&nh_grp->list);
+	mlxsw_sp_nexthop_group_remove(mlxsw_sp, nh_grp);
 	for (i = 0; i < nh_grp->count; i++) {
 		nh = &nh_grp->nexthops[i];
 		mlxsw_sp_nexthop_fini(mlxsw_sp, nh);
@@ -1464,59 +1503,15 @@ mlxsw_sp_nexthop_group_destroy(struct mlxsw_sp *mlxsw_sp,
 	kfree(nh_grp);
 }
 
-static bool mlxsw_sp_nexthop_match(struct mlxsw_sp_nexthop *nh,
-				   struct fib_info *fi)
-{
-	int i;
-
-	for (i = 0; i < fi->fib_nhs; i++) {
-		struct fib_nh *fib_nh = &fi->fib_nh[i];
-		struct neighbour *n = nh->neigh_entry->key.n;
-
-		if (memcmp(n->primary_key, &fib_nh->nh_gw,
-			   sizeof(fib_nh->nh_gw)) == 0 &&
-		    n->dev == fib_nh->nh_dev)
-			return true;
-	}
-	return false;
-}
-
-static bool mlxsw_sp_nexthop_group_match(struct mlxsw_sp_nexthop_group *nh_grp,
-					 struct fib_info *fi)
-{
-	int i;
-
-	if (nh_grp->count != fi->fib_nhs)
-		return false;
-	for (i = 0; i < nh_grp->count; i++) {
-		struct mlxsw_sp_nexthop *nh = &nh_grp->nexthops[i];
-
-		if (!mlxsw_sp_nexthop_match(nh, fi))
-			return false;
-	}
-	return true;
-}
-
-static struct mlxsw_sp_nexthop_group *
-mlxsw_sp_nexthop_group_find(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
-{
-	struct mlxsw_sp_nexthop_group *nh_grp;
-
-	list_for_each_entry(nh_grp, &mlxsw_sp->router.nexthop_group_list,
-			    list) {
-		if (mlxsw_sp_nexthop_group_match(nh_grp, fi))
-			return nh_grp;
-	}
-	return NULL;
-}
-
 static int mlxsw_sp_nexthop_group_get(struct mlxsw_sp *mlxsw_sp,
 				      struct mlxsw_sp_fib_entry *fib_entry,
 				      struct fib_info *fi)
 {
+	struct mlxsw_sp_nexthop_group_key key;
 	struct mlxsw_sp_nexthop_group *nh_grp;
 
-	nh_grp = mlxsw_sp_nexthop_group_find(mlxsw_sp, fi);
+	key.fi = fi;
+	nh_grp = mlxsw_sp_nexthop_group_lookup(mlxsw_sp, key);
 	if (!nh_grp) {
 		nh_grp = mlxsw_sp_nexthop_group_create(mlxsw_sp, fi);
 		if (IS_ERR(nh_grp))
@@ -2053,11 +2048,15 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
 	int err;
 
 	INIT_LIST_HEAD(&mlxsw_sp->router.nexthop_neighs_list);
-	INIT_LIST_HEAD(&mlxsw_sp->router.nexthop_group_list);
 	err = __mlxsw_sp_router_init(mlxsw_sp);
 	if (err)
 		return err;
 
+	err = rhashtable_init(&mlxsw_sp->router.nexthop_group_ht,
+			      &mlxsw_sp_nexthop_group_ht_params);
+	if (err)
+		goto err_nexthop_group_ht_init;
+
 	mlxsw_sp_lpm_init(mlxsw_sp);
 	err = mlxsw_sp_vrs_init(mlxsw_sp);
 	if (err)
@@ -2080,6 +2079,8 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
 err_neigh_init:
 	mlxsw_sp_vrs_fini(mlxsw_sp);
 err_vrs_init:
+	rhashtable_destroy(&mlxsw_sp->router.nexthop_group_ht);
+err_nexthop_group_ht_init:
 	__mlxsw_sp_router_fini(mlxsw_sp);
 	return err;
 }
@@ -2089,5 +2090,6 @@ void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp)
 	unregister_fib_notifier(&mlxsw_sp->fib_nb);
 	mlxsw_sp_neigh_fini(mlxsw_sp);
 	mlxsw_sp_vrs_fini(mlxsw_sp);
+	rhashtable_destroy(&mlxsw_sp->router.nexthop_group_ht);
 	__mlxsw_sp_router_fini(mlxsw_sp);
 }
-- 
2.7.4

[patch net-next 03/15] mlxsw: spectrum_router: Store nexthops in a hash table

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

Later in the patchset we'll add the NH_{ADD,DEL} events which will let
us know when a nexthop is considered to be dead. Based on these events
we need to be able to add or remove the nexthop from the device's
tables.

Therefore, store the private nexthop structs in a hash table and use the
kernel's fib_nh struct as the key, so that we'll be able to easily find
them when the events are received.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  1 +
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 58 ++++++++++++++++++++--
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 8014d15..eb743fe 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -255,6 +255,7 @@ struct mlxsw_sp_router {
 	struct mlxsw_sp_vr *vrs;
 	struct rhashtable neigh_ht;
 	struct rhashtable nexthop_group_ht;
+	struct rhashtable nexthop_ht;
 	struct {
 		struct delayed_work dw;
 		unsigned long interval;	/* ms */
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index a511b95..3d540d9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1090,11 +1090,17 @@ static void mlxsw_sp_neigh_fini(struct mlxsw_sp *mlxsw_sp)
 	rhashtable_destroy(&mlxsw_sp->router.neigh_ht);
 }
 
+struct mlxsw_sp_nexthop_key {
+	struct fib_nh *fib_nh;
+};
+
 struct mlxsw_sp_nexthop {
 	struct list_head neigh_list_node; /* member of neigh entry list */
 	struct mlxsw_sp_nexthop_group *nh_grp; /* pointer back to the group
 						* this belongs to
 						*/
+	struct rhash_head ht_node;
+	struct mlxsw_sp_nexthop_key key;
 	u8 should_offload:1, /* set indicates this neigh is connected and
 			      * should be put to KVD linear area of this group.
 			      */
@@ -1152,6 +1158,26 @@ mlxsw_sp_nexthop_group_lookup(struct mlxsw_sp *mlxsw_sp,
 				      mlxsw_sp_nexthop_group_ht_params);
 }
 
+static const struct rhashtable_params mlxsw_sp_nexthop_ht_params = {
+	.key_offset = offsetof(struct mlxsw_sp_nexthop, key),
+	.head_offset = offsetof(struct mlxsw_sp_nexthop, ht_node),
+	.key_len = sizeof(struct mlxsw_sp_nexthop_key),
+};
+
+static int mlxsw_sp_nexthop_insert(struct mlxsw_sp *mlxsw_sp,
+				   struct mlxsw_sp_nexthop *nh)
+{
+	return rhashtable_insert_fast(&mlxsw_sp->router.nexthop_ht,
+				      &nh->ht_node, mlxsw_sp_nexthop_ht_params);
+}
+
+static void mlxsw_sp_nexthop_remove(struct mlxsw_sp *mlxsw_sp,
+				    struct mlxsw_sp_nexthop *nh)
+{
+	rhashtable_remove_fast(&mlxsw_sp->router.nexthop_ht, &nh->ht_node,
+			       mlxsw_sp_nexthop_ht_params);
+}
+
 static int mlxsw_sp_adj_index_mass_update_vr(struct mlxsw_sp *mlxsw_sp,
 					     struct mlxsw_sp_vr *vr,
 					     u32 adj_index, u16 ecmp_size,
@@ -1384,6 +1410,12 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 	struct net_device *dev = fib_nh->nh_dev;
 	struct neighbour *n;
 	u8 nud_state, dead;
+	int err;
+
+	nh->key.fib_nh = fib_nh;
+	err = mlxsw_sp_nexthop_insert(mlxsw_sp, nh);
+	if (err)
+		return err;
 
 	/* Take a reference of neigh here ensuring that neigh would
 	 * not be detructed before the nexthop entry is finished.
@@ -1393,16 +1425,18 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 	n = neigh_lookup(&arp_tbl, &fib_nh->nh_gw, dev);
 	if (!n) {
 		n = neigh_create(&arp_tbl, &fib_nh->nh_gw, dev);
-		if (IS_ERR(n))
-			return PTR_ERR(n);
+		if (IS_ERR(n)) {
+			err = PTR_ERR(n);
+			goto err_neigh_create;
+		}
 		neigh_event_send(n, NULL);
 	}
 	neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
 	if (!neigh_entry) {
 		neigh_entry = mlxsw_sp_neigh_entry_create(mlxsw_sp, n);
 		if (IS_ERR(neigh_entry)) {
-			neigh_release(n);
-			return -EINVAL;
+			err = -EINVAL;
+			goto err_neigh_entry_create;
 		}
 	}
 
@@ -1423,6 +1457,12 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 	__mlxsw_sp_nexthop_neigh_update(nh, !(nud_state & NUD_VALID && !dead));
 
 	return 0;
+
+err_neigh_entry_create:
+	neigh_release(n);
+err_neigh_create:
+	mlxsw_sp_nexthop_remove(mlxsw_sp, nh);
+	return err;
 }
 
 static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
@@ -1445,6 +1485,8 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
 		mlxsw_sp_neigh_entry_destroy(mlxsw_sp, neigh_entry);
 
 	neigh_release(n);
+
+	mlxsw_sp_nexthop_remove(mlxsw_sp, nh);
 }
 
 static struct mlxsw_sp_nexthop_group *
@@ -2052,6 +2094,11 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
 	if (err)
 		return err;
 
+	err = rhashtable_init(&mlxsw_sp->router.nexthop_ht,
+			      &mlxsw_sp_nexthop_ht_params);
+	if (err)
+		goto err_nexthop_ht_init;
+
 	err = rhashtable_init(&mlxsw_sp->router.nexthop_group_ht,
 			      &mlxsw_sp_nexthop_group_ht_params);
 	if (err)
@@ -2081,6 +2128,8 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
 err_vrs_init:
 	rhashtable_destroy(&mlxsw_sp->router.nexthop_group_ht);
 err_nexthop_group_ht_init:
+	rhashtable_destroy(&mlxsw_sp->router.nexthop_ht);
+err_nexthop_ht_init:
 	__mlxsw_sp_router_fini(mlxsw_sp);
 	return err;
 }
@@ -2091,5 +2140,6 @@ void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp)
 	mlxsw_sp_neigh_fini(mlxsw_sp);
 	mlxsw_sp_vrs_fini(mlxsw_sp);
 	rhashtable_destroy(&mlxsw_sp->router.nexthop_group_ht);
+	rhashtable_destroy(&mlxsw_sp->router.nexthop_ht);
 	__mlxsw_sp_router_fini(mlxsw_sp);
 }
-- 
2.7.4

[patch net-next 04/15] mlxsw: spectrum_router: Use nexthop's scope to set action type

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

We currently use the scope of the FIB info to distinguish between a
direct unicast route and a gatewayed one. However, the kernel is
perfectly happy to configure a route with scope UNIVERSE to a directly
connected network.

Instead, we can rely on the first nexthop's scope to check if the route
is gatewayed or not.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 3d540d9..cad7e9a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1721,7 +1721,7 @@ mlxsw_sp_router_fib4_entry_init(struct mlxsw_sp *mlxsw_sp,
 		return 0;
 	}
 
-	if (fi->fib_scope != RT_SCOPE_UNIVERSE) {
+	if (fi->fib_nh->nh_scope != RT_SCOPE_LINK) {
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_LOCAL;
 		fib_entry->rif = r->rif;
 	} else {
-- 
2.7.4

[patch net-next 05/15] mlxsw: spectrum_router: Add gateway indication to nexthop group

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

The next patch is going to generalize the way in which we store routes.
Instead of attaching a nexthop group only to gatewayed routes, one will
be attached to each route, in a similar way to the way the FIB code
stores its routes.

The above means that any function operating on a nexthop group cannot
assume the group represents only gatewayed nexthops. One such function
is the one that refreshes a nexthop group and updates the adjacency
table following nexthop changes.

For a nexthop group that doesn't represent any gateways this function
would essentially be a NOP, but it would be useful if it did update the
action associated with any route using it. This will allow us to later
consolidate code paths when a nexthop changes following NH_{ADD,DEL}
events.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index cad7e9a..110a7bb 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1121,7 +1121,8 @@ struct mlxsw_sp_nexthop_group {
 	struct rhash_head ht_node;
 	struct list_head fib_list; /* list of fib entries that use this group */
 	struct mlxsw_sp_nexthop_group_key key;
-	u8 adj_index_valid:1;
+	u8 adj_index_valid:1,
+	   gateway:1; /* routes using the group use a gateway */
 	u32 adj_index;
 	u16 ecmp_size;
 	u16 count;
@@ -1292,6 +1293,11 @@ mlxsw_sp_nexthop_group_refresh(struct mlxsw_sp *mlxsw_sp,
 	int i;
 	int err;
 
+	if (!nh_grp->gateway) {
+		mlxsw_sp_nexthop_fib_entries_update(mlxsw_sp, nh_grp);
+		return;
+	}
+
 	for (i = 0; i < nh_grp->count; i++) {
 		nh = &nh_grp->nexthops[i];
 
@@ -1505,6 +1511,7 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
 	if (!nh_grp)
 		return ERR_PTR(-ENOMEM);
 	INIT_LIST_HEAD(&nh_grp->fib_list);
+	nh_grp->gateway = fi->fib_nh->nh_scope == RT_SCOPE_LINK;
 	nh_grp->count = fi->fib_nhs;
 	nh_grp->key.fi = fi;
 	for (i = 0; i < nh_grp->count; i++) {
-- 
2.7.4

[patch net-next 06/15] mlxsw: spectrum_router: Store routes in a more generic way

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

Up until now, the only FIB entries that were associated with a nexthop
group were routes to remote networks where all the nexthop devices had a
valid router interface (RIF). This is in contrast to the FIB code,
where all the routes are associated with a FIB info. The same design
choice needs to be applied to the driver's cache.

Based on the NH_{ADD,DEL} events which will be added later in the
patchset, we need to be able to change the action (forward / trap)
associated with all the routes using the nexthop group. However, if we
can't link between the nexthop and the routes using it, then the above
is impossible.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 40 +++++++++++++++-------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 110a7bb..8a72a1b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -127,7 +127,6 @@ struct mlxsw_sp_fib_entry {
 	struct mlxsw_sp_fib_key key;
 	enum mlxsw_sp_fib_entry_type type;
 	unsigned int ref_count;
-	u16 rif; /* used for action local */
 	struct mlxsw_sp_vr *vr;
 	struct fib_info *fi;
 	struct list_head nexthop_group_node;
@@ -1101,6 +1100,7 @@ struct mlxsw_sp_nexthop {
 						*/
 	struct rhash_head ht_node;
 	struct mlxsw_sp_nexthop_key key;
+	struct mlxsw_sp_rif *r;
 	u8 should_offload:1, /* set indicates this neigh is connected and
 			      * should be put to KVD linear area of this group.
 			      */
@@ -1127,6 +1127,7 @@ struct mlxsw_sp_nexthop_group {
 	u16 ecmp_size;
 	u16 count;
 	struct mlxsw_sp_nexthop nexthops[0];
+#define nh_rif	nexthops[0].r
 };
 
 static const struct rhashtable_params mlxsw_sp_nexthop_group_ht_params = {
@@ -1414,15 +1415,25 @@ 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 mlxsw_sp_rif *r;
 	struct neighbour *n;
 	u8 nud_state, dead;
 	int err;
 
+	nh->nh_grp = nh_grp;
 	nh->key.fib_nh = fib_nh;
 	err = mlxsw_sp_nexthop_insert(mlxsw_sp, nh);
 	if (err)
 		return err;
 
+	r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+	if (!r)
+		return 0;
+	nh->r = r;
+
+	if (!nh_grp->gateway)
+		return 0;
+
 	/* Take a reference of neigh here ensuring that neigh would
 	 * not be detructed before the nexthop entry is finished.
 	 * The reference is taken either in neigh_lookup() or
@@ -1453,7 +1464,6 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 		list_add_tail(&neigh_entry->nexthop_neighs_list_node,
 			      &mlxsw_sp->router.nexthop_neighs_list);
 
-	nh->nh_grp = nh_grp;
 	nh->neigh_entry = neigh_entry;
 	list_add_tail(&nh->neigh_list_node, &neigh_entry->nexthop_list);
 	read_lock_bh(&n->lock);
@@ -1477,6 +1487,9 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
 	struct mlxsw_sp_neigh_entry *neigh_entry = nh->neigh_entry;
 	struct neighbour *n = neigh_entry->key.n;
 
+	if (!neigh_entry)
+		goto out;
+
 	__mlxsw_sp_nexthop_neigh_update(nh, true);
 	list_del(&nh->neigh_list_node);
 	nh->neigh_entry = NULL;
@@ -1492,6 +1505,7 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
 
 	neigh_release(n);
 
+out:
 	mlxsw_sp_nexthop_remove(mlxsw_sp, nh);
 }
 
@@ -1619,6 +1633,7 @@ static int mlxsw_sp_fib_entry_op4_local(struct mlxsw_sp *mlxsw_sp,
 					struct mlxsw_sp_fib_entry *fib_entry,
 					enum mlxsw_reg_ralue_op op)
 {
+	struct mlxsw_sp_rif *r = fib_entry->nh_group->nh_rif;
 	char ralue_pl[MLXSW_REG_RALUE_LEN];
 	u32 *p_dip = (u32 *) fib_entry->key.addr;
 	struct mlxsw_sp_vr *vr = fib_entry->vr;
@@ -1628,7 +1643,7 @@ static int mlxsw_sp_fib_entry_op4_local(struct mlxsw_sp *mlxsw_sp,
 			      vr->id, fib_entry->key.prefix_len, *p_dip);
 	mlxsw_reg_ralue_act_local_pack(ralue_pl,
 				       MLXSW_REG_RALUE_TRAP_ACTION_NOP, 0,
-				       fib_entry->rif);
+				       r->rif);
 	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(ralue), ralue_pl);
 }
 
@@ -1697,7 +1712,6 @@ mlxsw_sp_router_fib4_entry_init(struct mlxsw_sp *mlxsw_sp,
 	struct fib_info *fi = fen_info->fi;
 	struct mlxsw_sp_rif *r = NULL;
 	int nhsel;
-	int err;
 
 	if (fen_info->type == RTN_LOCAL || fen_info->type == RTN_BROADCAST) {
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
@@ -1728,15 +1742,10 @@ mlxsw_sp_router_fib4_entry_init(struct mlxsw_sp *mlxsw_sp,
 		return 0;
 	}
 
-	if (fi->fib_nh->nh_scope != RT_SCOPE_LINK) {
+	if (fi->fib_nh->nh_scope != RT_SCOPE_LINK)
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_LOCAL;
-		fib_entry->rif = r->rif;
-	} else {
+	else
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_REMOTE;
-		err = mlxsw_sp_nexthop_group_get(mlxsw_sp, fib_entry, fi);
-		if (err)
-			return err;
-	}
 	fib_info_offload_inc(fen_info->fi);
 	return 0;
 }
@@ -1747,8 +1756,6 @@ mlxsw_sp_router_fib4_entry_fini(struct mlxsw_sp *mlxsw_sp,
 {
 	if (fib_entry->type != MLXSW_SP_FIB_ENTRY_TYPE_TRAP)
 		fib_info_offload_dec(fib_entry->fi);
-	if (fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_REMOTE)
-		mlxsw_sp_nexthop_group_put(mlxsw_sp, fib_entry);
 }
 
 static struct mlxsw_sp_fib_entry *
@@ -1788,8 +1795,14 @@ mlxsw_sp_fib_entry_get(struct mlxsw_sp *mlxsw_sp,
 	if (err)
 		goto err_fib4_entry_init;
 
+	err = mlxsw_sp_nexthop_group_get(mlxsw_sp, fib_entry, fi);
+	if (err)
+		goto err_nexthop_group_get;
+
 	return fib_entry;
 
+err_nexthop_group_get:
+	mlxsw_sp_router_fib4_entry_fini(mlxsw_sp, fib_entry);
 err_fib4_entry_init:
 	mlxsw_sp_fib_entry_destroy(fib_entry);
 err_fib_entry_create:
@@ -1821,6 +1834,7 @@ static void mlxsw_sp_fib_entry_put(struct mlxsw_sp *mlxsw_sp,
 	struct mlxsw_sp_vr *vr = fib_entry->vr;
 
 	if (--fib_entry->ref_count == 0) {
+		mlxsw_sp_nexthop_group_put(mlxsw_sp, fib_entry);
 		mlxsw_sp_router_fib4_entry_fini(mlxsw_sp, fib_entry);
 		mlxsw_sp_fib_entry_destroy(fib_entry);
 	}
-- 
2.7.4

[patch net-next 07/15] mlxsw: spectrum_router: Remove FIB info from FIB entry struct

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

After the previous changes, the FIB info is embedded in every nexthop
group struct, which in turn is embedded in every FIB entry struct.

We can therefore safely remove the FIB info from the entry struct. This
has the added advantage of making the router-related structs more
generic and suitable for use with IPv6 offloads.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 8a72a1b..74839f7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -128,7 +128,6 @@ struct mlxsw_sp_fib_entry {
 	enum mlxsw_sp_fib_entry_type type;
 	unsigned int ref_count;
 	struct mlxsw_sp_vr *vr;
-	struct fib_info *fi;
 	struct list_head nexthop_group_node;
 	struct mlxsw_sp_nexthop_group *nh_group;
 };
@@ -1755,7 +1754,7 @@ mlxsw_sp_router_fib4_entry_fini(struct mlxsw_sp *mlxsw_sp,
 				struct mlxsw_sp_fib_entry *fib_entry)
 {
 	if (fib_entry->type != MLXSW_SP_FIB_ENTRY_TYPE_TRAP)
-		fib_info_offload_dec(fib_entry->fi);
+		fib_info_offload_dec(fib_entry->nh_group->key.fi);
 }
 
 static struct mlxsw_sp_fib_entry *
@@ -1788,7 +1787,6 @@ mlxsw_sp_fib_entry_get(struct mlxsw_sp *mlxsw_sp,
 		goto err_fib_entry_create;
 	}
 	fib_entry->vr = vr;
-	fib_entry->fi = fi;
 	fib_entry->ref_count = 1;
 
 	err = mlxsw_sp_router_fib4_entry_init(mlxsw_sp, fen_info, fib_entry);
-- 
2.7.4

[patch net-next 08/15] mlxsw: spectrum_router: Refactor nexthop init routine

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

The nexthop init and de-init functions both have symmetric parts
concerned with the reflection of the neighbour entry into the device's
adjacency table, in case it's used by a gatewayed route.

These sections of code also need to be called when a nexthop is marked
as valid / invalid following NH_{ADD,DEL} events. Break these out into
appropriate functions, so that they could be invoked following the
reception of above events.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 80 +++++++++++++---------
 1 file changed, 49 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 74839f7..f535135 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1407,30 +1407,16 @@ mlxsw_sp_nexthop_neigh_update(struct mlxsw_sp *mlxsw_sp,
 	}
 }
 
-static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
-				 struct mlxsw_sp_nexthop_group *nh_grp,
-				 struct mlxsw_sp_nexthop *nh,
-				 struct fib_nh *fib_nh)
+static int mlxsw_sp_nexthop_neigh_init(struct mlxsw_sp *mlxsw_sp,
+				       struct mlxsw_sp_nexthop *nh)
 {
 	struct mlxsw_sp_neigh_entry *neigh_entry;
-	struct net_device *dev = fib_nh->nh_dev;
-	struct mlxsw_sp_rif *r;
+	struct fib_nh *fib_nh = nh->key.fib_nh;
 	struct neighbour *n;
 	u8 nud_state, dead;
 	int err;
 
-	nh->nh_grp = nh_grp;
-	nh->key.fib_nh = fib_nh;
-	err = mlxsw_sp_nexthop_insert(mlxsw_sp, nh);
-	if (err)
-		return err;
-
-	r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
-	if (!r)
-		return 0;
-	nh->r = r;
-
-	if (!nh_grp->gateway)
+	if (!nh->nh_grp->gateway)
 		return 0;
 
 	/* Take a reference of neigh here ensuring that neigh would
@@ -1438,13 +1424,11 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 	 * The reference is taken either in neigh_lookup() or
 	 * in neigh_create() in case n is not found.
 	 */
-	n = neigh_lookup(&arp_tbl, &fib_nh->nh_gw, dev);
+	n = neigh_lookup(&arp_tbl, &fib_nh->nh_gw, fib_nh->nh_dev);
 	if (!n) {
-		n = neigh_create(&arp_tbl, &fib_nh->nh_gw, dev);
-		if (IS_ERR(n)) {
-			err = PTR_ERR(n);
-			goto err_neigh_create;
-		}
+		n = neigh_create(&arp_tbl, &fib_nh->nh_gw, fib_nh->nh_dev);
+		if (IS_ERR(n))
+			return PTR_ERR(n);
 		neigh_event_send(n, NULL);
 	}
 	neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
@@ -1475,19 +1459,18 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 
 err_neigh_entry_create:
 	neigh_release(n);
-err_neigh_create:
-	mlxsw_sp_nexthop_remove(mlxsw_sp, nh);
 	return err;
 }
 
-static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
-				  struct mlxsw_sp_nexthop *nh)
+static void mlxsw_sp_nexthop_neigh_fini(struct mlxsw_sp *mlxsw_sp,
+					struct mlxsw_sp_nexthop *nh)
 {
 	struct mlxsw_sp_neigh_entry *neigh_entry = nh->neigh_entry;
-	struct neighbour *n = neigh_entry->key.n;
+	struct neighbour *n;
 
 	if (!neigh_entry)
-		goto out;
+		return;
+	n = neigh_entry->key.n;
 
 	__mlxsw_sp_nexthop_neigh_update(nh, true);
 	list_del(&nh->neigh_list_node);
@@ -1503,8 +1486,43 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
 		mlxsw_sp_neigh_entry_destroy(mlxsw_sp, neigh_entry);
 
 	neigh_release(n);
+}
 
-out:
+static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
+				 struct mlxsw_sp_nexthop_group *nh_grp,
+				 struct mlxsw_sp_nexthop *nh,
+				 struct fib_nh *fib_nh)
+{
+	struct net_device *dev = fib_nh->nh_dev;
+	struct mlxsw_sp_rif *r;
+	int err;
+
+	nh->nh_grp = nh_grp;
+	nh->key.fib_nh = fib_nh;
+	err = mlxsw_sp_nexthop_insert(mlxsw_sp, nh);
+	if (err)
+		return err;
+
+	r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+	if (!r)
+		return 0;
+	nh->r = r;
+
+	err = mlxsw_sp_nexthop_neigh_init(mlxsw_sp, nh);
+	if (err)
+		goto err_nexthop_neigh_init;
+
+	return 0;
+
+err_nexthop_neigh_init:
+	mlxsw_sp_nexthop_remove(mlxsw_sp, nh);
+	return err;
+}
+
+static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
+				  struct mlxsw_sp_nexthop *nh)
+{
+	mlxsw_sp_nexthop_neigh_fini(mlxsw_sp, nh);
 	mlxsw_sp_nexthop_remove(mlxsw_sp, nh);
 }
 
-- 
2.7.4

[patch net-next 09/15] mlxsw: spectrum_router: More accurately set offload flag

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

We currently set the RTNH_F_OFFLOAD flag for all routes using remote
action, but this isn't always correct. If none of the nexthops
associated with a gatewayed route can be offloaded into the device, then
any packet hitting it would be trapped to the CPU and forwarded by the
kernel.

Solve this by pushing the setting of the offload flag to after the route
was programmed into the device, thereby allowing us to take all the
parameters into account.

This change will also help us further in the patchset, when we refresh
routes following the reception of NH_{ADD,DEL} events.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 100 ++++++++++++++++-----
 1 file changed, 80 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index f535135..acabb35 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -130,6 +130,7 @@ struct mlxsw_sp_fib_entry {
 	struct mlxsw_sp_vr *vr;
 	struct list_head nexthop_group_node;
 	struct mlxsw_sp_nexthop_group *nh_group;
+	bool offloaded;
 };
 
 struct mlxsw_sp_fib {
@@ -1613,6 +1614,72 @@ static void mlxsw_sp_nexthop_group_put(struct mlxsw_sp *mlxsw_sp,
 	mlxsw_sp_nexthop_group_destroy(mlxsw_sp, nh_grp);
 }
 
+static bool
+mlxsw_sp_fib_entry_should_offload(const struct mlxsw_sp_fib_entry *fib_entry)
+{
+	struct mlxsw_sp_nexthop_group *nh_group = fib_entry->nh_group;
+
+	switch (fib_entry->type) {
+	case MLXSW_SP_FIB_ENTRY_TYPE_REMOTE:
+		return !!nh_group->adj_index_valid;
+	case MLXSW_SP_FIB_ENTRY_TYPE_LOCAL:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static void mlxsw_sp_fib_entry_offload_set(struct mlxsw_sp_fib_entry *fib_entry)
+{
+	fib_entry->offloaded = true;
+
+	switch (fib_entry->vr->proto) {
+	case MLXSW_SP_L3_PROTO_IPV4:
+		fib_info_offload_inc(fib_entry->nh_group->key.fi);
+		break;
+	case MLXSW_SP_L3_PROTO_IPV6:
+		WARN_ON_ONCE(1);
+	}
+}
+
+static void
+mlxsw_sp_fib_entry_offload_unset(struct mlxsw_sp_fib_entry *fib_entry)
+{
+	switch (fib_entry->vr->proto) {
+	case MLXSW_SP_L3_PROTO_IPV4:
+		fib_info_offload_dec(fib_entry->nh_group->key.fi);
+		break;
+	case MLXSW_SP_L3_PROTO_IPV6:
+		WARN_ON_ONCE(1);
+	}
+
+	fib_entry->offloaded = false;
+}
+
+static void
+mlxsw_sp_fib_entry_offload_refresh(struct mlxsw_sp_fib_entry *fib_entry,
+				   enum mlxsw_reg_ralue_op op, int err)
+{
+	switch (op) {
+	case MLXSW_REG_RALUE_OP_WRITE_DELETE:
+		if (!fib_entry->offloaded)
+			return;
+		return mlxsw_sp_fib_entry_offload_unset(fib_entry);
+	case MLXSW_REG_RALUE_OP_WRITE_WRITE:
+		if (err)
+			return;
+		if (mlxsw_sp_fib_entry_should_offload(fib_entry) &&
+		    !fib_entry->offloaded)
+			mlxsw_sp_fib_entry_offload_set(fib_entry);
+		else if (!mlxsw_sp_fib_entry_should_offload(fib_entry) &&
+			 fib_entry->offloaded)
+			mlxsw_sp_fib_entry_offload_unset(fib_entry);
+		return;
+	default:
+		return;
+	}
+}
+
 static int mlxsw_sp_fib_entry_op4_remote(struct mlxsw_sp *mlxsw_sp,
 					 struct mlxsw_sp_fib_entry *fib_entry,
 					 enum mlxsw_reg_ralue_op op)
@@ -1698,13 +1765,17 @@ static int mlxsw_sp_fib_entry_op(struct mlxsw_sp *mlxsw_sp,
 				 struct mlxsw_sp_fib_entry *fib_entry,
 				 enum mlxsw_reg_ralue_op op)
 {
+	int err = -EINVAL;
+
 	switch (fib_entry->vr->proto) {
 	case MLXSW_SP_L3_PROTO_IPV4:
-		return mlxsw_sp_fib_entry_op4(mlxsw_sp, fib_entry, op);
+		err = mlxsw_sp_fib_entry_op4(mlxsw_sp, fib_entry, op);
+		break;
 	case MLXSW_SP_L3_PROTO_IPV6:
-		return -EINVAL;
+		return err;
 	}
-	return -EINVAL;
+	mlxsw_sp_fib_entry_offload_refresh(fib_entry, op, err);
+	return err;
 }
 
 static int mlxsw_sp_fib_entry_update(struct mlxsw_sp *mlxsw_sp,
@@ -1722,9 +1793,9 @@ static int mlxsw_sp_fib_entry_del(struct mlxsw_sp *mlxsw_sp,
 }
 
 static int
-mlxsw_sp_router_fib4_entry_init(struct mlxsw_sp *mlxsw_sp,
-				const struct fib_entry_notifier_info *fen_info,
-				struct mlxsw_sp_fib_entry *fib_entry)
+mlxsw_sp_fib4_entry_type_set(struct mlxsw_sp *mlxsw_sp,
+			     const struct fib_entry_notifier_info *fen_info,
+			     struct mlxsw_sp_fib_entry *fib_entry)
 {
 	struct fib_info *fi = fen_info->fi;
 	struct mlxsw_sp_rif *r = NULL;
@@ -1763,18 +1834,9 @@ mlxsw_sp_router_fib4_entry_init(struct mlxsw_sp *mlxsw_sp,
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_LOCAL;
 	else
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_REMOTE;
-	fib_info_offload_inc(fen_info->fi);
 	return 0;
 }
 
-static void
-mlxsw_sp_router_fib4_entry_fini(struct mlxsw_sp *mlxsw_sp,
-				struct mlxsw_sp_fib_entry *fib_entry)
-{
-	if (fib_entry->type != MLXSW_SP_FIB_ENTRY_TYPE_TRAP)
-		fib_info_offload_dec(fib_entry->nh_group->key.fi);
-}
-
 static struct mlxsw_sp_fib_entry *
 mlxsw_sp_fib_entry_get(struct mlxsw_sp *mlxsw_sp,
 		       const struct fib_entry_notifier_info *fen_info)
@@ -1807,9 +1869,9 @@ mlxsw_sp_fib_entry_get(struct mlxsw_sp *mlxsw_sp,
 	fib_entry->vr = vr;
 	fib_entry->ref_count = 1;
 
-	err = mlxsw_sp_router_fib4_entry_init(mlxsw_sp, fen_info, fib_entry);
+	err = mlxsw_sp_fib4_entry_type_set(mlxsw_sp, fen_info, fib_entry);
 	if (err)
-		goto err_fib4_entry_init;
+		goto err_fib4_entry_type_set;
 
 	err = mlxsw_sp_nexthop_group_get(mlxsw_sp, fib_entry, fi);
 	if (err)
@@ -1818,8 +1880,7 @@ mlxsw_sp_fib_entry_get(struct mlxsw_sp *mlxsw_sp,
 	return fib_entry;
 
 err_nexthop_group_get:
-	mlxsw_sp_router_fib4_entry_fini(mlxsw_sp, fib_entry);
-err_fib4_entry_init:
+err_fib4_entry_type_set:
 	mlxsw_sp_fib_entry_destroy(fib_entry);
 err_fib_entry_create:
 	mlxsw_sp_vr_put(mlxsw_sp, vr);
@@ -1851,7 +1912,6 @@ static void mlxsw_sp_fib_entry_put(struct mlxsw_sp *mlxsw_sp,
 
 	if (--fib_entry->ref_count == 0) {
 		mlxsw_sp_nexthop_group_put(mlxsw_sp, fib_entry);
-		mlxsw_sp_router_fib4_entry_fini(mlxsw_sp, fib_entry);
 		mlxsw_sp_fib_entry_destroy(fib_entry);
 	}
 	mlxsw_sp_vr_put(mlxsw_sp, vr);
-- 
2.7.4

[patch net-next 10/15] mlxsw: spectrum_router: Determine offload status using generic function

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

The previous patch introduced a generic function to determine whether a
route should be offloaded or not. Make use of it here.

In the future we're going to add more conditions to this test (e.g.,
whether TOS is non-zero), so it makes sense to centralize it instead of
open coding it in a few places.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index acabb35..c452bd3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1696,7 +1696,7 @@ static int mlxsw_sp_fib_entry_op4_remote(struct mlxsw_sp *mlxsw_sp,
 	 * with provided ECMP size. Otherwise, setup trap and pass
 	 * traffic to kernel.
 	 */
-	if (fib_entry->nh_group->adj_index_valid) {
+	if (mlxsw_sp_fib_entry_should_offload(fib_entry)) {
 		trap_action = MLXSW_REG_RALUE_TRAP_ACTION_NOP;
 		adjacency_index = fib_entry->nh_group->adj_index;
 		ecmp_size = fib_entry->nh_group->ecmp_size;
-- 
2.7.4

[patch net-next 11/15] mlxsw: spectrum_router: Use trap action only for some route types

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

The device can have one of three actions associated with a route:

1) Remote - packets continue to the adjacency table
2) Local - packets continue to the neighbour table
3) Trap - packets continue to the CPU

The first two actions can also trap packets to the CPU, but they do so
using a different trap ID, which has a lower traffic class and less
allotted bandwidth.

We currently use the third action for both RTN_{LOCAL,BROADCAST} routes
and RTN_UNICAST routes not pointing to the switch ports.

However, packets that merely need to be forwarded by the switch are
likely not control packets and can be therefore scheduled towards the
CPU using a lower traffic class.

Achieve the above by assigning the third action only to local and
broadcast routes and have any other route use either of the first two
actions, based on whether the route is gatewayed or not.

This will also allow us to refresh routes using the local action and
have them trap packets when their RIF is no longer valid following a
NH_DEL event.

One side effect of this patch is that we no longer give special
treatment to multipath routes using both switch and non-switch ports
towards their nexthops. If at least one of the nexthops can be resolved,
then the device will forward the packets instead of trapping them.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 42 +++++++---------------
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index c452bd3..136a50c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1623,7 +1623,7 @@ mlxsw_sp_fib_entry_should_offload(const struct mlxsw_sp_fib_entry *fib_entry)
 	case MLXSW_SP_FIB_ENTRY_TYPE_REMOTE:
 		return !!nh_group->adj_index_valid;
 	case MLXSW_SP_FIB_ENTRY_TYPE_LOCAL:
-		return true;
+		return !!nh_group->nh_rif;
 	default:
 		return false;
 	}
@@ -1718,16 +1718,25 @@ static int mlxsw_sp_fib_entry_op4_local(struct mlxsw_sp *mlxsw_sp,
 					enum mlxsw_reg_ralue_op op)
 {
 	struct mlxsw_sp_rif *r = fib_entry->nh_group->nh_rif;
+	enum mlxsw_reg_ralue_trap_action trap_action;
 	char ralue_pl[MLXSW_REG_RALUE_LEN];
 	u32 *p_dip = (u32 *) fib_entry->key.addr;
 	struct mlxsw_sp_vr *vr = fib_entry->vr;
+	u16 trap_id = 0;
+	u16 rif = 0;
+
+	if (mlxsw_sp_fib_entry_should_offload(fib_entry)) {
+		trap_action = MLXSW_REG_RALUE_TRAP_ACTION_NOP;
+		rif = r->rif;
+	} else {
+		trap_action = MLXSW_REG_RALUE_TRAP_ACTION_TRAP;
+		trap_id = MLXSW_TRAP_ID_RTR_INGRESS0;
+	}
 
 	mlxsw_reg_ralue_pack4(ralue_pl,
 			      (enum mlxsw_reg_ralxx_protocol) vr->proto, op,
 			      vr->id, fib_entry->key.prefix_len, *p_dip);
-	mlxsw_reg_ralue_act_local_pack(ralue_pl,
-				       MLXSW_REG_RALUE_TRAP_ACTION_NOP, 0,
-				       r->rif);
+	mlxsw_reg_ralue_act_local_pack(ralue_pl, trap_action, trap_id, rif);
 	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(ralue), ralue_pl);
 }
 
@@ -1798,8 +1807,6 @@ mlxsw_sp_fib4_entry_type_set(struct mlxsw_sp *mlxsw_sp,
 			     struct mlxsw_sp_fib_entry *fib_entry)
 {
 	struct fib_info *fi = fen_info->fi;
-	struct mlxsw_sp_rif *r = NULL;
-	int nhsel;
 
 	if (fen_info->type == RTN_LOCAL || fen_info->type == RTN_BROADCAST) {
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
@@ -1807,29 +1814,6 @@ mlxsw_sp_fib4_entry_type_set(struct mlxsw_sp *mlxsw_sp,
 	}
 	if (fen_info->type != RTN_UNICAST)
 		return -EINVAL;
-
-	for (nhsel = 0; nhsel < fi->fib_nhs; nhsel++) {
-		const struct fib_nh *nh = &fi->fib_nh[nhsel];
-
-		if (!nh->nh_dev)
-			continue;
-		r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, nh->nh_dev);
-		if (!r) {
-			/* In case router interface is not found for
-			 * at least one of the nexthops, that means
-			 * the nexthop points to some device unrelated
-			 * to us. Set trap and pass the packets for
-			 * this prefix to kernel.
-			 */
-			break;
-		}
-	}
-
-	if (!r) {
-		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_TRAP;
-		return 0;
-	}
-
 	if (fi->fib_nh->nh_scope != RT_SCOPE_LINK)
 		fib_entry->type = MLXSW_SP_FIB_ENTRY_TYPE_LOCAL;
 	else
-- 
2.7.4

[patch net-next 12/15] ipv4: fib: Notify about nexthop status changes

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

When a multipath route is hit the kernel doesn't consider nexthops that
are DEAD or LINKDOWN when IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN is set.
Devices that offload multipath routes need to be made aware of nexthop
status changes. Otherwise, the device will keep forwarding packets to
non-functional nexthops.

Add the FIB_EVENT_NH_{ADD,DEL} events to the fib notification chain,
which notify capable devices when they should add or delete a nexthop
from their tables.

Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: David Ahern <dsa@cumulusnetworks.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/ip_fib.h     |  7 +++++++
 net/ipv4/fib_semantics.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 57c2a86..45a184e 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -214,11 +214,18 @@ struct fib_entry_notifier_info {
 	u32 nlflags;
 };
 
+struct fib_nh_notifier_info {
+	struct fib_notifier_info info; /* must be first */
+	struct fib_nh *fib_nh;
+};
+
 enum fib_event_type {
 	FIB_EVENT_ENTRY_ADD,
 	FIB_EVENT_ENTRY_DEL,
 	FIB_EVENT_RULE_ADD,
 	FIB_EVENT_RULE_DEL,
+	FIB_EVENT_NH_ADD,
+	FIB_EVENT_NH_DEL,
 };
 
 int register_fib_notifier(struct notifier_block *nb,
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 6306a67..317026a 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1355,6 +1355,36 @@ int fib_sync_down_addr(struct net_device *dev, __be32 local)
 	return ret;
 }
 
+static int call_fib_nh_notifiers(struct fib_nh *fib_nh,
+				 enum fib_event_type event_type)
+{
+	struct in_device *in_dev = __in_dev_get_rtnl(fib_nh->nh_dev);
+	struct fib_nh_notifier_info info = {
+		.fib_nh = fib_nh,
+	};
+
+	switch (event_type) {
+	case FIB_EVENT_NH_ADD:
+		if (fib_nh->nh_flags & RTNH_F_DEAD)
+			break;
+		if (IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
+		    fib_nh->nh_flags & RTNH_F_LINKDOWN)
+			break;
+		return call_fib_notifiers(dev_net(fib_nh->nh_dev), event_type,
+					  &info.info);
+	case FIB_EVENT_NH_DEL:
+		if ((IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
+		     fib_nh->nh_flags & RTNH_F_LINKDOWN) ||
+		    (fib_nh->nh_flags & RTNH_F_DEAD))
+			return call_fib_notifiers(dev_net(fib_nh->nh_dev),
+						  event_type, &info.info);
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 /* Event              force Flags           Description
  * NETDEV_CHANGE      0     LINKDOWN        Carrier OFF, not for scope host
  * NETDEV_DOWN        0     LINKDOWN|DEAD   Link down, not for scope host
@@ -1396,6 +1426,8 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event, bool force)
 					nexthop_nh->nh_flags |= RTNH_F_LINKDOWN;
 					break;
 				}
+				call_fib_nh_notifiers(nexthop_nh,
+						      FIB_EVENT_NH_DEL);
 				dead++;
 			}
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
@@ -1550,6 +1582,7 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
 				continue;
 			alive++;
 			nexthop_nh->nh_flags &= ~nh_flags;
+			call_fib_nh_notifiers(nexthop_nh, FIB_EVENT_NH_ADD);
 		} endfor_nexthops(fi)
 
 		if (alive > 0) {
-- 
2.7.4

Re: [patch net-next 12/15] ipv4: fib: Notify about nexthop status changes

[Andy Gospodarek]

On Wed, Feb 08, 2017 at 11:16:39AM +0100, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> When a multipath route is hit the kernel doesn't consider nexthops that
> are DEAD or LINKDOWN when IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN is set.
> Devices that offload multipath routes need to be made aware of nexthop
> status changes. Otherwise, the device will keep forwarding packets to
> non-functional nexthops.
> 
> Add the FIB_EVENT_NH_{ADD,DEL} events to the fib notification chain,
> which notify capable devices when they should add or delete a nexthop
> from their tables.

This looks good -- thanks for doing this.

IIUC the hardware forwarding use case for your hardware covered by David
Ahern's patch[1] to the ipv4 software path selection is already covered,
so this is probably the last known link/neighbor forwarding issue for
ipv4 that needs coverage.

1. a6db449 net: ipv4: Consider failed nexthops in multipath routes

> Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
> Cc: David Ahern <dsa@cumulusnetworks.com>
> Cc: Andy Gospodarek <andy@greyhouse.net>

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>

> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/net/ip_fib.h     |  7 +++++++
>  net/ipv4/fib_semantics.c | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 57c2a86..45a184e 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -214,11 +214,18 @@ struct fib_entry_notifier_info {
>  	u32 nlflags;
>  };
>  
> +struct fib_nh_notifier_info {
> +	struct fib_notifier_info info; /* must be first */
> +	struct fib_nh *fib_nh;
> +};
> +
>  enum fib_event_type {
>  	FIB_EVENT_ENTRY_ADD,
>  	FIB_EVENT_ENTRY_DEL,
>  	FIB_EVENT_RULE_ADD,
>  	FIB_EVENT_RULE_DEL,
> +	FIB_EVENT_NH_ADD,
> +	FIB_EVENT_NH_DEL,
>  };
>  
>  int register_fib_notifier(struct notifier_block *nb,
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 6306a67..317026a 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -1355,6 +1355,36 @@ int fib_sync_down_addr(struct net_device *dev, __be32 local)
>  	return ret;
>  }
>  
> +static int call_fib_nh_notifiers(struct fib_nh *fib_nh,
> +				 enum fib_event_type event_type)
> +{
> +	struct in_device *in_dev = __in_dev_get_rtnl(fib_nh->nh_dev);
> +	struct fib_nh_notifier_info info = {
> +		.fib_nh = fib_nh,
> +	};
> +
> +	switch (event_type) {
> +	case FIB_EVENT_NH_ADD:
> +		if (fib_nh->nh_flags & RTNH_F_DEAD)
> +			break;
> +		if (IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
> +		    fib_nh->nh_flags & RTNH_F_LINKDOWN)
> +			break;
> +		return call_fib_notifiers(dev_net(fib_nh->nh_dev), event_type,
> +					  &info.info);
> +	case FIB_EVENT_NH_DEL:
> +		if ((IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
> +		     fib_nh->nh_flags & RTNH_F_LINKDOWN) ||
> +		    (fib_nh->nh_flags & RTNH_F_DEAD))
> +			return call_fib_notifiers(dev_net(fib_nh->nh_dev),
> +						  event_type, &info.info);
> +	default:
> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
>  /* Event              force Flags           Description
>   * NETDEV_CHANGE      0     LINKDOWN        Carrier OFF, not for scope host
>   * NETDEV_DOWN        0     LINKDOWN|DEAD   Link down, not for scope host
> @@ -1396,6 +1426,8 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event, bool force)
>  					nexthop_nh->nh_flags |= RTNH_F_LINKDOWN;
>  					break;
>  				}
> +				call_fib_nh_notifiers(nexthop_nh,
> +						      FIB_EVENT_NH_DEL);
>  				dead++;
>  			}
>  #ifdef CONFIG_IP_ROUTE_MULTIPATH
> @@ -1550,6 +1582,7 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
>  				continue;
>  			alive++;
>  			nexthop_nh->nh_flags &= ~nh_flags;
> +			call_fib_nh_notifiers(nexthop_nh, FIB_EVENT_NH_ADD);
>  		} endfor_nexthops(fi)
>  
>  		if (alive > 0) {
> -- 
> 2.7.4
> 

Re: [patch net-next 12/15] ipv4: fib: Notify about nexthop status changes

[Ido Schimmel]

On Wed, Feb 08, 2017 at 09:56:00AM -0500, Andy Gospodarek wrote:
> On Wed, Feb 08, 2017 at 11:16:39AM +0100, Jiri Pirko wrote:
> > From: Ido Schimmel <idosch@mellanox.com>
> > 
> > When a multipath route is hit the kernel doesn't consider nexthops that
> > are DEAD or LINKDOWN when IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN is set.
> > Devices that offload multipath routes need to be made aware of nexthop
> > status changes. Otherwise, the device will keep forwarding packets to
> > non-functional nexthops.
> > 
> > Add the FIB_EVENT_NH_{ADD,DEL} events to the fib notification chain,
> > which notify capable devices when they should add or delete a nexthop
> > from their tables.
> 
> This looks good -- thanks for doing this.
> 
> IIUC the hardware forwarding use case for your hardware covered by David
> Ahern's patch[1] to the ipv4 software path selection is already covered,
> so this is probably the last known link/neighbor forwarding issue for
> ipv4 that needs coverage.
> 
> 1. a6db449 net: ipv4: Consider failed nexthops in multipath routes

Yep, it aligns the kernel's datapath with what we already implemented in
mlxsw. In case the neighbour can't be resolve, then the nexthop isn't
reflected to the device (we need the MAC...).

In the case of multipath routes, if some of the nexthops can be
reflected, then we do so, but periodically ask the kernel to try and
resolve the others. Otherwise, these nexthops will never be resolved, as
the kernel doesn't see the packets hitting the multipath route and
therefore lacks the motivation to resolve its nexthops.

> > Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
> > Cc: David Ahern <dsa@cumulusnetworks.com>
> > Cc: Andy Gospodarek <andy@greyhouse.net>
> 
> Reviewed-by: Andy Gospodarek <gospo@broadcom.com>

Thanks for reviewing!

Re: [patch net-next 12/15] ipv4: fib: Notify about nexthop status changes

[David Ahern]

On 2/8/17 8:32 AM, Ido Schimmel wrote:
> In the case of multipath routes, if some of the nexthops can be
> reflected, then we do so, but periodically ask the kernel to try and
> resolve the others. Otherwise, these nexthops will never be resolved, as
> the kernel doesn't see the packets hitting the multipath route and
> therefore lacks the motivation to resolve its nexthops.

It should get the motivation once the neigh entry is cleaned up. That
can take a long time based on gc settings, but it can also happen from
the remote side if it sends an arp message.

Re: [patch net-next 12/15] ipv4: fib: Notify about nexthop status changes

[Ido Schimmel]

On Wed, Feb 08, 2017 at 11:05:54AM -0700, David Ahern wrote:
> On 2/8/17 8:32 AM, Ido Schimmel wrote:
> > In the case of multipath routes, if some of the nexthops can be
> > reflected, then we do so, but periodically ask the kernel to try and
> > resolve the others. Otherwise, these nexthops will never be resolved, as
> > the kernel doesn't see the packets hitting the multipath route and
> > therefore lacks the motivation to resolve its nexthops.
> 
> It should get the motivation once the neigh entry is cleaned up. That
> can take a long time based on gc settings, but it can also happen from
> the remote side if it sends an arp message.

And when the remote side does that and the neighbour becomes valid we no
longer try to actively resolve it.

Re: [patch net-next 12/15] ipv4: fib: Notify about nexthop status changes

[Andy Gospodarek]

On Wed, Feb 8, 2017 at 5:16 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> From: Ido Schimmel <idosch@mellanox.com>
>
> When a multipath route is hit the kernel doesn't consider nexthops that
> are DEAD or LINKDOWN when IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN is set.
> Devices that offload multipath routes need to be made aware of nexthop
> status changes. Otherwise, the device will keep forwarding packets to
> non-functional nexthops.
>
> Add the FIB_EVENT_NH_{ADD,DEL} events to the fib notification chain,
> which notify capable devices when they should add or delete a nexthop
> from their tables.

This looks good -- thanks for doing this.

IIUC the hardware forwarding use case for your hardware covered by David
Ahern's patch[1] to the ipv4 software path selection is already covered,
so this is probably the last known link/neighbor forwarding issue for
ipv4 that needs coverage.

1. a6db449 net: ipv4: Consider failed nexthops in multipath routes

> Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
> Cc: David Ahern <dsa@cumulusnetworks.com>
> Cc: Andy Gospodarek <andy@greyhouse.net>

Reviewed-by Andy Gospodarek <gospo@broadcom.com>

> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/net/ip_fib.h     |  7 +++++++
>  net/ipv4/fib_semantics.c | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 57c2a86..45a184e 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -214,11 +214,18 @@ struct fib_entry_notifier_info {
>         u32 nlflags;
>  };
>
> +struct fib_nh_notifier_info {
> +       struct fib_notifier_info info; /* must be first */
> +       struct fib_nh *fib_nh;
> +};
> +
>  enum fib_event_type {
>         FIB_EVENT_ENTRY_ADD,
>         FIB_EVENT_ENTRY_DEL,
>         FIB_EVENT_RULE_ADD,
>         FIB_EVENT_RULE_DEL,
> +       FIB_EVENT_NH_ADD,
> +       FIB_EVENT_NH_DEL,
>  };
>
>  int register_fib_notifier(struct notifier_block *nb,
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 6306a67..317026a 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -1355,6 +1355,36 @@ int fib_sync_down_addr(struct net_device *dev, __be32 local)
>         return ret;
>  }
>
> +static int call_fib_nh_notifiers(struct fib_nh *fib_nh,
> +                                enum fib_event_type event_type)
> +{
> +       struct in_device *in_dev = __in_dev_get_rtnl(fib_nh->nh_dev);
> +       struct fib_nh_notifier_info info = {
> +               .fib_nh = fib_nh,
> +       };
> +
> +       switch (event_type) {
> +       case FIB_EVENT_NH_ADD:
> +               if (fib_nh->nh_flags & RTNH_F_DEAD)
> +                       break;
> +               if (IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
> +                   fib_nh->nh_flags & RTNH_F_LINKDOWN)
> +                       break;
> +               return call_fib_notifiers(dev_net(fib_nh->nh_dev), event_type,
> +                                         &info.info);
> +       case FIB_EVENT_NH_DEL:
> +               if ((IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
> +                    fib_nh->nh_flags & RTNH_F_LINKDOWN) ||
> +                   (fib_nh->nh_flags & RTNH_F_DEAD))
> +                       return call_fib_notifiers(dev_net(fib_nh->nh_dev),
> +                                                 event_type, &info.info);
> +       default:
> +               break;
> +       }
> +
> +       return NOTIFY_DONE;
> +}
> +
>  /* Event              force Flags           Description
>   * NETDEV_CHANGE      0     LINKDOWN        Carrier OFF, not for scope host
>   * NETDEV_DOWN        0     LINKDOWN|DEAD   Link down, not for scope host
> @@ -1396,6 +1426,8 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event, bool force)
>                                         nexthop_nh->nh_flags |= RTNH_F_LINKDOWN;
>                                         break;
>                                 }
> +                               call_fib_nh_notifiers(nexthop_nh,
> +                                                     FIB_EVENT_NH_DEL);
>                                 dead++;
>                         }
>  #ifdef CONFIG_IP_ROUTE_MULTIPATH
> @@ -1550,6 +1582,7 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
>                                 continue;
>                         alive++;
>                         nexthop_nh->nh_flags &= ~nh_flags;
> +                       call_fib_nh_notifiers(nexthop_nh, FIB_EVENT_NH_ADD);
>                 } endfor_nexthops(fi)
>
>                 if (alive > 0) {
> --
> 2.7.4
>

[patch net-next 13/15] mlxsw: spectrum_router: Reflect nexthop status changes

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

When a packet hits a multipath route in the device's routing table, a
hash is computed over its headers, which is then used to select the
appropriate nexthop from the device's adjacency table.

There are situations in which the kernel removes a nexthop from a
multipath route (e.g., no carrier) and the device should do the same.

Upon the reception of NH_{ADD,DEL} events, add or remove a nexthop from
the device's adjacency table and refresh all the routes using the
nexthop group. If all the nexthops of a multipath route are invalid,
then any packet hitting the route would be trapped to the CPU for
forwarding.

If all the nexthops are DEAD, then the kernel would remove the route
entirely. On the other hand, if all the nexthops are merely LINKDOWN,
then the kernel would keep the route and forward any incoming packet
using a different route.

While the last case might sound like a problem, it's expected that a
routing daemon running in user space would remove such a route from the
FIB as it's dumped with the DEAD flag set.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 59 +++++++++++++++++++++-
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 136a50c..1c68b40 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1180,6 +1180,14 @@ static void mlxsw_sp_nexthop_remove(struct mlxsw_sp *mlxsw_sp,
 			       mlxsw_sp_nexthop_ht_params);
 }
 
+static struct mlxsw_sp_nexthop *
+mlxsw_sp_nexthop_lookup(struct mlxsw_sp *mlxsw_sp,
+			struct mlxsw_sp_nexthop_key key)
+{
+	return rhashtable_lookup_fast(&mlxsw_sp->router.nexthop_ht, &key,
+				      mlxsw_sp_nexthop_ht_params);
+}
+
 static int mlxsw_sp_adj_index_mass_update_vr(struct mlxsw_sp *mlxsw_sp,
 					     struct mlxsw_sp_vr *vr,
 					     u32 adj_index, u16 ecmp_size,
@@ -1417,7 +1425,7 @@ static int mlxsw_sp_nexthop_neigh_init(struct mlxsw_sp *mlxsw_sp,
 	u8 nud_state, dead;
 	int err;
 
-	if (!nh->nh_grp->gateway)
+	if (!nh->nh_grp->gateway || nh->neigh_entry)
 		return 0;
 
 	/* Take a reference of neigh here ensuring that neigh would
@@ -1527,6 +1535,39 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
 	mlxsw_sp_nexthop_remove(mlxsw_sp, nh);
 }
 
+static void mlxsw_sp_nexthop_event(struct mlxsw_sp *mlxsw_sp,
+				   unsigned long event, struct fib_nh *fib_nh)
+{
+	struct mlxsw_sp_nexthop_key key;
+	struct mlxsw_sp_nexthop *nh;
+	struct mlxsw_sp_rif *r;
+
+	if (mlxsw_sp->router.aborted)
+		return;
+
+	key.fib_nh = fib_nh;
+	nh = mlxsw_sp_nexthop_lookup(mlxsw_sp, key);
+	if (WARN_ON_ONCE(!nh))
+		return;
+
+	r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, fib_nh->nh_dev);
+	if (!r)
+		return;
+
+	switch (event) {
+	case FIB_EVENT_NH_ADD:
+		nh->r = r;
+		mlxsw_sp_nexthop_neigh_init(mlxsw_sp, nh);
+		break;
+	case FIB_EVENT_NH_DEL:
+		mlxsw_sp_nexthop_neigh_fini(mlxsw_sp, nh);
+		nh->r = NULL;
+		break;
+	}
+
+	mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh->nh_grp);
+}
+
 static struct mlxsw_sp_nexthop_group *
 mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
 {
@@ -2085,7 +2126,10 @@ static void __mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp)
 
 struct mlxsw_sp_fib_event_work {
 	struct work_struct work;
-	struct fib_entry_notifier_info fen_info;
+	union {
+		struct fib_entry_notifier_info fen_info;
+		struct fib_nh_notifier_info fnh_info;
+	};
 	struct mlxsw_sp *mlxsw_sp;
 	unsigned long event;
 };
@@ -2114,6 +2158,12 @@ static void mlxsw_sp_router_fib_event_work(struct work_struct *work)
 	case FIB_EVENT_RULE_DEL:
 		mlxsw_sp_router_fib4_abort(mlxsw_sp);
 		break;
+	case FIB_EVENT_NH_ADD: /* fall through */
+	case FIB_EVENT_NH_DEL:
+		mlxsw_sp_nexthop_event(mlxsw_sp, fib_work->event,
+				       fib_work->fnh_info.fib_nh);
+		fib_info_put(fib_work->fnh_info.fib_nh->nh_parent);
+		break;
 	}
 	rtnl_unlock();
 	kfree(fib_work);
@@ -2147,6 +2197,11 @@ static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
 		 */
 		fib_info_hold(fib_work->fen_info.fi);
 		break;
+	case FIB_EVENT_NH_ADD: /* fall through */
+	case FIB_EVENT_NH_DEL:
+		memcpy(&fib_work->fnh_info, ptr, sizeof(fib_work->fnh_info));
+		fib_info_hold(fib_work->fnh_info.fib_nh->nh_parent);
+		break;
 	}
 
 	mlxsw_core_schedule_work(&fib_work->work);
-- 
2.7.4

[patch net-next 14/15] mlxsw: spectrum_router: Don't reflect LINKDOWN nexthops

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

The kernel resolves the nexthops for a given route using
FIB_LOOKUP_IGNORE_LINKSTATE which means a notification can be sent for a
route with one of its nexthops being LINKDOWN.

In case IGNORE_ROUTES_WITH_LINKDOWN is set for the nexthop netdev, then
we shouldn't reflect the nexthop to the device's table.

Once the nexthop netdev's carrier goes up we'll be notified using NH_ADD
and reflect it to the device.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 1c68b40..8dfc025 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -40,6 +40,7 @@
 #include <linux/bitops.h>
 #include <linux/in6.h>
 #include <linux/notifier.h>
+#include <linux/inetdevice.h>
 #include <net/netevent.h>
 #include <net/neighbour.h>
 #include <net/arp.h>
@@ -1503,6 +1504,7 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 				 struct fib_nh *fib_nh)
 {
 	struct net_device *dev = fib_nh->nh_dev;
+	struct in_device *in_dev;
 	struct mlxsw_sp_rif *r;
 	int err;
 
@@ -1512,6 +1514,11 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 	if (err)
 		return err;
 
+	in_dev = __in_dev_get_rtnl(dev);
+	if (in_dev && IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
+	    fib_nh->nh_flags & RTNH_F_LINKDOWN)
+		return 0;
+
 	r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
 	if (!r)
 		return 0;
-- 
2.7.4

[patch net-next 15/15] mlxsw: spectrum_router: Flush resources when RIF is deleted

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

When the last IP address is removed from a netdev, its RIF is deleted.
However, if user didn't first remove neighbours and nexthops using this
interface, then they would still be present in the device's tables.

Therefore, whenever a RIF is deleted, make sure all the neighbours and
nexthops (adjacency entries) using it are removed from the relevant
tables as well.

The action associated with any route using this RIF would be refreshed,
most likely to trap. If the kernel decides to remove the route (f.e.,
because all the nexthops are now DEAD), then an event would be sent,
causing the route to be removed from the device.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |  6 ++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  4 +
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 86 +++++++++++++++++++++-
 3 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index c2a6751..8de5ed5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3473,6 +3473,8 @@ mlxsw_sp_rif_alloc(u16 rif, struct net_device *l3_dev, struct mlxsw_sp_fid *f)
 	if (!r)
 		return NULL;
 
+	INIT_LIST_HEAD(&r->nexthop_list);
+	INIT_LIST_HEAD(&r->neigh_list);
 	ether_addr_copy(r->addr, l3_dev->dev_addr);
 	r->mtu = l3_dev->mtu;
 	r->ref_count = 1;
@@ -3541,6 +3543,8 @@ static void mlxsw_sp_vport_rif_sp_destroy(struct mlxsw_sp_port *mlxsw_sp_vport,
 	u16 fid = f->fid;
 	u16 rif = r->rif;
 
+	mlxsw_sp_router_rif_gone_sync(mlxsw_sp, r);
+
 	mlxsw_sp->rifs[rif] = NULL;
 	f->r = NULL;
 
@@ -3770,6 +3774,8 @@ void mlxsw_sp_rif_bridge_destroy(struct mlxsw_sp *mlxsw_sp,
 	struct mlxsw_sp_fid *f = r->f;
 	u16 rif = r->rif;
 
+	mlxsw_sp_router_rif_gone_sync(mlxsw_sp, r);
+
 	mlxsw_sp->rifs[rif] = NULL;
 	f->r = NULL;
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index eb743fe..145897c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -108,6 +108,8 @@ struct mlxsw_sp_fid {
 };
 
 struct mlxsw_sp_rif {
+	struct list_head nexthop_list;
+	struct list_head neigh_list;
 	struct net_device *dev;
 	unsigned int ref_count;
 	struct mlxsw_sp_fid *f;
@@ -602,6 +604,8 @@ int mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp);
 void mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp);
 int mlxsw_sp_router_netevent_event(struct notifier_block *unused,
 				   unsigned long event, void *ptr);
+void mlxsw_sp_router_rif_gone_sync(struct mlxsw_sp *mlxsw_sp,
+				   struct mlxsw_sp_rif *r);
 
 int mlxsw_sp_kvdl_alloc(struct mlxsw_sp *mlxsw_sp, unsigned int entry_count);
 void mlxsw_sp_kvdl_free(struct mlxsw_sp *mlxsw_sp, int entry_index);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 8dfc025..ac8944e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -610,6 +610,7 @@ struct mlxsw_sp_neigh_key {
 };
 
 struct mlxsw_sp_neigh_entry {
+	struct list_head rif_list_node;
 	struct rhash_head ht_node;
 	struct mlxsw_sp_neigh_key key;
 	u16 rif;
@@ -686,6 +687,8 @@ mlxsw_sp_neigh_entry_create(struct mlxsw_sp *mlxsw_sp, struct neighbour *n)
 	if (err)
 		goto err_neigh_entry_insert;
 
+	list_add(&neigh_entry->rif_list_node, &r->neigh_list);
+
 	return neigh_entry;
 
 err_neigh_entry_insert:
@@ -697,6 +700,7 @@ static void
 mlxsw_sp_neigh_entry_destroy(struct mlxsw_sp *mlxsw_sp,
 			     struct mlxsw_sp_neigh_entry *neigh_entry)
 {
+	list_del(&neigh_entry->rif_list_node);
 	mlxsw_sp_neigh_entry_remove(mlxsw_sp, neigh_entry);
 	mlxsw_sp_neigh_entry_free(neigh_entry);
 }
@@ -1090,12 +1094,34 @@ static void mlxsw_sp_neigh_fini(struct mlxsw_sp *mlxsw_sp)
 	rhashtable_destroy(&mlxsw_sp->router.neigh_ht);
 }
 
+static int mlxsw_sp_neigh_rif_flush(struct mlxsw_sp *mlxsw_sp,
+				    const struct mlxsw_sp_rif *r)
+{
+	char rauht_pl[MLXSW_REG_RAUHT_LEN];
+
+	mlxsw_reg_rauht_pack(rauht_pl, MLXSW_REG_RAUHT_OP_WRITE_DELETE_ALL,
+			     r->rif, r->addr);
+	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(rauht), rauht_pl);
+}
+
+static void mlxsw_sp_neigh_rif_gone_sync(struct mlxsw_sp *mlxsw_sp,
+					 struct mlxsw_sp_rif *r)
+{
+	struct mlxsw_sp_neigh_entry *neigh_entry, *tmp;
+
+	mlxsw_sp_neigh_rif_flush(mlxsw_sp, r);
+	list_for_each_entry_safe(neigh_entry, tmp, &r->neigh_list,
+				 rif_list_node)
+		mlxsw_sp_neigh_entry_destroy(mlxsw_sp, neigh_entry);
+}
+
 struct mlxsw_sp_nexthop_key {
 	struct fib_nh *fib_nh;
 };
 
 struct mlxsw_sp_nexthop {
 	struct list_head neigh_list_node; /* member of neigh entry list */
+	struct list_head rif_list_node;
 	struct mlxsw_sp_nexthop_group *nh_grp; /* pointer back to the group
 						* this belongs to
 						*/
@@ -1417,6 +1443,25 @@ mlxsw_sp_nexthop_neigh_update(struct mlxsw_sp *mlxsw_sp,
 	}
 }
 
+static void mlxsw_sp_nexthop_rif_init(struct mlxsw_sp_nexthop *nh,
+				      struct mlxsw_sp_rif *r)
+{
+	if (nh->r)
+		return;
+
+	nh->r = r;
+	list_add(&nh->rif_list_node, &r->nexthop_list);
+}
+
+static void mlxsw_sp_nexthop_rif_fini(struct mlxsw_sp_nexthop *nh)
+{
+	if (!nh->r)
+		return;
+
+	list_del(&nh->rif_list_node);
+	nh->r = NULL;
+}
+
 static int mlxsw_sp_nexthop_neigh_init(struct mlxsw_sp *mlxsw_sp,
 				       struct mlxsw_sp_nexthop *nh)
 {
@@ -1522,7 +1567,7 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 	r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
 	if (!r)
 		return 0;
-	nh->r = r;
+	mlxsw_sp_nexthop_rif_init(nh, r);
 
 	err = mlxsw_sp_nexthop_neigh_init(mlxsw_sp, nh);
 	if (err)
@@ -1539,6 +1584,7 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
 				  struct mlxsw_sp_nexthop *nh)
 {
 	mlxsw_sp_nexthop_neigh_fini(mlxsw_sp, nh);
+	mlxsw_sp_nexthop_rif_fini(nh);
 	mlxsw_sp_nexthop_remove(mlxsw_sp, nh);
 }
 
@@ -1563,18 +1609,30 @@ static void mlxsw_sp_nexthop_event(struct mlxsw_sp *mlxsw_sp,
 
 	switch (event) {
 	case FIB_EVENT_NH_ADD:
-		nh->r = r;
+		mlxsw_sp_nexthop_rif_init(nh, r);
 		mlxsw_sp_nexthop_neigh_init(mlxsw_sp, nh);
 		break;
 	case FIB_EVENT_NH_DEL:
 		mlxsw_sp_nexthop_neigh_fini(mlxsw_sp, nh);
-		nh->r = NULL;
+		mlxsw_sp_nexthop_rif_fini(nh);
 		break;
 	}
 
 	mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh->nh_grp);
 }
 
+static void mlxsw_sp_nexthop_rif_gone_sync(struct mlxsw_sp *mlxsw_sp,
+					   struct mlxsw_sp_rif *r)
+{
+	struct mlxsw_sp_nexthop *nh, *tmp;
+
+	list_for_each_entry_safe(nh, tmp, &r->nexthop_list, rif_list_node) {
+		mlxsw_sp_nexthop_neigh_fini(mlxsw_sp, nh);
+		mlxsw_sp_nexthop_rif_fini(nh);
+		mlxsw_sp_nexthop_group_refresh(mlxsw_sp, nh->nh_grp);
+	}
+}
+
 static struct mlxsw_sp_nexthop_group *
 mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
 {
@@ -2089,6 +2147,28 @@ static void mlxsw_sp_router_fib4_abort(struct mlxsw_sp *mlxsw_sp)
 		dev_warn(mlxsw_sp->bus_info->dev, "Failed to set abort trap.\n");
 }
 
+static int mlxsw_sp_router_rif_disable(struct mlxsw_sp *mlxsw_sp, u16 rif)
+{
+	char ritr_pl[MLXSW_REG_RITR_LEN];
+	int err;
+
+	mlxsw_reg_ritr_rif_pack(ritr_pl, rif);
+	err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(ritr), ritr_pl);
+	if (WARN_ON_ONCE(err))
+		return err;
+
+	mlxsw_reg_ritr_enable_set(ritr_pl, false);
+	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(ritr), ritr_pl);
+}
+
+void mlxsw_sp_router_rif_gone_sync(struct mlxsw_sp *mlxsw_sp,
+				   struct mlxsw_sp_rif *r)
+{
+	mlxsw_sp_router_rif_disable(mlxsw_sp, r->rif);
+	mlxsw_sp_nexthop_rif_gone_sync(mlxsw_sp, r);
+	mlxsw_sp_neigh_rif_gone_sync(mlxsw_sp, r);
+}
+
 static int __mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
 {
 	char rgcr_pl[MLXSW_REG_RGCR_LEN];
-- 
2.7.4

[patch net-next 14/15] mlxsw: spectrum_router: Don't reflect LINKDOWN nexthops

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

The kernel resolves the nexthops for a given route using
FIB_LOOKUP_IGNORE_LINKSTATE which means a notification can be sent for a
route with one of its nexthops being LINKDOWN.

In case IGNORE_ROUTES_WITH_LINKDOWN is set for the nexthop netdev, then
we shouldn't reflect the nexthop to the device's table.

Once the nexthop netdev's carrier goes up we'll be notified using NH_ADD
and reflect it to the device.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 1c68b40..8dfc025 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -40,6 +40,7 @@
 #include <linux/bitops.h>
 #include <linux/in6.h>
 #include <linux/notifier.h>
+#include <linux/inetdevice.h>
 #include <net/netevent.h>
 #include <net/neighbour.h>
 #include <net/arp.h>
@@ -1503,6 +1504,7 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 				 struct fib_nh *fib_nh)
 {
 	struct net_device *dev = fib_nh->nh_dev;
+	struct in_device *in_dev;
 	struct mlxsw_sp_rif *r;
 	int err;
 
@@ -1512,6 +1514,11 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
 	if (err)
 		return err;
 
+	in_dev = __in_dev_get_rtnl(dev);
+	if (in_dev && IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
+	    fib_nh->nh_flags & RTNH_F_LINKDOWN)
+		return 0;
+
 	r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
 	if (!r)
 		return 0;
-- 
2.7.4

Re: [patch net-next 00/15] mlxsw: Reflect nexthop status changes

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed,  8 Feb 2017 11:16:27 +0100

> From: Jiri Pirko <jiri@mellanox.com>
> 
> Ido says:
> 
> When the kernel forwards IPv4 packets via multipath routes it doesn't
> consider nexthops that are dead or linkdown. For example, if the nexthop
> netdev is administratively down or doesn't have a carrier.
> 
> Devices capable of offloading such multipath routes need to be made
> aware of changes in the reflected nexthops' status. Otherwise, the
> device might forward packets via non-functional nexthops, resulting in
> packet loss. This patchset aims to fix that.
> 
> The first 11 patches deal with the necessary restructuring in the
> mlxsw driver, so that it's able to correctly add and remove nexthops
> from the device's adjacency table.
> 
> The 12th patch adds the NH_{ADD,DEL} events to the FIB notification
> chain. These notifications are sent whenever the kernel decides to add
> or remove a nexthop from the forwarding plane.
> 
> Finally, the last three patches add support for these events in the
> mlxsw driver, which is currently the only driver capable of offloading
> multipath routes.

Looks really nice, series applied, thanks!

Re: [patch net-next 00/15] mlxsw: Reflect nexthop status changes

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Wed, 08 Feb 2017 15:28:48 -0500 (EST)

> Looks really nice, series applied, thanks!

Jiri, just FYI, I bungled up merging this.  And I am trying to fix
that up.

I forgot to include patch #14, but I'll apply that now in the
mainline.

I get a warning, in mlxsw_sp_nexthop_group_create() because the
compiler thinks that 'nh' can be use uninitialized.  And I think
the compiler is pointing out something legitimate.

This cleanup loop in err_nexthop_group_insert and err_nexthop_init
needs to take the 'nh' from &nh_grp->nexthops[i] instead of using
whatever is left in 'nh' for all mlxsw_sp_nexthop_fini() calls.

So I'm going to augment the commit of patch #14 to make it go:

@@ -1667,8 +1667,10 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
 
 err_nexthop_group_insert:
 err_nexthop_init:
-	for (i--; i >= 0; i--)
+	for (i--; i >= 0; i--) {
+		nh = &nh_grp->nexthops[i];
 		mlxsw_sp_nexthop_fini(mlxsw_sp, nh);
+	}
 	kfree(nh_grp);
 	return ERR_PTR(err);
 }

Re: [patch net-next 00/15] mlxsw: Reflect nexthop status changes

[Jiri Pirko]

Wed, Feb 08, 2017 at 09:43:45PM CET, davem@davemloft.net wrote:
>From: David Miller <davem@davemloft.net>
>Date: Wed, 08 Feb 2017 15:28:48 -0500 (EST)
>
>> Looks really nice, series applied, thanks!
>
>Jiri, just FYI, I bungled up merging this.  And I am trying to fix
>that up.
>
>I forgot to include patch #14, but I'll apply that now in the
>mainline.

Yeah, the lost patch #14. I had to resend it to get it to patchwork.
That's probably what confused your tools.


>
>I get a warning, in mlxsw_sp_nexthop_group_create() because the
>compiler thinks that 'nh' can be use uninitialized.  And I think
>the compiler is pointing out something legitimate.
>
>This cleanup loop in err_nexthop_group_insert and err_nexthop_init
>needs to take the 'nh' from &nh_grp->nexthops[i] instead of using
>whatever is left in 'nh' for all mlxsw_sp_nexthop_fini() calls.
>
>So I'm going to augment the commit of patch #14 to make it go:
>
>@@ -1667,8 +1667,10 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
> 
> err_nexthop_group_insert:
> err_nexthop_init:
>-	for (i--; i >= 0; i--)
>+	for (i--; i >= 0; i--) {
>+		nh = &nh_grp->nexthops[i];
> 		mlxsw_sp_nexthop_fini(mlxsw_sp, nh);
>+	}
> 	kfree(nh_grp);
> 	return ERR_PTR(err);
> }

Oh. I did not catch is. But this is totally unrelated to this patchset.
Please send it as a separate fix. Or I can do it if yo want.

Fixes: a7ff87acd995 ("mlxsw: spectrum_router: Implement next-hop routing")
Acked-by: Jiri Pirko <jiri@mellanox.com>

Re: [patch net-next 00/15] mlxsw: Reflect nexthop status changes

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 8 Feb 2017 21:58:35 +0100

> Wed, Feb 08, 2017 at 09:43:45PM CET, davem@davemloft.net wrote:
>>From: David Miller <davem@davemloft.net>
>>Date: Wed, 08 Feb 2017 15:28:48 -0500 (EST)
>>
>>> Looks really nice, series applied, thanks!
>>
>>Jiri, just FYI, I bungled up merging this.  And I am trying to fix
>>that up.
>>
>>I forgot to include patch #14, but I'll apply that now in the
>>mainline.
> 
> Yeah, the lost patch #14. I had to resend it to get it to patchwork.
> That's probably what confused your tools.
> 
> 
>>
>>I get a warning, in mlxsw_sp_nexthop_group_create() because the
>>compiler thinks that 'nh' can be use uninitialized.  And I think
>>the compiler is pointing out something legitimate.
>>
>>This cleanup loop in err_nexthop_group_insert and err_nexthop_init
>>needs to take the 'nh' from &nh_grp->nexthops[i] instead of using
>>whatever is left in 'nh' for all mlxsw_sp_nexthop_fini() calls.
>>
>>So I'm going to augment the commit of patch #14 to make it go:
>>
>>@@ -1667,8 +1667,10 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
>> 
>> err_nexthop_group_insert:
>> err_nexthop_init:
>>-	for (i--; i >= 0; i--)
>>+	for (i--; i >= 0; i--) {
>>+		nh = &nh_grp->nexthops[i];
>> 		mlxsw_sp_nexthop_fini(mlxsw_sp, nh);
>>+	}
>> 	kfree(nh_grp);
>> 	return ERR_PTR(err);
>> }
> 
> Oh. I did not catch is. But this is totally unrelated to this patchset.
> Please send it as a separate fix. Or I can do it if yo want.
> 
> Fixes: a7ff87acd995 ("mlxsw: spectrum_router: Implement next-hop routing")
> Acked-by: Jiri Pirko <jiri@mellanox.com>

Sorry, I integrated it into the patch #14 commit, and it's already pushed
out.

Re: [patch net-next 00/15] mlxsw: Reflect nexthop status changes

[Ido Schimmel]

On Wed, Feb 08, 2017 at 03:43:45PM -0500, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 08 Feb 2017 15:28:48 -0500 (EST)
> 
> > Looks really nice, series applied, thanks!
> 
> Jiri, just FYI, I bungled up merging this.  And I am trying to fix
> that up.
> 
> I forgot to include patch #14, but I'll apply that now in the
> mainline.
> 
> I get a warning, in mlxsw_sp_nexthop_group_create() because the
> compiler thinks that 'nh' can be use uninitialized.  And I think
> the compiler is pointing out something legitimate.
> 
> This cleanup loop in err_nexthop_group_insert and err_nexthop_init
> needs to take the 'nh' from &nh_grp->nexthops[i] instead of using
> whatever is left in 'nh' for all mlxsw_sp_nexthop_fini() calls.
> 
> So I'm going to augment the commit of patch #14 to make it go:
> 
> @@ -1667,8 +1667,10 @@ mlxsw_sp_nexthop_group_create(struct mlxsw_sp *mlxsw_sp, struct fib_info *fi)
>  
>  err_nexthop_group_insert:
>  err_nexthop_init:
> -	for (i--; i >= 0; i--)
> +	for (i--; i >= 0; i--) {
> +		nh = &nh_grp->nexthops[i];
>  		mlxsw_sp_nexthop_fini(mlxsw_sp, nh);
> +	}

Looks good to me. Thanks for fixing that up!

This is now consistent with mlxsw_sp_nexthop_group_destroy().


>  	kfree(nh_grp);
>  	return ERR_PTR(err);
>  }
>