diff mbox

[dpdk-dev,5/6] net/tap: add packet type management

Message ID 5044b7ef7b9e5207fb2202bd5715524bd0d49379.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 fail Compilation issues

Commit Message

Pascal Mazon March 3, 2017, 9:46 a.m. UTC 
  Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet type.

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

Comments

Wiles, Keith March 3, 2017, 3:31 p.m. UTC | #1 
> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet type.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
> doc/guides/nics/features/tap.ini |  1 +
> drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
> 2 files changed, 16 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index 6aa11874e2bc..7f3f4d661dd7 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -13,6 +13,7 @@ MTU update           = Y
> Multicast MAC filter = Y
> Speed capabilities   = Y
> Unicast MAC filter   = Y
> +Packet type parsing  = Y
> Other kdrv           = Y
> ARMv7                = Y
> ARMv8                = Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 64b84cd76321..e4af36a6d142 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -36,6 +36,7 @@
> #include <rte_malloc.h>
> #include <rte_vdev.h>
> #include <rte_kvargs.h>
> +#include <rte_net.h>
> 
> #include <sys/types.h>
> #include <sys/stat.h>
> @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> 		mbuf->data_len = len;
> 		mbuf->pkt_len = len;
> 		mbuf->port = rxq->in_port;
> +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> +						      RTE_PTYPE_ALL_MASK);
> 
> 		/* account for the receive frame */
> 		bufs[num_rx++] = mbuf;
> @@ -760,6 +763,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> 	return 0;
> }
> 
> +static const uint32_t*
> +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
> +{
> +	static const uint32_t ptypes[] = {
> +		RTE_PTYPE_UNKNOWN,
> +
> +	};
> +
> +	return ptypes;
> +}

Can we just add the code to grab the ptype value instead of just saying not supported.

The original code would just return an error from ethdev correct, what was wrong with that one. I would like to see the tap PMD just return the ptype would that not be more useful?

> +
> static const struct eth_dev_ops ops = {
> 	.dev_start              = tap_dev_start,
> 	.dev_stop               = tap_dev_stop,
> @@ -784,6 +798,7 @@ static const struct eth_dev_ops ops = {
> 	.mtu_set                = tap_mtu_set,
> 	.stats_get              = tap_stats_get,
> 	.stats_reset            = tap_stats_reset,
> +	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
> };
> 
> static int
> -- 
> 2.8.0.rc0
> 

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

> 
> > On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
> > wrote:
> > 
> > Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet
> > type.
> > 
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > ---
> > doc/guides/nics/features/tap.ini |  1 +
> > drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
> > 2 files changed, 16 insertions(+)
> > 
> > diff --git a/doc/guides/nics/features/tap.ini
> > b/doc/guides/nics/features/tap.ini index 6aa11874e2bc..7f3f4d661dd7
> > 100644 --- a/doc/guides/nics/features/tap.ini
> > +++ b/doc/guides/nics/features/tap.ini
> > @@ -13,6 +13,7 @@ MTU update           = Y
> > Multicast MAC filter = Y
> > Speed capabilities   = Y
> > Unicast MAC filter   = Y
> > +Packet type parsing  = Y
> > Other kdrv           = Y
> > ARMv7                = Y
> > ARMv8                = Y
> > diff --git a/drivers/net/tap/rte_eth_tap.c
> > b/drivers/net/tap/rte_eth_tap.c index 64b84cd76321..e4af36a6d142
> > 100644 --- a/drivers/net/tap/rte_eth_tap.c
> > +++ b/drivers/net/tap/rte_eth_tap.c
> > @@ -36,6 +36,7 @@
> > #include <rte_malloc.h>
> > #include <rte_vdev.h>
> > #include <rte_kvargs.h>
> > +#include <rte_net.h>
> > 
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf
> > **bufs, uint16_t nb_pkts) mbuf->data_len = len;
> > 		mbuf->pkt_len = len;
> > 		mbuf->port = rxq->in_port;
> > +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> > +
> > RTE_PTYPE_ALL_MASK);
> > 
> > 		/* account for the receive frame */
> > 		bufs[num_rx++] = mbuf;
> > @@ -760,6 +763,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t
> > mtu) return 0;
> > }
> > 
> > +static const uint32_t*
> > +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
> > +{
> > +	static const uint32_t ptypes[] = {
> > +		RTE_PTYPE_UNKNOWN,
> > +
> > +	};
> > +
> > +	return ptypes;
> > +}
> 
> Can we just add the code to grab the ptype value instead of just
> saying not supported.
> 
> The original code would just return an error from ethdev correct,
> what was wrong with that one. I would like to see the tap PMD just
> return the ptype would that not be more useful?
> 

tap PMD depends on the rte_net_get_ptype(), which code may change in the
future to support more packet types. Those changes would then need to be
reflected on the tap PMD, to be consistent.

I reported only RTE_PTYPE_UNKNOWN to avoid keeping a tight sync with the
rte_net library. As we're allowed to be more precise in the packet types
we actually set, compared to those we declare as supported, I thought it
best.

Would you indeed rather we copied all currently supported packet types
from rte_net to tap_dev_supported_ptypes_get()?

Regards,
Pascal

