Re: [PATCH net-next v5] rtnetlink: Support fine-grained netdevice bulk deletion
flat view
From: David Ahern <hidden>
Date: 2021-12-07 04:12:36
On 12/5/21 2:36 AM, Lahav Schlesinger wrote:
quoted hunk ↗ jump to hunk
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index fd030e02f16d..5165cc699d97 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c@@ -37,6 +37,7 @@ #include <linux/pci.h> #include <linux/etherdevice.h> #include <linux/bpf.h> +#include <linux/sort.h> #include <linux/uaccess.h>@@ -1880,6 +1881,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = { [IFLA_PROTO_DOWN_REASON] = { .type = NLA_NESTED }, [IFLA_NEW_IFINDEX] = NLA_POLICY_MIN(NLA_S32, 1), [IFLA_PARENT_DEV_NAME] = { .type = NLA_NUL_STRING }, + [IFLA_IFINDEX] = { .type = NLA_S32 },
you need to make sure this new attribute can not be used in setlink requests or getlink.
quoted hunk ↗ jump to hunk
}; static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {@@ -3050,6 +3052,78 @@ static int rtnl_group_dellink(const struct net *net, int group) return 0; } +static int dev_ifindex_cmp(const void *a, const void *b) +{ + struct net_device * const *dev1 = a, * const *dev2 = b;
const struct net_device *dev1 = 1, *dev2 = b;
quoted hunk ↗ jump to hunk
+ + return (*dev1)->ifindex - (*dev2)->ifindex; +} + +static int rtnl_ifindex_dellink(struct net *net, struct nlattr *head, int len, + struct netlink_ext_ack *extack) +{ + int i = 0, num_devices = 0, rem; + struct net_device **dev_list; + const struct nlattr *nla; + LIST_HEAD(list_kill); + int ret; + + nla_for_each_attr(nla, head, len, rem) { + if (nla_type(nla) == IFLA_IFINDEX) + num_devices++; + }
The need to walk the list twice (3 really with the sort) to means the array solution is better.
quoted hunk ↗ jump to hunk
+ + dev_list = kmalloc_array(num_devices, sizeof(*dev_list), GFP_KERNEL); + if (!dev_list) + return -ENOMEM; + + nla_for_each_attr(nla, head, len, rem) { + const struct rtnl_link_ops *ops; + struct net_device *dev; + int ifindex; + + if (nla_type(nla) != IFLA_IFINDEX) + continue;
and this business too. We have arrays in other places (net/ipv4/nexthop.c), so this is not the first.
quoted hunk ↗ jump to hunk
+ + ifindex = nla_get_s32(nla); + ret = -ENODEV; + dev = __dev_get_by_index(net, ifindex); + if (!dev) { + NL_SET_ERR_MSG_ATTR(extack, nla, "Unknown ifindex"); + goto out_free; + } + + ret = -EOPNOTSUPP; + ops = dev->rtnl_link_ops; + if (!ops || !ops->dellink) { + NL_SET_ERR_MSG_ATTR(extack, nla, "Device cannot be deleted"); + goto out_free; + } + + dev_list[i++] = dev; + } + + /* Sort devices, so we could skip duplicates */ + sort(dev_list, num_devices, sizeof(*dev_list), dev_ifindex_cmp, NULL);
how did this sort change the results? 10k compares and re-order has to add some overhead.
quoted hunk ↗ jump to hunk
+ + for (i = 0; i < num_devices; i++) { + struct net_device *dev = dev_list[i]; + + if (i != 0 && dev_list[i - 1]->ifindex == dev->ifindex)
if (i && ...) I liked the array variant better. Jakub?