diff mbox

[dpdk-dev,2/6] net/tap: add speed capabilities

Message ID 0e99fe0868cca2bfbc03a7f99ca3dbfb9a9d42ea.1488534161.git.pascal.mazon@6wind.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Pascal Mazon March 3, 2017, 9:46 a.m. UTC 
  Tap PMD is flexible, it supports any speed.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

Comments

Wiles, Keith March 3, 2017, 3:27 p.m. UTC | #1 
> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> Tap PMD is flexible, it supports any speed.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
> doc/guides/nics/features/tap.ini |  1 +
> drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index d9b47a003654..dad5a0561087 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -9,6 +9,7 @@ Jumbo frame          = Y
> Promiscuous mode     = Y
> Allmulticast mode    = Y
> Basic stats          = Y
> +Speed capabilities   = Y
> Unicast MAC filter   = Y
> Other kdrv           = Y
> ARMv7                = Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 994c8be701c8..6670dfbb35ce 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
> 	return 0;
> }
> 
> +static uint32_t
> +tap_dev_speed_capa(void)
> +{
> +	uint32_t speed = pmd_link.link_speed;
> +	uint32_t capa = 0;
> +
> +	if (speed >= ETH_SPEED_NUM_10M)
> +		capa |= ETH_LINK_SPEED_10M;
> +	if (speed >= ETH_SPEED_NUM_100M)
> +		capa |= ETH_LINK_SPEED_100M;
> +	if (speed >= ETH_SPEED_NUM_1G)
> +		capa |= ETH_LINK_SPEED_1G;
> +	if (speed >= ETH_SPEED_NUM_5G)
> +		capa |= ETH_LINK_SPEED_2_5G;
> +	if (speed >= ETH_SPEED_NUM_5G)
> +		capa |= ETH_LINK_SPEED_5G;
> +	if (speed >= ETH_SPEED_NUM_10G)
> +		capa |= ETH_LINK_SPEED_10G;
> +	if (speed >= ETH_SPEED_NUM_20G)
> +		capa |= ETH_LINK_SPEED_20G;
> +	if (speed >= ETH_SPEED_NUM_25G)
> +		capa |= ETH_LINK_SPEED_25G;
> +	if (speed >= ETH_SPEED_NUM_40G)
> +		capa |= ETH_LINK_SPEED_40G;
> +	if (speed >= ETH_SPEED_NUM_50G)
> +		capa |= ETH_LINK_SPEED_50G;
> +	if (speed >= ETH_SPEED_NUM_56G)
> +		capa |= ETH_LINK_SPEED_56G;
> +	if (speed >= ETH_SPEED_NUM_100G)
> +		capa |= ETH_LINK_SPEED_100G;

In the real world the NIC may only support 50G an not say 10M, so in that case this code would be wrong as it would set all of the speeds up to 50G. I do not think the code should be changed, but I add a comment to tell the developer the issue here. I do not want someone copying the code and thinking is i correct for a real device.

> +
> +	return capa;
> +}
> +
> static void
> tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> {
> @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> 	dev_info->max_tx_queues = internals->nb_queues;
> 	dev_info->min_rx_bufsize = 0;
> 	dev_info->pci_dev = NULL;
> +	dev_info->speed_capa = tap_dev_speed_capa();
> }
> 
> static void
> -- 
> 2.8.0.rc0
> 

