netdev - Re: [PATCH net-next 2/3] net: ipv4: flush neighbor entries when carrier is off

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: <04601732-fa0e-e8e9-58de-28f0509d8b31@gmail.com>
Date:   Mon, 6 Nov 2017 23:49:54 +0900
From:   David Ahern <dsahern@...il.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, jhs@...atatu.com,
        Satish Ashok <sashok@...ulusnetworks.com>
Subject: Re: [PATCH net-next 2/3] net: ipv4: flush neighbor entries when
 carrier is off

On 11/6/17 7:47 PM, Ido Schimmel wrote:
> Hi David,
> 
> On Sun, Nov 05, 2017 at 10:57:52PM -0800, David Ahern wrote:
>> Commit a6db4494d218c ("net: ipv4: Consider failed nexthops in multipath
>> routes") added support for checking neighbor state when selecting a path
>> for multipath route lookups. It works but incurs a delay waiting for
>> the neighbor entry to timeout. Improve the path selection by flushing
>> non-permanent neighbor entries when carrier is off.
>>
>> Signed-off-by: Satish Ashok <sashok@...ulusnetworks.com>
>> Signed-off-by: David Ahern <dsahern@...il.com>
>> ---
>>  net/ipv4/fib_frontend.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
>> index f02819134ba2..aa8fea74858f 100644
>> --- a/net/ipv4/fib_frontend.c
>> +++ b/net/ipv4/fib_frontend.c
>> @@ -1226,10 +1226,13 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
>>  		break;
>>  	case NETDEV_CHANGE:
>>  		flags = dev_get_flags(dev);
>> -		if (flags & (IFF_RUNNING | IFF_LOWER_UP))
>> +		if (flags & (IFF_RUNNING | IFF_LOWER_UP)) {
>>  			fib_sync_up(dev, RTNH_F_LINKDOWN);
>> -		else
>> +		} else {
>>  			fib_sync_down_dev(dev, event, false);
>> +			if (IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev))
> 
> Can you please explain why the flushing is conditioned on this sysctl
> being enabled? If carrier is down and the sysctl is enabled, then the
> nexthop will not be used anyway.

I pulled the patch from our queue in response to a problem Jamal reported.

For the output path, the result set by fib_table_lookup is ignored and
fib_select_path is invoked which for multipath can pick a nexthop with
carrier down. Perhaps that is the better change ... to check dead and
linkdown flags in fib_select_multipath. I'll try it out tomorrow.