From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <nelio.laranjeiro@6wind.com>
Received: from mail-wi0-f178.google.com (mail-wi0-f178.google.com
 [209.85.212.178]) by dpdk.org (Postfix) with ESMTP id CC8FD6A80
 for <dev@dpdk.org>; Mon, 14 Sep 2015 12:02:37 +0200 (CEST)
Received: by wiclk2 with SMTP id lk2so133373036wic.0
 for <dev@dpdk.org>; Mon, 14 Sep 2015 03:02:37 -0700 (PDT)
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-type:content-disposition
 :content-transfer-encoding:in-reply-to:user-agent;
 bh=uYrtLNNlDdLxPJ4qrfIUKkJDL4PPQ7Aw4J5dHLrV1+k=;
 b=LmDjztAwMTgIKmwSx4z/m0k4TIUThWQrC3Fyyy5y2uEYlgnegst5WnhdozO+EwztPe
 dylWyMU+Wc/Mt0l9jpLvjXTY8U0v9JXvoCZVkB/hITaHOA1FKwGsLX0lhatFp00+wjC9
 zbzxeL5hJj+EHh8p5FbDRlys5PeoNLGngzZd6kmA+sTa37h7xcpU6VhqvkwBC7qRsvHf
 jcZfAn0dBNKQfJr3Tm1/jKZHV8Eopoq7P+NBcg+mmzS1iUXA1YsOPwLHTAGBXeGCWebD
 wNjF4Bzz8Zdh026NxbQIVmRBza/nSlXVvzEBzYIsFHe1fu/FduKxLSVmzY0w7Xu05AXf
 ucCw==
X-Gm-Message-State: ALoCoQmkfPKfALWk1PSKeoXavlbjoam8tK5GPN+j/I2auckn0PBN0kDtVb1TUuFzrS4Z4/Ida+4y
X-Received: by 10.180.86.137 with SMTP id p9mr25069904wiz.38.1442224957623;
 Mon, 14 Sep 2015 03:02:37 -0700 (PDT)
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 s9sm14668212wjy.16.2015.09.14.03.02.36
 (version=TLSv1.2 cipher=RC4-SHA bits=128/128);
 Mon, 14 Sep 2015 03:02:37 -0700 (PDT)
Date: Mon, 14 Sep 2015 12:02:22 +0200
From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro <nelio.laranjeiro@6wind.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Message-ID: <20150914100222.GE25122@autoinstall.dev.6wind.com>
References: <20150909131037.GA25122@autoinstall.dev.6wind.com>
 <2699193.9riTyGPe1z@xps13>
 <CA+3n-TorPf4Kp3YsA3TpDpJ2pyqwQ_x3OVjA_2jm9_-q_8+uFA@mail.gmail.com>
 <2046894.c3eJ0QZGuc@xps13>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <2046894.c3eJ0QZGuc@xps13>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: dev@dpdk.org, Morten =?iso-8859-1?Q?Br=F8rup?= <mb@smartsharesystems.com>
Subject: Re: [dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability
 bitmap
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: Mon, 14 Sep 2015 10:02:38 -0000

On Sun, Sep 13, 2015 at 11:18:33PM +0200, Thomas Monjalon wrote:
> 2015-09-13 21:14, Marc Sune:
> > 2015-09-09 15:33 GMT+02:00 Thomas Monjalon <thomas.monjalon@6wind.com>:
> > > 2015-09-09 15:10, Nélio Laranjeiro:
> > > > I think V2 is better, maybe you can add a function to convert a single
> > > > bitmap value to the equivalent integer and get rid of ETH_SPEED_XXX
> > > macros.
> > > >
> > > > Thomas what is your opinion?
> > >
> > > Your proposal looks good Nelio.
> > 
> > I am confused, specially since you were the one advocating for having a
> > unified set of constants for speeds (discussion in v2).
> 
> Yes, my first thought was advocating an unification between capabilities and
> negotiated link properties.
> After I was convinced by Nelio's arguments: bitmap is good for capabilities
> (especially to describe every capabilities in one field) but integer is better
> for negotiated speed (especially for aggregated links).
> Converting bitmap speed into integer should be easy to implement in a function.
> 
> > In any case, as I see it, if we want to address the comments of  M. Brorup:
> > 
> > http://comments.gmane.org/gmane.comp.networking.dpdk.devel/19664

I read it.

> > 
> > we need bitmaps for rte_eth_conf link_speed to set the advertised speeds.
> 
> Yes I forgot this interesting comment. It is saying we need
> 	1/ capabilities
> 	2/ advertised modes (for auto-negotiation or fixed config)
> 	3/ negotiated mode
> Previously we were focused only on 1/ and 3/.
> 2/ was only limited to a mode configured without negotiation and was using the
> same field as 3/.
> Maybe we should really have 3 different fields. 1/ and 2/ would use a bitmap.
> 
> > Let me know if you really want to come back to v2 or not.
> 
> It needs more discussion. What do you think of the above proposal?
> What is the opinion of Nelio? Morten?

I agree with Morten, and with this proposition (we should be possible to
disable some capabilities in the advertise field).

For that we should have those 3 fields:
  1/ Capability (as a bitmap),  necessary for the user to configure the
     behavior of the PHY.
  2/ Advertise (as a bitmap) to configure the PHY.
  3/ speed, duplex, status (as rte_eth_link structure), necessary to
     verify that the configuration corresponds to what has been set and
     for other purposes.

I still think Marc needs to go back to V2 and continue from this one.
And as Thomas suggested, he could have a function to get rid of the
double defines and use the sames ones for bitmap and non bitmap fields.

Just for information, before I started this discussion I have worked on
a patch to change the rte_eth_link.link_speed from 16 to 32 bit, I did
not pushed it because, it should be pushed afters Marc's one, and it
will need some rework after that.

-- 
Nélio Laranjeiro
6WIND