Re: [PATCH v2 net-next 5/5] ipv6: Flip to fib_nexthop_info

[David Ahern <dsahern@gmail.com>]

On 4/1/19 2:15 PM, Martin Lau wrote:
>> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>> index 6a8f8cd4d6db..7e8d5ed116fe 100644
>> --- a/net/ipv4/fib_semantics.c
>> +++ b/net/ipv4/fib_semantics.c
>> @@ -45,6 +45,7 @@
>>  #include <net/nexthop.h>
>>  #include <net/lwtunnel.h>
>>  #include <net/fib_notifier.h>
>> +#include <net/addrconf.h>
>>  
>>  #include "fib_lookup.h"
>>  
>> @@ -1320,8 +1321,8 @@ struct fib_info *fib_create_info(struct fib_config *cfg,
>>  	return ERR_PTR(err);
>>  }
>>  
>> -static int fib_nexthop_info(struct sk_buff *skb, const struct fib_nh_common *nh,
>> -			    unsigned int *flags, bool skip_oif)
>> +int fib_nexthop_info(struct sk_buff *skb, const struct fib_nh_common *nh,
> nh should be renamed to nhc in Patch 4?

for consistency it should. Will fix.

> 
>> +		     unsigned int *flags, bool skip_oif, bool for_ipv4)
> I assume "bool for_ipv4" will be used in a later patch also, can it be
> removed from the current patch?

yes, will remove.

> 
>>  {
>>  	if (nh->nhc_flags & RTNH_F_DEAD)
>>  		*flags |= RTNH_F_DEAD;
>> @@ -1335,6 +1336,10 @@ static int fib_nexthop_info(struct sk_buff *skb, const struct fib_nh_common *nh,
>>  			if (ip_ignore_linkdown(nh->nhc_dev))
>>  				*flags |= RTNH_F_DEAD;
>>  			break;
>> +		case AF_INET6:
>> +			if (ip6_ignore_linkdown(nh->nhc_dev))
>> +				*flags |= RTNH_F_DEAD;
>> +			break;
>>  		}
>>  		rcu_read_unlock();
>>  	}
>> @@ -1345,6 +1350,11 @@ static int fib_nexthop_info(struct sk_buff *skb, const struct fib_nh_common *nh,
>>  			if (nla_put_in_addr(skb, RTA_GATEWAY, nh->nhc_gw.ipv4))
>>  				goto nla_put_failure;
>>  			break;
>> +		case AF_INET6:
>> +			if (nla_put_in6_addr(skb, RTA_GATEWAY,
>> +					     &nh->nhc_gw.ipv6) < 0)
>> +				goto nla_put_failure;
>> +			break;
>>  		}
>>  	}
>>  
> 
> [ ... ]
> 
>> -/* add multipath next hop */
>> -static int rt6_add_nexthop(struct sk_buff *skb, const struct fib6_nh *fib6_nh)
>> -{
>> -	const struct net_device *dev = fib6_nh->fib_nh_dev;
>> -	struct rtnexthop *rtnh;
>> -	unsigned int flags = 0;
>> -
>> -	rtnh = nla_reserve_nohdr(skb, sizeof(*rtnh));
>> -	if (!rtnh)
>> -		goto nla_put_failure;
>> -
>> -	rtnh->rtnh_hops = fib6_nh->fib_nh_weight - 1;
>> -	rtnh->rtnh_ifindex = dev ? dev->ifindex : 0;
>> -
>> -	if (rt6_nexthop_info(skb, fib6_nh, &flags, true) < 0)
>> -		goto nla_put_failure;
>> -
>> -	rtnh->rtnh_flags = flags;
>> -
>> -	/* length of rtnetlink header + attributes */
>> -	rtnh->rtnh_len = nlmsg_get_pos(skb) - (void *)rtnh;
>> -
>> -	return 0;
>> -
>> -nla_put_failure:
>> -	return -EMSGSIZE;
>> -}
>> -
>>  static int rt6_fill_node(struct net *net, struct sk_buff *skb,
>>  			 struct fib6_info *rt, struct dst_entry *dst,
> Off topic question question,
> 
> After "net/ipv6: Flip FIB entries to fib6_info" (which was a great step forward),
> any update on the followup in renaming "struct fib6_info *rt" to
> "struct fib6_info *f6i"?
> 

There are still places that use rt instead of f6i. I could do a sweep of
the code. I have been trying to fix as I go whenever it makes sense -
e.g., the name change does not distract from the goal of the patch.