[net-next] neigh: add netlink filtering based on LLADDR for dump

neighbours table dump supports today two filtering:
 * based on interface index
 * based on master index

This patch adds a new filtering, based on layer two address. That will
help to replace something like it:

 ip neigh show | grep aa:11:22:bb:ee:ff

by a better command:

 ip neigh show lladdr aa:11:22:bb:ee:ff

Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
---
 net/core/neighbour.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

On 10/8/20 12:59 PM, Florent Fourcot wrote:
> neighbours table dump supports today two filtering:
>  * based on interface index
>  * based on master index
> 
> This patch adds a new filtering, based on layer two address. That will
> help to replace something like it:
> 
>  ip neigh show | grep aa:11:22:bb:ee:ff
> 
> by a better command:
> 
>  ip neigh show lladdr aa:11:22:bb:ee:ff
> 
> Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
> ---
>  net/core/neighbour.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 8e39e28b0a8d..4b32bf49a005 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2542,9 +2542,25 @@ static bool neigh_ifindex_filtered(struct net_device *dev, int filter_idx)
>  	return false;
>  }
>  
> +static bool neigh_lladdr_filtered(struct neighbour *neigh, const u8 *lladdr)
> +{
> +	if (!lladdr)
> +		return false;
> +
> +	/* Ignore all empty values when lladdr filtering is set */
> +	if (!neigh->dev->addr_len)
> +		return true;
> +
> +	if (memcmp(lladdr, neigh->ha, neigh->dev->addr_len) != 0)

Where do you check that lladdr contains exactly neigh->dev->addr_len bytes ?

> +		return true;
> +
> +	return false;
> +}
> +
>  struct neigh_dump_filter {
>  	int master_idx;
>  	int dev_idx;
> +	void *lladdr;
>  };
>  
>  static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
> @@ -2558,7 +2574,7 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
>  	struct neigh_hash_table *nht;
>  	unsigned int flags = NLM_F_MULTI;
>  
> -	if (filter->dev_idx || filter->master_idx)
> +	if (filter->dev_idx || filter->master_idx || filter->lladdr)
>  		flags |= NLM_F_DUMP_FILTERED;
>  
>  	rcu_read_lock_bh();
> @@ -2573,7 +2589,8 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
>  			if (idx < s_idx || !net_eq(dev_net(n->dev), net))
>  				goto next;
>  			if (neigh_ifindex_filtered(n->dev, filter->dev_idx) ||
> -			    neigh_master_filtered(n->dev, filter->master_idx))
> +			    neigh_master_filtered(n->dev, filter->master_idx) ||
> +			    neigh_lladdr_filtered(n, filter->lladdr))
>  				goto next;
>  			if (neigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
>  					    cb->nlh->nlmsg_seq,
> @@ -2689,6 +2706,9 @@ static int neigh_valid_dump_req(const struct nlmsghdr *nlh,
>  		case NDA_MASTER:
>  			filter->master_idx = nla_get_u32(tb[i]);
>  			break;
> +		case NDA_LLADDR:
> +			filter->lladdr = nla_data(tb[i]);

This comes from user space, and could contains an arbitrary amount of bytes, like 0 byte.

You probably have to store the full attribute, so that you can use nla_len() and nla_data()

> +			break;
>  		default:
>  			if (strict_check) {
>  				NL_SET_ERR_MSG(extack, "Unsupported attribute in neighbor dump request");
>

Hello Éric,


>> +	if (memcmp(lladdr, neigh->ha, neigh->dev->addr_len) != 0)
> 
> Where do you check that lladdr contains exactly neigh->dev->addr_len bytes ?

True, I do not check. I had some doubt about the best implementation, 
since we could do:
  * exact matching
  * prefix matching (with a memcmp on length of lladdr)

Do you may have an opinion on the best choice?


>> +		case NDA_LLADDR:
>> +			filter->lladdr = nla_data(tb[i]);
> 
> This comes from user space, and could contains an arbitrary amount of bytes, like 0 byte.
> 
> You probably have to store the full attribute, so that you can use nla_len() and nla_data()
> 

I will send a v2.

By the way, it looks like neigh_add() function never check if NDA_LLADDR 
length is greater than dev->addr_len (it only rejects smaller values). 
Should we add a check on it? I do not see any impact today, except than 
user does not receive an error on invalid data, it will configure an 
entry anyway.

Thanks,