Re: [PATCH RFC net-next 07/20] net/ipv6: Move nexthop data to fib6_nh

[Wei Wang]

On Mon, Feb 26, 2018 at 2:47 PM, David Ahern  wrote:
> On 2/26/18 3:28 PM, Wei Wang wrote:
>> On Sun, Feb 25, 2018 at 11:47 AM, David Ahern  wrote:
>>> Introduce fib6_nh structure and move nexthop related data from
>>> rt6_info and rt6_info.dst to fib6_nh. References to dev, gateway or
>>> lwtstate from a FIB lookup perspective are converted to use fib6_nh;
>>> datapath references to dst version are left as is.
>>>
>>
>> My understanding is that after your whole patch series, sibling routes
>> will still have their own fib6_info. Does it make sense to make this
>> fib6_nh as an array in fib6_info so that sibling routes will share
>> fib6_info but will have their own fib6_nh as a future improvement? It
>> matches ipv4 behavior. And I think it will make the sibling route
>> handling code easier?
>
> I was not planning to. IPv6 allowing individual nexthops to be added and
> deleted is very convenient. I do agree the existing sibling route
> linkage makes the code much more complicated than it needs to be.
>
> After this set, I plan to send patches for nexthops as separate objects
> - which will have an impact on how multipath routes are done. With
> nexthop objects there will be 1 prefix route pointing to a nexthop
> object that is multipath (meaning it points in turn to a series of
> nexthop objects). This provides the simplification (no sibling linkage)
> without losing the individual nexhtop add / delete option.

Got it. Thanks for the explanation.

Re: [PATCH RFC net-next 07/20] net/ipv6: Move nexthop data to fib6_nh

[David Ahern]

On 2/26/18 3:28 PM, Wei Wang wrote:
> On Sun, Feb 25, 2018 at 11:47 AM, David Ahern  wrote:
>> Introduce fib6_nh structure and move nexthop related data from
>> rt6_info and rt6_info.dst to fib6_nh. References to dev, gateway or
>> lwtstate from a FIB lookup perspective are converted to use fib6_nh;
>> datapath references to dst version are left as is.
>>
> 
> My understanding is that after your whole patch series, sibling routes
> will still have their own fib6_info. Does it make sense to make this
> fib6_nh as an array in fib6_info so that sibling routes will share
> fib6_info but will have their own fib6_nh as a future improvement? It
> matches ipv4 behavior. And I think it will make the sibling route
> handling code easier?

I was not planning to. IPv6 allowing individual nexthops to be added and
deleted is very convenient. I do agree the existing sibling route
linkage makes the code much more complicated than it needs to be.

After this set, I plan to send patches for nexthops as separate objects
- which will have an impact on how multipath routes are done. With
nexthop objects there will be 1 prefix route pointing to a nexthop
object that is multipath (meaning it points in turn to a series of
nexthop objects). This provides the simplification (no sibling linkage)
without losing the individual nexhtop add / delete option.

Re: [PATCH RFC net-next 07/20] net/ipv6: Move nexthop data to fib6_nh

[Wei Wang]

On Sun, Feb 25, 2018 at 11:47 AM, David Ahern  wrote:
> Introduce fib6_nh structure and move nexthop related data from
> rt6_info and rt6_info.dst to fib6_nh. References to dev, gateway or
> lwtstate from a FIB lookup perspective are converted to use fib6_nh;
> datapath references to dst version are left as is.
>

My understanding is that after your whole patch series, sibling routes
will still have their own fib6_info. Does it make sense to make this
fib6_nh as an array in fib6_info so that sibling routes will share
fib6_info but will have their own fib6_nh as a future improvement? It
matches ipv4 behavior. And I think it will make the sibling route
handling code easier?

