DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/4] testpmd: Add support to configure 25G and 50G speeds
From: De Lara Guarch, Pablo @ 2016-10-12  1:24 UTC (permalink / raw)
  To: Ajit Khaparde, Yigit, Ferruh; +Cc: dev@dpdk.org
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA07C0F@IRSMSX108.ger.corp.intel.com>

Hi,

> From: Ajit Khaparde [mailto:ajit.khaparde@broadcom.com]
> Sent: Monday, October 10, 2016 11:41 AM
> To: Yigit, Ferruh
> Cc: dev@dpdk.org; De Lara Guarch, Pablo
> Subject: Re: [dpdk-dev] [PATCH 1/4] testpmd: Add support to configure 25G
> and 50G speeds
> 
> On Mon, Oct 10, 2016 at 10:01 AM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> Hi Ajit,
> 
> On 9/29/2016 6:03 PM, Ajit Khaparde wrote:
> > Support to configure 25G and 50G speeds is missing from testpmd.
> > This patch also updates the testpmd user guide accordingly.
> >
> > Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> 
> This patch seems not really part of the patchset for bnxt driver, but
> standalone testpmd only modification,
> ​​
> and can be threaded as single patch.
> ​OK. Got it.
> I had worked on this change in the middle of other things.
> ​So it got bundled along with other bnxt patches.
> 
> 
> I am adding testpmd maintainer to cc.
> ​Let me know if you want me to send it again as a standalone patch.

Yes, I think it is a good idea.
Apart from that, there are a couple of things to fix:

There are two lines that exceed the character limit (from checkpatch):
WARNING: line over 80 characters
#55: FILE: app/test-pmd/cmdline.c:1084:
+       .help_str = "port config all speed 10|100|1000|10000|25000|40000|50000|100000|auto duplex "

WARNING: line over 80 characters
#73: FILE: app/test-pmd/cmdline.c:1159:
+       .help_str = "port config X speed 10|100|1000|10000|25000|40000|50000|100000|auto duplex "

check-git-log.sh complains about the title:

Wrong headline label:
        testpmd: Add support to configure 25G and 50G speeds
Wrong headline uppercase:
        testpmd: Add support to configure 25G and 50G speeds

Title should be "app/testpmd: add ...."

Could you send a separate v2 with this changes?
And probably change the bnxt patchset to 3 patches only.

Thanks,
Pablo

^ permalink raw reply

* [PATCH] hash: fix incorrect bucket size usage
From: Pablo de Lara @ 2016-10-12  1:26 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, Pablo de Lara

Multiwriter insert function was using a fixed value for
the bucket size, instead of using the
RTE_HASH_BUCKET_ENTRIES macro, which value was changed
recently (making it inconsistent in this case).

Fixes: be856325cba3 ("hash: add scalable multi-writer insertion with Intel TSX")

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash_x86.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash_x86.h b/lib/librte_hash/rte_cuckoo_hash_x86.h
index 47aec6d..0c94244 100644
--- a/lib/librte_hash/rte_cuckoo_hash_x86.h
+++ b/lib/librte_hash/rte_cuckoo_hash_x86.h
@@ -167,7 +167,8 @@ rte_hash_cuckoo_make_space_mw_tm(const struct rte_hash *h,
 
 	/* Cuckoo bfs Search */
 	while (likely(tail != head && head <
-					queue + RTE_HASH_BFS_QUEUE_MAX_LEN - 4)) {
+					queue + RTE_HASH_BFS_QUEUE_MAX_LEN -
+					RTE_HASH_BUCKET_ENTRIES)) {
 		curr_bkt = tail->bkt;
 		for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
 			if (curr_bkt->key_idx[i] == EMPTY_SLOT) {
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v7 2/2] app/test_pmd: add tests for new API's
From: De Lara Guarch, Pablo @ 2016-10-12  2:05 UTC (permalink / raw)
  To: Iremonger, Bernard, Thomas Monjalon
  Cc: Yigit, Ferruh, dev@dpdk.org, Shah, Rahul R, Lu, Wenzhuo,
	az5157@att.com
In-Reply-To: <8CEF83825BEC744B83065625E567D7C21A090B2D@IRSMSX108.ger.corp.intel.com>

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Iremonger, Bernard
> Sent: Tuesday, October 11, 2016 9:35 AM
> To: Thomas Monjalon
> Cc: Yigit, Ferruh; dev@dpdk.org; Shah, Rahul R; Lu, Wenzhuo;
> az5157@att.com
> Subject: Re: [dpdk-dev] [PATCH v7 2/2] app/test_pmd: add tests for new API's
> 
> Hi Thomas,
> 
> <snip>
> 
> > > > Subject: Re: [dpdk-dev] [PATCH v7 2/2] app/test_pmd: add tests for
> > > > new API's
> > > >
> > > > 2016-10-11 16:09, Ferruh Yigit:
> > > > > This will cause a compilation error for shared libraries. Because
> > > > > PMDs not linked against application when compiled as shared
> > > > > library but used as plugins.
> > > > >
> > > > > Since it has been decided to have NIC specific APIs, we need to
> > > > > re-work that approach to fix shared library compilation.
> > > >
> > > > If testpmd uses the ixgbe API, it must be linked with the PMD.
> > > > Is there any issue adapting the testpmd makefile?
> > > > Hope that dlopen an already linked PMD is nicely managed.
> > >
> > > The ixgbe API will  be used by other apps, for example Virtual
> > > Function Daemon (VFD) Moving the following line in rte.app.mak solves
> > > the problem
> > >
> > > Line 117: _LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD)      += -
> > lrte_pmd_ixgbe
> > >
> > > To Line 103.
> > >
> > > Will this be acceptable?
> >
> > I think we must not link PMD in the general case but let this responsibility to
> > the application in case it uses some specific functions.
> > Does it make sense?
> 
> Yes, ok, will just modify the testpmd makefile  for this case.

A couple of things:
You would need to use #ifdef RTE_LIBRTE_IXGBE_PMD in testpmd, right?
In case IXGBE is not available (maybe just modifying the makefile solves the problem).

Could you also add these new functions in the testpmd help?
Just add them in cmd_help_long_parsed.

Thanks,
Pablo

> 
> Regards,
> 
> Bernard.

^ permalink raw reply

