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

[Nélio Laranjeiro]

Hi Marc,

A small remark bellow.

On Tue, Mar 01, 2016 at 01:45:50AM +0100, 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
>   and beyond (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.
> * Added rte_eth_speed_to_bm_flag() to version map.
> 
> Signed-off-by: Marc Sune <marcdevel at gmail.com>
> ---
>  app/test-pipeline/init.c                  |   2 +-
>  app/test-pmd/cmdline.c                    | 124 +++++++++++++++---------------
>  app/test-pmd/config.c                     |   4 +-
>  app/test/virtual_pmd.c                    |   4 +-
>  drivers/net/af_packet/rte_eth_af_packet.c |   5 +-
>  drivers/net/bnx2x/bnx2x_ethdev.c          |   8 +-
>  drivers/net/bonding/rte_eth_bond_8023ad.c |  14 ++--
>  drivers/net/cxgbe/base/t4_hw.c            |   8 +-
>  drivers/net/cxgbe/cxgbe_ethdev.c          |   2 +-
>  drivers/net/e1000/em_ethdev.c             | 116 ++++++++++++++--------------
>  drivers/net/e1000/igb_ethdev.c            | 111 +++++++++++++-------------
>  drivers/net/fm10k/fm10k_ethdev.c          |   8 +-
>  drivers/net/i40e/i40e_ethdev.c            |  73 +++++++++---------
>  drivers/net/i40e/i40e_ethdev_vf.c         |  11 +--
>  drivers/net/ixgbe/ixgbe_ethdev.c          |  78 ++++++++-----------
>  drivers/net/mlx4/mlx4.c                   |   6 +-
>  drivers/net/mlx5/mlx5_ethdev.c            |  10 +--
>  drivers/net/mpipe/mpipe_tilegx.c          |   6 +-
>  drivers/net/nfp/nfp_net.c                 |   4 +-
>  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/virtio/virtio_ethdev.c        |   2 +-
>  drivers/net/virtio/virtio_ethdev.h        |   2 -
>  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 +++++++++++++++++----------
>  lib/librte_ether/rte_ether_version.map    |   6 ++
>  30 files changed, 448 insertions(+), 350 deletions(-)
> 
> diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
> index db2196b..6a69fe2 100644
> --- a/app/test-pipeline/init.c
> +++ b/app/test-pipeline/init.c
> @@ -200,7 +200,7 @@ app_ports_check_link(void)
>  		port = (uint8_t) app.ports[i];
>  		memset(&link, 0, sizeof(link));
>  		rte_eth_link_get_nowait(port, &link);
> -		RTE_LOG(INFO, USER1, "Port %u (%u Gbps) %s\n",
> +		RTE_LOG(INFO, USER1, "Port %u (%d Gbps) %s\n",
>  			port,
>  			link.link_speed / 1000,
>  			link.link_status ? "UP" : "DOWN");
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 52e9f5f..57ad25f 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -956,14 +956,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)

"value" variables should have the more friendly name, we need to read the
code to understand what is value1 and value2.

>[...] 

-- 
Nélio Laranjeiro
6WIND