> Signed-off-by: David Ahern 
> ---
>  .../net/ethernet/mellanox/mlxsw/spectrum_router.c  |  32 ++--
>  include/net/ip6_fib.h  |  16 +-
>  include/net/ip6_route.h|   6 +-
>  net/ipv6/addrconf.c|   2 +-
>  net/ipv6/ip6_fib.c |   6 +-
>  net/ipv6/route.c   | 164 
> -
>  6 files changed, 127 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c 
> b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 05146970c19c..90d01df783b3 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -2700,9 +2700,9 @@ mlxsw_sp_nexthop6_group_cmp(const struct 
> mlxsw_sp_nexthop_group *nh_grp,
> struct in6_addr *gw;
> int ifindex, weight;
>
> -   ifindex = mlxsw_sp_rt6->rt->dst.dev->ifindex;
> -   weight = mlxsw_sp_rt6->rt->rt6i_nh_weight;
> -   gw = _sp_rt6->rt->rt6i_gateway;
> +   ifindex = mlxsw_sp_rt6->rt->fib6_nh.nh_dev->ifindex;
> +   weight = mlxsw_sp_rt6->rt->fib6_nh.nh_weight;
> +   gw = _sp_rt6->rt->fib6_nh.nh_gw;
> if (!mlxsw_sp_nexthop6_group_has_nexthop(nh_grp, gw, ifindex,
>  weight))
> return false;
> @@ -2768,7 +2768,7 @@ mlxsw_sp_nexthop6_group_hash(struct mlxsw_sp_fib6_entry 
> *fib6_entry, u32 seed)
> struct net_device *dev;
>
> list_for_each_entry(mlxsw_sp_rt6, _entry->rt6_list, list) {
> -   dev = mlxsw_sp_rt6->rt->dst.dev;
> +   dev = mlxsw_sp_rt6->rt->fib6_nh.nh_dev;
> val ^= dev->ifindex;
> }
>
> @@ -3766,9 +3766,9 @@ mlxsw_sp_rt6_nexthop(struct mlxsw_sp_nexthop_group 
> *nh_grp,
> struct mlxsw_sp_nexthop *nh = _grp->nexthops[i];
> struct rt6_info *rt = mlxsw_sp_rt6->rt;
>
> -   if (nh->rif && nh->rif->dev == rt->dst.dev &&
> +   if (nh->rif && nh->rif->dev == rt->fib6_nh.nh_dev &&
> ipv6_addr_equal((const struct in6_addr *) >gw_addr,
> -   >rt6i_gateway))
> +   >fib6_nh.nh_gw))
> return nh;
> continue;
> }
> @@ -3825,7 +3825,7 @@ mlxsw_sp_fib6_entry_offload_set(struct 
> mlxsw_sp_fib_entry *fib_entry)
>
> if (fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_LOCAL) {
> list_first_entry(_entry->rt6_list, struct mlxsw_sp_rt6,
> -list)->rt->rt6i_nh_flags |= RTNH_F_OFFLOAD;
> +list)->rt->fib6_nh.nh_flags |= 
> RTNH_F_OFFLOAD;
> return;
> }
>
> @@ -3835,9 +3835,9 @@ mlxsw_sp_fib6_entry_offload_set(struct 
> mlxsw_sp_fib_entry *fib_entry)
>
> nh = mlxsw_sp_rt6_nexthop(nh_grp, mlxsw_sp_rt6);
> if (nh && nh->offloaded)
> -   mlxsw_sp_rt6->rt->rt6i_nh_flags |= RTNH_F_OFFLOAD;
> +   mlxsw_sp_rt6->rt->fib6_nh.nh_flags |= RTNH_F_OFFLOAD;
> else
> -   mlxsw_sp_rt6->rt->rt6i_nh_flags &= ~RTNH_F_OFFLOAD;
> +   mlxsw_sp_rt6->rt->fib6_nh.nh_flags &= ~RTNH_F_OFFLOAD;
> }
>  }
>
> @@ -3852,7 +3852,7 @@ mlxsw_sp_fib6_entry_offload_unset(struct 
> mlxsw_sp_fib_entry *fib_entry)
> list_for_each_entry(mlxsw_sp_rt6, _entry->rt6_list, list) {
> struct rt6_info *rt = mlxsw_sp_rt6->rt;
>
> -   rt->rt6i_nh_flags &= ~RTNH_F_OFFLOAD;
> +   rt->fib6_nh.nh_flags &= ~RTNH_F_OFFLOAD;
> }
>  }
>
> @@ -4748,8 +4748,8 @@ static bool mlxsw_sp_nexthop6_ipip_type(const struct 
> mlxsw_sp *mlxsw_sp,
> const struct rt6_info *rt,
> enum mlxsw_sp_ipip_type *ret)
>  {
> -   return rt->dst.dev &&
> -  mlxsw_sp_netdev_ipip_type(mlxsw_sp, rt->dst.dev, ret);
> +   return rt->fib6_nh.nh_dev &&
> + 

[PATCH RFC net-next 07/20] net/ipv6: Move nexthop data to fib6_nh

[David Ahern]

Introduce fib6_nh structure and move nexthop related data from
rt6_info and rt6_info.dst to fib6_nh. References to dev, gateway or
lwtstate from a FIB lookup perspective are converted to use fib6_nh;
datapath references to dst version are left as is.

Signed-off-by: David Ahern 
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  |  32 ++--
 include/net/ip6_fib.h  |  16 +-
 include/net/ip6_route.h|   6 +-
 net/ipv6/addrconf.c|   2 +-
 net/ipv6/ip6_fib.c |   6 +-
 net/ipv6/route.c   | 164 -
 6 files changed, 127 insertions(+), 99 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c 
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 05146970c19c..90d01df783b3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2700,9 +2700,9 @@ mlxsw_sp_nexthop6_group_cmp(const struct 
mlxsw_sp_nexthop_group *nh_grp,
struct in6_addr *gw;
int ifindex, weight;
 
-   ifindex = mlxsw_sp_rt6->rt->dst.dev->ifindex;
-   weight = mlxsw_sp_rt6->rt->rt6i_nh_weight;
-   gw = _sp_rt6->rt->rt6i_gateway;
+   ifindex = mlxsw_sp_rt6->rt->fib6_nh.nh_dev->ifindex;
+   weight = mlxsw_sp_rt6->rt->fib6_nh.nh_weight;
+   gw = _sp_rt6->rt->fib6_nh.nh_gw;
if (!mlxsw_sp_nexthop6_group_has_nexthop(nh_grp, gw, ifindex,
 weight))
return false;
@@ -2768,7 +2768,7 @@ mlxsw_sp_nexthop6_group_hash(struct mlxsw_sp_fib6_entry 
*fib6_entry, u32 seed)
struct net_device *dev;
 
list_for_each_entry(mlxsw_sp_rt6, _entry->rt6_list, list) {
-   dev = mlxsw_sp_rt6->rt->dst.dev;
+   dev = mlxsw_sp_rt6->rt->fib6_nh.nh_dev;
val ^= dev->ifindex;
}
 
@@ -3766,9 +3766,9 @@ mlxsw_sp_rt6_nexthop(struct mlxsw_sp_nexthop_group 
*nh_grp,
struct mlxsw_sp_nexthop *nh = _grp->nexthops[i];
struct rt6_info *rt = mlxsw_sp_rt6->rt;
 
-   if (nh->rif && nh->rif->dev == rt->dst.dev &&
+   if (nh->rif && nh->rif->dev == rt->fib6_nh.nh_dev &&
ipv6_addr_equal((const struct in6_addr *) >gw_addr,
-   >rt6i_gateway))
+   >fib6_nh.nh_gw))
return nh;
continue;
}
@@ -3825,7 +3825,7 @@ mlxsw_sp_fib6_entry_offload_set(struct mlxsw_sp_fib_entry 
*fib_entry)
 
if (fib_entry->type == MLXSW_SP_FIB_ENTRY_TYPE_LOCAL) {
list_first_entry(_entry->rt6_list, struct mlxsw_sp_rt6,
-list)->rt->rt6i_nh_flags |= RTNH_F_OFFLOAD;
+list)->rt->fib6_nh.nh_flags |= RTNH_F_OFFLOAD;
return;
}
 
