From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ferruh.yigit@intel.com>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by dpdk.org (Postfix) with ESMTP id CE9092BD1;
 Tue, 31 Jan 2017 17:17:12 +0100 (CET)
Received: from orsmga001.jf.intel.com ([10.7.209.18])
 by fmsmga105.fm.intel.com with ESMTP; 31 Jan 2017 08:17:11 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.33,315,1477983600"; d="scan'208";a="1089330134"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38])
 ([10.237.220.38])
 by orsmga001.jf.intel.com with ESMTP; 31 Jan 2017 08:17:10 -0800
To: Shahaf Shuler <shahafs@mellanox.com>, adrien.mazarguil@6wind.com,
 nelio.laranjeiro@6wind.com
References: <1485348178-43771-1-git-send-email-shahafs@mellanox.com>
 <1485863129-6326-1-git-send-email-shahafs@mellanox.com>
Cc: dev@dpdk.org, stable@dpdk.org
From: Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <c9f8112d-b308-9dff-5d2b-71862dbcb3b7@intel.com>
Date: Tue, 31 Jan 2017 16:17:09 +0000
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.0
MIME-Version: 1.0
In-Reply-To: <1485863129-6326-1-git-send-email-shahafs@mellanox.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/mlx5: fix link status
	query
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 31 Jan 2017 16:17:13 -0000

On 1/31/2017 11:45 AM, Shahaf Shuler wrote:
> Trying to query the link status through new kernel ioctl API
> ETHTOOL_GLINKSETTINGS was always failing due to kernel bug.
> The bug was fixed on version 4.9
> this patch uses the legacy ioctl API for lower kernels.
> 
> Fixes: 188408719888 ("net/mlx5: fix support for newer link speeds")
> CC: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---

<...>

> @@ -707,7 +708,7 @@ struct priv *
>  static int
>  mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, int wait_to_complete)
>  {
> -#ifdef ETHTOOL_GLINKSETTINGS
> +#if KERNEL_VERSION(4, 9, 0) <= LINUX_VERSION_CODE

Mostly it is not good idea to do kernel version check in the .c file.

It is possible to move this comparison to the .h file, and set a feature
macro based on comparison result, like HAVE_ETHTOOL_GLINKSETTINGS, and
use this macro in the .c file.

This makes .c code easier to understand. And the abstraction in the
header file lets you update the comparison in the future without
changing the code itself.

But it is your call, do you prefer to continue with this one?

>  	struct priv *priv = mlx5_get_priv(dev);
>  	struct ethtool_link_settings edata = {
>  		.cmd = ETHTOOL_GLINKSETTINGS,
<...>