Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220407074428.1623-1-aajith@arista.com> <d7a85a29-0d7f-b5e2-c908-4aa9f89bb476@kernel.org>
In-Reply-To: <d7a85a29-0d7f-b5e2-c908-4aa9f89bb476@kernel.org>
From:   Arun Ajith S <aajith@arista.com>
Date:   Mon, 11 Apr 2022 21:11:18 +0530
Message-ID: <CAOvjArQcH1KRV3B1V9urYEV+6i3ZL6NbmkYjbu1icFBJZ3JVOQ@mail.gmail.com>
Subject: Re: [PATCH net-next v2] net/ipv6: Introduce accept_unsolicited_na
 knob to implement router-side changes for RFC9131
To:     David Ahern <dsahern@kernel.org>
Cc:     netdev@vger.kernel.org, davem@davemloft.net,
        linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
        yoshfuji@linux-ipv6.org, kuba@kernel.org, pabeni@redhat.com,
        corbet@lwn.net, prestwoj@gmail.com, gilligan@arista.com,
        noureddine@arista.com, gk@arista.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi David,

Thank you very much for the review.
I will make the changes you suggested.
Please see inline the question about mausezahn.

On Sat, Apr 9, 2022 at 6:48 AM David Ahern <dsahern@kernel.org> wrote:
>
> On 4/7/22 1:44 AM, Arun Ajith S wrote:
> > Add a new neighbour cache entry in STALE state for routers on receiving
> > an unsolicited (gratuitous) neighbour advertisement with
> > target link-layer-address option specified.
> > This is similar to the arp_accept configuration for IPv4.
> > A new sysctl endpoint is created to turn on this behaviour:
> > /proc/sys/net/ipv6/conf/interface/accept_unsolicited_na.
> >
> > Signed-off-by: Arun Ajith S <aajith@arista.com>
> > Tested-by: Arun Ajith S <aajith@arista.com>
>
> you don't need the Tested-by line since you wrote the patch; you are
> expected to test it.
>
>
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index 1afc4c024981..1b4d278d0454 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -5587,6 +5587,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
> >       array[DEVCONF_IOAM6_ID] = cnf->ioam6_id;
> >       array[DEVCONF_IOAM6_ID_WIDE] = cnf->ioam6_id_wide;
> >       array[DEVCONF_NDISC_EVICT_NOCARRIER] = cnf->ndisc_evict_nocarrier;
> > +     array[DEVCONF_ACCEPT_UNSOLICITED_NA] = cnf->accept_unsolicited_na;
> >  }
> >
> >  static inline size_t inet6_ifla6_size(void)
> > @@ -7037,6 +7038,13 @@ static const struct ctl_table addrconf_sysctl[] = {
> >               .extra1         = (void *)SYSCTL_ZERO,
> >               .extra2         = (void *)SYSCTL_ONE,
> >       },
> > +     {
> > +             .procname       = "accept_unsolicited_na",
> > +             .data           = &ipv6_devconf.accept_unsolicited_na,
> > +             .maxlen         = sizeof(int),
> > +             .mode           = 0644,
> > +             .proc_handler   = proc_dointvec,
> > +     },
>
> I realize drop_unsolicited_na does not have limits, but this is a new
> sysctl - add the upper and lower bounds via extra1 and extra2 arguments.
>
>
>
> also, please add test cases under tools/testing/selftests/net. You can
> use fib_tests.sh as a template. mausezahn is already used in a number of
> tests; it should be able to create the NA packets. Be sure to cover
> combinations of drop and accept settings.

mausezahn doesn't have good support for ICMPv6.
I tried using --type icmp6 -t icmp6 "type=136, payload=<HEX-PAYLOAD>"
to manually craft a NA packet with  the target address and the target
ll addr option.
But it still doesn't allow me to set the flags to mark it as an
unsolicited advertisement.

How about this alternative for a test:
1. Setup a veth tunnel across two namespaces, one end being the host
and the other the router.
2. On the host side, I can configure
net.ipv6.conf.<interface>.ndisc_notify to send out unsolicited NAs.
3. On the router side, I can try out various combinations of
(accept_unsolicited_na, drop_unsolicted_na and forwarding)

Thanks,
Arun