Regards,
Keith
Pascal Mazon March 6, 2017, 1:58 p.m. UTC | #2 
On Fri, 3 Mar 2017 15:27:12 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> 
> > On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
> > wrote:
> > 
> > Tap PMD is flexible, it supports any speed.
> > 
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > ---
> > doc/guides/nics/features/tap.ini |  1 +
> > drivers/net/tap/rte_eth_tap.c    | 35
> > +++++++++++++++++++++++++++++++++++ 2 files changed, 36
> > insertions(+)
> > 
> > diff --git a/doc/guides/nics/features/tap.ini
> > b/doc/guides/nics/features/tap.ini index d9b47a003654..dad5a0561087
> > 100644 --- a/doc/guides/nics/features/tap.ini
> > +++ b/doc/guides/nics/features/tap.ini
> > @@ -9,6 +9,7 @@ Jumbo frame          = Y
> > Promiscuous mode     = Y
> > Allmulticast mode    = Y
> > Basic stats          = Y
> > +Speed capabilities   = Y
> > Unicast MAC filter   = Y
> > Other kdrv           = Y
> > ARMv7                = Y
> > diff --git a/drivers/net/tap/rte_eth_tap.c
> > b/drivers/net/tap/rte_eth_tap.c index 994c8be701c8..6670dfbb35ce
> > 100644 --- a/drivers/net/tap/rte_eth_tap.c
> > +++ b/drivers/net/tap/rte_eth_tap.c
> > @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev
> > __rte_unused) return 0;
> > }
> > 
> > +static uint32_t
> > +tap_dev_speed_capa(void)
> > +{
> > +	uint32_t speed = pmd_link.link_speed;
> > +	uint32_t capa = 0;
> > +
> > +	if (speed >= ETH_SPEED_NUM_10M)
> > +		capa |= ETH_LINK_SPEED_10M;
> > +	if (speed >= ETH_SPEED_NUM_100M)
> > +		capa |= ETH_LINK_SPEED_100M;
> > +	if (speed >= ETH_SPEED_NUM_1G)
> > +		capa |= ETH_LINK_SPEED_1G;
> > +	if (speed >= ETH_SPEED_NUM_5G)
> > +		capa |= ETH_LINK_SPEED_2_5G;
> > +	if (speed >= ETH_SPEED_NUM_5G)
> > +		capa |= ETH_LINK_SPEED_5G;
> > +	if (speed >= ETH_SPEED_NUM_10G)
> > +		capa |= ETH_LINK_SPEED_10G;
> > +	if (speed >= ETH_SPEED_NUM_20G)
> > +		capa |= ETH_LINK_SPEED_20G;
> > +	if (speed >= ETH_SPEED_NUM_25G)
> > +		capa |= ETH_LINK_SPEED_25G;
> > +	if (speed >= ETH_SPEED_NUM_40G)
> > +		capa |= ETH_LINK_SPEED_40G;
> > +	if (speed >= ETH_SPEED_NUM_50G)
> > +		capa |= ETH_LINK_SPEED_50G;
> > +	if (speed >= ETH_SPEED_NUM_56G)
> > +		capa |= ETH_LINK_SPEED_56G;
> > +	if (speed >= ETH_SPEED_NUM_100G)
> > +		capa |= ETH_LINK_SPEED_100G;
> 
> In the real world the NIC may only support 50G an not say 10M, so in
> that case this code would be wrong as it would set all of the speeds
> up to 50G. I do not think the code should be changed, but I add a
> comment to tell the developer the issue here. I do not want someone
> copying the code and thinking is i correct for a real device.
> 

That's true for actual hardware. But tap is completely virtual, so
actually it could support any speed (it is limited by userland-kernel
communication speed).

What speed would you rather have the tap PMD report as capable of?

Best regards,
Pascal

> > +
> > +	return capa;
> > +}
> > +
> > static void
> > tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info
> > *dev_info) {
> > @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct
> > rte_eth_dev_info *dev_info) dev_info->max_tx_queues =
> > internals->nb_queues; dev_info->min_rx_bufsize = 0;
> > 	dev_info->pci_dev = NULL;
> > +	dev_info->speed_capa = tap_dev_speed_capa();
> > }
> > 
> > static void
> > -- 
> > 2.8.0.rc0
> > 
> 
> Regards,
> Keith
>
Wiles, Keith March 6, 2017, 2:38 p.m. UTC | #3 
> On Mar 6, 2017, at 7:58 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> On Fri, 3 Mar 2017 15:27:12 +0000
> "Wiles, Keith" <keith.wiles@intel.com> wrote:
> 
>> 
>>> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
>>> wrote:
>>> 
>>> Tap PMD is flexible, it supports any speed.
>>> 
>>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
>>> ---
>>> doc/guides/nics/features/tap.ini |  1 +
>>> drivers/net/tap/rte_eth_tap.c    | 35
>>> +++++++++++++++++++++++++++++++++++ 2 files changed, 36
>>> insertions(+)
>>> 
>>> diff --git a/doc/guides/nics/features/tap.ini
>>> b/doc/guides/nics/features/tap.ini index d9b47a003654..dad5a0561087
>>> 100644 --- a/doc/guides/nics/features/tap.ini
>>> +++ b/doc/guides/nics/features/tap.ini
>>> @@ -9,6 +9,7 @@ Jumbo frame          = Y
>>> Promiscuous mode     = Y
>>> Allmulticast mode    = Y
>>> Basic stats          = Y
>>> +Speed capabilities   = Y
>>> Unicast MAC filter   = Y
>>> Other kdrv           = Y
>>> ARMv7                = Y
>>> diff --git a/drivers/net/tap/rte_eth_tap.c
>>> b/drivers/net/tap/rte_eth_tap.c index 994c8be701c8..6670dfbb35ce
>>> 100644 --- a/drivers/net/tap/rte_eth_tap.c
>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev
>>> __rte_unused) return 0;
>>> }
>>> 
>>> +static uint32_t
>>> +tap_dev_speed_capa(void)
>>> +{
>>> +	uint32_t speed = pmd_link.link_speed;
>>> +	uint32_t capa = 0;
>>> +
>>> +	if (speed >= ETH_SPEED_NUM_10M)
>>> +		capa |= ETH_LINK_SPEED_10M;
>>> +	if (speed >= ETH_SPEED_NUM_100M)
>>> +		capa |= ETH_LINK_SPEED_100M;
>>> +	if (speed >= ETH_SPEED_NUM_1G)
>>> +		capa |= ETH_LINK_SPEED_1G;
>>> +	if (speed >= ETH_SPEED_NUM_5G)
>>> +		capa |= ETH_LINK_SPEED_2_5G;
>>> +	if (speed >= ETH_SPEED_NUM_5G)
>>> +		capa |= ETH_LINK_SPEED_5G;
>>> +	if (speed >= ETH_SPEED_NUM_10G)
>>> +		capa |= ETH_LINK_SPEED_10G;
>>> +	if (speed >= ETH_SPEED_NUM_20G)
>>> +		capa |= ETH_LINK_SPEED_20G;
>>> +	if (speed >= ETH_SPEED_NUM_25G)
>>> +		capa |= ETH_LINK_SPEED_25G;
>>> +	if (speed >= ETH_SPEED_NUM_40G)
>>> +		capa |= ETH_LINK_SPEED_40G;
>>> +	if (speed >= ETH_SPEED_NUM_50G)
>>> +		capa |= ETH_LINK_SPEED_50G;
>>> +	if (speed >= ETH_SPEED_NUM_56G)
>>> +		capa |= ETH_LINK_SPEED_56G;
>>> +	if (speed >= ETH_SPEED_NUM_100G)
>>> +		capa |= ETH_LINK_SPEED_100G;
>> 
>> In the real world the NIC may only support 50G an not say 10M, so in
>> that case this code would be wrong as it would set all of the speeds
>> up to 50G. I do not think the code should be changed, but I add a
>> comment to tell the developer the issue here. I do not want someone
>> copying the code and thinking is i correct for a real device.
>> 
> 
> That's true for actual hardware. But tap is completely virtual, so
> actually it could support any speed (it is limited by userland-kernel
> communication speed).
> 
> What speed would you rather have the tap PMD report as capable of?

This note was a very minor picky point and you can ignore my comment here. It is not worth the time to deal with and you have better things to do :-)