* Re: ZUC PMD as shared library
From: De Lara Guarch, Pablo @ 2016-10-12  2:23 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev@dpdk.org
In-Reply-To: <1537716.E2UdgpDg0I@xps13>

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Saturday, October 08, 2016 10:19 AM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org
> Subject: ZUC PMD as shared library
> 
> Hi Pablo,
> 
> You are probably aware of the issue, but I would like to make it clear
> in case someone else run into the same trouble:
> 
> It is impossible to build the ZUC crypto PMD as a shared library:
> 
> libsso-zuc-0.1.1/build/libsso_zuc.a(sso_zuc_yasm.o):
> 	relocation R_X86_64_32 against `EK_d' can not be used
> 	when making a shared object; recompile with -fPIC
> 
> The library libsso-zuc-0.1.1 needs an update to make the asm code
> relocatable.
> Should we explicit this limitation in the PMD doc?

Sorry for not replying to this earlier.
Yes, you are right, thanks for pointing it out.
I think the same problem is in KASUMI, so I will send a doc update for both PMDs.

Thanks,
Pablo

^ permalink raw reply

* Re: [RFC v2] Generic flow director/filtering/classification API
From: Zhao1, Wei @ 2016-10-12  2:38 UTC (permalink / raw)
  To: Adrien Mazarguil; +Cc: dev@dpdk.org
In-Reply-To: <20161011082116.GL17252@6wind.com>

Hi  Adrien Mazarguil,

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Tuesday, October 11, 2016 4:21 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC v2] Generic flow director/filtering/classification
> API
> 
> Hi Wei,
> 
> On Tue, Oct 11, 2016 at 01:47:53AM +0000, Zhao1, Wei wrote:
> > Hi  Adrien Mazarguil,
> >      There is a struct rte_flow_action_rss in rte_flow.txt, the  member
> rss_conf is a pointer type, is there any convenience in using pointer?
> > Why not using  struct rte_eth_rss_conf rss_conf type, as
> rte_flow_item_ipv4/ rte_flow_item_ipv6 struct member?
> >
> > Thank you.
> >
> >  struct rte_flow_action_rss {
> > 	struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
> > 	uint16_t queues; /**< Number of entries in queue[]. */
> > 	uint16_t queue[]; /**< Queues indices to use. */ };
> 
> Well I thought it made sharing flow RSS configuration with its counterpart in
> struct rte_eth_conf easier (this pointer should even be const). Also, while
> ABI breakage would still occur if rte_eth_rss_conf happened to be modified,
> the impact on this API would be limited as it would not cause a change in
> structure size. We'd ideally need some kind of version field to be completely
> safe but I guess that would be somewhat overkill.
> 
> Now considering this API was written without an initial implementation, all
> structure definitions that do not make sense are still open to debate, we can
> adjust them as needed.
> 
> --
> Adrien Mazarguil
> 6WIND

Your explanation seems very reasonable for me, structure pointer is an very experienced usage in this situation.
Thank you!

^ permalink raw reply

* Re: [PATCH 2/5] i40e: implement vector PMD for ARM architecture
From: Zhang, Qi Z @ 2016-10-12  2:46 UTC (permalink / raw)
  To: Jianbo Liu
  Cc: Zhang, Helin, Wu, Jingjing, jerin.jacob@caviumnetworks.com,
	dev@dpdk.org
In-Reply-To: <1472032425-16136-3-git-send-email-jianbo.liu@linaro.org>

Hi Jianbo:

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianbo Liu
> Sent: Wednesday, August 24, 2016 5:54 PM
> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; jerin.jacob@caviumnetworks.com; dev@dpdk.org
> Cc: Jianbo Liu <jianbo.liu@linaro.org>
> Subject: [dpdk-dev] [PATCH 2/5] i40e: implement vector PMD for ARM
> architecture
> 
> Use ARM NEON intrinsic to implement i40e vPMD
> 
> Signed-off-by: Jianbo Liu <jianbo.liu@linaro.org>
> ---
>  drivers/net/i40e/Makefile             |   4 +
>  drivers/net/i40e/i40e_rxtx_vec_neon.c | 581
> ++++++++++++++++++++++++++++++++++
>  2 files changed, 585 insertions(+)
>  create mode 100644 drivers/net/i40e/i40e_rxtx_vec_neon.c
> 
> diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile index
> 53fe145..9e92b38 100644
> --- a/drivers/net/i40e/Makefile
> +++ b/drivers/net/i40e/Makefile
> @@ -97,7 +97,11 @@ SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_dcb.c
> 
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev.c
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_rxtx.c
> +ifeq ($(CONFIG_RTE_ARCH_ARM64),y)
> +SRCS-$(CONFIG_RTE_LIBRTE_I40E_INC_VECTOR) += i40e_rxtx_vec_neon.c
> else
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_INC_VECTOR) += i40e_rxtx_vec.c
> +endif
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev_vf.c
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_pf.c
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_fdir.c diff --git
> a/drivers/net/i40e/i40e_rxtx_vec_neon.c
> b/drivers/net/i40e/i40e_rxtx_vec_neon.c
> new file mode 100644
> index 0000000..015fa9f
> --- /dev/null
> +++ b/drivers/net/i40e/i40e_rxtx_vec_neon.c
> @@ -0,0 +1,581 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2016, Linaro Limited
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> + */
> +
> +#include <stdint.h>
> +#include <rte_ethdev.h>
> +#include <rte_malloc.h>
> +
> +#include "base/i40e_prototype.h"
> +#include "base/i40e_type.h"
> +#include "i40e_ethdev.h"
> +#include "i40e_rxtx.h"
> +#include "i40e_rxtx_vec_common.h"
> +
> +#include <arm_neon.h>
> +
> +#pragma GCC diagnostic ignored "-Wcast-qual"
> +
> +static inline void
> +i40e_rxq_rearm(struct i40e_rx_queue *rxq) {
> +	int i;
> +	uint16_t rx_id;
> +	volatile union i40e_rx_desc *rxdp;
> +	struct i40e_rx_entry *rxep = &rxq->sw_ring[rxq->rxrearm_start];
> +	struct rte_mbuf *mb0, *mb1;
> +	uint64x2_t dma_addr0, dma_addr1;
> +	uint64x2_t zero = vdupq_n_u64(0);
> +	uint64_t paddr;
> +	uint8x8_t p;
> +
> +	rxdp = rxq->rx_ring + rxq->rxrearm_start;
> +
> +	/* Pull 'n' more MBUFs into the software ring */
> +	if (unlikely(rte_mempool_get_bulk(rxq->mp,
> +					  (void *)rxep,
> +					  RTE_I40E_RXQ_REARM_THRESH) < 0)) {
> +		if (rxq->rxrearm_nb + RTE_I40E_RXQ_REARM_THRESH >=
> +		    rxq->nb_rx_desc) {
> +			for (i = 0; i < RTE_I40E_DESCS_PER_LOOP; i++) {
> +				rxep[i].mbuf = &rxq->fake_mbuf;
> +				vst1q_u64((uint64_t *)&rxdp[i].read, zero);
> +			}
> +		}
> +		rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed +=
> +			RTE_I40E_RXQ_REARM_THRESH;
> +		return;
> +	}
> +
> +	p = vld1_u8((uint8_t *)&rxq->mbuf_initializer);
> +
> +	/* Initialize the mbufs in vector, process 2 mbufs in one loop */
> +	for (i = 0; i < RTE_I40E_RXQ_REARM_THRESH; i += 2, rxep += 2) {
> +		mb0 = rxep[0].mbuf;
> +		mb1 = rxep[1].mbuf;
> +
> +		 /* Flush mbuf with pkt template.
> +		 * Data to be rearmed is 6 bytes long.
> +		 * Though, RX will overwrite ol_flags that are coming next
> +		 * anyway. So overwrite whole 8 bytes with one load:
> +		 * 6 bytes of rearm_data plus first 2 bytes of ol_flags.
> +		 */
> +		vst1_u8((uint8_t *)&mb0->rearm_data, p);
> +		paddr = mb0->buf_physaddr + RTE_PKTMBUF_HEADROOM;
> +		dma_addr0 = vdupq_n_u64(paddr);
> +
> +		/* flush desc with pa dma_addr */
> +		vst1q_u64((uint64_t *)&rxdp++->read, dma_addr0);
> +
> +		vst1_u8((uint8_t *)&mb1->rearm_data, p);
> +		paddr = mb1->buf_physaddr + RTE_PKTMBUF_HEADROOM;
> +		dma_addr1 = vdupq_n_u64(paddr);
> +		vst1q_u64((uint64_t *)&rxdp++->read, dma_addr1);
> +	}
> +
> +	rxq->rxrearm_start += RTE_I40E_RXQ_REARM_THRESH;
> +	if (rxq->rxrearm_start >= rxq->nb_rx_desc)
> +		rxq->rxrearm_start = 0;
> +
> +	rxq->rxrearm_nb -= RTE_I40E_RXQ_REARM_THRESH;
> +
> +	rx_id = (uint16_t)((rxq->rxrearm_start == 0) ?
> +			     (rxq->nb_rx_desc - 1) : (rxq->rxrearm_start - 1));
> +
> +	/* Update the tail pointer on the NIC */
> +	I40E_PCI_REG_WRITE(rxq->qrx_tail, rx_id); }
> +
> +/* Handling the offload flags (olflags) field takes computation
> + * time when receiving packets. Therefore we provide a flag to disable
> + * the processing of the olflags field when they are not needed. This
> + * gives improved performance, at the cost of losing the offload info
> + * in the received packet
> + */
> +#ifdef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
> +
> +static inline void
> +desc_to_olflags_v(uint16x8_t staterr, struct rte_mbuf **rx_pkts) {
> +	uint16x8_t vlan0, vlan1, rss;
> +	union {
> +		uint16_t e[4];
> +		uint64_t dword;
> +	} vol;
> +
> +	/* mask everything except RSS, flow director and VLAN flags
> +	 * bit2 is for VLAN tag, bit11 for flow director indication
> +	 * bit13:12 for RSS indication.
> +	 */
> +	const uint16x8_t rss_vlan_msk = {
> +			0x3804, 0x3804, 0x3804, 0x3804,
> +			0x0000, 0x0000, 0x0000, 0x0000};
> +
> +	/* map rss and vlan type to rss hash and vlan flag */
> +	const uint8x16_t vlan_flags = {
> +			0, 0, 0, 0,
> +			PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED, 0, 0, 0,
> +			0, 0, 0, 0,
> +			0, 0, 0, 0};
> +
> +	const uint8x16_t rss_flags = {
> +			0, PKT_RX_FDIR, 0, 0,
> +			0, 0, PKT_RX_RSS_HASH, PKT_RX_RSS_HASH | PKT_RX_FDIR,
> +			0, 0, 0, 0,
> +			0, 0, 0, 0};
> +
> +	vlan1 = vandq_u16(staterr, rss_vlan_msk);
> +	vlan0 = vreinterpretq_u16_u8(vqtbl1q_u8(vlan_flags,
> +						vreinterpretq_u8_u16(vlan1)));
> +
> +	rss = vshrq_n_u16(vlan1, 11);
> +	rss = vreinterpretq_u16_u8(vqtbl1q_u8(rss_flags,
> +					      vreinterpretq_u8_u16(rss)));
> +
> +	vlan0 = vorrq_u16(vlan0, rss);
> +	vol.dword = vgetq_lane_u64(vreinterpretq_u64_u16(vlan0), 0);
> +
> +	rx_pkts[0]->ol_flags = vol.e[0];
> +	rx_pkts[1]->ol_flags = vol.e[1];
> +	rx_pkts[2]->ol_flags = vol.e[2];
> +	rx_pkts[3]->ol_flags = vol.e[3];
> +}
> +#else
> +#define desc_to_olflags_v(staterr, rx_pkts) do {} while (0) #endif
> +
> +#define PKTLEN_SHIFT     10
> +
> +#define I40E_VPMD_DESC_DD_MASK	0x0001000100010001ULL
> +
> + /*
> + * Notice:
> + * - nb_pkts < RTE_I40E_DESCS_PER_LOOP, just return no packet
> + * - nb_pkts > RTE_I40E_VPMD_RX_BURST, only scan
> RTE_I40E_VPMD_RX_BURST
> + *   numbers of DD bits
> + */
> +static inline uint16_t
> +_recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_pkts,
> +		   uint16_t nb_pkts, uint8_t *split_packet) {
> +	volatile union i40e_rx_desc *rxdp;
> +	struct i40e_rx_entry *sw_ring;
> +	uint16_t nb_pkts_recd;
> +	int pos;
> +	uint64_t var;
> +
> +	/* mask to shuffle from desc. to mbuf */
> +	uint8x16_t shuf_msk = {
> +		0xFF, 0xFF,   /* pkt_type set as unknown */
> +		0xFF, 0xFF,   /* pkt_type set as unknown */
> +		14, 15,       /* octet 15~14, low 16 bits pkt_len */
> +		0xFF, 0xFF,   /* skip high 16 bits pkt_len, zero out */
> +		14, 15,       /* octet 15~14, 16 bits data_len */
> +		2, 3,         /* octet 2~3, low 16 bits vlan_macip */
> +		4, 5, 6, 7    /* octet 4~7, 32bits rss */
> +		};
> +
> +	uint8x16_t eop_check = {
> +		0x02, 0x00, 0x02, 0x00,
> +		0x02, 0x00, 0x02, 0x00,
> +		0x00, 0x00, 0x00, 0x00,
> +		0x00, 0x00, 0x00, 0x00
> +		};
> +
> +	uint16x8_t crc_adjust = {
> +		0, 0,         /* ignore pkt_type field */
> +		rxq->crc_len, /* sub crc on pkt_len */
> +		0,            /* ignore high-16bits of pkt_len */
> +		rxq->crc_len, /* sub crc on data_len */
> +		0, 0, 0       /* ignore non-length fields */
> +		};
> +
> +	/* nb_pkts shall be less equal than RTE_I40E_MAX_RX_BURST */
> +	nb_pkts = RTE_MIN(nb_pkts, RTE_I40E_MAX_RX_BURST);
> +
> +	/* nb_pkts has to be floor-aligned to RTE_I40E_DESCS_PER_LOOP */
> +	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_I40E_DESCS_PER_LOOP);
> +
> +	/* Just the act of getting into the function from the application is
> +	 * going to cost about 7 cycles
> +	 */
> +	rxdp = rxq->rx_ring + rxq->rx_tail;
> +
> +	rte_prefetch_non_temporal(rxdp);
> +
> +	/* See if we need to rearm the RX queue - gives the prefetch a bit
> +	 * of time to act
> +	 */
> +	if (rxq->rxrearm_nb > RTE_I40E_RXQ_REARM_THRESH)
> +		i40e_rxq_rearm(rxq);
> +
> +	/* Before we start moving massive data around, check to see if
> +	 * there is actually a packet available
> +	 */
> +	if (!(rxdp->wb.qword1.status_error_len &
> +			rte_cpu_to_le_32(1 << I40E_RX_DESC_STATUS_DD_SHIFT)))
> +		return 0;
> +
> +	/* Cache is empty -> need to scan the buffer rings, but first move
> +	 * the next 'n' mbufs into the cache
> +	 */
> +	sw_ring = &rxq->sw_ring[rxq->rx_tail];
> +
> +	/* A. load 4 packet in one loop
> +	 * [A*. mask out 4 unused dirty field in desc]
> +	 * B. copy 4 mbuf point from swring to rx_pkts
> +	 * C. calc the number of DD bits among the 4 packets
> +	 * [C*. extract the end-of-packet bit, if requested]
> +	 * D. fill info. from desc to mbuf
> +	 */
> +
> +	for (pos = 0, nb_pkts_recd = 0; pos < nb_pkts;
> +			pos += RTE_I40E_DESCS_PER_LOOP,
> +			rxdp += RTE_I40E_DESCS_PER_LOOP) {
> +		uint64x2_t descs[RTE_I40E_DESCS_PER_LOOP];
> +		uint8x16_t pkt_mb1, pkt_mb2, pkt_mb3, pkt_mb4;
> +		uint16x8x2_t sterr_tmp1, sterr_tmp2;
> +		uint64x2_t mbp1, mbp2;
> +		uint16x8_t staterr;
> +		uint16x8_t tmp;
> +		uint64_t stat;
> +
> +		int32x4_t len_shl = {0, 0, 0, PKTLEN_SHIFT};
> +
> +		/* B.1 load 1 mbuf point */
> +		mbp1 = vld1q_u64((uint64_t *)&sw_ring[pos]);
> +		/* Read desc statuses backwards to avoid race condition */
> +		/* A.1 load 4 pkts desc */
> +		descs[3] =  vld1q_u64((uint64_t *)(rxdp + 3));
> +		rte_rmb();
> +
> +		/* B.2 copy 2 mbuf point into rx_pkts  */
> +		vst1q_u64((uint64_t *)&rx_pkts[pos], mbp1);
> +
> +		/* B.1 load 1 mbuf point */
> +		mbp2 = vld1q_u64((uint64_t *)&sw_ring[pos + 2]);
> +
> +		descs[2] =  vld1q_u64((uint64_t *)(rxdp + 2));
> +		/* B.1 load 2 mbuf point */
> +		descs[1] =  vld1q_u64((uint64_t *)(rxdp + 1));
> +		descs[0] =  vld1q_u64((uint64_t *)(rxdp));
> +
> +		/* B.2 copy 2 mbuf point into rx_pkts  */
> +		vst1q_u64((uint64_t *)&rx_pkts[pos + 2], mbp2);
> +
> +		if (split_packet) {
> +			rte_mbuf_prefetch_part2(rx_pkts[pos]);
> +			rte_mbuf_prefetch_part2(rx_pkts[pos + 1]);
> +			rte_mbuf_prefetch_part2(rx_pkts[pos + 2]);
> +			rte_mbuf_prefetch_part2(rx_pkts[pos + 3]);
> +		}
> +
> +		/* avoid compiler reorder optimization */
> +		rte_compiler_barrier();
> +
> +		/* pkt 3,4 shift the pktlen field to be 16-bit aligned*/
> +		uint32x4_t len3 = vshlq_u32(vreinterpretq_u32_u64(descs[3]),
> +					    len_shl);
> +		descs[3] = vreinterpretq_u64_u32(len3);
> +		uint32x4_t len2 = vshlq_u32(vreinterpretq_u32_u64(descs[2]),
> +					    len_shl);
> +		descs[2] = vreinterpretq_u64_u32(len2);
> +
> +		/* D.1 pkt 3,4 convert format from desc to pktmbuf */
> +		pkt_mb4 = vqtbl1q_u8(vreinterpretq_u8_u64(descs[3]), shuf_msk);
> +		pkt_mb3 = vqtbl1q_u8(vreinterpretq_u8_u64(descs[2]), shuf_msk);
> +
> +		/* C.1 4=>2 filter staterr info only */
> +		sterr_tmp2 = vzipq_u16(vreinterpretq_u16_u64(descs[1]),
> +				       vreinterpretq_u16_u64(descs[3]));
> +		/* C.1 4=>2 filter staterr info only */
> +		sterr_tmp1 = vzipq_u16(vreinterpretq_u16_u64(descs[0]),
> +				       vreinterpretq_u16_u64(descs[2]));
> +
> +		/* C.2 get 4 pkts staterr value  */
> +		staterr = vzipq_u16(sterr_tmp1.val[1],
> +				    sterr_tmp2.val[1]).val[0];
> +		stat = vgetq_lane_u64(vreinterpretq_u64_u16(staterr), 0);
> +
> +		desc_to_olflags_v(staterr, &rx_pkts[pos]);
> +
> +		/* D.2 pkt 3,4 set in_port/nb_seg and remove crc */
> +		tmp = vsubq_u16(vreinterpretq_u16_u8(pkt_mb4), crc_adjust);
> +		pkt_mb4 = vreinterpretq_u8_u16(tmp);
> +		tmp = vsubq_u16(vreinterpretq_u16_u8(pkt_mb3), crc_adjust);
> +		pkt_mb3 = vreinterpretq_u8_u16(tmp);
> +
> +		/* pkt 1,2 shift the pktlen field to be 16-bit aligned*/
> +		uint32x4_t len1 = vshlq_u32(vreinterpretq_u32_u64(descs[1]),
> +					    len_shl);
> +		descs[1] = vreinterpretq_u64_u32(len1);
> +		uint32x4_t len0 = vshlq_u32(vreinterpretq_u32_u64(descs[0]),
> +					    len_shl);
> +		descs[0] = vreinterpretq_u64_u32(len0);
> +
> +		/* D.1 pkt 1,2 convert format from desc to pktmbuf */
> +		pkt_mb2 = vqtbl1q_u8(vreinterpretq_u8_u64(descs[1]), shuf_msk);
> +		pkt_mb1 = vqtbl1q_u8(vreinterpretq_u8_u64(descs[0]), shuf_msk);
> +
> +		/* D.3 copy final 3,4 data to rx_pkts */
> +		vst1q_u8((void *)&rx_pkts[pos + 3]->rx_descriptor_fields1,
> +				 pkt_mb4);
> +		vst1q_u8((void *)&rx_pkts[pos + 2]->rx_descriptor_fields1,
> +				 pkt_mb3);
> +
> +		/* D.2 pkt 1,2 set in_port/nb_seg and remove crc */
> +		tmp = vsubq_u16(vreinterpretq_u16_u8(pkt_mb2), crc_adjust);
> +		pkt_mb2 = vreinterpretq_u8_u16(tmp);
> +		tmp = vsubq_u16(vreinterpretq_u16_u8(pkt_mb1), crc_adjust);
> +		pkt_mb1 = vreinterpretq_u8_u16(tmp);
> +
> +		/* C* extract and record EOP bit */
> +		if (split_packet) {
> +			uint8x16_t eop_shuf_mask = {
> +					0x00, 0x02, 0x04, 0x06,
> +					0xFF, 0xFF, 0xFF, 0xFF,
> +					0xFF, 0xFF, 0xFF, 0xFF,
> +					0xFF, 0xFF, 0xFF, 0xFF};
> +			uint8x16_t eop_bits;
> +
> +			/* and with mask to extract bits, flipping 1-0 */
> +			eop_bits = vmvnq_u8(vreinterpretq_u8_u16(staterr));
> +			eop_bits = vandq_u8(eop_bits, eop_check);
> +			/* the staterr values are not in order, as the count
> +			 * count of dd bits doesn't care. However, for end of
> +			 * packet tracking, we do care, so shuffle. This also
> +			 * compresses the 32-bit values to 8-bit
> +			 */
> +			eop_bits = vqtbl1q_u8(eop_bits, eop_shuf_mask);
> +
> +			/* store the resulting 32-bit value */
> +			vst1q_lane_u32((uint32_t *)split_packet,
> +				       vreinterpretq_u32_u8(eop_bits), 0);
> +			split_packet += RTE_I40E_DESCS_PER_LOOP;
> +
> +			/* zero-out next pointers */
> +			rx_pkts[pos]->next = NULL;
> +			rx_pkts[pos + 1]->next = NULL;
> +			rx_pkts[pos + 2]->next = NULL;
> +			rx_pkts[pos + 3]->next = NULL;
> +		}
> +
> +		rte_prefetch_non_temporal(rxdp + RTE_I40E_DESCS_PER_LOOP);
> +
> +		/* D.3 copy final 1,2 data to rx_pkts */
> +		vst1q_u8((void *)&rx_pkts[pos + 1]->rx_descriptor_fields1,
> +			 pkt_mb2);
> +		vst1q_u8((void *)&rx_pkts[pos]->rx_descriptor_fields1,
> +			 pkt_mb1);
> +		/* C.4 calc avaialbe number of desc */
> +		var = __builtin_popcountll(stat & I40E_VPMD_DESC_DD_MASK);
> +		nb_pkts_recd += var;
> +		if (likely(var != RTE_I40E_DESCS_PER_LOOP))
> +			break;
> +	}
> +
> +	/* Update our internal tail pointer */
> +	rxq->rx_tail = (uint16_t)(rxq->rx_tail + nb_pkts_recd);
> +	rxq->rx_tail = (uint16_t)(rxq->rx_tail & (rxq->nb_rx_desc - 1));
> +	rxq->rxrearm_nb = (uint16_t)(rxq->rxrearm_nb + nb_pkts_recd);
> +
> +	return nb_pkts_recd;
> +}
> +
> + /*
> + * Notice:
> + * - nb_pkts < RTE_I40E_DESCS_PER_LOOP, just return no packet
> + * - nb_pkts > RTE_I40E_VPMD_RX_BURST, only scan
> RTE_I40E_VPMD_RX_BURST
> + *   numbers of DD bits
> + */
> +uint16_t
> +i40e_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
> +		   uint16_t nb_pkts)
> +{
> +	return _recv_raw_pkts_vec(rx_queue, rx_pkts, nb_pkts, NULL); }
> +
> + /* vPMD receive routine that reassembles scattered packets
> + * Notice:
> + * - nb_pkts < RTE_I40E_DESCS_PER_LOOP, just return no packet
> + * - nb_pkts > RTE_I40E_VPMD_RX_BURST, only scan
> RTE_I40E_VPMD_RX_BURST
> + *   numbers of DD bits
> + */
> +uint16_t
> +i40e_recv_scattered_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
> +			     uint16_t nb_pkts)
> +{
> +
> +	struct i40e_rx_queue *rxq = rx_queue;
> +	uint8_t split_flags[RTE_I40E_VPMD_RX_BURST] = {0};
> +
> +	/* get some new buffers */
> +	uint16_t nb_bufs = _recv_raw_pkts_vec(rxq, rx_pkts, nb_pkts,
> +			split_flags);
> +	if (nb_bufs == 0)
> +		return 0;
> +
> +	/* happy day case, full burst + no packets to be joined */
> +	const uint64_t *split_fl64 = (uint64_t *)split_flags;
> +
> +	if (rxq->pkt_first_seg == NULL &&
> +			split_fl64[0] == 0 && split_fl64[1] == 0 &&
> +			split_fl64[2] == 0 && split_fl64[3] == 0)
> +		return nb_bufs;
> +
> +	/* reassemble any packets that need reassembly*/
> +	unsigned i = 0;
> +
> +	if (rxq->pkt_first_seg == NULL) {
> +		/* find the first split flag, and only reassemble then*/
> +		while (i < nb_bufs && !split_flags[i])
> +			i++;
> +		if (i == nb_bufs)
> +			return nb_bufs;
> +	}
> +	return i + reassemble_packets(rxq, &rx_pkts[i], nb_bufs - i,
> +		&split_flags[i]);
> +}
> +
> +static inline void
> +vtx1(volatile struct i40e_tx_desc *txdp,
> +		struct rte_mbuf *pkt, uint64_t flags) {
> +	uint64_t high_qw = (I40E_TX_DESC_DTYPE_DATA |
> +			((uint64_t)flags  << I40E_TXD_QW1_CMD_SHIFT) |
> +			((uint64_t)pkt->data_len <<
> I40E_TXD_QW1_TX_BUF_SZ_SHIFT));
> +
> +	uint64x2_t descriptor = {pkt->buf_physaddr + pkt->data_off, high_qw};
> +	vst1q_u64((uint64_t *)txdp, descriptor); }
> +
> +static inline void
> +vtx(volatile struct i40e_tx_desc *txdp,
> +		struct rte_mbuf **pkt, uint16_t nb_pkts,  uint64_t flags) {
> +	int i;
> +
> +	for (i = 0; i < nb_pkts; ++i, ++txdp, ++pkt)
> +		vtx1(txdp, *pkt, flags);
> +}
> +
> +uint16_t
> +i40e_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
> +		   uint16_t nb_pkts)
> +{
> +	struct i40e_tx_queue *txq = (struct i40e_tx_queue *)tx_queue;
> +	volatile struct i40e_tx_desc *txdp;
> +	struct i40e_tx_entry *txep;
> +	uint16_t n, nb_commit, tx_id;
> +	uint64_t flags = I40E_TD_CMD;
> +	uint64_t rs = I40E_TX_DESC_CMD_RS | I40E_TD_CMD;
> +	int i;
> +
> +	/* cross rx_thresh boundary is not allowed */
> +	nb_pkts = RTE_MIN(nb_pkts, txq->tx_rs_thresh);
> +
> +	if (txq->nb_tx_free < txq->tx_free_thresh)
> +		i40e_tx_free_bufs(txq);
> +
> +	nb_commit = nb_pkts = (uint16_t)RTE_MIN(txq->nb_tx_free, nb_pkts);
> +	if (unlikely(nb_pkts == 0))
> +		return 0;
> +
> +	tx_id = txq->tx_tail;
> +	txdp = &txq->tx_ring[tx_id];
> +	txep = &txq->sw_ring[tx_id];
> +
> +	txq->nb_tx_free = (uint16_t)(txq->nb_tx_free - nb_pkts);
> +
> +	n = (uint16_t)(txq->nb_tx_desc - tx_id);
> +	if (nb_commit >= n) {
> +		tx_backlog_entry(txep, tx_pkts, n);
> +
> +		for (i = 0; i < n - 1; ++i, ++tx_pkts, ++txdp)
> +			vtx1(txdp, *tx_pkts, flags);
> +
> +		vtx1(txdp, *tx_pkts++, rs);
> +
> +		nb_commit = (uint16_t)(nb_commit - n);
> +
> +		tx_id = 0;
> +		txq->tx_next_rs = (uint16_t)(txq->tx_rs_thresh - 1);
> +
> +		/* avoid reach the end of ring */
> +		txdp = &txq->tx_ring[tx_id];
> +		txep = &txq->sw_ring[tx_id];
> +	}
> +
> +	tx_backlog_entry(txep, tx_pkts, nb_commit);
> +
> +	vtx(txdp, tx_pkts, nb_commit, flags);
> +
> +	tx_id = (uint16_t)(tx_id + nb_commit);
> +	if (tx_id > txq->tx_next_rs) {
> +		txq->tx_ring[txq->tx_next_rs].cmd_type_offset_bsz |=
> +			rte_cpu_to_le_64(((uint64_t)I40E_TX_DESC_CMD_RS) <<
> +						I40E_TXD_QW1_CMD_SHIFT);
> +		txq->tx_next_rs =
> +			(uint16_t)(txq->tx_next_rs + txq->tx_rs_thresh);
> +	}
> +
> +	txq->tx_tail = tx_id;
> +
> +	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
> +
> +	return nb_pkts;
> +}
> +
> +void __attribute__((cold))
> +i40e_rx_queue_release_mbufs_vec(struct i40e_rx_queue *rxq) {
> +	_i40e_rx_queue_release_mbufs_vec(rxq);
> +}
> +
> +int __attribute__((cold))
> +i40e_rxq_vec_setup(struct i40e_rx_queue *rxq) {
> +	return i40e_rxq_vec_setup_default(rxq); }
> +
> +int __attribute__((cold))
> +i40e_txq_vec_setup(struct i40e_tx_queue __rte_unused *txq) {
> +	return 0;
> +}
> +
> +int __attribute__((cold))
> +i40e_rx_vec_dev_conf_condition_check(struct rte_eth_dev *dev) {
> +	return i40e_rx_vec_dev_conf_condition_check_default(dev);
> +}
> --
> 2.4.11

ptype and bad checksum offload is enabled with below patches 
http://dpdk.org/dev/patchwork/patch/16394
http://dpdk.org/dev/patchwork/patch/16395
You may take a look to see if it's necessary to enable them for ARM also.

Thanks!
Qi

^ permalink raw reply

* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Yuanhan Liu @ 2016-10-12  2:53 UTC (permalink / raw)
  To: Wang, Zhihong, Jianbo Liu; +Cc: Maxime Coquelin, dev@dpdk.org, Thomas Monjalon
In-Reply-To: <CAP4Qi392=aOMrSyTu-5qwpSLpwK-NVdHp-aztT-xT=BcRPWoew@mail.gmail.com>

On Thu, Sep 22, 2016 at 01:47:45PM +0800, Jianbo Liu wrote:
> On 22 September 2016 at 10:29, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> > On Wed, Sep 21, 2016 at 08:54:11PM +0800, Jianbo Liu wrote:
> >> >> > My setup consists of one host running a guest.
> >> >> > The guest generates as much 64bytes packets as possible using
> >> >>
> >> >> Have you tested with other different packet size?
> >> >> My testing shows that performance is dropping when packet size is more
> >> >> than 256.
> >> >
> >> >
> >> > Hi Jianbo,
> >> >
> >> > Thanks for reporting this.
> >> >
> >> >  1. Are you running the vector frontend with mrg_rxbuf=off?
> >> >
> Yes, my testing is mrg_rxbuf=off, but not vector frontend PMD.
> 
> >> >  2. Could you please specify what CPU you're running? Is it Haswell
> >> >     or Ivy Bridge?
> >> >
> It's an ARM server.
> 
> >> >  3. How many percentage of drop are you seeing?
> The testing result:
> size (bytes)     improvement (%)
> 64                   3.92
> 128                 11.51
> 256                  24.16
> 512                  -13.79
> 1024                -22.51
> 1500                -12.22
> A correction is that performance is dropping if byte size is larger than 512.

I have thought of this twice. Unfortunately, I think I may need NACK this
series.

Merging two code path into one is really good: as you stated, it improves
the maintainability. But only if we see no performance regression on both 
path after the refactor. Unfortunately, that's not the case here: it hurts
the performance for one code path (non-mrg Rx).

That makes me think we may should not do the code path merge at all. I think
that also aligns with what you have said before (internally): we could do the
merge if it gives comparable performance before and after that.

Besides that, I don't quite like the way you did in patch 2 (rewrite enqueue):
you made a lot of changes in one patch. That means if something wrong happened,
it is hard to narrow down which change introduces that regression. Badly,
that's exactly what we met here. Weeks have been passed, I see no progress.

That's the reason we like the idea of "one patch only does one thing, an
atomic thing".

So I will apply the first patch (it's a bug fixing patch) and ask you to
refactor the rest, without the code path merge.

I think we could still have a good maintainability code base if we introduce 
more common helper functions that can be used on both Rx path, or even on
Tx path (such as update_used_ring, or shadow_used_ring).

It's a bit late for too many changes for v16.11. I think you could just
grab patch 6 (vhost: optimize cache access) to the old mrg-Rx code path,
if that also helps the performance? Let us handle the left in next release,
such as shadow used ring.

Thanks.

	--yliu

^ permalink raw reply

* Re: [PATCH 1/5] i40e: extract non-x86 specific code from vector driver
From: Zhang, Qi Z @ 2016-10-12  2:55 UTC (permalink / raw)
  To: 'Jianbo Liu'
  Cc: Zhang, Helin, Wu, Jingjing, jerin.jacob@caviumnetworks.com,
	dev@dpdk.org
In-Reply-To: <1472032425-16136-2-git-send-email-jianbo.liu@linaro.org>

Hi Jianbo

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianbo Liu
> Sent: Wednesday, August 24, 2016 5:54 PM
> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; jerin.jacob@caviumnetworks.com; dev@dpdk.org
> Cc: Jianbo Liu <jianbo.liu@linaro.org>
> Subject: [dpdk-dev] [PATCH 1/5] i40e: extract non-x86 specific code from vector
> driver
> 
> move scalar code which does not use x86 intrinsic functions to new file
> "i40e_rxtx_vec_common.h", while keeping x86 code in i40e_rxtx_vec.c.
> This allows the scalar code to to be shared among vector drivers for different
> platforms.
> 
> Signed-off-by: Jianbo Liu <jianbo.liu@linaro.org>
> ---
>  drivers/net/i40e/i40e_rxtx_vec.c        | 184 +-----------------------
>  drivers/net/i40e/i40e_rxtx_vec_common.h | 239
> ++++++++++++++++++++++++++++++++
>  2 files changed, 243 insertions(+), 180 deletions(-)  create mode 100644
> drivers/net/i40e/i40e_rxtx_vec_common.h
> 
> diff --git a/drivers/net/i40e/i40e_rxtx_vec.c b/drivers/net/i40e/i40e_rxtx_vec.c
> index 51fb282..f847469 100644
> --- a/drivers/net/i40e/i40e_rxtx_vec.c
> +++ b/drivers/net/i40e/i40e_rxtx_vec.c
> @@ -39,6 +39,7 @@
>  #include "base/i40e_type.h"
>  #include "i40e_ethdev.h"
>  #include "i40e_rxtx.h"
> +#include "i40e_rxtx_vec_common.h"
> 
>  #include <tmmintrin.h>
> 
> @@ -421,68 +422,6 @@ i40e_recv_pkts_vec(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  	return _recv_raw_pkts_vec(rx_queue, rx_pkts, nb_pkts, NULL);  }
> 
> -static inline uint16_t
> -reassemble_packets(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_bufs,
> -		   uint16_t nb_bufs, uint8_t *split_flags)
> -{
> -	struct rte_mbuf *pkts[RTE_I40E_VPMD_RX_BURST]; /*finished pkts*/
> -	struct rte_mbuf *start = rxq->pkt_first_seg;
> -	struct rte_mbuf *end =  rxq->pkt_last_seg;
> -	unsigned pkt_idx, buf_idx;
> -
> -	for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) {
> -		if (end != NULL) {
> -			/* processing a split packet */
> -			end->next = rx_bufs[buf_idx];
> -			rx_bufs[buf_idx]->data_len += rxq->crc_len;
> -
> -			start->nb_segs++;
> -			start->pkt_len += rx_bufs[buf_idx]->data_len;
> -			end = end->next;
> -
> -			if (!split_flags[buf_idx]) {
> -				/* it's the last packet of the set */
> -				start->hash = end->hash;
> -				start->ol_flags = end->ol_flags;
> -				/* we need to strip crc for the whole packet */
> -				start->pkt_len -= rxq->crc_len;
> -				if (end->data_len > rxq->crc_len) {
> -					end->data_len -= rxq->crc_len;
> -				} else {
> -					/* free up last mbuf */
> -					struct rte_mbuf *secondlast = start;
> -
> -					while (secondlast->next != end)
> -						secondlast = secondlast->next;
> -					secondlast->data_len -= (rxq->crc_len -
> -							end->data_len);
> -					secondlast->next = NULL;
> -					rte_pktmbuf_free_seg(end);
> -					end = secondlast;
> -				}
> -				pkts[pkt_idx++] = start;
> -				start = end = NULL;
> -			}
> -		} else {
> -			/* not processing a split packet */
> -			if (!split_flags[buf_idx]) {
> -				/* not a split packet, save and skip */
> -				pkts[pkt_idx++] = rx_bufs[buf_idx];
> -				continue;
> -			}
> -			end = start = rx_bufs[buf_idx];
> -			rx_bufs[buf_idx]->data_len += rxq->crc_len;
> -			rx_bufs[buf_idx]->pkt_len += rxq->crc_len;
> -		}
> -	}
> -
> -	/* save the partial packet for next time */
> -	rxq->pkt_first_seg = start;
> -	rxq->pkt_last_seg = end;
> -	memcpy(rx_bufs, pkts, pkt_idx * (sizeof(*pkts)));
> -	return pkt_idx;
> -}
> -
>   /* vPMD receive routine that reassembles scattered packets
>   * Notice:
>   * - nb_pkts < RTE_I40E_DESCS_PER_LOOP, just return no packet @@
> -548,73 +487,6 @@ vtx(volatile struct i40e_tx_desc *txdp,
>  		vtx1(txdp, *pkt, flags);
>  }
> 
> -static inline int __attribute__((always_inline)) -i40e_tx_free_bufs(struct
> i40e_tx_queue *txq) -{
> -	struct i40e_tx_entry *txep;
> -	uint32_t n;
> -	uint32_t i;
> -	int nb_free = 0;
> -	struct rte_mbuf *m, *free[RTE_I40E_TX_MAX_FREE_BUF_SZ];
> -
> -	/* check DD bits on threshold descriptor */
> -	if ((txq->tx_ring[txq->tx_next_dd].cmd_type_offset_bsz &
> -			rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) !=
> -			rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
> -		return 0;
> -
> -	n = txq->tx_rs_thresh;
> -
> -	 /* first buffer to free from S/W ring is at index
> -	  * tx_next_dd - (tx_rs_thresh-1)
> -	  */
> -	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];
> -	m = __rte_pktmbuf_prefree_seg(txep[0].mbuf);
> -	if (likely(m != NULL)) {
> -		free[0] = m;
> -		nb_free = 1;
> -		for (i = 1; i < n; i++) {
> -			m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
> -			if (likely(m != NULL)) {
> -				if (likely(m->pool == free[0]->pool)) {
> -					free[nb_free++] = m;
> -				} else {
> -					rte_mempool_put_bulk(free[0]->pool,
> -							     (void *)free,
> -							     nb_free);
> -					free[0] = m;
> -					nb_free = 1;
> -				}
> -			}
> -		}
> -		rte_mempool_put_bulk(free[0]->pool, (void **)free, nb_free);
> -	} else {
> -		for (i = 1; i < n; i++) {
> -			m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
> -			if (m != NULL)
> -				rte_mempool_put(m->pool, m);
> -		}
> -	}
> -
> -	/* buffers were freed, update counters */
> -	txq->nb_tx_free = (uint16_t)(txq->nb_tx_free + txq->tx_rs_thresh);
> -	txq->tx_next_dd = (uint16_t)(txq->tx_next_dd + txq->tx_rs_thresh);
> -	if (txq->tx_next_dd >= txq->nb_tx_desc)
> -		txq->tx_next_dd = (uint16_t)(txq->tx_rs_thresh - 1);
> -
> -	return txq->tx_rs_thresh;
> -}
> -
> -static inline void __attribute__((always_inline)) -tx_backlog_entry(struct
> i40e_tx_entry *txep,
> -		 struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> -{
> -	int i;
> -
> -	for (i = 0; i < (int)nb_pkts; ++i)
> -		txep[i].mbuf = tx_pkts[i];
> -}
> -
>  uint16_t
>  i40e_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
>  		   uint16_t nb_pkts)
> @@ -685,37 +557,13 @@ i40e_xmit_pkts_vec(void *tx_queue, struct
> rte_mbuf **tx_pkts,  void __attribute__((cold))
> i40e_rx_queue_release_mbufs_vec(struct i40e_rx_queue *rxq)  {
> -	const unsigned mask = rxq->nb_rx_desc - 1;
> -	unsigned i;
> -
> -	if (rxq->sw_ring == NULL || rxq->rxrearm_nb >= rxq->nb_rx_desc)
> -		return;
> -
> -	/* free all mbufs that are valid in the ring */
> -	for (i = rxq->rx_tail; i != rxq->rxrearm_start; i = (i + 1) & mask)
> -		rte_pktmbuf_free_seg(rxq->sw_ring[i].mbuf);
> -	rxq->rxrearm_nb = rxq->nb_rx_desc;
> -
> -	/* set all entries to NULL */
> -	memset(rxq->sw_ring, 0, sizeof(rxq->sw_ring[0]) * rxq->nb_rx_desc);
> +	_i40e_rx_queue_release_mbufs_vec(rxq);
>  }
> 
>  int __attribute__((cold))
>  i40e_rxq_vec_setup(struct i40e_rx_queue *rxq)  {
> -	uintptr_t p;
> -	struct rte_mbuf mb_def = { .buf_addr = 0 }; /* zeroed mbuf */
> -
> -	mb_def.nb_segs = 1;
> -	mb_def.data_off = RTE_PKTMBUF_HEADROOM;
> -	mb_def.port = rxq->port_id;
> -	rte_mbuf_refcnt_set(&mb_def, 1);
> -
> -	/* prevent compiler reordering: rearm_data covers previous fields */
> -	rte_compiler_barrier();
> -	p = (uintptr_t)&mb_def.rearm_data;
> -	rxq->mbuf_initializer = *(uint64_t *)p;
> -	return 0;
> +	return i40e_rxq_vec_setup_default(rxq);
>  }
> 
>  int __attribute__((cold))
> @@ -728,34 +576,10 @@ int __attribute__((cold))
> i40e_rx_vec_dev_conf_condition_check(struct rte_eth_dev *dev)  {  #ifndef
> RTE_LIBRTE_IEEE1588
> -	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> -	struct rte_fdir_conf *fconf = &dev->data->dev_conf.fdir_conf;
> -
>  	/* need SSE4.1 support */
>  	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
>  		return -1;
> -
> -#ifndef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
> -	/* whithout rx ol_flags, no VP flag report */
> -	if (rxmode->hw_vlan_strip != 0 ||
> -	    rxmode->hw_vlan_extend != 0)
> -		return -1;
>  #endif
> 
> -	/* no fdir support */
> -	if (fconf->mode != RTE_FDIR_MODE_NONE)
> -		return -1;
> -
> -	 /* - no csum error report support
> -	 * - no header split support
> -	 */
> -	if (rxmode->hw_ip_checksum == 1 ||
> -	    rxmode->header_split == 1)
> -		return -1;
> -
> -	return 0;
> -#else
> -	RTE_SET_USED(dev);
> -	return -1;
> -#endif
> +	return i40e_rx_vec_dev_conf_condition_check_default(dev);
>  }
> diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h
> b/drivers/net/i40e/i40e_rxtx_vec_common.h
> new file mode 100644
> index 0000000..b31b39e
> --- /dev/null
> +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
> @@ -0,0 +1,239 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> + */
> +
> +#ifndef _I40E_RXTX_VEC_COMMON_H_
> +#define _I40E_RXTX_VEC_COMMON_H_
> +#include <stdint.h>
> +#include <rte_ethdev.h>
> +#include <rte_malloc.h>
> +
> +#include "i40e_ethdev.h"
> +#include "i40e_rxtx.h"
> +
> +static inline uint16_t
> +reassemble_packets(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_bufs,
> +		   uint16_t nb_bufs, uint8_t *split_flags) {
> +	struct rte_mbuf *pkts[RTE_I40E_VPMD_RX_BURST]; /*finished pkts*/
> +	struct rte_mbuf *start = rxq->pkt_first_seg;
> +	struct rte_mbuf *end =  rxq->pkt_last_seg;
> +	unsigned pkt_idx, buf_idx;
> +
> +	for (buf_idx = 0, pkt_idx = 0; buf_idx < nb_bufs; buf_idx++) {
> +		if (end != NULL) {
> +			/* processing a split packet */
> +			end->next = rx_bufs[buf_idx];
> +			rx_bufs[buf_idx]->data_len += rxq->crc_len;
> +
> +			start->nb_segs++;
> +			start->pkt_len += rx_bufs[buf_idx]->data_len;
> +			end = end->next;
> +
> +			if (!split_flags[buf_idx]) {
> +				/* it's the last packet of the set */
> +				start->hash = end->hash;
> +				start->ol_flags = end->ol_flags;
> +				/* we need to strip crc for the whole packet */
> +				start->pkt_len -= rxq->crc_len;
> +				if (end->data_len > rxq->crc_len) {
> +					end->data_len -= rxq->crc_len;
> +				} else {
> +					/* free up last mbuf */
> +					struct rte_mbuf *secondlast = start;
> +
> +					while (secondlast->next != end)
> +						secondlast = secondlast->next;
> +					secondlast->data_len -= (rxq->crc_len -
> +							end->data_len);
> +					secondlast->next = NULL;
> +					rte_pktmbuf_free_seg(end);
> +					end = secondlast;
> +				}
> +				pkts[pkt_idx++] = start;
> +				start = end = NULL;
> +			}
> +		} else {
> +			/* not processing a split packet */
> +			if (!split_flags[buf_idx]) {
> +				/* not a split packet, save and skip */
> +				pkts[pkt_idx++] = rx_bufs[buf_idx];
> +				continue;
> +			}
> +			end = start = rx_bufs[buf_idx];
> +			rx_bufs[buf_idx]->data_len += rxq->crc_len;
> +			rx_bufs[buf_idx]->pkt_len += rxq->crc_len;
> +		}
> +	}
> +
> +	/* save the partial packet for next time */
> +	rxq->pkt_first_seg = start;
> +	rxq->pkt_last_seg = end;
> +	memcpy(rx_bufs, pkts, pkt_idx * (sizeof(*pkts)));
> +	return pkt_idx;
> +}
> +
> +static inline int __attribute__((always_inline))
> +i40e_tx_free_bufs(struct i40e_tx_queue *txq) {
> +	struct i40e_tx_entry *txep;
> +	uint32_t n;
> +	uint32_t i;
> +	int nb_free = 0;
> +	struct rte_mbuf *m, *free[RTE_I40E_TX_MAX_FREE_BUF_SZ];
> +
> +	/* check DD bits on threshold descriptor */
> +	if ((txq->tx_ring[txq->tx_next_dd].cmd_type_offset_bsz &
> +			rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) !=
> +			rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
> +		return 0;
> +
> +	n = txq->tx_rs_thresh;
> +
> +	 /* first buffer to free from S/W ring is at index
> +	  * tx_next_dd - (tx_rs_thresh-1)
> +	  */
> +	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];
> +	m = __rte_pktmbuf_prefree_seg(txep[0].mbuf);
> +	if (likely(m != NULL)) {
> +		free[0] = m;
> +		nb_free = 1;
> +		for (i = 1; i < n; i++) {
> +			m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
> +			if (likely(m != NULL)) {
> +				if (likely(m->pool == free[0]->pool)) {
> +					free[nb_free++] = m;
> +				} else {
> +					rte_mempool_put_bulk(free[0]->pool,
> +							     (void *)free,
> +							     nb_free);
> +					free[0] = m;
> +					nb_free = 1;
> +				}
> +			}
> +		}
> +		rte_mempool_put_bulk(free[0]->pool, (void **)free, nb_free);
> +	} else {
> +		for (i = 1; i < n; i++) {
> +			m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
> +			if (m != NULL)
> +				rte_mempool_put(m->pool, m);
> +		}
> +	}
> +
> +	/* buffers were freed, update counters */
> +	txq->nb_tx_free = (uint16_t)(txq->nb_tx_free + txq->tx_rs_thresh);
> +	txq->tx_next_dd = (uint16_t)(txq->tx_next_dd + txq->tx_rs_thresh);
> +	if (txq->tx_next_dd >= txq->nb_tx_desc)
> +		txq->tx_next_dd = (uint16_t)(txq->tx_rs_thresh - 1);
> +
> +	return txq->tx_rs_thresh;
> +}
> +
> +static inline void __attribute__((always_inline))
> +tx_backlog_entry(struct i40e_tx_entry *txep,
> +		 struct rte_mbuf **tx_pkts, uint16_t nb_pkts) {
> +	int i;
> +
> +	for (i = 0; i < (int)nb_pkts; ++i)
> +		txep[i].mbuf = tx_pkts[i];
> +}
> +
> +static inline void
> +_i40e_rx_queue_release_mbufs_vec(struct i40e_rx_queue *rxq) {
> +	const unsigned mask = rxq->nb_rx_desc - 1;
> +	unsigned i;
> +
> +	if (rxq->sw_ring == NULL || rxq->rxrearm_nb >= rxq->nb_rx_desc)
> +		return;
> +
> +	/* free all mbufs that are valid in the ring */
> +	for (i = rxq->rx_tail; i != rxq->rxrearm_start; i = (i + 1) & mask)
> +		rte_pktmbuf_free_seg(rxq->sw_ring[i].mbuf);
> +	rxq->rxrearm_nb = rxq->nb_rx_desc;
> +
> +	/* set all entries to NULL */
> +	memset(rxq->sw_ring, 0, sizeof(rxq->sw_ring[0]) * rxq->nb_rx_desc); }
> +
> +static inline int
> +i40e_rxq_vec_setup_default(struct i40e_rx_queue *rxq) {
> +	uintptr_t p;
> +	struct rte_mbuf mb_def = { .buf_addr = 0 }; /* zeroed mbuf */
> +
> +	mb_def.nb_segs = 1;
> +	mb_def.data_off = RTE_PKTMBUF_HEADROOM;
> +	mb_def.port = rxq->port_id;
> +	rte_mbuf_refcnt_set(&mb_def, 1);
> +
> +	/* prevent compiler reordering: rearm_data covers previous fields */
> +	rte_compiler_barrier();
> +	p = (uintptr_t)&mb_def.rearm_data;
> +	rxq->mbuf_initializer = *(uint64_t *)p;
> +	return 0;
> +}
> +
> +static inline int
> +i40e_rx_vec_dev_conf_condition_check_default(struct rte_eth_dev *dev) {
> +#ifndef RTE_LIBRTE_IEEE1588
> +	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> +	struct rte_fdir_conf *fconf = &dev->data->dev_conf.fdir_conf;
> +
> +#ifndef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
> +	/* whithout rx ol_flags, no VP flag report */
> +	if (rxmode->hw_vlan_strip != 0 ||
> +	    rxmode->hw_vlan_extend != 0)
> +		return -1;
> +#endif
> +
> +	/* no fdir support */
> +	if (fconf->mode != RTE_FDIR_MODE_NONE)
> +		return -1;
> +
> +	 /* - no csum error report support
> +	 * - no header split support
> +	 */
> +	if (rxmode->hw_ip_checksum == 1 ||
> +	    rxmode->header_split == 1)
> +		return -1;
> +
> +	return 0;
> +#else
> +	RTE_SET_USED(dev);
> +	return -1;
> +#endif
> +}
> +#endif
> --
> 2.4.11

Should we rename the function "_40e_rx_queue_release_mbufs_vec" to
"i40e_rx_queue_release_mbufs_vec_default", so functions be wrapped can follow a consistent rule?  
Thanks!
Qi

^ permalink raw reply

* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Yuanhan Liu @ 2016-10-12  3:21 UTC (permalink / raw)
  To: Michael S. Tsirkin, Thomas Monjalon
  Cc: Wang, Zhihong, Maxime Coquelin, Stephen Hemminger, dev@dpdk.org,
	qemu-devel@nongnu.org
In-Reply-To: <20161011065749.GO1597@yliu-dev.sh.intel.com>

On Tue, Oct 11, 2016 at 02:57:49PM +0800, Yuanhan Liu wrote:
> > > > > > There was an example: the vhost enqueue optmization patchset from
> > > > > > Zhihong [0] uses memset, and it introduces more than 15% drop (IIRC)

Though it doesn't matter now, but I have verified it yesterday (with and
wihtout memset), the drop could be up to 30+%.

This is to let you know that it could behaviour badly if memset is not
inlined.

> > > > > > on my Ivybridge server: it has no such issue on his server though.
> > > > > >
> > > > > > [0]: http://dpdk.org/ml/archives/dev/2016-August/045272.html
> > > > > >
> > > > > > 	--yliu
> > > > >
> > > > > I'd say that's weird. what's your config? any chance you
> > > > > are using an old compiler?
> > > > 
> > > > Not really, it's gcc 5.3.1. Maybe Zhihong could explain more. IIRC,
> > > > he said the memset is not well optimized for Ivybridge server.
> > > 
> > > The dst is remote in that case. It's fine on Haswell but has complication
> > > in Ivy Bridge which (wasn't supposed to but) causes serious frontend issue.
> > > 
> > > I don't think gcc inlined it there. I'm using fc24 gcc 6.1.1.
> > 
> > 
> > So try something like this then:
> 
> Yes, I saw memset is inlined when this diff is applied.

I have another concern though: It's a trick could let gcc do the inline,
I am not quite sure whether that's ture with other compilers (i.e. clang,
icc, or even, older gcc).

For this case, I think I still prefer some trick like
    *(struct ..*) = {0, }

Or even, we may could introduce rte_memset(). IIRC, that has been
proposed somehow before?

	--yliu

^ permalink raw reply

* Re: [PATCH 1/3] eal/drivers: prefix driver REGISTER macros with EAL
From: Shreyansh Jain @ 2016-10-12  4:29 UTC (permalink / raw)
  To: Neil Horman, Thomas Monjalon; +Cc: david.marchand@6wind.com, dev
In-Reply-To: <20161011145718.GB31866@hmsreliant.think-freely.org>

On Tuesday 11 October 2016 08:27 PM, Neil Horman wrote:
> On Tue, Oct 11, 2016 at 03:57:29PM +0200, Thomas Monjalon wrote:
>> 2016-10-11 09:38, Neil Horman:
>>> This also begs the question in my mind, is it really worth changing the macro?
>>> I really don't think it is.  The registration macros are pretty descriptive as
>>> they stand, and have already changed 3 or 4 times in the last 6 months, which
>>> suggests to me that any change here is really just churn more than meaningful
>>> change.  You can make the argument that the name might be more in line with the
>>> library its implemented in or what not, but in truth, its easy to understand
>>> what the macros do (in their previous or current incantations), and any change
>>> that just makes them the same as other macros in their naming is really more
>>> trouble than its worth.
>>
>> Neil, the long term goal is to stop having some identifiers which do not
>> start with RTE_ in our exported .h files.
>> I think it is a reasonable policy, for a library, to live in a well defined
>> namespace.

Understood and agreed.

>>
>
> I don't disagree that a consistent namespace is a nice thing, only that we've
> had 3 changes to these macros in the last few months, none of which have really
> moved us toward that goal.
>
> At least we can agree that the EAL_ macro being proposed isn't the right thing
> to do regardless of motivation :)

Macro proposed by this patch is not EAL_*. Thomas had already suggested 
that change for v1; v3 changes it to RTE_PMD_REGISTER_*.

--->8---
  DRIVER_REGISTER_PCI -> RTE_PMD_REGISTER_PCI
  DRIVER_REGISTER_PCI_TABLE -> RTE_PMD_REGISTER_PCI_TABLE
  DRIVER_REGISTER_VDEV -> RTE_PMD_REGISTER_VDEV
  DRIVER_REGISTER_PARAM_STRING -> RTE_PMD_REGISTER_PARAM_STRING
  DRIVER_EXPORT_* -> RTE_PMD_EXPORT_*
--->8---

-
Shreyansh

^ permalink raw reply

* Re: Project Governance and Linux Foundation
From: qin.chunhua @ 2016-10-12  5:44 UTC (permalink / raw)
  To: dev, users, tim.odriscoll

ZTE is supportive of improving the DPDK project governance including 
moving the project to the Linux Foundation. 
DPDK has been used in lots of ZTE's equipments and solutions  such as 
BBU,RNC,EPC,vEPC,vBRAS,vCPE,vRouter,vSwitch,and so on for many years. 
We have also done some optimization in DPDK. If  the DPDK  project 
governance moves to the Linux Foundation,we think this will promote ZTE 
and other companies to contribute in DPDK. 
 
Thanks
Chunhua
 
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of O'Driscoll, Tim
Sent: Monday, October 10, 2016 4:33 PM
To: dev@dpdk.org; users@dpdk.org
Subject: [dpdk-dev] Project Governance and Linux Foundation
 
This email is being sent on behalf of: Cavium, Cisco, Intel, NXP & Red 
Hat.
 
 
Since its creation as an open source project in 2013, DPDK has grown 
significantly. The number of DPDK users, contributors, commercial products 
that use DPDK and open source projects that depend on it have all 
increased consistently over that time. DPDK is now a key ingredient in 
networking and NFV, and we need to ensure that the project structure and 
governance are appropriate for such a critical project, and that they 
facilitate the project's continued growth.
 
For over a year now we've been discussing moving DPDK to the Linux 
Foundation. We believe it's now time to conclude that discussion and make 
the move. The benefits of doing this would include:
- The infrastructure for a project like DPDK should not be owned and 
controlled by any single company.
- Remove any remaining perception that DPDK is not truly open.
- Allow the project to avail of the infrastructure and services provided 
by the Linux Foundation. These include things like: Ability to host 
infrastructure for integration and testing (the FD.io CSIT lab is an 
example of this - see https://wiki.fd.io/view/CSIT/CSIT_LF_testbed); 
Support for legal issues including trademarks and branding, and the 
ability to sign agreements on behalf of the project; Ability to pool 
resources for events and brand promotion; Safe haven for community IP 
resources.
 
We don't propose to debate the details here. Instead, an open discussion 
session on DPDK Project Growth has been included in the agenda for the 
DPDK Summit Userspace 2016 event in Dublin. We propose using that session 
to agree that the DPDK project will move to the Linux Foundation, and then 
to move on to discussing the specifics. Things that we'll need to consider 
include:
- Whether DPDK moves to the Linux Foundation as an independent project or 
as part of a larger project like FD.io.
- Creation of a project charter similar to those created for FD.io (
https://fd.io/governance/technical-community-charter) and Open vSwitch 
(see 
http://openvswitch.org/pipermail/discuss/attachments/20160619/5a2df53e/attachment-0001.pdf
).
- Agreement on budget, membership levels etc. A draft budget was created 
by the LF during previous discussions (
https://docs.google.com/spreadsheets/d/1-3686Xb_jf4FtxdX8Mus9UwIxUb2vI_ppmJV5GnXcLg/edit#gid=302618256
), but it is possible to adopt an even more lightweight model.
 
We could look at alternatives to the Linux Foundation, but a) we've been 
talking to the LF for over a year now, and b) the preponderance of 
networking projects in LF, like ODL, FD.io, and OVS, makes it a natural 
destination for DPDK.
 
As highlighted in previous discussions on this topic, it's important to 
stress that the intent is not to make significant changes to the technical 
governance and decision making of the project. The project has a strong 
set of maintainers and a Technical Board in place already. What's required 
is to supplement that with an open governance structure taking advantage 
of the services offered by the Linux Foundation.
 
The purpose of this email is to outline what we want to achieve during 
that discussion session in Dublin, and to allow people to consider the 
issue and prepare in advance. If people want to comment via email on the 
mailing list, that's obviously fine, but we believe that an open and frank 
discussion when people meet in person in Dublin is the best way to 
progress this.
 
 
For reference, below is a brief history of the previous discussions on 
this topic:
 
September 2015:
- A DPDK community call was held to discuss project growth and possible 
improvements. This was the first public discussion on possible governance 
changes. The agreed next step was to discuss this in more detail at the 
2015 DPDK Summit Userspace event Dublin. Minutes of the call are at: 
http://dpdk.org/ml/archives/dev/2015-September/024120.html.
 
October 2015:
- An open discussion session on project governance was held at the 2015 
DPDK Summit Userspace event. For technical governance, we agreed to 
investigate creating a technical steering committee. For non-technical 
governance (including things like event planning, legal and trademark 
issues, hosting of the website etc.), we agreed to work with the Linux 
Foundation on a proposal for a lightweight governance model for DPDK. 
Minutes of the discussion are at: 
http://dpdk.org/ml/archives/dev/2015-October/024825.html.
 
- The proposal for a technical steering committee was subsequently 
discussed on the mailing list (
http://dpdk.org/ml/archives/dev/2015-October/026598.html) and agreed, 
leading to the creation of the DPDK Technical Board (
http://dpdk.org/dev#board).
 
December 2015:
- A community call was held to discuss migration to the Linux Foundation. 
Mike Dolan (VP of Strategic Programs at The Linux Foundation) gave an 
overview of the LF and the services they can provide. We agreed to form a 
small sub-team (Dave Neary, Thomas Monjalon, Stephen Hemminger, Tim 
O'Driscoll) to work with the LF on a more detailed proposal. Minutes of 
the call are at: http://dpdk.org/ml/archives/dev/2015-December/030532.html
.
 
February 2016:
- A community call was held to discuss the LF budget proposal (see 
https://docs.google.com/spreadsheets/d/1-3686Xb_jf4FtxdX8Mus9UwIxUb2vI_ppmJV5GnXcLg/edit#gid=302618256
). We agreed to discuss this further on the dev mailing list due to 
limited attendance on the call. Minutes of the call are at: 
http://dpdk.org/ml/archives/dev/2016-February/032720.html.
 
- A request was made on the dev and announce mailing lists too determine 
who supported the proposal to move to the Linux Foundation (
http://dpdk.org/ml/archives/dev/2016-February/033192.html). There was 
public support from Intel (
http://dpdk.org/ml/archives/dev/2016-February/033297.html) and Brocade (
http://dpdk.org/ml/archives/dev/2016-February/033359.html). 6WIND 
requested postponing the move for a few months (
http://dpdk.org/ml/archives/dev/2016-February/033299.html).
 
- The Fast Data (FD.io) project was established under the Linux Foundation 
(
https://fd.io/news/announcement/2016/02/linux-foundation-forms-open-source-effort-advance-io-services
).
 
June 2016:
- The Open vSwitch project proposed moving to the Linux Foundation (
http://openvswitch.org/pipermail/discuss/2016-June/021761.html).
 
August 2016:
- The Open vSwitch project moved to the Linux Foundation (
https://www.linuxfoundation.org/announcements/open-vswitch-joins-linux-foundation-open-networking-ecosystem
).
 

^ permalink raw reply

* [PATCH v3] test_cryptodev_perf: IV and digest should be stored at a DMAeble address
From: akhil.goyal @ 2016-10-12 11:16 UTC (permalink / raw)
  To: arkadiuszx.kusztal, declan.doherty
  Cc: john.griffin, fiona.trahe, deepak.k.jain, dev, Akhil Goyal
In-Reply-To: <20161007170608.27685-1-akhil.goyal@nxp.com>

From: Akhil Goyal <akhil.goyal@nxp.com>

For physical crypto devices, IV and digest are processed by the crypto
device which need the contents to be written on some DMA able address.

So in order to do that, IV and digest are accomodated in the packet.

Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
v2: patch rebased
v3: fix broken compilation
---
 app/test/test_cryptodev_perf.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c
index 4aee9af..d498195 100644
--- a/app/test/test_cryptodev_perf.c
+++ b/app/test/test_cryptodev_perf.c
@@ -2722,9 +2722,12 @@ test_perf_set_crypto_op_aes(struct rte_crypto_op *op, struct rte_mbuf *m,
 	op->sym->auth.aad.length = AES_CIPHER_IV_LENGTH;
 
 	/* Cipher Parameters */
-	op->sym->cipher.iv.data = aes_iv;
+	op->sym->cipher.iv.data = (uint8_t *)m->buf_addr + m->data_off;
+	op->sym->cipher.iv.phys_addr = rte_pktmbuf_mtophys(m);
 	op->sym->cipher.iv.length = AES_CIPHER_IV_LENGTH;
 
+	rte_memcpy(op->sym->cipher.iv.data, aes_iv, AES_CIPHER_IV_LENGTH);
+
 	/* Data lengths/offsets Parameters */
 	op->sym->auth.data.offset = 0;
 	op->sym->auth.data.length = data_len;
@@ -2893,7 +2896,9 @@ test_perf_aes_sha(uint8_t dev_id, uint16_t queue_id,
 				rte_pktmbuf_free(mbufs[k]);
 			return -1;
 		}
-
+		/* Make room for Digest and IV in mbuf */
+		rte_pktmbuf_append(mbufs[i], digest_length);
+		rte_pktmbuf_prepend(mbufs[i], AES_CIPHER_IV_LENGTH);
 	}
 
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 0/3] fix flow director mask issues
From: Wenzhuo Lu @ 2016-10-12  6:24 UTC (permalink / raw)
  To: dev
In-Reply-To: <1476085594-32325-1-git-send-email-wenzhuo.lu@intel.com>

There're 2 issues about flow director mask.
1, MAC address mask is not supported in mac-vlan mode.
2, All the parameter are defined as big endian, but
   they're not treated approriately.
This patch set is used to fix these 2 issues.

v2:
- Re-split the patches and reword commit log. No code change.

Wenzhuo Lu (3):
  ixgbe: fix wrong flow director mask
  app/testpmd: fix wrong flow director mask
  app/testpmd: fix flow director endian issue

 app/test-pmd/cmdline.c                      | 11 +++-----
 app/test-pmd/config.c                       | 43 +++++++++++++++++------------
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  3 +-
 drivers/net/ixgbe/ixgbe_fdir.c              | 10 +++----
 4 files changed, 35 insertions(+), 32 deletions(-)

-- 
1.9.3

^ permalink raw reply

* [PATCH v2 1/3] ixgbe: fix wrong flow director mask
From: Wenzhuo Lu @ 2016-10-12  6:24 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu
In-Reply-To: <1476253462-13000-1-git-send-email-wenzhuo.lu@intel.com>

In mac-vlan mode, MAC address mask is not supported by HW.
The MAC address mask should not be set in mac-vlan mode.
Instead, only set it in tunnel mode.

Fixes: 82fb702077f6 ("ixgbe: support new flow director modes for X550")
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_fdir.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_fdir.c b/drivers/net/ixgbe/ixgbe_fdir.c
index 861c7cb..4b81ee3 100644
--- a/drivers/net/ixgbe/ixgbe_fdir.c
+++ b/drivers/net/ixgbe/ixgbe_fdir.c
@@ -432,12 +432,12 @@ fdir_set_input_mask_x550(struct rte_eth_dev *dev,
 		fdiripv6m |= IXGBE_FDIRIP6M_TUNNEL_TYPE |
 				IXGBE_FDIRIP6M_TNI_VNI;
 
-	mac_mask = input_mask->mac_addr_byte_mask;
-	fdiripv6m |= (mac_mask << IXGBE_FDIRIP6M_INNER_MAC_SHIFT)
-			& IXGBE_FDIRIP6M_INNER_MAC;
-	info->mask.mac_addr_byte_mask = input_mask->mac_addr_byte_mask;
-
 	if (mode == RTE_FDIR_MODE_PERFECT_TUNNEL) {
+		mac_mask = input_mask->mac_addr_byte_mask;
+		fdiripv6m |= (mac_mask << IXGBE_FDIRIP6M_INNER_MAC_SHIFT)
+				& IXGBE_FDIRIP6M_INNER_MAC;
+		info->mask.mac_addr_byte_mask = input_mask->mac_addr_byte_mask;
+
 		switch (input_mask->tunnel_type_mask) {
 		case 0:
 			/* Mask turnnel type */
-- 
1.9.3

^ permalink raw reply related

* [PATCH v2 3/3] app/testpmd: fix flow director endian issue
From: Wenzhuo Lu @ 2016-10-12  6:24 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu
In-Reply-To: <1476253462-13000-1-git-send-email-wenzhuo.lu@intel.com>

The vlan mask and tunnel id mask of flow director
are defined as big endian. So they should be
converted.
When the mask is printed, the parameters are not
converted either. This patch converts the mask
parameters.
Some lines of the mask print are too long, split
them to more lines.

Fixes: 7c554b4f0484 ("app/testpmd: update display of flow director information")
Fixes: 53b2bb9b7ea7 ("app/testpmd: new flow director commands")
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/cmdline.c |  6 +++---
 app/test-pmd/config.c  | 34 +++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 259090a..7c5c5e7 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -9074,16 +9074,16 @@ cmd_flow_director_mask_parsed(void *parsed_result,
 			return;
 		}
 
-		mask->vlan_tci_mask = res->vlan_mask;
+		mask->vlan_tci_mask = rte_cpu_to_be_16(res->vlan_mask);
 	} else if (fdir_conf.mode ==  RTE_FDIR_MODE_PERFECT_TUNNEL) {
 		if (strcmp(res->mode_value, "Tunnel")) {
 			printf("Please set mode to Tunnel.\n");
 			return;
 		}
 
-		mask->vlan_tci_mask = res->vlan_mask;
+		mask->vlan_tci_mask = rte_cpu_to_be_16(res->vlan_mask);
 		mask->mac_addr_byte_mask = res->mac_addr_byte_mask;
-		mask->tunnel_id_mask = res->tunnel_id_mask;
+		mask->tunnel_id_mask = rte_cpu_to_be_32(res->tunnel_id_mask);
 		mask->tunnel_type_mask = res->tunnel_type_mask;
 	} else {
 		if (strcmp(res->mode_value, "IP")) {
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 238c4fd..742a8d8 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2045,25 +2045,33 @@ set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_value)
 static inline void
 print_fdir_mask(struct rte_eth_fdir_masks *mask)
 {
-	printf("\n    vlan_tci: 0x%04x", mask->vlan_tci_mask);
+	printf("\n    vlan_tci: 0x%04x", rte_be_to_cpu_16(mask->vlan_tci_mask));
 
 	if (fdir_conf.mode == RTE_FDIR_MODE_PERFECT_TUNNEL)
 		printf(", mac_addr: 0x%02x, tunnel_type: 0x%01x,"
 			" tunnel_id: 0x%08x",
 			mask->mac_addr_byte_mask, mask->tunnel_type_mask,
-			mask->tunnel_id_mask);
+			rte_be_to_cpu_32(mask->tunnel_id_mask));
 	else if (fdir_conf.mode != RTE_FDIR_MODE_PERFECT_MAC_VLAN) {
-		printf(", src_ipv4: 0x%08x, dst_ipv4: 0x%08x,"
-			" src_port: 0x%04x, dst_port: 0x%04x",
-			mask->ipv4_mask.src_ip, mask->ipv4_mask.dst_ip,
-			mask->src_port_mask, mask->dst_port_mask);
-
-		printf("\n    src_ipv6: 0x%08x,0x%08x,0x%08x,0x%08x,"
-			" dst_ipv6: 0x%08x,0x%08x,0x%08x,0x%08x",
-			mask->ipv6_mask.src_ip[0], mask->ipv6_mask.src_ip[1],
-			mask->ipv6_mask.src_ip[2], mask->ipv6_mask.src_ip[3],
-			mask->ipv6_mask.dst_ip[0], mask->ipv6_mask.dst_ip[1],
-			mask->ipv6_mask.dst_ip[2], mask->ipv6_mask.dst_ip[3]);
+		printf(", src_ipv4: 0x%08x, dst_ipv4: 0x%08x",
+			rte_be_to_cpu_32(mask->ipv4_mask.src_ip),
+			rte_be_to_cpu_32(mask->ipv4_mask.dst_ip));
+
+		printf("\n    src_port: 0x%04x, dst_port: 0x%04x",
+			rte_be_to_cpu_16(mask->src_port_mask),
+			rte_be_to_cpu_16(mask->dst_port_mask));
+
+		printf("\n    src_ipv6: 0x%08x,0x%08x,0x%08x,0x%08x",
+			rte_be_to_cpu_32(mask->ipv6_mask.src_ip[0]),
+			rte_be_to_cpu_32(mask->ipv6_mask.src_ip[1]),
+			rte_be_to_cpu_32(mask->ipv6_mask.src_ip[2]),
+			rte_be_to_cpu_32(mask->ipv6_mask.src_ip[3]));
+
+		printf("\n    dst_ipv6: 0x%08x,0x%08x,0x%08x,0x%08x",
+			rte_be_to_cpu_32(mask->ipv6_mask.dst_ip[0]),
+			rte_be_to_cpu_32(mask->ipv6_mask.dst_ip[1]),
+			rte_be_to_cpu_32(mask->ipv6_mask.dst_ip[2]),
+			rte_be_to_cpu_32(mask->ipv6_mask.dst_ip[3]));
 	}
 
 	printf("\n");
-- 
1.9.3

^ permalink raw reply related

* [PATCH v2 2/3] app/testpmd: fix wrong flow director mask
From: Wenzhuo Lu @ 2016-10-12  6:24 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu
In-Reply-To: <1476253462-13000-1-git-send-email-wenzhuo.lu@intel.com>

In mac-vlan mode, MAC address mask is not supported by HW.
The MAC address mask should not be set in mac-vlan mode.
Remove this parameter from the CLI. Remove MAC address
from mask print too.

Fixes: 53b2bb9b7ea7 ("app/testpmd: new flow director commands")
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/cmdline.c                      |  5 +----
 app/test-pmd/config.c                       | 13 ++++++-------
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  3 +--
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 6e95ca2..259090a 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -720,7 +720,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set flow director IP mask.\n\n"
 
 			"flow_director_mask (port_id) mode MAC-VLAN"
-			" vlan (vlan_value) mac (mac_value)\n"
+			" vlan (vlan_value)\n"
 			"    Set flow director MAC-VLAN mask.\n\n"
 
 			"flow_director_mask (port_id) mode Tunnel"
@@ -9075,7 +9075,6 @@ cmd_flow_director_mask_parsed(void *parsed_result,
 		}
 
 		mask->vlan_tci_mask = res->vlan_mask;
-		mask->mac_addr_byte_mask = res->mac_addr_byte_mask;
 	} else if (fdir_conf.mode ==  RTE_FDIR_MODE_PERFECT_TUNNEL) {
 		if (strcmp(res->mode_value, "Tunnel")) {
 			printf("Please set mode to Tunnel.\n");
@@ -9206,8 +9205,6 @@ cmdline_parse_inst_t cmd_set_flow_director_mac_vlan_mask = {
 		(void *)&cmd_flow_director_mask_mode_mac_vlan,
 		(void *)&cmd_flow_director_mask_vlan,
 		(void *)&cmd_flow_director_mask_vlan_value,
-		(void *)&cmd_flow_director_mask_mac,
-		(void *)&cmd_flow_director_mask_mac_value,
 		NULL,
 	},
 };
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 83bebfe..238c4fd 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2045,16 +2045,15 @@ set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_value)
 static inline void
 print_fdir_mask(struct rte_eth_fdir_masks *mask)
 {
-	printf("\n    vlan_tci: 0x%04x, ", mask->vlan_tci_mask);
+	printf("\n    vlan_tci: 0x%04x", mask->vlan_tci_mask);
 
-	if (fdir_conf.mode == RTE_FDIR_MODE_PERFECT_MAC_VLAN)
-		printf("mac_addr: 0x%02x", mask->mac_addr_byte_mask);
-	else if (fdir_conf.mode == RTE_FDIR_MODE_PERFECT_TUNNEL)
-		printf("mac_addr: 0x%02x, tunnel_type: 0x%01x, tunnel_id: 0x%08x",
+	if (fdir_conf.mode == RTE_FDIR_MODE_PERFECT_TUNNEL)
+		printf(", mac_addr: 0x%02x, tunnel_type: 0x%01x,"
+			" tunnel_id: 0x%08x",
 			mask->mac_addr_byte_mask, mask->tunnel_type_mask,
 			mask->tunnel_id_mask);
-	else {
-		printf("src_ipv4: 0x%08x, dst_ipv4: 0x%08x,"
+	else if (fdir_conf.mode != RTE_FDIR_MODE_PERFECT_MAC_VLAN) {
+		printf(", src_ipv4: 0x%08x, dst_ipv4: 0x%08x,"
 			" src_port: 0x%04x, dst_port: 0x%04x",
 			mask->ipv4_mask.src_ip, mask->ipv4_mask.dst_ip,
 			mask->src_port_mask, mask->dst_port_mask);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f87e0c2..ed04532 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1844,8 +1844,7 @@ Set flow director's input masks::
                       src_mask (ipv4_src) (ipv6_src) (src_port) \
                       dst_mask (ipv4_dst) (ipv6_dst) (dst_port)
 
-   flow_director_mask (port_id) mode MAC-VLAN vlan (vlan_value) \
-                      mac (mac_value)
+   flow_director_mask (port_id) mode MAC-VLAN vlan (vlan_value)
 
    flow_director_mask (port_id) mode Tunnel vlan (vlan_value) \
                       mac (mac_value) tunnel-type (tunnel_type_value) \
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH v2] log: respect rte_openlog_stream calls before rte_eal_init
From: Thomas Monjalon @ 2016-10-12  7:09 UTC (permalink / raw)
  To: John Ousterhout; +Cc: dev
In-Reply-To: <CAGXJAmx-OsCvwwrxp6gMT+s9PcCSN0zvzaO7UU3T9o4aE0g_9Q@mail.gmail.com>

2016-10-11 14:46, John Ousterhout:
> All of your suggestions look reasonable and fairly straightforward; I'll
> work on a new patch that includes them.
> 
> Given that rte_eal_log_init is a no-op (and won't even be invoked), would
> it be better to remove that function completely, and even delete the file
> containing it (eal_log.c), or is it better to retain the empty function in
> order to maintain a parallel structure with Linux? Personally I'd lean
> towards deleting the file. As it stands, the interface to that function
> doesn't even make sense for BSD; the arguments were chosen for Linux and
> are ignored in BSD.
> 
> Let me know your preference.

Yes you can remove the file.

^ permalink raw reply

* Re: ZUC PMD as shared library
From: Thomas Monjalon @ 2016-10-12  7:22 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: dev
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA07CC6@IRSMSX108.ger.corp.intel.com>

2016-10-12 02:23, De Lara Guarch, Pablo:
> Hi Thomas,
> 
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 
> > Hi Pablo,
> > 
> > You are probably aware of the issue, but I would like to make it clear
> > in case someone else run into the same trouble:
> > 
> > It is impossible to build the ZUC crypto PMD as a shared library:
> > 
> > libsso-zuc-0.1.1/build/libsso_zuc.a(sso_zuc_yasm.o):
> > 	relocation R_X86_64_32 against `EK_d' can not be used
> > 	when making a shared object; recompile with -fPIC
> > 
> > The library libsso-zuc-0.1.1 needs an update to make the asm code
> > relocatable.
> > Should we explicit this limitation in the PMD doc?
> 
> Sorry for not replying to this earlier.
> Yes, you are right, thanks for pointing it out.
> I think the same problem is in KASUMI, so I will send a doc update for both PMDs.

