[dpdk-dev] [PATCH] devargs: fix devargs truncation when format string is used

[Andrew Rybchenko]

Space for string terminating NUL character should be provided to
snprintf() to avoid the last symbol truncation.

Fixes: a23bc2c4e01b ("devargs: add non-variadic parsing function")

Reported-by: Ivan Malov 
Signed-off-by: Andrew Rybchenko 
---
 lib/librte_eal/common/eal_common_devargs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c 
b/lib/librte_eal/common/eal_common_devargs.c
index a22a2002e..5ec688aab 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -285,7 +285,7 @@ rte_devargs_parsef(struct rte_devargs *da, const char 
*format, ...)
}
 
va_start(ap, format);
-   vsnprintf(dev, len, format, ap);
+   vsnprintf(dev, len + 1, format, ap);
va_end(ap);
 
return rte_devargs_parse(da, dev);
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 2/2] librte_ip_frag: add mbuf counter

[Alex Kiselev]

Hi Konstantin.
> Hi Alex,
> Sorry for delay in reply.


>> >> There might be situations (kind of attack when a lot of
>> >> fragmented packets are sent to a dpdk application in order
>> >> to flood the fragmentation table) when no additional mbufs
>> >> must be added to the fragmentations table since it already
>> >> contains to many of them. Currently there is no way to
>> >> determine the number of mbufs holded int the fragmentation
>> >> table. This patch allows to keep track of the number of mbufs
>> >> holded in the fragmentation table.

>> > I understand your intention, but still not sure it is worth it.
>> > My thought was that you can estimate by upper limit (num_entries * 
>> > entries_per_bucket) or so.
>> No, I can't. The estimation error might be so big that there would be no 
>> difference at all.

> Not sure why? If you'll use upper limit, then worst thing could happen -
> you would start your table cleanup a bit earlier.
Since bucket size is 4, an estimation error might be 400%.
So, for example, if I want to setup the upper limit (max number mbufs that 
can be stored in frag table) to 20% of all my available mbufs
I have to be ready that 80% of all mbufs might end up in a frag table
(every bucket is full). Or if I take into account bucket size, and devide 20% 
by 4 in order the number mbufs to be exactly 20% in the worse case when every 
bucket is full,
I could end up in the opposite border situation when exactly single mbuf is 
stored
in every bucket, so upper limit of mbufs would be 20 / 4 = 5%. Both ways are not
good since either you have to reserve extra mbufs just to correct estimation 
error
or you upper limit would to small and you will be dropping good fragments. 


>> > Probably another way to account number of mbufs without changes in the lib 
>> > -
>> > apply something like that(assuming that your fragmets are not multisegs):

>> > uint32_t mbuf_in_frag_table = 0;
>> > 

>> n= dr->>cnt;
>> > mb = rte_ipv4_frag_reassemble_packet(...);
>> > if (mb != NULL)
>> >mbuf_in_frag_table += mb->nb_segs;
>> > mbuf_in_frag_table += dr->cnt - n + 1;

> Sorry, my bad, I think it should be 
> mbuf_in_frag_table  -= dr->cnt - n + 1;


>> > In theory that could be applied even if fragments might be multisegs, but 
>> > for that,
>> > we'll need to change rte_ip_frag_free_death_row() to return total number 
>> > of freed segments.

>> That should be a little bit more complicated wrapper code:

>> uint32_t mbuf_in_frag_table = 0;
>> 

>> n= dr->cnt;
>> reassembled_mbuf = rte_ipv4_frag_reassemble_packet(..., fragmented_mbuf, 
>> ...);
>> if (reassembled_mbuf == NULL)
>> mbuf_in_frag_table += fragmented_mbuf->nb_segs;

> We don't know for sure here.
> fragmented_mbuf could be in death row by now.
Yes. That's exactly why you have to keep track of
mbufs here and later after rte_ip_frag_free_death_row().

User have to think about frag table and death row as a single entity, 
kind of a black box, since it's impossible to say where 
(in the frag table or in the death row) your mbuf will be
after you call rte_ipv4_frag_reassemble_packet(). So, a caller/user should
keep track of mbuf on every border/interface of that black box.
One interface is rte_ipv4_frag_reassemble_packet and the other is 
rte_ip_frag_free_death_row.

So, that's why it's easier to keep track of mbufs inside the library.
  

>> else
>> mbuf_in_frag_table -= reassembled_mbuf->nb_segs;
>> mbuf_in_frag_table += dr->cnt - n;


>> Also, in that case every rte_ip_frag_free_death_row() needs a wrapper code 
>> too.

>> n= dr->cnt;
>> rte_ip_frag_free_death_row(..)
>> mbuf_in_frag_table += dr->cnt - n;

> I don't think it is necessary.
> After packet is put in the death-row it is no longer in the table.
It's critical, since from a user point of view death row and frag table
is a black box due rte_ipv4_frag_reassemble_packet() doesn't indicate a caller
where his packet has been stored (in the frag table or death row).

> Konstantin



>> I think my approach is simplier.

>> > Konstantin


>> >> Signed-off-by: Alex Kiselev 
>> >> ---
>> >>  lib/librte_ip_frag/ip_frag_common.h| 16 +---
>> >>  lib/librte_ip_frag/ip_frag_internal.c  | 16 +---
>> >>  lib/librte_ip_frag/rte_ip_frag.h   | 18 +-
>> >>  lib/librte_ip_frag/rte_ip_frag_common.c|  1 +
>> >>  lib/librte_ip_frag/rte_ip_frag_version.map |  1 +
>> >>  lib/librte_ip_frag/rte_ipv4_reassembly.c   |  2 +-
>> >>  lib/librte_ip_frag/rte_ipv6_reassembly.c   |  2 +-
>> >>  7 files changed, 39 insertions(+), 17 deletions(-)

>> >> diff --git a/lib/librte_ip_frag/ip_frag_common.h 
>> >> b/lib/librte_ip_frag/ip_frag_common.h
>> >> index 0fdcc7d0f..9fe5c0559 100644
>> >> --- a/lib/librte_ip_frag/ip_frag_common.h
>> >> +++ b/lib/librte_ip_frag/ip_frag_common.h
>> >> @@ -32,15 +32,15 @@
>> >>  #endif /* IP_FRAG_TBL_STAT */

>> >>  /* internal functions declarations */
>> >> -struct rte_mbuf * ip_frag_process(struct ip_frag_pkt 

Re: [dpdk-dev] [PATCH v2] librte_ethdev: improve description for port name api

[Ferruh Yigit]

On 7/17/2018 7:32 PM, Jasvinder Singh wrote:
> Improve description of api used to get port name from port id or
> vice-versa.
> 
> Signed-off-by: Jasvinder Singh 

Reviewed-by: Ferruh Yigit 

[dpdk-dev] [Bug 72] Unable to install dpdk on arm64

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=72

Stanislav Chlebec (stanislav.chle...@gmail.com) changed:

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
 Resolution|--- |INVALID

--- Comment #7 from Stanislav Chlebec (stanislav.chle...@gmail.com) ---
Thanks

-- 
You are receiving this mail because:
You are the assignee for the bug.

[dpdk-dev] [PATCH] net/sfc: fix assert in set multicast address list

[Andrew Rybchenko]

Return value equal to zero means success and really expected.

Fixes: 0fa0070e4391 ("net/sfc: support multicast addresses list controls")
Cc: sta...@dpdk.org

Signed-off-by: Andrew Rybchenko 
---
 drivers/net/sfc/sfc_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 36a807730..9decbf5af 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1027,7 +1027,7 @@ sfc_set_mc_addr_list(struct rte_eth_dev *dev, struct 
ether_addr *mc_addr_set,
if (rc != 0)
sfc_err(sa, "cannot set multicast address list (rc = %u)", rc);
 
-   SFC_ASSERT(rc > 0);
+   SFC_ASSERT(rc >= 0);
return -rc;
 }
 
-- 
2.17.1

[dpdk-dev] [PATCH] net/sfc: handle unknown L3 packet class in EF10 event parser

[Andrew Rybchenko]

Fix debug build assertion if unknown L3 packet is received.

Fixes: 638bddc99faa ("net/sfc: implement EF10 native Rx datapath")
Fixes: c121f00836ca ("net/sfc: move EF10 Rx event parser to shared header")
Cc: sta...@dpdk.org

Signed-off-by: Andrew Rybchenko 
---
 drivers/net/sfc/sfc_ef10_rx_ev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/sfc/sfc_ef10_rx_ev.h b/drivers/net/sfc/sfc_ef10_rx_ev.h
index 37b40056e..868c755f2 100644
--- a/drivers/net/sfc/sfc_ef10_rx_ev.h
+++ b/drivers/net/sfc/sfc_ef10_rx_ev.h
@@ -122,6 +122,8 @@ sfc_ef10_rx_ev_to_offloads(const efx_qword_t rx_ev, struct 
rte_mbuf *m,
if (tun_ptype == 0)
l2_ptype = RTE_PTYPE_L2_ETHER_ARP;
break;
+   case ESE_DZ_L3_CLASS_UNKNOWN:
+   break;
default:
/* Unexpected Layer 3 class */
SFC_ASSERT(false);
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3] net/mlx5: add support for 32bit systems

[Ferruh Yigit]

On 7/13/2018 7:16 AM, Shahaf Shuler wrote:
> 
> Thursday, July 12, 2018 3:02 PM, Mordechay Haimovsky:
>> Subject: [PATCH v3] net/mlx5: add support for 32bit systems
>>
>> This patch adds support for building and running mlx5 PMD on 32bit systems
>> such as i686.
>>
>> The main issue to tackle was handling the 32bit access to the UAR as quoted
>> from the mlx5 PRM:
>> QP and CQ DoorBells require 64-bit writes. For best performance, it is
>> recommended to execute the QP/CQ DoorBell as a single 64-bit write
>> operation. For platforms that do not support 64 bit writes, it is possible to
>> issue the 64 bits DoorBells through two consecutive writes, each write 32
>> bits, as described below:
>> * The order of writing each of the Dwords is from lower to upper
>>   addresses.
>> * No other DoorBell can be rung (or even start ringing) in the midst  of an 
>> on-
>> going write of a DoorBell over a given UAR page.
>> The last rule implies that in a multi-threaded environment, the access to a
>> UAR page (which can be accessible by all threads in the process) must be
>> synchronized (for example, using a semaphore) unless an atomic write of 64
>> bits in a single bus operation is guaranteed. Such a synchronization is not
>> required for when ringing DoorBells on different UAR pages.
>>
>> Signed-off-by: Moti Haimovsky 
> 
> Applied to next-net-mlx (again), thanks. 
> 
> Guidelines for 32b compilation and testing:
> 1. fetch the latest rdma-core from github. Make sure you have commit 
> "708c8242 mlx5: Fix compilation on 32 bit systems when sse3 is on"
> 2. compile rdma-core for 32b by
>   mkdir build32
>   cd build32
>   CFLAGS="-Werror -m32" cmake -GNinja .. -DENABLE_RESOLVE_NEIGH=0 
> -DIOCTL_MODE=both (approach taken from rdma-core travis build 
> https://github.com/linux-rdma/rdma-core/blob/master/buildlib/travis-build#L20)
>  
>   Ninja (or ninja-build)
> 3. compile and run DPDK against build32 directory

I have confirmed the 32bit build with gcc, thanks for the update.

Only with 32bit ICC getting following errors [1] related to the #pragma usage.

[1]
.../dpdk/drivers/net/mlx5/mlx5_prm.h(14): error #2282: unrecognized GCC pragma
  #pragma GCC diagnostic ignored "-Wpedantic"
 ^

Re: [dpdk-dev] [PATCH] ethdev: fix queue mapping documentation

[Ferruh Yigit]

On 6/29/2018 4:16 PM, Andrew Rybchenko wrote:
> On 06/29/2018 12:44 PM, Jerin Jacob wrote:
>> The RTE_MAX_ETHPORT_QUEUE_STATS_MAPS does not exists, change
>> to the correct definition(RTE_ETHDEV_QUEUE_STAT_CNTRS)
>>
>> Fixes: 5de201df8927 ("ethdev: add stats per queue")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Jerin Jacob 
>
> Reviewed-by: Andrew Rybchenko 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v9 0/2] app/testpmd implement VXLAN/NVGRE Encap/Decap

[Ferruh Yigit]

On 7/6/2018 7:43 AM, Nelio Laranjeiro wrote:
> This series adds an easy and maintainable configuration version support for
> those two actions for 18.08 by using global variables in testpmd to store the
> necessary information for the tunnel encapsulation.  Those variables are used
> in conjunction of RTE_FLOW_ACTION_{VXLAN,NVGRE}_ENCAP action to create easily
> the action for flows.
> 
> A common way to use it:
> 
>  set vxlan ip-version ipv4 vni 4 udp-src 4 udp-dst 4 ip-src 27.0.0.1
> ip-dst 128.0.0.1 eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
>  flow create 0 ingress pattern end actions vxlan_encap /
> queue index 0 / end
> 
>  set vxlan-with-vlan ip-version ipv4 vni 4 udp-src 4 udp-dst 4 p-src
>  127.0.0.1 ip-dst 128.0.0.1 vlan-tci 34 eth-src 11:11:11:11:11:11
>  eth-dst 22:22:22:22:22:22
>  flow create 0 ingress pattern end actions vxlan_encap /
>  queue index 0 / end
> 
>  set vxlan ip-version ipv6 vni 4 udp-src 4 udp-dst 4 ip-src ::1
> ip-dst :: eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
>  flow create 0 ingress pattern end actions vxlan_encap /
>  queue index 0 / end
> 
>  set vxlan-with-vlan ip-version ipv6 vni 4 udp-src 4 udp-dst 4
>  ip-src ::1 ip-dst :: vlan-tci 34 eth-src 11:11:11:11:11:11
>  eth-dst 22:22:22:22:22:22
>  flow create 0 ingress pattern end actions vxlan_encap /
>  queue index 0 / end
> 
> This also replace the proposal done by Mohammad Abdul Awal [1] which handles
> in a more complex way for the same work.
> 
> Note this API has already a modification planned for 18.11 [2] thus those
> series should have a limited life for a single release.
> 
> [1] https://dpdk.org/ml/archives/dev/2018-May/101403.html
> [2] https://dpdk.org/ml/archives/dev/2018-June/103485.html
> 
> Changes in v9:
> 
> - fix the help for NVGRE.
> 
> Changes in v8:
> 
> - add static tokens in the command line to be user friendly.
> 
> Changes in v7:
> 
> - add missing documentation added in v5 and removed in v6 by mistake.
> 
> Changes in v6:
> 
> - fix compilation under redhat 7.5 with gcc 4.8.5 20150623
> 
> Changes in v5:
> 
> - fix documentation generation.
> - add more explanation on how to generate several encapsulated flows.
> 
> Changes in v4:
> 
> - fix big endian issue on vni and tni.
> - add samples to the documentation.
> - set the VXLAN UDP source port to 0 by default to let the driver generate it
>   from the inner hash as described in the RFC 7348.
> - use default rte flow mask for each item.
> 
> Changes in v3:
> 
> - support VLAN in the outer encapsulation.
> - fix the documentation with missing arguments.
> 
> Changes in v2:
> 
> - add default IPv6 values for NVGRE encapsulation.
> - replace VXLAN to NVGRE in comments concerning NVGRE layer.
> 
> 
> Nelio Laranjeiro (2):
>   app/testpmd: add VXLAN encap/decap support
>   app/testpmd: add NVGRE encap/decap support

Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix use of uninitialized field

[Iremonger, Bernard]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Krzysztof Kanas
> Sent: Tuesday, July 10, 2018 7:51 AM
> To: krzysztof.ka...@caviumnetworks.com; dev@dpdk.org; Lu, Wenzhuo
> ; Wu, Jingjing 
> Cc: Nithin Dabilpuram ; t...@semihalf.com
> Subject: [dpdk-dev] [PATCH 1/2] app/testpmd: fix use of uninitialized field
> 
> print_err_msg uses message field that may be not initialized causing
> segmentation fault.
> 
> Fixes: 12f76f5247e2 ("app/testpmd: add command to resume a TM node")
> Cc: t...@semihalf.com
> 
> Signed-off-by: Krzysztof Kanas 

Acked-by: Bernard Iremonger 

Re: [dpdk-dev] [PATCH] devargs: fix devargs truncation when format string is used

[Gaëtan Rivet]

Hi,

On Wed, Jul 18, 2018 at 08:23:30AM +0100, Andrew Rybchenko wrote:
> Space for string terminating NUL character should be provided to
> snprintf() to avoid the last symbol truncation.
> 
> Fixes: a23bc2c4e01b ("devargs: add non-variadic parsing function")
> 
> Reported-by: Ivan Malov 
> Signed-off-by: Andrew Rybchenko 
Acked-by: Gaetan Rivet 
> ---
>  lib/librte_eal/common/eal_common_devargs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_devargs.c 
> b/lib/librte_eal/common/eal_common_devargs.c
> index a22a2002e..5ec688aab 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -285,7 +285,7 @@ rte_devargs_parsef(struct rte_devargs *da, const char 
> *format, ...)
>   }
>  
>   va_start(ap, format);
> - vsnprintf(dev, len, format, ap);
> + vsnprintf(dev, len + 1, format, ap);

Indeed, thanks for reporting and fixing.

