From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [RFC 02/14] virtio: use eth_link_read/write (and bug
	fix)
Date: Fri, 05 Jan 2018 16:04:23 +0100
Message-ID: <3049472.WSApEpTIyO@xps>
References: <20170714183027.16021-1-stephen@networkplumber.org>
 <0a1a1f6d-1fef-5a9b-bf80-822aeb2123f0@solarflare.com>
 <20170717092836.0d8932fc@xeon-e3>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, Stephen Hemminger <sthemmin@microsoft.com>
To: Stephen Hemminger <stephen@networkplumber.org>,
 Andrew Rybchenko <arybchenko@solarflare.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com
 [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id A97431D7
 for <dev@dpdk.org>; Fri,  5 Jan 2018 16:04:45 +0100 (CET)
In-Reply-To: <20170717092836.0d8932fc@xeon-e3>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

17/07/2017 18:28, Stephen Hemminger:
> On Mon, 17 Jul 2017 19:14:16 +0300
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
> > On 07/17/2017 07:01 PM, Stephen Hemminger wrote:
> > > On Sun, 16 Jul 2017 15:33:26 +0300
> > > Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> > >  
> > >>> +	link.link_autoneg = ETH_LINK_SPEED_FIXED;  
> > >> As I understand link_autoneg is 1 bit field with boolean semantics. I.e. 0/false - no autoneg, 1/true - autoneg.
> > >> It looks like it has wrong comment:
> > >>           uint16_t link_autoneg : 1;  /**< ETH_LINK_SPEED_[AUTONEG/FIXED] */
> > >>
> > >> since
> > >> #define ETH_LINK_SPEED_AUTONEG  (0 <<  0)  /**< Autonegotiate (all speeds) */
> > >> #define ETH_LINK_SPEED_FIXED    (1 <<  0)  /**< Disable autoneg (fixed speed) */
> > >>
> > >> whereas
> > >> #define ETH_LINK_FIXED          0 /**< No autonegotiation. */
> > >> #define ETH_LINK_AUTONEG        1 /**< Autonegotiated. */
> > >>
> > >> In general this attempt to introduce bug is the result of wrong comment which is caused by very similar
> > >> defines with opposite values.  
> > > Orignal observation was because some drivers (vmxnet3) were setting autoneg = fixed
> > > and others were not. Turns out it makes no difference
> > > since FIXED == 0, the old code and new code have same effect.  
> > 
> > May be I miss something, but  ETH_LINK_SPEED_FIXED==1, but 
> > ETH_LINK_FIXED==0.
> > So, initially it was 0 (fixed speed), but now it is 1 (autoneg).
> 
> My understanding is that ETH_SPEED_XXX and ETH_LINK_FIXED are for rte_eth_link
> structure; and ETH_LINK_SPEED_XXX values are for dev_info->speed_capa

Right.
The comment is wrong.
rte_eth_link.link_autoneg must uses ETH_LINK_[FIXED/AUTONEG]
ETH_LINK_SPEED_[AUTONEG/FIXED] is for rte_eth_conf.link_speeds
and for rte_eth_dev_info.speed_capa.

So in this patch, you should set link.link_autoneg = ETH_LINK_FIXED.

I will send a patch to fix the comment in ethdev.