No, KASUMI works as shared library, because there is no asm code.

Documenting the bug would be nice as a first step.
But it is a serious bug, so a fix in ZUC library is highly desirable.

^ permalink raw reply

* Re: Project Governance and Linux Foundation
From: Thomas Monjalon @ 2016-10-12  7:43 UTC (permalink / raw)
  To: qin.chunhua; +Cc: dev, users
In-Reply-To: <OF3421A464.9F382155-ON4825804A.001F4E8F-4825804A.001F880D@zte.com.cn>

Hi,

This is a really interesting use case.
Questions below,

2016-10-12 13:44, qin.chunhua@zte.com.cn:
> ZTE is supportive of improving the DPDK project governance including 
> moving the project to the Linux Foundation. 
> DPDK has been used in lots of ZTE's equipments and solutions  such as 
> BBU,RNC,EPC,vEPC,vBRAS,vCPE,vRouter,vSwitch,and so on for many years. 
> We have also done some optimization in DPDK.

You mean you made some optimizations to DPDK but keep them secret?

> If  the DPDK  project governance moves to the Linux Foundation,
> we think this will promote ZTE and other companies to contribute in DPDK.

Why it would help you to contribute?
Why not contribute today?
The project is truly open, a lot of major companies are contributing,
why not yours?

^ permalink raw reply

