netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch iproute2/net-next 0/4] devlink: optimize ifname handling
@ 2022-12-05 12:21 Jiri Pirko
  2022-12-05 12:21 ` [patch iproute2/net-next 1/4] devlink: add ifname_map_add/del() helpers Jiri Pirko
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Jiri Pirko @ 2022-12-05 12:21 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern, kuba, moshe, saeedm

From: Jiri Pirko <jiri@nvidia.com>

This patchset enhances devlink tool to benefit from two recently
introduces netlink changes in kernel:

patch #2:
Benefits from RT netlink extension by IFLA_DEVLINK_PORT attribute.
Kernel sends devlink port handle info for every netdev.
Use this attribute to directly obtain devlink port handle for ifname
passed by user as a command line option.

patch #4:
Benefit from the fact that kernel sends PORT_NEW event on devlink
netlink whenever related netdevice ifname changes. Use that to make
the printed out ifname up-to-date when running devlink monitor command.

patches #1 and #3 are just small dependencies of the patched above.

Jiri Pirko (4):
  devlink: add ifname_map_add/del() helpers
  devlink: get devlink port for ifname using RTNL get link command
  devlink: push common code to __pr_out_port_handle_start_tb()
  devlink: update ifname map when message contains
    DEVLINK_ATTR_PORT_NETDEV_NAME

 devlink/devlink.c | 177 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 150 insertions(+), 27 deletions(-)

-- 
2.37.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch iproute2/net-next 1/4] devlink: add ifname_map_add/del() helpers
  2022-12-05 12:21 [patch iproute2/net-next 0/4] devlink: optimize ifname handling Jiri Pirko
@ 2022-12-05 12:21 ` Jiri Pirko
  2022-12-05 12:21 ` [patch iproute2/net-next 2/4] devlink: get devlink port for ifname using RTNL get link command Jiri Pirko
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2022-12-05 12:21 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern, kuba, moshe, saeedm

From: Jiri Pirko <jiri@nvidia.com>

Add couple of helpers to alloc/free of map object alongside with list
addition/removal.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 devlink/devlink.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 4a5eee7a13e8..d224655cd0e9 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -778,16 +778,35 @@ static int function_attr_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
+static int ifname_map_add(struct dl *dl, const char *ifname,
+			  const char *bus_name, const char *dev_name,
+			  uint32_t port_index)
+{
+	struct ifname_map *ifname_map;
+
+	ifname_map = ifname_map_alloc(bus_name, dev_name, port_index, ifname);
+	if (!ifname_map)
+		return -ENOMEM;
+	list_add(&ifname_map->list, &dl->ifname_map_list);
+	return 0;
+}
+
+static void ifname_map_del(struct ifname_map *ifname_map)
+{
+	list_del(&ifname_map->list);
+	ifname_map_free(ifname_map);
+}
+
 static int ifname_map_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
 	struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
 	struct dl *dl = data;
-	struct ifname_map *ifname_map;
 	const char *bus_name;
 	const char *dev_name;
 	uint32_t port_index;
 	const char *port_ifname;
+	int err;
 
 	mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
 	if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
@@ -801,11 +820,9 @@ static int ifname_map_cb(const struct nlmsghdr *nlh, void *data)
 	dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]);
 	port_index = mnl_attr_get_u32(tb[DEVLINK_ATTR_PORT_INDEX]);
 	port_ifname = mnl_attr_get_str(tb[DEVLINK_ATTR_PORT_NETDEV_NAME]);
-	ifname_map = ifname_map_alloc(bus_name, dev_name,
-				      port_index, port_ifname);
-	if (!ifname_map)
+	err = ifname_map_add(dl, port_ifname, bus_name, dev_name, port_index);
+	if (err)
 		return MNL_CB_ERROR;
-	list_add(&ifname_map->list, &dl->ifname_map_list);
 
 	return MNL_CB_OK;
 }
@@ -816,8 +833,7 @@ static void ifname_map_fini(struct dl *dl)
 
 	list_for_each_entry_safe(ifname_map, tmp,
 				 &dl->ifname_map_list, list) {
-		list_del(&ifname_map->list);
-		ifname_map_free(ifname_map);
+		ifname_map_del(ifname_map);
 	}
 }
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [patch iproute2/net-next 2/4] devlink: get devlink port for ifname using RTNL get link command
  2022-12-05 12:21 [patch iproute2/net-next 0/4] devlink: optimize ifname handling Jiri Pirko
  2022-12-05 12:21 ` [patch iproute2/net-next 1/4] devlink: add ifname_map_add/del() helpers Jiri Pirko