>   va_end(ap);
>  
>   return rte_devargs_parse(da, dev);
> -- 
> 2.17.1
> 

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH 2/2] app/testpmd: fix help string for tm commit cmd

[Iremonger, Bernard]

Hi Krzysztof,

> -Original Message-
> From: Krzysztof Kanas [mailto:krzysztof.ka...@caviumnetworks.com]
> Sent: Tuesday, July 10, 2018 7:51 AM
> To: krzysztof.ka...@caviumnetworks.com; dev@dpdk.org; Lu, Wenzhuo
> ; Wu, Jingjing ; Iremonger,
> Bernard 
> Cc: Nithin Dabilpuram 
> Subject: [PATCH 2/2] app/testpmd: fix help string for tm commit cmd
> 
> Fixes: 86dd86088506 ("app/testpmd: fix use of uninitialized field")
> Cc: krzysztof.ka...@caviumnetworks.com
> 
> Signed-off-by: Krzysztof Kanas 
> ---
>  app/test-pmd/cmdline_tm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/cmdline_tm.c b/app/test-pmd/cmdline_tm.c index
> 4f515241703a..09acc1b87293 100644
> --- a/app/test-pmd/cmdline_tm.c
> +++ b/app/test-pmd/cmdline_tm.c
> @@ -2172,7 +2172,7 @@ static void
> cmd_port_tm_hierarchy_commit_parsed(void *parsed_result,
> cmdline_parse_inst_t cmd_port_tm_hierarchy_commit = {
>   .f = cmd_port_tm_hierarchy_commit_parsed,
>   .data = NULL,
> - .help_str = "Set port tm node shaper profile",
> + .help_str = "Commit port tm hierarchy",
>   .tokens = {
>   (void *)&cmd_port_tm_hierarchy_commit_port,
>   (void *)&cmd_port_tm_hierarchy_commit_tm,
> --
> 2.18.0

check-git-log.sh is giving the following error:

./devtools/check-git-log.sh -1
Wrong 'Fixes' reference:
Fixes: 86dd86088506 ("app/testpmd: fix use of uninitialized field")

Regards,

Bernard.

Re: [dpdk-dev] [PATCH v5 09/10] mk: update make targets for classified testcases

[Burakov, Anatoly]

On 17-Jul-18 5:00 PM, Reshma Pattan wrote:

Makefiles are updated with new test case lists.
Test cases are classified as -
P1 - Main test cases,
P2 - Cryptodev/driver test cases,
P3 - Perf test cases which takes longer than 10s,
P4 - Logging/Dump test cases.

Makefile is updated with different targets
for the above classified groups.
Test cases for different targets are listed accordingly.

Cc: sta...@dpdk.org

Signed-off-by: Jananee Parthasarathy 
Reviewed-by: Reshma Pattan 
---


Nitpick, next patch should probably go before this one, but that's not 
critical.


Reviewed-by: Anatoly Burakov 

--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v5 10/10] mk: remove unnecessary make rules of test

[Burakov, Anatoly]

On 17-Jul-18 5:00 PM, Reshma Pattan wrote:

make rule test-basic is duplicate of test rule.
removed unused test-mempool and test-ring make rules.

Fixes: a3df7f8d9c ("mk: rename test related rules")
Fixes: a3df7f8d9c ("mk: rename test related rules")


Fixline appears two times :) Thomas can fix it on apply though.


CC: sta...@dpdk.org
CC: ferruh.yi...@intel.com

Signed-off-by: Reshma Pattan 
---
  mk/rte.sdkroot.mk | 4 ++--
  mk/rte.sdktest.mk | 7 +++
  2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/mk/rte.sdkroot.mk b/mk/rte.sdkroot.mk
index ea3473ebf..18c88017e 100644
--- a/mk/rte.sdkroot.mk
+++ b/mk/rte.sdkroot.mk
@@ -68,8 +68,8 @@ config defconfig showconfigs showversion showversionum:
  cscope gtags tags etags:
$(Q)$(RTE_SDK)/devtools/build-tags.sh $@ $T
  
-.PHONY: test test-basic test-fast test-ring test-mempool test-perf coverage test-drivers test-dump

-test test-basic test-fast test-ring test-mempool test-perf coverage 
test-drivers test-dump:
+.PHONY: test test-fast test-perf coverage test-drivers test-dump
+test test-fast test-perf coverage test-drivers test-dump:
$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdktest.mk $@
  
  test: test-build

diff --git a/mk/rte.sdktest.mk b/mk/rte.sdktest.mk
index 13d1efb6a..295592809 100644
--- a/mk/rte.sdktest.mk
+++ b/mk/rte.sdktest.mk
@@ -18,7 +18,7 @@ DIR := $(shell basename $(RTE_OUTPUT))
  #
  # test: launch auto-tests, very simple for now.
  #
-.PHONY: test test-basic test-fast test-perf test-drivers test-dump coverage
+.PHONY: test test-fast test-perf test-drivers test-dump coverage
  
  PERFLIST=ring_perf,mempool_perf,memcpy_perf,hash_perf,timer_perf,\

   reciprocal_division,reciprocal_division_perf,lpm_perf,red_all,\
@@ -31,8 +31,7 @@ 
DRIVERSLIST=link_bonding,link_bonding_mode4,link_bonding_rssconf,\
  cryptodev_scheduler,cryptodev_aesni_gcm,cryptodev_null,\
  cryptodev_sw_snow3g,cryptodev_sw_kasumi,cryptodev_sw_zuc
  DUMPLIST=dump_struct_sizes,dump_mempool,dump_malloc_stats,dump_devargs,\
- dump_log_types,dump_ring,quit,dump_physmem,dump_memzone,\
- devargs_autotest
+ dump_log_types,dump_ring,dump_physmem,dump_memzone
  
  SPACESTR:=

  SPACESTR+=
@@ -46,7 +45,7 @@ test-perf: WHITELIST=$(STRIPPED_PERFLIST)
  test-drivers: WHITELIST=$(STRIPPED_DRIVERSLIST)
  test-dump: WHITELIST=$(STRIPPED_DUMPLIST)
  
-test test-basic test-fast test-perf test-drivers test-dump:

+test test-fast test-perf test-drivers test-dump:
@mkdir -p $(AUTOTEST_DIR) ; \
cd $(AUTOTEST_DIR) ; \
if [ -f $(RTE_OUTPUT)/app/test ]; then \



Reviewed-by: Anatoly Burakov 

--
Thanks,
Anatoly

Re: [dpdk-dev] [dpdk-stable] [PATCH 0/2] support MAC changes when no live changes allowed

[Ferruh Yigit]

On 7/9/2018 6:14 PM, Alejandro Lucero wrote:
> This is a patched to fix a functionality coming with the first public
> release: changing/setting MAC address.
> 
> The original patch assumes all NICs can safely change or set the MAC
> in any case. However, this is not always true. NFP depends on the firmware
> capabilities and this is not always supported. There are other NICs with
> this same limitation, although, as far as I know, not in DPDK. Linux kernel
> has a IFF_LIVE_ADDR_CHANGE flag and two NICs are checking this flag for
> allowing or not live MAC changes.
> 
> The flag proposed in this patch is just the opposite: advertise if live
> change not supported and assuming it is supported other way.
> 
> Although most NICs support rte_eth_dev_default_mac_addr_set and this
> function returns and error when live change is not supported, note that
> this function is invoked during port start but the value returned is not
> checked. It is likely this is good enough for most of the cases, but
> bonding is relying on this start then mac set/change, and a PMD ports is
> not properly configured for being used as an slave port in some bonding
> modes.

Hi Alejandro,

Overall looks good to me, only it can be good to update "rte_eth_dev_start" API
doc to mention about affect of new flag and perhaps update release notes "API
Changes" section to document behavior change of the "rte_eth_dev_start" API
based on flag.

But, the ethdev library patch was late for this release, I suggest considering
the patch for next release, meanwhile it can get more reviews.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] net/cxgbe: update release notes for flow API support

[Ferruh Yigit]

On 7/6/2018 3:42 PM, Rahul Lakkireddy wrote:
> Signed-off-by: Rahul Lakkireddy 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH] eal: fix build with gcc 4.7

[Thomas Monjalon]

16/07/2018 22:52, Gaëtan Rivet:
> Hi Pablo,
> 
> On Mon, Jul 16, 2018 at 07:26:27AM +0100, Pablo de Lara wrote:
> > Fixed possible out-of-bounds issue:
> > 
> > lib/librte_eal/common/eal_common_devargs.c:
> > In function ‘rte_devargs_layers_parse’:
> > lib/librte_eal/common/eal_common_devargs.c:121:7:
> > error: array subscript is above array bounds
> > 
> > Bugzilla ID: 71
> > Fixes: 338327d731e6 ("devargs: add function to parse device layers")
> > 
> 
> Thanks for fixing this.
> 
> > Signed-off-by: Pablo de Lara 
> Acked-by: Gaetan Rivet 

Applied, thanks

Re: [dpdk-dev] [PATCH] net/pcap: set queue started and stopped

[Ferruh Yigit]

On 7/9/2018 9:21 PM, Gage Eads wrote:
> Set the rx and tx queue state appropriately when the queues or device are
> started or stopped.

Is there a specific reason to enable these dev_ops, if so can you please
document in commit log?

> 
> Signed-off-by: Gage Eads 
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 42 
> +
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index 6bd4a7d79..21e466bcd 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -430,6 +430,10 @@ eth_dev_start(struct rte_eth_dev *dev)
>   return -1;
>   rx->pcap = tx->pcap;
>   }
> +
> + dev->data->tx_queue_state[0] = RTE_ETH_QUEUE_STATE_STARTED;
> + dev->data->rx_queue_state[0] = RTE_ETH_QUEUE_STATE_STARTED;

pcap also supports multiple queue, instead of hardcoding the queue 0 it can be
possible to iterate through dev->data->nb_rx_queues, dev->data->nb_tx_queues.

And I think it is not good to set this in "internals->single_iface" condition,
it is better to do these assignments just above "status_up" after all queues
initialized.

> +
>   goto status_up;
>   }
>  
> @@ -490,6 +494,8 @@ eth_dev_stop(struct rte_eth_dev *dev)
>   pcap_close(tx->pcap);
>   tx->pcap = NULL;
>   rx->pcap = NULL;
> + dev->data->tx_queue_state[0] = RTE_ETH_QUEUE_STATE_STOPPED;
> + dev->data->rx_queue_state[0] = RTE_ETH_QUEUE_STATE_STOPPED;

same here, just above "status_down" is better place and by using
dev->data->nb_[r/t]x_queues

>   goto status_down;
>   }
>  

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/cxgbe: fix init failure due to new flash parts

[Ferruh Yigit]

On 7/9/2018 4:43 PM, Rahul Lakkireddy wrote:
> Add decode logic for new flash parts shipped with new Chelsio NICs
> to fix initialization failure on these NICs.
> 
> Cc: sta...@dpdk.org

I guess Fixes not added intentionally since this is not fixing a previous code
but fixing a behavior with new flash parts, but fix is required to be
backported. Please reply with a Fixes line if this is not correct.

> 
> Signed-off-by: Rahul Lakkireddy 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v2] net/thunderx: avoid sq door bell writes on zero packets

[Ferruh Yigit]

On 7/11/2018 10:05 AM, Jerin Jacob wrote:
> -Original Message-
>> Date: Wed, 11 Jul 2018 13:54:36 +0530
>> From: Kiran Kumar 
>> To: jerin.ja...@caviumnetworks.com, maciej.cze...@caviumnetworks.com,
>>  ferruh.yi...@intel.com
>> Cc: dev@dpdk.org, Kiran Kumar ,
>>  sta...@dpdk.org
>> Subject: [dpdk-dev] [PATCH v2] net/thunderx: avoid sq door bell writes on
>>  zero packets
>> X-Mailer: git-send-email 2.7.4
>>
>> Avoid sq door bell write on zero packet case to reduce additional
>> traffic on register bus.
>>
>> Fixes: 1c421f18e0 ("net/thunderx: add single and multi-segment Tx")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Kiran Kumar 
> 
> Acked-by: Jerin Jacob 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/cxgbe: fix init failure due to new flash parts

[Rahul Lakkireddy]

On Wednesday, July 07/18/18, 2018 at 14:56:12 +0530, Ferruh Yigit wrote:
> On 7/9/2018 4:43 PM, Rahul Lakkireddy wrote:
> > Add decode logic for new flash parts shipped with new Chelsio NICs
> > to fix initialization failure on these NICs.
> > 
> > Cc: sta...@dpdk.org
> 
> I guess Fixes not added intentionally since this is not fixing a previous code
> but fixing a behavior with new flash parts, but fix is required to be
> backported. Please reply with a Fixes line if this is not correct.
> 

Correct. Fixes line was not added intentionally. It's not fixing
any bug with previous code, but rather adding decode logic to detect
the Chelsio NICs with new flash parts. Without this patch, CXGBE PMD
fails to initialize on these new Chelsio NICs with new flash parts.
This fix needs to be backported to allow CXGBE PMD in stable releases
also to detect and initialize on these new Chelsio NICs.

Thanks,
Rahul

[dpdk-dev] [PATCH v2 2/2] app/testpmd: fix help string for tm commit cmd

[Krzysztof Kanas]

Fixes: bd475cefc7cb ("app/testpmd: fix use of uninitialized field")
Cc: krzysztof.ka...@caviumnetworks.com

Signed-off-by: Krzysztof Kanas 
---
v2:
* Fix the Fixes commit message line
---
 app/test-pmd/cmdline_tm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_tm.c b/app/test-pmd/cmdline_tm.c
index 4f515241703a..09acc1b87293 100644
--- a/app/test-pmd/cmdline_tm.c
+++ b/app/test-pmd/cmdline_tm.c
@@ -2172,7 +2172,7 @@ static void cmd_port_tm_hierarchy_commit_parsed(void 
*parsed_result,
 cmdline_parse_inst_t cmd_port_tm_hierarchy_commit = {
.f = cmd_port_tm_hierarchy_commit_parsed,
.data = NULL,
-   .help_str = "Set port tm node shaper profile",
+   .help_str = "Commit port tm hierarchy",
.tokens = {
(void *)&cmd_port_tm_hierarchy_commit_port,
(void *)&cmd_port_tm_hierarchy_commit_tm,
-- 
2.18.0

Re: [dpdk-dev] [dpdk-stable] [PATCH v3] app/testpmd: fix little perf drop

[Ferruh Yigit]

On 7/16/2018 4:02 AM, Lu, Wenzhuo wrote:
> Hi,
> 
>> -Original Message-
>> From: Li, Xiaoyun
>> Sent: Thursday, July 12, 2018 3:56 PM
>> To: Zhang, Qi Z ; Lu, Wenzhuo
>> 
>> Cc: dev@dpdk.org; Li, Xiaoyun ; sta...@dpdk.org
>> Subject: [PATCH v3] app/testpmd: fix little perf drop
>>
>> There is about 3% perf drop. And it is because of a bitrate calculation in 
>> the
>> datapath. So improve it by maintaining an array of port indexes in testpmd,
>> which is updated with ethdev events.
>>
>> Fixes: 8728ccf37615 ("fix ethdev ports enumeration")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Xiaoyun Li 
> Acked-by: Wenzhuo Lu 

Applied to dpdk-next-net/master, thanks.

Is it possible that someone from test team confirm that performance recovered
after this commit and report to mail list?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] doc: update the enic guide and features

[Ferruh Yigit]

On 7/18/2018 3:15 AM, John Daley wrote:
> From: Hyong Youb Kim 
> 
> Make a few updates in preparation for 18.08.
> - Use SPDX
> - Add 1400 series VIC adapters to supported models
> - Describe the VXLAN port number
> - Expand the description for ig-vlan-rewrite
> - Add inner RSS and checksum to the features
> 
> Signed-off-by: Hyong Youb Kim 
> Reviewed-by: John Daley 

<...>

> @@ -534,4 +548,4 @@ Any questions or bugs should be reported to DPDK 
> community and to the ENIC PMD
>  maintainers:
>  
>  - John Daley 
> -- Nelson Escobar 
> +- Hyong Youb Kim 

This information is duplication of MAINTAINERS file, what would you think
removing names from here and reference to maintainers file?

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v2] net/qede: move SPDX tags to source files

[Ferruh Yigit]

On 7/14/2018 2:34 AM, Rasesh Mody wrote:
> We were using LICENSE.qede_pmd to reference inclusion of SPDX licensing
> tag from all the source file. Remove the LICENSE.qede_pmd file and
> directly include SPDX tags in source files.
> 
> Signed-off-by: Rasesh Mody 

Acked-by: Hemant Agrawal 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH] vhost: fix buffer length calculation

[Wang, Zhihong]

