[dpdk-dev,2/2] mlx5: don't depend on kernel version

  This driver uses ethtool to get link status. The ethtool API has new
and old deprecated API. Rather than checking kernel version, use the
same algorithm that the ethtool command does; check the new API first
and if that fails, try the old one.

Also, use common code for getting link state up/down and comparing
for changes.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/mlx5/mlx5_ethdev.c | 110 ++++++++++++++++++-----------------------
 1 file changed, 47 insertions(+), 63 deletions(-)
  

Hi Stephen,

Please see few comments bellow,

On Tue, Jan 02, 2018 at 12:53:10PM -0800, Stephen Hemminger wrote:
> This driver uses ethtool to get link status. The ethtool API has new
> and old deprecated API. Rather than checking kernel version, use the
> same algorithm that the ethtool command does; check the new API first
> and if that fails, try the old one.
> 
> Also, use common code for getting link state up/down and comparing
> for changes.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/mlx5/mlx5_ethdev.c | 110 ++++++++++++++++++-----------------------
>  1 file changed, 47 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 388507f109f7..2dc32cdf58b9 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -49,7 +49,6 @@
>  #include <netinet/in.h>
>  #include <linux/ethtool.h>
>  #include <linux/sockios.h>
> -#include <linux/version.h>
>  #include <fcntl.h>
>  #include <stdalign.h>
>  #include <sys/un.h>
> @@ -757,36 +756,25 @@ mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev)
>   *   Pointer to Ethernet device structure.
>   */
>  static int
> -mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev)
> +mlx5_link_update_unlocked_gset(struct priv *priv, struct ifreq *ifr,
> +			       struct rte_eth_link *dev_link)

Function documentation should also describe the new parameter.

>  {
> -	struct priv *priv = mlx5_get_priv(dev);
>  	struct ethtool_cmd edata = {
>  		.cmd = ETHTOOL_GSET /* Deprecated since Linux v4.5. */
>  	};
> -	struct ifreq ifr;
> -	struct rte_eth_link dev_link;
>  	int link_speed = 0;
>  
> -	/* priv_lock() is not taken to allow concurrent calls. */
> -
> -	if (priv_ifreq(priv, SIOCGIFFLAGS, &ifr)) {
> -		WARN("ioctl(SIOCGIFFLAGS) failed: %s", strerror(errno));
> -		return -1;
> -	}
> -	memset(&dev_link, 0, sizeof(dev_link));
> -	dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> -				(ifr.ifr_flags & IFF_RUNNING));
> -	ifr.ifr_data = (void *)&edata;
> -	if (priv_ifreq(priv, SIOCETHTOOL, &ifr)) {
> +	ifr->ifr_data = (void *)&edata;
> +	if (priv_ifreq(priv, SIOCETHTOOL, ifr)) {
>  		WARN("ioctl(SIOCETHTOOL, ETHTOOL_GSET) failed: %s",
>  		     strerror(errno));
>  		return -1;
>  	}
>  	link_speed = ethtool_cmd_speed(&edata);
>  	if (link_speed == -1)
> -		dev_link.link_speed = 0;
> +		dev_link->link_speed = 0;
>  	else
> -		dev_link.link_speed = link_speed;
> +		dev_link->link_speed = link_speed;
>  	priv->link_speed_capa = 0;
>  	if (edata.supported & SUPPORTED_Autoneg)
>  		priv->link_speed_capa |= ETH_LINK_SPEED_AUTONEG;
> @@ -800,17 +788,9 @@ mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev)
>  			       SUPPORTED_40000baseSR4_Full |
>  			       SUPPORTED_40000baseLR4_Full))
>  		priv->link_speed_capa |= ETH_LINK_SPEED_40G;
> -	dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
> +	dev_link->link_duplex = ((edata.duplex == DUPLEX_HALF) ?
>  				ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> -	dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> -			ETH_LINK_SPEED_FIXED);
> -	if (memcmp(&dev_link, &dev->data->dev_link, sizeof(dev_link))) {
> -		/* Link status changed. */
> -		dev->data->dev_link = dev_link;
> -		return 0;
> -	}
> -	/* Link status is still the same. */
> -	return -1;
> +	return 0;
>  }
>  
>  /**
> @@ -820,23 +800,14 @@ mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev)
>   *   Pointer to Ethernet device structure.
>   */
>  static int
> -mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev)
> +mlx5_link_update_unlocked_gs(struct priv *priv, struct ifreq *ifr,
> +			     struct rte_eth_link *dev_link)

Same here.

