2016-03-26 11:24, Marc: > On 26 March 2016 at 09:08, Thomas Monjalon <thomas.monjalon at 6wind.com> > wrote: > > 2016-03-25 22:30, Marc: > > > From v9 to v10 patchset the values ETH_LINK_SPEED_AUTONEG and > > ETH_LINK_SPEED_FIXED were flipped. Reverting this makes it work: > > > > > > marc at Beluga:~/personal/dpdk/tools$ git diff > > > diff --git a/lib/librte_ether/rte_ethdev.h > > b/lib/librte_ether/rte_ethdev.h > > > index ef2502a..fb247a7 100644 > > > --- a/lib/librte_ether/rte_ethdev.h > > > +++ b/lib/librte_ether/rte_ethdev.h > > > @@ -244,8 +244,8 @@ struct rte_eth_stats { > > > /** > > > * Device supported speeds bitmap flags > > > */ > > > -#define ETH_LINK_SPEED_FIXED (0 << 0) /**< Disable autoneg (fixed > > speed) */ > > > -#define ETH_LINK_SPEED_AUTONEG (1 << 0) /**< Autonegotiate (all > > speeds) */ > > > +#define ETH_LINK_SPEED_AUTONEG (0 << 0) /**< Autonegotiate (all > > speeds) */ > > > +#define ETH_LINK_SPEED_FIXED (1 << 0) /**< Disable autoneg (fixed > > speed) */ > > > #define ETH_LINK_SPEED_10M_HD (1 << 1) /**< 10 Mbps half-duplex */ > > > #define ETH_LINK_SPEED_10M (1 << 2) /**< 10 Mbps full-duplex */ > > > #define ETH_LINK_SPEED_100M_HD (1 << 3) /**< 100 Mbps half-duplex */ > > > > > > I think having autoneg == 0 is better. Do you agree Thomas? > > > > No I do not agree, because this flag is now used also for > > rte_eth_link.link_autoneg. > > So it is more logic to set it to 1. > > Having to explicitly specify ETH_LINK_SPEED_AUTONEG in link_speeds during > port configuration for achieving auto-negociation, which is what 98% of > applications will use, seems anti-natural to me and breaks existing > applications.
Yes, you're right. We have to avoid apps modifications. By keeping autoneg the default behaviour (value 0), we avoid issues. > The only benefit of your approach is not to have another macro #define > ETH_LINK_AUTONEG 1, which is marginal IMHO. Yes, you're right. I suggest adding 2 macros for rte_eth_link.link_autoneg: #define ETH_LINK_FIXED 0 /**< No autonegotiation. */ #define ETH_LINK_AUTONEG 1 /**< Autonegotiated. */ and keep these ones to use with rte_eth_conf.link_speeds and rte_eth_dev_info.speed_capa: #define ETH_LINK_SPEED_AUTONEG (0 << 0) /**< Autonegotiate (all speeds) */ #define ETH_LINK_SPEED_FIXED (1 << 0) /**< Disable autoneg (fixed speed) */ Opinions?