DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] vhost: disable indirect descriptors feature
From: Maxime Coquelin @ 2016-10-18  7:04 UTC (permalink / raw)
  To: yuanhan.liu, dev; +Cc: zhihong.wang, ciara.loftus
In-Reply-To: <1476717036-17399-1-git-send-email-maxime.coquelin@redhat.com>

Hi Yuanhan,

On 10/17/2016 05:10 PM, Maxime Coquelin wrote:
> Commit 2304dd73d287 ("vhost: support indirect Tx descriptors")
> adds support for indirect descriptors for Tx, but not for Rx.
>
> The problem is that it does not work with windows guests, which
> uses indirect descriptors for the Rx, and also with Linux guests
> when using kernel driver with mergeable buffers feature disabled.
>
> While indirect descriptors support is also added to the Rx path,
> let's disable the feature.
>
> Reported-by: Zhihong Wang <zhihong.wang@intel.com>
> Reported-by: Ciara Loftus <ciara.loftus@intel.com>
> Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/vhost.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index 469117a..f5f8f92 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -65,8 +65,7 @@
>  				(1ULL << VIRTIO_NET_F_CSUM)    | \
>  				(1ULL << VIRTIO_NET_F_GUEST_CSUM) | \
>  				(1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
> -				(1ULL << VIRTIO_NET_F_GUEST_TSO6) | \
> -				(1ULL << VIRTIO_RING_F_INDIRECT_DESC))
> +				(1ULL << VIRTIO_NET_F_GUEST_TSO6))
>
>  uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;

I have implemented Indirect descs for the Rx path yesterday.
It deserves more testing, but early tests show it fix the issues found
with VIRTIO_RING_F_INDIRECT_DESC (both with and without mergeable
buffers).


Thanks to Zhihong series you reworked, the changes to be done for
mergeable buffers case is greatly simplified.
I'll send the series later today.

Thanks,
Maxime

^ permalink raw reply

* [PATCH 2/2] examples/tep_term: Fix packet len for multi-seg mbuf
From: Michael Qiu @ 2016-10-18  5:49 UTC (permalink / raw)
  To: dev; +Cc: jianfeng.tan, Michael Qiu
In-Reply-To: <1476769747-23289-1-git-send-email-qiudayu@chinac.com>

For multi-seg mbuf, ip->total_length should be pkt_len subtract
ether len.

Fixes: 4abe471ed6fc("examples/tep_term: implement VXLAN processing")

Signed-off-by: Michael Qiu <qiudayu@chinac.com>
---
 examples/tep_termination/vxlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/tep_termination/vxlan.c b/examples/tep_termination/vxlan.c
index b57f867..9142c8d 100644
--- a/examples/tep_termination/vxlan.c
+++ b/examples/tep_termination/vxlan.c
@@ -218,7 +218,7 @@ encapsulation(struct rte_mbuf *m, uint8_t queue_id)
 	/* copy in IP header */
 	ip = rte_memcpy(ip, &app_ip_hdr[vport_id],
 		sizeof(struct ipv4_hdr));
-	ip->total_length = rte_cpu_to_be_16(m->data_len
+	ip->total_length = rte_cpu_to_be_16(m->pkt_len
 				- sizeof(struct ether_hdr));
 
 	/* outer IP checksum */
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 1/2] examples/tep_term: Fix l4_len issue
From: Michael Qiu @ 2016-10-18  5:49 UTC (permalink / raw)
  To: dev; +Cc: jianfeng.tan, Michael Qiu

l4_len is not fixed, althrough mostly it is a fixed value,
but when guest using iperf to do some tests, the l4_len
will have another 12 bytes optional fields.

Fixes: 2bb43bd4350a("examples/tep_term: add TSO offload configuration")

Signed-off-by: Michael Qiu <qiudayu@chinac.com>
---
 examples/tep_termination/vxlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/tep_termination/vxlan.c b/examples/tep_termination/vxlan.c
index 5ee1f95..b57f867 100644
--- a/examples/tep_termination/vxlan.c
+++ b/examples/tep_termination/vxlan.c
@@ -147,7 +147,7 @@ process_inner_cksums(struct ether_hdr *eth_hdr, union tunnel_offload_info *info)
 		if (tso_segsz != 0) {
 			ol_flags |= PKT_TX_TCP_SEG;
 			info->tso_segsz = tso_segsz;
-			info->l4_len = sizeof(struct tcp_hdr);
+			info->l4_len = (tcp_hdr->data_off & 0xf0) >> 2;
 		}
 
 	} else if (l4_proto == IPPROTO_SCTP) {
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH] net/enic: fix fdir UDP v4 source port mask
From: John Daley @ 2016-10-18  5:17 UTC (permalink / raw)
  To: bruce.richardson; +Cc: dev, John Daley

This fixes issue found by Coverity where a typo caused the flow director
UDP IPv4 source port mask to be assigned the destination port mask
supplied by the caller.

Coverity issue: 137860

Fixes: dfbd6a9cb504 ("net/enic: extend flow director support for 1300 series")

Signed-off-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic_clsf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c
index d2413d7..53a7977 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -155,7 +155,7 @@ copy_fltr_v2(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
 			udp_val.src_port = input->flow.udp4_flow.src_port;
 		}
 		if (input->flow.udp4_flow.dst_port) {
-			udp_mask.src_port = masks->dst_port_mask;
+			udp_mask.dst_port = masks->dst_port_mask;
 			udp_val.dst_port = input->flow.udp4_flow.dst_port;
 		}
 
-- 
2.10.0

^ permalink raw reply related

* Re: [PATCH v7 0/7] vhost: optimize mergeable Rx path
From: Jianbo Liu @ 2016-10-18  2:25 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, Maxime Coquelin
In-Reply-To: <1476437678-7102-1-git-send-email-yuanhan.liu@linux.intel.com>