* Re: [PATCH v3 0/7] vhost: add dequeue zero copy support
From: Yuanhan Liu @ 2016-10-12  7:48 UTC (permalink / raw)
  To: dev; +Cc: Maxime Coquelin
In-Reply-To: <1475998080-4644-1-git-send-email-yuanhan.liu@linux.intel.com>

On Sun, Oct 09, 2016 at 03:27:53PM +0800, Yuanhan Liu wrote:
> This patch set enables vhost dequeue zero copy. The majority work goes
> to patch 4: "vhost: add dequeue zero copy".

Applied to dpdk-next-virtio.

	--yliu
> 
> The basic idea of dequeue zero copy is, instead of copying data from the
> desc buf, here we let the mbuf reference the desc buf addr directly.
> 
> The major issue behind that is how and when to update the used ring.
> You could check the commit log of patch 4 for more details.
> 
> Patch 5 introduces a new flag, RTE_VHOST_USER_DEQUEUE_ZERO_COPY, to enable
> dequeue zero copy, which is disabled by default.
> 
> The performance gain is quite impressive. For a simple dequeue workload
> (running rxonly in vhost-pmd and runnin txonly in guest testpmd), it yields
> 50+% performance boost for packet size 1500B. For VM2VM iperf test case,
> it's even better: about 70% boost.
> 
> For small packets, the performance is worse (it's expected, as the extra
> overhead introduced by zero copy outweighs the benefits from saving few
> bytes copy).
> 
> v3: - rebase: mainly for removing conflicts with the Tx indirect patch
>     - don't update last_used_idx twice for zero-copy mode
>     - handle two mssiing "Tx -> dequeue" renames in log and usage
> 
> v2: - renamed "tx zero copy" to "dequeue zero copy", to reduce confusions.
>     - hnadle the case that a desc buf might across 2 host phys pages
>     - use MAP_POPULATE to let kernel populate the page tables
>     - updated release note
>     - doc-ed the limitations for the vm2nic case
>     - merge 2 continuous guest phys memory region
>     - and few more trivial changes, please see them in the corresponding
>       patches
> 
> ---
> Yuanhan Liu (7):
>   vhost: simplify memory regions handling
>   vhost: get guest/host physical address mappings
>   vhost: introduce last avail idx for dequeue
>   vhost: add dequeue zero copy
>   vhost: add a flag to enable dequeue zero copy
>   examples/vhost: add an option to enable dequeue zero copy
>   net/vhost: add an option to enable dequeue zero copy
> 
>  doc/guides/prog_guide/vhost_lib.rst    |  35 +++-
>  doc/guides/rel_notes/release_16_11.rst |  13 ++
>  drivers/net/vhost/rte_eth_vhost.c      |  13 ++
>  examples/vhost/main.c                  |  19 +-
>  lib/librte_vhost/rte_virtio_net.h      |   1 +
>  lib/librte_vhost/socket.c              |   5 +
>  lib/librte_vhost/vhost.c               |  12 ++
>  lib/librte_vhost/vhost.h               | 102 ++++++++---
>  lib/librte_vhost/vhost_user.c          | 315 ++++++++++++++++++++++-----------
>  lib/librte_vhost/virtio_net.c          | 196 +++++++++++++++++---
>  10 files changed, 549 insertions(+), 162 deletions(-)
> 
> -- 
> 1.9.0

