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;
Message-ID: <d6240f70-858b-07ac-cab0-8483e16eff57@6wind.com>
Date:   Mon, 18 Jul 2022 10:35:55 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Reply-To: nicolas.dichtel@6wind.com
Subject: Re: [PATCH v3 net-next 2/3] net: ipv6: new accept_untracked_na option
 to accept na only if in-network
Content-Language: en-US
To:     Jaehee Park <jhpark1013@gmail.com>, netdev@vger.kernel.org
Cc:     davem@davemloft.net, yoshfuji@linux-ipv6.org, dsahern@kernel.org,
        dsahern@gmail.com, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, shuah@kernel.org, linux-kernel@vger.kernel.org,
        aajith@arista.com, roopa@nvidia.com, roopa.prabhu@gmail.com,
        aroulin@nvidia.com, sbrivio@redhat.com
References: <cover.1657755188.git.jhpark1013@gmail.com>
 <56d57be31141c12e9034cfa7570f2012528ca884.1657755189.git.jhpark1013@gmail.com>
From:   Nicolas Dichtel <nicolas.dichtel@6wind.com>
Organization: 6WIND
In-Reply-To: <56d57be31141c12e9034cfa7570f2012528ca884.1657755189.git.jhpark1013@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


Le 14/07/2022 à 01:40, Jaehee Park a écrit :
> This patch adds a third knob, '2', which extends the
> accept_untracked_na option to learn a neighbor only if the src ip is
> in the same subnet as an address configured on the interface that
> received the neighbor advertisement. This is similar to the arp_accept
> configuration for ipv4.
> 
> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> Suggested-by: Roopa Prabhu <roopa@nvidia.com>
> ---
>  Documentation/networking/ip-sysctl.rst | 51 +++++++++++++++-----------
>  net/ipv6/addrconf.c                    |  2 +-
>  net/ipv6/ndisc.c                       | 29 ++++++++++++---
>  3 files changed, 55 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index 5c017fc1e24d..722ec4f491db 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -2483,27 +2483,36 @@ drop_unsolicited_na - BOOLEAN
>  
>  	By default this is turned off.
>  
> -accept_untracked_na - BOOLEAN
> -	Add a new neighbour cache entry in STALE state for routers on receiving a
> -	neighbour advertisement (either solicited or unsolicited) with target
> -	link-layer address option specified if no neighbour entry is already
> -	present for the advertised IPv6 address. Without this knob, NAs received
> -	for untracked addresses (absent in neighbour cache) are silently ignored.
> -
> -	This is as per router-side behaviour documented in RFC9131.
> -
> -	This has lower precedence than drop_unsolicited_na.
> -
> -	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.
> +accept_untracked_na - INTEGER
> +	Define behavior for accepting neighbor advertisements from devices that
> +	are absent in the neighbor cache:
> +
> +	- 0 - (default) Do not accept unsolicited and untracked neighbor
> +	  advertisements.
> +
> +	- 1 - Add a new neighbor cache entry in STALE state for routers on
> +	  receiving a neighbor advertisement (either solicited or unsolicited)
> +	  with target link-layer address option specified if no neighbor entry
> +	  is already present for the advertised IPv6 address. Without this knob,
> +	  NAs received for untracked addresses (absent in neighbor cache) are
> +	  silently ignored.
> +
> +	  This is as per router-side behavior documented in RFC9131.
> +
> +	  This has lower precedence than drop_unsolicited_na.
> +
> +	  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 neighbor-solicitation.
> +	  The prerequisite is that the host is configured to send unsolicited
> +	  neighbor advertisements on interface bringup. This setting should be
> +	  used in conjunction with the ndisc_notify setting on the host to
> +	  satisfy this prerequisite.
> +
> +	- 2 - Extend option (1) to add a new neighbor cache entry only if the
> +	  source IP address is in the same subnet as an address configured on
> +	  the interface that received the neighbor advertisement.
>  
>  enhanced_dad - BOOLEAN
>  	Include a nonce option in the IPv6 neighbor solicitation messages used for
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 88becb037eb6..6ed807b6c647 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -7042,7 +7042,7 @@ static const struct ctl_table addrconf_sysctl[] = {
>  		.data		= &ipv6_devconf.accept_untracked_na,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec_minmax,
> +		.proc_handler	= proc_dointvec,
>  		.extra1		= (void *)SYSCTL_ZERO,
>  		.extra2		= (void *)SYSCTL_ONE,
Maybe keeping proc_dointvec_minmax with SYSCTL_TWO for extra2 is better to avoid
accepting all values. It enables to add another value later.


Regards,
Nicolas