@@ -3835,9 +3835,9 @@ mlxsw_sp_fib6_entry_offload_set(struct mlxsw_sp_fib_entry 
*fib_entry)
 
nh = mlxsw_sp_rt6_nexthop(nh_grp, mlxsw_sp_rt6);
if (nh && nh->offloaded)
-   mlxsw_sp_rt6->rt->rt6i_nh_flags |= RTNH_F_OFFLOAD;
+   mlxsw_sp_rt6->rt->fib6_nh.nh_flags |= RTNH_F_OFFLOAD;
else
-   mlxsw_sp_rt6->rt->rt6i_nh_flags &= ~RTNH_F_OFFLOAD;
+   mlxsw_sp_rt6->rt->fib6_nh.nh_flags &= ~RTNH_F_OFFLOAD;
}
 }
 
@@ -3852,7 +3852,7 @@ mlxsw_sp_fib6_entry_offload_unset(struct 
mlxsw_sp_fib_entry *fib_entry)
list_for_each_entry(mlxsw_sp_rt6, _entry->rt6_list, list) {
struct rt6_info *rt = mlxsw_sp_rt6->rt;
 
-   rt->rt6i_nh_flags &= ~RTNH_F_OFFLOAD;
+   rt->fib6_nh.nh_flags &= ~RTNH_F_OFFLOAD;
}
 }
 
@@ -4748,8 +4748,8 @@ static bool mlxsw_sp_nexthop6_ipip_type(const struct 
mlxsw_sp *mlxsw_sp,
const struct rt6_info *rt,
enum mlxsw_sp_ipip_type *ret)
 {
-   return rt->dst.dev &&
-  mlxsw_sp_netdev_ipip_type(mlxsw_sp, rt->dst.dev, ret);
+   return rt->fib6_nh.nh_dev &&
+  mlxsw_sp_netdev_ipip_type(mlxsw_sp, rt->fib6_nh.nh_dev, ret);
 }
 
 static int mlxsw_sp_nexthop6_type_init(struct mlxsw_sp *mlxsw_sp,
@@ -4759,7 +4759,7 @@ static int mlxsw_sp_nexthop6_type_init(struct mlxsw_sp 
*mlxsw_sp,
 {
const struct mlxsw_sp_ipip_ops *ipip_ops;
struct mlxsw_sp_ipip_entry *ipip_entry;
-   struct net_device *dev = rt->dst.dev;
+   struct net_device *dev = rt->fib6_nh.nh_dev;
struct mlxsw_sp_rif *rif;
int err;
 
@@ -4802,11 +4802,11 @@ static int mlxsw_sp_nexthop6_init(struct mlxsw_sp 
*mlxsw_sp,
  struct mlxsw_sp_nexthop