Re: [dpdk-dev] [PATCH 1/2] common/mlx5: share kernel interface name getter

[Matan Azrad <matan@mellanox.com>]

Yes, more conflict are exists with current master-net-mlx.
I have newest version rebased on master-net-mlx.

I will send new version, for all the old versions...

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, June 18, 2020 7:07 PM
> To: Matan Azrad <matan@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 1/2] common/mlx5: share kernel interface name getter
> 
> Your series does not apply, there are no common ancestor so I can't do 3-
> way merge.
> 
> Could you please rebase it on top of next-virtio's master branch?
> 
> Thanks,
> Maxime
> 
> On 6/2/20 5:53 PM, Matan Azrad wrote:
> > Some configuration of the mlx5 port are done by the kernel net device
> > associated to the IB device represents the PCI device.
> >
> > The DPDK mlx5 driver uses linux system calls, for example ioctl, in
> > order to configure per port configurations requested by the DPDK user.
> >
> > One of the basic knowledges required to access the correct kernel net
> > device is its name.
> >
> > Move function to get interface name from IB device path to the common
> > library.
> >
> > Signed-off-by: Matan Azrad <matan@mellanox.com>
> > ---
> >  drivers/common/mlx5/mlx5_common.c               | 88
> ++++++++++++++++++++++++
> >  drivers/common/mlx5/mlx5_common.h               |  3 +
> >  drivers/common/mlx5/rte_common_mlx5_version.map |  2 +
> >  drivers/net/mlx5/mlx5.c                         |  2 +-
> >  drivers/net/mlx5/mlx5.h                         |  1 -
> >  drivers/net/mlx5/mlx5_ethdev.c                  | 90 +------------------------
> >  6 files changed, 96 insertions(+), 90 deletions(-)
> >
> > diff --git a/drivers/common/mlx5/mlx5_common.c
> > b/drivers/common/mlx5/mlx5_common.c
> > index 1c77763..3f0d8d3 100644
> > --- a/drivers/common/mlx5/mlx5_common.c
> > +++ b/drivers/common/mlx5/mlx5_common.c
> > @@ -8,8 +8,10 @@
> >  #ifdef RTE_IBVERBS_LINK_DLOPEN
> >  #include <dlfcn.h>
> >  #endif
> > +#include <dirent.h>
> >
> >  #include <rte_errno.h>
> > +#include <rte_string_fns.h>
> >
> >  #include "mlx5_common.h"
> >  #include "mlx5_common_utils.h"
> > @@ -167,6 +169,92 @@ enum mlx5_class
> >  	return;
> >  }
> >
> > +/**
> > + * Get kernel interface name from IB device path.
> > + *
> > + * @param[in] ibdev_path
> > + *   Pointer to IB device path.
> > + * @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_sysfs(const char *ibdev_path, char
> > +(*ifname)[IF_NAMESIZE]) {
> > +	DIR *dir;
> > +	struct dirent *dent;
> > +	unsigned int dev_type = 0;
> > +	unsigned int dev_port_prev = ~0u;
> > +	char match[IF_NAMESIZE] = "";
> > +
> > +	MLX5_ASSERT(ibdev_path);
> > +	{
> > +		MKSTR(path, "%s/device/net", ibdev_path);
> > +
> > +		dir = opendir(path);
> > +		if (dir == NULL) {
> > +			rte_errno = errno;
> > +			return -rte_errno;
> > +		}
> > +	}
> > +	while ((dent = readdir(dir)) != NULL) {
> > +		char *name = dent->d_name;
> > +		FILE *file;
> > +		unsigned int dev_port;
> > +		int r;
> > +
> > +		if ((name[0] == '.') &&
> > +		    ((name[1] == '\0') ||
> > +		     ((name[1] == '.') && (name[2] == '\0'))))
> > +			continue;
> > +
> > +		MKSTR(path, "%s/device/net/%s/%s",
> > +		      ibdev_path, name,
> > +		      (dev_type ? "dev_id" : "dev_port"));
> > +
> > +		file = fopen(path, "rb");
> > +		if (file == NULL) {
> > +			if (errno != ENOENT)
> > +				continue;
> > +			/*
> > +			 * Switch to dev_id when dev_port does not exist as
> > +			 * is the case with Linux kernel versions < 3.15.
> > +			 */
> > +try_dev_id:
> > +			match[0] = '\0';
> > +			if (dev_type)
> > +				break;
> > +			dev_type = 1;
> > +			dev_port_prev = ~0u;
> > +			rewinddir(dir);
> > +			continue;
> > +		}
> > +		r = fscanf(file, (dev_type ? "%x" : "%u"), &dev_port);
> > +		fclose(file);
> > +		if (r != 1)
> > +			continue;
> > +		/*
> > +		 * Switch to dev_id when dev_port returns the same value
> for
> > +		 * all ports. May happen when using a MOFED release older
> than
> > +		 * 3.0 with a Linux kernel >= 3.15.
> > +		 */
> > +		if (dev_port == dev_port_prev)
> > +			goto try_dev_id;
> > +		dev_port_prev = dev_port;
> > +		if (dev_port == 0)
> > +			strlcpy(match, name, sizeof(match));
> > +	}
> > +	closedir(dir);
> > +	if (match[0] == '\0') {
> > +		rte_errno = ENOENT;
> > +		return -rte_errno;
> > +	}
> > +	strncpy(*ifname, match, sizeof(*ifname));
> > +	return 0;
> > +}
> > +
> >  #ifdef MLX5_GLUE
> >
> >  /**
> > diff --git a/drivers/common/mlx5/mlx5_common.h
> > b/drivers/common/mlx5/mlx5_common.h
> > index 8cd3ea5..4f6373b 100644
> > --- a/drivers/common/mlx5/mlx5_common.h
> > +++ b/drivers/common/mlx5/mlx5_common.h
> > @@ -6,6 +6,7 @@
> >  #define RTE_PMD_MLX5_COMMON_H_
> >
> >  #include <stdio.h>
> > +#include <net/if.h>
> >
> >  #include <rte_pci.h>
> >  #include <rte_debug.h>
> > @@ -212,6 +213,8 @@ enum mlx5_class {
> >  __rte_internal
> >  void mlx5_translate_port_name(const char *port_name_in,
> >  			      struct mlx5_switch_info *port_info_out);
> > +__rte_internal
> > +int mlx5_get_ifname_sysfs(const char *ibdev_path, char
> > +(*ifname)[IF_NAMESIZE]);
> >
> >  extern uint8_t haswell_broadwell_cpu;
> >
> > diff --git a/drivers/common/mlx5/rte_common_mlx5_version.map
> > b/drivers/common/mlx5/rte_common_mlx5_version.map
> > index b3410df..a62584d 100644
> > --- a/drivers/common/mlx5/rte_common_mlx5_version.map
> > +++ b/drivers/common/mlx5/rte_common_mlx5_version.map
> > @@ -33,6 +33,8 @@ INTERNAL {
> >  	mlx5_devx_cmd_query_virtq;
> >  	mlx5_devx_get_out_command_status;
> >
> > +	mlx5_get_ifname_sysfs;
> > +
> >  	mlx5_mp_init_primary;
> >  	mlx5_mp_uninit_primary;
> >  	mlx5_mp_init_secondary;
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> > 469ff73..91af47b 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -3570,7 +3570,7 @@ struct mlx5_flow_id_pool *
> >  					 */
> >  					continue;
> >  				}
> > -				ret = mlx5_get_master_ifname
> > +				ret = mlx5_get_ifname_sysfs
> >  					(ibv_match[i]->ibdev_path,
> &ifname);
> >  				if (!ret)
> >  					list[ns].ifindex =
> > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> > 2908c8b..536afae 100644
> > --- a/drivers/net/mlx5/mlx5.h
> > +++ b/drivers/net/mlx5/mlx5.h
> > @@ -645,7 +645,6 @@ int mlx5_udp_tunnel_port_add(struct rte_eth_dev
> > *dev,
> >  /* mlx5_ethdev.c */
> >
> >  int mlx5_get_ifname(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_get_mtu(struct rte_eth_dev *dev, uint16_t *mtu); diff --git
> > a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> > index b837ce6..62344d1 100644
> > --- a/drivers/net/mlx5/mlx5_ethdev.c
> > +++ b/drivers/net/mlx5/mlx5_ethdev.c
> > @@ -129,95 +129,9 @@ struct ethtool_link_settings {  #endif
> >
> >  /**
> > - * Get master interface name from private structure.
> > - *
> > - * @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_master_ifname(const char *ibdev_path, char
> > (*ifname)[IF_NAMESIZE]) -{
> > -	DIR *dir;
> > -	struct dirent *dent;
> > -	unsigned int dev_type = 0;
> > -	unsigned int dev_port_prev = ~0u;
> > -	char match[IF_NAMESIZE] = "";
> > -
> > -	MLX5_ASSERT(ibdev_path);
> > -	{
> > -		MKSTR(path, "%s/device/net", ibdev_path);
> > -
> > -		dir = opendir(path);
> > -		if (dir == NULL) {
> > -			rte_errno = errno;
> > -			return -rte_errno;
> > -		}
> > -	}
> > -	while ((dent = readdir(dir)) != NULL) {
> > -		char *name = dent->d_name;
> > -		FILE *file;
> > -		unsigned int dev_port;
> > -		int r;
> > -
> > -		if ((name[0] == '.') &&
> > -		    ((name[1] == '\0') ||
> > -		     ((name[1] == '.') && (name[2] == '\0'))))
> > -			continue;
> > -
> > -		MKSTR(path, "%s/device/net/%s/%s",
> > -		      ibdev_path, name,
> > -		      (dev_type ? "dev_id" : "dev_port"));
> > -
> > -		file = fopen(path, "rb");
> > -		if (file == NULL) {
> > -			if (errno != ENOENT)
> > -				continue;
> > -			/*
> > -			 * Switch to dev_id when dev_port does not exist as
> > -			 * is the case with Linux kernel versions < 3.15.
> > -			 */
> > -try_dev_id:
> > -			match[0] = '\0';
> > -			if (dev_type)
> > -				break;
> > -			dev_type = 1;
> > -			dev_port_prev = ~0u;
> > -			rewinddir(dir);
> > -			continue;
> > -		}
> > -		r = fscanf(file, (dev_type ? "%x" : "%u"), &dev_port);
> > -		fclose(file);
> > -		if (r != 1)
> > -			continue;
> > -		/*
> > -		 * Switch to dev_id when dev_port returns the same value
> for
> > -		 * all ports. May happen when using a MOFED release older
> than
> > -		 * 3.0 with a Linux kernel >= 3.15.
> > -		 */
> > -		if (dev_port == dev_port_prev)
> > -			goto try_dev_id;
> > -		dev_port_prev = dev_port;
> > -		if (dev_port == 0)
> > -			strlcpy(match, name, sizeof(match));
> > -	}
> > -	closedir(dir);
> > -	if (match[0] == '\0') {
> > -		rte_errno = ENOENT;
> > -		return -rte_errno;
> > -	}
> > -	strncpy(*ifname, match, sizeof(*ifname));
> > -	return 0;
> > -}
> > -
> > -/**
> >   * Get interface name from private structure.
> >   *
> > - * This is a port representor-aware version of mlx5_get_master_ifname().
> > + * This is a port representor-aware version of mlx5_get_ifname_sysfs().
> >   *
> >   * @param[in] dev
> >   *   Pointer to Ethernet device.
> > @@ -238,7 +152,7 @@ struct ethtool_link_settings {
> >  	ifindex = mlx5_ifindex(dev);
> >  	if (!ifindex) {
> >  		if (!priv->representor)
> > -			return mlx5_get_master_ifname(priv->sh-
> >ibdev_path,
> > +			return mlx5_get_ifname_sysfs(priv->sh-
> >ibdev_path,
> >  						      ifname);
> >  		rte_errno = ENXIO;
> >  		return -rte_errno;
> >