[dpdk-dev] [PATCH v3] net/ixgbe: enable PTYPE offload for x86 vector PMD

[Zhang, Qi Z]

Hi Ferruh:

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, June 12, 2017 6:18 PM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>; Ananyev, Konstantin
> <konstantin.ananyev at intel.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu at intel.com>; Zhang, Helin
> <helin.zhang at intel.com>; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] net/ixgbe: enable PTYPE offload for x86
> vector PMD
> 
> On 6/11/2017 6:17 AM, Qi Zhang wrote:
> > Hardware PTYPE in Rx desc will be parsed to fill mbuf's packet_type.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
> 
> <...>
> 
> > diff --git a/drivers/net/ixgbe/Makefile b/drivers/net/ixgbe/Makefile
> > index 5529d81..8b86fe3 100644
> > --- a/drivers/net/ixgbe/Makefile
> > +++ b/drivers/net/ixgbe/Makefile
> > @@ -125,6 +125,11 @@ SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) +=
> > ixgbe_82599_bypass.c  endif
> >  SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += rte_pmd_ixgbe.c
> >
> > +# vector PMD driver needs SSE4.1 support ifeq ($(findstring
> > +RTE_MACHINE_CPUFLAG_SSE4_1,$(CFLAGS)),)
> > +CFLAGS_ixgbe_rxtx_vec_sse.o += -msse4.1 endif
> 
> Hi Qi,
> 
> What happens if runtime doesn't have SSE4.1 support?
> 
> Should we add a dynamic check as done in 140e [1] ?
> 
I think dynamic check is necessary, thanks for capture this.
> 
> And I am for extracting this part into another patch, to highlight what is done
> here.
> 
> patch 1: Increase ixgbe minimum required support to SSE4.1 Patch 2: Add
> PTYPE support to vector PMD
> 
> [1]		
> http://dpdk.org/browse/dpdk/tree/drivers/net/i40e/i40e_rxtx_vec_sse.c#n6
> 26
> 
Agree, two patches looks better.
> 
> > +
> >  # install this header file
> >  SYMLINK-$(CONFIG_RTE_LIBRTE_IXGBE_PMD)-include :=
> rte_pmd_ixgbe.h
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 2083cde..3e39648 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -3717,6 +3717,14 @@ ixgbe_dev_supported_ptypes_get(struct
> rte_eth_dev *dev)
> >  	    dev->rx_pkt_burst == ixgbe_recv_pkts_lro_bulk_alloc ||
> >  	    dev->rx_pkt_burst == ixgbe_recv_pkts_bulk_alloc)
> >  		return ptypes;
> > +
> > +#if defined(RTE_ARCH_X86_64) || \
> > +	defined(RTE_ARCH_X86_X32) || \
> > +	defined(RTE_ARCH_I686)
> 
> It is possible to use single "RTE_ARCH_X86" here, to cover all three.
I saw all x86 configure will have this macro enabled, it’s the one I'm looking for, thanks
> 
> > +	if (dev->rx_pkt_burst == ixgbe_recv_pkts_vec ||
> > +	    dev->rx_pkt_burst == ixgbe_recv_scattered_pkts_vec)
> > +		return ptypes;
> > +#endif
> >  	return NULL;
> >  }
> 
> <...>