@ 2022-12-05 12:21 ` Jiri Pirko
  2022-12-08 19:00   ` Jacob Keller
  2022-12-05 12:21 ` [patch iproute2/net-next 3/4] devlink: push common code to __pr_out_port_handle_start_tb() Jiri Pirko
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Jiri Pirko @ 2022-12-05 12:21 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern, kuba, moshe, saeedm

From: Jiri Pirko <jiri@nvidia.com>

Currently, when user specifies ifname as a handle on command line of
devlink, the related devlink port is looked-up in previously taken dump
of all devlink ports on the system. There are 3 problems with that:
1) The dump iterates over all devlink instances in kernel and takes a
   devlink instance lock for each.
2) Dumping all devlink ports would not scale.
3) Alternative ifnames are not exposed by devlink netlink interface.

Instead, benefit from RTNL get link command extension and get the
devlink port handle info from IFLA_DEVLINK_PORT attribute, if supported.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 devlink/devlink.c | 96 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 5 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index d224655cd0e9..80c18d690c10 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -43,6 +43,8 @@
 #include "json_print.h"
 #include "utils.h"
 #include "namespace.h"
+#include "libnetlink.h"
+#include "../ip/ip_common.h"
 
 #define ESWITCH_MODE_LEGACY "legacy"
 #define ESWITCH_MODE_SWITCHDEV "switchdev"
@@ -797,6 +799,81 @@ static void ifname_map_del(struct ifname_map *ifname_map)
 	ifname_map_free(ifname_map);
 }
 
+static int ifname_map_rtnl_port_parse(struct dl *dl, const char *ifname,
+				      struct rtattr *nest)
+{
+	struct rtattr *tb[DEVLINK_ATTR_MAX + 1];
+	const char *bus_name;
+	const char *dev_name;
+	uint32_t port_index;
+
+	parse_rtattr_nested(tb, DEVLINK_ATTR_MAX, nest);
+	if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
+	    !tb[DEVLINK_ATTR_PORT_INDEX])
+		return -ENOENT;
+
+	bus_name = rta_getattr_str(tb[DEVLINK_ATTR_BUS_NAME]);
+	dev_name = rta_getattr_str(tb[DEVLINK_ATTR_DEV_NAME]);
+	port_index = rta_getattr_u32(tb[DEVLINK_ATTR_PORT_INDEX]);
+	return ifname_map_add(dl, ifname, bus_name, dev_name, port_index);
+}
+
+static int ifname_map_rtnl_init(struct dl *dl, const char *ifname)
+{
+	struct iplink_req req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+		.n.nlmsg_flags = NLM_F_REQUEST,
+		.n.nlmsg_type = RTM_GETLINK,
+		.i.ifi_family = AF_UNSPEC,
+	};
+	struct rtattr *tb[IFLA_MAX + 1];
+	struct rtnl_handle rth;
+	struct ifinfomsg *ifi;
+	struct nlmsghdr *n;
+	int len;
+	int err;
+
+	if (rtnl_open(&rth, 0) < 0) {
+		pr_err("Cannot open rtnetlink\n");
+		return -EINVAL;
+	}
+
+	addattr_l(&req.n, sizeof(req),
+		  !check_ifname(ifname) ? IFLA_IFNAME : IFLA_ALT_IFNAME,
+		  ifname, strlen(ifname) + 1);
+
+	if (rtnl_talk(&rth, &req.n, &n) < 0) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (n->nlmsg_type != RTM_NEWLINK) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	ifi = NLMSG_DATA(n);
+	len = n->nlmsg_len;
+
+	len -= NLMSG_LENGTH(sizeof(*ifi));
+	if (len < 0) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, NLA_F_NESTED);
+	if (!tb[IFLA_DEVLINK_PORT]) {
+		err = -EOPNOTSUPP;
+		goto out;
+	}
+
+	err = ifname_map_rtnl_port_parse(dl, ifname, tb[IFLA_DEVLINK_PORT]);
+
+out:
+	rtnl_close(&rth);
+	return err;
+}
+
 static int ifname_map_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
@@ -842,11 +919,20 @@ static void ifname_map_init(struct dl *dl)
 	INIT_LIST_HEAD(&dl->ifname_map_list);
 }
 
-static int ifname_map_load(struct dl *dl)
+static int ifname_map_load(struct dl *dl, const char *ifname)
 {
 	struct nlmsghdr *nlh;
 	int err;
 
+	if (ifname) {
+		err = ifname_map_rtnl_init(dl, ifname);
+		if (!err)
+			return 0;
+		/* In case kernel does not support devlink port info passed over
+		 * RT netlink, fall-back to ports dump.
+		 */
+	}
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_GET,
 			       NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP);
 
@@ -858,14 +944,14 @@ static int ifname_map_load(struct dl *dl)
 	return 0;
 }
 
-static int ifname_map_check_load(struct dl *dl)
+static int ifname_map_check_load(struct dl *dl, const char *ifname)
 {
 	int err;
 
 	if (dl->map_loaded)
 		return 0;
 
-	err = ifname_map_load(dl);
+	err = ifname_map_load(dl, ifname);
 	if (err) {
 		pr_err("Failed to create index map\n");
 		return err;
@@ -882,7 +968,7 @@ static int ifname_map_lookup(struct dl *dl, const char *ifname,
 	struct ifname_map *ifname_map;
 	int err;
 
-	err = ifname_map_check_load(dl);
+	err = ifname_map_check_load(dl, ifname);
 	if (err)
 		return err;
 
@@ -905,7 +991,7 @@ static int ifname_map_rev_lookup(struct dl *dl, const char *bus_name,
 
 	int err;
 
-	err = ifname_map_check_load(dl);
+	err = ifname_map_check_load(dl, NULL);
 	if (err)
 		return err;
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [patch iproute2/net-next 3/4] devlink: push common code to __pr_out_port_handle_start_tb()
  2022-12-05 12:21 [patch iproute2/net-next 0/4] devlink: optimize ifname handling Jiri Pirko
  2022-12-05 12:21 ` [patch iproute2/net-next 1/4] devlink: add ifname_map_add/del() helpers Jiri Pirko
  2022-12-05 12:21 ` [patch iproute2/net-next 2/4] devlink: get devlink port for ifname using RTNL get link command Jiri Pirko
@ 2022-12-05 12:21 ` Jiri Pirko
  2022-12-05 12:21 ` [patch iproute2/net-next 4/4] devlink: update ifname map when message contains DEVLINK_ATTR_PORT_NETDEV_NAME Jiri Pirko
  2022-12-08 17:50 ` [patch iproute2/net-next 0/4] devlink: optimize ifname handling patchwork-bot+netdevbpf
  4 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2022-12-05 12:21 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern, kuba, moshe, saeedm

