Thread (18 messages) 18 messages, 5 authors, 2022-01-04

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?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help