On Wed, Apr 13, 2022 at 01:51:57PM +0300, Nikolay Aleksandrov wrote:
> When NLM_F_BULK is specified in a fdb del message we need to handle it
> differently. First since this is a new call we can strictly validate the
> passed attributes, at first only ifindex and vlan are allowed as these
> will be the initially supported filter attributes, any other attribute
> is rejected. The mac address is no longer mandatory, but we use it
> to error out in older kernels because it cannot be specified with bulk
> request (the attribute is not allowed) and then we have to dispatch
> the call to ndo_fdb_del_bulk if the device supports it. The del bulk
> callback can do further validation of the attributes if necessary.
>
> Signed-off-by: Nikolay Aleksandrov <[email protected]>
> ---
> v4: mark PF_BRIDGE/RTM_DELNEIGH with RTNL_FLAG_BULK_DEL_SUPPORTED
>
> net/core/rtnetlink.c | 67 +++++++++++++++++++++++++++++++-------------
> 1 file changed, 48 insertions(+), 19 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 63c7df52a667..520d50fcaaea 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4169,22 +4169,34 @@ int ndo_dflt_fdb_del(struct ndmsg *ndm,
> }
> EXPORT_SYMBOL(ndo_dflt_fdb_del);
>
> +static const struct nla_policy fdb_del_bulk_policy[NDA_MAX + 1] = {
> + [NDA_VLAN] = { .type = NLA_U16 },
In earlier versions br_vlan_valid_id() was used to validate the VLAN,
but I don't see it anymore. Maybe use
NLA_POLICY_RANGE(1, VLAN_N_VID - 2)
?
I realize that invalid values won't do anything, but I think it's better
to only allow valid ranges.
> + [NDA_IFINDEX] = NLA_POLICY_MIN(NLA_S32, 1),
> +};
> +
> static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
> struct netlink_ext_ack *extack)
> {
> + bool del_bulk = !!(nlh->nlmsg_flags & NLM_F_BULK);
> struct net *net = sock_net(skb->sk);
> + const struct net_device_ops *ops;
> struct ndmsg *ndm;
> struct nlattr *tb[NDA_MAX+1];
> struct net_device *dev;
> - __u8 *addr;
> + __u8 *addr = NULL;
> int err;
> u16 vid;
>
> if (!netlink_capable(skb, CAP_NET_ADMIN))
> return -EPERM;
>
> - err = nlmsg_parse_deprecated(nlh, sizeof(*ndm), tb, NDA_MAX, NULL,
> - extack);
> + if (!del_bulk) {
> + err = nlmsg_parse_deprecated(nlh, sizeof(*ndm), tb, NDA_MAX,
> + NULL, extack);
> + } else {
> + err = nlmsg_parse(nlh, sizeof(*ndm), tb, NDA_MAX,
> + fdb_del_bulk_policy, extack);
> + }
> if (err < 0)
> return err;
>
> @@ -4200,9 +4212,12 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct
> nlmsghdr *nlh,
> return -ENODEV;
> }
>
> - if (!tb[NDA_LLADDR] || nla_len(tb[NDA_LLADDR]) != ETH_ALEN) {
> - NL_SET_ERR_MSG(extack, "invalid address");
> - return -EINVAL;
> + if (!del_bulk) {
> + if (!tb[NDA_LLADDR] || nla_len(tb[NDA_LLADDR]) != ETH_ALEN) {
> + NL_SET_ERR_MSG(extack, "invalid address");
> + return -EINVAL;
> + }
> + addr = nla_data(tb[NDA_LLADDR]);
> }
>
> if (dev->type != ARPHRD_ETHER) {
> @@ -4210,8 +4225,6 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct
> nlmsghdr *nlh,
> return -EINVAL;
> }
>
> - addr = nla_data(tb[NDA_LLADDR]);
> -
> err = fdb_vid_parse(tb[NDA_VLAN], &vid, extack);
> if (err)
> return err;
> @@ -4222,10 +4235,16 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct
> nlmsghdr *nlh,
> if ((!ndm->ndm_flags || ndm->ndm_flags & NTF_MASTER) &&
> netif_is_bridge_port(dev)) {
> struct net_device *br_dev = netdev_master_upper_dev_get(dev);
> - const struct net_device_ops *ops = br_dev->netdev_ops;
>
> - if (ops->ndo_fdb_del)
> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> + ops = br_dev->netdev_ops;
> + if (!del_bulk) {
> + if (ops->ndo_fdb_del)
> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> + } else {
> + if (ops->ndo_fdb_del_bulk)
> + err = ops->ndo_fdb_del_bulk(ndm, tb, dev, vid,
> + extack);
> + }
>
> if (err)
> goto out;
> @@ -4235,15 +4254,24 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct
> nlmsghdr *nlh,
>
> /* Embedded bridge, macvlan, and any other device support */
> if (ndm->ndm_flags & NTF_SELF) {
> - if (dev->netdev_ops->ndo_fdb_del)
> - err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
> - vid);
> - else
> - err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
> + ops = dev->netdev_ops;
> + if (!del_bulk) {
> + if (ops->ndo_fdb_del)
> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> + else
> + err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
> + } else {
> + /* in case err was cleared by NTF_MASTER call */
> + err = -EOPNOTSUPP;
> + if (ops->ndo_fdb_del_bulk)
> + err = ops->ndo_fdb_del_bulk(ndm, tb, dev, vid,
> + extack);
> + }
>
> if (!err) {
> - rtnl_fdb_notify(dev, addr, vid, RTM_DELNEIGH,
> - ndm->ndm_state);
> + if (!del_bulk)
> + rtnl_fdb_notify(dev, addr, vid, RTM_DELNEIGH,
> + ndm->ndm_state);
> ndm->ndm_flags &= ~NTF_SELF;
> }
> }
> @@ -6145,7 +6173,8 @@ void __init rtnetlink_init(void)
> rtnl_register(PF_UNSPEC, RTM_DELLINKPROP, rtnl_dellinkprop, NULL, 0);
>
> rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, 0);
> - rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, 0);
> + rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL,
> + RTNL_FLAG_BULK_DEL_SUPPORTED);
> rtnl_register(PF_BRIDGE, RTM_GETNEIGH, rtnl_fdb_get, rtnl_fdb_dump, 0);
>
> rtnl_register(PF_BRIDGE, RTM_GETLINK, NULL, rtnl_bridge_getlink, 0);
> --
> 2.35.1
>