From: Jiri Pirko <jiri@nvidia.com>

There is a common code in pr_out_port_handle_start() and
pr_out_port_handle_start_arr(). As the next patch is going to extend it
even more, push the code into common helper.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 devlink/devlink.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 80c18d690c10..2d9ba32b4140 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2764,7 +2764,8 @@ static void __pr_out_port_handle_start(struct dl *dl, const char *bus_name,
 	}
 }
 
-static void pr_out_port_handle_start(struct dl *dl, struct nlattr **tb, bool try_nice)
+static void __pr_out_port_handle_start_tb(struct dl *dl, struct nlattr **tb,
+					  bool try_nice, bool array)
 {
 	const char *bus_name;
 	const char *dev_name;
@@ -2773,19 +2774,17 @@ static void pr_out_port_handle_start(struct dl *dl, struct nlattr **tb, bool try
 	bus_name = mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]);
 	dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]);
 	port_index = mnl_attr_get_u32(tb[DEVLINK_ATTR_PORT_INDEX]);
-	__pr_out_port_handle_start(dl, bus_name, dev_name, port_index, try_nice, false);
+	__pr_out_port_handle_start(dl, bus_name, dev_name, port_index, try_nice, array);
 }
 
-static void pr_out_port_handle_start_arr(struct dl *dl, struct nlattr **tb, bool try_nice)
+static void pr_out_port_handle_start(struct dl *dl, struct nlattr **tb, bool try_nice)
 {
-	const char *bus_name;
-	const char *dev_name;
-	uint32_t port_index;
+	__pr_out_port_handle_start_tb(dl, tb, try_nice, false);
+}
 
