2015-10-05 12:59 GMT+02:00 Neil Horman <nhorman at tuxdriver.com>:
> On Sun, Oct 04, 2015 at 11:12:46PM +0200, Marc Sune wrote:
> > This patch redesigns the API to set the link speed/s configure
> > for an ethernet port. Specifically:
> >
> > - it allows to define a set of advertised speeds for
> > auto-negociation.
> > - it allows to disable link auto-negociation (single fixed speed).
> > - default: auto-negociate all supported speeds.
> >
> > Other changes:
> >
> > * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
> > values of all supported link speeds, in Mbps.
> > * Converted link_speed to uint32_t to accomodate 100G speeds
> > (bug).
> > * Added autoneg flag in struct rte_eth_link to indicate if
> > link speed was a result of auto-negociation or was fixed
> > by configuration.
> > * Added utility function to convert numeric speeds to bitmap
> > fields.
> > * Adapted testpmd to the new link API.
> >
> > Signed-off-by: Marc Sune <marcdevel at gmail.com>
> >
> > diff --git a/lib/librte_ether/rte_ethdev.c
> b/lib/librte_ether/rte_ethdev.c
> > index f593f6e..29b2960 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -1072,6 +1072,55 @@ rte_eth_dev_check_mq_mode(uint8_t port_id,
> uint16_t nb_rx_q, uint16_t nb_tx_q,
> > }
> >
> > int
> > +rte_eth_speed_to_bm_flag(uint32_t speed, int duplex, uint32_t *flag)
> > +{
> > + switch (speed) {
> > + case ETH_SPEED_NUM_10M:
> > + *flag = (duplex) ? ETH_LINK_SPEED_10M :
> > +
> ETH_LINK_SPEED_10M_HD;
> > + break;
> > + case ETH_SPEED_NUM_100M:
> > + *flag = (duplex) ? ETH_LINK_SPEED_100M :
> > +
> ETH_LINK_SPEED_100M_HD;
> > + break;
> > + case ETH_SPEED_NUM_1G:
> > + *flag = ETH_LINK_SPEED_1G;
> > + break;
> > + case ETH_SPEED_NUM_2_5G:
> > + *flag = ETH_LINK_SPEED_2_5G;
> > + break;
> > + case ETH_SPEED_NUM_5G:
> > + *flag = ETH_LINK_SPEED_5G;
> > + break;
> > + case ETH_SPEED_NUM_10G:
> > + *flag = ETH_LINK_SPEED_10G;
> > + break;
> > + case ETH_SPEED_NUM_20G:
> > + *flag = ETH_LINK_SPEED_20G;
> > + break;
> > + case ETH_SPEED_NUM_25G:
> > + *flag = ETH_LINK_SPEED_25G;
> > + break;
> > + case ETH_SPEED_NUM_40G:
> > + *flag = ETH_LINK_SPEED_40G;
> > + break;
> > + case ETH_SPEED_NUM_50G:
> > + *flag = ETH_LINK_SPEED_50G;
> > + break;
> > + case ETH_SPEED_NUM_56G:
> > + *flag = ETH_LINK_SPEED_56G;
> > + break;
> > + case ETH_SPEED_NUM_100G:
> > + *flag = ETH_LINK_SPEED_100G;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> This nees to go in the appropriate version.map file for shared library
> building.
>
> > +int
> > rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t
> nb_tx_q,
> > const struct rte_eth_conf *dev_conf)
> > {
> > diff --git a/lib/librte_ether/rte_ethdev.h
> b/lib/librte_ether/rte_ethdev.h
> > index 951a423..bd333e4 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -238,26 +238,59 @@ struct rte_eth_stats {
> > };
> >
> > /**
> > + * Device supported speeds bitmap flags
> > + */
> > +#define ETH_LINK_SPEED_AUTONEG (0 << 0) /*<
> Autonegociate (all speeds) */
> > +#define ETH_LINK_SPEED_NO_AUTONEG (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 */
> > +#define ETH_LINK_SPEED_100M (1 << 4) /*< 100 Mbps full-duplex
> */
> > +#define ETH_LINK_SPEED_1G (1 << 5) /*< 1 Gbps */
> > +#define ETH_LINK_SPEED_2_5G (1 << 6) /*< 2.5 Gbps */
> > +#define ETH_LINK_SPEED_5G (1 << 7) /*< 5 Gbps */
> > +#define ETH_LINK_SPEED_10G (1 << 8) /*< 10 Mbps */
> > +#define ETH_LINK_SPEED_20G (1 << 9) /*< 20 Gbps */
> > +#define ETH_LINK_SPEED_25G (1 << 10) /*< 25 Gbps */
> > +#define ETH_LINK_SPEED_40G (1 << 11) /*< 40 Gbps */
> > +#define ETH_LINK_SPEED_50G (1 << 12) /*< 50 Gbps */
> > +#define ETH_LINK_SPEED_56G (1 << 13) /*< 56 Gbps */
> > +#define ETH_LINK_SPEED_100G (1 << 14) /*< 100 Gbps */
> > +
> > +/**
> > + * Ethernet numeric link speeds in Mbps
> > + */
> > +#define ETH_SPEED_NUM_NONE 0 /*< Not defined */
> > +#define ETH_SPEED_NUM_10M 10 /*< 10 Mbps */
> > +#define ETH_SPEED_NUM_100M 100 /*< 100 Mbps */
> > +#define ETH_SPEED_NUM_1G 1000 /*< 1 Gbps */
> > +#define ETH_SPEED_NUM_2_5G 2500 /*< 2.5 Gbps */
> > +#define ETH_SPEED_NUM_5G 5000 /*< 5 Gbps */
> > +#define ETH_SPEED_NUM_10G 10000 /*< 10 Mbps */
> > +#define ETH_SPEED_NUM_20G 20000 /*< 20 Gbps */
> > +#define ETH_SPEED_NUM_25G 25000 /*< 25 Gbps */
> > +#define ETH_SPEED_NUM_40G 40000 /*< 40 Gbps */
> > +#define ETH_SPEED_NUM_50G 50000 /*< 50 Gbps */
> > +#define ETH_SPEED_NUM_56G 56000 /*< 56 Gbps */
> > +#define ETH_SPEED_NUM_100G 100000 /*< 100 Gbps */
> > +
> > +/**
> > * A structure used to retrieve link-level information of an Ethernet
> port.
> > */
> > struct rte_eth_link {
> > - uint16_t link_speed; /**< ETH_LINK_SPEED_[10, 100, 1000,
> 10000] */
> > - uint16_t link_duplex; /**< ETH_LINK_[HALF_DUPLEX, FULL_DUPLEX]
> */
> > - uint8_t link_status : 1; /**< 1 -> link up, 0 -> link down */
> > -}__attribute__((aligned(8))); /**< aligned for atomic64 read/write
> */
> > -
> > -#define ETH_LINK_SPEED_AUTONEG 0 /**< Auto-negotiate link speed.
> */
> > -#define ETH_LINK_SPEED_10 10 /**< 10 megabits/second. */
> > -#define ETH_LINK_SPEED_100 100 /**< 100 megabits/second. */
> > -#define ETH_LINK_SPEED_1000 1000 /**< 1 gigabits/second. */
> > -#define ETH_LINK_SPEED_10000 10000 /**< 10 gigabits/second. */
> > -#define ETH_LINK_SPEED_10G 10000 /**< alias of 10
> gigabits/second. */
> > -#define ETH_LINK_SPEED_20G 20000 /**< 20 gigabits/second. */
> > -#define ETH_LINK_SPEED_40G 40000 /**< 40 gigabits/second. */
> > + uint32_t link_speed; /**< Link speed (ETH_SPEED_NUM_) */
> > + uint16_t link_duplex; /**< 1 -> full duplex, 0 -> half duplex
> */
> > + uint8_t link_autoneg : 1; /**< 1 -> link speed has been autoneg */
> > + uint8_t link_status : 1; /**< 1 -> link up, 0 -> link down */
> > +} __attribute__((aligned(8))); /**< aligned for atomic64
> read/write */
> >
> > -#define ETH_LINK_AUTONEG_DUPLEX 0 /**< Auto-negotiate duplex. */
> > -#define ETH_LINK_HALF_DUPLEX 1 /**< Half-duplex connection. */
> > -#define ETH_LINK_FULL_DUPLEX 2 /**< Full-duplex connection. */
> > +/* Utility constants */
> > +#define ETH_LINK_HALF_DUPLEX 0 /**< Half-duplex connection. */
> > +#define ETH_LINK_FULL_DUPLEX 1 /**< Full-duplex connection. */
> > +#define ETH_LINK_SPEED_FIXED 0 /**< Link speed was not
> autonegociated. */
> > +#define ETH_LINK_SPEED_NEG 1 /**< Link speed was
> autonegociated. */
> > +#define ETH_LINK_DOWN 0 /**< Link is down. */
> > +#define ETH_LINK_UP 1 /**< Link is up. */
> >
> > /**
> > * A structure used to configure the ring threshold registers of an
> RX/TX
> > @@ -747,10 +780,14 @@ struct rte_intr_conf {
> > * configuration settings may be needed.
> > */
> > struct rte_eth_conf {
> > - uint16_t link_speed;
> > - /**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
> > - uint16_t link_duplex;
> > - /**< ETH_LINK_[HALF_DUPLEX|FULL_DUPLEX], or 0 for autonegotation */
> > + uint32_t link_speeds; /**< bitmap of ETH_LINK_SPEED_XXX of speeds
> to be
> > + used. ETH_LINK_SPEED_NO_AUTONEG disables
> link
> > + autonegociation, and a unique speed shall
> be
> > + set. Otherwise, the bitmap defines the set
> of
> > + speeds to be advertised. If the special
> value
> > + ETH_LINK_SPEED_AUTONEG (0) is used, all
> speeds
> > + supported are advertised.
> > + */
> This structure is allocated by applications, and the link_speed/duplex
> field may
> be accessed by them. As such this is an ABI change and should go through
> the
> ABI process. Arguably, while its not really supposed to be done, given
> that
> there are so many changes comming to rte_eth_dev for 2.2, I could see an
> argument for sliding this in with those changes, so we didn't have to wait
> for
> an additional relelase cycle.
>
If I understand you correctly you are arguing that this patch should go
through the process of NEXT_ABI -> unique implementation. I keep saying I
do not understand what is the benefit of doing so when the ABI for 2.2 is
going to change anyway (even other patches will modify ethdev ABI).
Having a strict rule for ABI changes between minor releases (e.g. 2.1.0 and
2.1.1) is a must or if the next release is announced to be ABI compatible.
Other than that I don't see it, specially since users have to recompile
anyway their binaries due to (other) ABI changes.
But maybe I am missing something?
> The removal of the Speed/Duplex macros should also be noted.
>
Agree. I will add it into the doc
Marc
