From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f195.google.com (mail-wr0-f195.google.com [209.85.128.195]) by dpdk.org (Postfix) with ESMTP id 0F17E23B for ; Wed, 14 Mar 2018 18:10:41 +0100 (CET) Received: by mail-wr0-f195.google.com with SMTP id v65so5475966wrc.11 for ; Wed, 14 Mar 2018 10:10:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=tWotLwTGTOrcELZwliKfkJTO3io24Rd+yiSQ5BYHMyE=; b=lgu188Oi0jQFugskcUomI5W7CKXluIwuQVP5bMC4xm45gQY1rPTWLl11CMecxLEDGs GsO7N9LRnuYeXRo9oQJpH4poIAqMpCIvpAh7xUXTfzeXgB9xl+/PHhFc06Fv7BIz0M9c Jz3Xa14D5zbXnghW+LYqI5a6vI8WKqx6LzDHoggFJ3Y1gJ7tgPnsd1OftP+ZifONUYsw 1liRr0Pg1t0RLuyWtnVxL/tMIkUZTjJpSEAXVIB7ALSqjuQdZbhLshTtNlVIyw9jkFTm XYDmpAvwZugKlQ1MtY8Dx4tUVvQHIraxBC/5Iq7FFFJNP6t0R7Xj4hBOt4zOnzG99781 LfBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=tWotLwTGTOrcELZwliKfkJTO3io24Rd+yiSQ5BYHMyE=; b=sxX1QBdlV/ie7F/jcWQ0dd/nZrv64xVzqkjZfqtUolT7xPImKhu8b57PvciTtwrC5E rv5o/hA9BEPAN3iWG0JBn3vy34vti8hPY7xc44Gih4h/0GDvSRVBf3Txv3oVWXfs2HIt QbgUsdt0YKj/TRamfHueLmUGVa9v8COhYR4CsFBM8TbzyJ+zISt4NXZNBtD/dWxHuKL4 vOP+gYdezJCsg7825CpEUNnPItsZCyDywmf8E2OsCQBbGuYGS6zoqfYFw33Omgz3D8Wb vYvIz1qw9f8MLvlnzQ0flRXP6PasY41+OXoiQvQJwm4RQ8NL4hWFdgpME8W0woiualH4 7C8Q== X-Gm-Message-State: AElRT7EpNcP9dSZphj6CeKV3G22u+JYyCBMwwyFyV+jh9ZzDZvBbzX+2 k6FeIHyPnhNLhPx0oOK2eu14fg== X-Google-Smtp-Source: AG47ELsuTxLsw7jnzXA3lW3v3eYUYbKwsDOUpcGw8o7PduQgtZB4TlgKlcRA3wBxmKigqFPLodQbzQ== X-Received: by 10.223.199.137 with SMTP id l9mr4761043wrg.6.1521047440670; Wed, 14 Mar 2018 10:10:40 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id p29sm2022564wmf.3.2018.03.14.10.10.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Mar 2018 10:10:39 -0700 (PDT) Date: Wed, 14 Mar 2018 18:10:26 +0100 From: Adrien Mazarguil To: Nelio Laranjeiro Cc: Yongseok Koh , dev@dpdk.org Message-ID: <20180314171026.GM3994@6wind.com> References: <205f899062e6acf2d8886bbbd6dac159023a2c04.1520944256.git.nelio.laranjeiro@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <205f899062e6acf2d8886bbbd6dac159023a2c04.1520944256.git.nelio.laranjeiro@6wind.com> Subject: Re: [dpdk-dev] [PATCH 2/5] net/mlx5: retrieve device index from Netlink X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Mar 2018 17:10:42 -0000 On Tue, Mar 13, 2018 at 01:50:36PM +0100, Nelio Laranjeiro wrote: > This patch new file is not compiled yet, it starts a series necessary to > fix some issues with VF devices. > > Signed-off-by: Nelio Laranjeiro The fact this new file is not compiled in at this point won't be obvious when running git bisect and makes validation more difficult. I suggest to simply merge it into the next patch. A few more comments on this code below. > --- > drivers/net/mlx5/mlx5_vf.c | 134 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 134 insertions(+) > create mode 100644 drivers/net/mlx5/mlx5_vf.c > > diff --git a/drivers/net/mlx5/mlx5_vf.c b/drivers/net/mlx5/mlx5_vf.c Regarding the name of this file, it looks to me like all the enclosed functions could work equally well with non-VF devices. While this series is targeted at VFs, internal functions are actually more versatile. I suggest a more generic name, e.g. "mlx5_nl.c". > new file mode 100644 > index 000000000..357407f56 > --- /dev/null > +++ b/drivers/net/mlx5/mlx5_vf.c > @@ -0,0 +1,134 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright 2018 6WIND S.A. > + * Copyright 2018 Mellanox Technologies, Ltd. > + */ > + > +#include > +#include > + > +#include > + > +#include "mlx5.h" > +#include "mlx5_utils.h" > + > +/* Data exchanged between Netlink library and PMD. */ > +struct mlx5_vf_iface { > + struct rte_eth_dev *dev; /**< Pointer to Ethernet device. */ > + int iface_idx; /**< Network Interface index. */ > +}; Minor documentation nits here and elsewhere, you should remove the random capitalization of "Interface", "Message", "Header" and so on in the middle of sentences. > + > +/** > + * Parse Netlink message to retrieve the interface index. > + * > + * @param nh > + * Pointer to Netlink Message Header. > + * @param arg > + * PMD data register with this callback. > + * > + * @return > + * 0 on success, -1 otherwise. Looks like it only returns 0, never -1. You should probably describe that ((struct mlx5_vf_iface)arg)->iface_idx is left to -1 when its index can't be found. > + */ > +static int > +mlx5_vf_iface_idx_cb(struct nlmsghdr *nh, void *arg) > +{ > + struct mlx5_vf_iface *data = arg; > + struct rte_eth_dev *dev = data->dev; > + const struct ether_addr *mac = &dev->data->mac_addrs[0]; > + struct ifinfomsg *iface; > + struct rtattr *attribute; > + int len; > + > + /** > + * Leave right away if the index does not match its initialised value. > + * Interface index has already been found. > + */ > + if (data->iface_idx != -1) > + return 0; > + iface = NLMSG_DATA(nh); > + len = nh->nlmsg_len - NLMSG_LENGTH(sizeof(*iface)); > + for (attribute = IFLA_RTA(iface); > + RTA_OK(attribute, len); > + attribute = RTA_NEXT(attribute, len)) { > + if (attribute->rta_type == IFLA_ADDRESS && > + !memcmp(RTA_DATA(attribute), mac, ETHER_ADDR_LEN)) { Hmm, I'm not sure a matching MAC address is safe enough to determine it's the right netdevice. Could be a bridge (br0) interface or any other virtual thing. Although not great, you should probably rely on /sys as in mlx5_get_ifname(). > +#ifndef NDEBUG > + struct ether_addr m; > + > + memcpy(&m, RTA_DATA(attribute), ETHER_ADDR_LEN); > + DRV_LOG(DEBUG, > + "port %u interace %d MAC address" interace => interface > + " %02X:%02X:%02X:%02X:%02X:%02X", > + dev->data->port_id, > + iface->ifi_index, > + m.addr_bytes[0], m.addr_bytes[1], > + m.addr_bytes[2], m.addr_bytes[3], > + m.addr_bytes[4], m.addr_bytes[5]); > +#endif > + data->iface_idx = iface->ifi_index; > + return 0; > + } > + } > + data->iface_idx = -1; > + return 0; > +} > + > +/** > + * Retrieve interface Netlink interface index. > + * > + * @param dev > + * Pointer to Ethernet device. > + * > + * @return > + * Interface index, a negative errno value otherwise and rte_errno is set. > + */ > +static int > +mlx5_vf_iface_idx(struct rte_eth_dev *dev) > +{ > + struct nl_req { > + struct nlmsghdr hdr; > + struct rtgenmsg gen; > + } req = { > + .hdr = { > + .nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg)), > + .nlmsg_type = RTM_GETLINK, > + .nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP | NLM_F_ROOT, > + }, > + .gen = { > + .rtgen_family = AF_UNSPEC, Extra space before AF_UNSPEC. > + }, > + }; > + struct mlx5_vf_iface data = { > + .dev = dev, > + .iface_idx = -1, > + }; > + int fd; > + int ret; > + > + fd = rte_nl_init(RTMGRP_LINK); > + if (fd < 0) { > + rte_errno = errno; > + goto error; > + } > + ret = rte_nl_send(fd, &req.hdr); > + if (ret == -1) { > + rte_errno = errno; > + goto error; > + } > + ret = rte_nl_recv(fd, mlx5_vf_iface_idx_cb, &data); > + if (ret == -1) { > + rte_errno = errno; > + goto error; > + } > + rte_nl_final(fd); > + if (data.iface_idx == -1) { > + rte_errno = EAGAIN; > + goto error; > + } > + return data.iface_idx; > +error: > + if (fd >= 0) > + rte_nl_final(fd); > + DRV_LOG(DEBUG, "port %u cannot retrieve Netlink Interface index %s", > + dev->data->port_id, strerror(rte_errno)); > + return -rte_errno; > +} > -- > 2.11.0 > Due to possible MAC addresses collisions, I suggest a simpler approach: replacing all this code with a combination of mlx5_get_ifname() and if_nametoindex(). Thoughts? -- Adrien Mazarguil 6WIND