> -Original Message-
> From: Bie, Tiwei
> Sent: Tuesday, July 17, 2018 9:11 PM
> To: maxime.coque...@redhat.com; Wang, Zhihong
> ; dev@dpdk.org
> Cc: Wang, Yinan ; Yao, Lei A 
> Subject: [PATCH] vhost: fix buffer length calculation
> 
> Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")
> 
> Reported-by: Yinan Wang 
> Signed-off-by: Tiwei Bie 
> ---
>  lib/librte_vhost/virtio_net.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index 2b7ffcf92..07cc0c845 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -720,7 +720,8 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>   uint16_t hdr_vec_idx = 0;
> 
>   while (remain) {
> - len = remain;
> + len = RTE_MIN(remain,
> +
>   buf_vec[hdr_vec_idx].buf_len);
>   dst = buf_vec[hdr_vec_idx].buf_addr;
>   rte_memcpy((void *)(uintptr_t)dst,
>   (void *)(uintptr_t)src,
> @@ -747,7 +748,7 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>   hdr_addr = 0;
>   }
> 
> - cpy_len = RTE_MIN(buf_len, mbuf_avail);
> + cpy_len = RTE_MIN(buf_avail, mbuf_avail);
> 
>   if (likely(cpy_len > MAX_BATCH_LEN ||
>   vq->batch_copy_nb_elems >= vq-
> >size)) {
> @@ -1112,7 +1113,8 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>* in a contiguous virtual area.
>*/
>   while (remain) {
> - len = remain;
> + len = RTE_MIN(remain,
> + buf_vec[hdr_vec_idx].buf_len);
>   src = buf_vec[hdr_vec_idx].buf_addr;
>   rte_memcpy((void *)(uintptr_t)dst,
>  (void *)(uintptr_t)src, len);
> --
> 2.18.0

Acked-by: Zhihong Wang 

Thanks

[dpdk-dev] [PATCH] eal: fix circular dependency in EAL proc type detection

[Anatoly Burakov]

Currently, we need runtime dir to put all of our runtime info in,
including the DPDK shared config. However, we use the shared
config to determine our proc type, and this happens earlier than
we actually create the config dir and thus can know where to
place the config file.

Fix this by moving runtime dir creation right after the EAL
arguments parsing, but before proc type autodetection. Also,
previously we were creating the config file unconditionally,
even if we specified no_shconf - fix it by only creating
the config file if no_shconf is not set.

Fixes: adf1d867361c ("eal: move runtime config file to new location")

Signed-off-by: Anatoly Burakov 
---
 lib/librte_eal/bsdapp/eal/eal.c   | 33 ++-
 lib/librte_eal/linuxapp/eal/eal.c | 33 ++-
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 73cdf07b8..7b399bc9d 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -286,12 +286,17 @@ eal_proc_type_detect(void)
enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
const char *pathname = eal_runtime_config_path();
 
-   /* if we can open the file but not get a write-lock we are a secondary
-* process. NOTE: if we get a file handle back, we keep that open
-* and don't close it to prevent a race condition between multiple 
opens */
-   if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
-   (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
-   ptype = RTE_PROC_SECONDARY;
+   /* if there no shared config, there can be no secondary processes */
+   if (!internal_config.no_shconf) {
+   /* if we can open the file but not get a write-lock we are a
+* secondary process. NOTE: if we get a file handle back, we
+* keep that open and don't close it to prevent a race condition
+* between multiple opens.
+*/
+   if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
+   (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
+   ptype = RTE_PROC_SECONDARY;
+   }
 
RTE_LOG(INFO, EAL, "Auto-detected process type: %s\n",
ptype == RTE_PROC_PRIMARY ? "PRIMARY" : "SECONDARY");
@@ -468,6 +473,14 @@ eal_parse_args(int argc, char **argv)
}
}
 
+   /* create runtime data directory */
+   if (internal_config.no_shconf == 0 &&
+   eal_create_runtime_dir() < 0) {
+   RTE_LOG(ERR, EAL, "Cannot create runtime directory\n");
+   ret = -1;
+   goto out;
+   }
+
if (eal_adjust_config(&internal_config) != 0) {
ret = -1;
goto out;
@@ -600,14 +613,6 @@ rte_eal_init(int argc, char **argv)
return -1;
}
 
-   /* create runtime data directory */
-   if (internal_config.no_shconf == 0 &&
-   eal_create_runtime_dir() < 0) {
-   rte_eal_init_alert("Cannot create runtime directory\n");
-   rte_errno = EACCES;
-   return -1;
-   }
-
/* FreeBSD always uses legacy memory model */
internal_config.legacy_mem = true;
 
diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
b/lib/librte_eal/linuxapp/eal/eal.c
index d75ae9dae..d2d5aae80 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -344,12 +344,17 @@ eal_proc_type_detect(void)
enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
const char *pathname = eal_runtime_config_path();
 
-   /* if we can open the file but not get a write-lock we are a secondary
-* process. NOTE: if we get a file handle back, we keep that open
-* and don't close it to prevent a race condition between multiple 
opens */
-   if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
-   (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
-   ptype = RTE_PROC_SECONDARY;
+   /* if there no shared config, there can be no secondary processes */
+   if (!internal_config.no_shconf) {
+   /* if we can open the file but not get a write-lock we are a
+* secondary process. NOTE: if we get a file handle back, we
+* keep that open and don't close it to prevent a race condition
+* between multiple opens.
+*/
+   if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
+   (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
+   ptype = RTE_PROC_SECONDARY;
+   }
 
RTE_LOG(INFO, EAL, "Auto-detected process type: %s\n",
ptype == RTE_PROC_PRIMARY ? "PRIMARY" : "SECONDARY");
@@ -692,6 +697,14 @@ eal_parse_args(int argc, char **argv)
}
}
 
+   /* cr

[dpdk-dev] [PATCH] net/bonding: fix invalid port id error

[Radu Nicolau]

setting up the bonding options before calling rte_eth_dev_probing_finish
triggers an invalid port id error because of port state not set, or set
unused

Fixes: fbe90cdd776c ("ethdev: add probing finish function")
Cc: sta...@dpdk.org

Signed-off-by: Radu Nicolau 
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
b/drivers/net/bonding/rte_eth_bond_pmd.c
index fc4d4fd..1320cfd 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3238,6 +3238,7 @@ bond_probe(struct rte_vdev_device *dev)
internals = rte_eth_devices[port_id].data->dev_private;
internals->kvlist = kvlist;
 
+   rte_eth_dev_probing_finish(&rte_eth_devices[port_id]);
 
if (rte_kvargs_count(kvlist, PMD_BOND_AGG_MODE_KVARG) == 1) {
if (rte_kvargs_process(kvlist,
@@ -3257,7 +3258,6 @@ bond_probe(struct rte_vdev_device *dev)
rte_eth_bond_8023ad_agg_selection_set(port_id, AGG_STABLE);
}
 
-   rte_eth_dev_probing_finish(&rte_eth_devices[port_id]);
RTE_BOND_LOG(INFO, "Create bonded device %s on port %d in mode %u on "
"socket %u.",   name, port_id, bonding_mode, socket_id);
return 0;
-- 
2.7.5

[dpdk-dev] [PATCH 1/3] net/ixgbe: remove redundant queue id checks

[Ferruh Yigit]

remove queue id checks from dev_ops
ixgbe_dev_[rx/tx]_queue_[start/stop]

queue id checks already done by ethdev APIs that are calling these
dev_ops

Signed-off-by: Ferruh Yigit 
---
Cc: Keith Wiles 
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 139 +++--
 1 file changed, 63 insertions(+), 76 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 354181664..f82b74a9a 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -5176,34 +5176,30 @@ ixgbe_dev_rx_queue_start(struct rte_eth_dev *dev, 
uint16_t rx_queue_id)
PMD_INIT_FUNC_TRACE();
hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-   if (rx_queue_id < dev->data->nb_rx_queues) {
-   rxq = dev->data->rx_queues[rx_queue_id];
-
-   /* Allocate buffers for descriptor rings */
-   if (ixgbe_alloc_rx_queue_mbufs(rxq) != 0) {
-   PMD_INIT_LOG(ERR, "Could not alloc mbuf for queue:%d",
-rx_queue_id);
-   return -1;
-   }
-   rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
-   rxdctl |= IXGBE_RXDCTL_ENABLE;
-   IXGBE_WRITE_REG(hw, IXGBE_RXDCTL(rxq->reg_idx), rxdctl);
+   rxq = dev->data->rx_queues[rx_queue_id];
 
-   /* Wait until RX Enable ready */
-   poll_ms = RTE_IXGBE_REGISTER_POLL_WAIT_10_MS;
-   do {
-   rte_delay_ms(1);
-   rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
-   } while (--poll_ms && !(rxdctl & IXGBE_RXDCTL_ENABLE));
-   if (!poll_ms)
-   PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d",
-rx_queue_id);
-   rte_wmb();
-   IXGBE_WRITE_REG(hw, IXGBE_RDH(rxq->reg_idx), 0);
-   IXGBE_WRITE_REG(hw, IXGBE_RDT(rxq->reg_idx), rxq->nb_rx_desc - 
1);
-   dev->data->rx_queue_state[rx_queue_id] = 
RTE_ETH_QUEUE_STATE_STARTED;
-   } else
+   /* Allocate buffers for descriptor rings */
+   if (ixgbe_alloc_rx_queue_mbufs(rxq) != 0) {
+   PMD_INIT_LOG(ERR, "Could not alloc mbuf for queue:%d",
+rx_queue_id);
return -1;
+   }
+   rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
+   rxdctl |= IXGBE_RXDCTL_ENABLE;
+   IXGBE_WRITE_REG(hw, IXGBE_RXDCTL(rxq->reg_idx), rxdctl);
+
+   /* Wait until RX Enable ready */
+   poll_ms = RTE_IXGBE_REGISTER_POLL_WAIT_10_MS;
+   do {
+   rte_delay_ms(1);
+   rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
+   } while (--poll_ms && !(rxdctl & IXGBE_RXDCTL_ENABLE));
+   if (!poll_ms)
+   PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d", rx_queue_id);
+   rte_wmb();
+   IXGBE_WRITE_REG(hw, IXGBE_RDH(rxq->reg_idx), 0);
+   IXGBE_WRITE_REG(hw, IXGBE_RDT(rxq->reg_idx), rxq->nb_rx_desc - 1);
+   dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
 
return 0;
 }
@@ -5224,30 +5220,26 @@ ixgbe_dev_rx_queue_stop(struct rte_eth_dev *dev, 
uint16_t rx_queue_id)
PMD_INIT_FUNC_TRACE();
hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-   if (rx_queue_id < dev->data->nb_rx_queues) {
-   rxq = dev->data->rx_queues[rx_queue_id];
+   rxq = dev->data->rx_queues[rx_queue_id];
+
+   rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
+   rxdctl &= ~IXGBE_RXDCTL_ENABLE;
+   IXGBE_WRITE_REG(hw, IXGBE_RXDCTL(rxq->reg_idx), rxdctl);
 
+   /* Wait until RX Enable bit clear */
+   poll_ms = RTE_IXGBE_REGISTER_POLL_WAIT_10_MS;
+   do {
+   rte_delay_ms(1);
rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
-   rxdctl &= ~IXGBE_RXDCTL_ENABLE;
-   IXGBE_WRITE_REG(hw, IXGBE_RXDCTL(rxq->reg_idx), rxdctl);
+   } while (--poll_ms && (rxdctl & IXGBE_RXDCTL_ENABLE));
+   if (!poll_ms)
+   PMD_INIT_LOG(ERR, "Could not disable Rx Queue %d", rx_queue_id);
 
-   /* Wait until RX Enable bit clear */
-   poll_ms = RTE_IXGBE_REGISTER_POLL_WAIT_10_MS;
-   do {
-   rte_delay_ms(1);
-   rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
-   } while (--poll_ms && (rxdctl & IXGBE_RXDCTL_ENABLE));
-   if (!poll_ms)
-   PMD_INIT_LOG(ERR, "Could not disable Rx Queue %d",
-rx_queue_id);
-
-   rte_delay_us(RTE_IXGBE_WAIT_100_US);
+   rte_delay_us(RTE_IXGBE_WAIT_100_US);
 
-   ixgbe_rx_queue_release_mbufs(rxq);
-   ixgbe_reset_rx_queue(adapter, rxq);
-   dev->data->rx_queue_state[rx_queue_id] = 
RTE_ETH_QUEUE_STATE_STOPPED;
-   } 

[dpdk-dev] [PATCH 2/3] net/i40e: remove redundant queue id checks

[Ferruh Yigit]

remove queue id checks from dev_ops
i40e_dev_[rx/tx]_queue_[start/stop]
i40evf_dev_[rx/tx]_queue_[start/stop]

queue id checks already done by ethdev APIs that are calling these
dev_ops

Signed-off-by: Ferruh Yigit 
---
Cc: Keith Wiles 
---
 drivers/net/i40e/i40e_ethdev_vf.c | 107 +++--
 drivers/net/i40e/i40e_rxtx.c  | 127 ++
 2 files changed, 107 insertions(+), 127 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