^ permalink raw reply

* Re: [PATCH v4] drivers/net:new PMD using tun/tap host interface
From: Michał Mirosław @ 2016-10-12  8:14 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: dev@dpdk.org, pmatilai@redhat.com, yuanhan.liu@linux.intel.com
In-Reply-To: <0685C9CE-94DC-43E4-A34B-17912E51B5ED@intel.com>

2016-10-11 22:56 GMT+02:00 Wiles, Keith <keith.wiles@intel.com>:
>> On Oct 11, 2016, at 6:30 AM, Michał Mirosław <mirqus@gmail.com> wrote:
>>
>> 2016-10-04 16:45 GMT+02:00, Keith Wiles <keith.wiles@intel.com>:
>>> The rte_eth_tap.c PMD creates a device using TUN/TAP interfaces
>>> on the local host. The PMD allows for DPDK and the host to
>>> communicate using a raw device interface on the host and in
>>> the DPDK application. The device created is a Tap device with
>>> a L2 packet header.
>> [...]
>>> +static uint16_t
>>> +pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>>> +{
>>> +    int len, n;
>>> +    struct rte_mbuf *mbuf;
>>> +    struct rx_queue *rxq = queue;
>>> +    struct pollfd pfd;
>>> +    uint16_t num_rx;
>>> +    unsigned long num_rx_bytes = 0;
>>> +
>>> +    pfd.events = POLLIN;
>>> +    pfd.fd = rxq->fd;
>>> +    for (num_rx = 0; num_rx < nb_pkts; ) {
>>> +            n = poll(&pfd, 1, 0);
>>> +
>>> +            if (n <= 0)
>>> +                    break;
>>> +
>>
>> Considering that syscalls are rather expensive, it would be cheaper to
>> allocate an mbuf here and free it when read() returns -1 instead of
>> calling poll() to check whether a packet is waiting. This way you
>> save a syscall per packet and replace one syscall with one mbuf free
>> per poll.
>
> I made this change, but saw no performance difference in the two methods. Removing poll seems reasonable as it is simpler. TAP is already so slow is why the performance did not change is my guess. Anyone wanting to use TAP as a high performance interface is going to be surprised. I believe the best use case for the TAP interface is for control or exception path.

Agreed, TAP does not look like designed for performance as a first goal.

You could do the same simplification for TX path, BTW.

Best Regards,
Michał Mirosław

^ permalink raw reply

* [PATCH] examples/l2fwd-crypto: fix verify with decrypt in chain
From: Piotr Azarewicz @ 2016-10-12  8:58 UTC (permalink / raw)
  To: pablo.de.lara.guarch, gangx.yang; +Cc: dev

This patch fix setting crypto operation data parameters in l2fwd-crypto
application.
>From now decryption in chain with auth verify work fine.

How to reproduce the issue:

1. Run l2fwd_crypto with command:
-c 0x3 -n 4 --vdev "crypto_aesni_mb" \
--vdev "crypto_aesni_mb" \
-- -p 0x3 --chain CIPHER_HASH \
--cipher_op ENCRYPT --cipher_algo AES_CBC \
--cipher_key 00:01:02:03:04:05:06:07:08:09:0a:0b:0c:0d:0e:0f \
--iv 00:01:02:03:04:05:06:07:08:09:0a:0b:0c:0d:0e:ff \
--auth_op GENERATE --auth_algo SHA1_HMAC \
--auth_key
11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:
11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:
11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11

2. Send packet with payload and capture forwarded packet.
Payload in forwarded packet is encrypted, what is good.

3. Run l2fwd_crypto with command:
-c 0x3 -n 4 --vdev "crypto_aesni_mb" \
--vdev "crypto_aesni_mb" \
-- -p 0x3 --chain HASH_CIPHER \
--cipher_op DECRYPT --cipher_algo AES_CBC \
--cipher_key 00:01:02:03:04:05:06:07:08:09:0a:0b:0c:0d:0e:0f \
--iv 00:01:02:03:04:05:06:07:08:09:0a:0b:0c:0d:0e:ff \
--auth_op VERIFY --auth_algo SHA1_HMAC \
--auth_key
11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:
11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:
11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11

4. Send earlier captured packet and capture forwarded packet.
Payload in newly captured packet is not decrypted, what is wrong.

Fixes: 387259bd6c67 ("examples/l2fwd-crypto: add sample application")

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
---
 examples/l2fwd-crypto/main.c |   23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index dae45f5..43fef59 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -451,6 +451,10 @@ l2fwd_simple_crypto_enqueue(struct rte_mbuf *m,
 
 	/* Zero pad data to be crypto'd so it is block aligned */
 	data_len  = rte_pktmbuf_data_len(m) - ipdata_offset;
+
+	if (cparams->do_hash && cparams->hash_verify)
+		data_len -= cparams->digest_length;
+
 	pad_len = data_len % cparams->block_size ? cparams->block_size -
 			(data_len % cparams->block_size) : 0;
 
@@ -472,8 +476,8 @@ l2fwd_simple_crypto_enqueue(struct rte_mbuf *m,
 			op->sym->auth.digest.data = (uint8_t *)rte_pktmbuf_append(m,
 				cparams->digest_length);
 		} else {
-			op->sym->auth.digest.data = (uint8_t *)rte_pktmbuf_append(m,
-				cparams->digest_length);
+			op->sym->auth.digest.data = rte_pktmbuf_mtod(m,
+				uint8_t *) + ipdata_offset + data_len;
 		}
 
 		op->sym->auth.digest.phys_addr = rte_pktmbuf_mtophys_offset(m,
@@ -508,21 +512,10 @@ l2fwd_simple_crypto_enqueue(struct rte_mbuf *m,
 				cparams->cipher_algo == RTE_CRYPTO_CIPHER_KASUMI_F8 ||
 				cparams->cipher_algo == RTE_CRYPTO_CIPHER_ZUC_EEA3) {
 			op->sym->cipher.data.offset = ipdata_offset << 3;
-			if (cparams->do_hash && cparams->hash_verify)
-				/* Do not cipher the hash tag */
-				op->sym->cipher.data.length = (data_len -
-					cparams->digest_length) << 3;
-			else
-				op->sym->cipher.data.length = data_len << 3;
-
+			op->sym->cipher.data.length = data_len << 3;
 		} else {
 			op->sym->cipher.data.offset = ipdata_offset;
-			if (cparams->do_hash && cparams->hash_verify)
-				/* Do not cipher the hash tag */
-				op->sym->cipher.data.length = data_len -
-					cparams->digest_length;
-			else
-				op->sym->cipher.data.length = data_len;
+			op->sym->cipher.data.length = data_len;
 		}
 	}
 
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Wang, Zhihong @ 2016-10-12 12:22 UTC (permalink / raw)
  To: Yuanhan Liu, Jianbo Liu, Thomas Monjalon, Maxime Coquelin; +Cc: dev@dpdk.org
In-Reply-To: <20161012025307.GG16751@yliu-dev.sh.intel.com>



> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, October 12, 2016 10:53 AM
> To: Wang, Zhihong <zhihong.wang@intel.com>; Jianbo Liu <jianbo.liu@linaro.org>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Thomas
> Monjalon <thomas.monjalon@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH v3 0/5] vhost: optimize enqueue
> 
> On Thu, Sep 22, 2016 at 01:47:45PM +0800, Jianbo Liu wrote:
> > On 22 September 2016 at 10:29, Yuanhan Liu <yuanhan.liu@linux.intel.com>
> wrote:
> > > On Wed, Sep 21, 2016 at 08:54:11PM +0800, Jianbo Liu wrote:
> > >> >> > My setup consists of one host running a guest.
> > >> >> > The guest generates as much 64bytes packets as possible using
> > >> >>
> > >> >> Have you tested with other different packet size?
> > >> >> My testing shows that performance is dropping when packet size is more
> > >> >> than 256.
> > >> >
> > >> >
> > >> > Hi Jianbo,
> > >> >
> > >> > Thanks for reporting this.
> > >> >
> > >> >  1. Are you running the vector frontend with mrg_rxbuf=off?
> > >> >
> > Yes, my testing is mrg_rxbuf=off, but not vector frontend PMD.
> >
> > >> >  2. Could you please specify what CPU you're running? Is it Haswell
> > >> >     or Ivy Bridge?
> > >> >
> > It's an ARM server.
> >
> > >> >  3. How many percentage of drop are you seeing?
> > The testing result:
> > size (bytes)     improvement (%)
> > 64                   3.92
> > 128                 11.51
> > 256                  24.16
> > 512                  -13.79
> > 1024                -22.51
> > 1500                -12.22
> > A correction is that performance is dropping if byte size is larger than 512.
> 
> I have thought of this twice. Unfortunately, I think I may need NACK this
> series.
> 
> Merging two code path into one is really good: as you stated, it improves
> the maintainability. But only if we see no performance regression on both
> path after the refactor. Unfortunately, that's not the case here: it hurts
> the performance for one code path (non-mrg Rx).
> 
> That makes me think we may should not do the code path merge at all. I think
> that also aligns with what you have said before (internally): we could do the
> merge if it gives comparable performance before and after that.
> 
> Besides that, I don't quite like the way you did in patch 2 (rewrite enqueue):
> you made a lot of changes in one patch. That means if something wrong
> happened,
> it is hard to narrow down which change introduces that regression. Badly,
> that's exactly what we met here. Weeks have been passed, I see no progress.
> 
> That's the reason we like the idea of "one patch only does one thing, an
> atomic thing".


Yuanhan, folks,

Thanks for the analysis. I disagree here though.

I analyze, develop, benchmark on x86 platforms, where this patch
works great.

I've been trying to analyze on ARM too but it takes time and I've
had a schedule. Also since the ARM perf issue comes when it's
v6 already, I might not be able to make it in time. However
that's what I have to do for this patch to be merged in this
or the next release.

In the meantime, may I suggest we consider the possibility to
have dedicated codes for **perf critical paths** for different
kinds of architecture?

It can be hard for a person to have both the knowledge and the
development environment for multiple archs at the same time.

Moreover, different optimization techniques might be required for
different archs, so it's hard and unnecessary to make a function
works for all archs, sometimes it's just not the right thing to do.


Thanks
Zhihong


> 
> So I will apply the first patch (it's a bug fixing patch) and ask you to
> refactor the rest, without the code path merge.
> 
> I think we could still have a good maintainability code base if we introduce
> more common helper functions that can be used on both Rx path, or even on
> Tx path (such as update_used_ring, or shadow_used_ring).
> 
> It's a bit late for too many changes for v16.11. I think you could just
> grab patch 6 (vhost: optimize cache access) to the old mrg-Rx code path,
> if that also helps the performance? Let us handle the left in next release,
> such as shadow used ring.
> 
> Thanks.
> 
> 	--yliu

^ permalink raw reply

* Re: [PATCH v3] mk: gcc -march support for intel processors code names
From: Thomas Monjalon @ 2016-10-12 12:33 UTC (permalink / raw)
  To: Reshma Pattan; +Cc: dev
In-Reply-To: <1476135193-4721-1-git-send-email-reshma.pattan@intel.com>

2016-10-10 22:33, Reshma Pattan:
> The GCC 4.9 -march option supports the intel code names for processors,
> for example -march=silvermont, -march=broadwell.
> The RTE_MACHINE config flag can be used to pass code name to
> the compiler as -march flag.
> 
> Release notes is updated.
> 
> Linux and FreeBSD getting started guides are updated with recommended
> gcc version as 4.9 and above.
> 
> Some of the gmake command examples in sample application guide and driver
> guides are updated with gcc version as 4.9.
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>

Applied, thanks

How the -march=broadwell option will work with icc and clang?

^ permalink raw reply

* Re: [PATCH] pdump: fix dir permissions value in mkdir call
From: Thomas Monjalon @ 2016-10-12 12:40 UTC (permalink / raw)
  To: Reshma Pattan; +Cc: dev, Remy Horton
In-Reply-To: <7360a723-cdb9-67c4-0297-de6aa3d18215@intel.com>

2016-10-10 22:44, Remy Horton:
> 
> On 10/10/2016 15:35, Reshma Pattan wrote:
> [..]
> > Fixes: e4ffa2d3 ("pdump: fix error handlings")
> > Fixes: bdd8dcc6 ("pdump: fix default socket path")
> >
> > Reported-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> > ---
> >  lib/librte_pdump/rte_pdump.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Acked-by: Remy Horton <remy.horton@intel.com>

Applied, thanks

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox