2022-04-07 17:28:35

by Arun Ajith S

[permalink] [raw]
Subject: [PATCH net-next v2] net/ipv6: Introduce accept_unsolicited_na knob to implement router-side changes for RFC9131

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 <[email protected]>
Tested-by: Arun Ajith S <[email protected]>
---
Documentation/networking/ip-sysctl.rst | 23 +++++++++++++++++++++++
include/linux/ipv6.h | 1 +
include/uapi/linux/ipv6.h | 1 +
net/ipv6/addrconf.c | 8 ++++++++
net/ipv6/ndisc.c | 20 +++++++++++++++++++-
5 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index b0024aa7b051..9e17efe343ac 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2467,6 +2467,29 @@ drop_unsolicited_na - BOOLEAN

By default this is turned off.

+accept_unsolicited_na - BOOLEAN
+ Add a new neighbour cache entry in STALE state for routers on receiving an
+ unsolicited neighbour advertisement with target link-layer address option
+ specified. This is as per router-side behavior documented in RFC9131.
+ This has lower precedence than drop_unsolicited_na.
+ drop accept fwding behaviour
+ ---- ------ ------ ----------------------------------------------
+ 1 X X Drop NA packet and don't pass up the stack
+ 0 0 X Pass NA packet up the stack, don't update NC
+ 0 1 0 Pass NA packet up the stack, don't update NC
+ 0 1 1 Pass NA packet up the stack, and add a STALE
+ NC entry
+ This will optimize the return path for the initial off-link communication
+ that is initiated by a directly connected host, by ensuring that
+ the first-hop router which turns on this setting doesn't have to
+ buffer the initial return packets to do neighbour-solicitation.
+ The prerequisite is that the host is configured to send
+ unsolicited neighbour advertisements on interface bringup.
+ This setting should be used in conjunction with the ndisc_notify setting
+ on the host to satisfy this prerequisite.
+
+ By default this is turned off.
+
enhanced_dad - BOOLEAN
Include a nonce option in the IPv6 neighbor solicitation messages used for
duplicate address detection per RFC7527. A received DAD NS will only signal
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 16870f86c74d..918bfea4ef5f 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -61,6 +61,7 @@ struct ipv6_devconf {
__s32 suppress_frag_ndisc;
__s32 accept_ra_mtu;
__s32 drop_unsolicited_na;
+ __s32 accept_unsolicited_na;
struct ipv6_stable_secret {
bool initialized;
struct in6_addr secret;
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index d4178dace0bf..549ddeaf788b 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -194,6 +194,7 @@ enum {
DEVCONF_IOAM6_ID,
DEVCONF_IOAM6_ID_WIDE,
DEVCONF_NDISC_EVICT_NOCARRIER,
+ DEVCONF_ACCEPT_UNSOLICITED_NA,
DEVCONF_MAX
};

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,
+ },
{
/* sentinel */
}
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index fcb288b0ae13..254addad0dd3 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -979,6 +979,7 @@ static void ndisc_recv_na(struct sk_buff *skb)
struct inet6_dev *idev = __in6_dev_get(dev);
struct inet6_ifaddr *ifp;
struct neighbour *neigh;
+ bool create_neigh;

if (skb->len < sizeof(struct nd_msg)) {
ND_PRINTK(2, warn, "NA: packet too short\n");
@@ -999,6 +1000,7 @@ static void ndisc_recv_na(struct sk_buff *skb)
/* For some 802.11 wireless deployments (and possibly other networks),
* there will be a NA proxy and unsolicitd packets are attacks
* and thus should not be accepted.
+ * drop_unsolicited_na takes precedence over accept_unsolicited_na
*/
if (!msg->icmph.icmp6_solicited && idev &&
idev->cnf.drop_unsolicited_na)
@@ -1039,7 +1041,23 @@ static void ndisc_recv_na(struct sk_buff *skb)
in6_ifa_put(ifp);
return;
}
- neigh = neigh_lookup(&nd_tbl, &msg->target, dev);
+ /* RFC 9131 updates original Neighbour Discovery RFC 4861.
+ * An unsolicited NA can now create a neighbour cache entry
+ * on routers if it has Target LL Address option.
+ *
+ * drop accept fwding behaviour
+ * ---- ------ ------ ----------------------------------------------
+ * 1 X X Drop NA packet and don't pass up the stack
+ * 0 0 X Pass NA packet up the stack, don't update NC
+ * 0 1 0 Pass NA packet up the stack, don't update NC
+ * 0 1 1 Pass NA packet up the stack, and add a STALE
+ * NC entry
+ * Note that we don't do a (daddr == all-routers-mcast) check.
+ */
+ create_neigh = !msg->icmph.icmp6_solicited && lladdr &&
+ idev && idev->cnf.forwarding &&
+ idev->cnf.accept_unsolicited_na;
+ neigh = __neigh_lookup(&nd_tbl, &msg->target, dev, create_neigh);

if (neigh) {
u8 old_flags = neigh->flags;
--
2.27.0
---
Changes from v1:
- Change bad documentation and commit description. (source link-layer-address option -> target link-layer-address option)
- CCed all maintainers from .scripts/get_maintainer.pl
- Rebased to latest origin/master


2022-04-12 06:22:32

by Arun Ajith S

[permalink] [raw]
Subject: Re: [PATCH net-next v2] net/ipv6: Introduce accept_unsolicited_na knob to implement router-side changes for RFC9131

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 <[email protected]> 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 <[email protected]>
> > Tested-by: Arun Ajith S <[email protected]>
>
> 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

2022-04-12 06:58:04

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH net-next v2] net/ipv6: Introduce accept_unsolicited_na knob to implement router-side changes for RFC9131

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 <[email protected]>
> Tested-by: Arun Ajith S <[email protected]>

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.

2022-04-12 20:45:47

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH net-next v2] net/ipv6: Introduce accept_unsolicited_na knob to implement router-side changes for RFC9131

On 4/11/22 9:41 AM, Arun Ajith S wrote:
>
> 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)
>

that works too. even simpler.