b/drivers/net/i40e/i40e_ethdev_vf.c
index 3796c9161..001c301b9 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1583,37 +1583,35 @@ static int
 i40evf_dev_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 {
struct i40e_rx_queue *rxq;
-   int err = 0;
+   int err;
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
PMD_INIT_FUNC_TRACE();
 
-   if (rx_queue_id < dev->data->nb_rx_queues) {
-   rxq = dev->data->rx_queues[rx_queue_id];
+   rxq = dev->data->rx_queues[rx_queue_id];
 
-   err = i40e_alloc_rx_queue_mbufs(rxq);
-   if (err) {
-   PMD_DRV_LOG(ERR, "Failed to allocate RX queue mbuf");
-   return err;
-   }
-
-   rte_wmb();
+   err = i40e_alloc_rx_queue_mbufs(rxq);
+   if (err) {
+   PMD_DRV_LOG(ERR, "Failed to allocate RX queue mbuf");
+   return err;
+   }
 
-   /* Init the RX tail register. */
-   I40E_PCI_REG_WRITE(rxq->qrx_tail, rxq->nb_rx_desc - 1);
-   I40EVF_WRITE_FLUSH(hw);
+   rte_wmb();
 
-   /* Ready to switch the queue on */
-   err = i40evf_switch_queue(dev, TRUE, rx_queue_id, TRUE);
+   /* Init the RX tail register. */
+   I40E_PCI_REG_WRITE(rxq->qrx_tail, rxq->nb_rx_desc - 1);
+   I40EVF_WRITE_FLUSH(hw);
 
-   if (err)
-   PMD_DRV_LOG(ERR, "Failed to switch RX queue %u on",
-   rx_queue_id);
-   else
-   dev->data->rx_queue_state[rx_queue_id] = 
RTE_ETH_QUEUE_STATE_STARTED;
+   /* Ready to switch the queue on */
+   err = i40evf_switch_queue(dev, TRUE, rx_queue_id, TRUE);
+   if (err) {
+   PMD_DRV_LOG(ERR, "Failed to switch RX queue %u on",
+   rx_queue_id);
+   return err;
}
+   dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
 
-   return err;
+   return 0;
 }
 
 static int
@@ -1622,45 +1620,39 @@ i40evf_dev_rx_queue_stop(struct rte_eth_dev *dev, 
uint16_t rx_queue_id)
struct i40e_rx_queue *rxq;
int err;
 
-   if (rx_queue_id < dev->data->nb_rx_queues) {
-   rxq = dev->data->rx_queues[rx_queue_id];
-
-   err = i40evf_switch_queue(dev, TRUE, rx_queue_id, FALSE);
-
-   if (err) {
-   PMD_DRV_LOG(ERR, "Failed to switch RX queue %u off",
-   rx_queue_id);
-   return err;
-   }
+   rxq = dev->data->rx_queues[rx_queue_id];
 
-   i40e_rx_queue_release_mbufs(rxq);
-   i40e_reset_rx_queue(rxq);
-   dev->data->rx_queue_state[rx_queue_id] = 
RTE_ETH_QUEUE_STATE_STOPPED;
+   err = i40evf_switch_queue(dev, TRUE, rx_queue_id, FALSE);
+   if (err) {
+   PMD_DRV_LOG(ERR, "Failed to switch RX queue %u off",
+   rx_queue_id);
+   return err;
}
 
+   i40e_rx_queue_release_mbufs(rxq);
+   i40e_reset_rx_queue(rxq);
+   dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
+
return 0;
 }
 
 static int
 i40evf_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 {
-   int err = 0;
+   int err;
 
PMD_INIT_FUNC_TRACE();
 
-   if (tx_queue_id < dev->data->nb_tx_queues) {
-
-   /* Ready to switch the queue on */
-   err = i40evf_switch_queue(dev, FALSE, tx_queue_id, TRUE);
-
-   if (err)
-   PMD_DRV_LOG(ERR, "Failed to switch TX queue %u on",
-   tx_queue_id);
-   else
-   dev->data->tx_queue_state[tx_queue_id] = 
RTE_ETH_QUEUE_STATE_STARTED;
+   /* Ready to switch the queue on */
+   err = i40evf_switch_queue(dev, FALSE, tx_queue_id, TRUE);
+   if (err) {
+   PMD_DRV_LOG(ERR, "Failed to switch TX queue %u on",
+   tx_queue_id);
+   return err;
}
+   dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
 
-   return err;
+   return 0;
 }
 
 static int
@@ -1669,22 +1661,19 @@ i40evf_dev_tx_queue_stop(struct rte_eth_dev *dev, 
uint16_t tx_queue_id)
struct i40e_tx_queue *txq;
int err;
 
-   

[dpdk-dev] [PATCH 3/3] net/fm10k: remove redundant queue id checks

[Ferruh Yigit]

remove queue id checks from dev_ops
fm10k_dev_[rx/tx]_queue_[start/stop]

queue id checks already done by ethdev APIs that are calling these
dev_ops

Signed-off-by: Ferruh Yigit 
---
Cc: Keith Wiles 
---
 drivers/net/fm10k/fm10k_ethdev.c | 119 ++-
 1 file changed, 54 insertions(+), 65 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 7c505451a..541a49b75 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -810,52 +810,50 @@ static int
 fm10k_dev_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 {
struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-   int err = -1;
+   int err;
uint32_t reg;
struct fm10k_rx_queue *rxq;
 
PMD_INIT_FUNC_TRACE();
 
-   if (rx_queue_id < dev->data->nb_rx_queues) {
-   rxq = dev->data->rx_queues[rx_queue_id];
-   err = rx_queue_reset(rxq);
-   if (err == -ENOMEM) {
-   PMD_INIT_LOG(ERR, "Failed to alloc memory : %d", err);
-   return err;
-   } else if (err == -EINVAL) {
-   PMD_INIT_LOG(ERR, "Invalid buffer address alignment :"
-   " %d", err);
-   return err;
-   }
+   rxq = dev->data->rx_queues[rx_queue_id];
+   err = rx_queue_reset(rxq);
+   if (err == -ENOMEM) {
+   PMD_INIT_LOG(ERR, "Failed to alloc memory : %d", err);
+   return err;
+   } else if (err == -EINVAL) {
+   PMD_INIT_LOG(ERR, "Invalid buffer address alignment :"
+   " %d", err);
+   return err;
+   }
 
-   /* Setup the HW Rx Head and Tail Descriptor Pointers
-* Note: this must be done AFTER the queue is enabled on real
-* hardware, but BEFORE the queue is enabled when using the
-* emulation platform. Do it in both places for now and remove
-* this comment and the following two register writes when the
-* emulation platform is no longer being used.
-*/
-   FM10K_WRITE_REG(hw, FM10K_RDH(rx_queue_id), 0);
-   FM10K_WRITE_REG(hw, FM10K_RDT(rx_queue_id), rxq->nb_desc - 1);
+   /* Setup the HW Rx Head and Tail Descriptor Pointers
+* Note: this must be done AFTER the queue is enabled on real
+* hardware, but BEFORE the queue is enabled when using the
+* emulation platform. Do it in both places for now and remove
+* this comment and the following two register writes when the
+* emulation platform is no longer being used.
+*/
+   FM10K_WRITE_REG(hw, FM10K_RDH(rx_queue_id), 0);
+   FM10K_WRITE_REG(hw, FM10K_RDT(rx_queue_id), rxq->nb_desc - 1);
 
-   /* Set PF ownership flag for PF devices */
-   reg = FM10K_READ_REG(hw, FM10K_RXQCTL(rx_queue_id));
-   if (hw->mac.type == fm10k_mac_pf)
-   reg |= FM10K_RXQCTL_PF;
-   reg |= FM10K_RXQCTL_ENABLE;
-   /* enable RX queue */
-   FM10K_WRITE_REG(hw, FM10K_RXQCTL(rx_queue_id), reg);
-   FM10K_WRITE_FLUSH(hw);
+   /* Set PF ownership flag for PF devices */
+   reg = FM10K_READ_REG(hw, FM10K_RXQCTL(rx_queue_id));
+   if (hw->mac.type == fm10k_mac_pf)
+   reg |= FM10K_RXQCTL_PF;
+   reg |= FM10K_RXQCTL_ENABLE;
+   /* enable RX queue */
+   FM10K_WRITE_REG(hw, FM10K_RXQCTL(rx_queue_id), reg);
+   FM10K_WRITE_FLUSH(hw);
 
-   /* Setup the HW Rx Head and Tail Descriptor Pointers
-* Note: this must be done AFTER the queue is enabled
-*/
-   FM10K_WRITE_REG(hw, FM10K_RDH(rx_queue_id), 0);
-   FM10K_WRITE_REG(hw, FM10K_RDT(rx_queue_id), rxq->nb_desc - 1);
-   dev->data->rx_queue_state[rx_queue_id] = 
RTE_ETH_QUEUE_STATE_STARTED;
-   }
+   /* Setup the HW Rx Head and Tail Descriptor Pointers
+* Note: this must be done AFTER the queue is enabled
+*/
+   FM10K_WRITE_REG(hw, FM10K_RDH(rx_queue_id), 0);
+   FM10K_WRITE_REG(hw, FM10K_RDT(rx_queue_id), rxq->nb_desc - 1);
+   dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
 
-   return err;
+   return 0;
 }
 
 static int
@@ -865,14 +863,12 @@ fm10k_dev_rx_queue_stop(struct rte_eth_dev *dev, uint16_t 
rx_queue_id)
 
PMD_INIT_FUNC_TRACE();
 
-   if (rx_queue_id < dev->data->nb_rx_queues) {
-   /* Disable RX queue */
-   rx_queue_disable(hw, rx_queue_id);
+   /* Disable RX queue */
+   rx_queue_disable(hw, rx_queue_id);
 
-   /* Free mbuf and clean HW ring */
-   rx_queue_clean(dev->data->rx_queues[rx_queue_id]);
-   dev->data->rx_queue_state[rx_queue_id] =

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/sfc: fix filter exceptions logic

[Ferruh Yigit]

On 7/14/2018 8:38 AM, Andrew Rybchenko wrote:
> From: Igor Romanov 
> 
> Now exception logic handles these cases:
> 
> When FW variant does not support filters with transport ports, but
> IP protocol filters are supported, TCP/UDP protocol filters may be
> used. When FW variant does not support filters with IPv4/6 addresses
> or IP protocol, but filters with EtherType are supported, IPv4 and
> IPv6 EtherTypes may be used
> 
> Fixes: 096dba799b4a ("net/sfc: avoid creation of ineffective flow rules")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Igor Romanov 
> Signed-off-by: Andrew Rybchenko 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH] net/sfc: fallback to filter with zero vid

[Ferruh Yigit]

On 7/14/2018 8:38 AM, Andrew Rybchenko wrote:
> From: Igor Romanov 
> 
> Fallback to filter with VLAN=0 if match without VLAN is not supported
> Strictly speaking it is not 100% equivalent, but good tradeoff -
> untagged and priority only tagged frames will match.
> 
> Signed-off-by: Igor Romanov 
> Signed-off-by: Andrew Rybchenko 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v2] net/enic: pick the right Rx handler after changing MTU

[Ferruh Yigit]

On 7/18/2018 3:02 AM, John Daley wrote:
> From: Hyong Youb Kim 
> 
> enic_set_mtu always reverts to the default Rx handler after changing
> MTU. Try to use the simpler, non-scatter handler in this case as well.
> 
> Fixes: 35e2cb6a1795 ("net/enic: add simple Rx handler")
> 
> Signed-off-by: Hyong Youb Kim 
> Reviewed-by: John Daley 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH 1/4] eal: fix hotplug add and hotplug remove

[Gaëtan Rivet]

Hi Qi,

On Thu, Jul 12, 2018 at 10:01:41PM +0800, Qi Zhang wrote:
> If hotplug add an already plugged PCI device, it will
> cause rte_pci_device->device.name be corrupted due to unexpected
> rte_devargs_remove. Also if try to hotplug remove an already
> unplugged device, it will cause segment fault due to unexpected
> bus->unplug on a rte_device whose driver is NULL.
> The patch fix these issues.
> 
> Fixes: 7e8b26650146 ("eal: fix hotplug add / remove")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Qi Zhang 

I think we should consolidate this API at some point, maybe list the
possible error values as a part of it and remove the experimental tag.

In any case, the fix seems correct, thanks,

Acked-by: Gaetan Rivet 

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH 2/4] bus/pci: fix PCI address compare

[Gaëtan Rivet]

On Thu, Jul 12, 2018 at 10:01:42PM +0800, Qi Zhang wrote:
> When use memcmp to compare two PCI address, sizeof(struct rte_pci_addr)
> is 4 bytes aligned, and it is 8. While only 7 byte of struct rte_pci_addr
> is valid. So compare the 8th byte will cause the unexpected result, which
> happens when repeatedly attach/detach a device.
> 
> Fixes: 94c0776b1bad ("vfio: support hotplug")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Qi Zhang 
Acked-by: Gaetan Rivet 
> ---
>  drivers/bus/pci/linux/pci_vfio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c 
> b/drivers/bus/pci/linux/pci_vfio.c
> index aeeaa9ed8..933b95540 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -642,7 +642,7 @@ pci_vfio_unmap_resource(struct rte_pci_device *dev)
>   vfio_res_list = RTE_TAILQ_CAST(rte_vfio_tailq.head, 
> mapped_pci_res_list);
>   /* Get vfio_res */
>   TAILQ_FOREACH(vfio_res, vfio_res_list, next) {
> - if (memcmp(&vfio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
> + if (rte_pci_addr_cmp(&vfio_res->pci_addr, &dev->addr))
>   continue;
>   break;
>   }
> -- 
> 2.13.6
> 

-- 
Gaëtan Rivet
6WIND

[dpdk-dev] [PATCH v5] net/bonding: fix slave add for mode 4

[Radu Nicolau]

Moved the link status validity check from the slave add to the slave
activation step. Otherwise slave add will fail for mode 4 if
the ports are all stopped but only one of them checked.

Removed activate slave call from slave add function.

Fixes: b77d21cc2364 ("ethdev: add link status get/set helper functions")
Bugzilla ID: 52
Cc: sta...@dpdk.org

Signed-off-by: Radu Nicolau 
---
v5: removed activate_slave call from slave add
v4: reworked patch
v3: updated commit msg
v2: add fix and Bugzilla references

 drivers/net/bonding/rte_eth_bond_api.c | 13 -
 drivers/net/bonding/rte_eth_bond_pmd.c | 11 +++
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c 
b/drivers/net/bonding/rte_eth_bond_api.c
index 49fa2d7..8bc04cf 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -345,14 +345,6 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, 
uint16_t slave_port_id)
internals->tx_queue_offload_capa &= 
dev_info.tx_queue_offload_capa;
internals->flow_type_rss_offloads &= 
dev_info.flow_type_rss_offloads;
 
-   if (link_properties_valid(bonded_eth_dev,
-   &slave_eth_dev->data->dev_link) != 0) {
-   RTE_BOND_LOG(ERR, "Invalid link properties for slave %d"
-   " in bonding mode %d", slave_port_id,
-   internals->mode);
-   return -1;
-   }
-
/* RETA size is GCD of all slaves RETA sizes, so, if all sizes 
will be
 * the power of 2, the lower one is GCD
 */
@@ -412,11 +404,6 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, 
uint16_t slave_port_id)
!internals->user_defined_primary_port)
bond_ethdev_primary_set(internals,
slave_port_id);
-
-   if (find_slave_by_id(internals->active_slaves,
-internals->active_slave_count,
-slave_port_id) == 
internals->active_slave_count)
-   activate_slave(bonded_eth_dev, slave_port_id);
}
}
 
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
b/drivers/net/bonding/rte_eth_bond_pmd.c
index fc4d4fd..c156888 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2732,6 +2732,17 @@ bond_ethdev_lsc_event_callback(uint16_t port_id, enum 
rte_eth_event_type type,
mac_address_slaves_update(bonded_eth_dev);
}
 
+   /* check link state properties if bonded link is up*/
+   if (bonded_eth_dev->data->dev_link.link_status == ETH_LINK_UP) {
+   if (link_properties_valid(bonded_eth_dev, &link) != 0)
+   RTE_BOND_LOG(ERR, "Invalid link properties "
+"for slave %d in bonding mode %d",
+port_id, internals->mode);
+   } else {
+   /* inherit slave link properties */
+   link_properties_set(bonded_eth_dev, &link);
+   }
+
activate_slave(bonded_eth_dev, port_id);
 
/* If user has defined the primary port then default to using 
it */
-- 
2.7.5

Re: [dpdk-dev] [PATCH v2 1/2] net/thunderx: enable Rx checksum offload

[Ferruh Yigit]

On 7/16/2018 10:26 AM, Pavan Nikhilesh wrote:
> Add L3/L4 Rx checksum offload and update capabilities.
> 
> Signed-off-by: Pavan Nikhilesh 
> ---
>  v2 Changes:
>  - Add Rx checksum offload support for l3fwd.

Overall looks good to me, any ack from driver maintainers?

[dpdk-dev] [PATCH] examples/l3fw-power: do not exit on power lib init failure

[Radu Nicolau]

Do not exit the application if power library fails to initialize
or high performance cores configuration cannot be used.

Signed-off-by: Radu Nicolau 
---
 examples/l3fwd-power/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index d15cd52..42518fe 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1683,10 +1683,10 @@ main(int argc, char **argv)
rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n");
 
if (init_power_library())
-   rte_exit(EXIT_FAILURE, "init_power_library failed\n");
+   RTE_LOG(ERR, POWER, "init_power_library failed\n");
 
if (update_lcore_params() < 0)
-   rte_exit(EXIT_FAILURE, "update_lcore_params failed\n");
+   RTE_LOG(ERR, POWER, "update_lcore_params failed\n");
 
if (check_lcore_params() < 0)
rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
-- 
2.7.5

Re: [dpdk-dev] [PATCH] net/pcap: set queue started and stopped

[Eads, Gage]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Wednesday, July 18, 2018 4:14 AM
> To: Eads, Gage ; dev@dpdk.org
> Subject: Re: [PATCH] net/pcap: set queue started and stopped
> 
> On 7/9/2018 9:21 PM, Gage Eads wrote:
> > Set the rx and tx queue state appropriately when the queues or device
> > are started or stopped.
> 
> Is there a specific reason to enable these dev_ops, if so can you please
> document in commit log?

Yes, the purpose of the patch is to enable the rte_eth_dev_{rx, 
tx}_queue_{start, stop} functions for the PCAP PMD. I'll update the message in 
v2.

> 
> >
> > Signed-off-by: Gage Eads 
> > ---
> >  drivers/net/pcap/rte_eth_pcap.c | 42
> > +
> >  1 file changed, 42 insertions(+)
> >
> > diff --git a/drivers/net/pcap/rte_eth_pcap.c
> > b/drivers/net/pcap/rte_eth_pcap.c index 6bd4a7d79..21e466bcd 100644
> > --- a/drivers/net/pcap/rte_eth_pcap.c
> > +++ b/drivers/net/pcap/rte_eth_pcap.c
> > @@ -430,6 +430,10 @@ eth_dev_start(struct rte_eth_dev *dev)
> > return -1;
> > rx->pcap = tx->pcap;
> > }
> > +
> > +   dev->data->tx_queue_state[0] =
> RTE_ETH_QUEUE_STATE_STARTED;
> > +   dev->data->rx_queue_state[0] =
> RTE_ETH_QUEUE_STATE_STARTED;
> 
> pcap also supports multiple queue, instead of hardcoding the queue 0 it can be
> possible to iterate through dev->data->nb_rx_queues, dev->data-
> >nb_tx_queues.
> 
> And I think it is not good to set this in "internals->single_iface" 
> condition, it is
> better to do these assignments just above "status_up" after all queues
> initialized.
> 
> > +
> > goto status_up;
> > }
> >
> > @@ -490,6 +494,8 @@ eth_dev_stop(struct rte_eth_dev *dev)
> > pcap_close(tx->pcap);
> > tx->pcap = NULL;
> > rx->pcap = NULL;
> > +   dev->data->tx_queue_state[0] =
> RTE_ETH_QUEUE_STATE_STOPPED;
> > +   dev->data->rx_queue_state[0] =
> RTE_ETH_QUEUE_STATE_STOPPED;
> 
> same here, just above "status_down" is better place and by using
> dev->data->nb_[r/t]x_queues

Agreed, I will move the started and stopped assignments as you suggested.

Re: [dpdk-dev] [PATCH] net/pcap: set queue started and stopped

[Ferruh Yigit]

On 7/18/2018 3:17 PM, Eads, Gage wrote:
> 
> 
>> -Original Message-
>> From: Yigit, Ferruh
>> Sent: Wednesday, July 18, 2018 4:14 AM
>> To: Eads, Gage ; dev@dpdk.org
>> Subject: Re: [PATCH] net/pcap: set queue started and stopped
>>
>> On 7/9/2018 9:21 PM, Gage Eads wrote:
>>> Set the rx and tx queue state appropriately when the queues or device
>>> are started or stopped.
>>
>> Is there a specific reason to enable these dev_ops, if so can you please
>> document in commit log?
> 
> Yes, the purpose of the patch is to enable the rte_eth_dev_{rx, 
> tx}_queue_{start, stop} functions for the PCAP PMD. I'll update the message 
> in v2.

I guess that part is clear :) I was asking if there is a higher level reason to
enable queue start/stop on these PMDs?
Is there some specific usecase not working for you when these are not enabled?