-	bus_name = mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]);
-	dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]);
-	port_index = mnl_attr_get_u32(tb[DEVLINK_ATTR_PORT_INDEX]);
-	__pr_out_port_handle_start(dl, bus_name, dev_name, port_index, try_nice, true);
+static void pr_out_port_handle_start_arr(struct dl *dl, struct nlattr **tb, bool try_nice)
+{
+	__pr_out_port_handle_start_tb(dl, tb, try_nice, true);
 }
 
 static void pr_out_port_handle_end(struct dl *dl)
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [patch iproute2/net-next 4/4] devlink: update ifname map when message contains DEVLINK_ATTR_PORT_NETDEV_NAME
  2022-12-05 12:21 [patch iproute2/net-next 0/4] devlink: optimize ifname handling Jiri Pirko
                   ` (2 preceding siblings ...)
  2022-12-05 12:21 ` [patch iproute2/net-next 3/4] devlink: push common code to __pr_out_port_handle_start_tb() Jiri Pirko
@ 2022-12-05 12:21 ` Jiri Pirko
  2022-12-08 17:50 ` [patch iproute2/net-next 0/4] devlink: optimize ifname handling patchwork-bot+netdevbpf
  4 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2022-12-05 12:21 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern, kuba, moshe, saeedm

From: Jiri Pirko <jiri@nvidia.com>

Recent kernels send PORT_NEW message with when ifname changes,
so benefit from that by having ifnames updated.

Whenever there is a message containing DEVLINK_ATTR_PORT_NETDEV_NAME
attribute, use it to update ifname map.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 devlink/devlink.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 2d9ba32b4140..3125a3db98dc 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -242,6 +242,18 @@ static void ifname_map_free(struct ifname_map *ifname_map)
 	free(ifname_map);
 }
 
+static int ifname_map_update(struct ifname_map *ifname_map, const char *ifname)
+{
+	char *new_ifname;
+
+	new_ifname = strdup(ifname);
+	if (!new_ifname)
+		return -ENOMEM;
+	free(ifname_map->ifname);
+	ifname_map->ifname = new_ifname;
+	return 0;
+}
+
 #define DL_OPT_HANDLE		BIT(0)
 #define DL_OPT_HANDLEP		BIT(1)
 #define DL_OPT_PORT_TYPE	BIT(2)
