| Message ID | 20201008105911.28350-1-florent.fourcot@wifirst.fr (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [net-next] neigh: add netlink filtering based on LLADDR for dump | expand |
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,
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) + 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]); + break; default: if (strict_check) { NL_SET_ERR_MSG(extack, "Unsupported attribute in neighbor dump request");
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(-)