> 
>>
>>>
>>> Signed-off-by: Gage Eads 
>>> ---
>>>  drivers/net/pcap/rte_eth_pcap.c | 42
>>> +
>>>  1 file changed, 42 insertions(+)
>>>
>>> diff --git a/drivers/net/pcap/rte_eth_pcap.c
>>> b/drivers/net/pcap/rte_eth_pcap.c index 6bd4a7d79..21e466bcd 100644
>>> --- a/drivers/net/pcap/rte_eth_pcap.c
>>> +++ b/drivers/net/pcap/rte_eth_pcap.c
>>> @@ -430,6 +430,10 @@ eth_dev_start(struct rte_eth_dev *dev)
>>> return -1;
>>> rx->pcap = tx->pcap;
>>> }
>>> +
>>> +   dev->data->tx_queue_state[0] =
>> RTE_ETH_QUEUE_STATE_STARTED;
>>> +   dev->data->rx_queue_state[0] =
>> RTE_ETH_QUEUE_STATE_STARTED;
>>
>> pcap also supports multiple queue, instead of hardcoding the queue 0 it can 
>> be
>> possible to iterate through dev->data->nb_rx_queues, dev->data-
>>> nb_tx_queues.
>>
>> And I think it is not good to set this in "internals->single_iface" 
>> condition, it is
>> better to do these assignments just above "status_up" after all queues
>> initialized.
>>
>>> +
>>> goto status_up;
>>> }
>>>
>>> @@ -490,6 +494,8 @@ eth_dev_stop(struct rte_eth_dev *dev)
>>> pcap_close(tx->pcap);
>>> tx->pcap = NULL;
>>> rx->pcap = NULL;
>>> +   dev->data->tx_queue_state[0] =
>> RTE_ETH_QUEUE_STATE_STOPPED;
>>> +   dev->data->rx_queue_state[0] =
>> RTE_ETH_QUEUE_STATE_STOPPED;
>>
>> same here, just above "status_down" is better place and by using
>> dev->data->nb_[r/t]x_queues
> 
> Agreed, I will move the started and stopped assignments as you suggested.
> 

[dpdk-dev] [PATCH] sched: add 64-bit counter retrieval API

[alangordondewar]

From: Alan Dewar 

Add new APIs to retrieve counters in 64-bit wide fields.

Signed-off-by: Alan Dewar 
---
 lib/librte_sched/rte_sched.c   | 72 
 lib/librte_sched/rte_sched.h   | 76 ++
 lib/librte_sched/rte_sched_version.map |  2 +
 3 files changed, 150 insertions(+)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index 9269e5c..4396366 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -1070,6 +1070,43 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
return 0;
 }
 
+int __rte_experimental
+rte_sched_subport_read_stats64(struct rte_sched_port *port,
+  uint32_t subport_id,
+  struct rte_sched_subport_stats64 *stats64,
+  uint32_t *tc_ov)
+{
+   struct rte_sched_subport *s;
+   uint32_t tc;
+
+   /* Check user parameters */
+   if (port == NULL || subport_id >= port->n_subports_per_port ||
+   stats64 == NULL || tc_ov == NULL)
+   return -1;
+
+   s = port->subport + subport_id;
+
+   /* Copy subport stats and clear */
+   for (tc = 0; tc < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; tc++) {
+   stats64->n_pkts_tc[tc] = s->stats.n_pkts_tc[tc];
+   stats64->n_pkts_tc_dropped[tc] =
+   s->stats.n_pkts_tc_dropped[tc];
+   stats64->n_bytes_tc[tc] = s->stats.n_bytes_tc[tc];
+   stats64->n_bytes_tc_dropped[tc] =
+   s->stats.n_bytes_tc_dropped[tc];
+#ifdef RTE_SCHED_RED
+   stats64->n_pkts_red_dropped[tc] =
+   s->stats.n_pkts_red_dropped[tc];
+#endif
+   }
+   memset(&s->stats, 0, sizeof(struct rte_sched_subport_stats));
+
+   /* Subport TC oversubscription status */
+   *tc_ov = s->tc_ov;
+
+   return 0;
+}
+
 int
 rte_sched_queue_read_stats(struct rte_sched_port *port,
uint32_t queue_id,
@@ -1099,6 +1136,41 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
return 0;
 }
 
+int __rte_experimental
+rte_sched_queue_read_stats64(struct rte_sched_port *port,
+   uint32_t queue_id,
+   struct rte_sched_queue_stats64 *stats64,
+   uint16_t *qlen)
+{
+   struct rte_sched_queue *q;
+   struct rte_sched_queue_extra *qe;
+
+   /* Check user parameters */
+   if ((port == NULL) ||
+   (queue_id >= rte_sched_port_queues_per_port(port)) ||
+   (stats64 == NULL) ||
+   (qlen == NULL)) {
+   return -1;
+   }
+   q = port->queue + queue_id;
+   qe = port->queue_extra + queue_id;
+
+   /* Copy queue stats and clear */
+   stats64->n_pkts = qe->stats.n_pkts;
+   stats64->n_pkts_dropped = qe->stats.n_pkts_dropped;
+   stats64->n_bytes = qe->stats.n_bytes;
+   stats64->n_bytes_dropped = qe->stats.n_bytes_dropped;
+#ifdef RTE_SCHED_RED
+   stats64->n_pkts_red_dropped = qe->stats.n_pkts_red_dropped;
+#endif
+   memset(&qe->stats, 0, sizeof(struct rte_sched_queue_stats));
+
+   /* Queue length */
+   *qlen = q->qw - q->qr;
+
+   return 0;
+}
+
 static inline uint32_t
 rte_sched_port_qindex(struct rte_sched_port *port, uint32_t subport, uint32_t 
pipe, uint32_t traffic_class, uint32_t queue)
 {
diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
index 84fa896..f37a4d6 100644
--- a/lib/librte_sched/rte_sched.h
+++ b/lib/librte_sched/rte_sched.h
@@ -141,6 +141,25 @@ struct rte_sched_subport_stats {
 #endif
 };
 
+struct rte_sched_subport_stats64 {
+   /* Packets */
+   uint64_t n_pkts_tc[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
+   /**< Number of packets successfully written */
+   uint64_t n_pkts_tc_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
+   /**< Number of packets dropped */
+
+   /* Bytes */
+   uint64_t n_bytes_tc[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
+   /**< Number of bytes successfully written for each traffic class */
+   uint64_t n_bytes_tc_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
+   /**< Number of bytes dropped for each traffic class */
+
+#ifdef RTE_SCHED_RED
+   uint64_t n_pkts_red_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
+   /**< Number of packets dropped by red */
+#endif
+};
+
 /*
  * Pipe configuration parameters. The period and credits_per_period
  * parameters are measured in bytes, with one byte meaning the time
@@ -182,6 +201,19 @@ struct rte_sched_queue_stats {
uint32_t n_bytes_dropped;/**< Bytes dropped */
 };
 
+struct rte_sched_queue_stats64 {
+   /* Packets */
+   uint64_t n_pkts; /**< Packets successfully written */
+   uint64_t n_pkts_dropped; /**< Packets dropped */
+#ifdef RTE_SCHED_RED
+   uint64_t n_pkts_red_dropped; /**< Packets dropped by RED */
+#endif
+
+   /* Bytes */
+   uint64_t n_bytes;

Re: [dpdk-dev] Does lthread_cond_wait need a mutex?

[Wiles, Keith]

> On Jul 17, 2018, at 10:43 PM, wubenq...@ruijie.com.cn wrote:
> 
> Hi~
>Reference: 
> http://doc.dpdk.org/guides-18.05/sample_app_ug/performance_thread.html?highlight=lthread
>The L-thread subsystem provides a set of functions that are logically 
> equivalent to the corresponding functions offered by the POSIX pthread 
> library.
>I think there is a bug with pthread_cond_wait of lthread implement.
>Look at this code, there are two lthread:
> 
> lthread1:
>pthread_mutex_lock(mutex); //a1
>if (predicate == FALSE) {//a2
>pthread_cond_wait(cond, mutex)//a3
>}
>pthread_mutex_unlock(mutex);//a4
> 
> int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
> {
> if (override) {
> pthread_mutex_unlock(mutex); //a31
> int rv = lthread_cond_wait(*(struct lthread_cond **)cond, 0); //a32
> 
> pthread_mutex_lock(mutex); //a33
> return rv;
> }
> return _sys_pthread_funcs.f_pthread_cond_wait(cond, mutex);
> }
> 
> lthread2:
>pthread_mutex_lock(mutex);//b1
>predicate = TRUE;//b2
>pthread_mutex_unlock(mutex);//b3
>pthread_cond_signal(cond);//b4
> 
> 
>If the sequence is:
>a1->a2->a31->b1->b2->b3->b4->a32
>Will lthread1 sleep forever?

Maybe is it possible, my brain is not working this morning. Please remember 
that lthreads must give up control or lthread will continue to and can not be 
preempted.

Does that fix the problem?

> 
> 
> 吴本卿(研五 福州)

Regards,
Keith

[dpdk-dev] [PATCH v3 1/2] net/thunderx: enable Rx checksum offload

[Pavan Nikhilesh]

Add L3/L4 Rx checksum offload and update capabilities.

Signed-off-by: Pavan Nikhilesh 
---
 v3 Changes:
 - rebase on top of next-net

 v2 Changes:
 - Add Rx checksum offload support for l3fwd.

 drivers/net/thunderx/nicvf_ethdev.c | 33 -
 drivers/net/thunderx/nicvf_ethdev.h |  1 +
 drivers/net/thunderx/nicvf_rxtx.c   | 73 +
 drivers/net/thunderx/nicvf_rxtx.h   | 15 --
 drivers/net/thunderx/nicvf_struct.h | 27 ++-
 5 files changed, 113 insertions(+), 36 deletions(-)

diff --git a/drivers/net/thunderx/nicvf_ethdev.c 
b/drivers/net/thunderx/nicvf_ethdev.c
index 5e15a88a5..eba05fdf0 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -355,11 +355,9 @@ nicvf_dev_supported_ptypes_get(struct rte_eth_dev *dev)
}

memcpy((char *)ptypes + copied, &ptypes_end, sizeof(ptypes_end));
-   if (dev->rx_pkt_burst == nicvf_recv_pkts ||
-   dev->rx_pkt_burst == nicvf_recv_pkts_multiseg)
-   return ptypes;

-   return NULL;
+   /* All Ptypes are supported in all Rx functions. */
+   return ptypes;
 }

 static void
@@ -916,13 +914,18 @@ nicvf_set_tx_function(struct rte_eth_dev *dev)
 static void
 nicvf_set_rx_function(struct rte_eth_dev *dev)
 {
-   if (dev->data->scattered_rx) {
-   PMD_DRV_LOG(DEBUG, "Using multi-segment rx callback");
-   dev->rx_pkt_burst = nicvf_recv_pkts_multiseg;
-   } else {
-   PMD_DRV_LOG(DEBUG, "Using single-segment rx callback");
-   dev->rx_pkt_burst = nicvf_recv_pkts;
-   }
+   struct nicvf *nic = nicvf_pmd_priv(dev);
+
+   const eth_rx_burst_t rx_burst_func[2][2] = {
+   /* [NORMAL/SCATTER] [NO_CKSUM/CKSUM] */
+   [0][0] = nicvf_recv_pkts_no_offload,
+   [0][1] = nicvf_recv_pkts_cksum,
+   [1][0] = nicvf_recv_pkts_multiseg_no_offload,
+   [1][1] = nicvf_recv_pkts_multiseg_cksum,
+   };
+
+   dev->rx_pkt_burst =
+   rx_burst_func[dev->data->scattered_rx][nic->offload_cksum];
 }

 static int
@@ -1243,6 +1246,9 @@ nicvf_rxq_mbuf_setup(struct nicvf_rxq *rxq)
offsetof(struct rte_mbuf, data_off) != 4);
RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, port) -
offsetof(struct rte_mbuf, data_off) != 6);
+   RTE_BUILD_BUG_ON(offsetof(struct nicvf_rxq, rxq_fastpath_data_end) -
+   offsetof(struct nicvf_rxq,
+   rxq_fastpath_data_start) > 128);
mb_def.nb_segs = 1;
mb_def.data_off = RTE_PKTMBUF_HEADROOM + (nic->skip_bytes);
mb_def.port = rxq->port_id;
@@ -1743,7 +1749,7 @@ nicvf_dev_start(struct rte_eth_dev *dev)
return ret;
}

-   /* Configure callbacks based on scatter mode */
+   /* Configure callbacks based on offloads */
nicvf_set_tx_function(dev);
nicvf_set_rx_function(dev);

@@ -1962,6 +1968,9 @@ nicvf_dev_configure(struct rte_eth_dev *dev)
}
}

+   if (rxmode->offloads & DEV_RX_OFFLOAD_CHECKSUM)
+   nic->offload_cksum = 1;
+
PMD_INIT_LOG(DEBUG, "Configured ethdev port%d hwcap=0x%" PRIx64,
dev->data->port_id, nicvf_hw_cap(nic));

diff --git a/drivers/net/thunderx/nicvf_ethdev.h 
b/drivers/net/thunderx/nicvf_ethdev.h
index 9af508803..ae440fef2 100644
--- a/drivers/net/thunderx/nicvf_ethdev.h
+++ b/drivers/net/thunderx/nicvf_ethdev.h
@@ -38,6 +38,7 @@
DEV_TX_OFFLOAD_MULTI_SEGS)

 #define NICVF_RX_OFFLOAD_CAPA ( \
+   DEV_RX_OFFLOAD_CHECKSUM| \
DEV_RX_OFFLOAD_VLAN_STRIP  | \
DEV_RX_OFFLOAD_CRC_STRIP   | \
DEV_RX_OFFLOAD_JUMBO_FRAME | \
diff --git a/drivers/net/thunderx/nicvf_rxtx.c 
b/drivers/net/thunderx/nicvf_rxtx.c
index 6e075e23c..4980dab79 100644
--- a/drivers/net/thunderx/nicvf_rxtx.c
+++ b/drivers/net/thunderx/nicvf_rxtx.c
@@ -331,6 +331,20 @@ nicvf_rx_classify_pkt(cqe_rx_word0_t cqe_rx_w0)
return ptype_table[cqe_rx_w0.l3_type][cqe_rx_w0.l4_type];
 }

