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

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

On Thu, 9 Mar 2017 14:26:08 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 3/7/2017 4:31 PM, Pascal Mazon 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 d76f1dc83b03..edb5d2a82f12
> > 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);
> 
> Isn't PMD become inconsistent with this update. It reports
> RTE_PTYPE_UNKNOWN, but sets mbuf->packet_type with various ptype.

I discussed this briefly with Keith, but my argument was that the
librte_net did not provide a list of supported packet types, so I didn't
want to have to keep tap supported packet type list in sync with the
lib.
But it's actually just a few lines to add, I'll do that and also set a
comment to mention that it must be in sync with librte_net.

> 
> Do we want software packet type parsing in PMD level? Any change some
> users may not interested with this data at all.

Well, most PMDs support packet type parsing, and among the vdev, I
inspired that part from virtio, which does software ptype parsing.
An application coded with physical hardware PMD in mind doesn't have to
be recoded when switching to a tap PMD.

Furthermore, the tap PMD is already using syscalls in its datapath, the
cost of software packet parsing won't really change overall performance.

Regards,
Pascal

> 
> >  
> >  		/* account for the receive frame */
> >  		bufs[num_rx++] = mbuf;
> > @@ -769,6 +772,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,
> > @@ -793,6 +807,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
> > 
>