[patch net-next 0/6] ipv4: fib: Provide per-nexthop offload indication

[patch net-next 0/6] ipv4: fib: Provide per-nexthop offload indication

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Ido says:

Offload indication for IPv4 routes is currently set in the FIB info's
flags. When multipath routes are employed, this can lead to a route being
marked as offloaded although only one of its nexthops is actually
offloaded.

Instead, this patchset aims to proivde a higher resolution for the offload
indication and report it on a per-nexthop basis.

Example output from patched iproute:

$ ip route show 192.168.200.0/24
192.168.200.0/24
        nexthop via 192.168.100.2 dev enp3s0np7 weight 1 offload
        nexthop via 192.168.101.3 dev enp3s0np8 weight 1

And once the second gateway is resolved:

$ ip route show 192.168.200.0/24
192.168.200.0/24
        nexthop via 192.168.100.2 dev enp3s0np7 weight 1 offload
        nexthop via 192.168.101.3 dev enp3s0np8 weight 1 offload

First patch teaches the kernel to look for the offload indication in the
nexthop flags. Patches 2-5 adjust current capable drivers to provide
offload indication on a per-nexthop basis. Last patch removes no longer
used functions to set offload indication in the FIB info's flags.

Ido Schimmel (6):
  ipv4: fib: Set offload indication according to nexthop flags
  rocker: Provide offload indication using nexthop flags
  mlxsw: spectrum_router: Provide offload indication using nexthop flags
  mlxsw: spectrum_router: Don't check state when refreshing offload
    indication
  mlxsw: spectrum_router: Refresh offload indication upon group refresh
  ipv4: fib: Remove unused functions

 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 73 ++++++++++++++++++----
 drivers/net/ethernet/rocker/rocker_ofdpa.c         |  6 +-
 include/net/ip_fib.h                               | 13 ----
 net/ipv4/fib_semantics.c                           |  2 +
 4 files changed, 65 insertions(+), 29 deletions(-)

-- 
2.9.3

[patch net-next 1/6] ipv4: fib: Set offload indication according to nexthop flags

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

We're going to have capable drivers indicate route offload using the
nexthop flags, but for non-multipath routes these flags aren't dumped to
user space.

Instead, set the offload indication in the route message flags.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/ipv4/fib_semantics.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index b8d1817..f62dc24 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1342,6 +1342,8 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
 			    IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev))
 				rtm->rtm_flags |= RTNH_F_DEAD;
 		}
+		if (fi->fib_nh->nh_flags & RTNH_F_OFFLOAD)
+			rtm->rtm_flags |= RTNH_F_OFFLOAD;
 #ifdef CONFIG_IP_ROUTE_CLASSID
 		if (fi->fib_nh[0].nh_tclassid &&
 		    nla_put_u32(skb, RTA_FLOW, fi->fib_nh[0].nh_tclassid))
-- 
2.9.3

[patch net-next 2/6] rocker: Provide offload indication using nexthop flags

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

We want to stop using the FIB info's flags to provide the offlaod
indication and instead do that on a per-nexthop basis.

Convert rocker to do just that. It only supports one nexthop per-route,
so conversion is simple.

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

diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index 600e30e..da4e26b 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -2761,7 +2761,7 @@ static int ofdpa_fib4_add(struct rocker *rocker,
 				  fen_info->tb_id, 0);
 	if (err)
 		return err;
-	fib_info_offload_inc(fen_info->fi);
+	fen_info->fi->fib_nh->nh_flags |= RTNH_F_OFFLOAD;
 	return 0;
 }
 
@@ -2776,7 +2776,7 @@ static int ofdpa_fib4_del(struct rocker *rocker,
 	ofdpa_port = ofdpa_port_dev_lower_find(fen_info->fi->fib_dev, rocker);
 	if (!ofdpa_port)
 		return 0;
-	fib_info_offload_dec(fen_info->fi);
+	fen_info->fi->fib_nh->nh_flags &= ~RTNH_F_OFFLOAD;
 	return ofdpa_port_fib_ipv4(ofdpa_port, htonl(fen_info->dst),
 				   fen_info->dst_len, fen_info->fi,
 				   fen_info->tb_id, OFDPA_OP_FLAG_REMOVE);
@@ -2803,7 +2803,7 @@ static void ofdpa_fib4_abort(struct rocker *rocker)
 						       rocker);
 		if (!ofdpa_port)
 			continue;