On 14 October 2016 at 17:34, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> This is a new set of patches to optimize the mergeable Rx code path.
> No refactoring (rewrite) was made this time. It just applies some
> findings from Zhihong (kudos to him!) that could improve the mergeable
> Rx path on the old code.
......

> ---
> Yuanhan Liu (4):
>   vhost: simplify mergeable Rx vring reservation
>   vhost: use last avail idx for avail ring reservation
>   vhost: prefetch avail ring
>   vhost: retrieve avail head once
>
> Zhihong Wang (3):
>   vhost: remove useless volatile
>   vhost: optimize cache access
>   vhost: shadow used ring update
>
>  lib/librte_vhost/vhost.c      |  13 ++-
>  lib/librte_vhost/vhost.h      |   5 +-
>  lib/librte_vhost/vhost_user.c |  23 +++--
>  lib/librte_vhost/virtio_net.c | 193 +++++++++++++++++++++++++-----------------
>  4 files changed, 149 insertions(+), 85 deletions(-)
>

Reviewed-by: Jianbo Liu <jianbo.liu@linaro.org>

^ permalink raw reply

* [PATCH v2 3/3] net/fm10k: fix out of ofder Rx read issue
From: Qi Zhang @ 2016-10-17 18:29 UTC (permalink / raw)
  To: jingjing.wu, helin.zhang; +Cc: dev, Qi Zhang
In-Reply-To: <1476728982-39985-1-git-send-email-qi.z.zhang@intel.com>

In vPMD, when load Rx desc with _mm_loadu_si128,
volatile point will be cast into non-volatile point.
So GCC is allowed to reorder the load instructions,
while Rx read's correctness is reply on these load
instructions to follow a backward sequence strictly,
so we add compile barrier to prevent compiler reorder.

Fixes: 7092be8437bd ("fm10k: add vector Rx")

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
- fix check-git-log.sh warning.
- add more detail commit message.

 drivers/net/fm10k/fm10k_rxtx_vec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c
index b724486..27f3e43 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
@@ -478,6 +478,7 @@ fm10k_recv_raw_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 		/* Read desc statuses backwards to avoid race condition */
 		/* A.1 load 4 pkts desc */
 		descs0[3] = _mm_loadu_si128((__m128i *)(rxdp + 3));
+		rte_compiler_barrier();
 
 		/* B.2 copy 2 mbuf point into rx_pkts  */
 		_mm_storeu_si128((__m128i *)&rx_pkts[pos], mbp1);
@@ -486,8 +487,10 @@ fm10k_recv_raw_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 		mbp2 = _mm_loadu_si128((__m128i *)&mbufp[pos+2]);
 
 		descs0[2] = _mm_loadu_si128((__m128i *)(rxdp + 2));
+		rte_compiler_barrier();
 		/* B.1 load 2 mbuf point */
 		descs0[1] = _mm_loadu_si128((__m128i *)(rxdp + 1));
+		rte_compiler_barrier();
 		descs0[0] = _mm_loadu_si128((__m128i *)(rxdp));
 
 		/* B.2 copy 2 mbuf point into rx_pkts  */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 1/3] net/i40e: fix out of order Rx read issue
From: Qi Zhang @ 2016-10-17 18:29 UTC (permalink / raw)
  To: jingjing.wu, helin.zhang; +Cc: dev, Qi Zhang
In-Reply-To: <1476728982-39985-1-git-send-email-qi.z.zhang@intel.com>

In vPMD, when load Rx desc with _mm_loadu_si128,
volatile point will be cast into non-volatile point.
So GCC is allowed to reorder the load instructions,
while Rx read's correctness is reply on these load
instructions to follow a backward sequence strictly,
so we add compile barrier to prevent compiler reorder.

Fixes: 9ed94e5bb04e ("i40e: add vector Rx")

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
- fix check-git-log.sh warning.
- add more detail commit message.

 drivers/net/i40e/i40e_rxtx_vec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/i40e/i40e_rxtx_vec.c b/drivers/net/i40e/i40e_rxtx_vec.c
index 0ee0241..ab63501 100644
--- a/drivers/net/i40e/i40e_rxtx_vec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec.c
@@ -305,6 +305,7 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		/* Read desc statuses backwards to avoid race condition */
 		/* A.1 load 4 pkts desc */
 		descs[3] = _mm_loadu_si128((__m128i *)(rxdp + 3));
+		rte_compiler_barrier();
 
 		/* B.2 copy 2 mbuf point into rx_pkts  */
 		_mm_storeu_si128((__m128i *)&rx_pkts[pos], mbp1);
@@ -313,8 +314,10 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		mbp2 = _mm_loadu_si128((__m128i *)&sw_ring[pos+2]);
 
 		descs[2] = _mm_loadu_si128((__m128i *)(rxdp + 2));
+		rte_compiler_barrier();
 		/* B.1 load 2 mbuf point */
 		descs[1] = _mm_loadu_si128((__m128i *)(rxdp + 1));
+		rte_compiler_barrier();
 		descs[0] = _mm_loadu_si128((__m128i *)(rxdp));
 
 		/* B.2 copy 2 mbuf point into rx_pkts  */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 2/3] net/ixgbe: fix out of order Rx read issue
From: Qi Zhang @ 2016-10-17 18:29 UTC (permalink / raw)
  To: jingjing.wu, helin.zhang; +Cc: dev, Qi Zhang
In-Reply-To: <1476728982-39985-1-git-send-email-qi.z.zhang@intel.com>

In vPMD, when load Rx desc with _mm_loadu_si128,
volatile point will be cast into non-volatile point.
So GCC is allowed to reorder the load instructions,
while Rx read's correctness is reply on these load
instructions to follow a backward sequence strictly,
so we add compile barrier to prevent compiler reorder.

