[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2016-02-12 Thread Marc
On 11 February 2016 at 16:27, N?lio Laranjeiro 
wrote:

> On Tue, Feb 02, 2016 at 11:30:59PM +0100, Marc wrote:
> > On 2 February 2016 at 03:20, Stephen Hemminger <
> stephen at networkplumber.org>
> > wrote:
> >
> > > On Thu, 28 Jan 2016 17:33:20 +
> > > Harish Patil  wrote:
> > >
> > > > * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
> > > >   values of all supported link speeds, in Mbps.
> > >
> > > I would prefer that there were no speed value macros.
> > > Linux used to have these, but people kept adding new hardware speeds
> > > and it soon gets out of date.
> > >
> >
> > I see what you mean, but I am not sure I agree. Link speeds are
> generally a
> > reduced amount of items (~20). Though it is true it can eventually grow,
> > but at small rate. Having numeric constants all over the source seems
> less
> > readable and less maintainable (e.g. less "grepable"/"sedable") to me.
> >
> >
> > >
> > > If you are going to redo it, then just increase speed to 64 bit, and
> allow
> > > any non-zero value.
> > >
> >
> > Value is now 32 bits, which I think is enough for future rates in mbps.
> > Since these constants were there, and before doing something to have to
> > revert it, can someone else give his/her opinion on this?
>
> For non 64bit architecture it is better to keep it on 32 bit but, if this
> field is only used on control plane we can afford 64 bit field and avoid
> another ABI breakage (in a far future).
>
> Even if this 32 bit field seems large enough you can already find on
> Internet some reports of transmission of petabit/s [1], we can imagine a
> NIC which provide this possibility by aggregating a lot of optical links
> and DPDK only will see it as single one.
>

OK, since it is not performance critical I will change it to 64 bit value.


>
> > If there is consensus, I've no problem on removing it for v8
>

Still the question about numeric speed MACROs is open.

best
marc




> >
> > Thanks
> > marc
>
> [1] http://optics.org/news/4/1/29
>
> --
> N?lio Laranjeiro
> 6WIND
>


[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2016-02-11 Thread Nélio Laranjeiro
On Tue, Feb 02, 2016 at 11:30:59PM +0100, Marc wrote:
> On 2 February 2016 at 03:20, Stephen Hemminger 
> wrote:
> 
> > On Thu, 28 Jan 2016 17:33:20 +
> > Harish Patil  wrote:
> >
> > > * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
> > >   values of all supported link speeds, in Mbps.
> >
> > I would prefer that there were no speed value macros.
> > Linux used to have these, but people kept adding new hardware speeds
> > and it soon gets out of date.
> >
> 
> I see what you mean, but I am not sure I agree. Link speeds are generally a
> reduced amount of items (~20). Though it is true it can eventually grow,
> but at small rate. Having numeric constants all over the source seems less
> readable and less maintainable (e.g. less "grepable"/"sedable") to me.
> 
> 
> >
> > If you are going to redo it, then just increase speed to 64 bit, and allow
> > any non-zero value.
> >
> 
> Value is now 32 bits, which I think is enough for future rates in mbps.
> Since these constants were there, and before doing something to have to
> revert it, can someone else give his/her opinion on this?

For non 64bit architecture it is better to keep it on 32 bit but, if this
field is only used on control plane we can afford 64 bit field and avoid
another ABI breakage (in a far future).

Even if this 32 bit field seems large enough you can already find on
Internet some reports of transmission of petabit/s [1], we can imagine a
NIC which provide this possibility by aggregating a lot of optical links
and DPDK only will see it as single one.

> If there is consensus, I've no problem on removing it for v8
> 
> Thanks
> marc

[1] http://optics.org/news/4/1/29

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2016-02-02 Thread Marc
On 2 February 2016 at 03:20, Stephen Hemminger 
wrote:

> On Thu, 28 Jan 2016 17:33:20 +
> Harish Patil  wrote:
>
> > * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
> >   values of all supported link speeds, in Mbps.
>
> I would prefer that there were no speed value macros.
> Linux used to have these, but people kept adding new hardware speeds
> and it soon gets out of date.
>

I see what you mean, but I am not sure I agree. Link speeds are generally a
reduced amount of items (~20). Though it is true it can eventually grow,
but at small rate. Having numeric constants all over the source seems less
readable and less maintainable (e.g. less "grepable"/"sedable") to me.


>
> If you are going to redo it, then just increase speed to 64 bit, and allow
> any non-zero value.
>

Value is now 32 bits, which I think is enough for future rates in mbps.
Since these constants were there, and before doing something to have to
revert it, can someone else give his/her opinion on this?

If there is consensus, I've no problem on removing it for v8

Thanks
marc


[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2016-02-02 Thread Stephen Hemminger
On Thu, 28 Jan 2016 17:33:20 +
Harish Patil  wrote:

> * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
>   values of all supported link speeds, in Mbps.

I would prefer that there were no speed value macros.
Linux used to have these, but people kept adding new hardware speeds
and it soon gets out of date.

If you are going to redo it, then just increase speed to 64 bit, and allow
any non-zero value.


[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2016-01-28 Thread Harish Patil


From: Marc Sune mailto:marcde...@gmail.com>>
Date: Sunday, October 4, 2015 at 2:12 PM
To: "dev at dpdk.org<mailto:dev at dpdk.org>" mailto:dev at 
dpdk.org>>
Subject: [dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

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 mailto:marcdevel at gmail.com>>
---
 app/test-pmd/cmdline.c | 124 +++--
 app/test/virtual_pmd.c |   4 +-
 drivers/net/af_packet/rte_eth_af_packet.c  |   5 +-
 drivers/net/bonding/rte_eth_bond_8023ad.c  |  14 ++--
 drivers/net/cxgbe/base/t4_hw.c |   8 +-
 drivers/net/e1000/base/e1000_80003es2lan.c |   6 +-
 drivers/net/e1000/base/e1000_82541.c   |   8 +-
 drivers/net/e1000/base/e1000_82543.c   |   4 +-
 drivers/net/e1000/base/e1000_82575.c   |  11 +--
 drivers/net/e1000/base/e1000_api.c |   2 +-
 drivers/net/e1000/base/e1000_api.h |   2 +-
 drivers/net/e1000/base/e1000_defines.h |   4 +-
 drivers/net/e1000/base/e1000_hw.h  |   2 +-
 drivers/net/e1000/base/e1000_ich8lan.c |   4 +-
 drivers/net/e1000/base/e1000_mac.c |   9 ++-
 drivers/net/e1000/base/e1000_mac.h |   6 +-
 drivers/net/e1000/base/e1000_vf.c  |   4 +-
 drivers/net/e1000/base/e1000_vf.h  |   2 +-
 drivers/net/e1000/em_ethdev.c  | 108 -
 drivers/net/e1000/igb_ethdev.c | 103 
 drivers/net/fm10k/fm10k_ethdev.c   |   8 +-
 drivers/net/i40e/i40e_ethdev.c |  70 
 drivers/net/i40e/i40e_ethdev_vf.c  |  11 +--
 drivers/net/ixgbe/ixgbe_ethdev.c   |  72 -
 drivers/net/mlx4/mlx4.c|   2 +
 drivers/net/mpipe/mpipe_tilegx.c   |   6 +-
 drivers/net/null/rte_eth_null.c|   5 +-
 drivers/net/pcap/rte_eth_pcap.c|   9 ++-
 drivers/net/ring/rte_eth_ring.c|   5 +-
 drivers/net/vmxnet3/vmxnet3_ethdev.c   |   5 +-
 drivers/net/xenvirt/rte_eth_xenvirt.c  |   5 +-
 examples/ip_pipeline/config_parse.c|   3 +-
 lib/librte_ether/rte_ethdev.c  |  49 
 lib/librte_ether/rte_ethdev.h  | 113 --
 34 files changed, 437 insertions(+), 356 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0f8f48f..c62f5be 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -897,14 +897,65 @@ struct cmd_config_speed_all {
 cmdline_fixed_string_t value2;
 };

+static int
+parse_and_check_speed_duplex(char *value1, char *value2, uint32_t *link_speed)
+{
+
+int duplex;
+
+if (!strcmp(value2, "half")) {
+duplex = 0;
+} else if (!strcmp(value2, "full")) {
+duplex = 1;
+} else if (!strcmp(value2, "auto")) {
+duplex = 1;
+} else {
+printf("Unknown parameter\n");
+return -1;
+}
+
+if (!strcmp(value1, "10")) {
+*link_speed = (duplex) ? ETH_LINK_SPEED_10M :
+ETH_LINK_SPEED_10M_HD;
+} else if (!strcmp(value1, "100")) {
+*link_speed = (duplex) ? ETH_LINK_SPEED_100M :
+ETH_LINK_SPEED_100M_HD;
+} else if (!strcmp(value1, "1000")) {
+if (!duplex)
+goto invalid_speed_param;
+*link_speed = ETH_LINK_SPEED_1G;
+} else if (!strcmp(value1, "1")) {
+if (!duplex)
+goto invalid_speed_param;
+*link_speed = ETH_LINK_SPEED_10G;
+} else if (!strcmp(value1, "4")) {
+if (!duplex)
+goto invalid_speed_param;
+*link_speed = ETH_LINK_SPEED_40G;
+} else if (!strcmp(value1, "auto")) {
+if (!duplex)
+goto invalid_speed_param;
+*link_speed = ETH_LINK_SPEED_AUTONEG;
+} else {
+printf("Unknown parameter\n");
+return -1;
+}
+
+return 0;
+
+invalid_speed_param:
+
+printf("Invalid speed parameter\n");
+return -1;
+}
+
 static void
 cmd_config_speed_all_parsed(void *parsed_result,
 __attribute__((unused)) struct cmdline *cl,
  

[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2015-10-07 Thread Marc Sune
2015-10-06 15:48 GMT+02:00 N?lio Laranjeiro :

> Hi Marc,
>
> On Sun, Oct 04, 2015 at 11:12:46PM +0200, Marc Sune wrote:
> >[...]
> >  /**
> > + * 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_10M10 /*< 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_10G1  /*< 10 Mbps */
> > +#define ETH_SPEED_NUM_20G2  /*< 20 Gbps */
> > +#define ETH_SPEED_NUM_25G25000  /*< 25 Gbps */
> > +#define ETH_SPEED_NUM_40G4  /*< 40 Gbps */
> > +#define ETH_SPEED_NUM_50G5  /*< 50 Gbps */
> > +#define ETH_SPEED_NUM_56G56000  /*< 56 Gbps */
> > +#define ETH_SPEED_NUM_100G   10 /*< 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,
> 1] */
> > - 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_11   /**< 10 gigabits/second. */
> > -#define ETH_LINK_SPEED_10G  1   /**< alias of 10
> gigabits/second. */
> > -#define ETH_LINK_SPEED_20G  2   /**< 20 gigabits/second. */
> > -#define ETH_LINK_SPEED_40G  4   /**< 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 */
> >[...]
>
> Pretty good.  One question, why did you not merge link_duplex, autoneg,
> and status like:
>
> struct rte_eth_link {
> uint32_t link_speed;
> uint32_t link_duplex:1;
> uint32_t link_autoneg:1;
> uint32_t link_status:1;
> };
>
> is it really useful to keep a uint16_t for the duplex alone?
>

You are right, I've missed this one. Will be fixed in v6.


>
> Another point, the comment about link_duplex field should point to the
> defines you have changed i.e. ETH_LINK_HALF_DUPLEX, ETH_LINK_FULL_DUPLEX.
>

Right. Will do that as part of v6 + comments of Neil.

Marc


> Regards,
>
> --
> N?lio Laranjeiro
> 6WIND
>


[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2015-10-07 Thread Marc Sune
2015-10-05 12:59 GMT+02:00 Neil Horman :

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

[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2015-10-06 Thread Nélio Laranjeiro
Hi Marc,

On Sun, Oct 04, 2015 at 11:12:46PM +0200, Marc Sune wrote:
>[...]
>  /**
> + * 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_10M10 /*< 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_10G1  /*< 10 Mbps */
> +#define ETH_SPEED_NUM_20G2  /*< 20 Gbps */
> +#define ETH_SPEED_NUM_25G25000  /*< 25 Gbps */
> +#define ETH_SPEED_NUM_40G4  /*< 40 Gbps */
> +#define ETH_SPEED_NUM_50G5  /*< 50 Gbps */
> +#define ETH_SPEED_NUM_56G56000  /*< 56 Gbps */
> +#define ETH_SPEED_NUM_100G   10 /*< 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, 1] */
> - 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_11   /**< 10 gigabits/second. */
> -#define ETH_LINK_SPEED_10G  1   /**< alias of 10 gigabits/second. */
> -#define ETH_LINK_SPEED_20G  2   /**< 20 gigabits/second. */
> -#define ETH_LINK_SPEED_40G  4   /**< 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 */
>[...]

Pretty good.  One question, why did you not merge link_duplex, autoneg,
and status like:

struct rte_eth_link {
uint32_t link_speed;
uint32_t link_duplex:1;
uint32_t link_autoneg:1;
uint32_t link_status:1;
};

is it really useful to keep a uint16_t for the duplex alone?

Another point, the comment about link_duplex field should point to the
defines you have changed i.e. ETH_LINK_HALF_DUPLEX, ETH_LINK_FULL_DUPLEX.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2015-10-05 Thread Neil Horman
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 
>  
> 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_10M10 /*< 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_10G1  /*< 10 Mbps */
> +#define ETH_SPEED_NUM_20G2  /*< 20 Gbps */
> +#define ETH_SPEED_NUM_25G25000  /*< 25 Gbps */
> +#def

[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2015-10-04 Thread Marc Sune
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 
---
 app/test-pmd/cmdline.c | 124 +++--
 app/test/virtual_pmd.c |   4 +-
 drivers/net/af_packet/rte_eth_af_packet.c  |   5 +-
 drivers/net/bonding/rte_eth_bond_8023ad.c  |  14 ++--
 drivers/net/cxgbe/base/t4_hw.c |   8 +-
 drivers/net/e1000/base/e1000_80003es2lan.c |   6 +-
 drivers/net/e1000/base/e1000_82541.c   |   8 +-
 drivers/net/e1000/base/e1000_82543.c   |   4 +-
 drivers/net/e1000/base/e1000_82575.c   |  11 +--
 drivers/net/e1000/base/e1000_api.c |   2 +-
 drivers/net/e1000/base/e1000_api.h |   2 +-
 drivers/net/e1000/base/e1000_defines.h |   4 +-
 drivers/net/e1000/base/e1000_hw.h  |   2 +-
 drivers/net/e1000/base/e1000_ich8lan.c |   4 +-
 drivers/net/e1000/base/e1000_mac.c |   9 ++-
 drivers/net/e1000/base/e1000_mac.h |   6 +-
 drivers/net/e1000/base/e1000_vf.c  |   4 +-
 drivers/net/e1000/base/e1000_vf.h  |   2 +-
 drivers/net/e1000/em_ethdev.c  | 108 -
 drivers/net/e1000/igb_ethdev.c | 103 
 drivers/net/fm10k/fm10k_ethdev.c   |   8 +-
 drivers/net/i40e/i40e_ethdev.c |  70 
 drivers/net/i40e/i40e_ethdev_vf.c  |  11 +--
 drivers/net/ixgbe/ixgbe_ethdev.c   |  72 -
 drivers/net/mlx4/mlx4.c|   2 +
 drivers/net/mpipe/mpipe_tilegx.c   |   6 +-
 drivers/net/null/rte_eth_null.c|   5 +-
 drivers/net/pcap/rte_eth_pcap.c|   9 ++-
 drivers/net/ring/rte_eth_ring.c|   5 +-
 drivers/net/vmxnet3/vmxnet3_ethdev.c   |   5 +-
 drivers/net/xenvirt/rte_eth_xenvirt.c  |   5 +-
 examples/ip_pipeline/config_parse.c|   3 +-
 lib/librte_ether/rte_ethdev.c  |  49 
 lib/librte_ether/rte_ethdev.h  | 113 --
 34 files changed, 437 insertions(+), 356 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0f8f48f..c62f5be 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -897,14 +897,65 @@ struct cmd_config_speed_all {
cmdline_fixed_string_t value2;
 };

+static int
+parse_and_check_speed_duplex(char *value1, char *value2, uint32_t *link_speed)
+{
+
+   int duplex;
+
+   if (!strcmp(value2, "half")) {
+   duplex = 0;
+   } else if (!strcmp(value2, "full")) {
+   duplex = 1;
+   } else if (!strcmp(value2, "auto")) {
+   duplex = 1;
+   } else {
+   printf("Unknown parameter\n");
+   return -1;
+   }
+
+   if (!strcmp(value1, "10")) {
+   *link_speed = (duplex) ? ETH_LINK_SPEED_10M :
+   ETH_LINK_SPEED_10M_HD;
+   } else if (!strcmp(value1, "100")) {
+   *link_speed = (duplex) ? ETH_LINK_SPEED_100M :
+   ETH_LINK_SPEED_100M_HD;
+   } else if (!strcmp(value1, "1000")) {
+   if (!duplex)
+   goto invalid_speed_param;
+   *link_speed = ETH_LINK_SPEED_1G;
+   } else if (!strcmp(value1, "1")) {
+   if (!duplex)
+   goto invalid_speed_param;
+   *link_speed = ETH_LINK_SPEED_10G;
+   } else if (!strcmp(value1, "4")) {
+   if (!duplex)
+   goto invalid_speed_param;
+   *link_speed = ETH_LINK_SPEED_40G;
+   } else if (!strcmp(value1, "auto")) {
+   if (!duplex)
+   goto invalid_speed_param;
+   *link_speed = ETH_LINK_SPEED_AUTONEG;
+   } else {
+   printf("Unknown parameter\n");
+   return -1;
+   }
+
+   return 0;
+
+invalid_speed_param:
+
+   printf("Invalid speed parameter\n");
+   return -1;
+}
+
 static void
 cmd_config_speed_all_parsed(void *parsed_result,
__attribute__((unused)) struct cmdline *cl,
__attribute__((unused)) void *data)
 {
struct cmd_config_speed_all *res = parsed_result;
-   uint16_t lin