Thread (167 messages) 167 messages, 19 authors, 2016-04-06

Re: [PATCH v11 0/8] ethdev: 100G and link speed API refactoring

From: Marc <hidden>
Date: 2016-03-26 10:24:44

On 26 March 2016 at 09:08, Thomas Monjalon [off-list ref]
wrote:
Hi Marc,

Thanks for finding time to help.

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
marc@Beluga:~/personal/dpdk/tools$ git diff
diff --git a/lib/librte_ether/rte_ethdev.h
b/lib/librte_ether/rte_ethdev.h
quoted
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) */
quoted
-#define ETH_LINK_SPEED_AUTONEG  (1 <<  0)  /**< Autonegotiate (all
speeds) */
quoted
+#define ETH_LINK_SPEED_AUTONEG  (0 <<  0)  /**< Autonegotiate (all
speeds) */
quoted
+#define ETH_LINK_SPEED_FIXED    (1 <<  0)  /**< Disable autoneg (fixed
speed) */
quoted
 #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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help