Fixes: c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
- fix check-git-log.sh warning.
- add more detail commit message.

 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
index ad8a9ff..abbf284 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
@@ -343,6 +343,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		/* Read desc statuses backwards to avoid race condition */
 		/* A.1 load 4 pkts desc */
 		descs[3] = _mm_loadu_si128((__m128i *)(rxdp + 3));
+		rte_compiler_barrier();
 
 		/* B.2 copy 2 mbuf point into rx_pkts  */
 		_mm_storeu_si128((__m128i *)&rx_pkts[pos], mbp1);
@@ -351,8 +352,10 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		mbp2 = _mm_loadu_si128((__m128i *)&sw_ring[pos+2]);
 
 		descs[2] = _mm_loadu_si128((__m128i *)(rxdp + 2));
+		rte_compiler_barrier();
 		/* B.1 load 2 mbuf point */
 		descs[1] = _mm_loadu_si128((__m128i *)(rxdp + 1));
+		rte_compiler_barrier();
 		descs[0] = _mm_loadu_si128((__m128i *)(rxdp));
 
 		/* B.2 copy 2 mbuf point into rx_pkts  */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 0/3] net: fix out of order Rx read issue
From: Qi Zhang @ 2016-10-17 18:29 UTC (permalink / raw)
  To: jingjing.wu, helin.zhang; +Cc: dev, Qi Zhang

In vPMD, when load Rx desc with _mm_loadu_si128, 
volatile point will be cast into non-volatile point.
So GCC is allowed to reorder the load instructions, 
while Rx read's correctness is reply on these load 
instructions to follow a backward sequence strictly, 
so we add compile barrier to prevent compiler reorder.
We already met this issue on i40e with GCC6 and we 
fixed this on ixgbe and fm10k also.

v2: 
- fix check-git-log.sh warning.
- add more detail commit message.

Qi Zhang (3):
  net/i40e: fix out of order Rx read issue
  net/ixgbe: fix out of order Rx read issue
  net/fm10k: fix out of ofder Rx read issue

 drivers/net/fm10k/fm10k_rxtx_vec.c     | 3 +++
 drivers/net/i40e/i40e_rxtx_vec.c       | 3 +++
 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 3 +++
 3 files changed, 9 insertions(+)

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB with SRIOV
From: Lu, Wenzhuo @ 2016-10-18  1:07 UTC (permalink / raw)
  To: Iremonger, Bernard, dev@dpdk.org, Shah, Rahul R
In-Reply-To: <8CEF83825BEC744B83065625E567D7C21A0934D0@IRSMSX108.ger.corp.intel.com>

Hi Bernard,


> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, October 18, 2016 12:27 AM
> To: Lu, Wenzhuo; dev@dpdk.org; Shah, Rahul R
> Subject: RE: [PATCH v2] net/ixgbe: support multiqueue mode VMDq DCB with
> SRIOV
> 
> Hi Wenzhuo,
> 
> > >  	if (hw->mac.type != ixgbe_mac_82598EB) { @@ -3339,11 +3340,17
> > @@
> > > ixgbe_dcb_tx_hw_config(struct ixgbe_hw *hw,
> > >  		if (dcb_config->vt_mode)
> > >  			reg |= IXGBE_MTQC_VT_ENA;
> > >  		IXGBE_WRITE_REG(hw, IXGBE_MTQC, reg);
> > > -
> > > -		/* Disable drop for all queues */
> > > -		for (q = 0; q < 128; q++)
> > > -			IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > -				(IXGBE_QDE_WRITE | (q <<
> > > IXGBE_QDE_IDX_SHIFT)));
> > > +		if (RTE_ETH_DEV_SRIOV(dev).active == 0) {
> > > +			/* Disable drop for all queues in VMDQ mode*/
> > > +			for (q = 0; q < 128; q++)
> > > +				IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > +						(IXGBE_QDE_WRITE | (q <<
> > > IXGBE_QDE_IDX_SHIFT) | IXGBE_QDE_ENABLE));
> > > +		} else {
> > > +			/* Enable drop for all queues in SRIOV mode */
> > > +			for (q = 0; q < 128; q++)
> > > +				IXGBE_WRITE_REG(hw, IXGBE_QDE,
> > > +						(IXGBE_QDE_WRITE | (q <<
> > > IXGBE_QDE_IDX_SHIFT)));
> > > +		}
> > I think it has nothing to do with mq mode. Do I miss something?
> 
> Behavior is different when SRIOV is enabled.
I don't understand why the behavior is different. To my opinion, the drop has nothing to do with the mode. We can enable or disable it.
The old behavior is disabling it by default. Now you change it to disabling it by default in NO-SRIOV mode, but enabling it in SRIOV mode.
What I don't get is the reason.