>  {
> -	struct priv *priv = mlx5_get_priv(dev);
>  	struct ethtool_link_settings gcmd = { .cmd = ETHTOOL_GLINKSETTINGS };
> -	struct ifreq ifr;
> -	struct rte_eth_link dev_link;
>  	uint64_t sc;
>  
> -	if (priv_ifreq(priv, SIOCGIFFLAGS, &ifr)) {
> -		WARN("ioctl(SIOCGIFFLAGS) failed: %s", strerror(errno));
> -		return -1;
> -	}
> -	memset(&dev_link, 0, sizeof(dev_link));
> -	dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> -				(ifr.ifr_flags & IFF_RUNNING));
> -	ifr.ifr_data = (void *)&gcmd;
> -	if (priv_ifreq(priv, SIOCETHTOOL, &ifr)) {
> +	ifr->ifr_data = (void *)&gcmd;
> +	if (priv_ifreq(priv, SIOCETHTOOL, ifr)) {
>  		DEBUG("ioctl(SIOCETHTOOL, ETHTOOL_GLINKSETTINGS) failed: %s",
>  		      strerror(errno));
>  		return -1;
> @@ -849,13 +820,13 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev)
>  	struct ethtool_link_settings *ecmd = (void *)data;
>  
>  	*ecmd = gcmd;
> -	ifr.ifr_data = (void *)ecmd;
> -	if (priv_ifreq(priv, SIOCETHTOOL, &ifr)) {
> +	ifr->ifr_data = (void *)ecmd;
> +	if (priv_ifreq(priv, SIOCETHTOOL, ifr)) {
>  		DEBUG("ioctl(SIOCETHTOOL, ETHTOOL_GLINKSETTINGS) failed: %s",
>  		      strerror(errno));
>  		return -1;
>  	}
> -	dev_link.link_speed = ecmd->speed;
> +	dev_link->link_speed = ecmd->speed;
>  	sc = ecmd->link_mode_masks[0] |
>  		((uint64_t)ecmd->link_mode_masks[1] << 32);
>  	priv->link_speed_capa = 0;
> @@ -893,17 +864,9 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev)
>  		  MLX5_BITSHIFT(ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT) |
>  		  MLX5_BITSHIFT(ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT)))
>  		priv->link_speed_capa |= ETH_LINK_SPEED_100G;
> -	dev_link.link_duplex = ((ecmd->duplex == DUPLEX_HALF) ?
> +	dev_link->link_duplex = ((ecmd->duplex == DUPLEX_HALF) ?
>  				ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> -	dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> -				  ETH_LINK_SPEED_FIXED);
> -	if (memcmp(&dev_link, &dev->data->dev_link, sizeof(dev_link))) {
> -		/* Link status changed. */
> -		dev->data->dev_link = dev_link;
> -		return 0;
> -	}
> -	/* Link status is still the same. */
> -	return -1;
> +	return 0;
>  }
>  
>  /**
> @@ -917,15 +880,36 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev)
>  int
>  mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete __rte_unused)
>  {
> -	struct utsname utsname;
> -	int ver[3];
> -
> -	if (uname(&utsname) == -1 ||
> -	    sscanf(utsname.release, "%d.%d.%d",
> -		   &ver[0], &ver[1], &ver[2]) != 3 ||
> -	    KERNEL_VERSION(ver[0], ver[1], ver[2]) < KERNEL_VERSION(4, 9, 0))
> -		return mlx5_link_update_unlocked_gset(dev);
> -	return mlx5_link_update_unlocked_gs(dev);
> +	struct priv *priv = mlx5_get_priv(dev);
> +	struct rte_eth_link dev_link;
> +	struct ifreq ifr;
> +	int ret;
> +	
> +	if (priv_ifreq(priv, SIOCGIFFLAGS, &ifr)) {
> +		WARN("ioctl(SIOCGIFFLAGS) failed: %s", strerror(errno));
> +		return -1;
> +	}
> +	memset(&dev_link, 0, sizeof(dev_link));
> +	dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> +				(ifr.ifr_flags & IFF_RUNNING));
> +
> +	dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> +			ETH_LINK_SPEED_FIXED);
> +
> +	ret = mlx5_link_update_unlocked_gs(priv, &ifr, &dev_link);
> +	if (ret)
> +		ret = mlx5_link_update_unlocked_gset(priv, &ifr, &dev_link);
> +
> +	if (ret)
> +		return ret;
> +
> +	if (memcmp(&dev_link, &dev->data->dev_link, sizeof(dev_link))) {
> +		/* Link status changed. */
> +		dev->data->dev_link = dev_link;
> +		return 0;
> +	}
> +	/* Link status is still the same. */
> +	return -1;

This function does not respect the PMD coding style, please remove the
blank lines between the code.

Thanks,