Re: [dpdk-dev] [PATCH 1/2] net/mlx5: add kernel version function

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

On Thu, Feb 15, 2018 at 09:47:27AM +0100, Nelio Laranjeiro wrote:
> Use a function to retrieve the version of the kernel.
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

A couple of nits, please see below.

> ---
>  drivers/net/mlx5/mlx5_ethdev.c | 13 +++++--------
>  drivers/net/mlx5/mlx5_utils.h  | 26 ++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index b73cb53df..0ce9f438a 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -18,11 +18,9 @@
>  #include <net/if.h>
>  #include <sys/ioctl.h>
>  #include <sys/socket.h>
> -#include <sys/utsname.h>
>  #include <netinet/in.h>
>  #include <linux/ethtool.h>
>  #include <linux/sockios.h>
> -#include <linux/version.h>

Looks like this one should remain; this file still uses KERNEL_VERSION().

>  #include <fcntl.h>
>  #include <stdalign.h>
>  #include <sys/un.h>
> @@ -715,15 +713,14 @@ int
>  priv_link_update(struct priv *priv, int wait_to_complete)
>  {
>  	struct rte_eth_dev *dev = priv->dev;
> -	struct utsname utsname;
> -	int ver[3];
>  	int ret;
>  	struct rte_eth_link dev_link = dev->data->dev_link;
> +	unsigned int current_version = mlx5_kernel_version();
> +	int use_new_api = current_version > 0 ?
> +		current_version >= KERNEL_VERSION(4, 9, 0) :
> +		0;
>  
> -	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))
> +	if (use_new_api)
>  		ret = mlx5_link_update_unlocked_gset(dev, wait_to_complete);
>  	else
>  		ret = mlx5_link_update_unlocked_gs(dev, wait_to_complete);
> diff --git a/drivers/net/mlx5/mlx5_utils.h b/drivers/net/mlx5/mlx5_utils.h
> index e1bfb9cd9..bd179ed21 100644
> --- a/drivers/net/mlx5/mlx5_utils.h
> +++ b/drivers/net/mlx5/mlx5_utils.h
> @@ -12,6 +12,8 @@
>  #include <limits.h>
>  #include <assert.h>
>  #include <errno.h>
> +#include <sys/utsname.h>
> +#include <linux/version.h>
>  
>  #include "mlx5_defs.h"
>  
> @@ -159,4 +161,28 @@ log2above(unsigned int v)
>  	return l + r;
>  }
>  
> +/**
> + * retrieve the current kernel version.
> + *
> + * @return
> + *   the current kernel version or negative errno on error.
> + */

Missing caps!

> +static inline unsigned int
> +mlx5_kernel_version(void)
> +{
> +	struct utsname utsname;
> +	int ver[3];
> +	int ret;
> +
> +	ret = uname(&utsname);
> +	if (ret == -1)
> +		goto error;
> +	ret = sscanf(utsname.release, "%d.%d.%d", &ver[0], &ver[1], &ver[2]);
> +	if (ret != 3)
> +		goto error;
> +	return KERNEL_VERSION(ver[0], ver[1], ver[2]);
> +error:
> +	return -errno;
> +}
> +
>  #endif /* RTE_PMD_MLX5_UTILS_H_ */
> -- 
> 2.11.0
> 

-- 
Adrien Mazarguil
6WIND