17/07/2017 18:28, Stephen Hemminger: > On Mon, 17 Jul 2017 19:14:16 +0300 > Andrew Rybchenko <arybche...@solarflare.com> wrote: > > > On 07/17/2017 07:01 PM, Stephen Hemminger wrote: > > > On Sun, 16 Jul 2017 15:33:26 +0300 > > > Andrew Rybchenko <arybche...@solarflare.com> wrote: > > > > > >>> + link.link_autoneg = ETH_LINK_SPEED_FIXED; > > >> As I understand link_autoneg is 1 bit field with boolean semantics. I.e. > > >> 0/false - no autoneg, 1/true - autoneg. > > >> It looks like it has wrong comment: > > >> uint16_t link_autoneg : 1; /**< > > >> ETH_LINK_SPEED_[AUTONEG/FIXED] */ > > >> > > >> since > > >> #define ETH_LINK_SPEED_AUTONEG (0 << 0) /**< Autonegotiate (all > > >> speeds) */ > > >> #define ETH_LINK_SPEED_FIXED (1 << 0) /**< Disable autoneg (fixed > > >> speed) */ > > >> > > >> whereas > > >> #define ETH_LINK_FIXED 0 /**< No autonegotiation. */ > > >> #define ETH_LINK_AUTONEG 1 /**< Autonegotiated. */ > > >> > > >> In general this attempt to introduce bug is the result of wrong comment > > >> which is caused by very similar > > >> defines with opposite values. > > > Orignal observation was because some drivers (vmxnet3) were setting > > > autoneg = fixed > > > and others were not. Turns out it makes no difference > > > since FIXED == 0, the old code and new code have same effect. > > > > May be I miss something, but ETH_LINK_SPEED_FIXED==1, but > > ETH_LINK_FIXED==0. > > So, initially it was 0 (fixed speed), but now it is 1 (autoneg). > > My understanding is that ETH_SPEED_XXX and ETH_LINK_FIXED are for rte_eth_link > structure; and ETH_LINK_SPEED_XXX values are for dev_info->speed_capa
Right. The comment is wrong. rte_eth_link.link_autoneg must uses ETH_LINK_[FIXED/AUTONEG] ETH_LINK_SPEED_[AUTONEG/FIXED] is for rte_eth_conf.link_speeds and for rte_eth_dev_info.speed_capa. So in this patch, you should set link.link_autoneg = ETH_LINK_FIXED. I will send a patch to fix the comment in ethdev.
