[dpdk-dev] [PATCH v8 3/4] ethdev: redesign link speed config API
Matej,
On 16 February 2016 at 11:28, Matej Vido wrote:
> D?a 14.02.2016 o 23:17 Marc Sune nap?sal(a):
>
>> 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 uint64_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
>> ---
>> 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/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 ++
>> 29 files changed, 443 insertions(+), 345 deletions(-)
>>
>> [...]
>>
>
> Hi,
>
> some drivers (at least: e1000, i40e, ixgbe, mpipe, nfp, virtio, vmxnet3)
> use rte_atomic64_cmpset() to read and write link state like:
>
> static inline int
> rte_em_dev_atomic_read_link_status(struct rte_eth_dev *dev,
> struct rte_eth_link *link)
> {
> struct rte_eth_link *dst = link;
> struct rte_eth_link *src = &(dev->data->dev_link);
>
> if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
> *(uint64_t *)src) == 0)
> return -1;
>
> return 0;
> }
>
> static inline int
> rte_em_dev_atomic_write_link_status(struct rte_eth_dev *dev,
> struct rte_eth_link *link)
> {
> struct rte_eth_link *dst = &(dev->data->dev_link);
> struct rte_eth_link *src = link;
>
> if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
> *(uint64_t *)src) == 0)
> return -1;
>
> return 0;
> }
>
>
> When link_speed is changed to uint64_t, struct rte_eth_link exceeds 64
> bits. Shouldn't these functions be adapted in this patch series?
>
> Szedata2 PMD will also use rte_atomic64_cmpset() after patch [1].
> I can take care of this change in szedata2 PMD when patch [1] is accepted
> together with adjusting speeds in szedata2 PMD.
>
Indeed, thanks. I will take care of them in v9 (incl. szedata2).
marc
> [1] http://dpdk.org/ml/archives/dev/2016-January/032281.html
>
> Regards,
> Matej
>
[dpdk-dev] [PATCH v8 3/4] ethdev: redesign link speed config API
D?a 14.02.2016 o 23:17 Marc Sune nap?sal(a):
> 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 uint64_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
> ---
> 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/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 ++
> 29 files changed, 443 insertions(+), 345 deletions(-)
>
> [...]
Hi,
some drivers (at least: e1000, i40e, ixgbe, mpipe, nfp, virtio, vmxnet3)
use rte_atomic64_cmpset() to read and write link state like:
static inline int
rte_em_dev_atomic_read_link_status(struct rte_eth_dev *dev,
struct rte_eth_link *link)
{
struct rte_eth_link *dst = link;
struct rte_eth_link *src = &(dev->data->dev_link);
if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
*(uint64_t *)src) == 0)
return -1;
return 0;
}
static inline int
rte_em_dev_atomic_write_link_status(struct rte_eth_dev *dev,
struct rte_eth_link *link)
{
struct rte_eth_link *dst = &(dev->data->dev_link);
struct rte_eth_link *src = link;
if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
*(uint64_t *)src) == 0)
return -1;
return 0;
}
When link_speed is changed to uint64_t, struct rte_eth_link exceeds 64
bits. Shouldn't these functions be adapted in this patch series?
Szedata2 PMD will also use rte_atomic64_cmpset() after patch [1].
I can take care of this change in szedata2 PMD when patch [1] is
accepted together with adjusting speeds in szedata2 PMD.
[1] http://dpdk.org/ml/archives/dev/2016-January/032281.html
Regards,
Matej
[dpdk-dev] [PATCH v8 3/4] ethdev: redesign link speed config API
On 15 February 2016 at 09:46, N?lio Laranjeiro
wrote:
> On Sun, Feb 14, 2016 at 11:17:38PM +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.
> >
> >[...]
> > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> > index c5688a7..01c3a5c 100644
> > --- a/drivers/net/mlx4/mlx4.c
> > +++ b/drivers/net/mlx4/mlx4.c
> > @@ -4265,8 +4265,8 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *info)
> > if (priv_get_ifname(priv, ) == 0)
> > info->if_index = if_nametoindex(ifname);
> >
> > - info->speed_capa = ETH_SPEED_CAP_10G |ETH_SPEED_CAP_40G |
> > - ETH_SPEED_CAP_56G;
> > + info->speed_capa = ETH_LINK_SPEED_10G |ETH_LINK_SPEED_40G |
> > + ETH_LINK_SPEED_56G;
> >
> > priv_unlock(priv);
> > }
> > @@ -4636,6 +4636,8 @@ mlx4_link_update_unlocked(struct rte_eth_dev *dev,
> int wait_to_complete)
> > dev_link.link_speed = link_speed;
> > dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
> > ETH_LINK_HALF_DUPLEX :
> ETH_LINK_FULL_DUPLEX);
> > + dev_link.link_autoneg = ~(dev->data->dev_conf.link_speeds &
> > + ETH_LINK_SPEED_NO_AUTONEG);
> > if (memcmp(_link, >data->dev_link, sizeof(dev_link))) {
> > /* Link status changed. */
> > dev->data->dev_link = dev_link;
> >[...]
>
> The same modification are missing in mlx5.
>
>
Damn.. are the required closed-source libraries for MLX4/5 available
somewhere or is there a chance automatic builds can also compile drivers
that require external dependencies (like this MLX)?
It is very prone to errors having to guess what is going to happen without
being able to compile these drivers.
Marc
--
> N?lio Laranjeiro
> 6WIND
>
[dpdk-dev] [PATCH v8 3/4] ethdev: redesign link speed config API
Hi Marc,
You can download MLNX_OFED from Mellanox web site:
http://www.mellanox.com/page/products_dyn?product_family=26=linux_sw_drivers
Best Regards,
Olga
-Original Message-
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Marc
Sent: Monday, February 15, 2016 1:00 PM
To: N?lio Laranjeiro
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] [PATCH v8 3/4] ethdev: redesign link speed config API
On 15 February 2016 at 09:46, N?lio Laranjeiro
wrote:
> On Sun, Feb 14, 2016 at 11:17:38PM +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.
> >
> >[...]
> > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> >index c5688a7..01c3a5c 100644
> > --- a/drivers/net/mlx4/mlx4.c
> > +++ b/drivers/net/mlx4/mlx4.c
> > @@ -4265,8 +4265,8 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev,
> > struct
> rte_eth_dev_info *info)
> > if (priv_get_ifname(priv, ) == 0)
> > info->if_index = if_nametoindex(ifname);
> >
> > - info->speed_capa = ETH_SPEED_CAP_10G |ETH_SPEED_CAP_40G |
> > - ETH_SPEED_CAP_56G;
> > + info->speed_capa = ETH_LINK_SPEED_10G |ETH_LINK_SPEED_40G |
> > + ETH_LINK_SPEED_56G;
> >
> > priv_unlock(priv);
> > }
> > @@ -4636,6 +4636,8 @@ mlx4_link_update_unlocked(struct rte_eth_dev
> > *dev,
> int wait_to_complete)
> > dev_link.link_speed = link_speed;
> > dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
> > ETH_LINK_HALF_DUPLEX :
> ETH_LINK_FULL_DUPLEX);
> > + dev_link.link_autoneg = ~(dev->data->dev_conf.link_speeds &
> > +
> > + ETH_LINK_SPEED_NO_AUTONEG);
> > if (memcmp(_link, >data->dev_link, sizeof(dev_link))) {
> > /* Link status changed. */
> > dev->data->dev_link = dev_link; [...]
>
> The same modification are missing in mlx5.
>
>
Damn.. are the required closed-source libraries for MLX4/5 available somewhere
or is there a chance automatic builds can also compile drivers that require
external dependencies (like this MLX)?
It is very prone to errors having to guess what is going to happen without
being able to compile these drivers.
Marc
--
> N?lio Laranjeiro
> 6WIND
>
[dpdk-dev] [PATCH v8 3/4] ethdev: redesign link speed config API
On Sun, Feb 14, 2016 at 11:17:38PM +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.
>
>[...]
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index c5688a7..01c3a5c 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -4265,8 +4265,8 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *info)
> if (priv_get_ifname(priv, ) == 0)
> info->if_index = if_nametoindex(ifname);
>
> - info->speed_capa = ETH_SPEED_CAP_10G |ETH_SPEED_CAP_40G |
> - ETH_SPEED_CAP_56G;
> + info->speed_capa = ETH_LINK_SPEED_10G |ETH_LINK_SPEED_40G |
> + ETH_LINK_SPEED_56G;
>
> priv_unlock(priv);
> }
> @@ -4636,6 +4636,8 @@ mlx4_link_update_unlocked(struct rte_eth_dev *dev, int
> wait_to_complete)
> dev_link.link_speed = link_speed;
> dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
> ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> + dev_link.link_autoneg = ~(dev->data->dev_conf.link_speeds &
> + ETH_LINK_SPEED_NO_AUTONEG);
> if (memcmp(_link, >data->dev_link, sizeof(dev_link))) {
> /* Link status changed. */
> dev->data->dev_link = dev_link;
>[...]
The same modification are missing in mlx5.
--
N?lio Laranjeiro
6WIND
[dpdk-dev] [PATCH v8 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 uint64_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
---
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/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 ++
29 files changed, 443 insertions(+), 345 deletions(-)
diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
index db2196b..08fb041 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(, 0, sizeof(link));
rte_eth_link_get_nowait(port, );
- RTE_LOG(INFO, USER1, "Port %u (%u Gbps) %s\n",
+ RTE_LOG(INFO, USER1, "Port %u (%" PRIu64 " 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)
+{
+
+ 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;
+
