From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id A97431D7 for ; Fri, 5 Jan 2018 16:04:45 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id E4BAA2102A; Fri, 5 Jan 2018 10:04:44 -0500 (EST) Received: from frontend1 ([10.202.2.160]) by compute1.internal (MEProxy); Fri, 05 Jan 2018 10:04:44 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=u2X7n2zlY5CEbq6cvUMKn3KOKw 7WhY94qqkwRaHqcRE=; b=CWSHsC7jrbeqoIbhNT86VdQIOeiWAgQD0h/TVJjEMu Q5D3mWcEtvhVTbbJuOdemh1nYDYxkkbWlz8IQHS4QjaLNDqF6KRZ0MRLzsSdmU8j JAs0wVYM3HC9bYmrH+J1yZPffGnWPjIdudWN1kVaZ9Rw2lYv6ARBvdXFkEGlRjxf E= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=u2X7n2 zlY5CEbq6cvUMKn3KOKw7WhY94qqkwRaHqcRE=; b=d1GlTe9c+iTuI1fmTxZRn9 cwy6BH9n1LtoL9tvtcmnVradqebGhmhwDC9/VvfMJaR4uEB4EXxH7A2xrlJGnLse dSZY0vrRfJRCQV43jcE7jRImwnGeGEEhsabA5h7xLRJt0Gxut0BoY89hCYEGSWgt RCIXuiLa14fBNGUj64JFxhp/bH5+X8CQrvwOZUoFHlAv0bmZaL1qy76ipzgR+MJ6 JRH/tZSNnXqTsQ1i42W+a7ua6lZ7gNP/wvMnrWduyudoJATa00rmx2jidhQ/vHxM GhKEDhO334sbC/2C0IojGe6Oav8pCLDvbSX1FA8qYXozsU9CYhEF1rvxc6DFLXPA == X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 92BAF7E43A; Fri, 5 Jan 2018 10:04:44 -0500 (EST) From: Thomas Monjalon To: Stephen Hemminger , Andrew Rybchenko Cc: dev@dpdk.org, Stephen Hemminger Date: Fri, 05 Jan 2018 16:04:23 +0100 Message-ID: <3049472.WSApEpTIyO@xps> In-Reply-To: <20170717092836.0d8932fc@xeon-e3> References: <20170714183027.16021-1-stephen@networkplumber.org> <0a1a1f6d-1fef-5a9b-bf80-822aeb2123f0@solarflare.com> <20170717092836.0d8932fc@xeon-e3> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [RFC 02/14] virtio: use eth_link_read/write (and bug fix) 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: Fri, 05 Jan 2018 15:04:45 -0000 17/07/2017 18:28, Stephen Hemminger: > On Mon, 17 Jul 2017 19:14:16 +0300 > Andrew Rybchenko wrote: > > > On 07/17/2017 07:01 PM, Stephen Hemminger wrote: > > > On Sun, 16 Jul 2017 15:33:26 +0300 > > > Andrew Rybchenko 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.