Re: [PATCH 5/6] net/tap: add packet type management

[Pascal Mazon <pascal.mazon@6wind.com>]

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
>