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