Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] net: ipv6: fix __rt6_purge_dflt_routers when forwarding
 is not set on all ifaces
To:     Brian Vazquez <brianvv@google.com>
Cc:     Eric Dumazet <edumazet@google.com>,
        Brian Vazquez <brianvv.kernel@gmail.com>,
        "David S . Miller" <davem@davemloft.net>,
        LKML <linux-kernel@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>,
        David Ahern <dsa@cumulusnetworks.com>
References: <20200901065758.1141786-1-brianvv@google.com>
 <CANn89iKA5Ut4AcZfsZi3bVpE33_pqgO=E1RhBzePUeBDn6gznQ@mail.gmail.com>
 <52832405-fa37-38fb-b8fb-d7bd7a0d1d52@gmail.com>
 <CAMzD94TmFiJRfgLp44z1GQ1zzg2Zy7o2Oa9GTTCed0kj5tLdLg@mail.gmail.com>
From:   David Ahern <dsahern@gmail.com>
Message-ID: <267ab500-18e3-d870-343e-499d0e96f989@gmail.com>
Date:   Tue, 1 Sep 2020 10:39:31 -0600
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0)
 Gecko/20100101 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <CAMzD94TmFiJRfgLp44z1GQ1zzg2Zy7o2Oa9GTTCed0kj5tLdLg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 9/1/20 9:50 AM, Brian Vazquez wrote:
> Hey David,
> 
> On Tue, Sep 1, 2020 at 7:57 AM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 9/1/20 1:56 AM, Eric Dumazet wrote:
>>> On Tue, Sep 1, 2020 at 8:58 AM Brian Vazquez <brianvv@google.com> wrote:
>>>>
>>>> The problem is exposed when the system has multiple ifaces and
>>>> forwarding is enabled on a subset of them, __rt6_purge_dflt_routers will
>>>> clean the default route on all the ifaces which is not desired.
>>>>
>>>> This patches fixes that by cleaning only the routes where the iface has
>>>> forwarding enabled.
>>>>
>>>> Fixes: z ("net: ipv6: Fix processing of RAs in presence of VRF")
>>
>> are you sure that is a Fixes tag for this problem? looking at that
>> change it only handles RA for tables beyond the main table; it does not
>> change the logic of how many or which routes are purged.
> 
> That commit also added RT6_TABLE_HAS_DFLT_ROUTER so I thought that was
> the commit needed to be mentioned. But probably it shouldn't?

nah. That flag was added as an optimization. The patch referenced
earlier changed the code from looking at one table to looking at all of
them. The flag indicates which table have an RA based default route to
avoid unnecessary walks.

You could probably change it to a counter to handle the case of multiple
default route entries.


> Also Am I missing something or this is only called on on the sysctl path?

It is only called when accept_ra sysctl is enabled as I recall. That
setting requires forwarding to be disabled or overridden. See
Documentation/networking/ip-sysctl.rst.

It should be fairly easy to create a selftest using radvd and network
namespaces.