On 28 March 2016 at 15:42, Thomas Monjalon [off-list ref]
wrote:

Hi Marc,

2016-03-27 21:39, Marc:

quoted

On 27 March 2016 at 11:53, Thomas Monjalon [off-list ref]
wrote:

quoted

2016-03-26 11:24, Marc:

quoted

On 26 March 2016 at 09:08, Thomas Monjalon <

thomas.monjalon@6wind.com>

quoted

quoted

quoted

wrote:

quoted

2016-03-25 22:30, Marc:

quoted

From v9 to v10 patchset the values ETH_LINK_SPEED_AUTONEG and

ETH_LINK_SPEED_FIXED were flipped. Reverting this makes it work:

[...]

quoted

quoted

quoted

quoted

quoted

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

quoted

quoted

quoted

port configuration for achieving auto-negociation, which is what 98%

of

quoted

quoted

quoted

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.

quoted

The only benefit of your approach is not to have another macro

#define

quoted

quoted

quoted

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) */

quoted

quoted

#define ETH_LINK_SPEED_FIXED    (1 <<  0)  /**< Disable autoneg (fixed

speed) */

quoted

quoted

Opinions?

Agree on all of them. I can add them in the (hopefully) final v14, once

all

quoted

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