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

  Use a function to retrieve the version of the kernel.

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_ethdev.c | 13 +++++--------
 drivers/net/mlx5/mlx5_utils.h  | 26 ++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 8 deletions(-)
  

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
>
  

On Thu, 15 Feb 2018 09:47:27 +0100
Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:

> Use a function to retrieve the version of the kernel.
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

This type of logic will run into problems with Enterprise and vendor kernels.
What about users using older kernels with OFED? What about case where kernel
MLX drivers have been backported?

In general, it is better to adapt to the environment the code is running
in. Try the new feature, if it fails then log a message (at info level)
and fallback to the other strategy.
  

On Fri, Feb 16, 2018 at 10:03:04AM -0800, Stephen Hemminger wrote:
> On Thu, 15 Feb 2018 09:47:27 +0100
> Nelio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
> 
> > Use a function to retrieve the version of the kernel.
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> This type of logic will run into problems with Enterprise and vendor kernels.
> What about users using older kernels with OFED? What about case where kernel
> MLX drivers have been backported?
> 
> In general, it is better to adapt to the environment the code is running
> in. Try the new feature, if it fails then log a message (at info level)
> and fallback to the other strategy.

We have already discussed it, this new API in the kernel is buggee
between v4.5 to v4.9.  As there is now way in between those version to
detect if the API is the buggee one or not, we don't have any other
solution.

It is better to not use at all this new API than using a buggee one.
  

This series applies on top of [1] and cleans up the DPDK API implementation
for the link status.

[1] https://dpdk.org/dev/patchwork/patch/35653/

Changes in v2:
- Removes kernel version verification, the bug it tried to detected was fixed
  several commit after in the PMD.  Implementation in mlx5 kernel driver is
  only available since v4.9.
- Removes the alarm handler as this can be worked around by not acknowledging
  the event until the link becomes stable.
- Clean-up the API implementation by letting the application handle the
  interrupt and decide by itself to start/stop the device.

Nelio Laranjeiro (3):
  net/mlx5: remove kernel version check
  net/mlx5: fix link status behavior
  net/mlx5: fix link status to use wait to complete

 drivers/net/mlx5/mlx5.c         |   2 +-
 drivers/net/mlx5/mlx5.h         |   1 -
 drivers/net/mlx5/mlx5_defs.h    |   4 +-
 drivers/net/mlx5/mlx5_ethdev.c  | 244 +++++++++-------------------------------
 drivers/net/mlx5/mlx5_trigger.c |  15 ++-
 5 files changed, 69 insertions(+), 197 deletions(-)
  

Monday, March 12, 2018 3:43 PM, Nelio Laranjeiro:
> This series applies on top of [1] and cleans up the DPDK API implementation
> for the link status.
> 
> [1]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdp
> dk.org%2Fdev%2Fpatchwork%2Fpatch%2F35653%2F&data=02%7C01%7Csha
> hafs%40mellanox.com%7C53adbbb3cbac4c7036f508d5881f6948%7Ca652971c
> 7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636564590863924988&sdata=ic9Y
> UOQLHq719ebOXxX%2FlS%2BgzWX7R7CFa7qXrOEG7xs%3D&reserved=0
> 
> Changes in v2:
> - Removes kernel version verification, the bug it tried to detected was fixed
>   several commit after in the PMD.  Implementation in mlx5 kernel driver is
>   only available since v4.9.
> - Removes the alarm handler as this can be worked around by not
> acknowledging
>   the event until the link becomes stable.
> - Clean-up the API implementation by letting the application handle the
>   interrupt and decide by itself to start/stop the device.
> 
> Nelio Laranjeiro (3):
>   net/mlx5: remove kernel version check
>   net/mlx5: fix link status behavior
>   net/mlx5: fix link status to use wait to complete

Series applied to next-net-mlx, thanks.