> 
> Best regards,
> Pascal
> 
>>> +
>>> +	return capa;
>>> +}
>>> +
>>> static void
>>> tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info
>>> *dev_info) {
>>> @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct
>>> rte_eth_dev_info *dev_info) dev_info->max_tx_queues =
>>> internals->nb_queues; dev_info->min_rx_bufsize = 0;
>>> 	dev_info->pci_dev = NULL;
>>> +	dev_info->speed_capa = tap_dev_speed_capa();
>>> }
>>> 
>>> static void
>>> -- 
>>> 2.8.0.rc0
>>> 
>> 
>> Regards,
>> Keith
>> 
> 

Regards,
Keith
diff mbox

Patch

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index d9b47a003654..dad5a0561087 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@  Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 994c8be701c8..6670dfbb35ce 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -351,6 +351,40 @@  tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	return 0;
 }
 
+static uint32_t
+tap_dev_speed_capa(void)
+{
+	uint32_t speed = pmd_link.link_speed;
+	uint32_t capa = 0;
+
+	if (speed >= ETH_SPEED_NUM_10M)
+		capa |= ETH_LINK_SPEED_10M;
+	if (speed >= ETH_SPEED_NUM_100M)
+		capa |= ETH_LINK_SPEED_100M;
+	if (speed >= ETH_SPEED_NUM_1G)
+		capa |= ETH_LINK_SPEED_1G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_2_5G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_5G;
+	if (speed >= ETH_SPEED_NUM_10G)
+		capa |= ETH_LINK_SPEED_10G;
+	if (speed >= ETH_SPEED_NUM_20G)
+		capa |= ETH_LINK_SPEED_20G;
+	if (speed >= ETH_SPEED_NUM_25G)
+		capa |= ETH_LINK_SPEED_25G;
+	if (speed >= ETH_SPEED_NUM_40G)
+		capa |= ETH_LINK_SPEED_40G;
+	if (speed >= ETH_SPEED_NUM_50G)
+		capa |= ETH_LINK_SPEED_50G;
+	if (speed >= ETH_SPEED_NUM_56G)
+		capa |= ETH_LINK_SPEED_56G;
+	if (speed >= ETH_SPEED_NUM_100G)
+		capa |= ETH_LINK_SPEED_100G;
+
+	return capa;
+}
+
 static void
 tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
@@ -363,6 +397,7 @@  tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
+	dev_info->speed_capa = tap_dev_speed_capa();
 }
 
 static void