[dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap
Nélio Laranjeiro
nelio.laranjeiro at 6wind.com
Mon Sep 14 12:02:22 CEST 2015
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 at 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
More information about the dev
mailing list