[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bfaf41d2-b70b-12ef-ca79-bd96807c8fad@intel.com>
Date: Thu, 8 Dec 2022 11:00:01 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Jiri Pirko <jiri@...nulli.us>, <netdev@...r.kernel.org>
CC: <stephen@...workplumber.org>, <dsahern@...il.com>,
<kuba@...nel.org>, <moshe@...dia.com>, <saeedm@...dia.com>
Subject: Re: [patch iproute2/net-next 2/4] devlink: get devlink port for
ifname using RTNL get link command
On 12/5/2022 4:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@...dia.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@...el.com>
> Signed-off-by: Jiri Pirko <jiri@...dia.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;
>
Powered by blists - more mailing lists