> 
> >
> > >
> > >  		/* Enable the Tx desc arbiter */
> > >  		reg = IXGBE_READ_REG(hw, IXGBE_RTTDCS); @@ -3378,7
> > > +3385,7 @@ ixgbe_vmdq_dcb_hw_tx_config(struct rte_eth_dev *dev,
> > >  			vmdq_tx_conf->nb_queue_pools == ETH_16_POOLS
> > ?
> > > 0xFFFF : 0xFFFFFFFF);
> > >
> > >  	/*Configure general DCB TX parameters*/
> > > -	ixgbe_dcb_tx_hw_config(hw, dcb_config);
> > > +	ixgbe_dcb_tx_hw_config(dev, dcb_config);
> > >  }
> > >
> > >  static void
> > > @@ -3661,7 +3668,7 @@ ixgbe_dcb_hw_configure(struct rte_eth_dev
> > *dev,
> > >  		/*get DCB TX configuration parameters from rte_eth_conf*/
> > >  		ixgbe_dcb_tx_config(dev, dcb_config);
> > >  		/*Configure general DCB TX parameters*/
> > > -		ixgbe_dcb_tx_hw_config(hw, dcb_config);
> > > +		ixgbe_dcb_tx_hw_config(dev, dcb_config);
> > >  		break;
> > >  	default:
> > >  		PMD_INIT_LOG(ERR, "Incorrect DCB TX mode
> > configuration"); @@
> > > -3810,9 +3817,6 @@ void ixgbe_configure_dcb(struct rte_eth_dev *dev)
> > >  	    (dev_conf->rxmode.mq_mode != ETH_MQ_RX_DCB_RSS))
> > >  		return;
> > >
> > > -	if (dev->data->nb_rx_queues != ETH_DCB_NUM_QUEUES)
> > > -		return;
> > I remember it's a limitation of implementation. The reason is the
> > resource allocation. Why could we remove it now?
> 
> ETH_DCB_NUM_QUEUES is 128,  nb_rx_queues may not be 128.
I think it's a limitation to force the queue number to be ETH_DCB_NUM_QUEUES.
Just to confirm it, have you try to set rx queue number to something different from 128, like 64, 32...

^ permalink raw reply

* Re: Project Governance and Linux Foundation
From: Dave Neary @ 2016-10-17 21:23 UTC (permalink / raw)
  To: O'Driscoll, Tim, Hobywan Kenoby, dev@dpdk.org, users@dpdk.org
In-Reply-To: <26FA93C7ED1EAA44AB77D62FBE1D27BA675F6F33@IRSMSX108.ger.corp.intel.com>

Hi,

On 10/17/2016 07:52 AM, O'Driscoll, Tim wrote:
>> -----Original Message-----
>> I don't really understand what can be gained by moving to Linux
>> Foundation, but I am almost sure that no individual expert will be able
>> to take any leaderhip role as those roles will be fulfilled by Platinum,
>> Gold or Silver members: right ?
> 
> No. If DPDK were to move to LF as an independent project, then as discussed at the Userspace event in Dublin last year, and as documented in the original post below, the intention would be not to make any significant changes to the technical governance.
> 
> If DPDK were to move to FD.io the situation would be the same. The FD.io Technical Community Charter (https://fd.io/governance/technical-community-charter) specifies how Project Technical Leaders and Committers are nominated and approved, but there's no requirement for people in those roles to come from Platinum, Gold or Silver FD.io members. Those decisions are based purely on technical merit.

I just want to second what Tim said - it's important for Red Hat, at
least, that the technical governance of a project be kept separate from
any membership of an organization managing the budget for the project.

The technical management of the project can also be discussed, but it is
out of scope, IMHO, when talking about moving to fd.io or the Linux
Foundation.

>> The current DPDK version can run on virtually all processors (Intel, IBM
>> and ARM) and leverage all NICs: is there **really** anyone questionning
>> openness of the community?
> 
> I still hear concerns on this, and based on discussions with others who put their names to the post below, they do too. I think it's a perception that we need to address.

I would say that there is still a perception issue, for companies who
look at the active developers, the owners of the project's resources
(infra, domain name), and who have heard anecdotal evidence of issues in
the past. I think the project has made a lot of progress since I have
been following it, and I do not believe there are any major issues with
the independence of the project. However, there are still concerned
parties on this front, and the concerns can be easily addressed by a
move to the LF.

Regards,
Dave.

-- 
Dave Neary - NFV/SDN Community Strategy
Open Source and Standards, Red Hat - http://community.redhat.com
Ph: +1-978-399-2182 / Cell: +1-978-799-3338

^ permalink raw reply

* Re: [RFC] [PATCH v2] libeventdev: event driven programming model framework for DPDK
From: Eads, Gage @ 2016-10-17 20:26 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: dev@dpdk.org, thomas.monjalon@6wind.com, Richardson, Bruce,
	Vangati, Narender, hemant.agrawal@nxp.com
In-Reply-To: <20161017041812.GA3543@localhost.localdomain>



>  -----Original Message-----
>  From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
>  Sent: Sunday, October 16, 2016 11:18 PM
>  To: Eads, Gage <gage.eads@intel.com>
>  Cc: dev@dpdk.org; thomas.monjalon@6wind.com; Richardson, Bruce
>  <bruce.richardson@intel.com>; Vangati, Narender
>  <narender.vangati@intel.com>; hemant.agrawal@nxp.com
>  Subject: Re: [dpdk-dev] [RFC] [PATCH v2] libeventdev: event driven
>  programming model framework for DPDK
>  
>  On Fri, Oct 14, 2016 at 03:00:57PM +0000, Eads, Gage wrote:
>  > Thanks Jerin, this looks good. I've put a few notes/questions inline.
>  
>  Thanks Gage.
>  
>  >
>  > >  +
>  > >  +/**
>  > >  + * Get the device identifier for the named event device.
>  > >  + *
>  > >  + * @param name
>  > >  + *   Event device name to select the event device identifier.
>  > >  + *
>  > >  + * @return
>  > >  + *   Returns event device identifier on success.
>  > >  + *   - <0: Failure to find named event device.
>  > >  + */
>  > >  +extern uint8_t
>  > >  +rte_event_dev_get_dev_id(const char *name);
>  >
>  > This return type should be int8_t, or some signed type, to support the failure
>  case.
>  
>  Makes sense. I will change to int to make consistent with
>  rte_cryptodev_get_dev_id()
>  
>  >
>  > >  +};
>  > >  +
>  > >  +/**
>  > >  + * Schedule one or more events in the event dev.
>  > >  + *
>  > >  + * An event dev implementation may define this is a NOOP, for
>  > > instance if  + * the event dev performs its scheduling in hardware.
>  > >  + *
>  > >  + * @param dev_id
>  > >  + *   The identifier of the device.
>  > >  + */
>  > >  +extern void
>  > >  +rte_event_schedule(uint8_t dev_id);
>  >
>  > One idea: Have the function return the number of scheduled packets (or 0 for
>  implementations that do scheduling in hardware). This could be a helpful
>  diagnostic for the software scheduler.
>  
>  How about returning an implementation specific value ?
>  Rather than defining certain function associated with returned value.
>  Just to  make sure it works with all HW/SW implementations. Something like
>  below,
>  
>  /**
>   * Schedule one or more events in the event dev.
>   *
>   * An event dev implementation may define this is a NOOP, for instance if
>   * the event dev performs its scheduling in hardware.
>   *
>   * @param dev_id
>   *   The identifier of the device.
>   * @return
>   *   Implementation specific value from the event driver for diagnostic purpose
>   */
>  extern int
>  rte_event_schedule(uint8_t dev_id);
>  
>  

That's fine by me.

I also had a comment on the return value of rte_event_dev_info_get() in my previous email: "I'm wondering if this return type should be int, so we can return an error if the dev_id is invalid."

What do you think?

Thanks,
Gage

>  
>  

^ permalink raw reply

* Re: [PATCH v2 3/3] app/testpmd: fix flow director endian issue
From: De Lara Guarch, Pablo @ 2016-10-17 20:18 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev@dpdk.org; +Cc: Lu, Wenzhuo
In-Reply-To: <1476253462-13000-4-git-send-email-wenzhuo.lu@intel.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, October 11, 2016 11:24 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo
> Subject: [dpdk-dev] [PATCH v2 3/3] app/testpmd: fix flow director endian
> issue
> 
> 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>

Commit message should have a space between the Fixes and the Signed-off line.
Apart from that:

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply

* Re: [PATCH v2 2/3] app/testpmd: fix wrong flow director mask
From: De Lara Guarch, Pablo @ 2016-10-17 20:18 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev@dpdk.org; +Cc: Lu, Wenzhuo
In-Reply-To: <1476253462-13000-3-git-send-email-wenzhuo.lu@intel.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, October 11, 2016 11:24 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo
> Subject: [dpdk-dev] [PATCH v2 2/3] app/testpmd: fix wrong flow director mask
> 
> 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>

Commit message should have a space between the Fixes and the Signed-off line.
Apart from that:

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply

* Re: [PATCH] app/testpmd: fix pf/vf check of flow director
From: De Lara Guarch, Pablo @ 2016-10-17 20:02 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev@dpdk.org; +Cc: Lu, Wenzhuo
In-Reply-To: <1476067628-30842-1-git-send-email-wenzhuo.lu@intel.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Sunday, October 09, 2016 7:47 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo
> Subject: [dpdk-dev] [PATCH] app/testpmd: fix pf/vf check of flow director
> 
> Parameters pf & vf are added into most of flow director
> filter CLIs.
> But mac-valn and tunnel filters don't have these parameters,
> the parameters should not be checked for mac-vlan and tunnel
> filters.
> 
> Fixes: e6a68c013353 ("app/testpmd: extend commands for flow director in
> VF")
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Check-git-log script throws a couple of errors:

Wrong headline lowercase:
        app/testpmd: fix pf/vf check of flow director -> PF/VF
Missing blank line after 'Fixes' tag:
        Fixes: e6a68c013353 ("app/testpmd: extend commands for flow director in VF")

Apart from this:
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply

* Re: [PATCH] kni: fix build with kernel 4.9
From: De Lara Guarch, Pablo @ 2016-10-17 18:49 UTC (permalink / raw)
  To: Yigit, Ferruh, dev@dpdk.org; +Cc: Yigit, Ferruh
In-Reply-To: <20161017102314.9704-1-ferruh.yigit@intel.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Monday, October 17, 2016 3:23 AM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh
> Subject: [dpdk-dev] [PATCH] kni: fix build with kernel 4.9
> 
> compile error:
>   CC [M]  .../lib/librte_eal/linuxapp/kni/igb_main.o
> .../lib/librte_eal/linuxapp/kni/igb_main.c:2317:21:
> error: initialization from incompatible pointer type
> 	[-Werror=incompatible-pointer-types]
>   .ndo_set_vf_vlan = igb_ndo_set_vf_vlan,
>                      ^~~~~~~~~~~~~~~~~~~
> 
> Linux kernel 4.9 updates API for ndo_set_vf_vlan:
> Linux: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad
> support")
> 
> Use new API for Linux kernels >= 4.9
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply

* Re: [PATCH] app/test: add crypto continual tests
From: De Lara Guarch, Pablo @ 2016-10-17 18:35 UTC (permalink / raw)
  To: Kusztal, ArkadiuszX, dev@dpdk.org
  Cc: Trahe, Fiona, Griffin, John, Jain, Deepak K
In-Reply-To: <1476361061-7366-1-git-send-email-arkadiuszx.kusztal@intel.com>



> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Thursday, October 13, 2016 5:18 AM
> To: dev@dpdk.org
> Cc: Trahe, Fiona; De Lara Guarch, Pablo; Griffin, John; Jain, Deepak K; Kusztal,
> ArkadiuszX
> Subject: [PATCH] app/test: add crypto continual tests
> 
> This commit adds continual performace tests to Intel(R) QuickAssist
> Technology tests suite. Performance tests are run continually with
> some number of repeating loops.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>

Applied to dpdk-next-crypto.
Thanks,

Pablo

^ permalink raw reply

* Re: [PATCH] app/test: add tests with corrupted data for QAT test suite
From: De Lara Guarch, Pablo @ 2016-10-17 18:30 UTC (permalink / raw)
  To: Jain, Deepak K, Kusztal, ArkadiuszX, dev@dpdk.org
  Cc: Trahe, Fiona, Griffin, John
In-Reply-To: <A09C9DDE180C7E429EC68E2BFB95C903394765F2@IRSMSX107.ger.corp.intel.com>



> -----Original Message-----
> From: Jain, Deepak K
> Sent: Friday, October 14, 2016 1:14 AM
> To: Kusztal, ArkadiuszX; dev@dpdk.org
> Cc: Trahe, Fiona; De Lara Guarch, Pablo; Griffin, John
> Subject: RE: [PATCH] app/test: add tests with corrupted data for QAT test
> suite
> 
> 
> 
> > -----Original Message-----
> > From: Kusztal, ArkadiuszX
> > Sent: Thursday, October 13, 2016 11:04 AM
> > To: dev@dpdk.org
> > Cc: Trahe, Fiona <fiona.trahe@intel.com>; Jain, Deepak K
> > <deepak.k.jain@intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>; Griffin, John <john.griffin@intel.com>;
> > Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> > Subject: [PATCH] app/test: add tests with corrupted data for QAT test suite
> >
> > This commit adds tests with corrupted data to the Intel QuickAssist
> > Technology tests suite in test_cryptodev.c
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  app/test/test_cryptodev.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> >  };
> > --
> > 2.1.0
> Acked-by: Deepak Kumar Jain <deepak.k.jain@intel.com>

Applied to dpdk-next-crypto.
Thanks,

Pablo

^ permalink raw reply

* Re: [PATCH] net/bnx2x: fix icc build error
From: Mody, Rasesh @ 2016-10-17 18:04 UTC (permalink / raw)
  To: Ferruh Yigit, dev@dpdk.org; +Cc: Sony Chacko, Harish Patil, Rasesh Mody
In-Reply-To: <12674be5-bed0-6e61-7506-b6bb19460c97@intel.com>

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, October 17, 2016 5:12 AM
> 
> On 9/30/2016 3:45 PM, Ferruh Yigit wrote:
> > When bnx2x debug config enabled, icc (ICC) 17.0.0 20160721 gives the
> > following compilation error:
> >
> > .../drivers/net/bnx2x/elink.c(6682):
> > error #3656: variable "fw_ver1" may be used before its value is set
> > 	PMD_DRV_LOG(DEBUG,
> > 	^
> >
> > According logic in the code, this error is a false positive, but since
> > this is not in the fast path, fixing compiler error by assigning
> > initial value to variable.
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> >  drivers/net/bnx2x/elink.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/bnx2x/elink.c b/drivers/net/bnx2x/elink.c
> > index 149cc97..d9a72f0 100644
> > --- a/drivers/net/bnx2x/elink.c
> > +++ b/drivers/net/bnx2x/elink.c
> > @@ -6645,7 +6645,7 @@ static elink_status_t
> elink_8073_8727_external_rom_boot(struct bnx2x_softc *sc,
> >  							uint8_t port)
> >  {
> >  	uint32_t count = 0;
> > -	uint16_t fw_ver1, fw_msgout;
> > +	uint16_t fw_ver1 = 0, fw_msgout;
> >  	elink_status_t rc = ELINK_STATUS_OK;
> >
> >  	/* Boot port from external ROM  */
> >
> 
> Since this is an compilation fix, I believe it is good to have this in the release,
> can you please review the patch?

Thanks!

Acked-by: Rasesh Mody <rasesh.mody@qlogic.com>
 
> Thanks,
> ferruh

^ permalink raw reply

* Re: [PATCH v3 00/32] qede: update qede pmd to 1.2.0.1 and enable by default
From: Mody, Rasesh @ 2016-10-17 17:52 UTC (permalink / raw)
  To: Ferruh Yigit, Rasesh Mody, thomas.monjalon@6wind.com,
	bruce.richardson@intel.com
  Cc: dev@dpdk.org, Dept-EngDPDKDev@qlogic.com
In-Reply-To: <04d9a167-0e1c-9f4e-9dba-8f6f6e5eb9ae@intel.com>

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, October 17, 2016 10:06 AM
> 
> On 10/15/2016 9:07 PM, Rasesh Mody wrote:
> > Hi,
> >
> > This patch set includes changes to update the base driver, work with
> > newer FW 8.10.9.0, adds new features, includes enhancements and code
> > cleanup, provides bug fixes and updates documentation for the QEDE
> > poll mode driver.
> >
> > It enables QEDE PMD in the dpdk config by default. The dependency on
> > external library libz has been addressed.
> >
> > The patch set updates the QEDE PMD to 1.2.0.1.
> >
> > Review comments recived for v2 series have been addressed.
> >
> > Please apply to DPDK tree for v16.11 release.
> >
> > Thanks!
> > Rasesh
> >
> > Harish Patil (14):
> >   qede: change the API signature of ecore_mcp_cmd_and_union()
> >   qede: serialize access to MFW mbox
> >   qede: add APIs to support NIC selftests and query sensor info.
> >   qede: fix port (re)configuration issue
> >   qede/base: allow MTU change via vport-update
> >   qede: add missing 100G link speed capability
> >   qede: remove unused/dead code
> >   qede: fixes for VLAN filters
> >   qede: add enable/disable VLAN filtering
> >   qede: fix RSS related issues
> >   qede/base: add support to initiate PF FLR
> >   qede: skip slowpath polling for 100G VF device
> >   qede: fix driver version string
> >   qede: fix status_blk index for VF queues
> >
> > Rasesh Mody (16):
> >   qede/base: add new init files and rearrange the code
> >   qede/base: formatting changes
> >   qede: use FW CONFIG defines as needed
> >   qede/base: add HSI changes and register defines
> >   qede/base: fix the attention formating string
> >   qede/base: additional formatting/comment changes
> >   qede: fix 32 bit compilation
> >   qede/base: update base driver
> >   qede/base: rename structure and defines
> >   qede/base: comment enhancements
> >   qede/base: add MFW crash dump support
> >   qede/base: change rx tx queue start APIs
> >   qede: add support for queue statistics
> >   qede: remove zlib dependency and enable PMD by default
> >   doc: update qede pmd documentation
> >   qede: update driver version
> >
> > Sony Chacko (2):
> >   qede: enable support for unequal number of RX/TX queues
> >   qede: add scatter gather support
> >
> 
> Hi Rasesh,
> 
> This version of the patchset received after RC1, and to be in this release only
> remaining chance is to be merged in RC2.
> 
> And I would like to remind you that after RC2 you will have little time to test
> your new driver, would you like to push the driver to next release or stick
> with this one?
> 
> If you stick to release, can you please send new version -because of patch by
> patch compile error- timely so that it gives some time to reviewers before
> RC2?

Hi Ferruh,

We would like to include the driver changes in DPDK 16.11 release.

Please note that even though the driver patches are out for quite some time, we have thoroughly tested these changes.

Thanks!
-Rasesh 
 
> Thanks,
> ferruh
> 
> 
> 

^ permalink raw reply

* Re: [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
From: Shreyansh Jain @ 2016-10-17 17:29 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon
  Cc: dev@dpdk.org, viktorin@rehivetech.com, David Marchand,
	Hemant Agrawal
In-Reply-To: <1e1ac980-d857-877b-d629-8a6e9fd0a4c1@intel.com>

Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, October 17, 2016 7:13 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>
> Cc: dev@dpdk.org; viktorin@rehivetech.com; David Marchand
> <david.marchand@6wind.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
> Subject: Re: [dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device
> generalization
> 
> On 10/5/2016 12:57 PM, Shreyansh Jain wrote:
> > Hi Thomas,
> >
> > On Tuesday 04 October 2016 01:12 PM, Thomas Monjalon wrote:
> >> 2016-10-04 12:21, Shreyansh Jain:
> >>> Hi Thomas,
> >>>
> >>> On Monday 03 October 2016 07:58 PM, Thomas Monjalon wrote:
> >>>> Applied, thanks everybody for the great (re)work!
> >>>
> >>> Thanks!
> >>>
> > [...]
> > [...]
> >>>
> >>> It can be merged with changes for:
> >>>   - drv_name
> >>>   - EAL_ before _REGISTER_ macros
> >>>   - eth_driver => rte_driver naming
> >>
> >> Good.
> >> Could you make it this week, please?
> >>
> >
> > Certainly. At least some of those I can send within this week :)
> >
> 
> 
> I caught while running ABI validation script today, I think this patch
> should increase LIBABIVER of:
> - lib/librte_cryptodev
> - lib/librte_eal
> - lib/librte_ether
 
Should I be referring to [1] for understanding how/when to change the LIBABIVER?

[1] http://dpdk.org/doc/guides/contributing/versioning.html

> 
> Thanks,
> ferruh

^ permalink raw reply

* Re: [PATCH v3 02/32] qede/base: formatting changes
From: Mody, Rasesh @ 2016-10-17 17:25 UTC (permalink / raw)
  To: Ferruh Yigit, Rasesh Mody, thomas.monjalon@6wind.com,
	bruce.richardson@intel.com
  Cc: dev@dpdk.org, Dept-EngDPDKDev@qlogic.com
In-Reply-To: <64b17ffa-4405-98d4-29fc-180d45e74048@intel.com>

Hi Ferruh,

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, October 17, 2016 9:28 AM
> 
> Hi Rasesh,
> 
> On 10/15/2016 9:07 PM, Rasesh Mody wrote:
> > Fixes white spaces and tabs.
> >
> > Signed-off-by: Rasesh Mody <rasesh.mody@qlogic.com>
> > ---
> 
> When testing patch by patch compilation, applying this patch gives following
> build error, the build error disappears after patch 12/32.
> 
> It should successfully compile after each patch, can you please fix this?

We'll fix the build error and resubmit them today.

Thanks!
Rasesh

^ permalink raw reply

* Re: [PATCH v3 00/32] qede: update qede pmd to 1.2.0.1 and enable by default
From: Ferruh Yigit @ 2016-10-17 17:05 UTC (permalink / raw)
  To: Rasesh Mody, thomas.monjalon, bruce.richardson; +Cc: dev, Dept-EngDPDKDev
In-Reply-To: <1476562089-21823-1-git-send-email-rasesh.mody@qlogic.com>

On 10/15/2016 9:07 PM, Rasesh Mody wrote:
> Hi,
> 
> This patch set includes changes to update the base driver, work with
> newer FW 8.10.9.0, adds new features, includes enhancements and code
> cleanup, provides bug fixes and updates documentation for the QEDE
> poll mode driver.
> 
> It enables QEDE PMD in the dpdk config by default. The dependency on
> external library libz has been addressed.
> 
> The patch set updates the QEDE PMD to 1.2.0.1.
> 
> Review comments recived for v2 series have been addressed.
> 
> Please apply to DPDK tree for v16.11 release.
> 
> Thanks!
> Rasesh
> 
> Harish Patil (14):
>   qede: change the API signature of ecore_mcp_cmd_and_union()
>   qede: serialize access to MFW mbox
>   qede: add APIs to support NIC selftests and query sensor info.
>   qede: fix port (re)configuration issue
>   qede/base: allow MTU change via vport-update
>   qede: add missing 100G link speed capability
>   qede: remove unused/dead code
>   qede: fixes for VLAN filters
>   qede: add enable/disable VLAN filtering
>   qede: fix RSS related issues
>   qede/base: add support to initiate PF FLR
>   qede: skip slowpath polling for 100G VF device
>   qede: fix driver version string
>   qede: fix status_blk index for VF queues
> 
> Rasesh Mody (16):
>   qede/base: add new init files and rearrange the code
>   qede/base: formatting changes
>   qede: use FW CONFIG defines as needed
>   qede/base: add HSI changes and register defines
>   qede/base: fix the attention formating string
>   qede/base: additional formatting/comment changes
>   qede: fix 32 bit compilation
>   qede/base: update base driver
>   qede/base: rename structure and defines
>   qede/base: comment enhancements
>   qede/base: add MFW crash dump support
>   qede/base: change rx tx queue start APIs
>   qede: add support for queue statistics
>   qede: remove zlib dependency and enable PMD by default
>   doc: update qede pmd documentation
>   qede: update driver version
> 
> Sony Chacko (2):
>   qede: enable support for unequal number of RX/TX queues
>   qede: add scatter gather support
> 

Hi Rasesh,

This version of the patchset received after RC1, and to be in this
release only remaining chance is to be merged in RC2.

And I would like to remind you that after RC2 you will have little time
to test your new driver, would you like to push the driver to next
release or stick with this one?

If you stick to release, can you please send new version -because of
patch by patch compile error- timely so that it gives some time to
reviewers before RC2?

Thanks,
ferruh

^ permalink raw reply

* Re: [PATCH] examples/ipsec-secgw: Update checksum while decrementing ttl
From: De Lara Guarch, Pablo @ 2016-10-17 17:05 UTC (permalink / raw)
  To: Gonzalez Monroy, Sergio, Akhil Goyal, dev@dpdk.org
In-Reply-To: <73f643bf-9ca8-f38a-fbfb-5a127d9d01b5@intel.com>



> -----Original Message-----
> From: Gonzalez Monroy, Sergio
> Sent: Monday, October 10, 2016 5:05 AM
> To: De Lara Guarch, Pablo; Akhil Goyal; dev@dpdk.org
> Subject: Re: [PATCH] examples/ipsec-secgw: Update checksum while
> decrementing ttl
> 
> On 07/10/2016 21:53, De Lara Guarch, Pablo wrote:
> >> -----Original Message-----
> >> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> >> Sent: Tuesday, October 04, 2016 11:33 PM
> >> To: De Lara Guarch, Pablo; Gonzalez Monroy, Sergio; dev@dpdk.org
> >> Subject: Re: [PATCH] examples/ipsec-secgw: Update checksum while
> >> decrementing ttl
> >>
> >> On 10/5/2016 6:04 AM, De Lara Guarch, Pablo wrote:
> >>>
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio
> Gonzalez
> >>>> Monroy
> >>>> Sent: Monday, September 26, 2016 6:28 AM
> >>>> To: akhil.goyal@nxp.com; dev@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: Update
> checksum
> >>>> while decrementing ttl
> >>>>
> >>>> Hi Akhil,
> >>>>
> >>>> This application relies on checksum offload in both outbound and
> >> inbound
> >>>> paths (PKT_TX_IP_CKSUM flag).
> >> [Akhil]Agreed that the application relies on checksum offload, but here
> >> we are talking about the inner ip header. Inner IP checksum will be
> >> updated on the next end point after decryption. This would expect that
> >> the next end point must have checksum offload capability. What if we are
> >> capturing the encrypted packets on wireshark or say send it to some
> >> other machine which does not run DPDK and do not know about
> checksum
> >> offload, then wireshark/other machine will not be able to get the
> >> correct the checksum and will show error.
> 
> Understood, we need to have a valid inner checksum.
> RFC1624 states that the computation would be incorrect in
> corner/boundary case.
> I reckon you are basing your incremental update on RFC1141?
> 
> Also I think you should take care of endianess and increment the
> checksum with
> host_to_be(0x0100) instead of +1.
> 
> >>>> Because we assume that we always forward the packet in both paths,
> we
> >>>> decrement the ttl in both inbound and outbound.
> >>>> You seem to only increment (recalculate) the checksum of the inner IP
> >>>> header in the outbound path but not the inbound path.
> >> [Akhil]Correct I missed out the inbound path.
> >>>> Also, in the inbound path you have to consider a possible ECN value
> >> update.
> >> [Akhil]If I take care of the ECN then it would mean I need to calculate
> >> the checksum completely, incremental checksum wont give correct results.
> >> This would surely impact performance. Any suggestion on how should we
> >> take care of ECN update. Should I recalculate the checksum and send the
> >> patch for ECN update? Or do we have a better solution.
> 
> If I am understanding the RFCs mentioned above correctly, you should be
> able to do
> incremental checksum update for any 16bit field/value of the IP header.
> I don't see no reason why you couldn't do something like that, except
> that you would
> have to follow the full equation instead of just adding 0x0100, which
> would be always
> the case when decrementing TTL.
> 
> What do you think?

Any comments, Akhil?

^ permalink raw reply

* [PATCH v4 2/2] app/test_pmd: fix DCB configuration
From: Bernard Iremonger @ 2016-10-17 17:01 UTC (permalink / raw)
  To: dev, rahul.r.shah, wenzhuo.lu; +Cc: Bernard Iremonger
In-Reply-To: <1476720468-16774-1-git-send-email-bernard.iremonger@intel.com>

when dcb mode is DCB_VT_ENABLED set nb_rxq and nb_txq to 1.

Fixes: 2a977b891f99 ("app/testpmd: fix DCB configuration")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6185be6..ee567c3 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2002,8 +2002,8 @@ init_port_dcb_config(portid_t pid,
 	 * and has the same number of rxq and txq in dcb mode
 	 */
 	if (dcb_mode == DCB_VT_ENABLED) {
-		nb_rxq = rte_port->dev_info.max_rx_queues;
-		nb_txq = rte_port->dev_info.max_tx_queues;
+		nb_rxq = 1;
+		nb_txq = 1;
 	} else {
 		/*if vt is disabled, use all pf queues */
 		if (rte_port->dev_info.vmdq_pool_base == 0) {
-- 
2.10.1

^ permalink raw reply related


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