@@ -985,7 +997,7 @@ static int ifname_map_lookup(struct dl *dl, const char *ifname,
 
 static int ifname_map_rev_lookup(struct dl *dl, const char *bus_name,
 				 const char *dev_name, uint32_t port_index,
-				 char **p_ifname)
+				 const char **p_ifname)
 {
 	struct ifname_map *ifname_map;
 
@@ -999,6 +1011,12 @@ static int ifname_map_rev_lookup(struct dl *dl, const char *bus_name,
 		if (strcmp(bus_name, ifname_map->bus_name) == 0 &&
 		    strcmp(dev_name, ifname_map->dev_name) == 0 &&
 		    port_index == ifname_map->port_index) {
+			/* In case non-NULL ifname is passed, update the
+			 * looked-up entry.
+			 */
+			if (*p_ifname)
+				return ifname_map_update(ifname_map, *p_ifname);
+
 			*p_ifname = ifname_map->ifname;
 			return 0;
 		}
@@ -2715,11 +2733,10 @@ static bool should_arr_last_port_handle_end(struct dl *dl,
 
 static void __pr_out_port_handle_start(struct dl *dl, const char *bus_name,
 				       const char *dev_name,
-				       uint32_t port_index, bool try_nice,
-				       bool array)
+				       uint32_t port_index, const char *ifname,
+				       bool try_nice, bool array)
 {
 	static char buf[64];
-	char *ifname = NULL;
 
 	if (dl->no_nice_names || !try_nice ||
 	    ifname_map_rev_lookup(dl, bus_name, dev_name,
@@ -2767,6 +2784,7 @@ static void __pr_out_port_handle_start(struct dl *dl, const char *bus_name,
 static void __pr_out_port_handle_start_tb(struct dl *dl, struct nlattr **tb,
 					  bool try_nice, bool array)
 {
+	const char *ifname = NULL;
 	const char *bus_name;
 	const char *dev_name;
 	uint32_t port_index;
@@ -2774,7 +2792,10 @@ static void __pr_out_port_handle_start_tb(struct dl *dl, struct nlattr **tb,
 	bus_name = mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]);
 	dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]);
 	port_index = mnl_attr_get_u32(tb[DEVLINK_ATTR_PORT_INDEX]);
-	__pr_out_port_handle_start(dl, bus_name, dev_name, port_index, try_nice, array);
+	if (tb[DEVLINK_ATTR_PORT_NETDEV_NAME])
+		ifname = mnl_attr_get_str(tb[DEVLINK_ATTR_PORT_NETDEV_NAME]);
+	__pr_out_port_handle_start(dl, bus_name, dev_name, port_index,
+				   ifname, try_nice, array);
 }
 
 static void pr_out_port_handle_start(struct dl *dl, struct nlattr **tb, bool try_nice)
@@ -6160,7 +6181,8 @@ static void pr_out_occ_show(struct occ_show *occ_show)
 
 	list_for_each_entry(occ_port, &occ_show->port_list, list) {
 		__pr_out_port_handle_start(dl, opts->bus_name, opts->dev_name,
-					   occ_port->port_index, true, false);
+					   occ_port->port_index, NULL,
+					   true, false);
 		pr_out_occ_show_port(dl, occ_port);
 		pr_out_port_handle_end(dl);
 	}
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [patch iproute2/net-next 0/4] devlink: optimize ifname handling
  2022-12-05 12:21 [patch iproute2/net-next 0/4] devlink: optimize ifname handling Jiri Pirko
                   ` (3 preceding siblings ...)
  2022-12-05 12:21 ` [patch iproute2/net-next 4/4] devlink: update ifname map when message contains DEVLINK_ATTR_PORT_NETDEV_NAME Jiri Pirko
@ 2022-12-08 17:50 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-12-08 17:50 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, stephen, dsahern, kuba, moshe, saeedm

Hello:

This series was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Mon,  5 Dec 2022 13:21:54 +0100 you wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> This patchset enhances devlink tool to benefit from two recently
> introduces netlink changes in kernel:
> 
> patch #2:
> Benefits from RT netlink extension by IFLA_DEVLINK_PORT attribute.
> Kernel sends devlink port handle info for every netdev.
> Use this attribute to directly obtain devlink port handle for ifname
> passed by user as a command line option.
> 
> [...]

Here is the summary with links:
  - [iproute2/net-next,1/4] devlink: add ifname_map_add/del() helpers
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=f04f5e1d08d9
  - [iproute2/net-next,2/4] devlink: get devlink port for ifname using RTNL get link command
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=d5ae4c3fdba8
  - [iproute2/net-next,3/4] devlink: push common code to __pr_out_port_handle_start_tb()
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=18ff3ccbc853
  - [iproute2/net-next,4/4] devlink: update ifname map when message contains DEVLINK_ATTR_PORT_NETDEV_NAME
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=42b27dfc6e72

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch iproute2/net-next 2/4] devlink: get devlink port for ifname using RTNL get link command
  2022-12-05 12:21 ` [patch iproute2/net-next 2/4] devlink: get devlink port for ifname using RTNL get link command Jiri Pirko
@ 2022-12-08 19:00   ` Jacob Keller
  0 siblings, 0 replies; 7+ messages in thread
From: Jacob Keller @ 2022-12-08 19:00 UTC (permalink / raw)
  To: Jiri Pirko, netdev; +Cc: stephen, dsahern, kuba, moshe, saeedm



On 12/5/2022 4:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Currently, when user specifies ifname as a handle on command line of
> devlink, the related devlink port is looked-up in previously taken dump
> of all devlink ports on the system. There are 3 problems with that:
> 1) The dump iterates over all devlink instances in kernel and takes a
>     devlink instance lock for each.
> 2) Dumping all devlink ports would not scale.
> 3) Alternative ifnames are not exposed by devlink netlink interface.
> 

Oh neat I wasn't even aware we could pass netdev device names! That's slick.

> Instead, benefit from RTNL get link command extension and get the
> devlink port handle info from IFLA_DEVLINK_PORT attribute, if supported.
> 

Makes sense.

Code looks ok.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>   devlink/devlink.c | 96 ++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 91 insertions(+), 5 deletions(-)
> 
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index d224655cd0e9..80c18d690c10 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -43,6 +43,8 @@
>   #include "json_print.h"
>   #include "utils.h"
>   #include "namespace.h"
> +#include "libnetlink.h"
> +#include "../ip/ip_common.h"
>   
>   #define ESWITCH_MODE_LEGACY "legacy"
>   #define ESWITCH_MODE_SWITCHDEV "switchdev"
> @@ -797,6 +799,81 @@ static void ifname_map_del(struct ifname_map *ifname_map)
>   	ifname_map_free(ifname_map);
>   }
>   
> +static int ifname_map_rtnl_port_parse(struct dl *dl, const char *ifname,
> +				      struct rtattr *nest)
> +{
> +	struct rtattr *tb[DEVLINK_ATTR_MAX + 1];
> +	const char *bus_name;
> +	const char *dev_name;
> +	uint32_t port_index;
> +
> +	parse_rtattr_nested(tb, DEVLINK_ATTR_MAX, nest);
> +	if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
> +	    !tb[DEVLINK_ATTR_PORT_INDEX])
> +		return -ENOENT;
> +
> +	bus_name = rta_getattr_str(tb[DEVLINK_ATTR_BUS_NAME]);
> +	dev_name = rta_getattr_str(tb[DEVLINK_ATTR_DEV_NAME]);
> +	port_index = rta_getattr_u32(tb[DEVLINK_ATTR_PORT_INDEX]);
> +	return ifname_map_add(dl, ifname, bus_name, dev_name, port_index);
> +}
> +
> +static int ifname_map_rtnl_init(struct dl *dl, const char *ifname)
> +{
> +	struct iplink_req req = {
> +		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
> +		.n.nlmsg_flags = NLM_F_REQUEST,
> +		.n.nlmsg_type = RTM_GETLINK,
> +		.i.ifi_family = AF_UNSPEC,
> +	};
> +	struct rtattr *tb[IFLA_MAX + 1];
> +	struct rtnl_handle rth;
> +	struct ifinfomsg *ifi;
> +	struct nlmsghdr *n;
> +	int len;
> +	int err;
> +
> +	if (rtnl_open(&rth, 0) < 0) {
> +		pr_err("Cannot open rtnetlink\n");
> +		return -EINVAL;
> +	}
> +
> +	addattr_l(&req.n, sizeof(req),
> +		  !check_ifname(ifname) ? IFLA_IFNAME : IFLA_ALT_IFNAME,
> +		  ifname, strlen(ifname) + 1);
> +
> +	if (rtnl_talk(&rth, &req.n, &n) < 0) {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (n->nlmsg_type != RTM_NEWLINK) {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	ifi = NLMSG_DATA(n);
> +	len = n->nlmsg_len;
> +
> +	len -= NLMSG_LENGTH(sizeof(*ifi));
> +	if (len < 0) {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, NLA_F_NESTED);
> +	if (!tb[IFLA_DEVLINK_PORT]) {
> +		err = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
> +	err = ifname_map_rtnl_port_parse(dl, ifname, tb[IFLA_DEVLINK_PORT]);
> +
> +out:
> +	rtnl_close(&rth);
> +	return err;
> +}
> +
>   static int ifname_map_cb(const struct nlmsghdr *nlh, void *data)
>   {
>   	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
> @@ -842,11 +919,20 @@ static void ifname_map_init(struct dl *dl)
>   	INIT_LIST_HEAD(&dl->ifname_map_list);
>   }
>   
> -static int ifname_map_load(struct dl *dl)
> +static int ifname_map_load(struct dl *dl, const char *ifname)
>   {
>   	struct nlmsghdr *nlh;
>   	int err;
>   
> +	if (ifname) {
> +		err = ifname_map_rtnl_init(dl, ifname);
> +		if (!err)
> +			return 0;
> +		/* In case kernel does not support devlink port info passed over
> +		 * RT netlink, fall-back to ports dump.
> +		 */
> +	}
> +
>   	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_GET,
>   			       NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP);
>   
> @@ -858,14 +944,14 @@ static int ifname_map_load(struct dl *dl)
>   	return 0;
>   }
>   
> -static int ifname_map_check_load(struct dl *dl)
> +static int ifname_map_check_load(struct dl *dl, const char *ifname)
>   {
>   	int err;
>   
>   	if (dl->map_loaded)
>   		return 0;
>   
> -	err = ifname_map_load(dl);
> +	err = ifname_map_load(dl, ifname);
>   	if (err) {
>   		pr_err("Failed to create index map\n");
>   		return err;
> @@ -882,7 +968,7 @@ static int ifname_map_lookup(struct dl *dl, const char *ifname,
>   	struct ifname_map *ifname_map;
>   	int err;
>   
> -	err = ifname_map_check_load(dl);
> +	err = ifname_map_check_load(dl, ifname);
>   	if (err)
>   		return err;
>   
> @@ -905,7 +991,7 @@ static int ifname_map_rev_lookup(struct dl *dl, const char *bus_name,
>   
>   	int err;
>   
> -	err = ifname_map_check_load(dl);
> +	err = ifname_map_check_load(dl, NULL);
>   	if (err)
>   		return err;
>   

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-12-08 19:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-05 12:21 [patch iproute2/net-next 0/4] devlink: optimize ifname handling Jiri Pirko
2022-12-05 12:21 ` [patch iproute2/net-next 1/4] devlink: add ifname_map_add/del() helpers Jiri Pirko
2022-12-05 12:21 ` [patch iproute2/net-next 2/4] devlink: get devlink port for ifname using RTNL get link command Jiri Pirko
2022-12-08 19:00   ` Jacob Keller
2022-12-05 12:21 ` [patch iproute2/net-next 3/4] devlink: push common code to __pr_out_port_handle_start_tb() Jiri Pirko
2022-12-05 12:21 ` [patch iproute2/net-next 4/4] devlink: update ifname map when message contains DEVLINK_ATTR_PORT_NETDEV_NAME Jiri Pirko
2022-12-08 17:50 ` [patch iproute2/net-next 0/4] devlink: optimize ifname handling patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).