From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <nelio.laranjeiro@6wind.com>
Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41])
 by dpdk.org (Postfix) with ESMTP id EACC22BAA
 for <dev@dpdk.org>; Wed,  9 Mar 2016 11:20:54 +0100 (CET)
Received: by mail-wm0-f41.google.com with SMTP id p65so185578470wmp.1
 for <dev@dpdk.org>; Wed, 09 Mar 2016 02:20:54 -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
 :user-agent; bh=fVWs/GIhbDqexPOAi/qfwZEnCUKDOxf0GOXECSLUvSw=;
 b=RJcAkqKB1WIggkiHtlZ/DdDRuPwk6jyXaTE5DMrCiIDqFu+mMUDHx387nbZ0RXZITL
 C/+xb+K0YTCM5e8WzkjCs0s03I6oRcbPrZqxw8SmusUbpxOlIef8CCCqYbrX+5tcn3cC
 7LcxRoFM1C0yYjlKS20PuUpEExt7v3RIbkdfb70K2OukbkW451VDF+ByzNVa2NblQSLZ
 1OKu5eX89YGbwRsVDN2oprLsIF7YcKMPFjm1JlamT4IMy+uC4nZHVYXglVjDcqRg/nWy
 xgmE6yB2iSVKuo2TxqMgEi3488IVkVTM+wpfjQpfyV6YlVWm8hXyStINulQpGseNzV6j
 iz4w==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:references
 :mime-version:content-disposition:content-transfer-encoding
 :in-reply-to:user-agent;
 bh=fVWs/GIhbDqexPOAi/qfwZEnCUKDOxf0GOXECSLUvSw=;
 b=KalBlNFLGqSl3v1PIf2g/SjBt+3pV/ROsVZAM7q68k6HndctoczfKqXGAcY3ZuseGz
 fP1JsD78ocrWjsc8ok1NXDkfvYobjq9LJiKX8BkwqFyJUVJNpUjBm/C1A2LqMJVt4Wx9
 Nq1K2qaTvomHUouKiQPy2jV3Sd9gHnFpUJU+ZRkF13w/aL9L8hmbJsvRuwq8RVTUL4ej
 EpUqOjSrSd4MIXjTjex71D5S1wJp79Kpz42xzREciZd9no5N87CxmSAdHYs+2EAH0MlP
 HziJR1E039k6I3Ws3yOIeZvB2X/z6dsNnxgqrx3AzmwWobk/hsB4rNOWzW2Kmwe9aQch
 Xhwg==
X-Gm-Message-State: AD7BkJK3C8th6R05n06vF5KF7onDQ8itm1jCY7OpqUKITJMUsRp75QGTySEEHBRYaCSmEqC8
X-Received: by 10.194.185.144 with SMTP id fc16mr34175734wjc.123.1457518854820; 
 Wed, 09 Mar 2016 02:20:54 -0800 (PST)
Received: from autoinstall.dev.6wind.com
 (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177])
 by smtp.gmail.com with ESMTPSA id hq2sm7208593wjb.3.2016.03.09.02.20.53
 (version=TLSv1/SSLv3 cipher=OTHER);
 Wed, 09 Mar 2016 02:20:54 -0800 (PST)
Date: Wed, 9 Mar 2016 11:20:40 +0100
From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro <nelio.laranjeiro@6wind.com>
To: Marc <marcdevel@gmail.com>
Message-ID: <20160309102040.GQ27714@autoinstall.dev.6wind.com>
References: <1455488259-1000-1-git-send-email-marcdevel@gmail.com>
 <1456793151-1475-1-git-send-email-marcdevel@gmail.com>
 <1456793151-1475-4-git-send-email-marcdevel@gmail.com>
 <20160309084547.GM27714@autoinstall.dev.6wind.com>
 <CAExC=0S_me_Je0tvFh9LfRWi4XmsZrZvWJG32qPCaCQHRjCBvA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAExC=0S_me_Je0tvFh9LfRWi4XmsZrZvWJG32qPCaCQHRjCBvA@mail.gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v9 3/4] ethdev: redesign link speed config API
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <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: Wed, 09 Mar 2016 10:20:55 -0000

On Wed, Mar 09, 2016 at 11:09:55AM +0100, Marc wrote:
>    On 9 March 2016 at 09:45, Nélio Laranjeiro <[1]nelio.laranjeiro@6wind.com>
>    wrote:
> 
>      Hi Marc,
> 
>      A small remark bellow.
>      On Tue, Mar 01, 2016 at 01:45:50AM +0100, Marc Sune wrote:
>      > This patch redesigns the API to set the link speed/s configure
>      > for an ethernet port. Specifically:
>      >
>      > - it allows to define a set of advertised speeds for
>      >   auto-negociation.
>      > - it allows to disable link auto-negociation (single fixed speed).
>      > - default: auto-negociate all supported speeds.
>      >
>      > Other changes:
>      >
>      > * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
>      >   values of all supported link speeds, in Mbps.
>      > * Converted link_speed to uint32_t to accomodate 100G speeds
>      >   and beyond (bug).
>      > * Added autoneg flag in struct rte_eth_link to indicate if
>      >   link speed was a result of auto-negociation or was fixed
>      >   by configuration.
>      > * Added utility function to convert numeric speeds to bitmap
>      >   fields.
>      > * Added rte_eth_speed_to_bm_flag() to version map.
>      >
>      > Signed-off-by: Marc Sune <[2]marcdevel@gmail.com>
>      > ---
>      >  app/test-pipeline/init.c                  |   2 +-
>      >  app/test-pmd/cmdline.c                    | 124
>      +++++++++++++++---------------
>      >  app/test-pmd/config.c                     |   4 +-
>      >  app/test/virtual_pmd.c                    |   4 +-
>      >  drivers/net/af_packet/rte_eth_af_packet.c |   5 +-
>      >  drivers/net/bnx2x/bnx2x_ethdev.c          |   8 +-
>      >  drivers/net/bonding/rte_eth_bond_8023ad.c |  14 ++--
>      >  drivers/net/cxgbe/base/t4_hw.c            |   8 +-
>      >  drivers/net/cxgbe/cxgbe_ethdev.c          |   2 +-
>      >  drivers/net/e1000/em_ethdev.c             | 116
>      ++++++++++++++--------------
>      >  drivers/net/e1000/igb_ethdev.c            | 111
>      +++++++++++++-------------
>      >  drivers/net/fm10k/fm10k_ethdev.c          |   8 +-
>      >  drivers/net/i40e/i40e_ethdev.c            |  73 +++++++++---------
>      >  drivers/net/i40e/i40e_ethdev_vf.c         |  11 +--
>      >  drivers/net/ixgbe/ixgbe_ethdev.c          |  78 ++++++++-----------
>      >  drivers/net/mlx4/mlx4.c                   |   6 +-
>      >  drivers/net/mlx5/mlx5_ethdev.c            |  10 +--
>      >  drivers/net/mpipe/mpipe_tilegx.c          |   6 +-
>      >  drivers/net/nfp/nfp_net.c                 |   4 +-
>      >  drivers/net/null/rte_eth_null.c           |   5 +-
>      >  drivers/net/pcap/rte_eth_pcap.c           |   9 ++-
>      >  drivers/net/ring/rte_eth_ring.c           |   5 +-
>      >  drivers/net/virtio/virtio_ethdev.c        |   2 +-
>      >  drivers/net/virtio/virtio_ethdev.h        |   2 -
>      >  drivers/net/vmxnet3/vmxnet3_ethdev.c      |   5 +-
>      >  drivers/net/xenvirt/rte_eth_xenvirt.c     |   5 +-
>      >  examples/ip_pipeline/config_parse.c       |   3 +-
>      >  lib/librte_ether/rte_ethdev.c             |  49 ++++++++++++
>      >  lib/librte_ether/rte_ethdev.h             | 113
>      +++++++++++++++++----------
>      >  lib/librte_ether/rte_ether_version.map    |   6 ++
>      >  30 files changed, 448 insertions(+), 350 deletions(-)
>      >
>      > diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
>      > index db2196b..6a69fe2 100644
>      > --- a/app/test-pipeline/init.c
>      > +++ b/app/test-pipeline/init.c
>      > @@ -200,7 +200,7 @@ app_ports_check_link(void)
>      >               port = (uint8_t) app.ports[i];
>      >               memset(&link, 0, sizeof(link));
>      >               rte_eth_link_get_nowait(port, &link);
>      > -             RTE_LOG(INFO, USER1, "Port %u (%u Gbps) %s\n",
>      > +             RTE_LOG(INFO, USER1, "Port %u (%d Gbps) %s\n",
>      >                       port,
>      >                       link.link_speed / 1000,
>      >                       link.link_status ? "UP" : "DOWN");
>      > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>      > index 52e9f5f..57ad25f 100644
>      > --- a/app/test-pmd/cmdline.c
>      > +++ b/app/test-pmd/cmdline.c
>      > @@ -956,14 +956,65 @@ struct cmd_config_speed_all {
>      >       cmdline_fixed_string_t value2;
>      >  };
>      >
>      > +static int
>      > +parse_and_check_speed_duplex(char *value1, char *value2, uint32_t
>      *link_speed)
> 
>      "value" variables should have the more friendly name, we need to read
>      the
>      code to understand what is value1 and value2.
> 
>    Hello,
>    Ok, but note that the entire source code of cmdline.c uses value1 and
>    value2 to reference speed and duplex mode. This is not something I
>    introduced in this patch.
>    Marc

Ok, I did not look in the whole file only in your patch.  You kept
the logic of the file, it is good for me.

>      >[...]
>      --
>      Nélio Laranjeiro
>      6WIND
> 
> References
> 
>    Visible links
>    1. mailto:nelio.laranjeiro@6wind.com
>    2. mailto:marcdevel@gmail.com

-- 
Nélio Laranjeiro
6WIND