diff mbox series

[iproute2/net-next,2/4] devlink: get devlink port for ifname using RTNL get link command

Message ID 20221205122158.437522-3-jiri@resnulli.us (mailing list archive)
State Accepted
Commit d5ae4c3fdba8959a9adc74054701671268b442f2
Delegated to: David Ahern
Headers show
Series devlink: optimize ifname handling | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/apply fail Patch does not apply to net-next

Commit Message

Jiri Pirko Dec. 5, 2022, 12:21 p.m. UTC 
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(-)

Comments

Jacob Keller Dec. 8, 2022, 7 p.m. UTC | #1 
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;
>
diff mbox series

Patch

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;