From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro Subject: Re: [PATCH v4 0/2] ethdev: add port speed capability bitmap Date: Mon, 14 Sep 2015 12:02:22 +0200 Message-ID: <20150914100222.GE25122@autoinstall.dev.6wind.com> References: <20150909131037.GA25122@autoinstall.dev.6wind.com> <2699193.9riTyGPe1z@xps13> <2046894.c3eJ0QZGuc@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org, Morten =?iso-8859-1?Q?Br=F8rup?= To: Thomas Monjalon Return-path: 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 ; Mon, 14 Sep 2015 12:02:37 +0200 (CEST) Received: by wiclk2 with SMTP id lk2so133373036wic.0 for ; Mon, 14 Sep 2015 03:02:37 -0700 (PDT) Content-Disposition: inline In-Reply-To: <2046894.c3eJ0QZGuc@xps13> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 : > > > 2015-09-09 15:10, N=E9lio Laranjeiro: > > > > I think V2 is better, maybe you can add a function to convert a s= ingle > > > > bitmap value to the equivalent integer and get rid of ETH_SPEED_X= XX > > > macros. > > > > > > > > Thomas what is your opinion? > > > > > > Your proposal looks good Nelio. > >=20 > > I am confused, specially since you were the one advocating for having= a > > unified set of constants for speeds (discussion in v2). >=20 > Yes, my first thought was advocating an unification between capabilitie= s and > negotiated link properties. > After I was convinced by Nelio's arguments: bitmap is good for capabili= ties > (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 f= unction. >=20 > > In any case, as I see it, if we want to address the comments of M. B= rorup: > >=20 > > http://comments.gmane.org/gmane.comp.networking.dpdk.devel/19664 I read it. > >=20 > > we need bitmaps for rte_eth_conf link_speed to set the advertised spe= eds. >=20 > 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 us= ing the > same field as 3/. > Maybe we should really have 3 different fields. 1/ and 2/ would use a b= itmap. >=20 > > Let me know if you really want to come back to v2 or not. >=20 > 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. --=20 N=E9lio Laranjeiro 6WIND