[2/2] Revert "net/mlx5: fix master device Netlink socket sharing"

Message ID 1563514305-27405-3-git-send-email-viacheslavo@mellanox.com
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers show
Series
  • net/mlx5: cache the associated network device ifindex
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Viacheslav Ovsiienko July 19, 2019, 5:31 a.m.
This reverts commit e28111ac9864af09e826241a915dfff87a9c00ad.
The netlink requests are replaced by ifindex caching and
not needed anymore.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Fixes: e28111ac9864 ("net/mlx5: fix master device Netlink socket sharing")
---
 drivers/net/mlx5/mlx5.h        |   6 ---
 drivers/net/mlx5/mlx5_ethdev.c | 109 ++---------------------------------------
 2 files changed, 3 insertions(+), 112 deletions(-)

Comments

Stephen Hemminger July 19, 2019, 4:16 p.m. | #1
On Fri, 19 Jul 2019 05:31:45 +0000
Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:

> This reverts commit e28111ac9864af09e826241a915dfff87a9c00ad.
> The netlink requests are replaced by ifindex caching and
> not needed anymore.
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Fixes: e28111ac9864 ("net/mlx5: fix master device Netlink socket sharing")


Can mlx5 drop dependency o netlink (libmnl)?
Viacheslav Ovsiienko July 19, 2019, 4:21 p.m. | #2
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Friday, July 19, 2019 19:17
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>
> Subject: Re: [PATCH 2/2] Revert "net/mlx5: fix master device Netlink socket
> sharing"
> 
> On Fri, 19 Jul 2019 05:31:45 +0000
> Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:
> 
> > This reverts commit e28111ac9864af09e826241a915dfff87a9c00ad.
> > The netlink requests are replaced by ifindex caching and not needed
> > anymore.
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > Fixes: e28111ac9864 ("net/mlx5: fix master device Netlink socket
> > sharing")
> 
> 
> Can mlx5 drop dependency o netlink (libmnl)?

What do you mean?  Dependency on libmnl is already dropped.
The few remaining Netlink requests do not use libmnl (but does libnl).

With best regards, Slava
Stephen Hemminger July 19, 2019, 4:23 p.m. | #3
On Fri, 19 Jul 2019 16:21:17 +0000
Slava Ovsiienko <viacheslavo@mellanox.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Friday, July 19, 2019 19:17
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>; Shahaf Shuler
> > <shahafs@mellanox.com>
> > Subject: Re: [PATCH 2/2] Revert "net/mlx5: fix master device Netlink socket
> > sharing"
> > 
> > On Fri, 19 Jul 2019 05:31:45 +0000
> > Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:
> >   
> > > This reverts commit e28111ac9864af09e826241a915dfff87a9c00ad.
> > > The netlink requests are replaced by ifindex caching and not needed
> > > anymore.
> > >
> > > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > > Fixes: e28111ac9864 ("net/mlx5: fix master device Netlink socket
> > > sharing")  
> > 
> > 
> > Can mlx5 drop dependency o netlink (libmnl)?  
> 
> What do you mean?  Dependency on libmnl is already dropped.
> The few remaining Netlink requests do not use libmnl (but does libnl).
> 
> With best regards, Slava

Good to see, libnl is quite large, it would be good to drop that
(or use libmnl instead).

Patch

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 4ae6738..b150107 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -518,15 +518,9 @@  struct mlx5_priv {
 /* mlx5_ethdev.c */
 
 int mlx5_get_ifname(const struct rte_eth_dev *dev, char (*ifname)[IF_NAMESIZE]);
-int mlx5_get_ifname_base(const struct rte_eth_dev *base,
-			 const struct rte_eth_dev *dev,
-			 char (*ifname)[IF_NAMESIZE]);
 int mlx5_get_master_ifname(const char *ibdev_path, char (*ifname)[IF_NAMESIZE]);
 unsigned int mlx5_ifindex(const struct rte_eth_dev *dev);
 int mlx5_ifreq(const struct rte_eth_dev *dev, int req, struct ifreq *ifr);
-int mlx5_ifreq_base(const struct rte_eth_dev *base,
-		    const struct rte_eth_dev *dev,
-		    int req, struct ifreq *ifr);
 int mlx5_get_mtu(struct rte_eth_dev *dev, uint16_t *mtu);
 int mlx5_set_flags(struct rte_eth_dev *dev, unsigned int keep,
 		   unsigned int flags);
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 3803be3..e418676 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -240,51 +240,6 @@  struct ethtool_link_settings {
 }
 
 /**
- * Get interface name for the specified device, uses the extra base
- * device resources to perform Netlink requests.
- *
- * This is a port representor-aware version of mlx5_get_master_ifname().
- *
- * @param[in] base
- *   Pointer to Ethernet device to use Netlink socket from
- *   to perfrom requests.
- * @param[in] dev
- *   Pointer to Ethernet device.
- * @param[out] ifname
- *   Interface name output buffer.
- *
- * @return
- *   0 on success, a negative errno value otherwise and rte_errno is set.
- */
-int
-mlx5_get_ifname_base(const struct rte_eth_dev *base,
-		     const struct rte_eth_dev *dev,
-		     char (*ifname)[IF_NAMESIZE])
-{
-	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_priv *priv_base = base->data->dev_private;
-	unsigned int ifindex;
-
-	assert(priv);
-	assert(priv->sh);
-	assert(priv_base);
-	ifindex = priv_base->nl_socket_rdma >= 0 ?
-		  mlx5_nl_ifindex(priv_base->nl_socket_rdma,
-				  priv->sh->ibdev_name,
-				  priv->ibv_port) : 0;
-	if (!ifindex) {
-		if (!priv->representor)
-			return mlx5_get_master_ifname(priv->sh->ibdev_path,
-						      ifname);
-		rte_errno = ENXIO;
-		return -rte_errno;
-	}
-	if (if_indextoname(ifindex, &(*ifname)[0]))
-		return 0;
-	rte_errno = errno;
-	return -rte_errno;
-}
-/**
  * Get the interface index from device name.
  *
  * @param[in] dev
@@ -346,51 +301,6 @@  struct ethtool_link_settings {
 }
 
 /**
- * Perform ifreq ioctl() on specified Ethernet device,
- * ifindex, name and other attributes are requested
- * on the base device to avoid specified device Netlink
- * socket sharing (this is not thread-safe).
- *
- * @param[in] base
- *   Pointer to Ethernet device to get dev attributes.
- * @param[in] dev
- *   Pointer to Ethernet device to perform ioctl.
- * @param req
- *   Request number to pass to ioctl().
- * @param[out] ifr
- *   Interface request structure output buffer.
- *
- * @return
- *   0 on success, a negative errno value otherwise and rte_errno is set.
- */
-int
-mlx5_ifreq_base(const struct rte_eth_dev *base,
-		const struct rte_eth_dev *dev,
-		int req, struct ifreq *ifr)
-{
-	int sock = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP);
-	int ret = 0;
-
-	if (sock == -1) {
-		rte_errno = errno;
-		return -rte_errno;
-	}
-	ret = mlx5_get_ifname_base(base, dev, &ifr->ifr_name);
-	if (ret)
-		goto error;
-	ret = ioctl(sock, req, ifr);
-	if (ret == -1) {
-		rte_errno = errno;
-		goto error;
-	}
-	close(sock);
-	return 0;
-error:
-	close(sock);
-	return -rte_errno;
-}
-
-/**
  * Get device MTU.
  *
  * @param dev
@@ -872,15 +782,7 @@  int mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size)
 				ifr = (struct ifreq) {
 					.ifr_data = (void *)&edata,
 				};
-				/*
-				 * Use special version of mlx5_ifreq()
-				 * to get master device name with local
-				 * device Netlink socket. Using master
-				 * device Netlink socket is not thread
-				 * safe.
-				 */
-				ret = mlx5_ifreq_base(dev, master,
-						      SIOCETHTOOL, &ifr);
+				ret = mlx5_ifreq(master, SIOCETHTOOL, &ifr);
 			}
 		}
 		if (ret) {
@@ -977,12 +879,7 @@  int mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size)
 				ifr = (struct ifreq) {
 					.ifr_data = (void *)&gcmd,
 				};
-				/*
-				 * Avoid using master Netlink socket.
-				 * This is not thread-safe.
-				 */
-				ret = mlx5_ifreq_base(dev, master,
-						      SIOCETHTOOL, &ifr);
+				ret = mlx5_ifreq(master, SIOCETHTOOL, &ifr);
 			}
 		}
 		if (ret) {
@@ -1003,7 +900,7 @@  int mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size)
 
 	*ecmd = gcmd;
 	ifr.ifr_data = (void *)ecmd;
-	ret = mlx5_ifreq_base(dev, master ? master : dev, SIOCETHTOOL, &ifr);
+	ret = mlx5_ifreq(master ? master : dev, SIOCETHTOOL, &ifr);
 	if (ret) {
 		DRV_LOG(DEBUG,
 			"port %u ioctl(SIOCETHTOOL,"