From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f46.google.com (mail-wm0-f46.google.com [74.125.82.46]) by dpdk.org (Postfix) with ESMTP id 267B1DE5 for ; Wed, 1 Feb 2017 13:58:07 +0100 (CET) Received: by mail-wm0-f46.google.com with SMTP id 196so25726357wmm.1 for ; Wed, 01 Feb 2017 04:58:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=K3GpRXf70MegcwaxLm6HsDZBeYcLcuhSdTYIyNQCdFQ=; b=1ulvq5JneKBO1Fr5E3+OxXNUH4nz163ntd1fYyBogGOC1HeydUgyNZ2CPkjXPiwIha tvKBzdXc50mBNwxxpK6hf08IKshXrQKTusQUFk0nkyfm9GDXpyUFAIGs+t2WlUp/5V9y pDqtD79L2VMdYDV3RyQoqtNscWnsL/2ww+GlrdIA2rBvRXTpGd8eCmb25E6Ab+OfOyCN Q84FEomj8/YwbSm6OKgTknfrZLlLWGVEZjcmBRz2wiA3q1kn5t3UjnCSl+yDXNRr362z nS9tLmhK2hQMR9GSU9HTdHhk7mWdneQpEHpkdaurQ55WOxdWDl2ZlQrHCFzv9cNPLvGR u1XQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=K3GpRXf70MegcwaxLm6HsDZBeYcLcuhSdTYIyNQCdFQ=; b=cxEycuReuydnd57Wh6MoTjSAswTtv8jvNJmROF59kfzUqpOoYsVD1d6xK61FmuGtRE lS/COJWQubMGvWzc9ntu7Ejbglk77ht6QMPn+bVk3rRzhx5OJCzDdQQLiGsHjhtBUx/7 IfzCJsqIoBqLUEGQVdQ3nKY8ccqjKnG8dLFcAzqsvohSRmgZL4W3+sV+pDAMd2r3NIS4 k23J/nEHgPGuoeDrkhgrMmx5Il6JqOrOLj2/FhZChR8VmdD84LA4KKp4281yXNOjb9TW G7FdCXawBLgT7UhEXE6JrdtY15oo0UOvRe/zQIEmZyVsdxUOtrm5IPbbiuybYFtmfKk2 j3HA== X-Gm-Message-State: AIkVDXLE8ZCVh8zuaWwUVK+BTYU+bsVjZw/7xDEMo2J7ebiyUkatzgtVYVHGazjwvNsemCAS X-Received: by 10.223.148.230 with SMTP id 93mr2575921wrr.13.1485953886705; Wed, 01 Feb 2017 04:58:06 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id k43sm34000514wrc.46.2017.02.01.04.58.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Feb 2017 04:58:05 -0800 (PST) Date: Wed, 1 Feb 2017 13:57:58 +0100 From: Adrien Mazarguil To: Ferruh Yigit Cc: Shahaf Shuler , =?utf-8?B?TsOpbGlv?= Laranjeiro , "dev@dpdk.org" , "stable@dpdk.org" Message-ID: <20170201125758.GS10133@6wind.com> References: <1485348178-43771-1-git-send-email-shahafs@mellanox.com> <1485863129-6326-1-git-send-email-shahafs@mellanox.com> <20170201090711.GP10133@6wind.com> <64089318-6d12-28b4-61e8-6e52785ce692@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <64089318-6d12-28b4-61e8-6e52785ce692@intel.com> 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Feb 2017 12:58:07 -0000 On Wed, Feb 01, 2017 at 11:13:59AM +0000, Ferruh Yigit wrote: > On 2/1/2017 9:07 AM, Adrien Mazarguil wrote: > > On Wed, Feb 01, 2017 at 06:53:55AM +0000, Shahaf Shuler wrote: > >> : Tuesday, January 31, 2017 6:17 PM, Ferruh Yigit: > >>> 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 > >>>> --- > >>> > >>> <...> > >>> > >>>> @@ -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? > >> > >> This is a good suggestion. > >> Adrien, Nélio what do you think? > > > > Let's include this patch as-is. Doing so in a header file such as mlx5.h > > would require including linux/version.h from that file and cause the entire > > PMD to be even more OS-dependent. > > > > We'll move this check elsewhere in the future if we need several such > > workarounds, thanks. > > OK. > > One more thing, comment log says: > "The bug was fixed on version 4.9" > > And code does: > "+#if KERNEL_VERSION(4, 9, 0) <= LINUX_VERSION_CODE" > > If the bug is fixed in 4.9, should check be "<" instead of "<=" I'll concede the argument order used in this condition is somewhat unusual but it actually ends up being the same as: #if LINUX_VERSION_CODE > KERNEL_VERSION(4, 9, 0) Which is the correct intent. I guess you can update this line for clarity if you think it's necessary. > > > >>>> struct priv *priv = mlx5_get_priv(dev); > >>>> struct ethtool_link_settings edata = { > >>>> .cmd = ETHTOOL_GLINKSETTINGS, > >>> <...> > > > -- Adrien Mazarguil 6WIND