+static inline uint64_t __hot
+nicvf_set_olflags(const cqe_rx_word0_t cqe_rx_w0)
+{
+   static const uint64_t flag_table[3] __rte_cache_aligned = {
+   PKT_RX_IP_CKSUM_GOOD | PKT_RX_L4_CKSUM_GOOD,
+   PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_UNKNOWN,
+   PKT_RX_IP_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD,
+   };
+
+   const uint8_t idx = (cqe_rx_w0.err_opcode == CQE_RX_ERR_L4_CHK) << 1 |
+   (cqe_rx_w0.err_opcode == CQE_RX_ERR_IP_CHK);
+   return flag_table[idx];
+}
+
 static inline int __hot
 nicvf_fill_rbdr(struct nicvf_rxq *rxq, int to_fill)
 {
@@ -389,11 +403,13 @@ nicvf_rx_offload(cqe_rx_word0_t cqe_rx_w0, cqe_rx_word2_t 
cqe_rx_w2,
if (likely(cqe_rx_w0.rss_alg)) {
pkt->hash.rss = cqe_rx_w2.rss_tag;
pkt->ol_flags |= PKT_RX_RSS_HASH;
+
   

[dpdk-dev] [PATCH v3 2/2] net/thunderx: add support for Rx VLAN offload

[Pavan Nikhilesh]

From: "Kudurumalla, Rakesh" 

This feature is used to offload stripping of vlan header from recevied
packets and update vlan_tci field in mbuf when
DEV_RX_OFFLOAD_VLAN_STRIP & ETH_VLAN_STRIP_MASK flag is set.

Signed-off-by: Rakesh Kudurumalla 
Signed-off-by: Pavan Nikhilesh 
---
 drivers/net/thunderx/base/nicvf_hw.c |  1 +
 drivers/net/thunderx/nicvf_ethdev.c  | 54 ++--
 drivers/net/thunderx/nicvf_rxtx.c| 47 
 drivers/net/thunderx/nicvf_rxtx.h|  9 +
 drivers/net/thunderx/nicvf_struct.h  |  1 +
 5 files changed, 101 insertions(+), 11 deletions(-)

diff --git a/drivers/net/thunderx/base/nicvf_hw.c 
b/drivers/net/thunderx/base/nicvf_hw.c
index b07a2937d..5b1abe201 100644
--- a/drivers/net/thunderx/base/nicvf_hw.c
+++ b/drivers/net/thunderx/base/nicvf_hw.c
@@ -699,6 +699,7 @@ nicvf_vlan_hw_strip(struct nicvf *nic, bool enable)
else
val &= ~((STRIP_SECOND_VLAN | STRIP_FIRST_VLAN) << 25);
 
+   nic->vlan_strip = enable;
nicvf_reg_write(nic, NIC_VNIC_RQ_GEN_CFG, val);
 }
 
diff --git a/drivers/net/thunderx/nicvf_ethdev.c 
b/drivers/net/thunderx/nicvf_ethdev.c
index eba05fdf0..a55c3ca66 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -52,6 +52,8 @@ static void nicvf_dev_stop(struct rte_eth_dev *dev);
 static void nicvf_dev_stop_cleanup(struct rte_eth_dev *dev, bool cleanup);
 static void nicvf_vf_stop(struct rte_eth_dev *dev, struct nicvf *nic,
  bool cleanup);
+static int nicvf_vlan_offload_config(struct rte_eth_dev *dev, int mask);
+static int nicvf_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 
 RTE_INIT(nicvf_init_log)
 {
@@ -916,16 +918,21 @@ nicvf_set_rx_function(struct rte_eth_dev *dev)
 {
struct nicvf *nic = nicvf_pmd_priv(dev);
 
-   const eth_rx_burst_t rx_burst_func[2][2] = {
-   /* [NORMAL/SCATTER] [NO_CKSUM/CKSUM] */
-   [0][0] = nicvf_recv_pkts_no_offload,
-   [0][1] = nicvf_recv_pkts_cksum,
-   [1][0] = nicvf_recv_pkts_multiseg_no_offload,
-   [1][1] = nicvf_recv_pkts_multiseg_cksum,
+   const eth_rx_burst_t rx_burst_func[2][2][2] = {
+   /* [NORMAL/SCATTER] [CKSUM/NO_CKSUM] [VLAN_STRIP/NO_VLAN_STRIP] */
+   [0][0][0] = nicvf_recv_pkts_no_offload,
+   [0][0][1] = nicvf_recv_pkts_vlan_strip,
+   [0][1][0] = nicvf_recv_pkts_cksum,
+   [0][1][1] = nicvf_recv_pkts_cksum_vlan_strip,
+   [1][0][0] = nicvf_recv_pkts_multiseg_no_offload,
+   [1][0][1] = nicvf_recv_pkts_multiseg_vlan_strip,
+   [1][1][0] = nicvf_recv_pkts_multiseg_cksum,
+   [1][1][1] = nicvf_recv_pkts_multiseg_cksum_vlan_strip,
};
 
dev->rx_pkt_burst =
-   rx_burst_func[dev->data->scattered_rx][nic->offload_cksum];
+   rx_burst_func[dev->data->scattered_rx]
+   [nic->offload_cksum][nic->vlan_strip];
 }
 
 static int
@@ -1473,7 +1480,7 @@ nicvf_vf_start(struct rte_eth_dev *dev, struct nicvf 
*nic, uint32_t rbdrsz)
struct rte_mbuf *mbuf;
uint16_t rx_start, rx_end;
uint16_t tx_start, tx_end;
-   bool vlan_strip;
+   int mask;
 
PMD_INIT_FUNC_TRACE();
 
@@ -1594,9 +1601,9 @@ nicvf_vf_start(struct rte_eth_dev *dev, struct nicvf 
*nic, uint32_t rbdrsz)
 nic->rbdr->tail, nb_rbdr_desc, nic->vf_id);
 
/* Configure VLAN Strip */
-   vlan_strip = !!(dev->data->dev_conf.rxmode.offloads &
-   DEV_RX_OFFLOAD_VLAN_STRIP);
-   nicvf_vlan_hw_strip(nic, vlan_strip);
+   mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
+   ETH_VLAN_EXTEND_MASK;
+   ret = nicvf_vlan_offload_config(dev, mask);
 
/* Based on the packet type(IPv4 or IPv6), the nicvf HW aligns L3 data
 * to the 64bit memory address.
@@ -1990,6 +1997,7 @@ static const struct eth_dev_ops nicvf_eth_dev_ops = {
.dev_infos_get= nicvf_dev_info_get,
.dev_supported_ptypes_get = nicvf_dev_supported_ptypes_get,
.mtu_set  = nicvf_dev_set_mtu,
+   .vlan_offload_set = nicvf_vlan_offload_set,
.reta_update  = nicvf_dev_reta_update,
.reta_query   = nicvf_dev_reta_query,
.rss_hash_update  = nicvf_dev_rss_hash_update,
@@ -2006,6 +2014,30 @@ static const struct eth_dev_ops nicvf_eth_dev_ops = {
.get_reg  = nicvf_dev_get_regs,
 };
 
+static int
+nicvf_vlan_offload_config(struct rte_eth_dev *dev, int mask)
+{
+   struct rte_eth_rxmode *rxmode;
+   struct nicvf *nic = nicvf_pmd_priv(dev);
+   rxmode = &dev->data->dev_conf.rxmode;
+   if (mask & ETH_VLAN_STRIP_MASK) {
+   if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+   nicvf_vlan_hw_strip(nic, true);
+   else
+  

[dpdk-dev] Memory allocated using rte_zmalloc() has non-zeros

[Andrew Rybchenko]

Hi Anatoly,

I'm investigating issue which finally comes to the fact that memory 
allocated using

rte_zmalloc() has non zeros.

If I add memset just after allocation, everything is perfect and works fine.

I've found out that memset was removed from rte_zmalloc_socket() some 
time ago:


>>>
commit b78c9175118f7d61022ddc5c62ce54a1bd73cea5
Author: Sergio Gonzalez Monroy 
Date:   Tue Jul 5 12:01:16 2016 +0100

    mem: do not zero out memory on zmalloc

    Zeroing out memory on rte_zmalloc_socket is not required anymore 
since all

    allocated memory is already zeroed.

    Signed-off-by: Sergio Gonzalez Monroy 


<<<

but may be something has changed now that made above statement false.

I observe the problem when memory is reallocated. I.e. I configure 7 queues,
start, stop, reconfigure to 3 queues, start. Memory is allocated on 
start and

freed on stop, since we have less queues on the second start it is allocated
in a different way and reuses previously allocated/freed memory.

Do you have any ideas what could be wrong?

Andrew.

Re: [dpdk-dev] [0/9] examples/vm_power: 100% Busy Polling

[Thomas Monjalon]

13/07/2018 10:43, Hunt, David:
> 
> On 13/7/2018 9:33 AM, Thomas Monjalon wrote:
> > 13/07/2018 10:31, Hunt, David:
> >> Hi Thomas,
> >>
> >> On 12/7/2018 8:09 PM, Thomas Monjalon wrote:
> >>> 26/06/2018 11:23, David Hunt:
>  This patch set adds the capability to do out-of-band power
>  monitoring on a system. It uses a thread to monitor the branch
>  counters in the targeted cores, and calculates the branch ratio
>  if the running code.
> 
>  If the branch ratop is low (0.01), then
>  the code is most likely running in a tight poll loop and doing
>  nothing, i.e. receiving no packets. In this case we scale down
>  the frequency of that core.
> 
>  If the branch ratio is higher (>0.01), then it is likely that
>  the code is receiving and processing packets. In this case, we
>  scale up the frequency of that core.
> 
>  The cpu counters are read via /dev/cpu/x/msr, so requires the
>  msr kernel module to be loaded. Because this method is used,
>  the patch set is implemented with one file for x86 systems, and
>  another for non-x86 systems, with conditional compilation in
>  the Makefile. The non-x86 functions are stubs, and do not
>  currently implement any functionality.
> 
>  The vm_power_manager app has been modified to take a new parameter
>   --core-list or -l
>  which takes a list of cores in a comma-separated list format,
>  e.g. 1,3,5-7,9, which resolvest to a core list of 1,3,5,6,7,9
>  These cores will then be enabled for oob monitoring. When the
>  OOB monitoring thread starts, it reads the branch hits/miss
>  counters of each monitored core, and scales up/down accordingly.
> >>> It looks to be a feature which could be integrated in DPDK libs.
> >>> Why choosing to implement it fully in an example?
> >> I needed to set up a thread that looped tightly (~100uS interval) and
> >> run it on it's
> >> own core. From what I have seen in other cases, it is usually the
> >> application that
> >> allocates cores and decides what to run on them. I did think about putting
> >> some of it in a library, but for this case I thought it made more sense
> >> to keep
> >> it purely as a sample app.
> > I feel some code deserves to be in a library.
> > For instance, having different implementations per CPU is a good reason
> > to make a library.
> >
> 
> Sure, I can look at moving some of the code into the library in a future 
> release. However, I
> believe it's OK as it is for the current merge window.

I will to pull it in 18.08-rc2 if compilation is fine.

Re: [dpdk-dev] [PATCH] eal: fix circular dependency in EAL proc type detection

[Yao, Lei A]

> -Original Message-
> From: Burakov, Anatoly
> Sent: Wednesday, July 18, 2018 11:54 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce ; Xu, Qian Q
> ; Yao, Lei A ; Lu, PeipeiX
> 
> Subject: [PATCH] eal: fix circular dependency in EAL proc type detection
> 
> Currently, we need runtime dir to put all of our runtime info in,
> including the DPDK shared config. However, we use the shared
> config to determine our proc type, and this happens earlier than
> we actually create the config dir and thus can know where to
> place the config file.
> 
> Fix this by moving runtime dir creation right after the EAL
> arguments parsing, but before proc type autodetection. Also,
> previously we were creating the config file unconditionally,
> even if we specified no_shconf - fix it by only creating
> the config file if no_shconf is not set.
> 
> Fixes: adf1d867361c ("eal: move runtime config file to new location")
> 
> Signed-off-by: Anatoly Burakov 
Tested-by: Yao Lei
This patch pass the test with simple_mp sample. The secondary process
can recognize itself as "secondary process" even use  "--proc-type=auto"
parameter. 
> ---
>  lib/librte_eal/bsdapp/eal/eal.c   | 33 ++-
>  lib/librte_eal/linuxapp/eal/eal.c | 33 ++-
>  2 files changed, 38 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 73cdf07b8..7b399bc9d 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -286,12 +286,17 @@ eal_proc_type_detect(void)
>   enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
>   const char *pathname = eal_runtime_config_path();
> 
> - /* if we can open the file but not get a write-lock we are a secondary
> -  * process. NOTE: if we get a file handle back, we keep that open
> -  * and don't close it to prevent a race condition between multiple
> opens */
> - if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
> - (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
> - ptype = RTE_PROC_SECONDARY;
> + /* if there no shared config, there can be no secondary processes */
> + if (!internal_config.no_shconf) {
> + /* if we can open the file but not get a write-lock we are a
> +  * secondary process. NOTE: if we get a file handle back, we
> +  * keep that open and don't close it to prevent a race
> condition
> +  * between multiple opens.
> +  */
> + if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
> + (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
> + ptype = RTE_PROC_SECONDARY;
> + }
> 
>   RTE_LOG(INFO, EAL, "Auto-detected process type: %s\n",
>   ptype == RTE_PROC_PRIMARY ? "PRIMARY" :
> "SECONDARY");
> @@ -468,6 +473,14 @@ eal_parse_args(int argc, char **argv)
>   }
>   }
> 
> + /* create runtime data directory */
> + if (internal_config.no_shconf == 0 &&
> + eal_create_runtime_dir() < 0) {
> + RTE_LOG(ERR, EAL, "Cannot create runtime directory\n");
> + ret = -1;
> + goto out;
> + }
> +
>   if (eal_adjust_config(&internal_config) != 0) {
>   ret = -1;
>   goto out;
> @@ -600,14 +613,6 @@ rte_eal_init(int argc, char **argv)
>   return -1;
>   }
> 
> - /* create runtime data directory */
> - if (internal_config.no_shconf == 0 &&
> - eal_create_runtime_dir() < 0) {
> - rte_eal_init_alert("Cannot create runtime directory\n");
> - rte_errno = EACCES;
> - return -1;
> - }
> -
>   /* FreeBSD always uses legacy memory model */
>   internal_config.legacy_mem = true;
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
> b/lib/librte_eal/linuxapp/eal/eal.c
> index d75ae9dae..d2d5aae80 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -344,12 +344,17 @@ eal_proc_type_detect(void)
>   enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
>   const char *pathname = eal_runtime_config_path();
> 
> - /* if we can open the file but not get a write-lock we are a secondary
> -  * process. NOTE: if we get a file handle back, we keep that open
> -  * and don't close it to prevent a race condition between multiple
> opens */
> - if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
> - (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
> - ptype = RTE_PROC_SECONDARY;
> + /* if there no shared config, there can be no secondary processes */
> + if (!internal_config.no_shconf) {
> + /* if we can open the file but not get a write-lock we are a
> +  * secondary process. NOTE: if we get a file handle back, we
> +  * keep that open and don't close it to prevent a race
> c

Re: [dpdk-dev] [PATCH] net/pcap: set queue started and stopped

[Eads, Gage]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Wednesday, July 18, 2018 9:25 AM
> To: Eads, Gage ; dev@dpdk.org
> Subject: Re: [PATCH] net/pcap: set queue started and stopped
> 
> On 7/18/2018 3:17 PM, Eads, Gage wrote:
> >
> >
> >> -Original Message-
> >> From: Yigit, Ferruh
> >> Sent: Wednesday, July 18, 2018 4:14 AM
> >> To: Eads, Gage ; dev@dpdk.org
> >> Subject: Re: [PATCH] net/pcap: set queue started and stopped
> >>
> >> On 7/9/2018 9:21 PM, Gage Eads wrote:
> >>> Set the rx and tx queue state appropriately when the queues or
> >>> device are started or stopped.
> >>
> >> Is there a specific reason to enable these dev_ops, if so can you
> >> please document in commit log?
> >
> > Yes, the purpose of the patch is to enable the rte_eth_dev_{rx,
> tx}_queue_{start, stop} functions for the PCAP PMD. I'll update the message in
> v2.
> 
> I guess that part is clear :) I was asking if there is a higher level reason 
> to enable
> queue start/stop on these PMDs?
> Is there some specific usecase not working for you when these are not enabled?
> 

We have an application that uses the start/stop functions for deferred queue 
starting, and runs with a variety of PMDs. Even though the PCAP PMD doesn't 
have any notion "deferred start", some of the other PMDs we use do.

We've also got some local changes that, if RTE_LIBRTE_ETHDEV_DEBUG is true, 
will return an error if you try to receive or transmit from a queue whose state 
is STOPPED. Without the PCAP patch, this debug check fails. We're looking into 
submitting that debug code in the future, but in the meantime we wanted to make 
the PCAP compliant with the start/stop ethdev functions.

> >
> >>
> >>>
> >>> Signed-off-by: Gage Eads 
> >>> ---
> >>>  drivers/net/pcap/rte_eth_pcap.c | 42
> >>> +
> >>>  1 file changed, 42 insertions(+)
> >>>
> >>> diff --git a/drivers/net/pcap/rte_eth_pcap.c
> >>> b/drivers/net/pcap/rte_eth_pcap.c index 6bd4a7d79..21e466bcd 100644
> >>> --- a/drivers/net/pcap/rte_eth_pcap.c
> >>> +++ b/drivers/net/pcap/rte_eth_pcap.c
> >>> @@ -430,6 +430,10 @@ eth_dev_start(struct rte_eth_dev *dev)
> >>>   return -1;
> >>>   rx->pcap = tx->pcap;
> >>>   }
> >>> +
> >>> + dev->data->tx_queue_state[0] =
> >> RTE_ETH_QUEUE_STATE_STARTED;
> >>> + dev->data->rx_queue_state[0] =
> >> RTE_ETH_QUEUE_STATE_STARTED;
> >>
> >> pcap also supports multiple queue, instead of hardcoding the queue 0
> >> it can be possible to iterate through dev->data->nb_rx_queues,
> >> dev->data-
> >>> nb_tx_queues.
> >>
> >> And I think it is not good to set this in "internals->single_iface"
> >> condition, it is better to do these assignments just above
> >> "status_up" after all queues initialized.
> >>
> >>> +
> >>>   goto status_up;
> >>>   }
> >>>
> >>> @@ -490,6 +494,8 @@ eth_dev_stop(struct rte_eth_dev *dev)
> >>>   pcap_close(tx->pcap);
> >>>   tx->pcap = NULL;
> >>>   rx->pcap = NULL;
> >>> + dev->data->tx_queue_state[0] =
> >> RTE_ETH_QUEUE_STATE_STOPPED;
> >>> + dev->data->rx_queue_state[0] =
> >> RTE_ETH_QUEUE_STATE_STOPPED;
> >>
> >> same here, just above "status_down" is better place and by using
> >> dev->data->nb_[r/t]x_queues
> >
> > Agreed, I will move the started and stopped assignments as you suggested.
> >

Re: [dpdk-dev] Memory allocated using rte_zmalloc() has non-zeros

[Richardson, Bruce]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Andrew Rybchenko
> Sent: Wednesday, July 18, 2018 4:20 PM
> To: Burakov, Anatoly 
> Cc: dev@dpdk.org; Sergio Gonzalez Monroy
> 
> Subject: [dpdk-dev] Memory allocated using rte_zmalloc() has non-zeros
> 
> Hi Anatoly,
> 
> I'm investigating issue which finally comes to the fact that memory
> allocated using
> rte_zmalloc() has non zeros.
> 
> If I add memset just after allocation, everything is perfect and works
> fine.
> 
> I've found out that memset was removed from rte_zmalloc_socket() some time
> ago:
> 
>  >>>
> commit b78c9175118f7d61022ddc5c62ce54a1bd73cea5
> Author: Sergio Gonzalez Monroy 
> Date:   Tue Jul 5 12:01:16 2016 +0100
> 
>      mem: do not zero out memory on zmalloc
> 
>      Zeroing out memory on rte_zmalloc_socket is not required anymore
> since all
>      allocated memory is already zeroed.
> 
>      Signed-off-by: Sergio Gonzalez Monroy
> 
> <<<
> 
> but may be something has changed now that made above statement false.
> 
> I observe the problem when memory is reallocated. I.e. I configure 7
> queues, start, stop, reconfigure to 3 queues, start. Memory is allocated
> on start and freed on stop, since we have less queues on the second start
> it is allocated in a different way and reuses previously allocated/freed
> memory.
> 
> Do you have any ideas what could be wrong?
> 
Previously, the memory used to be zeroed on free, but if it's non-zero on 
realloc 
then it's likely that that has been dropped somewhere along the line.

/Bruce

Re: [dpdk-dev] [PATCH] net/pcap: set queue started and stopped

[Ferruh Yigit]

On 7/18/2018 5:04 PM, Eads, Gage wrote:
> 
> 
>> -Original Message-
>> From: Yigit, Ferruh
>> Sent: Wednesday, July 18, 2018 9:25 AM
>> To: Eads, Gage ; dev@dpdk.org
>> Subject: Re: [PATCH] net/pcap: set queue started and stopped
>>
>> On 7/18/2018 3:17 PM, Eads, Gage wrote:
>>>
>>>
 -Original Message-
 From: Yigit, Ferruh
 Sent: Wednesday, July 18, 2018 4:14 AM
 To: Eads, Gage ; dev@dpdk.org
 Subject: Re: [PATCH] net/pcap: set queue started and stopped

 On 7/9/2018 9:21 PM, Gage Eads wrote:
> Set the rx and tx queue state appropriately when the queues or
> device are started or stopped.

 Is there a specific reason to enable these dev_ops, if so can you
 please document in commit log?
>>>
>>> Yes, the purpose of the patch is to enable the rte_eth_dev_{rx,
>> tx}_queue_{start, stop} functions for the PCAP PMD. I'll update the message 
>> in
>> v2.
>>
>> I guess that part is clear :) I was asking if there is a higher level reason 
>> to enable
>> queue start/stop on these PMDs?
>> Is there some specific usecase not working for you when these are not 
>> enabled?
>>
> 
> We have an application that uses the start/stop functions for deferred queue 
> starting, and runs with a variety of PMDs. Even though the PCAP PMD doesn't 
> have any notion "deferred start", some of the other PMDs we use do.
> 
> We've also got some local changes that, if RTE_LIBRTE_ETHDEV_DEBUG is true, 
> will return an error if you try to receive or transmit from a queue whose 
> state is STOPPED. Without the PCAP patch, this debug check fails. We're 
> looking into submitting that debug code in the future, but in the meantime we 
> wanted to make the PCAP compliant with the start/stop ethdev functions.

Got it, thanks for clarification.

> 
>>>

>
> Signed-off-by: Gage Eads 
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 42
> +
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c
> b/drivers/net/pcap/rte_eth_pcap.c index 6bd4a7d79..21e466bcd 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -430,6 +430,10 @@ eth_dev_start(struct rte_eth_dev *dev)
>   return -1;
>   rx->pcap = tx->pcap;
>   }
> +
> + dev->data->tx_queue_state[0] =
 RTE_ETH_QUEUE_STATE_STARTED;
> + dev->data->rx_queue_state[0] =
 RTE_ETH_QUEUE_STATE_STARTED;

 pcap also supports multiple queue, instead of hardcoding the queue 0
 it can be possible to iterate through dev->data->nb_rx_queues,
 dev->data-
> nb_tx_queues.

 And I think it is not good to set this in "internals->single_iface"
 condition, it is better to do these assignments just above
 "status_up" after all queues initialized.

> +
>   goto status_up;
>   }
>
> @@ -490,6 +494,8 @@ eth_dev_stop(struct rte_eth_dev *dev)
>   pcap_close(tx->pcap);
>   tx->pcap = NULL;
>   rx->pcap = NULL;
> + dev->data->tx_queue_state[0] =
 RTE_ETH_QUEUE_STATE_STOPPED;
> + dev->data->rx_queue_state[0] =
 RTE_ETH_QUEUE_STATE_STOPPED;

 same here, just above "status_down" is better place and by using
 dev->data->nb_[r/t]x_queues
>>>
>>> Agreed, I will move the started and stopped assignments as you suggested.
>>>
> 

[dpdk-dev] [PATCH v2] net/pcap: set queue started and stopped

[Gage Eads]

Set the rx and tx queue state appropriately when the queues or device are
started or stopped. This enables usage of the ethdev rx/tx queue start/stop
functions with the PCAP PMD.

Signed-off-by: Gage Eads 
---
v2 changes:
- Expand the commit message
- Fix queue start/stop state setting in eth_dev_start and eth_dev_stop

 drivers/net/pcap/rte_eth_pcap.c | 49 +
 1 file changed, 49 insertions(+)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 6bd4a7d79..b58350a9e 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -430,6 +430,7 @@ eth_dev_start(struct rte_eth_dev *dev)
return -1;
rx->pcap = tx->pcap;
}
+
goto status_up;
}
 
@@ -465,6 +466,12 @@ eth_dev_start(struct rte_eth_dev *dev)
}
 
 status_up:
+   for (i = 0; i < dev->data->nb_rx_queues; i++)
+   dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
+
+   for (i = 0; i < dev->data->nb_tx_queues; i++)
+   dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
+
dev->data->dev_link.link_status = ETH_LINK_UP;
 
return 0;
@@ -517,6 +524,12 @@ eth_dev_stop(struct rte_eth_dev *dev)
}
 
 status_down:
+   for (i = 0; i < dev->data->nb_rx_queues; i++)
+   dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
+
+   for (i = 0; i < dev->data->nb_tx_queues; i++)
+   dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
+
dev->data->dev_link.link_status = ETH_LINK_DOWN;
 }
 
@@ -643,6 +656,38 @@ eth_tx_queue_setup(struct rte_eth_dev *dev,
return 0;
 }
 
+static int
+eth_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
+{
+   dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
+
+   return 0;
+}
+
+static int
+eth_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id)
+{
+   dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED;
+
+   return 0;
+}
+
+static int
+eth_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id)
+{
+   dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
+
+   return 0;
+}
+
+static int
+eth_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
+{
+   dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
+
+   return 0;
+}
+
 static const struct eth_dev_ops ops = {
.dev_start = eth_dev_start,
.dev_stop = eth_dev_stop,
@@ -651,6 +696,10 @@ static const struct eth_dev_ops ops = {
.dev_infos_get = eth_dev_info,
.rx_queue_setup = eth_rx_queue_setup,
.tx_queue_setup = eth_tx_queue_setup,
+   .rx_queue_start = eth_rx_queue_start,
+   .tx_queue_start = eth_tx_queue_start,
+   .rx_queue_stop = eth_rx_queue_stop,
+   .tx_queue_stop = eth_tx_queue_stop,
.rx_queue_release = eth_queue_release,
.tx_queue_release = eth_queue_release,
.link_update = eth_link_update,
-- 
2.13.6

Re: [dpdk-dev] Memory allocated using rte_zmalloc() has non-zeros

[Burakov, Anatoly]

On 18-Jul-18 4:20 PM, Andrew Rybchenko wrote:

Hi Anatoly,

I'm investigating issue which finally comes to the fact that memory 
allocated using

rte_zmalloc() has non zeros.

If I add memset just after allocation, everything is perfect and works 
fine.


I've found out that memset was removed from rte_zmalloc_socket() some 
time ago:


 >>>
commit b78c9175118f7d61022ddc5c62ce54a1bd73cea5
Author: Sergio Gonzalez Monroy 
Date:   Tue Jul 5 12:01:16 2016 +0100

     mem: do not zero out memory on zmalloc

     Zeroing out memory on rte_zmalloc_socket is not required anymore 
since all

     allocated memory is already zeroed.

     Signed-off-by: Sergio Gonzalez Monroy 


<<<

but may be something has changed now that made above statement false.

I observe the problem when memory is reallocated. I.e. I configure 7 
queues,
start, stop, reconfigure to 3 queues, start. Memory is allocated on 
start and
freed on stop, since we have less queues on the second start it is 
allocated

in a different way and reuses previously allocated/freed memory.

Do you have any ideas what could be wrong?

Andrew.




Hi Andrew,

I will look into it first thing tomorrow. In general, we memset(0) on 
free, and kernel gives us zeroed out pages initially, so the most likely 
point of failure is that i'm not overwring some malloc headers correctly 
on free.


--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v3 1/2] net/thunderx: enable Rx checksum offload

[Jerin Jacob]

-Original Message-
> Date: Wed, 18 Jul 2018 20:35:01 +0530
> From: Pavan Nikhilesh 
> To: jerin.ja...@caviumnetworks.com, santosh.shu...@caviumnetworks.com,
>  rkuduruma...@caviumnetworks.com, ferruh.yi...@intel.com
> Cc: dev@dpdk.org, Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v3 1/2] net/thunderx: enable Rx checksum offload
> X-Mailer: git-send-email 2.18.0
> 
> Add L3/L4 Rx checksum offload and update capabilities.
> 
> Signed-off-by: Pavan Nikhilesh 


Acked-by: Jerin Jacob 

Re: [dpdk-dev] [PATCH v3 2/2] net/thunderx: add support for Rx VLAN offload

[Jerin Jacob]

-Original Message-
> Date: Wed, 18 Jul 2018 20:35:02 +0530
> From: Pavan Nikhilesh 
> To: jerin.ja...@caviumnetworks.com, santosh.shu...@caviumnetworks.com,
>  rkuduruma...@caviumnetworks.com, ferruh.yi...@intel.com
> Cc: dev@dpdk.org, "Kudurumalla, Rakesh" ,
>  Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH v3 2/2] net/thunderx: add support for Rx VLAN
>  offload
> X-Mailer: git-send-email 2.18.0
> 
> From: "Kudurumalla, Rakesh" 
> 
> This feature is used to offload stripping of vlan header from recevied
> packets and update vlan_tci field in mbuf when
> DEV_RX_OFFLOAD_VLAN_STRIP & ETH_VLAN_STRIP_MASK flag is set.
> 
> Signed-off-by: Rakesh Kudurumalla 
> Signed-off-by: Pavan Nikhilesh 

Acked-by: Jerin Jacob 

Re: [dpdk-dev] [PATCH] app/eventdev: use proper teardown sequence

[Jerin Jacob]

-Original Message-
> Date: Tue, 17 Jul 2018 20:03:07 +0530
> From: Pavan Nikhilesh 
> To: jerin.ja...@caviumnetworks.com, santosh.shu...@caviumnetworks.com
> Cc: dev@dpdk.org, Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH] app/eventdev: use proper teardown sequence
> X-Mailer: git-send-email 2.18.0
> 
> Use proper teardown sequence when SIGINT is caught to prevent
> eventdev from going into undefined state.
> 
> Signed-off-by: Pavan Nikhilesh 
> ---
>  app/test-eventdev/evt_main.c | 6 +-
>  app/test-eventdev/test_pipeline_common.c | 1 -
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-eventdev/evt_main.c b/app/test-eventdev/evt_main.c
> index 57bb94570..bc25fb386 100644
> --- a/app/test-eventdev/evt_main.c
> +++ b/app/test-eventdev/evt_main.c
> @@ -25,8 +25,12 @@ signal_handler(int signum)
>   signum);
>   /* request all lcores to exit from the main loop */
>   *(int *)test->test_priv = true;
> - rte_wmb();
>  
> + if (test->ops.ethdev_destroy)
> + test->ops.ethdev_destroy(test, &opt);

I think, stopping the producer should be enough to have proper tear down
sequence, as, When producer stops, the dequeue eventually will not have the
events and it will exit properly. 


> +
> + rte_event_dev_stop(opt.dev_id);
> + rte_wmb();
>   rte_eal_mp_wait_lcore();
>  
>   if (test->ops.test_result)
> diff --git a/app/test-eventdev/test_pipeline_common.c 
> b/app/test-eventdev/test_pipeline_common.c
> index 719518ff3..70fd04517 100644
> --- a/app/test-eventdev/test_pipeline_common.c
> +++ b/app/test-eventdev/test_pipeline_common.c
> @@ -476,7 +476,6 @@ pipeline_eventdev_destroy(struct evt_test *test, struct 
> evt_options *opt)
>  {
>   RTE_SET_USED(test);
>  
> - rte_event_dev_stop(opt->dev_id);
>   rte_event_dev_close(opt->dev_id);
>  }
>  
> -- 
> 2.18.0
> 

[dpdk-dev] [Bug 2] Test bugs for checking the bug flow, no need any fix

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=2

Ajit Khaparde (ajit.khapa...@broadcom.com) changed:

   What|Removed |Added

 Status|IN_PROGRESS |RESOLVED
 Resolution|--- |INVALID

--- Comment #4 from Ajit Khaparde (ajit.khapa...@broadcom.com) ---
This was a test bug to check bug flow. Closing!

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: [dpdk-dev] Memory allocated using rte_zmalloc() has non-zeros

[Andrew Rybchenko]

On 18.07.2018 20:18, Burakov, Anatoly wrote:

On 18-Jul-18 4:20 PM, Andrew Rybchenko wrote:

Hi Anatoly,

I'm investigating issue which finally comes to the fact that memory 
allocated using

rte_zmalloc() has non zeros.

If I add memset just after allocation, everything is perfect and 
works fine.


I've found out that memset was removed from rte_zmalloc_socket() some 
time ago:


 >>>
commit b78c9175118f7d61022ddc5c62ce54a1bd73cea5
Author: Sergio Gonzalez Monroy 
Date:   Tue Jul 5 12:01:16 2016 +0100

 mem: do not zero out memory on zmalloc

 Zeroing out memory on rte_zmalloc_socket is not required anymore 
since all

 allocated memory is already zeroed.

 Signed-off-by: Sergio Gonzalez Monroy 


<<<

but may be something has changed now that made above statement false.

I observe the problem when memory is reallocated. I.e. I configure 7 
queues,
start, stop, reconfigure to 3 queues, start. Memory is allocated on 
start and
freed on stop, since we have less queues on the second start it is 
allocated

in a different way and reuses previously allocated/freed memory.

Do you have any ideas what could be wrong?

Andrew.




Hi Andrew,

I will look into it first thing tomorrow. In general, we memset(0) on 
free, and kernel gives us zeroed out pages initially, so the most 
likely point of failure is that i'm not overwring some malloc headers 
correctly on free.


OK, at least now I know how it is supposed to work in theory.

The following region was allocated  (the second number below is pointer 
plus size)

ALLOC 0x7fffa3264080-0x7fffa32640b8

Not zerod address is 16 bytes before:
(gdb) p/x ((uint64_t *)0x7fffa3264070)[0]
$4 = 0x42
(gdb) p/x ((uint64_t  *)0x7fffa3264070)[1]
$5 = 0x80

then freed
FREE 0x7fffa3264080-0x7fffa32640b8

but above values (gdb) are still the same
then it is allocated as the part of bigger memory chunk
ALLOC 0x7fffa3245b80-0x7fffa3265fd8
which should contain zeros, but above values are still the same.

It is interesting that it looks like it was the first block freed on the 
port stop. I'm not 100% sure since I've put printouts to my allocation 
wrapper, not EAL.


Many thanks,
Andrew.

Re: [dpdk-dev] Memory allocated using rte_zmalloc() has non-zeros

[Stephen Hemminger]

On Wed, 18 Jul 2018 22:52:12 +0300
Andrew Rybchenko  wrote:

> On 18.07.2018 20:18, Burakov, Anatoly wrote:
> > On 18-Jul-18 4:20 PM, Andrew Rybchenko wrote:  
> >> Hi Anatoly,
> >>
> >> I'm investigating issue which finally comes to the fact that memory 
> >> allocated using
> >> rte_zmalloc() has non zeros.
> >>
> >> If I add memset just after allocation, everything is perfect and 
> >> works fine.
> >>
> >> I've found out that memset was removed from rte_zmalloc_socket() some 
> >> time ago:
> >>  
> >>  >>>  
> >> commit b78c9175118f7d61022ddc5c62ce54a1bd73cea5
> >> Author: Sergio Gonzalez Monroy 
> >> Date:   Tue Jul 5 12:01:16 2016 +0100
> >>
> >>  mem: do not zero out memory on zmalloc
> >>
> >>  Zeroing out memory on rte_zmalloc_socket is not required anymore 
> >> since all
> >>  allocated memory is already zeroed.
> >>
> >>  Signed-off-by: Sergio Gonzalez Monroy 
> >> 
> >> <<<
> >>
> >> but may be something has changed now that made above statement false.
> >>
> >> I observe the problem when memory is reallocated. I.e. I configure 7 
> >> queues,
> >> start, stop, reconfigure to 3 queues, start. Memory is allocated on 
> >> start and
> >> freed on stop, since we have less queues on the second start it is 
> >> allocated
> >> in a different way and reuses previously allocated/freed memory.
> >>
> >> Do you have any ideas what could be wrong?
> >>
> >> Andrew.
> >>
> >>  
> >
> > Hi Andrew,
> >
> > I will look into it first thing tomorrow. In general, we memset(0) on 
> > free, and kernel gives us zeroed out pages initially, so the most 
> > likely point of failure is that i'm not overwring some malloc headers 
> > correctly on free.  
> 
> OK, at least now I know how it is supposed to work in theory.
> 
> The following region was allocated  (the second number below is pointer 
> plus size)
> ALLOC 0x7fffa3264080-0x7fffa32640b8
> 
> Not zerod address is 16 bytes before:
> (gdb) p/x ((uint64_t *)0x7fffa3264070)[0]
> $4 = 0x42
> (gdb) p/x ((uint64_t  *)0x7fffa3264070)[1]
> $5 = 0x80
> 
> then freed
> FREE 0x7fffa3264080-0x7fffa32640b8
> 
> but above values (gdb) are still the same
> then it is allocated as the part of bigger memory chunk
> ALLOC 0x7fffa3245b80-0x7fffa3265fd8
> which should contain zeros, but above values are still the same.
> 
> It is interesting that it looks like it was the first block freed on the 
> port stop. I'm not 100% sure since I've put printouts to my allocation 
> wrapper, not EAL.
> 
> Many thanks,
> Andrew.

memset here is what is supposed to clear the data.

struct malloc_elem *
malloc_elem_free(struct malloc_elem *elem)
{
void *ptr;
size_t data_len;

ptr = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN + elem->pad);
data_len = elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;

elem = malloc_elem_join_adjacent_free(elem);

malloc_elem_free_list_insert(elem);

elem->pad = 0;

/* decrease heap's count of allocated elements */
elem->heap->alloc_count--;

memset(ptr, 0, data_len);

Maybe data_len is not correct either because of bug, or your application 
clobbered
the malloc reserved regions  in the element.

More likely, gcc is incorrectly optimizing this away.

https://wiki.sei.cmu.edu/confluence/display/c/MSC06-C.+Beware+of+compiler+optimizations
https://www.cryptologie.net/article/419/zeroing-memory-compiler-optimizations-and-memset_s/

[dpdk-dev] [PATCH] devtools: fix symbol check for filename with space

[Thomas Monjalon]

If the patch filename or the temporary file path have a space
in their name, the script check-symbol-change.sh does not work.
The variables for the filenames must be enclosed in quotes
in order to preserve spaces.

Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
Cc: nhor...@tuxdriver.com

Signed-off-by: Thomas Monjalon 
---
 devtools/check-symbol-change.sh | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 9952a8d66..69b874ace 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -7,7 +7,7 @@ build_map_changes()
local fname=$1
local mapdb=$2
 
-   cat $fname | awk '
+   cat "$fname" | awk '
# Initialize our variables
BEGIN {map="";sym="";ar="";sec=""; in_sec=0; in_map=0}
 
@@ -71,10 +71,10 @@ build_map_changes()
print map " " sym " unknown del"
}
}
-   }' > ./$mapdb
+   }' > "$mapdb"
 
-   sort -u $mapdb > ./$mapdb.2
-   mv -f $mapdb.2 $mapdb
+   sort -u "$mapdb" > "$mapdb.2"
+   mv -f "$mapdb.2" "$mapdb"
 
 }
 
