netdev - Re: [PATCH v2 net-next 2/5] ipv4: Add fib_nh_common to fib

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1c7ae34e-db63-a834-6623-1170ec8899a7@gmail.com>
Date:   Mon, 1 Apr 2019 12:47:17 -0600
From:   David Ahern <dsahern@...il.com>
To:     Martin Lau <kafai@...com>, David Ahern <dsahern@...nel.org>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "idosch@...lanox.com" <idosch@...lanox.com>
Subject: Re: [PATCH v2 net-next 2/5] ipv4: Add fib_nh_common to fib_result

On 4/1/19 12:20 PM, Martin Lau wrote:
>> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
>> index 12a6d759cf57..5070bc531ca4 100644
>> --- a/include/net/ip_fib.h
>> +++ b/include/net/ip_fib.h
>> @@ -156,15 +156,16 @@ struct fib_rule;
>>  
>>  struct fib_table;
>>  struct fib_result {
>> -	__be32		prefix;
>> -	unsigned char	prefixlen;
>> -	unsigned char	nh_sel;
>> -	unsigned char	type;
>> -	unsigned char	scope;
>> -	u32		tclassid;
>> -	struct fib_info *fi;
>> -	struct fib_table *table;
>> -	struct hlist_head *fa_head;
>> +	__be32			prefix;
>> +	unsigned char		prefixlen;
>> +	unsigned char		nh_sel;
> Is nh_sel still needed?

yes, for fib_result_nl API and nl_fib_lookup.


>> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
>> index df777af7e278..f81c7dc7ff59 100644
>> --- a/net/ipv4/fib_semantics.c
>> +++ b/net/ipv4/fib_semantics.c
>> @@ -1075,6 +1075,24 @@ __be32 fib_info_update_nh_saddr(struct net *net, struct fib_nh *nh)
>>  	return nh->nh_saddr;
>>  }
>>  
>> +__be32 fib_result_prefsrc(struct net *net, struct fib_result *res)
>> +{
>> +	struct fib_nh_common *nhc = res->nhc;
>> +	struct fib_nh *nh;
>> +
>> +	if (res->fi->fib_prefsrc)
>> +		return res->fi->fib_prefsrc;
>> +
>> +	if (unlikely(nhc->nhc_family != AF_INET))
> Comparing with FIB_RES_PREFSRC, it is not immediately obvious
> why this test is needed.  May be a comment or a commit log message?

This branch is for the case of v6 with v4. I can remove from this patch
and add later. Right now, nhc will only be from a fib_nh.

> 
>> +		return inet_select_addr(nhc->nhc_dev, 0, res->fi->fib_scope);
>> +
>> +	nh = container_of(nhc, struct fib_nh, nh_common);
>> +	if (nh->nh_saddr_genid == atomic_read(&net->ipv4.dev_addr_genid))
>> +		return nh->nh_saddr;
>> +
>> +	return fib_info_update_nh_saddr(net, nh);
>> +}
>> +
>>  static bool fib_valid_prefsrc(struct fib_config *cfg, __be32 fib_prefsrc)
>>  {
>>  	if (cfg->fc_type != RTN_LOCAL || !cfg->fc_dst ||
>> @@ -1762,20 +1780,22 @@ void fib_select_multipath(struct fib_result *res, int hash)
>>  	struct net *net = fi->fib_net;
>>  	bool first = false;
>>  
>> -	for_nexthops(fi) {
>> +	change_nexthops(fi) {
> Is it mainly because of the 'const'?

yes.