On 26 March 2016 at 09:08, Thomas Monjalon <thomas.monjalon at 6wind.com>
wrote:
> Hi Marc,
>
> Thanks for finding time to help.
>
> 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.
The only benefit of your approach is not to have another macro #define
ETH_LINK_AUTONEG 1, which is marginal IMHO.
>
> Would it be possible to fix without reverting?
>
At least, all existing applications will have to be modified. I would have
to go through v12 again to see if there are other issues still to be fixed,
and also apply the 2 fixes I found for e1000.
Marc