-		fib_info_offload_dec(flow_entry->fi);
+		flow_entry->fi->fib_nh->nh_flags &= ~RTNH_F_OFFLOAD;
 		ofdpa_flow_tbl_del(ofdpa_port, OFDPA_OP_FLAG_REMOVE,
 				   flow_entry);
 	}
-- 
2.9.3

[patch net-next 3/6] mlxsw: spectrum_router: Provide offload indication using nexthop flags

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

In a similar fashion to previous patch, use the nexthop flags to provide
offload indication instead of the FIB info's flags.

In case a nexthop in a multipath route can't be offloaded (gateway's MAC
can't be resolved, for example), then its offload flag isn't set.

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

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index add03fa..4cdeedf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2103,13 +2103,47 @@ mlxsw_sp_fib_entry_should_offload(const struct mlxsw_sp_fib_entry *fib_entry)
 	}
 }
 
+static void
+mlxsw_sp_fib4_entry_offload_set(struct mlxsw_sp_fib_entry *fib_entry)
+{
+	struct mlxsw_sp_nexthop_group *nh_grp = fib_entry->nh_group;
+	int i;
+
+	if (fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_LOCAL) {
+		nh_grp->nexthops->key.fib_nh->nh_flags |= RTNH_F_OFFLOAD;
+		return;
+	}
+
+	for (i = 0; i < nh_grp->count; i++) {
+		struct mlxsw_sp_nexthop *nh = &nh_grp->nexthops[i];
+
+		if (nh->offloaded)
+			nh->key.fib_nh->nh_flags |= RTNH_F_OFFLOAD;
+		else
+			nh->key.fib_nh->nh_flags &= ~RTNH_F_OFFLOAD;
+	}
+}
+
+static void
+mlxsw_sp_fib4_entry_offload_unset(struct mlxsw_sp_fib_entry *fib_entry)
+{
+	struct mlxsw_sp_nexthop_group *nh_grp = fib_entry->nh_group;
+	int i;
+
+	for (i = 0; i < nh_grp->count; i++) {
+		struct mlxsw_sp_nexthop *nh = &nh_grp->nexthops[i];
+
+		nh->key.fib_nh->nh_flags &= ~RTNH_F_OFFLOAD;
+	}
+}
+
 static void mlxsw_sp_fib_entry_offload_set(struct mlxsw_sp_fib_entry *fib_entry)
 {
 	fib_entry->offloaded = true;
 
 	switch (fib_entry->fib_node->fib->proto) {
 	case MLXSW_SP_L3_PROTO_IPV4:
-		fib_info_offload_inc(fib_entry->nh_group->key.fi);
+		mlxsw_sp_fib4_entry_offload_set(fib_entry);
 		break;
 	case MLXSW_SP_L3_PROTO_IPV6:
 		WARN_ON_ONCE(1);
@@ -2121,7 +2155,7 @@ mlxsw_sp_fib_entry_offload_unset(struct mlxsw_sp_fib_entry *fib_entry)
 {
 	switch (fib_entry->fib_node->fib->proto) {
 	case MLXSW_SP_L3_PROTO_IPV4:
-		fib_info_offload_dec(fib_entry->nh_group->key.fi);
+		mlxsw_sp_fib4_entry_offload_unset(fib_entry);
 		break;
 	case MLXSW_SP_L3_PROTO_IPV6:
 		WARN_ON_ONCE(1);
-- 
2.9.3

[patch net-next 4/6] mlxsw: spectrum_router: Don't check state when refreshing offload indication

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

Previous patch removed the reliance on the counter in the FIB info to
set the offload indication, so we no longer need to keep an offload
state on each FIB entry and can just set or unset the RTNH_F_OFFLOAD
flag in each nexthop.

This is also necessary because we're going to need to refresh the
offload indication whenever the nexthop group associated with the FIB
entry is refreshed. Current check would prevent us from marking a newly
resolved nexthop as offloaded if the FIB entry is already marked as
offloaded.

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

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 4cdeedf..40aecbc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -394,7 +394,6 @@ struct mlxsw_sp_fib_entry {
 	enum mlxsw_sp_fib_entry_type type;
 	struct list_head nexthop_group_node;
 	struct mlxsw_sp_nexthop_group *nh_group;
-	bool offloaded;
 };
 
 struct mlxsw_sp_fib4_entry {
@@ -2139,8 +2138,6 @@ mlxsw_sp_fib4_entry_offload_unset(struct mlxsw_sp_fib_entry *fib_entry)
 
 static void mlxsw_sp_fib_entry_offload_set(struct mlxsw_sp_fib_entry *fib_entry)
 {
-	fib_entry->offloaded = true;
-
 	switch (fib_entry->fib_node->fib->proto) {
 	case MLXSW_SP_L3_PROTO_IPV4:
 		mlxsw_sp_fib4_entry_offload_set(fib_entry);
@@ -2160,8 +2157,6 @@ mlxsw_sp_fib_entry_offload_unset(struct mlxsw_sp_fib_entry *fib_entry)
 	case MLXSW_SP_L3_PROTO_IPV6:
 		WARN_ON_ONCE(1);
 	}
-
-	fib_entry->offloaded = false;
 }
 
 static void
@@ -2170,17 +2165,13 @@ mlxsw_sp_fib_entry_offload_refresh(struct mlxsw_sp_fib_entry *fib_entry,
 {
 	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)
+		if (mlxsw_sp_fib_entry_should_offload(fib_entry))
 			mlxsw_sp_fib_entry_offload_set(fib_entry);
-		else if (!mlxsw_sp_fib_entry_should_offload(fib_entry) &&
-			 fib_entry->offloaded)
+		else if (!mlxsw_sp_fib_entry_should_offload(fib_entry))
 			mlxsw_sp_fib_entry_offload_unset(fib_entry);
 		return;
 	default:
-- 
2.9.3

[patch net-next 5/6] mlxsw: spectrum_router: Refresh offload indication upon group refresh

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

Now that we provide offload indication using the nexthop's flags we must
refresh the offload indication whenever the offload state within the
group changes.

This didn't matter until now, as offload indication was provided using
the FIB info flags and multipath routes were marked as offloaded as long
as one of the nexthops was offloaded.

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

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 40aecbc..2f03c7e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1653,6 +1653,24 @@ mlxsw_sp_nexthop_fib_entries_update(struct mlxsw_sp *mlxsw_sp,
 }
 
 static void
+mlxsw_sp_fib_entry_offload_refresh(struct mlxsw_sp_fib_entry *fib_entry,
+				   enum mlxsw_reg_ralue_op op, int err);
+
+static void
+mlxsw_sp_nexthop_fib_entries_refresh(struct mlxsw_sp_nexthop_group *nh_grp)
+{
+	enum mlxsw_reg_ralue_op op = MLXSW_REG_RALUE_OP_WRITE_WRITE;
+	struct mlxsw_sp_fib_entry *fib_entry;
+
+	list_for_each_entry(fib_entry, &nh_grp->fib_list, nexthop_group_node) {
+		if (!mlxsw_sp_fib_node_entry_is_first(fib_entry->fib_node,
+						      fib_entry))
+			continue;
+		mlxsw_sp_fib_entry_offload_refresh(fib_entry, op, 0);
+	}
+}
+
+static void
 mlxsw_sp_nexthop_group_refresh(struct mlxsw_sp *mlxsw_sp,
 			       struct mlxsw_sp_nexthop_group *nh_grp)
 {
@@ -1739,6 +1757,10 @@ mlxsw_sp_nexthop_group_refresh(struct mlxsw_sp *mlxsw_sp,
 		dev_warn(mlxsw_sp->bus_info->dev, "Failed to mass-update adjacency index for nexthop group.\n");
 		goto set_trap;
 	}
+
+	/* Offload state within the group changed, so update the flags. */
+	mlxsw_sp_nexthop_fib_entries_refresh(nh_grp);
+
 	return;
 
 set_trap:
-- 
2.9.3

[patch net-next 6/6] ipv4: fib: Remove unused functions

[Jiri Pirko]

From: Ido Schimmel <idosch@mellanox.com>

Previous patches converted users of these functions to provide offload
indication using the nexthop's flags instead of the FIB info's.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/ip_fib.h | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 41d580c..ef8992d 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -124,7 +124,6 @@ struct fib_info {
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 	int			fib_weight;
 #endif
-	unsigned int		fib_offload_cnt;
 	struct rcu_head		rcu;
 	struct fib_nh		fib_nh[0];
 #define fib_dev		fib_nh[0].nh_dev
@@ -177,18 +176,6 @@ struct fib_result_nl {
 
 __be32 fib_info_update_nh_saddr(struct net *net, struct fib_nh *nh);
 
-static inline void fib_info_offload_inc(struct fib_info *fi)
-{
-	fi->fib_offload_cnt++;
-	fi->fib_flags |= RTNH_F_OFFLOAD;
-}
-
-static inline void fib_info_offload_dec(struct fib_info *fi)
-{
-	if (--fi->fib_offload_cnt == 0)
-		fi->fib_flags &= ~RTNH_F_OFFLOAD;
-}
-
 #define FIB_RES_SADDR(net, res)				\
 	((FIB_RES_NH(res).nh_saddr_genid ==		\
 	  atomic_read(&(net)->ipv4.dev_addr_genid)) ?	\
-- 
2.9.3

Re: [patch net-next 1/6] ipv4: fib: Set offload indication according to nexthop flags

[David Ahern]

On 8/2/17 1:56 AM, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> We're going to have capable drivers indicate route offload using the
> nexthop flags, but for non-multipath routes these flags aren't dumped to
> user space.
> 
> Instead, set the offload indication in the route message flags.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  net/ipv4/fib_semantics.c | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: David Ahern <dsahern@gmail.com>

Re: [patch net-next 6/6] ipv4: fib: Remove unused functions

[David Ahern]

On 8/2/17 1:56 AM, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> Previous patches converted users of these functions to provide offload
> indication using the nexthop's flags instead of the FIB info's.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/net/ip_fib.h | 13 -------------
>  1 file changed, 13 deletions(-)
> 

Acked-by: David Ahern <dsahern@gmail.com>

Re: [patch net-next 3/6] mlxsw: spectrum_router: Provide offload indication using nexthop flags

[David Ahern]

On 8/2/17 1:56 AM, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> In a similar fashion to previous patch, use the nexthop flags to provide
> offload indication instead of the FIB info's flags.
> 
> In case a nexthop in a multipath route can't be offloaded (gateway's MAC
> can't be resolved, for example), then its offload flag isn't set.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 38 ++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
>

Tested-by: David Ahern <dsahern@gmail.com>

Re: [patch net-next 5/6] mlxsw: spectrum_router: Refresh offload indication upon group refresh

[David Ahern]

On 8/2/17 1:56 AM, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> Now that we provide offload indication using the nexthop's flags we must
> refresh the offload indication whenever the offload state within the
> group changes.
> 
> This didn't matter until now, as offload indication was provided using
> the FIB info flags and multipath routes were marked as offloaded as long
> as one of the nexthops was offloaded.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 

Tested-by: David Ahern <dsahern@gmail.com>

Re: [patch net-next 4/6] mlxsw: spectrum_router: Don't check state when refreshing offload indication

[David Ahern]

On 8/2/17 1:56 AM, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> Previous patch removed the reliance on the counter in the FIB info to
> set the offload indication, so we no longer need to keep an offload
> state on each FIB entry and can just set or unset the RTNH_F_OFFLOAD
> flag in each nexthop.
> 
> This is also necessary because we're going to need to refresh the
> offload indication whenever the nexthop group associated with the FIB
> entry is refreshed. Current check would prevent us from marking a newly
> resolved nexthop as offloaded if the FIB entry is already marked as
> offloaded.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)

Tested-by: David Ahern <dsahern@gmail.com>

Re: [patch net-next 0/6] ipv4: fib: Provide per-nexthop offload indication

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed,  2 Aug 2017 09:56:00 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> Ido says:
> 
> Offload indication for IPv4 routes is currently set in the FIB info's
> flags. When multipath routes are employed, this can lead to a route being
> marked as offloaded although only one of its nexthops is actually
> offloaded.
> 
> Instead, this patchset aims to proivde a higher resolution for the offload
> indication and report it on a per-nexthop basis.
> 
> Example output from patched iproute:
> 
> $ ip route show 192.168.200.0/24
> 192.168.200.0/24
>         nexthop via 192.168.100.2 dev enp3s0np7 weight 1 offload
>         nexthop via 192.168.101.3 dev enp3s0np8 weight 1
> 
> And once the second gateway is resolved:
> 
> $ ip route show 192.168.200.0/24
> 192.168.200.0/24
>         nexthop via 192.168.100.2 dev enp3s0np7 weight 1 offload
>         nexthop via 192.168.101.3 dev enp3s0np8 weight 1 offload
> 
> First patch teaches the kernel to look for the offload indication in the
> nexthop flags. Patches 2-5 adjust current capable drivers to provide
> offload indication on a per-nexthop basis. Last patch removes no longer
> used functions to set offload indication in the FIB info's flags.

Series applied, thanks Jiri.