@@ -111,7 +111,7 @@ check_for_rule_violations()
# to be moving from an already supported
# section or its a violation
grep -q \
-   "$mname $symname [^EXPERIMENTAL] del" $mapdb
+   "$mname $symname [^EXPERIMENTAL] del" "$mapdb"
if [ $? -ne 0 ]
then
echo -n "ERROR: symbol $symname "
@@ -133,7 +133,7 @@ check_for_rule_violations()
echo "gone through the deprecation process"
fi
fi
-   done < $mapdb
+   done < "$mapdb"
 
return $ret
 }
@@ -150,10 +150,10 @@ clean_and_exit_on_sig()
exit $exit_code
 }
 
-build_map_changes $patch $mapfile
-check_for_rule_violations $mapfile
+build_map_changes "$patch" "$mapfile"
+check_for_rule_violations "$mapfile"
 exit_code=$?
 
-rm -f $mapfile
+rm -f "$mapfile"
 
 exit $exit_code
-- 
2.17.1

Re: [dpdk-dev] [PATCH] mem: fix alignment of requested virtual areas

[Thomas Monjalon]

17/07/2018 11:48, Stojaczyk, DariuszX:
> From: Burakov, Anatoly
> > 
> > The original code did not align any addresses that were requested as
> > page-aligned, but were different because addr_is_hint was set.
> > 
> > Below fix by Dariusz has introduced an issue where all unaligned addresses
> > were left as unaligned.
> > 
> > This patch is a partial revert of
> > commit 7fa7216ed48d ("mem: fix alignment of requested virtual areas")
> > 
> > and implements a proper fix for this issue, by asking for alignment in all
> > but the following two cases:
> > 
> > 1) page size is equal to system page size, or
> > 2) we got an aligned requested address, and will not accept a different one
> > 
> > This ensures that alignment is performed in all cases, except for those we
> > can guarantee that the address will not need alignment.
> > 
> > Fixes: b7cc54187ea4 ("mem: move virtual area function in common directory")
> > Fixes: 7fa7216ed48d ("mem: fix alignment of requested virtual areas")
> > Cc: dariuszx.stojac...@intel.com
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Anatoly Burakov 
> 
> Acked-by: Dariusz Stojaczyk 

