netdev - Re: [PATCH net-next 0/7] net: add struct nexthop to fib{6}

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]

Date:   Thu, 30 May 2019 12:52:43 -0600
From:   David Ahern <dsahern@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        David Miller <davem@...emloft.net>
Cc:     David Ahern <dsahern@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        Ido Schimmel <idosch@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Martin KaFai Lau <kafai@...com>, Wei Wang <weiwan@...gle.com>
Subject: Re: [PATCH net-next 0/7] net: add struct nexthop to fib{6}_info

On 5/30/19 12:27 PM, Alexei Starovoitov wrote:
> On Thu, May 30, 2019 at 11:01 AM David Miller <davem@...emloft.net> wrote:
>>
>> From: Alexei Starovoitov <alexei.starovoitov@...il.com>
>> Date: Thu, 30 May 2019 08:18:10 -0700
>>
>>> On Thu, May 30, 2019 at 8:16 AM David Ahern <dsahern@...il.com> wrote:
>>>>
>>>> On 5/30/19 9:06 AM, Alexei Starovoitov wrote:
>>>>> Huge number of core changes and zero tests.
>>>>
>>>> As mentioned in a past response, there are a number of tests under
>>>> selftests that exercise the code paths affected by this change.
>>>
>>> I see zero new tests added.
>>
>> If the existing tests give sufficient coverage, your objections are not
>> reasonable Alexei.
> 
> I completely disagree. Existing tests are not sufficient.
> It is a new feature for the kernel with corresponding iproute2 new features,
> yet there are zero tests.
> 

Your objection was based on changes to core code ("Huge number of core
changes and zero tests"), not new code paths.

The nexthop code paths are not live yet. More changes are needed before
that can happen. I have been sending the patches in small, reviewable
increments to be kind to reviewers than a single 27 patch set with
everything (the remaining set which is over the limit BTW).

Once iproute2 has the nexthop command (patches sent to the list) AND the
RTA_NHID patches are in, I have this as the final set:

8c0b06b9813e selftests: Add test cases for nexthop objects
ea5c19e4dc7c selftests: Add nexthop objects to router_multipath.sh
3be7b15d1e56 selftests: pmtu: Move running of test into a new function
a896b2206ea5 selftests: pmtu: Move route installs to a new function
cfa48193d0b8 selftests: pmtu: Add support for routing via nexthop objects
3d09a79208b9 selftests: icmp_redirect: Add support for routing via
nexthop objects

That includes:
1. a test script doing functional validation of the nexthop code
(net/ipv4/nexthop.c), along with stack integration (e.g., v6 nexthop
with v4 routes) and traffic (ping).

2. updates to existing exception tests (pmtu and redirect) with the
nexthop obects used for routing

3. updates to the existing router_multipath script with the nexthop
obects used for routing and validating balanced selection in paths.

As always, my patches can be viewed by anyone:
    https://github.com/dsahern/linux

Latest branch is:
   https://github.com/dsahern/linux/tree/5.2-next-nexthops-v14

In addition to the functional and runtime tests listed above:
1. I have all kinds of MPLS and network functional tests that I run. I
have already committed to sending those for inclusion, it is a matter of
time.

2. The FRR team has had a kernel with these patches for 5+ months adding
support for nexthops to FRR. They have their own test suites used for
their development.

3. The patches are included in our 4.19 kernel and subjected to our
suite of smoke and validation tests.