> > +
> > static const struct eth_dev_ops ops = {
> > 	.dev_start              = tap_dev_start,
> > 	.dev_stop               = tap_dev_stop,
> > @@ -784,6 +798,7 @@ static const struct eth_dev_ops ops = {
> > 	.mtu_set                = tap_mtu_set,
> > 	.stats_get              = tap_stats_get,
> > 	.stats_reset            = tap_stats_reset,
> > +	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
> > };
> > 
> > static int
> > -- 
> > 2.8.0.rc0
> > 
> 
> Regards,
> Keith
>
Wiles, Keith March 6, 2017, 2:46 p.m. UTC | #3 
> On Mar 6, 2017, at 8:10 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> On Fri, 3 Mar 2017 15:31:14 +0000
> "Wiles, Keith" <keith.wiles@intel.com> wrote:
> 
>> 
>>> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
>>> wrote:
>>> 
>>> Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet
>>> type.
>>> 
>>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
>>> ---
>>> doc/guides/nics/features/tap.ini |  1 +
>>> drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
>>> 2 files changed, 16 insertions(+)
>>> 
>>> diff --git a/doc/guides/nics/features/tap.ini
>>> b/doc/guides/nics/features/tap.ini index 6aa11874e2bc..7f3f4d661dd7
>>> 100644 --- a/doc/guides/nics/features/tap.ini
>>> +++ b/doc/guides/nics/features/tap.ini
>>> @@ -13,6 +13,7 @@ MTU update           = Y
>>> Multicast MAC filter = Y
>>> Speed capabilities   = Y
>>> Unicast MAC filter   = Y
>>> +Packet type parsing  = Y
>>> Other kdrv           = Y
>>> ARMv7                = Y
>>> ARMv8                = Y
>>> diff --git a/drivers/net/tap/rte_eth_tap.c
>>> b/drivers/net/tap/rte_eth_tap.c index 64b84cd76321..e4af36a6d142
>>> 100644 --- a/drivers/net/tap/rte_eth_tap.c
>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>> @@ -36,6 +36,7 @@
>>> #include <rte_malloc.h>
>>> #include <rte_vdev.h>
>>> #include <rte_kvargs.h>
>>> +#include <rte_net.h>
>>> 
>>> #include <sys/types.h>
>>> #include <sys/stat.h>
>>> @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf
>>> **bufs, uint16_t nb_pkts) mbuf->data_len = len;
>>> 		mbuf->pkt_len = len;
>>> 		mbuf->port = rxq->in_port;
>>> +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
>>> +
>>> RTE_PTYPE_ALL_MASK);
>>> 
>>> 		/* account for the receive frame */
>>> 		bufs[num_rx++] = mbuf;
>>> @@ -760,6 +763,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t
>>> mtu) return 0;
>>> }
>>> 
>>> +static const uint32_t*
>>> +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
>>> +{
>>> +	static const uint32_t ptypes[] = {
>>> +		RTE_PTYPE_UNKNOWN,
>>> +
>>> +	};
>>> +
>>> +	return ptypes;
>>> +}
>> 
>> Can we just add the code to grab the ptype value instead of just
>> saying not supported.
>> 
>> The original code would just return an error from ethdev correct,
>> what was wrong with that one. I would like to see the tap PMD just
>> return the ptype would that not be more useful?
>> 
> 
> tap PMD depends on the rte_net_get_ptype(), which code may change in the
> future to support more packet types. Those changes would then need to be
> reflected on the tap PMD, to be consistent.
> 
> I reported only RTE_PTYPE_UNKNOWN to avoid keeping a tight sync with the
> rte_net library. As we're allowed to be more precise in the packet types
> we actually set, compared to those we declare as supported, I thought it
> best.
> 
> Would you indeed rather we copied all currently supported packet types
> from rte_net to tap_dev_supported_ptypes_get()?

I see your point here, it not reasonable to copy all of the ptypes from rte_net to here ignore my comment.

> 
> Regards,
> Pascal
> 
>>> +
>>> static const struct eth_dev_ops ops = {
>>> 	.dev_start              = tap_dev_start,
>>> 	.dev_stop               = tap_dev_stop,
>>> @@ -784,6 +798,7 @@ static const struct eth_dev_ops ops = {
>>> 	.mtu_set                = tap_mtu_set,
>>> 	.stats_get              = tap_stats_get,
>>> 	.stats_reset            = tap_stats_reset,
>>> +	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
>>> };
>>> 
>>> static int
>>> -- 
>>> 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 6aa11874e2bc..7f3f4d661dd7 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -13,6 +13,7 @@  MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
+Packet type parsing  = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 64b84cd76321..e4af36a6d142 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -36,6 +36,7 @@ 
 #include <rte_malloc.h>
 #include <rte_vdev.h>
 #include <rte_kvargs.h>
+#include <rte_net.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -216,6 +217,8 @@  pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		mbuf->data_len = len;
 		mbuf->pkt_len = len;
 		mbuf->port = rxq->in_port;
+		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
+						      RTE_PTYPE_ALL_MASK);
 
 		/* account for the receive frame */
 		bufs[num_rx++] = mbuf;
@@ -760,6 +763,17 @@  tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return 0;
 }
 
+static const uint32_t*
+tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
+{
+	static const uint32_t ptypes[] = {
+		RTE_PTYPE_UNKNOWN,
+
+	};
+
+	return ptypes;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -784,6 +798,7 @@  static const struct eth_dev_ops ops = {
 	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
+	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
 };
 
 static int