netdev - Re: [PATCH] neigh: Support filtering neighbours for L3 slave

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <351cae13-8662-2f8f-dd8b-4127ead0ca2a@gmail.com>
Date:   Sun, 1 Aug 2021 11:50:16 -0600
From:   David Ahern <dsahern@...il.com>
To:     Lahav Schlesinger <lschlesinger@...venets.com>,
        netdev@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org, dsahern@...nel.org
Subject: Re: [PATCH] neigh: Support filtering neighbours for L3 slave

On 8/1/21 3:01 AM, Lahav Schlesinger wrote:
> Currently there's support for filtering neighbours for interfaces which
> are in a specific VRF (passing the VRF interface in 'NDA_MASTER'), but
> there's not support for filtering interfaces which are not in an L3
> domain (the "default VRF").
> 
> This means userspace is unable to show/flush neighbours in the default VRF
> (in contrast to a "real" VRF - Using "ip neigh show vrf <vrf_dev>").
> 
> Therefore for userspace to be able to do so, it must manually iterate
> over all the interfaces, check each one if it's in the default VRF, and
> if so send the matching flush/show message.
> 
> This patch adds the ability to do so easily, by passing a dummy value as
> the 'NDA_MASTER' ('NDV_NOT_L3_SLAVE').
> Note that 'NDV_NOT_L3_SLAVE' is a negative number, meaning it is not a valid
> ifindex, so it doesn't break existing programs.
> 
> I have a patch for iproute2 ready for adding this support in userspace.
> 
> Signed-off-by: Lahav Schlesinger <lschlesinger@...venets.com>
> Cc: David S. Miller <davem@...emloft.net>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: David Ahern <dsahern@...nel.org>
> ---
>  include/uapi/linux/neighbour.h | 2 ++
>  net/core/neighbour.c           | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
> index dc8b72201f6c..d4f4c2189c63 100644
> --- a/include/uapi/linux/neighbour.h
> +++ b/include/uapi/linux/neighbour.h
> @@ -196,4 +196,6 @@ enum {
>  };
>  #define NFEA_MAX (__NFEA_MAX - 1)
>  
> +#define NDV_NOT_L3_SLAVE	(-10)
> +
>  #endif
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 53e85c70c6e5..b280103b6806 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2529,6 +2529,9 @@ static bool neigh_master_filtered(struct net_device *dev, int master_idx)
>  {
>  	struct net_device *master;
>  
> +	if (master_idx == NDV_NOT_L3_SLAVE)
> +		return netif_is_l3_slave(dev);
> +
>  	if (!master_idx)
>  		return false;
>  
> 

you can not special case VRFs like this, and such a feature should apply
to links and addresses as well.

One idea is to pass "*_MASTER" as -1 (use "none" keyword for iproute2)
and then update kernel side to only return entries if the corresponding
device is not enslaved to another device. Unfortunately since I did not
check that _MASTER was non-zero in the current code, we can not use 0 as
a valid flag for "not enslaved". Be sure to document why -1 is used.