Applied, thanks

[dpdk-dev] [PATCH v2] devtools: fix symbol check for filename with space

[Thomas Monjalon]

If the patch filename or the temporary file path have a space
in their name, the script check-symbol-change.sh does not work.
The variables for the filenames must be enclosed in quotes
in order to preserve spaces.

Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
Cc: nhor...@tuxdriver.com

Signed-off-by: Thomas Monjalon 
---
v2: one occurence of "$mapfile" was missed in v1
---
 devtools/check-symbol-change.sh | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 9952a8d66..40b72073a 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -7,7 +7,7 @@ build_map_changes()
local fname=$1
local mapdb=$2
 
-   cat $fname | awk '
+   cat "$fname" | awk '
# Initialize our variables
BEGIN {map="";sym="";ar="";sec=""; in_sec=0; in_map=0}
 
@@ -71,10 +71,10 @@ build_map_changes()
print map " " sym " unknown del"
}
}
-   }' > ./$mapdb
+   }' > "$mapdb"
 
-   sort -u $mapdb > ./$mapdb.2
-   mv -f $mapdb.2 $mapdb
+   sort -u "$mapdb" > "$mapdb.2"
+   mv -f "$mapdb.2" "$mapdb"
 
 }
 
@@ -111,7 +111,7 @@ check_for_rule_violations()
# to be moving from an already supported
# section or its a violation
grep -q \
-   "$mname $symname [^EXPERIMENTAL] del" $mapdb
+   "$mname $symname [^EXPERIMENTAL] del" "$mapdb"
if [ $? -ne 0 ]
then
echo -n "ERROR: symbol $symname "
@@ -133,7 +133,7 @@ check_for_rule_violations()
echo "gone through the deprecation process"
fi
fi
-   done < $mapdb
+   done < "$mapdb"
 
return $ret
 }
@@ -146,14 +146,14 @@ exit_code=1
 
 clean_and_exit_on_sig()
 {
-   rm -f $mapfile
+   rm -f "$mapfile"
exit $exit_code
 }
 
-build_map_changes $patch $mapfile
-check_for_rule_violations $mapfile
+build_map_changes "$patch" "$mapfile"
+check_for_rule_violations "$mapfile"
 exit_code=$?
 
-rm -f $mapfile
+rm -f "$mapfile"
 
 exit $exit_code
-- 
2.17.1

Re: [dpdk-dev] [PATCH] examples/l2fwd-crypto: fix digest with AEAD algorithms

[Dwivedi, Ankur]

Hi Pablo,


This patch solves the bug.


Thanks

Ankur


From: Pablo de Lara 
Sent: Monday, July 16, 2018 1:56:16 PM
To: Dwivedi, Ankur; declan.dohe...@intel.com
Cc: dev@dpdk.org; Pablo de Lara; sta...@dpdk.org
Subject: [PATCH] examples/l2fwd-crypto: fix digest with AEAD algorithms

External Email

When performing authentication verification (both for AEAD algorithms,
such as AES-GCM, or for authentication algorithms, such as SHA1-HMAC),
the digest address is calculated based on the packet size and the
algorithm used (substracting digest size and IP header to the packet size).

However, for AEAD algorithms, this was not calculated correctly,
since the digest size was not being substracted.
Signed-off-by: Pablo de Lara 

Bugzilla ID: 44
Fixes: 2661f4fbe93d ("examples/l2fwd-crypto: add AEAD parameters")
Cc: sta...@dpdk.org
---
 examples/l2fwd-crypto/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index 9d6bb7857..9ac06a697 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -408,7 +408,7 @@ 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)
+   if ((cparams->do_hash || cparams->do_aead) && cparams->hash_verify)
data_len -= cparams->digest_length;

if (cparams->do_cipher) {
--
2.14.4

[dpdk-dev] [RFC] mem: poison memory when freed

[Stephen Hemminger]

DPDK malloc library allows broken programs to work because
the semantics of zmalloc and malloc are the same.

This patch changes to a more secure model which will catch
(and crash) programs that reuse memory already freed.

This supersedes earlier changes to zero memory on free and
avoid zeroing memory in zmalloc.

Signed-off-by: Stephen Hemminger 
---
 lib/librte_eal/common/malloc_elem.c | 5 -
 lib/librte_eal/common/rte_malloc.c  | 6 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/malloc_elem.c 
b/lib/librte_eal/common/malloc_elem.c
index efcb82677198..62cc0b385c0c 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -23,6 +23,8 @@
 #include "malloc_elem.h"
 #include "malloc_heap.h"
 
+#define MALLOC_POISON 0x6b  /**< Free memory. */
+
 size_t
 malloc_elem_find_max_iova_contig(struct malloc_elem *elem, size_t align)
 {
@@ -531,7 +533,8 @@ malloc_elem_free(struct malloc_elem *elem)
/* decrease heap's count of allocated elements */
elem->heap->alloc_count--;
 
-   memset(ptr, 0, data_len);
+   /* poison memory */
+   memset(ptr, MALLOC_POISON, data_len);
 
return elem;
 }
diff --git a/lib/librte_eal/common/rte_malloc.c 
b/lib/librte_eal/common/rte_malloc.c
index b51a6d111bde..b33c936fd491 100644
--- a/lib/librte_eal/common/rte_malloc.c
+++ b/lib/librte_eal/common/rte_malloc.c
@@ -70,7 +70,11 @@ rte_malloc(const char *type, size_t size, unsigned align)
 void *
 rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
 {
-   return rte_malloc_socket(type, size, align, socket);
+   void *ptr = rte_malloc_socket(type, size, align, socket);
+
+   if (ptr != NULL)
+   memset(ptr, 0, size);
+   return ptr;
 }
 
 /*
-- 
2.18.0

Re: [dpdk-dev] [PATCH] vhost: fix buffer length calculation

[Wang, Yinan]

On Tue, Jul 17, 2018 at 09:10:35PM +0800, Tiwei Bie wrote:
> Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")
> 
> Reported-by: Yinan Wang 
> Signed-off-by: Tiwei Bie 

Above fix can fix below issue:
https://mails.dpdk.org/archives/dev/2018-July/108137.html

Tested-by: Yinan Wang 

Re: [dpdk-dev] Does lthread_cond_wait need a mutex?

[wubenqing]

Hi~

If the lthreads run on different lcores, a race condition with lthread_mutex 
may occur.
Like this:
lthread1 run on lcore=1
lthread2 run on lcore=2
If the mutex is owned by lthread2, lthread1 try to lock the mutex that will 
block thread1. lthread_sched on lcore1 will insert lthread1 to the blocked 
queue of the mutex. lthread1 blocks until lthread2 unlock the mutex.
Is that right?

Let's go back to the previous question.

Refer to http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_cond_wait.html
"The pthread_cond_wait() and pthread_cond_timedwait() functions are used to 
block on a condition variable. They are called with mutex locked by the calling 
thread or undefined behaviour will result.

These functions atomically release mutex and cause the calling thread to block 
on the condition variable cond; atomically here means "atomically with respect 
to access by another thread to the mutex and then the condition variable"."


So I think there is a problem with pthread_cond_wait implemented by lthread. If 
that is the case, could lthread fix this problem?

Regards,
Wubenqing



From: Wiles, Keith
Date: 2018-07-18 23:01
To: 吴本卿(研五 福州)
CC: dev@dpdk.org
Subject: Re: [dpdk-dev] Does lthread_cond_wait need a mutex?



> On Jul 17, 2018, at 10:43 PM, wubenq...@ruijie.com.cn wrote:
>
> Hi~
>Reference: 
> http://doc.dpdk.org/guides-18.05/sample_app_ug/performance_thread.html?highlight=lthread
>The L-thread subsystem provides a set of functions that are logically 
> equivalent to the corresponding functions offered by the POSIX pthread 
> library.
>I think there is a bug with pthread_cond_wait of lthread implement.
>Look at this code, there are two lthread:
>
> lthread1:
>pthread_mutex_lock(mutex); //a1
>if (predicate == FALSE) {//a2
>pthread_cond_wait(cond, mutex)//a3
>}
>pthread_mutex_unlock(mutex);//a4
>
> int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
> {
> if (override) {
> pthread_mutex_unlock(mutex); //a31
> int rv = lthread_cond_wait(*(struct lthread_cond **)cond, 0); //a32
>
> pthread_mutex_lock(mutex); //a33
> return rv;
> }
> return _sys_pthread_funcs.f_pthread_cond_wait(cond, mutex);
> }
>
> lthread2:
>pthread_mutex_lock(mutex);//b1
>predicate = TRUE;//b2
>pthread_mutex_unlock(mutex);//b3
>pthread_cond_signal(cond);//b4
>
>
>If the sequence is:
>a1->a2->a31->b1->b2->b3->b4->a32
>Will lthread1 sleep forever?

Maybe is it possible, my brain is not working this morning. Please remember 
that lthreads must give up control or lthread will continue to and can not be 
preempted.

Does that fix the problem?

>
> 
> 吴本卿(研五 福州)

Regards,
Keith

Re: [dpdk-dev] [PATCH] vhost: fix buffer length calculation

[Tiwei Bie]

On Tue, Jul 17, 2018 at 09:10:35PM +0800, Tiwei Bie wrote:
> Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")
> 
> Reported-by: Yinan Wang 
> Signed-off-by: Tiwei Bie 

Applied to dpdk-next-virtio/master, thanks.

Re: [dpdk-dev] Does lthread_cond_wait need a mutex?

[Wiles, Keith]

> On Jul 18, 2018, at 7:34 PM, wubenq...@ruijie.com.cn wrote:
> 
> Hi~
> 
> If the lthreads run on different lcores, a race condition with lthread_mutex 
> may occur.
> Like this:
> lthread1 run on lcore=1
> lthread2 run on lcore=2
> If the mutex is owned by lthread2, lthread1 try to lock the mutex that will 
> block thread1. lthread_sched on lcore1 will insert lthread1 to the blocked 
> queue of the mutex. lthread1 blocks until lthread2 unlock the mutex. 
> Is that right?
> 
> Let's go back to the previous question. 
> 
> Refer to 
> http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_cond_wait.html
> "The pthread_cond_wait() and pthread_cond_timedwait() functions are used to 
> block on a condition variable. They are called with mutex locked by the 
> calling thread or undefined behaviour will result.
> These functions atomically release mutex and cause the calling thread to 
> block on the condition variable cond; atomically here means "atomically with 
> respect to access by another thread to the mutex and then the condition 
> variable”."

Yes, I believe you are correct. I was able to look at my version of lthreads 
and I had changed lthread_cond_wait() to have a mutex argument and removed the 
reserved variable. The same was done for timed wait function. As I remember I 
found a couple minor problems in lthreads, which I fixed. The problem is my 
lthread code is somewhat different then the code in DPDK and my changes would 
not apply to DPDK lthreads.

> 
> So I think there is a problem with pthread_cond_wait implemented by lthread. 
> If that is the case, could lthread fix this problem?
> 
> Regards,
> Wubenqing
> 
>  
> From: Wiles, Keith
> Date: 2018-07-18 23:01
> To: 吴本卿(研五 福州)
> CC: dev@dpdk.org
> Subject: Re: [dpdk-dev] Does lthread_cond_wait need a mutex?
> 
> 
> > On Jul 17, 2018, at 10:43 PM, wubenq...@ruijie.com.cn wrote:
> >
> > Hi~
> >Reference: 
> > http://doc.dpdk.org/guides-18.05/sample_app_ug/performance_thread.html?highlight=lthread
> >The L-thread subsystem provides a set of functions that are logically 
> > equivalent to the corresponding functions offered by the POSIX pthread 
> > library.
> >I think there is a bug with pthread_cond_wait of lthread implement.
> >Look at this code, there are two lthread:
> >
> > lthread1:
> >pthread_mutex_lock(mutex); //a1
> >if (predicate == FALSE) {//a2
> >pthread_cond_wait(cond, mutex)//a3
> >}
> >pthread_mutex_unlock(mutex);//a4
> >
> > int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
> > {
> > if (override) {
> > pthread_mutex_unlock(mutex); //a31
> > int rv = lthread_cond_wait(*(struct lthread_cond **)cond, 0); //a32
> >
> > pthread_mutex_lock(mutex); //a33
> > return rv;
> > }
> > return _sys_pthread_funcs.f_pthread_cond_wait(cond, mutex);
> > }
> >
> > lthread2:
> >pthread_mutex_lock(mutex);//b1
> >predicate = TRUE;//b2
> >pthread_mutex_unlock(mutex);//b3
> >pthread_cond_signal(cond);//b4
> >
> >
> >If the sequence is:
> >a1->a2->a31->b1->b2->b3->b4->a32
> >Will lthread1 sleep forever?
> 
> Maybe is it possible, my brain is not working this morning. Please remember 
> that lthreads must give up control or lthread will continue to and can not be 
> preempted.
> 
> Does that fix the problem?
> 
> >
> > 
> > 吴本卿(研五 福州)
> 
> Regards,
> Keith

Regards,
Keith