On 28 March 2016 at 15:42, Thomas Monjalon <thomas.monjalon at 6wind.com> wrote:
> Hi Marc, > > 2016-03-27 21:39, Marc: > > On 27 March 2016 at 11:53, Thomas Monjalon <thomas.monjalon at 6wind.com> > > wrote: > > > 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: > [...] > > > > > > 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? > > > > Agree on all of them. I can add them in the (hopefully) final v14, once > all > > or main drivers have been tested. > > It would be better to make the v14 as soon as possible to let others test > the latest revision. > Thanks > Adding these MACROs (ETH_LINK_FIXED, ETH_LINK_AUTONEG) are the only changes to be done for v14 so far. The rest were there already (v13). There is no point on doing this re-spin now, because these MACROs are anyway not used by applications yet (autoneg flag in rte_link is an addition to this patch set). So I would go for testing drivers on this v13 and collect any changes, eventually, for v14. marc
