From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f44.google.com (mail-pg0-f44.google.com [74.125.83.44]) by dpdk.org (Postfix) with ESMTP id CC2392C50 for ; Mon, 17 Jul 2017 18:28:43 +0200 (CEST) Received: by mail-pg0-f44.google.com with SMTP id k14so82462358pgr.0 for ; Mon, 17 Jul 2017 09:28:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=CnbkLkMADKhbYqOlEUjmqqTgk/H9SsuzfhlJf7lZO4k=; b=qphciG47HuWolYqTT7O31PiH8BzOfrfs0qakRForPso2n5sKSnuv7VsT/RhMOPBi5z JWvk7finamX/gPUt4qYXlV5DuDt3/ytlWsUHw37/CWPzWTkiUqLt/uzV9cnLMPIBDVsW 2HGuoEfCPZU/44t9FRJTh4LVSfhUdzhZdpXaTzeni2yzZTk4xD7HLjWzQJZ3DeJaxnKc hvwQa7uoDS/d2twbuxcq8nkPXKsZJVtfho99gZtR5tdTN4/fTqZPRjyboEbDYc4GtMA7 +Ui+sV5o+lHAePFyeSQnvt3EZ7VxGQFJA1uHTt286rvuHoG5qJA3LXPcF0huW2KN1msx HY4w== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=CnbkLkMADKhbYqOlEUjmqqTgk/H9SsuzfhlJf7lZO4k=; b=fWv6zC/PUNnvDe0RVTb86HIdsEBkFUcDKjtOSfgpOmjvgT/ThXlDN9am93KTznauyG GFmt/SXtoqMb7wodAEA499B/cIRxwNqUU3ZxyA7GFVasifK6qhy6s/SyXq7GD7dWyU46 U4IIR+kHA2Ju2GLfYCjV3jptTD/KV0na9lVOGE67+E6VxkUJuGD8qFYtKV+pKNtSPr4/ xsxITGaFXUbEoN5gCxHrpHb6y7/fDsgp0PkWzcGsxb3bsimHGDbUTO1BZzNkv6C5hZTD xI1JowA6+3QniT7k1+Vgl/WWT7YHW+gnexqaozio/Yg2jj26snNEsA6DkMtd3dPj7UQy ve2g== X-Gm-Message-State: AIVw113vWI7+w9oCnTYH6WRKrnECnHyO2vuTx4EEnlZ8Z0WOiuI3pz15 MwZ7/qq9C67E8kln X-Received: by 10.98.113.71 with SMTP id m68mr20044928pfc.220.1500308923037; Mon, 17 Jul 2017 09:28:43 -0700 (PDT) Received: from xeon-e3 (76-14-207-240.or.wavecable.com. [76.14.207.240]) by smtp.gmail.com with ESMTPSA id f15sm8376108pfj.127.2017.07.17.09.28.42 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 17 Jul 2017 09:28:42 -0700 (PDT) Date: Mon, 17 Jul 2017 09:28:36 -0700 From: Stephen Hemminger To: Andrew Rybchenko Cc: , Stephen Hemminger , "Thomas Monjalon" Message-ID: <20170717092836.0d8932fc@xeon-e3> In-Reply-To: <0a1a1f6d-1fef-5a9b-bf80-822aeb2123f0@solarflare.com> References: <20170714183027.16021-1-stephen@networkplumber.org> <20170714183027.16021-3-stephen@networkplumber.org> <20170717090122.18655a27@xeon-e3> <0a1a1f6d-1fef-5a9b-bf80-822aeb2123f0@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: Mon, 17 Jul 2017 16:28:44 -0000 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 Would be good to use some kind of typedef for this, maybe introduce a bitfield for speed_capa and get rid of the ETH_LINK_SPEED_XXX.