netdev - Re: [PATCH net-next v2 1/3] net: ipv6: Allow shorthand delete of all nexthops in multipath route

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 16 Jan 2017 18:27:36 -0700
From:   David Ahern <dsa@...ulusnetworks.com>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, ddutt@...ulusnetworks.com
Subject: Re: [PATCH net-next v2 1/3] net: ipv6: Allow shorthand delete of all
 nexthops in multipath route

On 1/16/17 5:51 PM, David Miller wrote:
> From: David Ahern <dsa@...ulusnetworks.com>
> Date: Sun, 15 Jan 2017 12:07:04 -0800
> 
>> @@ -2143,6 +2143,26 @@ int ip6_del_rt(struct rt6_info *rt)
>>  	return __ip6_del_rt(rt, &info);
>>  }
>>  
>> +/* called with table lock held */
>  ...
>> @@ -2176,10 +2196,9 @@ static int ip6_route_del(struct fib6_config *cfg)
>>  				continue;
>>  			if (cfg->fc_protocol && cfg->fc_protocol != rt->rt6i_protocol)
>>  				continue;
>> -			dst_hold(&rt->dst);
>> -			read_unlock_bh(&table->tb6_lock);
>>  
>> -			return __ip6_del_rt(rt, &cfg->fc_nlinfo);
>> +			err = __ip6_route_del(rt, cfg);
>> +			break;
>>  		}
> 
> fib6_del() (invoked by __ip6_route_del()) has to be invoked with the
> table lock held a sa writer, but here you are only holding it as a
> reader.

That table lock is still held. If you look up 2 lines I remove the line that releases the lock.

> 
> I also think some more thought has to be put into whether we can
> change behavior like this without using a flag, as suggested by Roopa.
> 

I'm all for changing the behavior by default, but AIUI that breaks the golden rule.

libnl as of libnl3_2_16-16-g29b71371e764  (a patch by Roopa) already handles both flavors -- if RTA_MULTIPATH is sent it handles it fine and if a series of routes are sent they are consolidated to 1 object with next hops.

iproute2 can handle it just fine as well. I made no changes to get the display format consistent. The problem here is scripts that rely on the old format.

quagga does *not* correctly handle the RTA_MULTIPATH attribute for IPv6 but then it does not properly handle IPv6 multipath routes it receives from the kernel at all. Same result with current kernel code or with this patch set - only the last nexthop is retained. (quagga routes pushed to kernel works, but kernel routes fed to quagga does not)

--

Notifications are still single route based. There is no user input to handle backwards compatibility in converting those to RTA_MULTIPATH. A global sysctl is the only real option I guess.

Anyways, I am open to either; just looking to further close the ipv4 / ipv6 gap.