date:20200206

Hi,

On Fri, Jan 24, 2020 at 08:25:18PM +, Viacheslav Ovsiienko wrote:
> This patch adds unit test for the mbufs allocated from
> the special pool with pinned external data buffers.
> 
> The pinned buffer mbufs are tested in the same way as
> regular ones with taking into account some specifics
> of cloning/attaching.
> 
> Signed-off-by: Viacheslav Ovsiienko 

Looks good to me, you can add my ack in the v2, once we understand
the issue with verify_mbuf_check_panics().

> @@ -1199,10 +1212,11 @@
>   buf = rte_pktmbuf_alloc(pktmbuf_pool);
>   if (buf == NULL)
>   return -1;
> + /*
>   printf("Checking good mbuf initially\n");
>   if (verify_mbuf_check_panics(buf) != -1)
>   return -1;
> -
> + */
>   printf("Now checking for error conditions\n");
>  
>   if (verify_mbuf_check_panics(NULL)) {
> @@ -2411,6 +2425,120 @@ struct test_case {

Note: on my platform, it still works if I remove this comment.

Regards,
Olivier

Re: [dpdk-dev] [PATCH] app/test: add test for mbuf with pinned external buffer

2020-02-06 Thread Slava Ovsiienko

Olivier, thanks for the reviewing.
I'll remove the comment and send the v2.
I use 1G huge pages, will retest over 2M
and continue finding why my host fails.

With best regards, Slava

> -Original Message-
> From: Olivier Matz 
> Sent: Thursday, February 6, 2020 10:17
> To: Slava Ovsiienko 
> Cc: dev@dpdk.org; Matan Azrad ; Raslan Darawsheh
> ; Ori Kam ; Shahaf Shuler
> ; step...@networkplumber.org;
> tho...@mellanox.net
> Subject: Re: [PATCH] app/test: add test for mbuf with pinned external buffer
> 
> Hi,
> 
> On Fri, Jan 24, 2020 at 08:25:18PM +, Viacheslav Ovsiienko wrote:
> > This patch adds unit test for the mbufs allocated from the special
> > pool with pinned external data buffers.
> >
> > The pinned buffer mbufs are tested in the same way as regular ones
> > with taking into account some specifics of cloning/attaching.
> >
> > Signed-off-by: Viacheslav Ovsiienko 
> 
> Looks good to me, you can add my ack in the v2, once we understand the
> issue with verify_mbuf_check_panics().
> 
> > @@ -1199,10 +1212,11 @@
> > buf = rte_pktmbuf_alloc(pktmbuf_pool);
> > if (buf == NULL)
> > return -1;
> > +   /*
> > printf("Checking good mbuf initially\n");
> > if (verify_mbuf_check_panics(buf) != -1)
> > return -1;
> > -
> > +   */
> > printf("Now checking for error conditions\n");
> >
> > if (verify_mbuf_check_panics(NULL)) { @@ -2411,6 +2425,120 @@
> struct
> > test_case {
> 
> Note: on my platform, it still works if I remove this comment.
> 
> Regards,
> Olivier

Re: [dpdk-dev] BUG: eBPF missing BPF_ABS

2020-02-06 Thread Morten Brørup

> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev,
> Konstantin
> Sent: Wednesday, February 5, 2020 10:16 PM
> 
> >
> > As I mentioned in my FOSDEM talk the current DPDK eBPF handling is
> > not usable for packet filters. I have ported the classic BPF to eBPF
> code
> > and the generated code is not usable by DPDK.
> >
> > The problem is that DPDK eBPF does not implement all the opcodes.
> > BPF_ABS is not implemented and must be. It is in the Linux kernel.
> 
> Yep, it doesn't.
> This is not a generic eBPF instruction, but sort of implicit function
> call
> to access skb data. At initial stage of librte_bpf development,
> I didn't think much about cBPF conversion, and to simplify things
> decided to put all special skb features aside.
> But sure, if that will enable DPDK cBPF support, let's add it in.
> Please fill a Bugzilla ticket to track that issue, and I'll try to
> find some time within 20.05 to look at it.
> Unless of course, you or someone else would like to volunteer for it.
> 
> Though at first step, we probably need to decide what should be
> our requirements for it in terms of DPDK.
> From https://www.kernel.org/doc/Documentation/networking/filter.txt:
> "eBPF has two non-generic instructions: (BPF_ABS |  | BPF_LD) and
> (BPF_IND |  | BPF_LD) which are used to access packet data.
> They had to be carried over from classic to have strong performance of
> socket filters running in eBPF interpreter. These instructions can only
> be used when interpreter context is a pointer to 'struct sk_buff' and
> have seven implicit operands. Register R6 is an implicit input that
> must
> contain pointer to sk_buff. Register R0 is an implicit output which
> contains
> the data fetched from the packet. Registers R1-R5 are scratch registers
> and must not be used to store the data across BPF_ABS | BPF_LD or
> BPF_IND | BPF_LD instructions.
> These instructions have implicit program exit condition as well. When
> eBPF program is trying to access the data beyond the packet boundary,
> the interpreter will abort the execution of the program. JIT compilers
> therefore must preserve this property. src_reg and imm32 fields are
> explicit inputs to these instructions.
> For example:
> BPF_IND | BPF_W | BPF_LD means:
> R0 = ntohl(*(u32 *) (((struct sk_buff *) R6)->data + src_reg + imm32))
> and R1 - R5 were scratched."
> 
> For RTE_BPF_ARG_PTR_MBUF context we probably want behavior similar
> to linux, i.e. BPF_IND | BPF_W | BPF_LD would mean something like:
> 
> 1) uint32_t tmp;
> R0 = &tmp;
> R0 = rte_pktmbuf_read((const struct rte_mbuf *)R6,  src_reg +
> imm32,
>   sizeof(uint32_t), R0);
>  if (R0 == NULL) return FAILED;
>  R0 = ntohl(*(uint32_t *)R0);
> 
> But what to do with when ctx is raw data buffer (RTE_BPF_ARG_PTR)?
> Should it be just:
> 2) R0 = ntohl(*(uint32_t *)(R6 + src_reg + imm32));
> 3) not allow LD_ABS/LD_IND in this mode at all.
> 

I think that 1) is the correct choice for the "cBPF filter" use case.

In that context I consider both 2) and 3) irrelevant because the 
RTE_BPF_ARG_PTR_MBUF type should be used for cBPF filtering. I have argued for 
this before.

Others have argued for using the RTE_BPF_ARG_PTR type instead. Let's consider 
using the RTE_BPF_ARG_PTR for a moment... Is there an implicit understanding 
that the data points to packet data? Then a range check in 2) might be relevant.

However, if the RTE_BPF_ARG_PTR_MBUF type is supported and used for cBPF->eBPF 
conversion, we would not need to support LD_ABS/LD_IND for the RTE_BPF_ARG_PTR 
type. So between 2) and 3), I support 3).

> Second question is implementation.
> I can see two main options here:
> a) if we plan to have our own cBPF->eBPF converter and support only it,
> we can add these extra instructions generation into converter itself.
> I.E. cBPF->eBPF conversion for LD_ABS/LD_IND will generate series
> of generic eBPF instructions.
> b) support eBPF LD_ABS/LD_IND in eBPF interpreter/jit
> 
> (a) probably a simpler way (eBPF interpreter/jit/verifier would remain
> unchanged),
> but seems way too limited. So I think (b) is a better choice, even more
> work implied
> (interpreter seems more or less straightforward, jit would probably
> need some effort).
> 

This is going to be used in the fast path, probably on all packets on an 
interface. So clearly b).

> Any thoughts/opinions?
> Konstantin

One more piece of information: Linux cBPF supports Auxiliary data (VLAN ID, 
Interface Index, etc.), i.e. metadata that are not part of the packet data, but 
can be found in the sk_buff/mbuf. Going for 1) and b) might make it easier 
adding support for these later.


Med venlig hilsen / kind regards
- Morten Brørup

Re: [dpdk-dev] BUG: eBPF missing BPF_ABS

2020-02-06 Thread Stephen Hemminger

I agree fixing offsets in cbpf to ebpf converter and passing mbuf is
easier. There is still the pathological case of multi segment mbuf. But
Linux XDP doesn't handle it either.

Let me put early version of filter2rteebf on GitHub

On Thu, Feb 6, 2020, 8:54 AM Morten Brørup  wrote:

> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev,
> > Konstantin
> > Sent: Wednesday, February 5, 2020 10:16 PM
> >
> > >
> > > As I mentioned in my FOSDEM talk the current DPDK eBPF handling is
> > > not usable for packet filters. I have ported the classic BPF to eBPF
> > code
> > > and the generated code is not usable by DPDK.
> > >
> > > The problem is that DPDK eBPF does not implement all the opcodes.
> > > BPF_ABS is not implemented and must be. It is in the Linux kernel.
> >
> > Yep, it doesn't.
> > This is not a generic eBPF instruction, but sort of implicit function
> > call
> > to access skb data. At initial stage of librte_bpf development,
> > I didn't think much about cBPF conversion, and to simplify things
> > decided to put all special skb features aside.
> > But sure, if that will enable DPDK cBPF support, let's add it in.
> > Please fill a Bugzilla ticket to track that issue, and I'll try to
> > find some time within 20.05 to look at it.
> > Unless of course, you or someone else would like to volunteer for it.
> >
> > Though at first step, we probably need to decide what should be
> > our requirements for it in terms of DPDK.
> > From https://www.kernel.org/doc/Documentation/networking/filter.txt:
> > "eBPF has two non-generic instructions: (BPF_ABS |  | BPF_LD) and
> > (BPF_IND |  | BPF_LD) which are used to access packet data.
> > They had to be carried over from classic to have strong performance of
> > socket filters running in eBPF interpreter. These instructions can only
> > be used when interpreter context is a pointer to 'struct sk_buff' and
> > have seven implicit operands. Register R6 is an implicit input that
> > must
> > contain pointer to sk_buff. Register R0 is an implicit output which
> > contains
> > the data fetched from the packet. Registers R1-R5 are scratch registers
> > and must not be used to store the data across BPF_ABS | BPF_LD or
> > BPF_IND | BPF_LD instructions.
> > These instructions have implicit program exit condition as well. When
> > eBPF program is trying to access the data beyond the packet boundary,
> > the interpreter will abort the execution of the program. JIT compilers
> > therefore must preserve this property. src_reg and imm32 fields are
> > explicit inputs to these instructions.
> > For example:
> > BPF_IND | BPF_W | BPF_LD means:
> > R0 = ntohl(*(u32 *) (((struct sk_buff *) R6)->data + src_reg + imm32))
> > and R1 - R5 were scratched."
> >
> > For RTE_BPF_ARG_PTR_MBUF context we probably want behavior similar
> > to linux, i.e. BPF_IND | BPF_W | BPF_LD would mean something like:
> >
> > 1) uint32_t tmp;
> > R0 = &tmp;
> > R0 = rte_pktmbuf_read((const struct rte_mbuf *)R6,  src_reg +
> > imm32,
> >   sizeof(uint32_t), R0);
> >  if (R0 == NULL) return FAILED;
> >  R0 = ntohl(*(uint32_t *)R0);
> >
> > But what to do with when ctx is raw data buffer (RTE_BPF_ARG_PTR)?
> > Should it be just:
> > 2) R0 = ntohl(*(uint32_t *)(R6 + src_reg + imm32));
> > 3) not allow LD_ABS/LD_IND in this mode at all.
> >
>
> I think that 1) is the correct choice for the "cBPF filter" use case.
>
> In that context I consider both 2) and 3) irrelevant because the
> RTE_BPF_ARG_PTR_MBUF type should be used for cBPF filtering. I have argued
> for this before.
>
> Others have argued for using the RTE_BPF_ARG_PTR type instead. Let's
> consider using the RTE_BPF_ARG_PTR for a moment... Is there an implicit
> understanding that the data points to packet data? Then a range check in 2)
> might be relevant.
>
> However, if the RTE_BPF_ARG_PTR_MBUF type is supported and used for
> cBPF->eBPF conversion, we would not need to support LD_ABS/LD_IND for the
> RTE_BPF_ARG_PTR type. So between 2) and 3), I support 3).
>
> > Second question is implementation.
> > I can see two main options here:
> > a) if we plan to have our own cBPF->eBPF converter and support only it,
> > we can add these extra instructions generation into converter itself.
> > I.E. cBPF->eBPF conversion for LD_ABS/LD_IND will generate series
> > of generic eBPF instructions.
> > b) support eBPF LD_ABS/LD_IND in eBPF interpreter/jit
> >
> > (a) probably a simpler way (eBPF interpreter/jit/verifier would remain
> > unchanged),
> > but seems way too limited. So I think (b) is a better choice, even more
> > work implied
> > (interpreter seems more or less straightforward, jit would probably
> > need some effort).
> >
>
> This is going to be used in the fast path, probably on all packets on an
> interface. So clearly b).
>
> > Any thoughts/opinions?
> > Konstantin
>
> One more piece of information: Linux cBPF supports Auxiliary data (VLAN
> ID, Interface Index, etc.), i.e. metadata that are not part of the packet

Re: [dpdk-dev] [PATCH v7 9/9] eal: add minimum viable code to support parsing

As discussed in community meeting, the goal was to have core parsing
in Windows EAL 20.02.
Given that there is a crash and a doubt on the imported getopt library,
I think it's better to postpone this series to 20.05.


06/02/2020 07:41, Dmitry Kozlyuk:
> > On 2/5/2020 4:39 PM, Pallavi Kadam wrote:
> > > On 2/5/2020 11:54 AM, Dmitry Kozlyuk wrote:  
> > >> Crashes at argument parsing, WinDbg log attached.  
> > > This patch works fine with meson=0.49.
> > > I was able to execute the 'helloworld' app.
> > >
> > > When I tried to build with meson=0.52 it failed with the error:
> > > [9/25] Linking target lib/librte_kvargs-20.0.dll.
> > > FAILED: lib/librte_kvargs-20.0.dll
> > > clang @lib/librte_kvargs-20.0.dll.rsp
> > > clang: error: no such file or directory: '/OPT:REF'  
> > 
> > Fixed this error with the patch you sent before.
> > were able to execute parsing:
> > 
> > $ ./build/examples/dpdk-helloworld.exe -l 4-8
> > EAL: Detected 20 lcore(s)
> > EAL: Detected 2 NUMA nodes
> > hello from core 5
> > hello from core 6
> > hello from core 7
> > hello from core 8
> > hello from core 4
> > 
> > >
> > > Not completely sure, if this could be the reason for the crash.  
> 
> The crash does not happen with any arguments, "-l 4-8" works fine as do
> "-cf" and "-v", but "--log-level=eal:8" crashes reproducibly. I assume
> application should not crash on invalid options, but report an error and exit.
> For the reference, I applied your patchset to the latest dpdk:master.
> 
> Z:\>build\native\clang\examples\dpdk-helloworld.exe
> EAL: Detected 4 lcore(s)
> EAL: Detected 1 NUMA nodes
> 
>   OK, no lcores specified.
> 
> Z:\>build\native\clang\examples\dpdk-helloworld.exe -cf
> EAL: Detected 4 lcore(s)
> EAL: Detected 1 NUMA nodes
> hello from core 1
> hello from core 2
> hello from core 3
> hello from core 0
> 
>   OK, 4 lcores enabled.
> 
> Z:\>build\native\clang\examples\dpdk-helloworld.exe -cf --log-level=eal:8
> 
>   Crash, WinDbg log attached to bug report.
> 
> Z:\>echo %errorlevel%
> -1073741819
> 
> Z:\>
> 
> Attaching the build log, but I don't think it matters, it's something with
> getopt implementation or it's use, judging by stack trace.
> 
> P.S. I can also see "--syslog" option in the help message, you should
> probably hide it via #ifndef, as you did with said option handling.

Re: [dpdk-dev] [PATCH v2 1/2] crypto/ccp: sha3 support enabling in ccp

On Wed, Feb 5, 2020 at 11:22 PM Thomas Monjalon  wrote:
> 05/02/2020 13:24, Akhil Goyal:
> > > For series,
> > > Acked-by: Ravi Kumar 
> > >
> > > >
> > > >From: Sardar Shamsher Singh 
> > > >
> > > >sha3 support enabled in AMD-CCP crypto controller
> > > >
> > > >Signed-off-by: Sardar Shamsher Singh 
> > > >---
> > Change patch title and description as below
> > cryptodev: fix missing SHA3 algo strings
> >
> > SHA3 support was added earlier but algo strings were
> > missing. This patch add the missing strings.
> >
> > Fixes: 1df800f89518 ("crypto/ccp: support SHA3 family")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Sardar Shamsher Singh 
> > Acked-by: Ravi Kumar 
> > Acked-by: Akhil Goyal 
> >
> > Applied to dpdk-next-crypto
>
> Sorry I must drop this patch because it triggers an ABI warning:
>   [C]'const char* rte_crypto_auth_algorithm_strings[]' was changed at 
> rte_crypto_sym.h:320:1:
> size of symbol changed from 168 to 232

This is still not clear to me, but here is how I understand the issue.


An exposed array (and its size) ends up in both the shared library and
the final binary data section.

[dmarchan@wsfd-netdev66 dpdk]$ readelf -sW
~/builds/build-gcc-shared/app/dpdk-test-crypto-perf |grep
rte_crypto_auth_algorithm_strings
86: 004141a0   168 OBJECT  GLOBAL DEFAULT   24
rte_crypto_auth_algorithm_strings@DPDK_20.0 (4)
   308: 004141a0   168 OBJECT  GLOBAL DEFAULT   24
rte_crypto_auth_algorithm_strings@@DPDK_20.0
[dmarchan@wsfd-netdev66 dpdk]$ readelf -sW
~/builds/build-gcc-shared/lib/librte_cryptodev.so |grep
rte_crypto_auth_algorithm_strings
57: b220   168 OBJECT  GLOBAL DEFAULT   23
rte_crypto_auth_algorithm_strings@@DPDK_20.0
   158: b220   168 OBJECT  GLOBAL DEFAULT   23
rte_crypto_auth_algorithm_strings

At runtime, the linker chooses to rewire all access to the final
binary data section, not the shared library local representation.

Now, if we update the array size, the shared library code is built
with the assumption of the increased size.
But at runtime with an "old" binary, the shared library code runs with
a shorter array, with potential out of bound access.

Interesting article:
https://developers.redhat.com/blog/2019/05/06/how-c-array-sizes-become-part-of-the-binary-interface-of-a-library/


-- 
David Marchand

Re: [dpdk-dev] [PATCH] mbuf: fix pinned memory free routine style issue

On Wed, Jan 22, 2020 at 08:50:35AM +, Viacheslav Ovsiienko wrote:
> Minor style issue is fixed.
> 
> Fixes: 6c8e50c2e549 ("mbuf: create pool with external memory buffers")
> 
> Signed-off-by: Viacheslav Ovsiienko 

Acked-by: Olivier Matz

Re: [dpdk-dev] [PATCH v3] mbuf: display more fields in dump

On Wed, Jan 22, 2020 at 09:39:56AM -0800, Stephen Hemminger wrote:
> The rte_pktmbuf_dump should display offset, refcount, and vlan
> info since these are often useful during debugging.
> 
> Signed-off-by: Stephen Hemminger 
> Acked-by: Andrew Rybchenko 

Acked-by: Olivier Matz

[dpdk-dev] [PATCH v2] app/test: add test for mbuf with pinned external buffer

2020-02-06 Thread Viacheslav Ovsiienko

This patch adds unit test for the mbufs allocated from
the special pool with pinned external data buffers.

The pinned buffer mbufs are tested in the same way as
regular ones with taking into account some specifics
of cloning/attaching.

Signed-off-by: Viacheslav Ovsiienko 
---
v2: uncomment the failed sanity check test 

 app/test/test_mbuf.c | 163 ++-
 1 file changed, 149 insertions(+), 14 deletions(-)

diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index 61ecffc..8200b4f 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -310,8 +310,17 @@
return -1;
 }
 
+static uint16_t
+testclone_refcnt_read(struct rte_mbuf *m)
+{
+   return RTE_MBUF_HAS_PINNED_EXTBUF(m) ?
+  rte_mbuf_ext_refcnt_read(m->shinfo) :
+  rte_mbuf_refcnt_read(m);
+}
+
 static int
-testclone_testupdate_testdetach(struct rte_mempool *pktmbuf_pool)
+testclone_testupdate_testdetach(struct rte_mempool *pktmbuf_pool,
+   struct rte_mempool *clone_pool)
 {
struct rte_mbuf *m = NULL;
struct rte_mbuf *clone = NULL;
@@ -331,7 +340,7 @@
*data = MAGIC_DATA;
 
/* clone the allocated mbuf */
-   clone = rte_pktmbuf_clone(m, pktmbuf_pool);
+   clone = rte_pktmbuf_clone(m, clone_pool);
if (clone == NULL)
GOTO_FAIL("cannot clone data\n");
 
@@ -339,7 +348,7 @@
if (*data != MAGIC_DATA)
GOTO_FAIL("invalid data in clone\n");
 
-   if (rte_mbuf_refcnt_read(m) != 2)
+   if (testclone_refcnt_read(m) != 2)
GOTO_FAIL("invalid refcnt in m\n");
 
/* free the clone */
@@ -358,7 +367,7 @@
data = rte_pktmbuf_mtod(m->next, unaligned_uint32_t *);
*data = MAGIC_DATA;
 
-   clone = rte_pktmbuf_clone(m, pktmbuf_pool);
+   clone = rte_pktmbuf_clone(m, clone_pool);
if (clone == NULL)
GOTO_FAIL("cannot clone data\n");
 
@@ -370,15 +379,15 @@
if (*data != MAGIC_DATA)
GOTO_FAIL("invalid data in clone->next\n");
 
-   if (rte_mbuf_refcnt_read(m) != 2)
+   if (testclone_refcnt_read(m) != 2)
GOTO_FAIL("invalid refcnt in m\n");
 
-   if (rte_mbuf_refcnt_read(m->next) != 2)
+   if (testclone_refcnt_read(m->next) != 2)
GOTO_FAIL("invalid refcnt in m->next\n");
 
/* try to clone the clone */
 
-   clone2 = rte_pktmbuf_clone(clone, pktmbuf_pool);
+   clone2 = rte_pktmbuf_clone(clone, clone_pool);
if (clone2 == NULL)
GOTO_FAIL("cannot clone the clone\n");
 
@@ -390,10 +399,10 @@
if (*data != MAGIC_DATA)
GOTO_FAIL("invalid data in clone2->next\n");
 
-   if (rte_mbuf_refcnt_read(m) != 3)
+   if (testclone_refcnt_read(m) != 3)
GOTO_FAIL("invalid refcnt in m\n");
 
-   if (rte_mbuf_refcnt_read(m->next) != 3)
+   if (testclone_refcnt_read(m->next) != 3)
GOTO_FAIL("invalid refcnt in m->next\n");
 
/* free mbuf */
@@ -418,7 +427,8 @@
 }
 
 static int
-test_pktmbuf_copy(struct rte_mempool *pktmbuf_pool)
+test_pktmbuf_copy(struct rte_mempool *pktmbuf_pool,
+ struct rte_mempool *clone_pool)
 {
struct rte_mbuf *m = NULL;
struct rte_mbuf *copy = NULL;
@@ -458,11 +468,14 @@
copy = NULL;
 
/* same test with a cloned mbuf */
-   clone = rte_pktmbuf_clone(m, pktmbuf_pool);
+   clone = rte_pktmbuf_clone(m, clone_pool);
if (clone == NULL)
GOTO_FAIL("cannot clone data\n");
 
-   if (!RTE_MBUF_CLONED(clone))
+   if ((!RTE_MBUF_HAS_PINNED_EXTBUF(m) &&
+!RTE_MBUF_CLONED(clone)) ||
+   (RTE_MBUF_HAS_PINNED_EXTBUF(m) &&
+!RTE_MBUF_HAS_EXTBUF(clone)))
GOTO_FAIL("clone did not give a cloned mbuf\n");
 
copy = rte_pktmbuf_copy(clone, pktmbuf_pool, 0, UINT32_MAX);
@@ -1199,6 +1212,7 @@
buf = rte_pktmbuf_alloc(pktmbuf_pool);
if (buf == NULL)
return -1;
+
printf("Checking good mbuf initially\n");
if (verify_mbuf_check_panics(buf) != -1)
return -1;
@@ -2411,6 +2425,120 @@ struct test_case {
return -1;
 }
 
+/*
+ * Test the mbuf pool with pinned external data buffers
+ *  - Allocate memory zone for external buffer
+ *  - Create the mbuf pool with pinned external buffer
+ *  - Check the created pool with relevant mbuf pool unit tests
+ */
+static int
+test_pktmbuf_ext_pinned_buffer(struct rte_mempool *std_pool)
+{
+
+   struct rte_pktmbuf_extmem ext_mem;
+   struct rte_mempool *pinned_pool = NULL;
+   const struct rte_memzone *mz = NULL;
+
+   printf("Test mbuf pool with external pinned data buffers\n");
+
+   /* Allocate memzone for the external data buffer */
+   mz = rte_memzone_reserve("pinned_pool",
+NB_MBUF * MBUF_DATA_SIZE,
+SOCK

Re: [dpdk-dev] [PATCH] app/test: add test for mbuf with pinned external buffer

2020-02-06 Thread Slava Ovsiienko

I checked with 2M huge pages - fork() works OK on my host, does not work over 
1G.
RH7.2/3.10.327. Will try other kernels.

With best regards, Slava

> -Original Message-
> From: Slava Ovsiienko
> Sent: Thursday, February 6, 2020 10:25
> To: Olivier Matz 
> Cc: dev@dpdk.org; Matan Azrad ; Raslan Darawsheh
> ; Ori Kam ; Shahaf Shuler
> ; step...@networkplumber.org;
> tho...@mellanox.net
> Subject: RE: [PATCH] app/test: add test for mbuf with pinned external buffer
> 
> Olivier, thanks for the reviewing.
> I'll remove the comment and send the v2.
> I use 1G huge pages, will retest over 2M and continue finding why my host
> fails.
> 
> With best regards, Slava
> 
> > -Original Message-
> > From: Olivier Matz 
> > Sent: Thursday, February 6, 2020 10:17
> > To: Slava Ovsiienko 
> > Cc: dev@dpdk.org; Matan Azrad ; Raslan
> Darawsheh
> > ; Ori Kam ; Shahaf Shuler
> > ; step...@networkplumber.org;
> > tho...@mellanox.net
> > Subject: Re: [PATCH] app/test: add test for mbuf with pinned external
> > buffer
> >
> > Hi,
> >
> > On Fri, Jan 24, 2020 at 08:25:18PM +, Viacheslav Ovsiienko wrote:
> > > This patch adds unit test for the mbufs allocated from the special
> > > pool with pinned external data buffers.
> > >
> > > The pinned buffer mbufs are tested in the same way as regular ones
> > > with taking into account some specifics of cloning/attaching.
> > >
> > > Signed-off-by: Viacheslav Ovsiienko 
> >
> > Looks good to me, you can add my ack in the v2, once we understand the
> > issue with verify_mbuf_check_panics().
> >
> > > @@ -1199,10 +1212,11 @@
> > >   buf = rte_pktmbuf_alloc(pktmbuf_pool);
> > >   if (buf == NULL)
> > >   return -1;
> > > + /*
> > >   printf("Checking good mbuf initially\n");
> > >   if (verify_mbuf_check_panics(buf) != -1)
> > >   return -1;
> > > -
> > > + */
> > >   printf("Now checking for error conditions\n");
> > >
> > >   if (verify_mbuf_check_panics(NULL)) { @@ -2411,6 +2425,120 @@
> > struct
> > > test_case {
> >
> > Note: on my platform, it still works if I remove this comment.
> >
> > Regards,
> > Olivier

[dpdk-dev] [PATCH v2] test/compress: replace test vector

2020-02-06 Thread Artur Trybula

This patch replaces an existing test vector with a new one containing
public domain text only. This is to avoid any potential issues
re-licensing content as BSD-3 which has no clear original license.

Fixes: b06aa643cac4 ("test/compress: add initial unit tests")

Signed-off-by: Artur Trybula 
---
v2 changes:
- changed test vector (the 3rd entry), text from
  Alice's Adventures in Wonderland instead of C-code (v1)
- added BSD SPDX header

 app/test/test_compressdev_test_buffer.h | 194 
 1 file changed, 98 insertions(+), 96 deletions(-)

diff --git a/app/test/test_compressdev_test_buffer.h 
b/app/test/test_compressdev_test_buffer.h
index c0492f89a..d24160244 100644
--- a/app/test/test_compressdev_test_buffer.h
+++ b/app/test/test_compressdev_test_buffer.h
@@ -1,3 +1,7 @@
+/* SPDX-License-Identifier: (BSD-3-Clause)
+ * Copyright(c) 2018-2020 Intel Corporation
+ */
+
 #ifndef TEST_COMPRESSDEV_TEST_BUFFERS_H_
 #define TEST_COMPRESSDEV_TEST_BUFFERS_H_
 
@@ -190,106 +194,104 @@ static const char test_buf_shakespeare[] =
"\n"
"ORLANDOGo apart, Adam, and thou shalt hear how he will\n";
 
-/* Snippet of source code in Pascal */
-static const char test_buf_pascal[] =
-   "   Ptr= 1..DMem;\n"
-   "   Loc= 1..IMem;\n"
-   "   Loc0   = 0..IMem;\n"
-   "   EdgeT  = (hout,lin,hin,lout); {Warning this order is important 
in}\n"
-   " {predicates such as gtS,geS}\n"
-   "   CardT  = (finite,infinite);\n"
-   "   ExpT   = Minexp..Maxexp;\n"
-   "   ManT   = Mininf..Maxinf; \n"
-   "   Pflag  = (PNull,PSoln,PTrace,PPrint);\n"
-   "   Sreal  = record\n"
-   "   edge:EdgeT;\n"
-   "   cardinality:CardT;\n"
-   "   exp:ExpT; {exponent}\n"
-   "   mantissa:ManT;\n"
-   "end;\n"
-   "   Int= record\n"
-   "   hi:Sreal;\n"
-   "   lo:Sreal;\n"
-   "end;\n"
-   "   Instr  = record\n"
-   "   Code:OpType;\n"
-   "   Pars: array[0..Par] of 0..DMem;\n"
-   "end;\n"
-   "   DataMem= record\n"
-   "   D:array [Ptr] of Int;\n"
-   "   S:array [Loc] of State;\n"
-   "   LastHalve:Loc;\n"
-   "   RHalve   :array [Loc] of real;\n"
-   "end;\n"
-   "   DataFlags=record\n"
-   "   PF   :array [Ptr] of Pflag;\n"
-   "end;\n"
-   "var\n"
-   "   Debug  : (none,activity,post,trace,dump);\n"
-   "   Cut: (once,all);\n"
-   "   GlobalEnd,Verifiable:boolean;\n"
-   "   HalveThreshold:real;\n"
-   "   I  : array [Loc] of Instr; {Memory holding instructions}\n"
-   "   End: Loc; {last instruction in I}\n"
-   "   ParN   : array [OpType] of -1..Par; {number of parameters for 
each \n"
-   "   opcode. -1 means no result}\n"
-   "ParIntersect : array [OpType] of boolean ;\n"
-   "   DInit  : DataMem; {initial memory which is cleared and \n"
-   "   used in first call}\n"
-   "   DF : DataFlags; {hold flags for variables, e.g. 
print/trace}\n"
-   "   MaxDMem:0..DMem;\n"
-   "   Shift  : array[0..Digits] of 1..maxint;{array of constant 
multipliers}\n"
-   "   {used for alignment 
etc.}\n"
-   "   Dummy  :Positive;\n"
-   "   {constant intervals and Sreals}\n"
-   "   PlusInfS,MinusInfS,PlusSmallS,MinusSmallS,ZeroS,\n"
-   "   PlusFiniteS,MinusFiniteS:Sreal;\n"
-   "   Zero,All,AllFinite:Int;\n"
-   "\n"
-   "procedure deblank;\n"
-   "var Ch:char;\n"
-   "begin\n"
-   "   while (not eof) and (input^ in [' ','   ']) do read(Ch);\n"
-   "end;\n"
-   "\n"
-   "procedure InitialOptions;\n"
-   "\n"
-   "#include '/user/profs/cleary/bin/options.i';\n"
-   "\n"
-   "   procedure Option;\n"
-   "   begin\n"
-   "  case Opt of\n"
-   "  'a','A':Debug:=activity;\n"
-   "  'd','D':Debug:=dump;\n"
-   "  'h','H':HalveThreshold:=StringNum/100;\n"
-   "  'n','N':Debug:=none;\n"
-   "  'p','P':Debug:=post;\n"
-   "  't','T':Debug:=trace;\n"
-   "  'v','V':Verifiable:=true;\n"
-   "  end;\n"
-   "   end;\n"
-   "\n"
-   "begin\n"
-   "   Debug:=trace;\n"
-   "   Verifiable:=false;\n"
-   "   HalveThreshold:=67/100;\n"
-   "   Options;\n"
-   "   writeln(Debug);\n"
-   "   writeln('Verifiable:',Verifiable);\n"
-   "   writeln('Halve threshold',HalveThreshold

Re: [dpdk-dev] [RFC] Accelerator API to chain packet processing functions

2020-02-06 Thread Coyle, David

Hi Jerin,
Thanks for the comments. Please see replies below.

Kind Regards,
David

> On Tue, Feb 4, 2020 at 8:15 PM David Coyle  wrote:
> >
> > Introduction
> > 
> >
> > This RFC introduces a new DPDK library, rte_accelerator.
> >
> > The main aim of this library is to provide a flexible and extensible way of
> combining one or more packet-processing functions into a single operation,
> thereby allowing these to be performed in parallel in optimized software
> libraries or in a hardware accelerator. These functions can include
> cryptography, compression and CRC/checksum calculation, while others can
> potentially be added in the future. Performing these functions in parallel as 
> a
> single operation can enable a significant performance improvement.
> >
> >
> > Background
> > ==
> >
> > There are a number of byte-wise operations which are present and
> common across many access network data-plane pipelines, such as Cipher,
> Authentication, CRC, Bit-Interleaved-Parity (BIP), other checksums etc. Some
> prototyping has been done at Intel in relation to the 01.org access-network-
> dataplanes project to prove that a significant performance improvement is
> possible when such byte-wise operations are combined into a single pass of
> packet data processing. This performance boost has been prototyped for
> both XGS-PON MAC data-plane and DOCSIS MAC data-plane pipelines.
> 
> 
> Could you share the relative performance numbers to show the gain?

[DC] As mentioned above, the main performance gains are when the packet 
processing operations can be combined into a single pass of the packet.
Both Crypto-CRC-BIP (for XGS-PON MAC) and Crypto-CRC (for DOCSIS MAC) have been 
implemented in the AESNI MB library as single pass operation chains.

We have modified the dpdk-crypto-perf-tester as part of our prototyping to test 
the cases where:
1) each packet processing function is done as an independent stage (e.g. 
calling rte_net_crc for CRC,  AESNI MB through rte_cryptodev for cipher, and a 
C function to calculate the BIP)
2) all packet processing functions done as a single-pass operation in AESNI MB 
through rte_cryptodev

We see the following results for 1024 byte input frames from 
dpdk-crypto-perf-tester:
- XGS-PON MAC (Crypto-CRC-BIP):
- 3 independent stages: 1429 cycles/buf (13.75Gbps)
- 1 single-pass stage: 896 cycles/buf (21.9Gbps)
37% cycle reduction

- DOCSIS MAC (Crypto-CRC):
- 2 independent stages: 1421 cycles/buf (13.84Gbps)
- 1 single-pass stage: 1133 cycles/buf (17.34Gbps)
20% cycle reduction

Adding the accelerator API will allow vendors gain the benefits of these cycle 
savings

> 
> >
> > The prototypes used some protocol-specific modifications to the DPDK
> cryptodev library. In order to make this performance improvement
> consumable by network access equipment vendors, a more extensible and
> correct solution is required that can be upstreamed into DPDK.
> >
> > Hence, the introduction of rte_accelerator.
> >
> >
> > Use Cases
> > =
> >
> > The primary use cases for this new library have already been mentioned.
> These are:
> >
> > - DOCSIS MAC: Crypto-CRC
> > - Order:
> > - Downstream: CRC, Encrypt
> > - Upstream: Decrypt, CRC
> > - Specifications:
> > - Crypto: 128-bit AES-CFB encryption variant for DOCSIS as
> described in section 11.1 of DOCSIS 3.1 Security Specification
> (https://apps.cablelabs.com/specification/CM-SP-SECv3.1)
> > - CRC: Ethernet 32-bit CRC as defined in
> > Ethernet/[ISO/IEC 8802-3]
> >
> > - XGS-PON MAC: Crypto-CRC-BIP
> > - Order:
> > - Downstream: CRC, Encrypt, BIP
> 
> I understand if the chain has two operations then it may possible to have
> handcrafted SW code to do both operations in one pass.
> I understand the spec is agnostic on a number of passes it does require to
> enable the xfrom but To understand the SW/HW capability, In the above
> case, "CRC, Encrypt, BIP", It is done in one pass in SW or three passes in SW
> or one pass using HW?

[DC] The CRC, Encrypt, BIP is also currently done as 1 pass in AESNI MB library 
SW.
However, this could also be performed as a single pass in a HW accelerator

> 
> 
> 
> > - Upstream: BIP, Decrypt, CRC
> > - Specifications:
> > - Crypto: AES-128 [NIST FIPS-197] cipher, used in counter 
> > mode
> (AES-CTR), as described in [NIST SP800-38A].
> > - CRC: Ethernet 32-bit CRC as defined in Ethernet/[ISO/IEC 
> > 8802-3]
> > - BIP: 4-byte bit-interleaved even parity (BIP) field
> > computed over the entire FS frame, refer to  ITU-T G.989.3, sections 8.1.1.5
> and 8.1.2.3 (https://www.itu.int/rec/dologin_pub.asp?lang=e&id=T-REC-
> G.989.3-201510-I!!PDF-E)
> >
--
In

Re: [dpdk-dev] BUG: eBPF missing BPF_ABS

2020-02-06 Thread Ananyev, Konstantin

> I agree fixing offsets in cbpf to ebpf converter and passing mbuf is easier. 
> There is still the pathological case of multi segment mbuf. But
> Linux XDP doesn't handle it either.

Why do you think multi-seg would be a problem?
If we’ll use rte_pktmbuf_read() for interpreter and something similar for jit,
multi-seg case should be covered, no?

>
> Let me put early version of filter2rteebf on GitHub
>
> On Thu, Feb 6, 2020, 8:54 AM Morten Brørup 
>  wrote:
> > From: dev [mailto:mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev,
> > Konstantin
> > Sent: Wednesday, February 5, 2020 10:16 PM
> >
> > >
> > > As I mentioned in my FOSDEM talk the current DPDK eBPF handling is
> > > not usable for packet filters. I have ported the classic BPF to eBPF
> > code
> > > and the generated code is not usable by DPDK.
> > >
> > > The problem is that DPDK eBPF does not implement all the opcodes.
> > > BPF_ABS is not implemented and must be. It is in the Linux kernel.
> >
> > Yep, it doesn't.
> > This is not a generic eBPF instruction, but sort of implicit function
> > call
> > to access skb data. At initial stage of librte_bpf development,
> > I didn't think much about cBPF conversion, and to simplify things
> > decided to put all special skb features aside.
> > But sure, if that will enable DPDK cBPF support, let's add it in.
> > Please fill a Bugzilla ticket to track that issue, and I'll try to
> > find some time within 20.05 to look at it.
> > Unless of course, you or someone else would like to volunteer for it.
> >
> > Though at first step, we probably need to decide what should be
> > our requirements for it in terms of DPDK.
> > From https://www.kernel.org/doc/Documentation/networking/filter.txt:
> > "eBPF has two non-generic instructions: (BPF_ABS |  | BPF_LD) and
> > (BPF_IND |  | BPF_LD) which are used to access packet data.
> > They had to be carried over from classic to have strong performance of
> > socket filters running in eBPF interpreter. These instructions can only
> > be used when interpreter context is a pointer to 'struct sk_buff' and
> > have seven implicit operands. Register R6 is an implicit input that
> > must
> > contain pointer to sk_buff. Register R0 is an implicit output which
> > contains
> > the data fetched from the packet. Registers R1-R5 are scratch registers
> > and must not be used to store the data across BPF_ABS | BPF_LD or
> > BPF_IND | BPF_LD instructions.
> > These instructions have implicit program exit condition as well. When
> > eBPF program is trying to access the data beyond the packet boundary,
> > the interpreter will abort the execution of the program. JIT compilers
> > therefore must preserve this property. src_reg and imm32 fields are
> > explicit inputs to these instructions.
> > For example:
> > BPF_IND | BPF_W | BPF_LD means:
> > R0 = ntohl(*(u32 *) (((struct sk_buff *) R6)->data + src_reg + imm32))
> > and R1 - R5 were scratched."
> >
> > For RTE_BPF_ARG_PTR_MBUF context we probably want behavior similar
> > to linux, i.e. BPF_IND | BPF_W | BPF_LD would mean something like:
> >
> > 1) uint32_t tmp;
> > R0 = &tmp;
> > R0 = rte_pktmbuf_read((const struct rte_mbuf *)R6,  src_reg +
> > imm32,
> >   sizeof(uint32_t), R0);
> >  if (R0 == NULL) return FAILED;
> >  R0 = ntohl(*(uint32_t *)R0);
> >
> > But what to do with when ctx is raw data buffer (RTE_BPF_ARG_PTR)?
> > Should it be just:
> > 2) R0 = ntohl(*(uint32_t *)(R6 + src_reg + imm32));
> > 3) not allow LD_ABS/LD_IND in this mode at all.
> >
>
> I think that 1) is the correct choice for the "cBPF filter" use case.
>
> In that context I consider both 2) and 3) irrelevant because the 
> RTE_BPF_ARG_PTR_MBUF type should be used for cBPF filtering. I have
> argued for this before.
>
> Others have argued for using the RTE_BPF_ARG_PTR type instead. Let's consider 
> using the RTE_BPF_ARG_PTR for a moment... Is there an
> implicit understanding that the data points to packet data? Then a range 
> check in 2) might be relevant.
>
> However, if the RTE_BPF_ARG_PTR_MBUF type is supported and used for 
> cBPF->eBPF conversion, we would not need to support
> LD_ABS/LD_IND for the RTE_BPF_ARG_PTR type. So between 2) and 3), I support 
> 3).
>
> > Second question is implementation.
> > I can see two main options here:
> > a) if we plan to have our own cBPF->eBPF converter and support only it,
> > we can add these extra instructions generation into converter itself.
> > I.E. cBPF->eBPF conversion for LD_ABS/LD_IND will generate series
> > of generic eBPF instructions.
> > b) support eBPF LD_ABS/LD_IND in eBPF interpreter/jit
> >
> > (a) probably a simpler way (eBPF interpreter/jit/verifier would remain
> > unchanged),
> > but seems way too limited. So I think (b) is a better choice, even more
> > work implied
> > (interpreter seems more or less straightforward, jit would probably
> > need some effort).
> >
>
> This is going to be used in the fast path, probably on all packets

[dpdk-dev] DPDK Release Status Meeting 6/02/2020

2020-02-06 Thread Ferruh Yigit

Minutes 6 February 2020
---

Agenda:
* Release Dates
* Subtrees
* OvS

Participants:
* Intel
* Marvell
* Mellanox
* NXP
* Red Hat


Release Dates
-

* v20.02 dates:
  * -rc2 pushed to  *Thursday 6 February 2020*
  * -rc3 pushed to  *Friday 14 February 2020*
  * Release pushed to   *Friday 21 February 2020*
* Release pushed because of extended PRC holidays

* v20.05 dates:
  * Proposal/V1:Friday 6 March 2020
  * Integration/Merge/RC1:  Friday 10 April 2020
  * Release:Friday 1 May 2020


Subtrees


* main
  * Pulled next-net & next-net-crypto
* Dropped two patches from crypto because of ABI breakages
  * ccp sha3 & chacha20-poly
  * ABI check patches merged
* In travis checks enabled by default

* next-net
  * pulled, nothing in backlog for -rc3, there can be fixes

* next-net-crypto
  * ipsecgw eventmode can be postponed, Akhil will check
  * CPU crypto patches merged

* next-net-eventdev
  * There can be a few fixes for -rc3

* next-net-virtio
  * Three small fixes for -rc3

* next-net-mlx
  * pulled

* LTS

  * 17.11.10-rc1 released, please test and report results
* https://mails.dpdk.org/archives/dev/2020-January/154915.html
* Got test results from Intel, Red Hat & Mellanox
  * QAT regression reported by Intel testing, need help from developers
  * mlx reported regressions, needs more details
* Will wait two more week for above issues clarified
  * Rough release date is mid February

  * 18.11.6 released
* https://mails.dpdk.org/archives/dev/2020-January/156760.html


OvS
---

* Release on track
* Backporting removing experimental tag under discussion in DPDK
  * https://mails.dpdk.org/archives/dev/2020-January/155419.html



DPDK Release Status Meetings


The DPDK Release Status Meeting is intended for DPDK Committers to discuss
the status of the master tree and sub-trees, and for project managers to
track progress or milestone dates.

The meeting occurs on Thursdays at 8:30 UTC. If you wish to attend just
send an email to "John McNamara " for the invite.

Re: [dpdk-dev] [RFC] Accelerator API to chain packet processing functions

On Thu, Feb 6, 2020 at 3:35 PM Coyle, David  wrote:
>
> Hi Jerin,

Hi David,

> Thanks for the comments. Please see replies below.
>
> Kind Regards,
> David
>
> > On Tue, Feb 4, 2020 at 8:15 PM David Coyle  wrote:
> > >
> > > Introduction
> > > 
> > >
> > > This RFC introduces a new DPDK library, rte_accelerator.
> > >
> > > The main aim of this library is to provide a flexible and extensible way 
> > > of
> > combining one or more packet-processing functions into a single operation,
> > thereby allowing these to be performed in parallel in optimized software
> > libraries or in a hardware accelerator. These functions can include
> > cryptography, compression and CRC/checksum calculation, while others can
> > potentially be added in the future. Performing these functions in parallel 
> > as a
> > single operation can enable a significant performance improvement.
> > >
> > >
> > > Background
> > > ==
> > >
> > > There are a number of byte-wise operations which are present and
> > common across many access network data-plane pipelines, such as Cipher,
> > Authentication, CRC, Bit-Interleaved-Parity (BIP), other checksums etc. Some
> > prototyping has been done at Intel in relation to the 01.org access-network-
> > dataplanes project to prove that a significant performance improvement is
> > possible when such byte-wise operations are combined into a single pass of
> > packet data processing. This performance boost has been prototyped for
> > both XGS-PON MAC data-plane and DOCSIS MAC data-plane pipelines.
> >
> >
> > Could you share the relative performance numbers to show the gain?
>
> [DC] As mentioned above, the main performance gains are when the packet 
> processing operations can be combined into a single pass of the packet.
> Both Crypto-CRC-BIP (for XGS-PON MAC) and Crypto-CRC (for DOCSIS MAC) have 
> been implemented in the AESNI MB library as single pass operation chains.
>
> We have modified the dpdk-crypto-perf-tester as part of our prototyping to 
> test the cases where:
> 1) each packet processing function is done as an independent stage (e.g. 
> calling rte_net_crc for CRC,  AESNI MB through rte_cryptodev for cipher, and 
> a C function to calculate the BIP)
> 2) all packet processing functions done as a single-pass operation in AESNI 
> MB through rte_cryptodev
>
> We see the following results for 1024 byte input frames from 
> dpdk-crypto-perf-tester:
> - XGS-PON MAC (Crypto-CRC-BIP):
> - 3 independent stages: 1429 cycles/buf (13.75Gbps)
> - 1 single-pass stage: 896 cycles/buf (21.9Gbps)
> 37% cycle reduction
>
> - DOCSIS MAC (Crypto-CRC):
> - 2 independent stages: 1421 cycles/buf (13.84Gbps)
> - 1 single-pass stage: 1133 cycles/buf (17.34Gbps)
> 20% cycle reduction
>
> Adding the accelerator API will allow vendors gain the benefits of these 
> cycle savings

Numbers make sense. I have seen a similar performance improvement
doing in one pass with CPU instructions.


> > > - XGS-PON MAC: Crypto-CRC-BIP
> > > - Order:
> > > - Downstream: CRC, Encrypt, BIP
> >
> > I understand if the chain has two operations then it may possible to have
> > handcrafted SW code to do both operations in one pass.
> > I understand the spec is agnostic on a number of passes it does require to
> > enable the xfrom but To understand the SW/HW capability, In the above
> > case, "CRC, Encrypt, BIP", It is done in one pass in SW or three passes in 
> > SW
> > or one pass using HW?
>
> [DC] The CRC, Encrypt, BIP is also currently done as 1 pass in AESNI MB 
> library SW.
> However, this could also be performed as a single pass in a HW accelerator

As a specification, cascading the xform chains make sense.
Do we have any HW that does support chaining the xforms more than
"two" in one pass?
i.e real chaining function where two blocks of HWs work hand in hand
for chaining.
If none, it may be better to abstract as synonymous API(No dequeue, no
enqueue) for the CPU use case.

[dpdk-dev] [PATCH] netvsc: update link info when getting device info

2020-02-06 Thread Mohammed Gamal

testpmd 'show summary' command always shows interface status as down
with 0 Mbps speed regardless of the underlying VF's status.
This happens as hn_dev_link_update() is never called, even on the initial
RNDIS_STATUS_MEDIA_CONNECT message as LSC interrupts are not yet enabled
at this point.

Let's call it and update link info when calling hn_dev_info_get().

Signed-off-by: Mohammed Gamal 
---
 drivers/net/netvsc/hn_ethdev.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index c79f92437..1120fc688 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -265,6 +265,11 @@ static int hn_dev_info_get(struct rte_eth_dev *dev,
if (rc != 0)
return rc;
 
+   /* fill in link status and link speed */
+   rc = hn_dev_link_update(dev, 0);
+   if (rc != 0)
+   return rc;
+
/* merges the offload and queues of vf */
return hn_vf_info_get(hv, dev_info);
 }
-- 
2.21.0

Re: [dpdk-dev] [PATCH 4/6] build: MinGW-w64 support for Meson

2020-02-06 Thread Bruce Richardson

On Wed, Feb 05, 2020 at 11:41:05PM +0300, Dmitry Kozlyuk wrote:
> > > > > > +if is_windows
> > > > > > +   # Require platform SDK for Windows 7 and above.
> > > > > > +   add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')   
> > > > > >  
> > > > > 
> > > > > Please explain. Why Windows 7 is needed? What this define is doing?  
> > > > 
> > > > Yes, Windows 7 and above is need for already existing code in 
> > > > eal_lcore.c,
> > > > specifically for GetLogicalProcessorInformation() call.
> > > > 
> > > > When including , one must define minimum API version the
> > > > application is compiled against [0]. MSVC and Clang default to the 
> > > > version of
> > > > platform SDK (that is, maximum supported). MinGW defaults to Windows 
> > > > XP, so
> > > > this definition must be either in  before #include 
> > > >  or
> > > > here. Because other files may include , I'd prefer to have a
> > > > global definition via compiler command-line.
> > > > 
> > > > [0]:
> > > > https://docs.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers
> > > >   
> > > 
> > > OK, thanks.
> > > Please reword the comment with something like
> > >   "Minimum supported API is Windows 7."
> > >   
> > For this, as an alternative to putting it as a project argument, you can 
> > just
> > add it to dpdk_conf which means it will end up as a define in the global
> > rte_build_config.h and so be directly included in each compilation unit
> > ahead of any other headers. (rte_config.h includes rte_build_config.h)
> 
> Can you please explain why using dpdk_conf is a better alternative? In
> lib/meson.build I can see add_project_arguments('_D_GNU_SOURCE', ...), which
> serves a similar purpose on POSIX systems. Compiler option also makes it
> impossible to forget or redefine this constant in code by mistake.
> 
I'm not necessarily saying it's better, it's just an alternative to
consider. :-) Having it in rte_config.h makes the define available to any
external apps using DPDK, which may or may not be desirable.

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: increase number of qps to lcore_params

2020-02-06 Thread Anoob Joseph

Hi Akhil,

> > > @Akhil, do you have any comments?
> > >
> > > Also, I think we should make it
> > > 
> > >
> > Looks good to me, but I believe this would need more changes and
> > testing in event patches.
> > Also it does not have any changes for lookaside cases.
> > Can we move this to next release and add lookaside case as well in a
> > single go.
> 
> [Anoob] Not a problem. I'll defer this to next release then.

This patch is not part of the event additions to ipsec-segcw. This patch is 
required to handle few corner cases, but is equally applicable to lookaside 
crypto as well. I had agreed to only deferring this patch.

Lukasz is preparing v4 of event mode patches with doc update and addressing one 
minor comment from Konstantin. If the event ipsec-secgw patches are not getting 
merged for this release, can you confirm it will be merged immediately after 
this release? I'm assuming you have finished the reviews as the patch was 
submitted early in this release cycle.

Thanks,
Anoob

> -Original Message-
> From: Anoob Joseph
> Sent: Monday, February 3, 2020 2:52 PM
> To: Akhil Goyal ; Ananyev, Konstantin
> ; Nicolau, Radu 
> Cc: Jerin Jacob Kollanukkaran ; Lukas Bartosik
> ; Narayana Prasad Raju Athreya
> ; dev@dpdk.org
> Subject: RE: [PATCH] examples/ipsec-secgw: increase number of qps to
> lcore_params
> 
> Hi Akhil,
> 
> Please see inline.
> 
> Thanks,
> Anoob
> 
> > -Original Message-
> > From: Akhil Goyal 
> > Sent: Monday, February 3, 2020 2:46 PM
> > To: Anoob Joseph ; Ananyev, Konstantin
> > ; Nicolau, Radu
> 
> > Cc: Jerin Jacob Kollanukkaran ; Lukas Bartosik
> > ; Narayana Prasad Raju Athreya
> > ; dev@dpdk.org
> > Subject: [EXT] RE: [PATCH] examples/ipsec-secgw: increase number of
> > qps to lcore_params
> >
> > External Email
> >
> > --
> > > > > > > > > Currently only one qp will be used for one core. The
> > > > > > > > > number of qps can be increased to match the number of
> > > > > > > > > lcore
> > params.
> > > > > > > >
> > > > > > > > I don't really understand the purpose of that patch
> > > > > > > > As I understand, all it does - unconditionally increases
> > > > > > > > number of crypto-queues mapped to the same lcore.
> > > > > > >
> > > > > > > [Anoob] With the current code, we will have only crypto qp
> > > > > > > mapped to one lcore. So even if you have large number of
> > > > > > > crypto qps, you would be only
> > > > > > using as much as the number of lcores.
> > > > > > >
> > > > > > > This is an inefficient model as a fat flow(say with large
> > > > > > > packet
> > > > > > > sizes) on one eth queue hitting one core can starve another
> > > > > > > flow which
> > > > > > happens to hit the same core, because both flows would get
> > > > > > queued to the same qp.
> > > > > > > And, we cannot just randomly submit to multiple qps from the
> > > > > > > same core as then the ordering would be messed up.
> > > > > >
> > > > > > No-one suggesting that one I believe.
> > > > > >
> > > > > > So the best possible usage model would be to map one eth queue
> > > > > > to one
> > > > > > > crypto qp. That way, the core wouldn't be unnecessarily
> > > > > > > pipeline the crypto
> > > > > > processing.
> > > > > >
> > > > > > I doubt it is really the 'best possible usage model'.
> > > > > > There could be cases when forcing lcore to use/manage more
> > > > > > crypto-queues will lead to negative effects: perf drop, not
> > > > > > enough crypto queues for all eth queues, etc.
> > > > > > If your HW/SW requires to match each eth queue with a separate
> > > > > > crypto-queue, then I think it should be an optional feature,
> > > > > > while keeping default behavior intact.
> > > > >
> > > > > [Anoob] I think the question here is more to do with s/w crypto
> > > > > PMDs vs h/w crypto PMDs. For s/w PMDs, having more queues
> > > > > doesn't really
> > > > make sense and for h/w PMDs it's better.
> > > >
> > > > Not always.
> > > > If these queues belongs to the same device, sometimes it is faster
> > > > to use just one queue for device per core.
> > > > HW descriptor status polling, etc. is not free.
> > > >
> > > > > I'll see how we can make this an optional feature. Would you be
> > > > > okay with allowing such behavior if the underlying PMD can
> > > > > support as many
> > > > queues as lcore_params?
> > > > >
> > > > > As in, if the PMD doesn't support enough queues, we do 1 qp per
> core.
> > > > Would that work for you?
> > > >
> > > > I am not fond of idea to change default mapping method silently.
> > > > My preference would be a new command-line option (--cdev-maping
> or
> > so).
> > > > Another thought, make it more fine-grained and user-controlled by
> > > > extending eth-port-queue-lcore mapping option.
> > > > from current: ,, to
> > > > ,,,.
> > > >
> > > > So let say with 2 cores , 2 eth ports/2 queues per port for
> > > > current mapping user would do:
> > > > # use cdev

Re: [dpdk-dev] [RFC PATCH 1/7] vfio: Include optional device match in vfio_device_ops callbacks

2020-02-06 Thread Cornelia Huck

On Tue, 04 Feb 2020 16:05:43 -0700
Alex Williamson  wrote:

> Allow bus drivers to provide their own callback to match a device to
> the user provided string.
> 
> Signed-off-by: Alex Williamson 
> ---
>  drivers/vfio/vfio.c  |   19 +++
>  include/linux/vfio.h |3 +++
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 388597930b64..dda1726adda8 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -875,11 +875,22 @@ EXPORT_SYMBOL_GPL(vfio_device_get_from_dev);
>  static struct vfio_device *vfio_device_get_from_name(struct vfio_group 
> *group,
>char *buf)
>  {
> - struct vfio_device *it, *device = NULL;
> + struct vfio_device *it, *device = ERR_PTR(-ENODEV);
>  
>   mutex_lock(&group->device_lock);
>   list_for_each_entry(it, &group->device_list, group_next) {
> - if (!strcmp(dev_name(it->dev), buf)) {
> + int ret;
> +
> + if (it->ops->match) {
> + ret = it->ops->match(it->device_data, buf);
> + if (ret < 0 && ret != -ENODEV) {
> + device = ERR_PTR(ret);
> + break;
> + }
> + } else
> + ret = strcmp(dev_name(it->dev), buf);

The asymmetric braces look a bit odd.

> +
> + if (!ret) {
>   device = it;
>   vfio_device_get(device);
>   break;
> @@ -1441,8 +1452,8 @@ static int vfio_group_get_device_fd(struct vfio_group 
> *group, char *buf)
>   return -EPERM;
>  
>   device = vfio_device_get_from_name(group, buf);
> - if (!device)
> - return -ENODEV;
> + if (IS_ERR(device))
> + return PTR_ERR(device);
>  
>   ret = device->ops->open(device->device_data);
>   if (ret) {
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index e42a711a2800..755e0f0e2900 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -26,6 +26,8 @@
>   * operations documented below
>   * @mmap: Perform mmap(2) on a region of the device file descriptor
>   * @request: Request for the bus driver to release the device
> + * @match: Optional device name match callback (return: 0 for match, -ENODEV
> + * (or >0) for no match and continue, other -errno: no match and 
> stop)

I'm wondering about these conventions.

If you basically want a tri-state return (match, don't match/continue,
don't match/stop), putting -ENODEV and >0 together feels odd. I would
rather expect either
- < 0 == don't match/stop, 0 == don't match/continue, > 0 == match, or
- 0 == match, -ENODEV (or some other defined error) == don't
  match/continue, all other values == don't match/abort?

>   */
>  struct vfio_device_ops {
>   char*name;
> @@ -39,6 +41,7 @@ struct vfio_device_ops {
>unsigned long arg);
>   int (*mmap)(void *device_data, struct vm_area_struct *vma);
>   void(*request)(void *device_data, unsigned int count);
> + int (*match)(void *device_data, char *buf);
>  };
>  
>  extern struct iommu_group *vfio_iommu_group_get(struct device *dev);
>

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: increase number of qps to lcore_params

2020-02-06 Thread Akhil Goyal




> 
> Hi Akhil,
> 
> > > > @Akhil, do you have any comments?
> > > >
> > > > Also, I think we should make it
> > > > 
> > > >
> > > Looks good to me, but I believe this would need more changes and
> > > testing in event patches.
> > > Also it does not have any changes for lookaside cases.
> > > Can we move this to next release and add lookaside case as well in a
> > > single go.
> >
> > [Anoob] Not a problem. I'll defer this to next release then.
> 
> This patch is not part of the event additions to ipsec-segcw. This patch is
> required to handle few corner cases, but is equally applicable to lookaside
> crypto as well. I had agreed to only deferring this patch.
> 
> Lukasz is preparing v4 of event mode patches with doc update and addressing
> one minor comment from Konstantin. If the event ipsec-secgw patches are not
> getting merged for this release, can you confirm it will be merged immediately
> after this release? I'm assuming you have finished the reviews as the patch 
> was
> submitted early in this release cycle.
> 
I would say Lukasz should wait for sending the next version till I complete the 
review.
I will notify as soon as possible.
I am not done with the review of the event patches yet. I need more time for 
that.
Yes, I believe these will be merged early in the next cycle.

-Akhil

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: increase number of qps to lcore_params

06/02/2020 11:46, Anoob Joseph:
> Hi Akhil,
> 
> > > > @Akhil, do you have any comments?
> > > >
> > > > Also, I think we should make it
> > > > 
> > > >
> > > Looks good to me, but I believe this would need more changes and
> > > testing in event patches.
> > > Also it does not have any changes for lookaside cases.
> > > Can we move this to next release and add lookaside case as well in a
> > > single go.
> > 
> > [Anoob] Not a problem. I'll defer this to next release then.
> 
> This patch is not part of the event additions to ipsec-segcw. This patch is 
> required to handle few corner cases, but is equally applicable to lookaside 
> crypto as well. I had agreed to only deferring this patch.
> 
> Lukasz is preparing v4 of event mode patches with doc update and addressing 
> one minor comment from Konstantin. If the event ipsec-secgw patches are not 
> getting merged for this release, can you confirm it will be merged 
> immediately after this release? I'm assuming you have finished the reviews as 
> the patch was submitted early in this release cycle.

Akhil, as all other tree maintainers, is doing its best.
Akhil is doing a great job, and had a lot of work in this release cycle.
Please be patient.

Re: [dpdk-dev] [PATCH v2 5/7] build: MinGW-w64 support for Meson

2020-02-06 Thread Bruce Richardson

On Thu, Feb 06, 2020 at 09:44:24AM +0300, Dmitry Kozlyuk wrote:
> MinGW-w64 linker does not mimic MS linker options, so the build system
> must differentiate between linkers on Windows. Use GNU linker options
> with GCC and MS linker options with Clang.
> 
> MinGW-w64 by default uses MSVCRT stdio, which does not comply to ANSI,
> most notably its formatting and string handling functions. MinGW-w64
> support for the Universal CRT (UCRT) is ongoing, but the toolchain
> provides its own standard-complying implementation of stdio. The latter
> is used in the patch to support formatting in DPDK.
> 
> Signed-off-by: Dmitry Kozlyuk 
> ---
>  config/meson.build | 14 ++
>  lib/librte_eal/meson.build |  3 +++
>  lib/meson.build|  8 ++--
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/config/meson.build b/config/meson.build
> index 28a57f56f..9b955f9ef 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -14,6 +14,10 @@ foreach env:supported_exec_envs
>   set_variable('is_' + env, exec_env == env)
>  endforeach
>  
> +# MS linker requires special treatment.
> +# FIXME: use cc.get_linker_id() with Meson >= 0.54
> +is_ms_linker = is_windows and (cc.get_id() == 'clang')
> +
>  # set the major version, which might be used by drivers and libraries
>  # depending on the configuration options
>  pver = meson.project_version().split('.')
> @@ -241,6 +245,16 @@ if is_freebsd
>   add_project_arguments('-D__BSD_VISIBLE', language: 'c')
>  endif
>  
> +if is_windows
> + # Minimum supported API is Windows 7.
> + add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
> +
> + # Use MinGW-w64 stdio, because DPDK assumes ANSI-compliant formatting.
> + if cc.get_id() == 'gcc'
> + add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
> + endif
> +endif
> +
>  if get_option('b_lto')
>   if cc.has_argument('-ffat-lto-objects')
>   add_project_arguments('-ffat-lto-objects', language: 'c')
> diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
> index 4be5118ce..eae8c2ba8 100644
> --- a/lib/librte_eal/meson.build
> +++ b/lib/librte_eal/meson.build
> @@ -20,6 +20,9 @@ endif
>  if cc.has_function('getentropy', prefix : '#include ')
>   cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
>  endif
> +if cc.get_id() == 'gcc'
> + cflags += '-D__USE_MINGW_ANSI_STDIO'
> +endif

Is this snippet still needed, given you add this as a project arg above?

[dpdk-dev] [PATCH] [v2 1/1] examples/kni: add SIGTERM signal handling

2020-02-06 Thread vattunuru

From: Vamsi Attunuru 

SIGTERM handling is added for graceful application exit.
Useful when application is terminated without specifying
any signal on 'kill' command.

Signed-off-by: Vamsi Attunuru 
Acked-by: Ferruh Yigit 
---
v2 Change:
* Changed commit log

 examples/kni/main.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/examples/kni/main.c b/examples/kni/main.c
index 5f713e6..29fc37e 100644
--- a/examples/kni/main.c
+++ b/examples/kni/main.c
@@ -176,9 +176,13 @@ signal_handler(int signum)
return;
}
 
-   /* When we receive a RTMIN or SIGINT signal, stop kni processing */
-   if (signum == SIGRTMIN || signum == SIGINT){
-   printf("\nSIGRTMIN/SIGINT received. KNI processing 
stopping.\n");
+   /*
+* When we receive a RTMIN or SIGINT or SIGTERM signal,
+* stop kni processing
+*/
+   if (signum == SIGRTMIN || signum == SIGINT || signum == SIGTERM) {
+   printf("\nSIGRTMIN/SIGINT/SIGTERM received. "
+   "KNI processing stopping.\n");
rte_atomic32_inc(&kni_stop);
return;
 }
@@ -1006,6 +1010,7 @@ main(int argc, char** argv)
signal(SIGUSR2, signal_handler);
signal(SIGRTMIN, signal_handler);
signal(SIGINT, signal_handler);
+   signal(SIGTERM, signal_handler);
 
/* Initialise EAL */
ret = rte_eal_init(argc, argv);
-- 
2.8.4

[dpdk-dev] [dpdk-dev PATCH v1 1/1] net/octeontx2: update link information for loopback port

2020-02-06 Thread Ashish Gupta

loopback devices are exposed as ethdev device in octeontx2.
dpdk link update APIs updating the link information for cgx
ports but skipping loopback ports.

When stack uses loopback port for forwarding, packets are
dropped as link status is down. Link information need to be
updated for loopback ports to avoid it.

Fixes: 38f566280a ("net/octeontx2: add link stats operations")
Cc: sta...@dpdk.org

Signed-off-by: Ashish Gupta 
---
 drivers/net/octeontx2/otx2_link.c | 49 +--
 1 file changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/net/octeontx2/otx2_link.c 
b/drivers/net/octeontx2/otx2_link.c
index f5679b06e..4128f56d9 100644
--- a/drivers/net/octeontx2/otx2_link.c
+++ b/drivers/net/octeontx2/otx2_link.c
@@ -82,32 +82,57 @@ otx2_eth_dev_link_status_update(struct otx2_dev *dev,
_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
+static int
+lbk_link_update(struct rte_eth_link *link)
+{
+   link->link_status = ETH_LINK_UP;
+   link->link_speed = ETH_SPEED_NUM_100G;
+   link->link_autoneg = ETH_LINK_FIXED;
+   link->link_duplex = ETH_LINK_FULL_DUPLEX;
+   return 0;
+}
+
+static int
+cgx_link_update(struct otx2_eth_dev *dev, struct rte_eth_link *link)
+{
+   struct otx2_mbox *mbox = dev->mbox;
+   struct cgx_link_info_msg *rsp;
+   int rc;
+   otx2_mbox_alloc_msg_cgx_get_linkinfo(mbox);
+   rc = otx2_mbox_process_msg(mbox, (void *)&rsp);
+   if (rc)
+   return rc;
+
+   link->link_status = rsp->link_info.link_up;
+   link->link_speed = rsp->link_info.speed;
+   link->link_autoneg = ETH_LINK_AUTONEG;
+
+   if (rsp->link_info.full_duplex)
+   link->link_duplex = rsp->link_info.full_duplex;
+   return 0;
+}
+
 int
 otx2_nix_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete)
 {
struct otx2_eth_dev *dev = otx2_eth_pmd_priv(eth_dev);
-   struct otx2_mbox *mbox = dev->mbox;
-   struct cgx_link_info_msg *rsp;
struct rte_eth_link link;
int rc;
 
RTE_SET_USED(wait_to_complete);
+   memset(&link, 0, sizeof(struct rte_eth_link));
 
-   if (otx2_dev_is_lbk(dev) || otx2_dev_is_sdp(dev))
+   if (otx2_dev_is_sdp(dev))
return 0;
 
-   otx2_mbox_alloc_msg_cgx_get_linkinfo(mbox);
-   rc = otx2_mbox_process_msg(mbox, (void *)&rsp);
+   if (otx2_dev_is_lbk(dev))
+   rc = lbk_link_update(&link);
+   else
+   rc = cgx_link_update(dev, &link);
+
if (rc)
return rc;
 
-   link.link_status = rsp->link_info.link_up;
-   link.link_speed = rsp->link_info.speed;
-   link.link_autoneg = ETH_LINK_AUTONEG;
-
-   if (rsp->link_info.full_duplex)
-   link.link_duplex = rsp->link_info.full_duplex;
-
return rte_eth_linkstatus_set(eth_dev, &link);
 }
 
-- 
2.14.3

[dpdk-dev] [PATCH v1] common/iavf: update the ABI version

2020-02-06 Thread Haiyue Wang

The new symbols should be in ABI version 20.0.1.

Fixes: 89214fe915b8 ("net/iavf/base: move to drivers common directory")

Signed-off-by: Haiyue Wang 
---
 drivers/common/iavf/rte_common_iavf_version.map | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/common/iavf/rte_common_iavf_version.map 
b/drivers/common/iavf/rte_common_iavf_version.map
index 1a0839262..2f11d67c0 100644
--- a/drivers/common/iavf/rte_common_iavf_version.map
+++ b/drivers/common/iavf/rte_common_iavf_version.map
@@ -1,4 +1,4 @@
-DPDK_20.0 {
+DPDK_20.0.1 {
global:
 
iavf_init_adminq;
-- 
2.17.1

[dpdk-dev] [PATCH] cryptodev: fix missing doxygen comment

2020-02-06 Thread Marcin Smoczynski

Add missing doxygen comment of rte_crypto_mbuf_to_vec's fields.

Signed-off-by: Marcin Smoczynski 
---
 lib/librte_cryptodev/rte_crypto_sym.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_cryptodev/rte_crypto_sym.h 
b/lib/librte_cryptodev/rte_crypto_sym.h
index deb46971f..9e887c110 100644
--- a/lib/librte_cryptodev/rte_crypto_sym.h
+++ b/lib/librte_cryptodev/rte_crypto_sym.h
@@ -861,7 +861,9 @@ __rte_crypto_sym_op_attach_sym_session(struct 
rte_crypto_sym_op *sym_op,
  * @param len
  *   Length of data to represent.
  * @param vec
+ *   Pointer to an output array of IO vectors.
  * @param num
+ *   Size of an output array.
  * @return
  *   - number of successfully filled entries in *vec* array.
  *   - negative number of elements in *vec* array required.
-- 
2.17.1

Re: [dpdk-dev] [PATCH] ring: fix namespace prefix of inline functions

On Thu, Jan 23, 2020 at 9:31 AM Thomas Monjalon  wrote:
>
> When adding custom element size feature, some internal inline functions
> were added in a public header without rte_ prefix.
> It is fixed by adding __rte_ring_.
>
> Fixes: cc4b218790f6 ("ring: support configurable element size")
>
> Reported-by: David Marchand 
> Signed-off-by: Thomas Monjalon 

Acked-by: Honnappa Nagarahalli 
Acked-by: Olivier Matz 

Applied, thanks.


-- 
David Marchand

Re: [dpdk-dev] [PATCH] cryptodev: fix missing doxygen comment

2020-02-06 Thread Ananyev, Konstantin




> -Original Message-
> From: Smoczynski, MarcinX 
> Sent: Thursday, February 6, 2020 12:36 PM
> To: akhil.go...@nxp.com; Ananyev, Konstantin ; 
> tho...@monjalon.net
> Cc: dev@dpdk.org; Smoczynski, MarcinX 
> Subject: [PATCH] cryptodev: fix missing doxygen comment
> 
> Add missing doxygen comment of rte_crypto_mbuf_to_vec's fields.
> 
> Signed-off-by: Marcin Smoczynski 
> ---
>  lib/librte_cryptodev/rte_crypto_sym.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto_sym.h 
> b/lib/librte_cryptodev/rte_crypto_sym.h
> index deb46971f..9e887c110 100644
> --- a/lib/librte_cryptodev/rte_crypto_sym.h
> +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> @@ -861,7 +861,9 @@ __rte_crypto_sym_op_attach_sym_session(struct 
> rte_crypto_sym_op *sym_op,
>   * @param len
>   *   Length of data to represent.
>   * @param vec
> + *   Pointer to an output array of IO vectors.
>   * @param num
> + *   Size of an output array.
>   * @return
>   *   - number of successfully filled entries in *vec* array.
>   *   - negative number of elements in *vec* array required.
> --

Acked-by: Konstantin Ananyev 

> 2.17.1

[dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY


Hello,

When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 
(on an Intel machine):


git clone git://dpdk.org/dpdk
cd dpdk
make config T=x86_64-native-linux-gcc
cd build
vi .config
  => disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
make
...
== Build drivers/net/octeontx2
  CC otx2_rx.o
In file included from .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
 from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
 from .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
 from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
.../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error: 
rte_security.h: No such file or directory

 #include 
  ^~~~
compilation terminated.

This seems cause by f44e7163775537 ('net/octeontx2: add security session 
operations').


Thanks

Thierry

--
Thierry Herbelot
6WIND
Software Engineer

Re: [dpdk-dev] [RFC PATCH 6/7] vfio/pci: Remove dev_fmt definition

2020-02-06 Thread Cornelia Huck

On Tue, 04 Feb 2020 16:06:24 -0700
Alex Williamson  wrote:

> It currently results in messages like:
> 
>  "vfio-pci :03:00.0: vfio_pci: ..."
> 
> Which is quite a bit redundant.
> 
> Signed-off-by: Alex Williamson 
> ---
>  drivers/vfio/pci/vfio_pci.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 026308aa18b5..343fe38ed06b 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -9,7 +9,6 @@
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -#define dev_fmt pr_fmt
>  
>  #include 
>  #include 
> 

Yes, that looks a bit superfluous.

Reviewed-by: Cornelia Huck

Re: [dpdk-dev] [dpdk-users] Should ''shmget" not be used to consume hugepages in DPDK?

2020-02-06 Thread Burakov, Anatoly

On 22-Dec-19 3:33 PM, Byonggon Chun wrote:

x-posting to dev mailing list.

Hi all.
I'm Kubernetes contributors and I'm working to make container isolation of
hugepages that allows us to set hugepages limit per container cgroup.
(At this point, limits are set on pod level cgroup even though we asked
hugepages as the container level resource)

I tore down testPMD and some parts of DPDK lib and I got a question after i
found there is no usage of "shmget" in DPDK.

My question is that Should "shmget" not be used to consume hugepages in
DPDK?
And here is following questions:
1) If we don't have to use "shmget", Why? Does it affect performance?
2) If I use "shmget" to get hugepages, should I call "mlock" syscall for it?

For more details, as I know there are three ways to consume hugepages in
kubernetes.
1) shmget with SHM_HUGETLB
2) mmap with hugetlbs filebacking
3) mmap with MAP_ANONYMOUS | MAP_HUGETLB

And I found that testPMD calls mlock syscall when it maps an anonymous
hugepages or external allocated
hugepages.https://github.com/DPDK/dpdk/blob/924e55fb340623f03fdf2ff7fbcfd78819d1db25/app/test-pmd/testpmd.c#L896https://github.com/DPDK/dpdk/blob/924e55fb340623f03fdf2ff7fbcfd78819d1db25/app/test-pmd/testpmd.c#L916

The reason we're not using shmget is not because it's "legacy" or
something else. It's because it doesn't give the guarantees that we want
to have in DPDK. Namely, that the same shared object in memory is mapped
at the same addresses. Last time i checked, shmget doesn't allow to map
things at a specific address - each process will have its own pointer to
shared memory, and accesses to shared memory by pointer are not valid
across process boundaries. This is contrary to DPDK's goals, because we
want to avoid address translation when working with multiple processes
(in that sense, DPDK's multiprocess is basically like having multiple
threads).

Thanks.

On Fri, Dec 20, 2019 at 9:42 PM Byonggon Chun
wrote:

shmget is a legacy Unix API and there is no point in using it.

Yeah, I agree with it,
I also prefer to use mmap with hugetlbfs in a DPDK container.

The reason why I started this mail thread is some DPDK users still use
shmget to consume hugepages, and I just wanted to find a good rationale to
convince them to use mmap.

But, at this point, I have only one rationale : shmget is a legacy UINIX
API.

On Fri, Dec 20, 2019 at 6:06 AM Stephen Hemminger <
step...@networkplumber.org> wrote:

On Fri, 20 Dec 2019 01:23:50 +0900
Byonggon Chun wrote:

Hi all.
I'm Kubernetes contributors and I'm working to make container isolation

hugepages that allows us to set hugepages limit per container cgroup.
(At this point, limits are set on pod level cgroup even though we asked
hugepages as the container level resource)

I tore down testPMD and some parts of DPDK lib and I got a question

after i

found there is no usage of "shmget" in DPDK.

for it?

For more details, as I know there are three ways to consume hugepages in
kubernetes.
1) shmget with SHM_HUGETLB
2) mmap with hugetlbs filebacking
3) mmap with MAP_ANONYMOUS | MAP_HUGETLB

And I found that testPMD calls mlock syscall when it maps an anonymous
hugepages or external allocated hugepages.

https://github.com/DPDK/dpdk/blob/924e55fb340623f03fdf2ff7fbcfd78819d1db25/app/test-pmd/testpmd.c#L896

https://github.com/DPDK/dpdk/blob/924e55fb340623f03fdf2ff7fbcfd78819d1db25/app/test-pmd/testpmd.c#L916

Thanks.

shmget is a legacy Unix API and there is no point in using it.
For new applications libhugetlbfs is preferable.

--
Thanks,
Anatoly

Re: [dpdk-dev] [EXT] Re: [PATCH] config: update Marvell ARMADA

23/01/2020 11:01, Jerin Jacob:
> On Mon, Dec 2, 2019 at 1:03 PM Jerin Jacob  wrote:
> >
> > On Mon, Dec 2, 2019 at 4:01 PM Liron Himi  wrote:
> > >
> > > Hi Jerin,
> > >
> > > I have created a patch to MUSDK that fix this conflicts as you suggested.
> > > This will be externally available in the next MUSDK release.
> > > Once it will be out, we can simplify the armada-config, but until then if 
> > > anyone want to build DPDK with MUSDK it either needs this patch or 
> > > compile MUSDK as shared lib
> >
> > Good that it is fixed in the library. +1 for this patch as short terms
> > solution till next MUSDK releases shows up.
> 
> With the above note,
> 
> Acked-by: Jerin Jacob 

Applied, thanks

Re: [dpdk-dev] [PATCH v1 1/1] config: add Broadcom Stingray for meson cross-compilation

06/01/2020 03:06, Qingmin Liu:
> Broadcom Stingray is armv8 CPU having cortex-a72. The implementor ID is
> 0x41 (arm) and the primary part number is 0xd08 (cortex-a72).
> 
> Signed-off-by: Qingmin Liu 

Applied, thanks

Re: [dpdk-dev] [PATCH 00/14] cleanup resources on shutdown

On Sat, Jan 4, 2020 at 2:34 AM Stephen Hemminger
 wrote:
>
> Recently started using valgrind with DPDK, and the results
> are not clean.
>
> The DPDK has a function that applications can use to tell it
> to cleanup resources on shutdown (rte_eal_cleanup). But the
> current coverage of that API is spotty. Many internal parts of
> DPDK leave files and allocated memory behind.
>
> This patch set is a start at getting the sub-parts of
> DPDK to cleanup after themselves. These are the easier ones,
> the harder and more critical ones are in the drivers
> and the memory subsystem.

I am too short on time to check and integrate this big series in rc2,
and from now it would be too risky to take in 20.02.
Can you respin it for 20.05 with FreeBSD fixes too?


Thanks.

-- 
David Marchand

Re: [dpdk-dev] [PATCH] bus/pci: set boot-up log prints to absolute minimum

21/01/2020 09:00, jer...@marvell.com:
> From: Jerin Jacob 
> 
> Some machines may have a lot of PCI devices, logs from PCI probe
> creates a lot of clutter on boot-up, typically one needs
> to scroll the screen to find other issues in boot-up.
> 
> This patch changes the loglevel of PCI probes to `debug`
> to reduce the clutter on default boot-up logs

I think the PCI probe informations are... informational.
Maybe you are just not interested in info logs.
If this is the case, I suggest to change the log level at runtime.

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] mem: fix incorrect munmap in error unwind

22/01/2020 18:06, Stephen Hemminger:
> The loop to unwind existing mmaps was only unmapping the
> first segment and the error paths after mmap() were not
> doing munmap of the current segment.
> 
> Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
> Cc: anatoly.bura...@intel.com
> Cc: sta...@dpdk.org
> Signed-off-by: Stephen Hemminger 
> Acked-by: Anatoly Burakov 
> ---
> v2 - incorporate feedback from David Marchand
>  fix missing munmap of current segment

Applied, thanks

Re: [dpdk-dev] [PATCH v3 0/4] make vhost PMD available for secondary processes

2020-02-06 Thread Maxime Coquelin




On 2/6/20 2:39 AM, Itsuro Oda wrote:
> vhost PMD has not been available for secondary processes since
> DPDK v18.11.  (https://bugs.dpdk.org/show_bug.cgi?id=194)
> (for a long term !)
> This series of patches intend to make vhost PMD available for
> secondary processes.
> Because now setting vhost driver to communicate with a vhost-user
> master (ex. Qemu) is accomplished by the probe function of the
> primary process, only the primary process can be a vhost-user
> slave.
> With this patch, setting vhost driver is delayed at eth_dev
> configuration in order to be able to set it from a secondary
> process. Because (in the first place,) setting vhost driver is not
> necessary to be done at probe (it is enough to be done up to eth_dev
> start), this fix is no problem for the primary process.
> There is a precondition that the same process has to operate
> a vhost interface from beginning to end (from eth_dev configuration
> to eth_dev close). (This patch leaves it to user's responsibility.)
> This precondition will not be a problem in most use cases
> (including SPP).
> 
> v2:
> - add signed-off-by
> - fix spelling error
> 
> v3:
> - rebase on dpdk-next-virtio master
> - change patch order
> - fix subject and commit message
> 
> Itsuro Oda (4):
>   net/vhost: allocate interface name from heap
>   net/vhost: delay vhost driver setup
>   net/vhost: make secondary probe complete
>   net/vhost: remove an unused member
> 
>  drivers/net/vhost/rte_eth_vhost.c | 152 +-
>  1 file changed, 88 insertions(+), 64 deletions(-)
> 

Thanks Itsuro, it looks good to me.
I will pull the series for v20.02-rc3.

Maxime

Re: [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg

2020-02-06 Thread Maxime Coquelin

Adding back Ivan as you removed it from the To: list.
So he may not have seen your comment.

On 1/29/20 11:10 AM, Adrian Moreno wrote:
> On 1/20/20 6:05 PM, Ivan Dyukov wrote:
>> Some applications like pktgen use link_speed to calculate
>> transmit rate. It limits outcome traffic to hardcoded 10G.
>>
>> This patch adds link_speed devarg which allows to configure
>> link_speed of virtio device.
>>
>> Signed-off-by: Ivan Dyukov 
>> ---
>>  drivers/net/virtio/virtio_ethdev.c | 101 -
>>  drivers/net/virtio/virtio_pci.h|   1 +
>>  2 files changed, 85 insertions(+), 17 deletions(-)
>>
> 
> Hi Ivan,
> 
> IMHO, this new option deserves being documented in doc/guides/nics/virtio.rst.
> 
> Otherwise it looks good to me.

I agree with Adrian here, the new option need to be documented.

Thanks,
Maxime

> Thank you.
> Adrian
>> diff --git a/drivers/net/virtio/virtio_ethdev.c 
>> b/drivers/net/virtio/virtio_ethdev.c
>> index 22323d9a5..5ef3c11a7 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -45,6 +45,10 @@ static int virtio_dev_promiscuous_enable(struct 
>> rte_eth_dev *dev);
>>  static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev);
>>  static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev);
>>  static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev);
>> +static uint32_t virtio_dev_speed_capa_get(uint32_t link_speed);
>> +static int virtio_dev_devargs_parse(struct rte_devargs *devargs,
>> +int *vdpa,
>> +uint32_t *link_speed);
>>  static int virtio_dev_info_get(struct rte_eth_dev *dev,
>>  struct rte_eth_dev_info *dev_info);
>>  static int virtio_dev_link_update(struct rte_eth_dev *dev,
>> @@ -1861,6 +1865,7 @@ int
>>  eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>>  {
>>  struct virtio_hw *hw = eth_dev->data->dev_private;
>> +uint32_t link_speed = ETH_SPEED_NUM_10G;
>>  int ret;
>>  
>>  if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
>> @@ -1886,7 +1891,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>>  
>>  return 0;
>>  }
>> -
>> +ret = virtio_dev_devargs_parse(eth_dev->device->devargs,
>> + NULL, &link_speed);
>> +if (ret < 0)
>> +return ret;
>> +hw->link_speed = link_speed;
>>  /*
>>   * Pass the information to the rte_eth_dev_close() that it should also
>>   * release the private port resources.
>> @@ -1953,6 +1962,14 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
>>  
>>  return 0;
>>  }
>> +#define VIRTIO_ARG_LINK_SPEED "link_speed"
>> +#define VIRTIO_ARG_VDPA   "vdpa"
>> +
>> +static const char * const valid_args[] = {
>> +VIRTIO_ARG_LINK_SPEED,
>> +VIRTIO_ARG_VDPA,
>> +NULL
>> +};
>>  
>>  static int vdpa_check_handler(__rte_unused const char *key,
>>  const char *value, void *ret_val)
>> @@ -1965,33 +1982,84 @@ static int vdpa_check_handler(__rte_unused const 
>> char *key,
>>  return 0;
>>  }
>>  
>> +
>> +static uint32_t
>> +virtio_dev_speed_capa_get(uint32_t link_speed)
>> +{
>> +switch (link_speed) {
>> +case ETH_SPEED_NUM_10G:
>> +return ETH_LINK_SPEED_10G;
>> +case ETH_SPEED_NUM_20G:
>> +return ETH_LINK_SPEED_20G;
>> +case ETH_SPEED_NUM_25G:
>> +return ETH_LINK_SPEED_25G;
>> +case ETH_SPEED_NUM_40G:
>> +return ETH_LINK_SPEED_40G;
>> +case ETH_SPEED_NUM_50G:
>> +return ETH_LINK_SPEED_50G;
>> +case ETH_SPEED_NUM_56G:
>> +return ETH_LINK_SPEED_56G;
>> +case ETH_SPEED_NUM_100G:
>> +return ETH_LINK_SPEED_100G;
>> +default:
>> +return 0;
>> +}
>> +}
>> +
>> +static int link_speed_handler(const char *key __rte_unused,
>> +const char *value, void *ret_val)
>> +{
>> +uint32_t val;
>> +if (!value || !ret_val)
>> +return -EINVAL;
>> +val = strtoul(value, NULL, 0);
>> +/* validate input */
>> +if (virtio_dev_speed_capa_get(val) == 0)
>> +return -EINVAL;
>> +*(uint32_t *)ret_val = val;
>> +
>> +return 0;
>> +}
>> +
>> +
>>  static int
>> -virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
>> +virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa,
>> +uint32_t *link_speed)
>>  {
>>  struct rte_kvargs *kvlist;
>> -const char *key = "vdpa";
>>  int ret = 0;
>>  
>>  if (devargs == NULL)
>>  return 0;
>>  
>> -kvlist = rte_kvargs_parse(devargs->args, NULL);
>> -if (kvlist == NULL)
>> +kvlist = rte_kvargs_parse(devargs->args, valid_args);
>> +if (kvlist == NULL) {
>> +PMD_INIT_LOG(ERR, "error when parsing param");
>>  return 0;
>> -
>> -if (!rte_kvargs_count(kvlist, key))
>> -goto exit;
>> -
>> -if (vdpa) {
>> +}
>> +if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) {
>>

Re: [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg

2020-02-06 Thread Adrian Moreno

On 2/6/20 3:22 PM, Maxime Coquelin wrote:
> Adding back Ivan as you removed it from the To: list.
> So he may not have seen your comment.
Oops, sorry about that.
Adrian
> 
> On 1/29/20 11:10 AM, Adrian Moreno wrote:
>> On 1/20/20 6:05 PM, Ivan Dyukov wrote:
>>> Some applications like pktgen use link_speed to calculate
>>> transmit rate. It limits outcome traffic to hardcoded 10G.
>>>
>>> This patch adds link_speed devarg which allows to configure
>>> link_speed of virtio device.
>>>
>>> Signed-off-by: Ivan Dyukov 
>>> ---
>>>  drivers/net/virtio/virtio_ethdev.c | 101 -
>>>  drivers/net/virtio/virtio_pci.h|   1 +
>>>  2 files changed, 85 insertions(+), 17 deletions(-)
>>>
>>
>> Hi Ivan,
>>
>> IMHO, this new option deserves being documented in 
>> doc/guides/nics/virtio.rst.
>>
>> Otherwise it looks good to me.
> 
> I agree with Adrian here, the new option need to be documented.
> 
> Thanks,
> Maxime
> 
>> Thank you.
>> Adrian
>>> diff --git a/drivers/net/virtio/virtio_ethdev.c 
>>> b/drivers/net/virtio/virtio_ethdev.c
>>> index 22323d9a5..5ef3c11a7 100644
>>> --- a/drivers/net/virtio/virtio_ethdev.c
>>> +++ b/drivers/net/virtio/virtio_ethdev.c
>>> @@ -45,6 +45,10 @@ static int virtio_dev_promiscuous_enable(struct 
>>> rte_eth_dev *dev);
>>>  static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev);
>>>  static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev);
>>>  static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev);
>>> +static uint32_t virtio_dev_speed_capa_get(uint32_t link_speed);
>>> +static int virtio_dev_devargs_parse(struct rte_devargs *devargs,
>>> +   int *vdpa,
>>> +   uint32_t *link_speed);
>>>  static int virtio_dev_info_get(struct rte_eth_dev *dev,
>>> struct rte_eth_dev_info *dev_info);
>>>  static int virtio_dev_link_update(struct rte_eth_dev *dev,
>>> @@ -1861,6 +1865,7 @@ int
>>>  eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>>>  {
>>> struct virtio_hw *hw = eth_dev->data->dev_private;
>>> +   uint32_t link_speed = ETH_SPEED_NUM_10G;
>>> int ret;
>>>  
>>> if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
>>> @@ -1886,7 +1891,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>>>  
>>> return 0;
>>> }
>>> -
>>> +   ret = virtio_dev_devargs_parse(eth_dev->device->devargs,
>>> +NULL, &link_speed);
>>> +   if (ret < 0)
>>> +   return ret;
>>> +   hw->link_speed = link_speed;
>>> /*
>>>  * Pass the information to the rte_eth_dev_close() that it should also
>>>  * release the private port resources.
>>> @@ -1953,6 +1962,14 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
>>>  
>>> return 0;
>>>  }
>>> +#define VIRTIO_ARG_LINK_SPEED "link_speed"
>>> +#define VIRTIO_ARG_VDPA   "vdpa"
>>> +
>>> +static const char * const valid_args[] = {
>>> +   VIRTIO_ARG_LINK_SPEED,
>>> +   VIRTIO_ARG_VDPA,
>>> +   NULL
>>> +};
>>>  
>>>  static int vdpa_check_handler(__rte_unused const char *key,
>>> const char *value, void *ret_val)
>>> @@ -1965,33 +1982,84 @@ static int vdpa_check_handler(__rte_unused const 
>>> char *key,
>>> return 0;
>>>  }
>>>  
>>> +
>>> +static uint32_t
>>> +virtio_dev_speed_capa_get(uint32_t link_speed)
>>> +{
>>> +   switch (link_speed) {
>>> +   case ETH_SPEED_NUM_10G:
>>> +   return ETH_LINK_SPEED_10G;
>>> +   case ETH_SPEED_NUM_20G:
>>> +   return ETH_LINK_SPEED_20G;
>>> +   case ETH_SPEED_NUM_25G:
>>> +   return ETH_LINK_SPEED_25G;
>>> +   case ETH_SPEED_NUM_40G:
>>> +   return ETH_LINK_SPEED_40G;
>>> +   case ETH_SPEED_NUM_50G:
>>> +   return ETH_LINK_SPEED_50G;
>>> +   case ETH_SPEED_NUM_56G:
>>> +   return ETH_LINK_SPEED_56G;
>>> +   case ETH_SPEED_NUM_100G:
>>> +   return ETH_LINK_SPEED_100G;
>>> +   default:
>>> +   return 0;
>>> +   }
>>> +}
>>> +
>>> +static int link_speed_handler(const char *key __rte_unused,
>>> +   const char *value, void *ret_val)
>>> +{
>>> +   uint32_t val;
>>> +   if (!value || !ret_val)
>>> +   return -EINVAL;
>>> +   val = strtoul(value, NULL, 0);
>>> +   /* validate input */
>>> +   if (virtio_dev_speed_capa_get(val) == 0)
>>> +   return -EINVAL;
>>> +   *(uint32_t *)ret_val = val;
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +
>>>  static int
>>> -virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
>>> +virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa,
>>> +   uint32_t *link_speed)
>>>  {
>>> struct rte_kvargs *kvlist;
>>> -   const char *key = "vdpa";
>>> int ret = 0;
>>>  
>>> if (devargs == NULL)
>>> return 0;
>>>  
>>> -   kvlist = rte_kvargs_parse(devargs->args, NULL);
>>> -   if (kvlist == NULL)
>>> +   kvlist = rte_kvargs_parse(devargs->args, valid_args);
>>> +   if (kvlist == NULL) {
>>> +   PMD_INIT_LOG(ERR, "error when parsing param");
>>> return 0;
>>> -
>>> -   if (!rte_kv

Re: [dpdk-dev] [PATCH] mbuf: fix pinned memory free routine style issue

06/02/2020 10:46, Olivier Matz:
> On Wed, Jan 22, 2020 at 08:50:35AM +, Viacheslav Ovsiienko wrote:
> > Minor style issue is fixed.
> > 
> > Fixes: 6c8e50c2e549 ("mbuf: create pool with external memory buffers")
> > 
> > Signed-off-by: Viacheslav Ovsiienko 
> 
> Acked-by: Olivier Matz 

Applied, thanks

Re: [dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY

2020-02-06 Thread Bruce Richardson

On Thu, Feb 06, 2020 at 02:39:28PM +0100, Thierry Herbelot wrote:
> Hello,
> 
> When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 (on an
> Intel machine):
> 
> git clone git://dpdk.org/dpdk
> cd dpdk
> make config T=x86_64-native-linux-gcc
> cd build
> vi .config
>   => disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
> make
> ...
> == Build drivers/net/octeontx2
>   CC otx2_rx.o
> In file included from .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
>  from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
>  from .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
>  from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
> .../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error:
> rte_security.h: No such file or directory
>  #include 
>   ^~~~
> compilation terminated.
> 
> This seems cause by f44e7163775537 ('net/octeontx2: add security session
> operations').
> 
Disabling parts of the build, particularly libraries, is always likely to
cause other build failures. I'm not sure we should, or even need to,
support the disabling of arbitrary libs in DPDK.

/Bruce

Re: [dpdk-dev] [PATCH v3] mbuf: display more fields in dump

06/02/2020 10:48, Olivier Matz:
> On Wed, Jan 22, 2020 at 09:39:56AM -0800, Stephen Hemminger wrote:
> > The rte_pktmbuf_dump should display offset, refcount, and vlan
> > info since these are often useful during debugging.
> > 
> > Signed-off-by: Stephen Hemminger 
> > Acked-by: Andrew Rybchenko 
> 
> Acked-by: Olivier Matz 

Applied, thanks

Re: [dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY


On 2/6/20 3:27 PM, Bruce Richardson wrote:

On Thu, Feb 06, 2020 at 02:39:28PM +0100, Thierry Herbelot wrote:

Hello,

When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 (on an
Intel machine):

git clone git://dpdk.org/dpdk
cd dpdk
make config T=x86_64-native-linux-gcc
cd build
vi .config
   => disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
make
...
== Build drivers/net/octeontx2
   CC otx2_rx.o
In file included from .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
  from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
  from .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
  from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
.../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error:
rte_security.h: No such file or directory
  #include 
   ^~~~
compilation terminated.

This seems cause by f44e7163775537 ('net/octeontx2: add security session
operations').


Disabling parts of the build, particularly libraries, is always likely to
cause other build failures. I'm not sure we should, or even need to,
support the disabling of arbitrary libs in DPDK.


Hello,

On the other hand, there is no reason delivering unused code in a DPDK 
application: an application should be free to select its needed 'modules'.


Thanks

Thierry



/Bruce

Re: [dpdk-dev] [PATCH] bus/pci: set boot-up log prints to absolute minimum

On Thu, Feb 6, 2020 at 7:44 PM Thomas Monjalon  wrote:
>
> 21/01/2020 09:00, jer...@marvell.com:
> > From: Jerin Jacob 
> >
> > Some machines may have a lot of PCI devices, logs from PCI probe
> > creates a lot of clutter on boot-up, typically one needs
> > to scroll the screen to find other issues in boot-up.
> >
> > This patch changes the loglevel of PCI probes to `debug`
> > to reduce the clutter on default boot-up logs
>
> I think the PCI probe informations are... informational.
> Maybe you are just not interested in info logs.
> If this is the case, I suggest to change the log level at runtime.

I am wondering, what would be the right balance, Following is DPDK
startup output from octeontx2[1]
It creates a lot of clutter in the "default" boot up. Why not enable
below prints using log level at runtime?
I believe it comes as a debug category, i.e information required to
debug if something is not working,
dpdk bind script already lists what is bound to DPDK.

Suggestion to remove clutter?

[1]
EAL: Detected 24 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
EAL: No available hugepages reported in hugepages-2048kB
EAL: Probing VFIO support...
EAL: VFIO support initialized
EAL: PCI device 0002:01:00.1 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:00.2 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:00.3 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:00.4 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:00.5 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:00.6 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:00.7 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:01.0 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:01.1 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:01.2 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:01.3 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:01.4 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:01.5 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:01.6 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:01.7 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:01:02.0 on NUMA socket 0
EAL:   probe driver: 177d:a0f8 net_octeontx2
EAL: PCI device 0002:02:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a063 net_octeontx2
EAL:   using IOMMU type 1 (Type 1)
EAL: PCI device 0002:03:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a063 net_octeontx2
EAL: PCI device 0002:04:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a063 net_octeontx2
EAL: PCI device 0002:05:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a063 net_octeontx2
EAL: PCI device 0002:06:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a063 net_octeontx2
EAL: PCI device 0002:07:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a063 net_octeontx2
EAL: PCI device 0002:08:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a0f9 event_octeontx2
EAL: PCI device 0002:09:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a0f9 event_octeontx2
EAL: PCI device 0002:0a:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a0f9 event_octeontx2
EAL: PCI device 0002:0b:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a0f9 event_octeontx2
EAL: PCI device 0002:0c:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a0fb mempool_octeontx2
EAL: PCI device 0002:0d:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a0fb mempool_octeontx2
EAL: PCI device 0002:0e:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a0f9 event_octeontx2
EAL: PCI device 0002:0f:00.0 on NUMA socket 0
EAL:   probe driver: 177d:a0fb mempool_octeontx2
APP: HPET is not enabled, using TSC as default timer

>
>
>

Re: [dpdk-dev] [PATCH v2] app/test: add test for mbuf with pinned external buffer

06/02/2020 10:49, Viacheslav Ovsiienko:
> This patch adds unit test for the mbufs allocated from
> the special pool with pinned external data buffers.
> 
> The pinned buffer mbufs are tested in the same way as
> regular ones with taking into account some specifics
> of cloning/attaching.
> 
> Signed-off-by: Viacheslav Ovsiienko 
> ---
> v2: uncomment the failed sanity check test 

Acked-by: Olivier Matz 

Applied, thanks

Re: [dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY

On Thu, Feb 6, 2020 at 8:06 PM Thierry Herbelot
 wrote:
>
> On 2/6/20 3:27 PM, Bruce Richardson wrote:
> > On Thu, Feb 06, 2020 at 02:39:28PM +0100, Thierry Herbelot wrote:
> >> Hello,
> >>
> >> When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 (on 
> >> an
> >> Intel machine):
> >>
> >> git clone git://dpdk.org/dpdk
> >> cd dpdk
> >> make config T=x86_64-native-linux-gcc
> >> cd build
> >> vi .config
> >>=> disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
> >> make
> >> ...
> >> == Build drivers/net/octeontx2
> >>CC otx2_rx.o
> >> In file included from .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
> >>   from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
> >>   from .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
> >>   from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
> >> .../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error:
> >> rte_security.h: No such file or directory
> >>   #include 
> >>^~~~
> >> compilation terminated.
> >>
> >> This seems cause by f44e7163775537 ('net/octeontx2: add security session
> >> operations').
> >>
> > Disabling parts of the build, particularly libraries, is always likely to
> > cause other build failures. I'm not sure we should, or even need to,
> > support the disabling of arbitrary libs in DPDK.
>
> Hello,
>
> On the other hand, there is no reason delivering unused code in a DPDK
> application: an application should be free to select its needed 'modules'.

Just to understand the use case, What would be the downside of
compiling unwanted code?
In meson, it takes only jiffies to compile code and If we use,
-no-whole-archive then the generated binary will  not  be bloated,
Considering the case where "make" build system will be deprecated soon
and,  for meson, I don't think, we are
planning to take the route of disabling the "core libraries".

Could you share the real-world use for this?
My only concern is we can not make tons of #define in the driver code.
So, eventually, we end up
disabling the driver.



>
> Thanks
>
> Thierry
>
> >
> > /Bruce
> >
>

Re: [dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY

2020-02-06 Thread Akhil Goyal

Hi Bruce/Thierry,

> On Thu, Feb 06, 2020 at 02:39:28PM +0100, Thierry Herbelot wrote:
> > Hello,
> >
> > When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 (on
> an
> > Intel machine):
> >
> > git clone git://dpdk.org/dpdk
> > cd dpdk
> > make config T=x86_64-native-linux-gcc
> > cd build
> > vi .config
> >   => disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
> > make
> > ...
> > == Build drivers/net/octeontx2
> >   CC otx2_rx.o
> > In file included from .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
> >  from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
> >  from .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
> >  from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
> > .../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error:
> > rte_security.h: No such file or directory
> >  #include 
> >   ^~~~
> > compilation terminated.
> >
> > This seems cause by f44e7163775537 ('net/octeontx2: add security session
> > operations').
> >
> Disabling parts of the build, particularly libraries, is always likely to
> cause other build failures. I'm not sure we should, or even need to,
> support the disabling of arbitrary libs in DPDK.
> 

This was followed in the past when rte_security was experimental
And we made sure that compilation should work even after disabling the
Lib. Now since the lib has been removed with the experimental tag, shall we
Follow the same in future as well. It depends on the community and the users
Of the code. If there is a valid use case, we can continue test compilation 
without
Rte_security.

-Akhil

Re: [dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY


On 2/6/20 3:48 PM, Jerin Jacob wrote:

On Thu, Feb 6, 2020 at 8:06 PM Thierry Herbelot
 wrote:


On 2/6/20 3:27 PM, Bruce Richardson wrote:

On Thu, Feb 06, 2020 at 02:39:28PM +0100, Thierry Herbelot wrote:

Hello,

When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 (on an
Intel machine):

git clone git://dpdk.org/dpdk
cd dpdk
make config T=x86_64-native-linux-gcc
cd build
vi .config
=> disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
make
...
== Build drivers/net/octeontx2
CC otx2_rx.o
In file included from .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
   from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
   from .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
   from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
.../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error:
rte_security.h: No such file or directory
   #include 
^~~~
compilation terminated.

This seems cause by f44e7163775537 ('net/octeontx2: add security session
operations').


Disabling parts of the build, particularly libraries, is always likely to
cause other build failures. I'm not sure we should, or even need to,
support the disabling of arbitrary libs in DPDK.


Hello,

On the other hand, there is no reason delivering unused code in a DPDK
application: an application should be free to select its needed 'modules'.


Just to understand the use case, What would be the downside of
compiling unwanted code?
In meson, it takes only jiffies to compile code and If we use,
-no-whole-archive then the generated binary will  not  be bloated,
Considering the case where "make" build system will be deprecated soon
and,  for meson, I don't think, we are
planning to take the route of disabling the "core libraries".

Could you share the real-world use for this?
My only concern is we can not make tons of #define in the driver code.
So, eventually, we end up
disabling the driver.


Hello Jerin,

Our use case is that IPsec is provided as part of 6WIND stack, not using 
the version from DPDK (we are using the crypto PMDs from DPDK).


In any case, as the compilation of DPDK is (still) driven by a separate 
configuration file, it should be possible that some combination of 
options are disabled, and still DPDK builds fine.


Thierry







 Thanks

 Thierry



/Bruce






--
Thierry Herbelot
6WIND
Senior Software Engineer

Tel: +33 1 39 30 92 61
Fax: +33 1 39 30 92 11
thierry.herbe...@6wind.com
www.6wind.com
Immeuble Central Gare - Bât C 1, place Charles de Gaulle 78180 
Montigny-le-Bretonneux, France


Ce courriel ainsi que toutes les pièces jointes, est uniquement destiné 
à son ou ses destinataires. Il contient des informations confidentielles 
qui sont la propriété de 6WIND. Toute révélation, distribution ou copie 
des informations qu'il contient est strictement interdite. Si vous avez 
reçu ce message par erreur, veuillez immédiatement le signaler à 
l'émetteur et détruire toutes les données reçues


This e-mail message, including any attachments, is for the sole use of 
the intended recipient(s) and contains information that is confidential 
and proprietary to 6WIND. All unauthorized review, use, disclosure or 
distribution is prohibited. If you are not the intended recipient, 
please contact the sender by reply e-mail and destroy all copies of the 
original message.

[dpdk-dev] [PATCH v6] app/testpmd: add portlist option to the testpmd

2020-02-06 Thread Hariprasad Govindharajan

In current version, we are setting the ports
using portmask. With portmask, we can use only
upto 64 ports. This portlist option enables the user
to use more than 64 ports.
Now we can specify the ports in 2 different ways
- Using portmask (-p [0x]nnn): mask must be in hex format
- Using portlist in the following format
  --portlist [-p2][,p3[-p4],...]

--portmask 0x2 is same as --portlist 1
--portmask 0x3 is same as --portlist 0-1

Signed-off-by: Hariprasad Govindharajan 
---
v6:
optimized the code to check for duplicates

v5:
added a check to validate the ports available before
setting them. also added comments in the testpmd file
for the new function

v4:
the parser is modified so that we don't ues 2 arrays
to convert the listed port values

v3:
squashed the 2 patches and made it 1 patch with
changes only in testpmd. Also working on optmizing
the parser

v2:
moved the parser function to testpmd
---
 app/test-pmd/config.c | 105 +-
 app/test-pmd/parameters.c |   5 ++
 app/test-pmd/testpmd.h|   3 +
 doc/guides/testpmd_app_ug/run_app.rst |   6 ++
 4 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 9669cbd..88144d8 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2564,7 +2564,6 @@ set_fwd_ports_list(unsigned int *portlist, unsigned int 
nb_pt)
unsigned int i;
portid_t port_id;
int record_now;
-
record_now = 0;
  again:
for (i = 0; i < nb_pt; i++) {
@@ -2587,6 +2586,110 @@ set_fwd_ports_list(unsigned int *portlist, unsigned int 
nb_pt)
}
 }
 
+/**
+ * Parse and obtain the list of forwarding ports
+ * from the user input
+ *
+ * @param[in] list
+ *   String containing the user input. User can specify
+ *   in these formats 1,3,5 or 1-3 or 1-2,5 or 3,5-6.
+ *   For example, if the user wants to use all the available
+ *   4 ports in his system, then the input can be 0-3 or 0,1,2,3.
+ *   If the user wants to use only the ports 1,2 then the input
+ *   is 1,2.
+ *   valid characters are '-' and ','
+ *   invalid chars like '.' or '#' will result in
+ *   EAL: Error - exiting with code: 1
+ * Cause: Invalid fwd port list
+ * @param[in] values
+ *   This array will be filled with valid ports available in
+ *   the system
+ * @param[in] maxsize
+ *   Size of the values array
+ * @return
+ *   -On success, returns valid count of ports.
+ *   -On failure, returns -1.
+ */
+static int
+parse_port_list(const char *list, unsigned int *values, int maxsize)
+{
+   int count = 0;
+   char *end = NULL;
+   int min, max;
+   int idx;
+   unsigned int freq[RTE_MAX_ETHPORTS] = {0};
+
+   if (list == NULL || values == NULL || maxsize < 0)
+   return -1;
+
+   /* Remove all blank characters ahead */
+   while (isblank(*list))
+   list++;
+
+   min = maxsize;
+
+   do {
+   while (isblank(*list))
+   list++;
+   if (*list == '\0')
+   return -1;
+   errno = 0;
+   idx = strtol(list, &end, 10);
+   if (errno || end == NULL)
+   return -1;
+   if (idx < 0 || idx >= maxsize)
+   return -1;
+   while (isblank(*end))
+   end++;
+   if (*end == '-') {
+   min = idx;
+   } else if ((*end == ',') || (*end == '\0')) {
+   max = idx;
+   if (min == maxsize)
+   min = idx;
+   for (idx = min; idx <= max; idx++) {
+   if ((count < maxsize) &&
+   rte_eth_dev_is_valid_port(idx)) {
+   if (freq[idx])
+   continue;
+   values[count] = idx;
+   freq[idx] = 1;
+   count++;
+   }
+   }
+   min = maxsize;
+   } else
+   return -1;
+   list = end + 1;
+   } while (*end != '\0');
+
+   if (count == 0)
+   return -1;
+   return count;
+}
+
+void
+parse_fwd_portlist(const char *portlist)
+{
+   int portcount = 0;
+   unsigned int portindex[RTE_MAX_ETHPORTS];
+
+   /*
+* parse_port_list() will mark the portindex array
+* with -1 if the port is not listed and with a positive value
+* for the listed ports. So, the parser is designed in
+* such a way that it will fill the portindex array with the
+* valid ports from the user,and the function set_fwd_ports_list()
+* will set those ports in the forward

Re: [dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY

On Thu, Feb 6, 2020 at 8:26 PM Thierry Herbelot
 wrote:
>
> On 2/6/20 3:48 PM, Jerin Jacob wrote:
> > On Thu, Feb 6, 2020 at 8:06 PM Thierry Herbelot
> >  wrote:
> >>
> >> On 2/6/20 3:27 PM, Bruce Richardson wrote:
> >>> On Thu, Feb 06, 2020 at 02:39:28PM +0100, Thierry Herbelot wrote:
>  Hello,
> 
>  When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 
>  (on an
>  Intel machine):
> 
>  git clone git://dpdk.org/dpdk
>  cd dpdk
>  make config T=x86_64-native-linux-gcc
>  cd build
>  vi .config
>  => disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
>  make
>  ...
>  == Build drivers/net/octeontx2
>  CC otx2_rx.o
>  In file included from 
>  .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
> from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
> from .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
> from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
>  .../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error:
>  rte_security.h: No such file or directory
> #include 
>  ^~~~
>  compilation terminated.
> 
>  This seems cause by f44e7163775537 ('net/octeontx2: add security session
>  operations').
> 
> >>> Disabling parts of the build, particularly libraries, is always likely to
> >>> cause other build failures. I'm not sure we should, or even need to,
> >>> support the disabling of arbitrary libs in DPDK.
> >>
> >> Hello,
> >>
> >> On the other hand, there is no reason delivering unused code in a DPDK
> >> application: an application should be free to select its needed 'modules'.
> >
> > Just to understand the use case, What would be the downside of
> > compiling unwanted code?
> > In meson, it takes only jiffies to compile code and If we use,
> > -no-whole-archive then the generated binary will  not  be bloated,
> > Considering the case where "make" build system will be deprecated soon
> > and,  for meson, I don't think, we are
> > planning to take the route of disabling the "core libraries".
> >
> > Could you share the real-world use for this?
> > My only concern is we can not make tons of #define in the driver code.
> > So, eventually, we end up
> > disabling the driver.
>
> Hello Jerin,

Hello Thierry,

>
> Our use case is that IPsec is provided as part of 6WIND stack, not using
> the version from DPDK (we are using the crypto PMDs from DPDK).

I see. But still, I don't see any issue even If the 6WIND stack is not using any
security or IPsec lib files. Both library and header files will be just unused
in the install directory. Right? Or am I missing something?

> In any case, as the compilation of DPDK is (still) driven by a separate
> configuration file, it should be possible that some combination of

No configuration file option with meson to opt-in and opt-out the library.

> options are disabled, and still DPDK builds fine.
>
> Thierry
>
> >
> >
> >
> >>
> >>  Thanks
> >>
> >>  Thierry
> >>
> >>>
> >>> /Bruce
> >>>
> >>
>
>
> --
> Thierry Herbelot
> 6WIND
> Senior Software Engineer
>
> Tel: +33 1 39 30 92 61
> Fax: +33 1 39 30 92 11
> thierry.herbe...@6wind.com
> www.6wind.com
> Immeuble Central Gare - Bât C 1, place Charles de Gaulle 78180
> Montigny-le-Bretonneux, France
>
> Ce courriel ainsi que toutes les pièces jointes, est uniquement destiné
> à son ou ses destinataires. Il contient des informations confidentielles
> qui sont la propriété de 6WIND. Toute révélation, distribution ou copie
> des informations qu'il contient est strictement interdite. Si vous avez
> reçu ce message par erreur, veuillez immédiatement le signaler à
> l'émetteur et détruire toutes les données reçues
>
> This e-mail message, including any attachments, is for the sole use of
> the intended recipient(s) and contains information that is confidential
> and proprietary to 6WIND. All unauthorized review, use, disclosure or
> distribution is prohibited. If you are not the intended recipient,
> please contact the sender by reply e-mail and destroy all copies of the
> original message.

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] fib: fix possible integer overflow

21/01/2020 16:07, Vladimir Medvedkin:
> This commit fixes possible integer overflow for
> prev_idx in build_common_root() CID 350596
> and
> tbl8_idx in write_edge() CID 350597
> 
> Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
> overflow_before_widen: Potentially overflowing expression tbl8_idx * 256
> with type int (32 bits, signed) is evaluated using 32-bit arithmetic,
> and then used in a context that expects an expression of
> type uint64_t (64 bits, unsigned).
> 
> Coverity issue: 350596, 350597
> Fixes: c3e12e0f0354 ("fib: add dataplane algorithm for IPv6")
> Cc: vladimir.medved...@intel.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Vladimir Medvedkin 

Applied, thanks

Re: [dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY


On 2/6/20 4:06 PM, Jerin Jacob wrote:

On Thu, Feb 6, 2020 at 8:26 PM Thierry Herbelot
 wrote:


On 2/6/20 3:48 PM, Jerin Jacob wrote:

On Thu, Feb 6, 2020 at 8:06 PM Thierry Herbelot
 wrote:


On 2/6/20 3:27 PM, Bruce Richardson wrote:

On Thu, Feb 06, 2020 at 02:39:28PM +0100, Thierry Herbelot wrote:

Hello,

When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 (on an
Intel machine):

git clone git://dpdk.org/dpdk
cd dpdk
make config T=x86_64-native-linux-gcc
cd build
vi .config
 => disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
make
...
== Build drivers/net/octeontx2
 CC otx2_rx.o
In file included from .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
from .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
.../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error:
rte_security.h: No such file or directory
#include 
 ^~~~
compilation terminated.

This seems cause by f44e7163775537 ('net/octeontx2: add security session
operations').


Disabling parts of the build, particularly libraries, is always likely to
cause other build failures. I'm not sure we should, or even need to,
support the disabling of arbitrary libs in DPDK.


Hello,

On the other hand, there is no reason delivering unused code in a DPDK
application: an application should be free to select its needed 'modules'.


Just to understand the use case, What would be the downside of
compiling unwanted code?
In meson, it takes only jiffies to compile code and If we use,
-no-whole-archive then the generated binary will  not  be bloated,
Considering the case where "make" build system will be deprecated soon
and,  for meson, I don't think, we are
planning to take the route of disabling the "core libraries".

Could you share the real-world use for this?
My only concern is we can not make tons of #define in the driver code.
So, eventually, we end up
disabling the driver.


Hello Jerin,


Hello Thierry,



Our use case is that IPsec is provided as part of 6WIND stack, not using
the version from DPDK (we are using the crypto PMDs from DPDK).


I see. But still, I don't see any issue even If the 6WIND stack is not using any
security or IPsec lib files. Both library and header files will be just unused
in the install directory. Right? Or am I missing something?


Hello Jerin,

All calls to libsecurity will still be part of the octeontx2 driver (by 
way of otx2_ethdev_sec.c), so the library will not be 'just unused in 
the install drectory'.


Precisely in a context of security code, it seems strange to bundle 
unused code in a larger application.





In any case, as the compilation of DPDK is (still) driven by a separate
configuration file, it should be possible that some combination of


No configuration file option with meson to opt-in and opt-out the library.


Indeed a better wording would be:
the compilation of DPDK can be driven by a separate configuration file, 
so ...


Thierry




options are disabled, and still DPDK builds fine.

 Thierry







  Thanks

  Thierry



/Bruce

Re: [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context

On Tue, Jan 14, 2020 at 10:05 AM Sunil Kumar Kori  wrote:
>
> From: Harman Kalra 
>
> Added an API to check if current execution is in interrupt
> context. This will be helpful to handle nested interrupt cases.
>
> Signed-off-by: Harman Kalra 
> Signed-off-by: Sunil Kumar Kori 
> Reviewed-by: Jerin Jacob 
> ---
> v7:
>  - No changes.
> v6:
>  - No changes.
> v5:
>  - Fix shared library compilation error
> v4:
>  - No changes.
> v3:
>  - API Comment is updated as per man page.
>  - Scope updated within the library/driver only.
>  - Remove experimental tag
> v2:
>  - Rebased patch on 19.11-rc4
>
>  lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++
>  lib/librte_eal/freebsd/eal/eal_interrupts.c|  5 +
>  lib/librte_eal/linux/eal/eal_interrupts.c  |  5 +
>  lib/librte_eal/rte_eal_version.map |  1 +
>  4 files changed, 22 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h 
> b/lib/librte_eal/common/include/rte_eal_interrupts.h
> index b370c0d26..19d1d45ab 100644
> --- a/lib/librte_eal/common/include/rte_eal_interrupts.h
> +++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
> @@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle 
> *intr_handle);
>  int
>  rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
>
> +/**
> + * @internal
> + * Check if currently executing in interrupt context
> + *
> + * @return
> + *  - non zero in case of interrupt context
> + *  - zero in case of process context
> + */
> +int
> +rte_thread_is_intr(void);
> +
>  #endif /* _RTE_EAL_INTERRUPTS_H_ */
> diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c 
> b/lib/librte_eal/freebsd/eal/eal_interrupts.c
> index f6831b790..ce2a27b4a 100644
> --- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
> +++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
> @@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle 
> *intr_handle)
>  {
> RTE_SET_USED(intr_handle);
>  }
> +
> +int rte_thread_is_intr(void)
> +{
> +   return pthread_equal(intr_thread, pthread_self());
> +}
> diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c 
> b/lib/librte_eal/linux/eal/eal_interrupts.c
> index 14ebb108c..cb8e10709 100644
> --- a/lib/librte_eal/linux/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linux/eal/eal_interrupts.c
> @@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle 
> *intr_handle)
>
> return 0;
>  }
> +
> +int rte_thread_is_intr(void)
> +{
> +   return pthread_equal(intr_thread, pthread_self());
> +}
> diff --git a/lib/librte_eal/rte_eal_version.map 
> b/lib/librte_eal/rte_eal_version.map
> index e38d02530..06ed2e9dc 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -91,6 +91,7 @@ DPDK_20.0 {
> rte_intr_free_epoll_fd;
> rte_intr_rx_ctl;
> rte_intr_tls_epfd;
> +   rte_thread_is_intr;
> rte_keepalive_create;
> rte_keepalive_dispatch_pings;
> rte_keepalive_mark_alive;
> --
> 2.17.1


We can't put this symbol in the stable ABI and just flag it as
internal in the function description.
Users won't notice this warning.

I moved this as an experimental API and enabled the use of
experimental api in octeontx2 common driver.
We can revisit once we have the rte_internal tag Neil had proposed.


Series applied, thanks.



--
David Marchand

Re: [dpdk-dev] drivers/octeontx2: compilation fails without RTE_LIBRTE_SECURITY

On Thu, Feb 6, 2020 at 8:48 PM Thierry Herbelot
 wrote:
>
> On 2/6/20 4:06 PM, Jerin Jacob wrote:
> > On Thu, Feb 6, 2020 at 8:26 PM Thierry Herbelot
> >  wrote:
> >>
> >> On 2/6/20 3:48 PM, Jerin Jacob wrote:
> >>> On Thu, Feb 6, 2020 at 8:06 PM Thierry Herbelot
> >>>  wrote:
> 
>  On 2/6/20 3:27 PM, Bruce Richardson wrote:
> > On Thu, Feb 06, 2020 at 02:39:28PM +0100, Thierry Herbelot wrote:
> >> Hello,
> >>
> >> When RTE_LIBRTE_SECURITY is disabled, compilation fails for octeontx2 
> >> (on an
> >> Intel machine):
> >>
> >> git clone git://dpdk.org/dpdk
> >> cd dpdk
> >> make config T=x86_64-native-linux-gcc
> >> cd build
> >> vi .config
> >>  => disable RTE_LIBRTE_IPSEC and RTE_LIBRTE_SECURITY
> >> make
> >> ...
> >> == Build drivers/net/octeontx2
> >>  CC otx2_rx.o
> >> In file included from 
> >> .../dpdk/drivers/net/octeontx2/otx2_ethdev_sec.h:10,
> >> from .../dpdk/drivers/net/octeontx2/otx2_rx.h:11,
> >> from 
> >> .../dpdk/drivers/net/octeontx2/otx2_ethdev.h:24,
> >> from .../dpdk/drivers/net/octeontx2/otx2_rx.c:7:
> >> .../dpdk/drivers/crypto/octeontx2/otx2_ipsec_fp.h:9:10: fatal error:
> >> rte_security.h: No such file or directory
> >> #include 
> >>  ^~~~
> >> compilation terminated.
> >>
> >> This seems cause by f44e7163775537 ('net/octeontx2: add security 
> >> session
> >> operations').
> >>
> > Disabling parts of the build, particularly libraries, is always likely 
> > to
> > cause other build failures. I'm not sure we should, or even need to,
> > support the disabling of arbitrary libs in DPDK.
> 
>  Hello,
> 
>  On the other hand, there is no reason delivering unused code in a DPDK
>  application: an application should be free to select its needed 
>  'modules'.
> >>>
> >>> Just to understand the use case, What would be the downside of
> >>> compiling unwanted code?
> >>> In meson, it takes only jiffies to compile code and If we use,
> >>> -no-whole-archive then the generated binary will  not  be bloated,
> >>> Considering the case where "make" build system will be deprecated soon
> >>> and,  for meson, I don't think, we are
> >>> planning to take the route of disabling the "core libraries".
> >>>
> >>> Could you share the real-world use for this?
> >>> My only concern is we can not make tons of #define in the driver code.
> >>> So, eventually, we end up
> >>> disabling the driver.
> >>
> >> Hello Jerin,
> >
> > Hello Thierry,
> >
> >>
> >> Our use case is that IPsec is provided as part of 6WIND stack, not using
> >> the version from DPDK (we are using the crypto PMDs from DPDK).
> >
> > I see. But still, I don't see any issue even If the 6WIND stack is not 
> > using any
> > security or IPsec lib files. Both library and header files will be just 
> > unused
> > in the install directory. Right? Or am I missing something?
>
> Hello Jerin,

Hello Thierry,

>
> All calls to libsecurity will still be part of the octeontx2 driver (by
> way of otx2_ethdev_sec.c), so the library will not be 'just unused in
> the install drectory'.

If you are not using the octeontx2 driver and it will be unused.
But if you are using the octeontx2 driver anyway there is a dependency that
we can not avoid it. So are we gaining anything here?

>
> Precisely in a context of security code, it seems strange to bundle
> unused code in a larger application.

Now that rte_secuirty is not experimental. Maybe there should
different treatment.
Assuming someone else needs to disable yet another library say
RTE_LIBRTE_CRYPTODEV,
again we are back to square one.

So I think, we need to have a policy on minimal dependency.

>
> >
> >> In any case, as the compilation of DPDK is (still) driven by a separate
> >> configuration file, it should be possible that some combination of
> >
> > No configuration file option with meson to opt-in and opt-out the library.
>
> Indeed a better wording would be:
> the compilation of DPDK can be driven by a separate configuration file,

But meson does not allow disabling the libraries.

If the stack has some other dependency on such assumptions, I think, better to
improve that now, in the view of meson build in mind.


> so ...
>
> Thierry
>
> >
> >> options are disabled, and still DPDK builds fine.
> >>
> >>  Thierry
> >>
> >>>
> >>>
> >>>
> 
>    Thanks
> 
>    Thierry
> 
> >
> > /Bruce
>
>

Re: [dpdk-dev] [PATCH v2] eal/mem: preallocate VA space in no-huge mode

24/01/2020 18:05, Anatoly Burakov:
> When --no-huge mode is used, the memory is currently allocated with
> mmap(NULL, ...). This is fine in most cases, but can fail in cases
> where DPDK is run on a machine with an IOMMU that is of more limited
> address width than that of a VA, because we're not specifying the
> address hint for mmap() call.
> 
> Fix it by preallocating VA space before mapping it.
> 
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Anatoly Burakov 

Applied, thanks

Re: [dpdk-dev] [dpdk-dev PATCH] raw/octeontx2_ep: fix coverity scan reported defects

27/01/2020 14:18, Mahipal Challa:
> Defects reported by coverity scan are resolved.
> Coverity issue: CID 353611 - Error handling.
> Coverity issue: CID 353622 - Error handling.
> Coverity issue: CID 353632 - NULL pointer dereference.
> 
> Signed-off-by: Mahipal Challa 

Applied, thanks

[dpdk-dev] [PATCH 0/7] bnxt patchset with bug fixes

From: Kalesh AP 

Please apply.

Kalesh AP (5):
  net/bnxt: avoid an unnecessary delay in port stop
  net/bnxt: log firmware debug notifications
  net/bnxt: register for debug notification async event from fw
  net/bnxt: fix to call port stop when error recovery fails
  net/bnxt: move locally used functions to static

Rahul Gupta (1):
  net/bnxt: fix default timeout for ver_get command

Santoshkumar Karanappa Rastapur (1):
  net/bnxt: fix buffer allocation reattempt logic

 drivers/net/bnxt/bnxt.h|  7 ---
 drivers/net/bnxt/bnxt_cpr.c|  5 +
 drivers/net/bnxt/bnxt_ethdev.c | 17 ++---
 drivers/net/bnxt/bnxt_hwrm.c   | 29 +
 drivers/net/bnxt/bnxt_hwrm.h   |  9 +++--
 drivers/net/bnxt/bnxt_rxr.c|  7 ---
 6 files changed, 43 insertions(+), 31 deletions(-)

-- 
2.10.1

[dpdk-dev] [PATCH 6/7] net/bnxt: move locally used functions to static

From: Kalesh AP 

bnxt_rss_ctxts() function is declared in header file bnxt.h,
implemented in bnxt_ethdev.c, and called only in bnxt_ethdev.c.

Also many functions are declared in header file bnxt_hwrm.h,
implemented in bnxt_hwrm.c, and called only in bnxt_hwrm.c.

This patch moves these function declarations into bnxt_ethdev.c
and bnxt_hwrm.c, as static functions.

Signed-off-by: Kalesh AP 
Reviewed-by: Ajit Kumar Khaparde 
---
 drivers/net/bnxt/bnxt.h|  1 -
 drivers/net/bnxt/bnxt_ethdev.c |  2 +-
 drivers/net/bnxt/bnxt_hwrm.c   | 15 ++-
 drivers/net/bnxt/bnxt_hwrm.h   |  5 -
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index e8a30fa..3ae08a2 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -686,7 +686,6 @@ int bnxt_link_update(struct rte_eth_dev *eth_dev, int 
wait_to_complete,
 bool exp_link_status);
 int bnxt_rcv_msg_from_vf(struct bnxt *bp, uint16_t vf_id, void *msg);
 int is_bnxt_in_error(struct bnxt *bp);
-uint16_t bnxt_rss_ctxts(const struct bnxt *bp);
 
 int bnxt_map_fw_health_status_regs(struct bnxt *bp);
 uint32_t bnxt_read_fw_status_reg(struct bnxt *bp, uint32_t index);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 3c40f4b..18aa313 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -150,7 +150,7 @@ int is_bnxt_in_error(struct bnxt *bp)
  * High level utility functions
  */
 
-uint16_t bnxt_rss_ctxts(const struct bnxt *bp)
+static uint16_t bnxt_rss_ctxts(const struct bnxt *bp)
 {
if (!BNXT_CHIP_THOR(bp))
return 1;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 4d99d82..acecf27 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2254,7 +2254,8 @@ int bnxt_clear_all_hwrm_stat_ctxs(struct bnxt *bp)
return 0;
 }
 
-int bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
+static int
+bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
 {
int rc;
unsigned int i;
@@ -2305,7 +2306,8 @@ int bnxt_alloc_all_hwrm_stat_ctxs(struct bnxt *bp)
return rc;
 }
 
-int bnxt_free_all_hwrm_ring_grps(struct bnxt *bp)
+static int
+bnxt_free_all_hwrm_ring_grps(struct bnxt *bp)
 {
uint16_t idx;
uint32_t rc = 0;
@@ -2396,7 +2398,8 @@ void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int 
queue_index)
bp->grp_info[queue_index].cp_fw_ring_id = INVALID_HW_RING_ID;
 }
 
-int bnxt_free_all_hwrm_rings(struct bnxt *bp)
+static int
+bnxt_free_all_hwrm_rings(struct bnxt *bp)
 {
unsigned int i;
 
@@ -2485,7 +2488,8 @@ int bnxt_alloc_hwrm_resources(struct bnxt *bp)
return 0;
 }
 
-int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
+static int
+bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 {
struct bnxt_filter_info *filter;
int rc = 0;
@@ -2546,7 +2550,8 @@ int bnxt_set_hwrm_vnic_filters(struct bnxt *bp, struct 
bnxt_vnic_info *vnic)
return rc;
 }
 
-void bnxt_free_tunnel_ports(struct bnxt *bp)
+static void
+bnxt_free_tunnel_ports(struct bnxt *bp)
 {
if (bp->vxlan_port_cnt)
bnxt_hwrm_tunnel_dst_port_free(bp, bp->vxlan_fw_dst_port_id,
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index 56a851b..5eb2ee8 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -142,14 +142,10 @@ int bnxt_hwrm_vnic_tpa_cfg(struct bnxt *bp,
 
 int bnxt_alloc_all_hwrm_stat_ctxs(struct bnxt *bp);
 int bnxt_clear_all_hwrm_stat_ctxs(struct bnxt *bp);
-int bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp);
-int bnxt_free_all_hwrm_rings(struct bnxt *bp);
-int bnxt_free_all_hwrm_ring_grps(struct bnxt *bp);
 int bnxt_alloc_all_hwrm_ring_grps(struct bnxt *bp);
 void bnxt_free_cp_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr);
 void bnxt_free_nq_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr);
 int bnxt_set_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic);
-int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic);
 void bnxt_free_all_hwrm_resources(struct bnxt *bp);
 void bnxt_free_hwrm_resources(struct bnxt *bp);
 void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index);
@@ -174,7 +170,6 @@ int bnxt_hwrm_tunnel_dst_port_alloc(struct bnxt *bp, 
uint16_t port,
uint8_t tunnel_type);
 int bnxt_hwrm_tunnel_dst_port_free(struct bnxt *bp, uint16_t port,
uint8_t tunnel_type);
-void bnxt_free_tunnel_ports(struct bnxt *bp);
 int bnxt_hwrm_set_default_vlan(struct bnxt *bp, int vf, uint8_t is_vf);
 int bnxt_hwrm_port_qstats(struct bnxt *bp);
 int bnxt_hwrm_port_clr_stats(struct bnxt *bp);
-- 
2.10.1

[dpdk-dev] [PATCH 2/7] net/bnxt: fix default timeout for ver_get command

From: Rahul Gupta 

Initially when driver is loading, there is no HWRM timeout configured
by FW, the VER_GET command needs use default timeout as 500ms and
while recovering from fatal/non-fatal FW error, it should use timeout
as 50ms.

Fixes: 458f0360e8dc ("net/bnxt: get default HWRM command timeout from FW")
Cc: sta...@dpdk.org

Signed-off-by: Rahul Gupta 
Reviewed-by: Kalesh Anakkur Purayil 
Reviewed-by: Somnath Kotur 
---
 drivers/net/bnxt/bnxt.h|  6 --
 drivers/net/bnxt/bnxt_ethdev.c |  4 ++--
 drivers/net/bnxt/bnxt_hwrm.c   | 11 ---
 drivers/net/bnxt/bnxt_hwrm.h   |  2 +-
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 68786a8..e8a30fa 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -606,8 +606,10 @@ struct bnxt {
uint16_tmax_resp_len;
uint16_thwrm_max_ext_req_len;
 
-/* default command timeout value of 50ms */
-#define HWRM_CMD_TIMEOUT   5
+/* default command timeout value of 500ms */
+#define DFLT_HWRM_CMD_TIMEOUT  50
+/* short command timeout value of 50ms */
+#define SHORT_HWRM_CMD_TIMEOUT 5
/* default HWRM request timeout value */
uint32_thwrm_cmd_timeout;
 
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 5378209..7147cc8 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3998,7 +3998,7 @@ static void bnxt_dev_recover(void *arg)
bp->flags &= ~BNXT_FLAG_FATAL_ERROR;
 
do {
-   rc = bnxt_hwrm_ver_get(bp);
+   rc = bnxt_hwrm_ver_get(bp, SHORT_HWRM_CMD_TIMEOUT);
if (rc == 0)
break;
rte_delay_ms(BNXT_FW_READY_WAIT_INTERVAL);
@@ -4688,7 +4688,7 @@ static int bnxt_init_fw(struct bnxt *bp)
 
bp->fw_cap = 0;
 
-   rc = bnxt_hwrm_ver_get(bp);
+   rc = bnxt_hwrm_ver_get(bp, DFLT_HWRM_CMD_TIMEOUT);
if (rc)
return rc;
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index f325aff..96b3431 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -100,11 +100,7 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void 
*msg,
if (bp->flags & BNXT_FLAG_FATAL_ERROR)
return 0;
 
-   /* For VER_GET command, set timeout as 50ms */
-   if (rte_cpu_to_le_16(req->req_type) == HWRM_VER_GET)
-   timeout = HWRM_CMD_TIMEOUT;
-   else
-   timeout = bp->hwrm_cmd_timeout;
+   timeout = bp->hwrm_cmd_timeout;
 
if (bp->flags & BNXT_FLAG_SHORT_CMD ||
msg_len > bp->max_req_len) {
@@ -949,7 +945,7 @@ int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp)
return rc;
 }
 
-int bnxt_hwrm_ver_get(struct bnxt *bp)
+int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
 {
int rc = 0;
struct hwrm_ver_get_input req = {.req_type = 0 };
@@ -960,6 +956,7 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
uint32_t dev_caps_cfg;
 
bp->max_req_len = HWRM_MAX_REQ_LEN;
+   bp->hwrm_cmd_timeout = timeout;
HWRM_PREP(req, VER_GET, BNXT_USE_CHIMP_MB);
 
req.hwrm_intf_maj = HWRM_VERSION_MAJOR;
@@ -994,7 +991,7 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
/* convert timeout to usec */
bp->hwrm_cmd_timeout *= 1000;
if (!bp->hwrm_cmd_timeout)
-   bp->hwrm_cmd_timeout = HWRM_CMD_TIMEOUT;
+   bp->hwrm_cmd_timeout = DFLT_HWRM_CMD_TIMEOUT;
 
if (resp->hwrm_intf_maj_8b != HWRM_VERSION_MAJOR) {
PMD_DRV_LOG(ERR, "Unsupported firmware API version\n");
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index d8d1360..2753720 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -120,7 +120,7 @@ int bnxt_hwrm_stat_ctx_free(struct bnxt *bp,
 int bnxt_hwrm_ctx_qstats(struct bnxt *bp, uint32_t cid, int idx,
 struct rte_eth_stats *stats, uint8_t rx);
 
-int bnxt_hwrm_ver_get(struct bnxt *bp);
+int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout);
 
 int bnxt_hwrm_vnic_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic);
 int bnxt_hwrm_vnic_cfg(struct bnxt *bp, struct bnxt_vnic_info *vnic);
-- 
2.10.1

[dpdk-dev] [PATCH 1/7] net/bnxt: avoid an unnecessary delay in port stop

From: Kalesh AP 

VFs and multifunction PFs do not have the privilege to change
link configuration. We force the physical link down as a part
of device stop only for single physical function(SPF).

This change also helps in eliminating the logs when a VF port
is stopped:

"Port 0: link state change event"
"bnxt_print_link_info(): Port 0 Link Up - speed 25000 Mbps - full-duplex"

Cc: sta...@dpdk.org

Signed-off-by: Kalesh AP 
Reviewed-by: Somnath Kotur 
Reviewed-by: Ajit Kumar Khaparde 
---
 drivers/net/bnxt/bnxt_ethdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index c1cb401..5378209 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -948,9 +948,10 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
bnxt_dev_set_link_down_op(eth_dev);
 
/* Wait for link to be reset and the async notification to process.
-* During reset recovery, there is no need to wait
+* During reset recovery, there is no need to wait and
+* VF/NPAR functions do not have privilege to change PHY config.
 */
-   if (!is_bnxt_in_error(bp))
+   if (!is_bnxt_in_error(bp) && BNXT_SINGLE_PF(bp))
bnxt_link_update(eth_dev, 1, ETH_LINK_DOWN);
 
/* Clean queue intr-vector mapping */
-- 
2.10.1

[dpdk-dev] [PATCH 5/7] net/bnxt: fix to call port stop when error recovery fails

From: Kalesh AP 

During live FW upgrade or error recovery, if restoring the filter
settings fail after port start, driver invokes bnxt_uninit_resources()
only. Fix it to invoke bnxt_dev_stop_op() first before calling
bnxt_uninit_resources().

Fixes: df6cd7c1f73a ("net/bnxt: handle reset notify async event from FW")
Cc: sta...@dpdk.org

Signed-off-by: Kalesh AP 
Reviewed-by: Somnath Kotur 
Reviewed-by: Rahul Gupta 
---
 drivers/net/bnxt/bnxt_ethdev.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7147cc8..3c40f4b 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -4022,15 +4022,17 @@ static void bnxt_dev_recover(void *arg)
rc = bnxt_dev_start_op(bp->eth_dev);
if (rc) {
PMD_DRV_LOG(ERR, "Failed to start port after reset\n");
-   goto err;
+   goto err_start;
}
 
rc = bnxt_restore_filters(bp);
if (rc)
-   goto err;
+   goto err_start;
 
PMD_DRV_LOG(INFO, "Recovered from FW reset\n");
return;
+err_start:
+   bnxt_dev_stop_op(bp->eth_dev);
 err:
bp->flags |= BNXT_FLAG_FATAL_ERROR;
bnxt_uninit_resources(bp, false);
-- 
2.10.1

[dpdk-dev] [PATCH 4/7] net/bnxt: register for debug notification async event from fw

From: Kalesh AP 

PF driver has to register for the debug notification async event
with firmware in the HWRM_FUNC_DRV_RGTR command.

Signed-off-by: Kalesh AP 
Reviewed-by: Ajit Kumar Khaparde 
---
 drivers/net/bnxt/bnxt_hwrm.c | 3 +++
 drivers/net/bnxt/bnxt_hwrm.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 96b3431..4d99d82 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -814,6 +814,9 @@ int bnxt_hwrm_func_driver_register(struct bnxt *bp)
req.async_event_fwd[1] |=
rte_cpu_to_le_32(ASYNC_CMPL_EVENT_ID_PF_DRVR_UNLOAD |
 ASYNC_CMPL_EVENT_ID_VF_CFG_CHANGE);
+   if (BNXT_PF(bp))
+   req.async_event_fwd[1] |=
+   rte_cpu_to_le_32(ASYNC_CMPL_EVENT_ID_DBG_NOTIFICATION);
 
rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index 2753720..56a851b 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -31,6 +31,8 @@ struct bnxt_cp_ring_info;
(1 << (HWRM_ASYNC_EVENT_CMPL_EVENT_ID_PF_DRVR_UNLOAD - 32))
 #define ASYNC_CMPL_EVENT_ID_VF_CFG_CHANGE  \
(1 << (HWRM_ASYNC_EVENT_CMPL_EVENT_ID_VF_CFG_CHANGE - 32))
+#define ASYNC_CMPL_EVENT_ID_DBG_NOTIFICATION   \
+   (1 << (HWRM_ASYNC_EVENT_CMPL_EVENT_ID_DEBUG_NOTIFICATION - 32))
 
 #define HWRM_QUEUE_SERVICE_PROFILE_LOSSY \
HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_LOSSY
-- 
2.10.1

[dpdk-dev] [PATCH 7/7] net/bnxt: fix buffer allocation reattempt logic

From: Santoshkumar Karanappa Rastapur 

In case of a buffer allocation failure, we reattempt buffer allocation
before the rx handler exits. We were not attempting this when producer
index is greater than the number of buffers to allocate. Fixed it with
correct checks.

Fixes: d9dd0b29ed31 ("net/bnxt: fix Rx handling and buffer allocation logic")
Cc: sta...@dpdk.org

Signed-off-by: Santoshkumar Karanappa Rastapur 
Reviewed-by: Somnath Kotur 
---
 drivers/net/bnxt/bnxt_rxr.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 1f47db9..bef9720 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -678,10 +678,11 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf 
**rx_pkts,
 
/* Attempt to alloc Rx buf in case of a previous allocation failure. */
if (rc == -ENOMEM) {
-   int i;
+   int i = RING_NEXT(rxr->rx_ring_struct, prod);
+   int cnt = nb_rx_pkts;
 
-   for (i = prod; i <= nb_rx_pkts;
-   i = RING_NEXT(rxr->rx_ring_struct, i)) {
+   for (; cnt;
+   i = RING_NEXT(rxr->rx_ring_struct, i), cnt--) {
struct bnxt_sw_rx_bd *rx_buf = &rxr->rx_buf_ring[i];
 
/* Buffer already allocated for this index. */
-- 
2.10.1

[dpdk-dev] [PATCH 3/7] net/bnxt: log firmware debug notifications

From: Kalesh AP 

The debug notifications are not functional in nature, they should
only have diagnostic value. Other than logging to system log,
drivers shall not take any other functional action based on this
async event.

Signed-off-by: Kalesh AP 
Reviewed-by: Somnath Kotur 
Reviewed-by: Ajit Kumar Khaparde 
---
 drivers/net/bnxt/bnxt_cpr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index bb316b9..0f7b5e9 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -133,6 +133,11 @@ void bnxt_handle_async_event(struct bnxt *bp,
 
bnxt_schedule_fw_health_check(bp);
break;
+   case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_DEBUG_NOTIFICATION:
+   PMD_DRV_LOG(INFO, "DNC event: evt_data1 %#x evt_data2 %#x\n",
+   rte_le_to_cpu_32(async_cmp->event_data1),
+   rte_le_to_cpu_32(async_cmp->event_data2));
+   break;
default:
PMD_DRV_LOG(DEBUG, "handle_async_event id = 0x%x\n", event_id);
break;
-- 
2.10.1

Re: [dpdk-dev] [PATCH] mbuf: fix to update documentation of QinQ stripped bit interpretation

2020-02-06 Thread Somnath Kotur

On Mon, Jan 6, 2020 at 2:05 PM Somnath Kotur  wrote:
>
> Certain hardware may be able to strip and/or save only the outermost
> VLAN instead of both the VLANs in the mbuf in a QinQ scenario.
> To handle such cases, we could re-interpret setting of just
> PKT_RX_QINQ_STRIPPED to indicate that only the outermost VLAN has
> been stripped and saved in mbuf->vlan_tci_outer.
> Only When both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2
> VLANs have been stripped by the hardware and their TCI are saved in
> mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
>
> Signed-off-by: Somnath Kotur 
> ---
>  lib/librte_mbuf/rte_mbuf_core.h | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h
> index 9a8557d..db1070b 100644
> --- a/lib/librte_mbuf/rte_mbuf_core.h
> +++ b/lib/librte_mbuf/rte_mbuf_core.h
> @@ -124,12 +124,19 @@
>  #define PKT_RX_FDIR_FLX  (1ULL << 14)
>
>  /**
> - * The 2 vlans have been stripped by the hardware and their tci are
> - * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
> + * The outer vlan has been stripped by the hardware and their tci are
> + * saved in mbuf->vlan_tci_outer (outer).
>   * This can only happen if vlan stripping is enabled in the RX
>   * configuration of the PMD.
> - * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN |
> - * PKT_RX_VLAN_STRIPPED | PKT_RX_QINQ) must also be set.
> + * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN |  PKT_RX_QINQ)
> + * must also be set.
> + * When both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2 
> vlans
> + * have been stripped by the hardware and their tci are saved in
> + * mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
> + * This can only happen if vlan stripping is enabled in the RX configuration
> + * of the PMD.
> + * When PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set,
> + * (PKT_RX_VLAN | PKT_RX_QINQ) must also be set.
>   */
>  #define PKT_RX_QINQ_STRIPPED (1ULL << 15)
>
> --
> 1.8.3.1

Seeing as its been a month since it was submitted and no comments on
it yet , are we ok to go ahead and merge this in please?

Thanks
Som

Re: [dpdk-dev] [RFC] Accelerator API to chain packet processing functions

2020-02-06 Thread Coyle, David

Hi Jerin, see reply below

> On Thu, Feb 6, 2020 at 3:35 PM Coyle, David  wrote:
> >
> > Hi Jerin,
> 
> Hi David,
> 
> > Thanks for the comments. Please see replies below.
> >
> > Kind Regards,
> > David
> >
> > > On Tue, Feb 4, 2020 at 8:15 PM David Coyle 
> wrote:
> > > >
> > > > Introduction
> > > > 
> > > >
> > > > This RFC introduces a new DPDK library, rte_accelerator.
> > > >
> > > > The main aim of this library is to provide a flexible and
> > > > extensible way of
> > > combining one or more packet-processing functions into a single
> > > operation, thereby allowing these to be performed in parallel in
> > > optimized software libraries or in a hardware accelerator. These
> > > functions can include cryptography, compression and CRC/checksum
> > > calculation, while others can potentially be added in the future.
> > > Performing these functions in parallel as a single operation can enable a
> significant performance improvement.
> > > >
> > > >
> > > > Background
> > > > ==
> > > >
> > > > There are a number of byte-wise operations which are present and
> > > common across many access network data-plane pipelines, such as
> > > Cipher, Authentication, CRC, Bit-Interleaved-Parity (BIP), other
> > > checksums etc. Some prototyping has been done at Intel in relation
> > > to the 01.org access-network- dataplanes project to prove that a
> > > significant performance improvement is possible when such byte-wise
> > > operations are combined into a single pass of packet data
> > > processing. This performance boost has been prototyped for both XGS-
> PON MAC data-plane and DOCSIS MAC data-plane pipelines.
> > >
> > >
> > > Could you share the relative performance numbers to show the gain?
> >
> > [DC] As mentioned above, the main performance gains are when the
> packet processing operations can be combined into a single pass of the
> packet.
> > Both Crypto-CRC-BIP (for XGS-PON MAC) and Crypto-CRC (for DOCSIS
> MAC) have been implemented in the AESNI MB library as single pass
> operation chains.
> >
> > We have modified the dpdk-crypto-perf-tester as part of our prototyping
> to test the cases where:
> > 1) each packet processing function is done as an independent stage
> > (e.g. calling rte_net_crc for CRC,  AESNI MB through rte_cryptodev for
> > cipher, and a C function to calculate the BIP)
> > 2) all packet processing functions done as a single-pass operation in
> > AESNI MB through rte_cryptodev
> >
> > We see the following results for 1024 byte input frames from dpdk-crypto-
> perf-tester:
> > - XGS-PON MAC (Crypto-CRC-BIP):
> > - 3 independent stages: 1429 cycles/buf (13.75Gbps)
> > - 1 single-pass stage: 896 cycles/buf (21.9Gbps)
> > 37% cycle reduction
> >
> > - DOCSIS MAC (Crypto-CRC):
> > - 2 independent stages: 1421 cycles/buf (13.84Gbps)
> > - 1 single-pass stage: 1133 cycles/buf (17.34Gbps)
> > 20% cycle reduction
> >
> > Adding the accelerator API will allow vendors gain the benefits of
> > these cycle savings
> 
> Numbers make sense. I have seen a similar performance improvement doing
> in one pass with CPU instructions.
> 
> 
> > > > - XGS-PON MAC: Crypto-CRC-BIP
> > > > - Order:
> > > > - Downstream: CRC, Encrypt, BIP
> > >
> > > I understand if the chain has two operations then it may possible to
> > > have handcrafted SW code to do both operations in one pass.
> > > I understand the spec is agnostic on a number of passes it does
> > > require to enable the xfrom but To understand the SW/HW capability,
> > > In the above case, "CRC, Encrypt, BIP", It is done in one pass in SW
> > > or three passes in SW or one pass using HW?
> >
> > [DC] The CRC, Encrypt, BIP is also currently done as 1 pass in AESNI MB
> library SW.
> > However, this could also be performed as a single pass in a HW
> > accelerator
> 
> As a specification, cascading the xform chains make sense.
> Do we have any HW that does support chaining the xforms more than "two"
> in one pass?
> i.e real chaining function where two blocks of HWs work hand in hand for
> chaining.
> If none, it may be better to abstract as synonymous API(No dequeue, no
> enqueue) for the CPU use case.

[DC] I'm not aware of any HW that supports this at the moment, but that's not 
to say it couldn't in the future - if anyone else has any examples though, 
please feel free to share.
Regardless, I don't see why we would introduce a different API for SW devices 
and HW devices.
It would be up to each underlying PMD to decide if/how it supports a particular 
accelerator xform chain, but from an application's point of view, the 
accelerator API is always the same

Re: [dpdk-dev] [RFC] Accelerator API to chain packet processing functions

On Thu, Feb 6, 2020 at 10:01 PM Coyle, David  wrote:

Hi David,

> >
> >
> > > > > - XGS-PON MAC: Crypto-CRC-BIP
> > > > > - Order:
> > > > > - Downstream: CRC, Encrypt, BIP
> > > >
> > > > I understand if the chain has two operations then it may possible to
> > > > have handcrafted SW code to do both operations in one pass.
> > > > I understand the spec is agnostic on a number of passes it does
> > > > require to enable the xfrom but To understand the SW/HW capability,
> > > > In the above case, "CRC, Encrypt, BIP", It is done in one pass in SW
> > > > or three passes in SW or one pass using HW?
> > >
> > > [DC] The CRC, Encrypt, BIP is also currently done as 1 pass in AESNI MB
> > library SW.
> > > However, this could also be performed as a single pass in a HW
> > > accelerator
> >
> > As a specification, cascading the xform chains make sense.
> > Do we have any HW that does support chaining the xforms more than "two"
> > in one pass?
> > i.e real chaining function where two blocks of HWs work hand in hand for
> > chaining.
> > If none, it may be better to abstract as synonymous API(No dequeue, no
> > enqueue) for the CPU use case.
>
> [DC] I'm not aware of any HW that supports this at the moment, but that's not 
> to say it couldn't in the future - if anyone else has any examples though, 
> please feel free to share.
> Regardless, I don't see why we would introduce a different API for SW devices 
> and HW devices.

There is a risk in drafting API that meant for HW without any HW
exists. Because there could be inefficiency on the metadata and fast
path API for both models.
For example, In the case of CPU based scheme, it will be pure overhead
emulate the "queue"(the enqueue and dequeue) for the sake of
abstraction where
CPU works better in the synchronous model and I have doubt that the
session-based scheme will work for HW or not as both difference  HW
needs to work hand in hand(IOMMU aspects for two PCI device)

Having said that, I agree with the need for use case and API for CPU
case. Till we find a HW spec, we need to make the solution as CPU
specific and latter extend based on HW metadata required.
Accelerator API sounds like HW accelerator and there is no HW support
then it may not good. We can change the API that works for the use
cases that we know how it works efficiently.

> It would be up to each underlying PMD to decide if/how it supports a 
> particular accelerator xform chain, but from an application's point of view, 
> the accelerator API is always the same
>
>

Re: [dpdk-dev] [PATCH] mbuf: fix to update documentation of QinQ stripped bit interpretation

Hi Somnath,

Sorry for the delay, please find some comments below.

I suggest the following title instead:

  mbuf: extend meaning of QinQ stripped bit

On Mon, Jan 06, 2020 at 02:04:23PM +0530, Somnath Kotur wrote:
> Certain hardware may be able to strip and/or save only the outermost
> VLAN instead of both the VLANs in the mbuf in a QinQ scenario.
> To handle such cases, we could re-interpret setting of just
> PKT_RX_QINQ_STRIPPED to indicate that only the outermost VLAN has
> been stripped and saved in mbuf->vlan_tci_outer.

To be sure we're on the same page: we are talking about case 7 of this
link: http://inbox.dpdk.org/dev/20191227145041.GQ22738@platinum/

So, even if the inner vlan_tci is not stripped from packet data, it has
to be saved in m->vlan_tci, because it is implied by PKT_RX_VLAN.

>From the same link, the case where the driver only strips+saves the
outer vlan without saving or stripping the inner one is case 3.

> Only When both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2
> VLANs have been stripped by the hardware and their TCI are saved in
> mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
> 
> Signed-off-by: Somnath Kotur 
> ---
>  lib/librte_mbuf/rte_mbuf_core.h | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h
> index 9a8557d..db1070b 100644
> --- a/lib/librte_mbuf/rte_mbuf_core.h
> +++ b/lib/librte_mbuf/rte_mbuf_core.h
> @@ -124,12 +124,19 @@
>  #define PKT_RX_FDIR_FLX  (1ULL << 14)
>  
>  /**
> - * The 2 vlans have been stripped by the hardware and their tci are
> - * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
> + * The outer vlan has been stripped by the hardware and their tci are
> + * saved in mbuf->vlan_tci_outer (outer).

their tci are -> its tci is

>   * This can only happen if vlan stripping is enabled in the RX
>   * configuration of the PMD.
> - * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN |
> - * PKT_RX_VLAN_STRIPPED | PKT_RX_QINQ) must also be set.
> + * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN |  PKT_RX_QINQ)
> + * must also be set.

ok

> + * When both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2 
> vlans
> + * have been stripped by the hardware and their tci are saved in
> + * mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).

This is correct, but I'd use a bullet list to add another sentence:

 * - If both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2 vlans
 *   have been stripped by the hardware and their tci are saved in
 *   mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
 * - If PKT_RX_QINQ_STRIPPED is set and PKT_RX_VLAN_STRIPPED is unset, only the
 *   outer vlan is removed from packet data, but both tci are saved in
 *   mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).

> + * This can only happen if vlan stripping is enabled in the RX configuration
> + * of the PMD.

The same exact sentence is above, this one can be removed.

> + * When PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set,
> + * (PKT_RX_VLAN | PKT_RX_QINQ) must also be set.

This can be removed too as it is redundant with above sentence:

 * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN |  PKT_RX_QINQ)
 * must also be set.


Thanks,
Olivier

Re: [dpdk-dev] [RFC PATCH 1/7] vfio: Include optional device match in vfio_device_ops callbacks

2020-02-06 Thread Alex Williamson

On Thu, 6 Feb 2020 12:14:19 +0100
Cornelia Huck  wrote:

> On Tue, 04 Feb 2020 16:05:43 -0700
> Alex Williamson  wrote:
> 
> > Allow bus drivers to provide their own callback to match a device to
> > the user provided string.
> > 
> > Signed-off-by: Alex Williamson 
> > ---
> >  drivers/vfio/vfio.c  |   19 +++
> >  include/linux/vfio.h |3 +++
> >  2 files changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index 388597930b64..dda1726adda8 100644
> > --- a/drivers/vfio/vfio.c
> > +++ b/drivers/vfio/vfio.c
> > @@ -875,11 +875,22 @@ EXPORT_SYMBOL_GPL(vfio_device_get_from_dev);
> >  static struct vfio_device *vfio_device_get_from_name(struct vfio_group 
> > *group,
> >  char *buf)
> >  {
> > -   struct vfio_device *it, *device = NULL;
> > +   struct vfio_device *it, *device = ERR_PTR(-ENODEV);
> >  
> > mutex_lock(&group->device_lock);
> > list_for_each_entry(it, &group->device_list, group_next) {
> > -   if (!strcmp(dev_name(it->dev), buf)) {
> > +   int ret;
> > +
> > +   if (it->ops->match) {
> > +   ret = it->ops->match(it->device_data, buf);
> > +   if (ret < 0 && ret != -ENODEV) {
> > +   device = ERR_PTR(ret);
> > +   break;
> > +   }
> > +   } else
> > +   ret = strcmp(dev_name(it->dev), buf);  
> 
> The asymmetric braces look a bit odd.

Ok

> > +
> > +   if (!ret) {
> > device = it;
> > vfio_device_get(device);
> > break;
> > @@ -1441,8 +1452,8 @@ static int vfio_group_get_device_fd(struct vfio_group 
> > *group, char *buf)
> > return -EPERM;
> >  
> > device = vfio_device_get_from_name(group, buf);
> > -   if (!device)
> > -   return -ENODEV;
> > +   if (IS_ERR(device))
> > +   return PTR_ERR(device);
> >  
> > ret = device->ops->open(device->device_data);
> > if (ret) {
> > diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> > index e42a711a2800..755e0f0e2900 100644
> > --- a/include/linux/vfio.h
> > +++ b/include/linux/vfio.h
> > @@ -26,6 +26,8 @@
> >   * operations documented below
> >   * @mmap: Perform mmap(2) on a region of the device file descriptor
> >   * @request: Request for the bus driver to release the device
> > + * @match: Optional device name match callback (return: 0 for match, 
> > -ENODEV
> > + * (or >0) for no match and continue, other -errno: no match and 
> > stop)  
> 
> I'm wondering about these conventions.
> 
> If you basically want a tri-state return (match, don't match/continue,
> don't match/stop), putting -ENODEV and >0 together feels odd. I would
> rather expect either
> - < 0 == don't match/stop, 0 == don't match/continue, > 0 == match, or

So sort of a bool + errno.  I shied away from this because returning
zero for success, or match, is such a common semantic, especially when
we're replacing a simple strcmp().  I suppose it's logically just
!strcmp() though, which avoids the abort case for a simple
implementation like patch 2/7.

> - 0 == match, -ENODEV (or some other defined error) == don't
>   match/continue, all other values == don't match/abort?

This is closest to what I arrived at in this version, but I found it
necessary to exclude positive values from the no-match/abort and
consider them as no-match/continue because I didn't want to assume the
errno for that case.

I think with your first option we arrive at something like this:

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index dda1726adda8..b5609a411139 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -883,14 +883,15 @@ static struct vfio_device 
*vfio_device_get_from_name(struct vfio_group *group,
 
if (it->ops->match) {
ret = it->ops->match(it->device_data, buf);
-   if (ret < 0 && ret != -ENODEV) {
+   if (ret < 0) {
device = ERR_PTR(ret);
break;
}
-   } else
-   ret = strcmp(dev_name(it->dev), buf);
+   } else {
+   ret = !strcmp(dev_name(it->dev), buf);
+   }
 
-   if (!ret) {
+   if (ret) {
device = it;
vfio_device_get(device);
break;
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 755e0f0e2900..029694b977f2 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -26,8 +26,9 @@
  * operations documented below
  * @mmap: Perform mmap(2) on a region of the device file descriptor
  * @request: Request for the bus driver to release the device
- * @match: Optional device name match callback (return: 0 for match, -ENO

[dpdk-dev] [PATCH v2 4/7] net/bnxt: register for debug notification event from FW

From: Kalesh AP 

PF driver has to register for the debug notification async event
with firmware in the HWRM_FUNC_DRV_RGTR command.

Signed-off-by: Kalesh AP 
Reviewed-by: Ajit Khaparde 
---
 drivers/net/bnxt/bnxt_hwrm.c | 3 +++
 drivers/net/bnxt/bnxt_hwrm.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 96b34317b..4d99d824e 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -814,6 +814,9 @@ int bnxt_hwrm_func_driver_register(struct bnxt *bp)
req.async_event_fwd[1] |=
rte_cpu_to_le_32(ASYNC_CMPL_EVENT_ID_PF_DRVR_UNLOAD |
 ASYNC_CMPL_EVENT_ID_VF_CFG_CHANGE);
+   if (BNXT_PF(bp))
+   req.async_event_fwd[1] |=
+   rte_cpu_to_le_32(ASYNC_CMPL_EVENT_ID_DBG_NOTIFICATION);
 
rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index 2753720ae..56a851b8a 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -31,6 +31,8 @@ struct bnxt_cp_ring_info;
(1 << (HWRM_ASYNC_EVENT_CMPL_EVENT_ID_PF_DRVR_UNLOAD - 32))
 #define ASYNC_CMPL_EVENT_ID_VF_CFG_CHANGE  \
(1 << (HWRM_ASYNC_EVENT_CMPL_EVENT_ID_VF_CFG_CHANGE - 32))
+#define ASYNC_CMPL_EVENT_ID_DBG_NOTIFICATION   \
+   (1 << (HWRM_ASYNC_EVENT_CMPL_EVENT_ID_DEBUG_NOTIFICATION - 32))
 
 #define HWRM_QUEUE_SERVICE_PROFILE_LOSSY \
HWRM_QUEUE_QPORTCFG_OUTPUT_QUEUE_ID0_SERVICE_PROFILE_LOSSY
-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH v2 1/7] net/bnxt: fix unnecessary delay in port stop

From: Kalesh AP 

VFs and multifunction PFs do not have the privilege to change
link configuration. We force the physical link down as a part
of device stop only for single physical function(SPF).

This change also helps in eliminating the logs when a VF port
is stopped:

"Port 0: link state change event"
"bnxt_print_link_info(): Port 0 Link Up - speed 25000 Mbps - full-duplex"

Fixes: 316e412299fd ("net/bnxt: fix crash when closing")
Cc: sta...@dpdk.org

Signed-off-by: Kalesh AP 
Reviewed-by: Somnath Kotur 
Reviewed-by: Ajit Khaparde 
---
 drivers/net/bnxt/bnxt_ethdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index c1cb40160..537820960 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -948,9 +948,10 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
bnxt_dev_set_link_down_op(eth_dev);
 
/* Wait for link to be reset and the async notification to process.
-* During reset recovery, there is no need to wait
+* During reset recovery, there is no need to wait and
+* VF/NPAR functions do not have privilege to change PHY config.
 */
-   if (!is_bnxt_in_error(bp))
+   if (!is_bnxt_in_error(bp) && BNXT_SINGLE_PF(bp))
bnxt_link_update(eth_dev, 1, ETH_LINK_DOWN);
 
/* Clean queue intr-vector mapping */
-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH v2 3/7] net/bnxt: log firmware debug notifications

From: Kalesh AP 

The debug notifications are not functional in nature, they should
only have diagnostic value. Other than logging to system log,
drivers shall not take any other functional action based on this
async event.

Signed-off-by: Kalesh AP 
Reviewed-by: Somnath Kotur 
Reviewed-by: Ajit Khaparde 
---
 drivers/net/bnxt/bnxt_cpr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index bb316b9e0..0f7b5e96c 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -133,6 +133,11 @@ void bnxt_handle_async_event(struct bnxt *bp,
 
bnxt_schedule_fw_health_check(bp);
break;
+   case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_DEBUG_NOTIFICATION:
+   PMD_DRV_LOG(INFO, "DNC event: evt_data1 %#x evt_data2 %#x\n",
+   rte_le_to_cpu_32(async_cmp->event_data1),
+   rte_le_to_cpu_32(async_cmp->event_data2));
+   break;
default:
PMD_DRV_LOG(DEBUG, "handle_async_event id = 0x%x\n", event_id);
break;
-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH v2 0/7] bnxt patchset with bug fixes

v1->v2: fixed the commit logs.

Kalesh AP (5):
  net/bnxt: fix unnecessary delay in port stop
  net/bnxt: log firmware debug notifications
  net/bnxt: register for debug notification event from FW
  net/bnxt: fix to call port stop when error recovery fails
  net/bnxt: move locally used functions to static

Rahul Gupta (1):
  net/bnxt: fix default timeout for command to get FW version

Santoshkumar Karanappa Rastapur (1):
  net/bnxt: fix buffer allocation reattempt logic

 drivers/net/bnxt/bnxt.h|  7 ---
 drivers/net/bnxt/bnxt_cpr.c|  5 +
 drivers/net/bnxt/bnxt_ethdev.c | 17 ++---
 drivers/net/bnxt/bnxt_hwrm.c   | 29 +
 drivers/net/bnxt/bnxt_hwrm.h   |  9 +++--
 drivers/net/bnxt/bnxt_rxr.c|  7 ---
 6 files changed, 43 insertions(+), 31 deletions(-)

-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH v2 2/7] net/bnxt: fix default timeout for command to get FW version

From: Rahul Gupta 

Initially when driver is loading, there is no HWRM timeout configured
by FW, the VER_GET command needs use default timeout as 500ms and
while recovering from fatal/non-fatal FW error, it should use timeout
as 50ms.

Fixes: 458f0360e8dc ("net/bnxt: get default HWRM command timeout from FW")
Cc: sta...@dpdk.org

Signed-off-by: Rahul Gupta 
Reviewed-by: Kalesh Anakkur Purayil 
Reviewed-by: Somnath Kotur 
Reviewed-by: Ajit Khaparde 
---
 drivers/net/bnxt/bnxt.h|  6 --
 drivers/net/bnxt/bnxt_ethdev.c |  4 ++--
 drivers/net/bnxt/bnxt_hwrm.c   | 11 ---
 drivers/net/bnxt/bnxt_hwrm.h   |  2 +-
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 68786a89b..e8a30fa31 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -606,8 +606,10 @@ struct bnxt {
uint16_tmax_resp_len;
uint16_thwrm_max_ext_req_len;
 
-/* default command timeout value of 50ms */
-#define HWRM_CMD_TIMEOUT   5
+/* default command timeout value of 500ms */
+#define DFLT_HWRM_CMD_TIMEOUT  50
+/* short command timeout value of 50ms */
+#define SHORT_HWRM_CMD_TIMEOUT 5
/* default HWRM request timeout value */
uint32_thwrm_cmd_timeout;
 
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 537820960..7147cc8fe 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3998,7 +3998,7 @@ static void bnxt_dev_recover(void *arg)
bp->flags &= ~BNXT_FLAG_FATAL_ERROR;
 
do {
-   rc = bnxt_hwrm_ver_get(bp);
+   rc = bnxt_hwrm_ver_get(bp, SHORT_HWRM_CMD_TIMEOUT);
if (rc == 0)
break;
rte_delay_ms(BNXT_FW_READY_WAIT_INTERVAL);
@@ -4688,7 +4688,7 @@ static int bnxt_init_fw(struct bnxt *bp)
 
bp->fw_cap = 0;
 
-   rc = bnxt_hwrm_ver_get(bp);
+   rc = bnxt_hwrm_ver_get(bp, DFLT_HWRM_CMD_TIMEOUT);
if (rc)
return rc;
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index f325aff82..96b34317b 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -100,11 +100,7 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void 
*msg,
if (bp->flags & BNXT_FLAG_FATAL_ERROR)
return 0;
 
-   /* For VER_GET command, set timeout as 50ms */
-   if (rte_cpu_to_le_16(req->req_type) == HWRM_VER_GET)
-   timeout = HWRM_CMD_TIMEOUT;
-   else
-   timeout = bp->hwrm_cmd_timeout;
+   timeout = bp->hwrm_cmd_timeout;
 
if (bp->flags & BNXT_FLAG_SHORT_CMD ||
msg_len > bp->max_req_len) {
@@ -949,7 +945,7 @@ int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp)
return rc;
 }
 
-int bnxt_hwrm_ver_get(struct bnxt *bp)
+int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
 {
int rc = 0;
struct hwrm_ver_get_input req = {.req_type = 0 };
@@ -960,6 +956,7 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
uint32_t dev_caps_cfg;
 
bp->max_req_len = HWRM_MAX_REQ_LEN;
+   bp->hwrm_cmd_timeout = timeout;
HWRM_PREP(req, VER_GET, BNXT_USE_CHIMP_MB);
 
req.hwrm_intf_maj = HWRM_VERSION_MAJOR;
@@ -994,7 +991,7 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
/* convert timeout to usec */
bp->hwrm_cmd_timeout *= 1000;
if (!bp->hwrm_cmd_timeout)
-   bp->hwrm_cmd_timeout = HWRM_CMD_TIMEOUT;
+   bp->hwrm_cmd_timeout = DFLT_HWRM_CMD_TIMEOUT;
 
if (resp->hwrm_intf_maj_8b != HWRM_VERSION_MAJOR) {
PMD_DRV_LOG(ERR, "Unsupported firmware API version\n");
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index d8d1360f9..2753720ae 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -120,7 +120,7 @@ int bnxt_hwrm_stat_ctx_free(struct bnxt *bp,
 int bnxt_hwrm_ctx_qstats(struct bnxt *bp, uint32_t cid, int idx,
 struct rte_eth_stats *stats, uint8_t rx);
 
-int bnxt_hwrm_ver_get(struct bnxt *bp);
+int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout);
 
 int bnxt_hwrm_vnic_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic);
 int bnxt_hwrm_vnic_cfg(struct bnxt *bp, struct bnxt_vnic_info *vnic);
-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH v2 6/7] net/bnxt: move locally used functions to static

From: Kalesh AP 

bnxt_rss_ctxts() function is declared in header file bnxt.h,
implemented in bnxt_ethdev.c, and called only in bnxt_ethdev.c.

Also many functions are declared in header file bnxt_hwrm.h,
implemented in bnxt_hwrm.c, and called only in bnxt_hwrm.c.

This patch moves these function declarations into bnxt_ethdev.c
and bnxt_hwrm.c, as static functions.

Signed-off-by: Kalesh AP 
Reviewed-by: Ajit Khaparde 
---
 drivers/net/bnxt/bnxt.h|  1 -
 drivers/net/bnxt/bnxt_ethdev.c |  2 +-
 drivers/net/bnxt/bnxt_hwrm.c   | 15 ++-
 drivers/net/bnxt/bnxt_hwrm.h   |  5 -
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index e8a30fa31..3ae08a2ed 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -686,7 +686,6 @@ int bnxt_link_update(struct rte_eth_dev *eth_dev, int 
wait_to_complete,
 bool exp_link_status);
 int bnxt_rcv_msg_from_vf(struct bnxt *bp, uint16_t vf_id, void *msg);
 int is_bnxt_in_error(struct bnxt *bp);
-uint16_t bnxt_rss_ctxts(const struct bnxt *bp);
 
 int bnxt_map_fw_health_status_regs(struct bnxt *bp);
 uint32_t bnxt_read_fw_status_reg(struct bnxt *bp, uint32_t index);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 3c40f4b59..18aa313fd 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -150,7 +150,7 @@ int is_bnxt_in_error(struct bnxt *bp)
  * High level utility functions
  */
 
-uint16_t bnxt_rss_ctxts(const struct bnxt *bp)
+static uint16_t bnxt_rss_ctxts(const struct bnxt *bp)
 {
if (!BNXT_CHIP_THOR(bp))
return 1;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 4d99d824e..acecf2784 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2254,7 +2254,8 @@ int bnxt_clear_all_hwrm_stat_ctxs(struct bnxt *bp)
return 0;
 }
 
-int bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
+static int
+bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp)
 {
int rc;
unsigned int i;
@@ -2305,7 +2306,8 @@ int bnxt_alloc_all_hwrm_stat_ctxs(struct bnxt *bp)
return rc;
 }
 
-int bnxt_free_all_hwrm_ring_grps(struct bnxt *bp)
+static int
+bnxt_free_all_hwrm_ring_grps(struct bnxt *bp)
 {
uint16_t idx;
uint32_t rc = 0;
@@ -2396,7 +2398,8 @@ void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int 
queue_index)
bp->grp_info[queue_index].cp_fw_ring_id = INVALID_HW_RING_ID;
 }
 
-int bnxt_free_all_hwrm_rings(struct bnxt *bp)
+static int
+bnxt_free_all_hwrm_rings(struct bnxt *bp)
 {
unsigned int i;
 
@@ -2485,7 +2488,8 @@ int bnxt_alloc_hwrm_resources(struct bnxt *bp)
return 0;
 }
 
-int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
+static int
+bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 {
struct bnxt_filter_info *filter;
int rc = 0;
@@ -2546,7 +2550,8 @@ int bnxt_set_hwrm_vnic_filters(struct bnxt *bp, struct 
bnxt_vnic_info *vnic)
return rc;
 }
 
-void bnxt_free_tunnel_ports(struct bnxt *bp)
+static void
+bnxt_free_tunnel_ports(struct bnxt *bp)
 {
if (bp->vxlan_port_cnt)
bnxt_hwrm_tunnel_dst_port_free(bp, bp->vxlan_fw_dst_port_id,
diff --git a/drivers/net/bnxt/bnxt_hwrm.h b/drivers/net/bnxt/bnxt_hwrm.h
index 56a851b8a..5eb2ee883 100644
--- a/drivers/net/bnxt/bnxt_hwrm.h
+++ b/drivers/net/bnxt/bnxt_hwrm.h
@@ -142,14 +142,10 @@ int bnxt_hwrm_vnic_tpa_cfg(struct bnxt *bp,
 
 int bnxt_alloc_all_hwrm_stat_ctxs(struct bnxt *bp);
 int bnxt_clear_all_hwrm_stat_ctxs(struct bnxt *bp);
-int bnxt_free_all_hwrm_stat_ctxs(struct bnxt *bp);
-int bnxt_free_all_hwrm_rings(struct bnxt *bp);
-int bnxt_free_all_hwrm_ring_grps(struct bnxt *bp);
 int bnxt_alloc_all_hwrm_ring_grps(struct bnxt *bp);
 void bnxt_free_cp_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr);
 void bnxt_free_nq_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr);
 int bnxt_set_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic);
-int bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic);
 void bnxt_free_all_hwrm_resources(struct bnxt *bp);
 void bnxt_free_hwrm_resources(struct bnxt *bp);
 void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index);
@@ -174,7 +170,6 @@ int bnxt_hwrm_tunnel_dst_port_alloc(struct bnxt *bp, 
uint16_t port,
uint8_t tunnel_type);
 int bnxt_hwrm_tunnel_dst_port_free(struct bnxt *bp, uint16_t port,
uint8_t tunnel_type);
-void bnxt_free_tunnel_ports(struct bnxt *bp);
 int bnxt_hwrm_set_default_vlan(struct bnxt *bp, int vf, uint8_t is_vf);
 int bnxt_hwrm_port_qstats(struct bnxt *bp);
 int bnxt_hwrm_port_clr_stats(struct bnxt *bp);
-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH v2 5/7] net/bnxt: fix to call port stop when error recovery fails

From: Kalesh AP 

During live FW upgrade or error recovery, if restoring the filter
settings fail after port start, driver invokes bnxt_uninit_resources()
only. Fix it to invoke bnxt_dev_stop_op() first before calling
bnxt_uninit_resources().

Fixes: df6cd7c1f73a ("net/bnxt: handle reset notify async event from FW")
Cc: sta...@dpdk.org

Signed-off-by: Kalesh AP 
Reviewed-by: Somnath Kotur 
Reviewed-by: Rahul Gupta 
---
 drivers/net/bnxt/bnxt_ethdev.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7147cc8fe..3c40f4b59 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -4022,15 +4022,17 @@ static void bnxt_dev_recover(void *arg)
rc = bnxt_dev_start_op(bp->eth_dev);
if (rc) {
PMD_DRV_LOG(ERR, "Failed to start port after reset\n");
-   goto err;
+   goto err_start;
}
 
rc = bnxt_restore_filters(bp);
if (rc)
-   goto err;
+   goto err_start;
 
PMD_DRV_LOG(INFO, "Recovered from FW reset\n");
return;
+err_start:
+   bnxt_dev_stop_op(bp->eth_dev);
 err:
bp->flags |= BNXT_FLAG_FATAL_ERROR;
bnxt_uninit_resources(bp, false);
-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH v2 7/7] net/bnxt: fix buffer allocation reattempt logic

From: Santoshkumar Karanappa Rastapur 

In case of a buffer allocation failure, we reattempt buffer allocation
before the rx handler exits. We were not attempting this when producer
index is greater than the number of buffers to allocate. Fixed it with
correct checks.

Fixes: d9dd0b29ed31 ("net/bnxt: fix Rx handling and buffer allocation logic")
Cc: sta...@dpdk.org

Signed-off-by: Santoshkumar Karanappa Rastapur 
Reviewed-by: Somnath Kotur 
---
 drivers/net/bnxt/bnxt_rxr.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 1f47db97b..bef9720f5 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -678,10 +678,11 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf 
**rx_pkts,
 
/* Attempt to alloc Rx buf in case of a previous allocation failure. */
if (rc == -ENOMEM) {
-   int i;
+   int i = RING_NEXT(rxr->rx_ring_struct, prod);
+   int cnt = nb_rx_pkts;
 
-   for (i = prod; i <= nb_rx_pkts;
-   i = RING_NEXT(rxr->rx_ring_struct, i)) {
+   for (; cnt;
+   i = RING_NEXT(rxr->rx_ring_struct, i), cnt--) {
struct bnxt_sw_rx_bd *rx_buf = &rxr->rx_buf_ring[i];
 
/* Buffer already allocated for this index. */
-- 
2.21.1 (Apple Git-122.3)

[dpdk-dev] [PATCH] doc: update release notes for Broadcom driver changes

Support was added in commit 94eb699bc82e ("net/bnxt: support flow mark action")

Signed-off-by: Ajit Khaparde 
---
 doc/guides/rel_notes/release_20_02.rst | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/rel_notes/release_20_02.rst 
b/doc/guides/rel_notes/release_20_02.rst
index 786f9b119..4f5391d97 100644
--- a/doc/guides/rel_notes/release_20_02.rst
+++ b/doc/guides/rel_notes/release_20_02.rst
@@ -118,6 +118,12 @@ New Features
   Added a new Mellanox vDPA  (``mlx5_vdpa``) PMD.
   See the :doc:`../vdpadevs/mlx5` guide for more details on this driver.
 
+* **Updated Broadcom bnxt  driver**
+
+  Updated Broadcom bnxt driver with new features and improvements, including:
+
+  * Added support for MARK action.
+
 * **Updated testpmd application.**
 
   Added support for ESP and L2TPv3 over IP rte_flow patterns to the testpmd
-- 
2.21.1 (Apple Git-122.3)

Re: [dpdk-dev] [PATCH v2 0/7] bnxt patchset with bug fixes

On Thu, Feb 6, 2020 at 10:44 AM Ajit Khaparde 
wrote:

> v1->v2: fixed the commit logs.
>
Patchset applied to dpdk-next-net-brcm.


>
> Kalesh AP (5):
>   net/bnxt: fix unnecessary delay in port stop
>   net/bnxt: log firmware debug notifications
>   net/bnxt: register for debug notification event from FW
>   net/bnxt: fix to call port stop when error recovery fails
>   net/bnxt: move locally used functions to static
>
> Rahul Gupta (1):
>   net/bnxt: fix default timeout for command to get FW version
>
> Santoshkumar Karanappa Rastapur (1):
>   net/bnxt: fix buffer allocation reattempt logic
>
>  drivers/net/bnxt/bnxt.h|  7 ---
>  drivers/net/bnxt/bnxt_cpr.c|  5 +
>  drivers/net/bnxt/bnxt_ethdev.c | 17 ++---
>  drivers/net/bnxt/bnxt_hwrm.c   | 29 +
>  drivers/net/bnxt/bnxt_hwrm.h   |  9 +++--
>  drivers/net/bnxt/bnxt_rxr.c|  7 ---
>  6 files changed, 43 insertions(+), 31 deletions(-)
>
> --
> 2.21.1 (Apple Git-122.3)
>
>

Re: [dpdk-dev] [PATCH] doc: update release notes for Broadcom driver changes

On Thu, Feb 6, 2020 at 10:48 AM Ajit Khaparde 
wrote:

> Support was added in commit 94eb699bc82e ("net/bnxt: support flow mark
> action")
>
> Signed-off-by: Ajit Khaparde 
>
Patch applied to dpdk-next-net-brcm. Thanks


> ---
>  doc/guides/rel_notes/release_20_02.rst | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_20_02.rst
> b/doc/guides/rel_notes/release_20_02.rst
> index 786f9b119..4f5391d97 100644
> --- a/doc/guides/rel_notes/release_20_02.rst
> +++ b/doc/guides/rel_notes/release_20_02.rst
> @@ -118,6 +118,12 @@ New Features
>Added a new Mellanox vDPA  (``mlx5_vdpa``) PMD.
>See the :doc:`../vdpadevs/mlx5` guide for more details on this driver.
>
> +* **Updated Broadcom bnxt  driver**
> +
> +  Updated Broadcom bnxt driver with new features and improvements,
> including:
> +
> +  * Added support for MARK action.
> +
>  * **Updated testpmd application.**
>
>Added support for ESP and L2TPv3 over IP rte_flow patterns to the
> testpmd
> --
> 2.21.1 (Apple Git-122.3)
>
>

[dpdk-dev] [PATCH 2/2] net/qede: Do not stop vport if not started.

2020-02-06 Thread Manish Chopra

Stopping an already disabled vport leads to firmware
assert. Stop the vport only if it was started.

Fixes: 2ea6f76aff40 ("qede: add core driver")
Cc: sta...@dpdk.org

Signed-off-by: Manish Chopra 
Signed-off-by: Rasesh Mody 
---
 drivers/net/qede/qede_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 500440c2c..74dfe895a 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1509,7 +1509,8 @@ static void qede_dev_close(struct rte_eth_dev *eth_dev)
if (eth_dev->data->dev_started)
qede_dev_stop(eth_dev);
 
-   qede_stop_vport(edev);
+   if (qdev->vport_started)
+   qede_stop_vport(edev);
qdev->vport_started = false;
qede_fdir_dealloc_resc(eth_dev);
qede_dealloc_fp_resc(eth_dev);
-- 
2.17.1

[dpdk-dev] [PATCH 1/2] net/qede: Fix VF re-load failure after ungraceful termination of DPDK

2020-02-06 Thread Manish Chopra

On ungraceful termination of DPDK application, PMD VF driver
fails to re-load due to PF seeing the VF in unexpected state
during VF acquisition handshake.

This patch fixes it by allowing VF to request the PF for soft
FLR during the load in such cases so that it can get cleanly
re-loaded.

Fixes: 2ea6f76aff40 ("qede: add core driver")

Signed-off-by: Manish Chopra 
Signed-off-by: Shahed Shaikh 
Signed-off-by: Rasesh Mody 
---
 drivers/net/qede/base/ecore_dev.c |  2 +-
 drivers/net/qede/base/ecore_dev_api.h |  3 ++
 drivers/net/qede/base/ecore_iov_api.h |  1 +
 drivers/net/qede/base/ecore_sriov.c   | 33 
 drivers/net/qede/base/ecore_vf.c  | 55 +--
 drivers/net/qede/base/ecore_vf.h  | 11 +-
 drivers/net/qede/base/ecore_vf_api.h  |  3 ++
 drivers/net/qede/base/ecore_vfpf_if.h | 41 
 drivers/net/qede/base/mcp_public.h|  2 +
 drivers/net/qede/qede_main.c  |  4 ++
 10 files changed, 149 insertions(+), 6 deletions(-)

diff --git a/drivers/net/qede/base/ecore_dev.c 
b/drivers/net/qede/base/ecore_dev.c
index f33b9910c..86ecfb269 100644
--- a/drivers/net/qede/base/ecore_dev.c
+++ b/drivers/net/qede/base/ecore_dev.c
@@ -5617,7 +5617,7 @@ ecore_hw_prepare_single(struct ecore_hwfn *p_hwfn, void 
OSAL_IOMEM *p_regview,
p_hwfn->db_phys_addr = db_phys_addr;
 
if (IS_VF(p_dev))
-   return ecore_vf_hw_prepare(p_hwfn);
+   return ecore_vf_hw_prepare(p_hwfn, p_params);
 
/* Validate that chip access is feasible */
if (REG_RD(p_hwfn, PXP_PF_ME_OPAQUE_ADDR) == 0x) {
diff --git a/drivers/net/qede/base/ecore_dev_api.h 
b/drivers/net/qede/base/ecore_dev_api.h
index 4d5cc1a0f..5ea8427a0 100644
--- a/drivers/net/qede/base/ecore_dev_api.h
+++ b/drivers/net/qede/base/ecore_dev_api.h
@@ -277,6 +277,9 @@ struct ecore_hw_prepare_params {
 
/* Indicates whether this PF serves a storage target */
bool b_is_target;
+
+   /* retry count for VF acquire on channel timeout */
+   u8 acquire_retry_cnt;
 };
 
 /**
diff --git a/drivers/net/qede/base/ecore_iov_api.h 
b/drivers/net/qede/base/ecore_iov_api.h
index c998dbf8d..545001812 100644
--- a/drivers/net/qede/base/ecore_iov_api.h
+++ b/drivers/net/qede/base/ecore_iov_api.h
@@ -51,6 +51,7 @@ enum ecore_iov_pf_to_vf_status {
PFVF_STATUS_NO_RESOURCE,
PFVF_STATUS_FORCED,
PFVF_STATUS_MALICIOUS,
+   PFVF_STATUS_ACQUIRED,
 };
 
 struct ecore_mcp_link_params;
diff --git a/drivers/net/qede/base/ecore_sriov.c 
b/drivers/net/qede/base/ecore_sriov.c
index deee04ac4..e60257e19 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -61,6 +61,39 @@ const char *qede_ecore_channel_tlvs_string[] = {
"CHANNEL_TLV_COALESCE_READ",
"CHANNEL_TLV_BULLETIN_UPDATE_MAC",
"CHANNEL_TLV_UPDATE_MTU",
+   "CHANNEL_TLV_RDMA_ACQUIRE",
+   "CHANNEL_TLV_RDMA_START",
+   "CHANNEL_TLV_RDMA_STOP",
+   "CHANNEL_TLV_RDMA_ADD_USER",
+   "CHANNEL_TLV_RDMA_REMOVE_USER",
+   "CHANNEL_TLV_RDMA_QUERY_COUNTERS",
+   "CHANNEL_TLV_RDMA_ALLOC_TID",
+   "CHANNEL_TLV_RDMA_REGISTER_TID",
+   "CHANNEL_TLV_RDMA_DEREGISTER_TID",
+   "CHANNEL_TLV_RDMA_FREE_TID",
+   "CHANNEL_TLV_RDMA_CREATE_CQ",
+   "CHANNEL_TLV_RDMA_RESIZE_CQ",
+   "CHANNEL_TLV_RDMA_DESTROY_CQ",
+   "CHANNEL_TLV_RDMA_CREATE_QP",
+   "CHANNEL_TLV_RDMA_MODIFY_QP",
+   "CHANNEL_TLV_RDMA_QUERY_QP",
+   "CHANNEL_TLV_RDMA_DESTROY_QP",
+   "CHANNEL_TLV_RDMA_CREATE_SRQ",
+   "CHANNEL_TLV_RDMA_MODIFY_SRQ",
+   "CHANNEL_TLV_RDMA_DESTROY_SRQ",
+   "CHANNEL_TLV_RDMA_QUERY_PORT",
+   "CHANNEL_TLV_RDMA_QUERY_DEVICE",
+   "CHANNEL_TLV_RDMA_IWARP_CONNECT",
+   "CHANNEL_TLV_RDMA_IWARP_ACCEPT",
+   "CHANNEL_TLV_RDMA_IWARP_CREATE_LISTEN",
+   "CHANNEL_TLV_RDMA_IWARP_DESTROY_LISTEN",
+   "CHANNEL_TLV_RDMA_IWARP_PAUSE_LISTEN",
+   "CHANNEL_TLV_RDMA_IWARP_REJECT",
+   "CHANNEL_TLV_RDMA_IWARP_SEND_RTR",
+   "CHANNEL_TLV_ESTABLISH_LL2_CONN",
+   "CHANNEL_TLV_TERMINATE_LL2_CONN",
+   "CHANNEL_TLV_ASYNC_EVENT",
+   "CHANNEL_TLV_SOFT_FLR",
"CHANNEL_TLV_MAX"
 };
 
diff --git a/drivers/net/qede/base/ecore_vf.c b/drivers/net/qede/base/ecore_vf.c
index c5c081426..db03bc494 100644
--- a/drivers/net/qede/base/ecore_vf.c
+++ b/drivers/net/qede/base/ecore_vf.c
@@ -226,7 +226,6 @@ enum _ecore_status_t ecore_vf_pf_release(struct ecore_hwfn 
*p_hwfn)
return _ecore_vf_pf_release(p_hwfn, true);
 }
 
-#define VF_ACQUIRE_THRESH 3
 static void ecore_vf_pf_acquire_reduce_resc(struct ecore_hwfn *p_hwfn,
struct vf_pf_resc_request *p_req,
struct pf_vf_resc *p_resp)
@@ -251,13 +250,47 @@ static void ecore_vf_pf_acquire_reduce_resc(struct 
ecore_hwfn *p_hwfn,
p_req->num_cids = p_resp->num_cids;
 }
 
-static enum _ecore_status_t eco

Re: [dpdk-dev] [PATCH v2 0/7] MinGW-w64 support

2020-02-06 Thread William Tu

On Wed, Feb 5, 2020 at 10:44 PM Dmitry Kozlyuk  wrote:
>
> This patch series add support for building DPDK using MinGW-w64.
>
> MinGW-w64 provides GNU toolchain and independent platform SDK on
> Windows. It also supports cross-compilation to Windows from POSIX
> systems by providing cross tollchains and libraries [0]. It does NOT
> emulate a full POSIX environment, like Cygwin or MSYS do.
>
> There are advantages in using MinGW-w64 in addition to Clang:
>
> 1. Cross-compilation out-of-the-box. MinGW-w64 is provides a pthread
>implementation, GNU getopt, and Windows platform SDK.
>
> 2. Easier porting of POSIX applications using DPDK to Windows, because
>application code can use the same benefits as mentioned above.
>
> 3. Having both primary compilers enabled on Windows provides more
>diagnostics and generally prevents non-portable code.
>
> [0]: http://mingw-w64.org
>
> v2 Changes:
>
> Add patch to use lowercase system header filenames.
> Move Meson cross-file for x86 to arch directory.
> Change wording in comments.
> Add Meson version warning in documentation.
>
> Dmitry Kozlyuk (7):
>   eal: introduce portable format attribute
>   eal: use portable format attribute
>   cmdline: use portable format attribute
>   eal/windows: use lowercase filenames for system headers
>   build: MinGW-w64 support for Meson
>   build: add cross-file for MinGW-w64
>   doc: guide for Windows build using MinGW-w64
>
>  config/meson.build  | 14 +
>  config/x86/meson_mingw.txt  | 14 +
>  doc/guides/windows_gsg/build_dpdk.rst   | 65 ++---
>  lib/librte_cmdline/cmdline.h|  4 +-
>  lib/librte_eal/common/include/rte_common.h  | 17 +-
>  lib/librte_eal/common/include/rte_debug.h   |  2 +-
>  lib/librte_eal/common/include/rte_devargs.h |  2 +-
>  lib/librte_eal/common/include/rte_log.h |  4 +-
>  lib/librte_eal/meson.build  |  3 +
>  lib/librte_eal/windows/eal/include/rte_os.h |  4 +-
>  lib/meson.build |  8 ++-
>  11 files changed, 118 insertions(+), 19 deletions(-)
>  create mode 100644 config/x86/meson_mingw.txt
>
> --
> 2.25.0
>
Hi Dmitry,

I applied your v2 patch and I did a native build on windows 10.
Hit an error showing
../lib/librte_eal/windows/eal/eal_lcore.c:54:2: error: 'for' loop
initial declarations are only allowed in C99 mode
  for (unsigned int socket = 0; socket <
  ^
../lib/librte_eal/windows/eal/eal_lcore.c:54:2: note: use option
-std=c99 or -std=gnu99 to compile your code

Then I fixed them by having separate declaration out side for loop.
then
$ ninja -C build
works

However the output looks weird:
C:\dpdk\build\examples>dpdk-helloworld.exe
EAL: Detected 2 lcore(s)
EAL: Detected 1 NUMA nodes
hehello fllo frorom cm core 1
ore 0

Regards,
William

--- more detailed ---
C:\dpdk>meson -v
0.53.999

C:\dpdk>gcc -v
Reading specs from c:/mingw-w64/bin/../lib64/gcc/x86_64-w64-mingw32/4.8.3/specs
COLLECT_GCC=gcc


C:\dpdk>meson --reconfigure -Dexamples=helloworld build
The Meson build system
Version: 0.53.999
Source dir: C:\dpdk
Build dir: C:\dpdk\build
Build type: native build
Program cat found: NO
Program more found: YES (C:\WINDOWS\system32\more.COM)
Project name: DPDK
Project version: 20.02.0-rc2
C compiler for the host machine: cc (gcc 4.8.3 "cc (GCC) 4.8.3")
C linker for the host machine: cc GNU ld.bfd 2.24
Host machine cpu family: x86_64
Host machine cpu: x86_64
Program cat found: NO
Program more found: YES (C:\WINDOWS\system32\more.COM)
Program ../buildtools/symlink-drivers-solibs.sh found: YES (sh
C:\dpdk\config\../buildtools/symlink-drivers-so
libs.sh)
Checking for size of "void *" : 8
Library m found: YES
Library numa found: NO
Found pkg-config: C:\mingw-w64\bin\pkg-config.EXE (0.25)

Re: [dpdk-dev] [PATCH v2] eal/mem: preallocate VA space in no-huge mode

06/02/2020 16:39, Thomas Monjalon:
> 24/01/2020 18:05, Anatoly Burakov:
> > When --no-huge mode is used, the memory is currently allocated with
> > mmap(NULL, ...). This is fine in most cases, but can fail in cases
> > where DPDK is run on a machine with an IOMMU that is of more limited
> > address width than that of a VA, because we're not specifying the
> > address hint for mmap() call.
> > 
> > Fix it by preallocating VA space before mapping it.
> > 
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Anatoly Burakov 
> 
> Applied, thanks

Eventually dropped from DPDK 20.02-rc2 because it is breaking no-huge mode.
Sorry

[dpdk-dev] [dpdk-announce] release candidate 20.02-rc2

A new DPDK release candidate is ready for testing:
https://git.dpdk.org/dpdk/tag/?id=v20.02-rc2
226 patches were integrated.

The release notes so far:
http://doc.dpdk.org/guides/rel_notes/release_20_02.html

Highlights of 20.02-rc2:
- ABI check tooling
- Mellanox vDPA driver
- Marvell OCTEON TX2 inline IPsec
- synchronous CPU crypto API
- CPU crypto based on new libraries (intel-ipsec-mb and 
AArch64cryptolib)
- reverted Chacha20-Poly1305 algorithm (because of ABI issue)
- event mode in l3fwd example

The -rc3 should include only some bug fixes, simple cleanups, doc
and tooling. We have one week to complete this milestone.
Then one more week (allowing -rc4) should be needed before the release.
The schedule is shifting a bit to let more time to Chinese contributors.

Thank you everyone

Re: [dpdk-dev] [PATCH] netvsc: update link info when getting device info

2020-02-06 Thread Stephen Hemminger

On Thu,  6 Feb 2020 12:55:41 +0200
Mohammed Gamal  wrote:

> testpmd 'show summary' command always shows interface status as down
> with 0 Mbps speed regardless of the underlying VF's status.
> This happens as hn_dev_link_update() is never called, even on the initial
> RNDIS_STATUS_MEDIA_CONNECT message as LSC interrupts are not yet enabled
> at this point.
> 
> Let's call it and update link info when calling hn_dev_info_get().
> 
> Signed-off-by: Mohammed Gamal 

This is not the right way to address this.

There is nothing in rte_eth_dev_info about link status.
That is in rte_eth_link_get.  I think the issue is that testpmd
calls with "nowait" and the definition of nowait is poorly designed in original
DPDK. The Intel version of wait/nowait is to have rte_eth_link_get wait until
the link comes up (in a spin loop). And nowait just polls state.

The current netvsc driver implements no wait, as "I will ask the host what
the link state is but not wait for a response". This is probably not
what testpmd wants.

I can easily fix netvsc to block even in nowait case, but first we need
to see what other drivers do.

Also link state is undefined if driver has never started.

Is testpmd using Link State Interrupt?

[dpdk-dev] [RFT] net/netvsc: initialize link state

2020-02-06 Thread Stephen Hemminger

If application is using link state interrupt, the correct link state
needs to be filled in when device is started. This is similar to
how virtio updates link information.

Reported-by: Mohammed Gamal 
Signed-off-by: Stephen Hemminger 
---
This version marked RFT because am in airport without access to a
machine to test it.

 drivers/net/netvsc/hn_ethdev.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index c79f924379fe..564620748daf 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -823,6 +823,10 @@ hn_dev_start(struct rte_eth_dev *dev)
if (error)
hn_rndis_set_rxfilter(hv, 0);
 
+   /* Initialize Link state */
+   if (error == 0)
+   hn_dev_link_update(dev, 0);
+
return error;
 }
 
-- 
2.20.1

[dpdk-dev] [PATCH v8 0/9] Windows patchset with additional EAL functionalities

This patchset includes additional functionalities for Windows EAL
to support command-line parsing feature and some EAL common code
on Windows.

This patchset can be applied to windpdk-next-dev branch in the draft repo.

v8 changes:
Fixed the naming conventions.
Fixed a crash encountered due to getopt function.
Removed "--syslog" from help options for Windows.

v7 changes:
Removed Windows "eal_filesystem.h" for now and will be added
later if required. Currently, Windows EAL uses common version
of eal_filesystem.h.

v6 changes:
Removed sysfs function as it was not required on Windows.
Removed syslog and dlfcn support for Windows.

v5 changes:
Fixed indentation in patch 6.

v4 changes:
Modified license/exceptions.txt file.
The following files in this patch-set require license exceptions as
listed:
dirent.h  MIT license
getopt.h  BSD-2-Clause license
getopt.c  ISC and BSD-2-Clause license

Removed syslog file in Windows and added ifndef Windows around syslog
classification parameters in the common code.

v3 Changes:
Modified generic rte_vect to add Windows support.
Moved RTE_CPU* definitions to OS specific file.
Added SPDX tag on top of third party files.

v2 Changes:
syslog.h: Replaced the BSD license boilerplate to SPDX tag.


Pallavi Kadam (9):
  license: add license exception for windows
  eal: dirent.h implementation for windows
  eal: add additional function overrides in windows header files
  eal: getopt implementation for windows
  eal: add function to detect process type
  eal: include SSE4 support for windows
  eal: remove syslog and dlfcn support for windows
  build: add additional common files support
  eal: add minimum viable code to support parsing

 lib/librte_eal/common/eal_common_options.c|  14 +
 .../common/include/arch/x86/rte_vect.h|   4 +-
 lib/librte_eal/windows/eal/eal.c  | 194 -
 lib/librte_eal/windows/eal/eal_debug.c|   1 +
 lib/librte_eal/windows/eal/eal_lcore.c|   3 +
 lib/librte_eal/windows/eal/eal_thread.c   |  11 +
 lib/librte_eal/windows/eal/getopt.c   | 470 +
 lib/librte_eal/windows/eal/include/dirent.h   | 664 ++
 lib/librte_eal/windows/eal/include/getopt.h   |  92 +++
 lib/librte_eal/windows/eal/include/pthread.h  |  66 ++
 lib/librte_eal/windows/eal/include/rte_os.h   |  42 ++
 lib/librte_eal/windows/eal/include/sched.h|  46 ++
 .../windows/eal/include/sys/queue.h   |   8 +
 lib/librte_eal/windows/eal/meson.build|   9 +-
 license/exceptions.txt|  12 +-
 15 files changed, 1625 insertions(+), 11 deletions(-)
 create mode 100644 lib/librte_eal/windows/eal/getopt.c
 create mode 100644 lib/librte_eal/windows/eal/include/dirent.h
 create mode 100644 lib/librte_eal/windows/eal/include/getopt.h

-- 
2.18.0.windows.1

[dpdk-dev] [PATCH v8 1/9] license: add license exception for windows

The Governing Board and Tech Board have provided exceptions for
MIT and BSD-2-Clause license files for DPDK support on Windows.

Signed-off-by: Pallavi Kadam 
Reviewed-by: Ranjit Menon 
---
 license/exceptions.txt | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/license/exceptions.txt b/license/exceptions.txt
index ee25bb9a1..60b983122 100644
--- a/license/exceptions.txt
+++ b/license/exceptions.txt
@@ -9,9 +9,11 @@ Note that following licenses are not exceptions:-
- Dual BSD-3-Clause OR LGPL-2.1
- GPL-2.0  (*Only for kernel code*)
 
--
+---
 SPDX Identifier TB Approval Date  GB Approval Date  File name
--
-1.
-
--
+---
+1.MIT   11/12/201911/12/2019
lib/librte_eal/windows/eal/include/dirent.h
+2.BSD-2-Clause  01/08/202001/08/2020
lib/librte_eal/windows/eal/include/getopt.h
+3.ISC AND
+  BSD-2-Clause  01/08/202001/08/2020
lib/librte_eal/windows/eal/getopt.c
+---
-- 
2.18.0.windows.1

[dpdk-dev] [PATCH v8 2/9] eal: dirent.h implementation for windows

Adding dirent.h on Windows to support common code.
eal_common_options.c includes this file.

The original contribution is under MIT license.
https://github.com/tronkko/dirent

Signed-off-by: Antara Ganesh Kolar 
Signed-off-by: Pallavi Kadam 
Reviewed-by: Ranjit Menon 
Reviewed-by: Keith Wiles 
---
 lib/librte_eal/windows/eal/include/dirent.h | 664 
 1 file changed, 664 insertions(+)
 create mode 100644 lib/librte_eal/windows/eal/include/dirent.h

diff --git a/lib/librte_eal/windows/eal/include/dirent.h 
b/lib/librte_eal/windows/eal/include/dirent.h
new file mode 100644
index 0..3a5750788
--- /dev/null
+++ b/lib/librte_eal/windows/eal/include/dirent.h
@@ -0,0 +1,664 @@
+/* SPDX-License-Identifier: MIT
+ * Dirent interface for Microsoft Visual Studio
+ * Version 1.21
+ * Copyright (C) 2006-2012 Toni Ronkko
+ * https://github.com/tronkko/dirent
+ */
+
+#ifndef DIRENT_H
+#define DIRENT_H
+
+/*
+ * Include windows.h without Windows Sockets 1.1 to prevent conflicts with
+ * Windows Sockets 2.0.
+ */
+#ifndef WIN32_LEAN_AND_MEAN
+#   define WIN32_LEAN_AND_MEAN
+#endif
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Maximum length of file name */
+#if !defined(PATH_MAX)
+#   define PATH_MAX MAX_PATH
+#endif
+
+/* File type flags for d_type */
+#define DT_UNKNOWN 0
+#define DT_REG S_IFREG
+#define DT_DIR S_IFDIR
+#define DT_CHR S_IFCHR
+
+/*
+ * File type macros.  Note that block devices, sockets and links cannot be
+ * distinguished on Windows and the macros S_ISBLK, S_ISSOCK and S_ISLNK are
+ * only defined for compatibility.  These macros should always return false
+ * on Windows.
+ */
+#if !defined(S_ISDIR)
+#   define S_ISDIR(mode) (((mode) & S_IFMT) == S_IFDIR)
+#endif
+#if !defined(S_ISREG)
+#   define S_ISREG(mode) (((mode) & S_IFMT) == S_IFREG)
+#endif
+
+/* Wide-character version */
+struct _wdirent {
+   /* Always zero */
+   long d_ino;
+
+   /* Structure size */
+   unsigned short d_reclen;
+
+   /* Length of name without \0 */
+   size_t d_namlen;
+
+   /* File type */
+   int d_type;
+
+   /* File name */
+   wchar_t d_name[PATH_MAX];
+};
+typedef struct _wdirent _wdirent;
+
+struct _WDIR {
+   /* Current directory entry */
+   struct _wdirent ent;
+
+   /* Private file data */
+   WIN32_FIND_DATAW data;
+
+   /* True if data is valid */
+   int cached;
+
+   /* Win32 search handle */
+   HANDLE handle;
+
+   /* Initial directory name */
+   wchar_t *patt;
+};
+typedef struct _WDIR _WDIR;
+
+static _WDIR *_wopendir(const wchar_t *dirname);
+static int _wclosedir(_WDIR *dirp);
+
+/* For compatibility with Symbian */
+#define wdirent _wdirent
+#define WDIR _WDIR
+#define wopendir _wopendir
+#define wclosedir _wclosedir
+
+/* Multi-byte character versions */
+struct dirent {
+   /* Always zero */
+   long d_ino;
+
+   /* Structure size */
+   unsigned short d_reclen;
+
+   /* Length of name without \0 */
+   size_t d_namlen;
+
+   /* File type */
+   int d_type;
+
+   /* File name */
+   char d_name[PATH_MAX];
+};
+typedef struct dirent dirent;
+
+struct DIR {
+   struct dirent ent;
+   struct _WDIR *wdirp;
+};
+typedef struct DIR DIR;
+
+static DIR *opendir(const char *dirname);
+static struct dirent *readdir(DIR *dirp);
+static int closedir(DIR *dirp);
+
+/* Internal utility functions */
+static WIN32_FIND_DATAW *dirent_first(_WDIR *dirp);
+static WIN32_FIND_DATAW *dirent_next(_WDIR *dirp);
+
+static int dirent_mbstowcs_s(
+   size_t *pReturnValue,
+   wchar_t *wcstr,
+   size_t sizeInWords,
+   const char *mbstr,
+   size_t count);
+
+static int dirent_wcstombs_s(
+   size_t *pReturnValue,
+   char *mbstr,
+   size_t sizeInBytes,
+   const wchar_t *wcstr,
+   size_t count);
+
+static void dirent_set_errno(int error);
+
+/*
+ * Open directory stream DIRNAME for read and return a pointer to the
+ * internal working area that is used to retrieve individual directory
+ * entries.
+ */
+static _WDIR*
+_wopendir(const wchar_t *dirname)
+{
+   _WDIR *dirp = NULL;
+   int error;
+
+   /* Must have directory name */
+   if (dirname == NULL || dirname[0] == '\0') {
+   dirent_set_errno(ENOENT);
+   return NULL;
+   }
+
+   /* Allocate new _WDIR structure */
+   dirp = (_WDIR *)malloc(sizeof(struct _WDIR));
+   if (dirp != NULL) {
+   DWORD n;
+
+   /* Reset _WDIR structure */
+   dirp->handle = INVALID_HANDLE_VALUE;
+   dirp->patt = NULL;
+   dirp->cached = 0;
+
+   /* Compute the length of full path plus zero terminator
+*
+* Note that on WinRT there's no way to convert relative paths
+* into absolute paths, so just assume its an absolute path.
+*/
+   #

[dpdk-dev] [PATCH v8 6/9] eal: include SSE4 support for windows

Modified \common\include\arch\x86\rte_vect.h
to include SSE4 header for Windows.

Signed-off-by: Antara Ganesh Kolar 
Signed-off-by: Pallavi Kadam 
Reviewed-by: Ranjit Menon 
Reviewed-by: Keith Wiles 
---
 lib/librte_eal/common/include/arch/x86/rte_vect.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/arch/x86/rte_vect.h 
b/lib/librte_eal/common/include/arch/x86/rte_vect.h
index cf4e9db38..df5a60762 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_vect.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_vect.h
@@ -15,7 +15,9 @@
 #include 
 #include "generic/rte_vect.h"
 
-#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
+#if (defined(__ICC) || \
+   (defined(_WIN64)) || \
+   (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
 
 #include  /* SSE4 */
 
-- 
2.18.0.windows.1

[dpdk-dev] [PATCH v8 7/9] eal: remove syslog and dlfcn support for windows

Excluding syslog/ dlfcn definitions and parameters
from Windows by adding #ifndef RTE_EXEC_ENV_WINDOWS.

Note: This is a temporary change. In future, separate
'unix' directory will be created for unix specific functions.

Signed-off-by: Pallavi Kadam 
Reviewed-by: Ranjit Menon 
---
 lib/librte_eal/common/eal_common_options.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_options.c 
b/lib/librte_eal/common/eal_common_options.c
index 5920233bc..75974dd5b 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -6,12 +6,16 @@
 #include 
 #include 
 #include 
+#ifndef RTE_EXEC_ENV_WINDOWS
 #include 
+#endif
 #include 
 #include 
 #include 
 #include 
+#ifndef RTE_EXEC_ENV_WINDOWS
 #include 
+#endif
 #include 
 #include 
 #include 
@@ -205,7 +209,9 @@ eal_reset_internal_config(struct internal_config 
*internal_cfg)
}
internal_cfg->base_virtaddr = 0;
 
+#ifdef LOG_DAEMON
internal_cfg->syslog_facility = LOG_DAEMON;
+#endif
 
/* if set to NONE, interrupt mode is determined automatically */
internal_cfg->vfio_intr_mode = RTE_INTR_MODE_NONE;
@@ -278,6 +284,7 @@ eal_plugindir_init(const char *path)
 int
 eal_plugins_init(void)
 {
+#ifndef RTE_EXEC_ENV_WINDOWS
struct shared_driver *solib = NULL;
struct stat sb;
 
@@ -306,6 +313,7 @@ eal_plugins_init(void)
 
}
return 0;
+#endif
 }
 
 /*
@@ -927,6 +935,7 @@ eal_parse_lcores(const char *lcores)
return ret;
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 static int
 eal_parse_syslog(const char *facility, struct internal_config *conf)
 {
@@ -965,6 +974,7 @@ eal_parse_syslog(const char *facility, struct 
internal_config *conf)
}
return -1;
 }
+#endif
 
 static int
 eal_parse_log_priority(const char *level)
@@ -1389,6 +1399,7 @@ eal_parse_common_option(int opt, const char *optarg,
}
break;
 
+#ifndef RTE_EXEC_ENV_WINDOWS
case OPT_SYSLOG_NUM:
if (eal_parse_syslog(optarg, conf) < 0) {
RTE_LOG(ERR, EAL, "invalid parameters for --"
@@ -1396,6 +1407,7 @@ eal_parse_common_option(int opt, const char *optarg,
return -1;
}
break;
+#endif
 
case OPT_LOG_LEVEL_NUM: {
if (eal_parse_log_level(optarg) < 0) {
@@ -1675,7 +1687,9 @@ eal_common_usage(void)
   "  (can be used multiple times)\n"
   "  --"OPT_VMWARE_TSC_MAP"Use VMware TSC map instead of 
native RDTSC\n"
   "  --"OPT_PROC_TYPE" Type of this process 
(primary|secondary|auto)\n"
+#ifndef RTE_EXEC_ENV_WINDOWS
   "  --"OPT_SYSLOG"Set syslog facility\n"
+#endif
   "  --"OPT_LOG_LEVEL"=   Set global log level\n"
   "  --"OPT_LOG_LEVEL"=:\n"
   "  Set specific log level\n"
-- 
2.18.0.windows.1

[dpdk-dev] [PATCH v8 4/9] eal: getopt implementation for windows

Adding getopt files to support parsing option on
Windows.

The original contribution is under BSD-2 license.
https://github.com/greenplum-db/libusual/blob/master/usual/getopt.c
https://github.com/greenplum-db/libusual/blob/master/usual/getopt.h

Signed-off-by: Antara Ganesh Kolar 
Signed-off-by: Pallavi Kadam 
Reviewed-by: Ranjit Menon 
Reviewed-by: Keith Wiles 
---
 lib/librte_eal/windows/eal/getopt.c | 470 
 lib/librte_eal/windows/eal/include/getopt.h |  92 
 lib/librte_eal/windows/eal/meson.build  |   1 +
 3 files changed, 563 insertions(+)
 create mode 100644 lib/librte_eal/windows/eal/getopt.c
 create mode 100644 lib/librte_eal/windows/eal/include/getopt.h

diff --git a/lib/librte_eal/windows/eal/getopt.c 
b/lib/librte_eal/windows/eal/getopt.c
new file mode 100644
index 0..170c9b5e0
--- /dev/null
+++ b/lib/librte_eal/windows/eal/getopt.c
@@ -0,0 +1,470 @@
+/* SPDX-License-Identifier: ISC AND BSD-2-Clause
+ * Copyright (c) 2002 Todd C. Miller 
+ *
+ * Sponsored in part by the Defense Advanced Research Projects
+ * Agency (DARPA) and Air Force Research Laboratory, Air Force
+ * Materiel Command, USAF, under agreement number F39502-99-1-0512.
+ */
+/*
+ * Copyright (c) 2000 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * This code is derived from software contributed to The NetBSD Foundation
+ * by Dieter Baron and Thomas Klausner.
+ */
+
+#include 
+
+#ifdef NEED_USUAL_GETOPT
+
+#include 
+#include 
+
+const char*optarg; /* argument associated with option */
+intopterr = 1; /* if error message should be printed */
+intoptind = 1; /* index into parent argv vector */
+intoptopt = '?';   /* character checked for validity */
+
+static void pass(void) {}
+#define warnx(a, ...) pass()
+
+#define PRINT_ERROR((opterr) && (*options != ':'))
+
+#define FLAG_PERMUTE   0x01/* permute non-options to the end of argv */
+#define FLAG_ALLARGS   0x02/* treat non-options as args to option "-1" */
+#define FLAG_LONGONLY  0x04/* operate as getopt_long_only */
+
+/* return values */
+#defineBADCH   ((int)'?')
+#defineBADARG  ((*options == ':') ? (int)':' : (int)'?')
+#defineINORDER 1
+
+#defineEMSG""
+
+static const char *place = EMSG; /* option letter processing */
+
+/* XXX: set optreset to 1 rather than these two */
+static int nonopt_start = -1; /* first non option argument (for permute) */
+static int nonopt_end = -1;   /* first option after non options (for permute) 
*/
+
+/* Error messages */
+static const char recargchar[] = "option requires an argument -- %c";
+static const char recargstring[] = "option requires an argument -- %s";
+static const char ambig[] = "ambiguous option -- %.*s";
+static const char noarg[] = "option doesn't take an argument -- %.*s";
+static const char illoptchar[] = "unknown option -- %c";
+static const char illoptstring[] = "unknown option -- %s";
+
+/*
+ * Compute the greatest common divisor of a and b.
+ */
+static int
+gcd(int a, int b)
+{
+   int c;
+
+   c = a % b;
+   while (c != 0) {
+   a = b;
+   b = c;
+   c = a % b;
+   }
+
+   return (b);
+}
+
+/*
+ * Exchange the block from nonopt_start to nonopt_end with the block
+ * from nonopt_end to opt_end (keeping the same order of arguments
+ * in each block).
+ */
+static void
+permute_args(int panonopt_start, int panonopt_end, int opt_end,
+   char **nargv)
+{
+   int cstart, cyclelen, i, j, ncycle, nnonopts, nopts, pos;
+   char *swap;
+
+   /*
+* compute lengths of blocks and number and size of cycles
+*/
+   nnonopts = panonopt_end - panonopt_start;
+   nopts = opt_end - panonopt_end;
+   ncycle = gcd(nnonopts, nopts);
+   cyclelen = (opt_end - panonopt_start) / ncycle;
+
+   for (i = 0; i < ncycle; i++) {
+   cstart = panonopt_end+i;
+   pos = cstart;
+   for (j = 0; j < cyclelen; j++) {
+   if (pos >= panonopt_end)
+   pos -= nnonopts;
+   else
+   pos += nopts;
+   swap = nargv[pos];
+   /* LINTED const cast */
+   ((char **) nargv)[pos] = nargv[cstart];
+   /* LINTED const cast */
+   ((char **)nargv)[cstart] = swap;
+   }
+   }
+}
+
+/*
+ * parse_long_options --
+ * Parse long options in argc/argv argument vector.
+ * Returns -1 if short_too is set and the option does not match long_options.
+ */
+static int
+parse_long_options(char **nargv, const char *options,
+   const struct option *long_options, int *idx, int short_too)
+{
+   const char *current_argv;
+   char *has_equal;
+   size_t current_argv_len;
+   int i, match;
+
+   current_argv = place;
+

[dpdk-dev] [PATCH v8 3/9] eal: add additional function overrides in windows header files

Adding additional function definitions for pthread, cpuset
implementation, asprintf implementation, in order to support
common code.

Signed-off-by: Bruce Richardson 
Signed-off-by: Pallavi Kadam 
Reviewed-by: Ranjit Menon 
Reviewed-by: Keith Wiles 
---
 lib/librte_eal/windows/eal/include/pthread.h  | 66 +++
 lib/librte_eal/windows/eal/include/rte_os.h   | 42 
 lib/librte_eal/windows/eal/include/sched.h| 46 +
 .../windows/eal/include/sys/queue.h   |  8 +++
 4 files changed, 162 insertions(+)

diff --git a/lib/librte_eal/windows/eal/include/pthread.h 
b/lib/librte_eal/windows/eal/include/pthread.h
index 503329266..4ac24de0a 100644
--- a/lib/librte_eal/windows/eal/include/pthread.h
+++ b/lib/librte_eal/windows/eal/include/pthread.h
@@ -14,12 +14,78 @@
 extern "C" {
 #endif
 
+#include 
+
+#define PTHREAD_BARRIER_SERIAL_THREAD TRUE
+
 /* defining pthread_t type on Windows since there is no in Microsoft libc*/
 typedef uintptr_t pthread_t;
 
 /* defining pthread_attr_t type on Windows since there is no in Microsoft 
libc*/
 typedef void *pthread_attr_t;
 
+typedef SYNCHRONIZATION_BARRIER pthread_barrier_t;
+
+#define pthread_barrier_init(barrier, attr, count) \
+   InitializeSynchronizationBarrier(barrier, count, -1)
+#define pthread_barrier_wait(barrier) EnterSynchronizationBarrier(barrier, \
+   SYNCHRONIZATION_BARRIER_FLAGS_BLOCK_ONLY)
+#define pthread_barrier_destroy(barrier) \
+   DeleteSynchronizationBarrier(barrier)
+#define pthread_cancel(thread) TerminateThread((HANDLE) thread, 0)
+
+/* pthread function overrides */
+#define pthread_self() \
+   ((pthread_t)GetCurrentThreadId())
+#define pthread_setaffinity_np(thread, size, cpuset) \
+   eal_set_thread_affinity_mask(thread, (unsigned long *) cpuset)
+#define pthread_getaffinity_np(thread, size, cpuset) \
+   eal_get_thread_affinity_mask(thread, (unsigned long *) cpuset)
+#define pthread_create(threadid, threadattr, threadfunc, args) \
+   eal_create_thread(threadid, threadfunc, args)
+
+static inline int
+eal_set_thread_affinity_mask(pthread_t threadid, unsigned long *cpuset)
+{
+   SetThreadAffinityMask((HANDLE) threadid, *cpuset);
+   return 0;
+}
+
+static inline int
+eal_get_thread_affinity_mask(pthread_t threadid, unsigned long *cpuset)
+{
+   /* Workaround for the lack of a GetThreadAffinityMask()
+*API in Windows
+*/
+   /* obtain previous mask by setting dummy mask */
+   DWORD dwprevaffinitymask =
+   SetThreadAffinityMask((HANDLE) threadid, 0x1);
+   /* set it back! */
+   SetThreadAffinityMask((HANDLE) threadid, dwprevaffinitymask);
+   *cpuset = dwprevaffinitymask;
+   return 0;
+}
+
+static inline int
+eal_create_thread(void *threadid, void *threadfunc, void *args)
+{
+   HANDLE hThread;
+   hThread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)threadfunc,
+   args, 0, (LPDWORD)threadid);
+   if (hThread) {
+   SetPriorityClass(GetCurrentProcess(), REALTIME_PRIORITY_CLASS);
+   SetThreadPriority(hThread, THREAD_PRIORITY_TIME_CRITICAL);
+   }
+   return ((hThread != NULL) ? 0 : E_FAIL);
+}
+
+static inline int
+pthread_join(pthread_t thread __attribute__((__unused__)),
+   void **value_ptr __attribute__((__unused__)))
+{
+   return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/windows/eal/include/rte_os.h 
b/lib/librte_eal/windows/eal/include/rte_os.h
index fdeae0c8f..9e762617b 100644
--- a/lib/librte_eal/windows/eal/include/rte_os.h
+++ b/lib/librte_eal/windows/eal/include/rte_os.h
@@ -18,6 +18,13 @@ extern "C" {
 #include 
 #include 
 #include 
+#include 
+
+/* limits.h replacement */
+#include 
+#ifndef PATH_MAX
+#define PATH_MAX _MAX_PATH
+#endif
 
 #define strerror_r(a, b, c) strerror_s(b, c, a)
 
@@ -28,6 +35,11 @@ typedef SSIZE_T ssize_t;
 
 #define strtok_r(str, delim, saveptr) strtok_s(str, delim, saveptr)
 
+#define index(a, b) strchr(a, b)
+#define rindex(a, b)strrchr(a, b)
+
+#define strncasecmp(s1, s2, count)_strnicmp(s1, s2, count)
+
 /**
  * Create a thread.
  * This function is private to EAL.
@@ -45,6 +57,36 @@ int eal_thread_create(pthread_t *thread);
  */
 void eal_create_cpu_map(void);
 
+static inline int
+asprintf(char **buffer, const char *format, ...)
+{
+   int size, ret;
+   va_list arg;
+
+   va_start(arg, format);
+   size = vsnprintf(NULL, 0, format, arg) + 1;
+   va_end(arg);
+
+   *buffer = malloc(size);
+   if (buffer == NULL)
+   printf("Cannot allocate memory");
+
+   va_start(arg, format);
+   ret = vsnprintf(*buffer, size, format, arg);
+   va_end(arg);
+   if (ret != size - 1) {
+   free(*buffer);
+   return -1;
+   }
+   return ret;
+}
+
+/* cpu_set macros implementation */
+#define RTE_CPU_AND(dst, src1, src2) CPU_AND(dst, src1, src2)
+#define RTE_CPU_OR(dst, src1,

[dpdk-dev] [PATCH v8 5/9] eal: add function to detect process type

Adding a function to detect process type, also included
header files to contain suitable function declarations
and to support extra warning flags.

Signed-off-by: Pallavi Kadam 
Signed-off-by: Antara Ganesh Kolar 
Reviewed-by: Ranjit Menon 
Reviewed-by: Keith Wiles 
---
 lib/librte_eal/windows/eal/eal.c| 61 -
 lib/librte_eal/windows/eal/eal_debug.c  |  1 +
 lib/librte_eal/windows/eal/eal_lcore.c  |  3 ++
 lib/librte_eal/windows/eal/eal_thread.c | 11 +
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/windows/eal/eal.c b/lib/librte_eal/windows/eal/eal.c
index ce460481f..6a1208c35 100644
--- a/lib/librte_eal/windows/eal/eal.c
+++ b/lib/librte_eal/windows/eal/eal.c
@@ -2,21 +2,48 @@
  * Copyright(c) 2019 Intel Corporation
  */
 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 
+/* define fd variable here, because file needs to be kept open for the
+ * duration of the program, as we hold a write lock on it in the primary proc
+ */
+static int mem_cfg_fd = -1;
+
+/* early configuration structure, when memory config is not mmapped */
+static struct rte_mem_config early_mem_config;
+
 /* Address of global and public configuration */
-static struct rte_config rte_config;
+static struct rte_config rte_config = {
+   .mem_config = &early_mem_config,
+};
 
 /* internal configuration (per-core) */
 struct lcore_config lcore_config[RTE_MAX_LCORE];
 
+/* internal configuration */
+struct internal_config internal_config;
+
+/* platform-specific runtime dir */
+static char runtime_dir[PATH_MAX];
+
+const char *
+rte_eal_get_runtime_dir(void)
+{
+   return runtime_dir;
+}
+
 /* Return a pointer to the configuration structure */
 struct rte_config *
 rte_eal_get_configuration(void)
@@ -24,6 +51,38 @@ rte_eal_get_configuration(void)
return &rte_config;
 }
 
+/* Detect if we are a primary or a secondary process */
+enum rte_proc_type_t
+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
+*/
+   errno_t err = _sopen_s(&mem_cfg_fd, pathname,
+   _O_RDWR, _SH_DENYNO, _S_IREAD | _S_IWRITE);
+   if (err == 0) {
+   OVERLAPPED soverlapped = { 0 };
+   soverlapped.Offset = sizeof(*rte_config.mem_config);
+   soverlapped.OffsetHigh = 0;
+
+   HANDLE hwinfilehandle = (HANDLE)_get_osfhandle(mem_cfg_fd);
+
+   if (!LockFileEx(hwinfilehandle,
+   LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0,
+   sizeof(*rte_config.mem_config), 0, &soverlapped))
+   ptype = RTE_PROC_SECONDARY;
+   }
+
+   RTE_LOG(INFO, EAL, "Auto-detected process type: %s\n",
+   ptype == RTE_PROC_PRIMARY ? "PRIMARY" : "SECONDARY");
+
+   return ptype;
+}
+
 static int
 sync_func(void *arg __rte_unused)
 {
diff --git a/lib/librte_eal/windows/eal/eal_debug.c 
b/lib/librte_eal/windows/eal/eal_debug.c
index edcf257cc..669be6ff9 100644
--- a/lib/librte_eal/windows/eal/eal_debug.c
+++ b/lib/librte_eal/windows/eal/eal_debug.c
@@ -4,6 +4,7 @@
 
 #include 
 #include 
+#include 
 
  /* call abort(), it will generate a coredump if enabled */
 void
diff --git a/lib/librte_eal/windows/eal/eal_lcore.c 
b/lib/librte_eal/windows/eal/eal_lcore.c
index d39f348a3..b3a6c63af 100644
--- a/lib/librte_eal/windows/eal/eal_lcore.c
+++ b/lib/librte_eal/windows/eal/eal_lcore.c
@@ -6,6 +6,9 @@
 
 #include 
 
+#include "eal_private.h"
+#include "eal_thread.h"
+
 /* global data structure that contains the CPU map */
 static struct _wcpu_map {
unsigned int total_procs;
diff --git a/lib/librte_eal/windows/eal/eal_thread.c 
b/lib/librte_eal/windows/eal/eal_thread.c
index 0591d4c7f..9e4bbaa08 100644
--- a/lib/librte_eal/windows/eal/eal_thread.c
+++ b/lib/librte_eal/windows/eal/eal_thread.c
@@ -10,11 +10,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "eal_private.h"
 
 RTE_DEFINE_PER_LCORE(unsigned int, _lcore_id) = LCORE_ID_ANY;
+RTE_DEFINE_PER_LCORE(unsigned int, _socket_id) = (unsigned int)SOCKET_ID_ANY;
+RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
 
 /*
  * Send a message to a slave lcore identified by slave_id to call a
@@ -152,3 +155,11 @@ eal_thread_create(pthread_t *thread)
 
return 0;
 }
+
+int
+rte_thread_setname(__rte_unused pthread_t id, __rte_unused const char *name)
+{
+   /* TODO */
+   /* This is a stub, not the expected result */
+   return 0;
+}
-- 
2.18.0.windows.1

[dpdk-dev] [PATCH v8 9/9] eal: add minimum viable code to support parsing

Adding specific logic for eal.c to support parsing on
Windows.

Signed-off-by: Pallavi Kadam 
Signed-off-by: Antara Ganesh Kolar 
Reviewed-by: Ranjit Menon 
Reviewed-by: Keith Wiles 
---
 lib/librte_eal/windows/eal/eal.c | 133 ++-
 1 file changed, 130 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/windows/eal/eal.c b/lib/librte_eal/windows/eal/eal.c
index 6a1208c35..5c8e7f472 100644
--- a/lib/librte_eal/windows/eal/eal.c
+++ b/lib/librte_eal/windows/eal/eal.c
@@ -16,6 +16,9 @@
 #include 
 #include 
 
+ /* Allow the application to print its usage message too if set */
+static rte_usage_hook_trte_application_usage_hook;
+
 /* define fd variable here, because file needs to be kept open for the
  * duration of the program, as we hold a write lock on it in the primary proc
  */
@@ -83,6 +86,124 @@ eal_proc_type_detect(void)
return ptype;
 }
 
+/* display usage */
+static void
+eal_usage(const char *prgname)
+{
+   printf("\nUsage: %s ", prgname);
+   eal_common_usage();
+   /* Allow the application to print its usage message too
+*if hook is set
+*/
+   if (rte_application_usage_hook) {
+   printf("= Application Usage =\n\n");
+   rte_application_usage_hook(prgname);
+   }
+}
+
+/* Parse the arguments for --log-level only */
+static void
+eal_log_level_parse(int argc, char **argv)
+{
+   int opt;
+   char **argvopt;
+   int option_index;
+
+   argvopt = argv;
+
+   eal_reset_internal_config(&internal_config);
+
+   while ((opt = getopt_long(argc, argvopt, eal_short_options,
+   eal_long_options, &option_index)) != EOF) {
+
+   int ret;
+
+   /* getopt is not happy, stop right now */
+   if (opt == '?')
+   break;
+
+   ret = (opt == OPT_LOG_LEVEL_NUM) ?
+   eal_parse_common_option(opt, optarg,
+   &internal_config) : 0;
+
+   /* common parser is not happy */
+   if (ret < 0)
+   break;
+   }
+
+   optind = 0; /* reset getopt lib */
+}
+
+/* Parse the argument given in the command line of the application */
+__attribute__((optnone)) static int
+eal_parse_args(int argc, char **argv)
+{
+   int opt, ret;
+   char **argvopt;
+   int option_index;
+   char *prgname = argv[0];
+
+   argvopt = argv;
+
+   while ((opt = getopt_long(argc, argvopt, eal_short_options,
+   eal_long_options, &option_index)) != EOF) {
+
+   int ret;
+
+   /* getopt is not happy, stop right now */
+   if (opt == '?') {
+   eal_usage(prgname);
+   return -1;
+   }
+
+   ret = eal_parse_common_option(opt, optarg, &internal_config);
+   /* common parser is not happy */
+   if (ret < 0) {
+   eal_usage(prgname);
+   return -1;
+   }
+   /* common parser handled this option */
+   if (ret == 0)
+   continue;
+
+   switch (opt) {
+   case 'h':
+   eal_usage(prgname);
+   exit(EXIT_SUCCESS);
+   default:
+   if (opt < OPT_LONG_MIN_NUM && isprint(opt)) {
+   RTE_LOG(ERR, EAL, "Option %c is not supported "
+   "on Windows\n", opt);
+   } else if (opt >= OPT_LONG_MIN_NUM &&
+   opt < OPT_LONG_MAX_NUM) {
+   RTE_LOG(ERR, EAL, "Option %s is not supported "
+   "on Windows\n",
+   eal_long_options[option_index].name);
+   } else {
+   RTE_LOG(ERR, EAL, "Option %d is not supported "
+   "on Windows\n", opt);
+   }
+   eal_usage(prgname);
+   return -1;
+   }
+   }
+
+   if (eal_adjust_config(&internal_config) != 0)
+   return -1;
+
+   /* sanity checks */
+   if (eal_check_common_options(&internal_config) != 0) {
+   eal_usage(prgname);
+   return -1;
+   }
+
+   if (optind >= 0)
+   argv[optind - 1] = prgname;
+   ret = optind - 1;
+   optind = 0; /* reset getopt lib */
+   return ret;
+}
+
 static int
 sync_func(void *arg __rte_unused)
 {
@@ -98,9 +219,11 @@ rte_eal_init_alert(const char *msg)
 
  /* Launch threads, called at application init(). */
 int
-rte_eal_init(int argc __rte_unused, char **argv __rte_unused)
+rte_eal_init(int argc, char **argv)
 {
-   int i;
+   int i, fctret;
+
+   eal_log_level_parse(argc, argv);

[dpdk-dev] [PATCH v8 8/9] build: add additional common files support

Added support for additional common files in meson build
to expand Windows EAL and to support the lcore parsing
feature on Windows.

Signed-off-by: Pallavi Kadam 
Signed-off-by: Antara Ganesh Kolar 
Reviewed-by: Ranjit Menon 
Reviewed-by: Keith Wiles 
---
 lib/librte_eal/windows/eal/meson.build | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/windows/eal/meson.build 
b/lib/librte_eal/windows/eal/meson.build
index d37222158..2a062c365 100644
--- a/lib/librte_eal/windows/eal/meson.build
+++ b/lib/librte_eal/windows/eal/meson.build
@@ -8,10 +8,16 @@ env_headers = files(
'include/rte_os.h',
 )
 common_sources = files(
+   '../../common/eal_common_bus.c',
+   '../../common/eal_common_class.c',
+   '../../common/eal_common_devargs.c',
'../../common/eal_common_errno.c',
'../../common/eal_common_launch.c',
'../../common/eal_common_lcore.c',
-   '../../common/eal_common_log.c'
+   '../../common/eal_common_log.c',
+   '../../common/eal_common_options.c',
+   '../../common/eal_common_thread.c',
+   '../../common/rte_option.c',
 )
 env_sources = files('eal.c',
'eal_debug.c',
-- 
2.18.0.windows.1

[dpdk-dev] [Bug 392] l3fwd fails to run with eventdev

2020-02-06 Thread bugzilla

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

Bug ID: 392
   Summary: l3fwd fails to run with eventdev
   Product: DPDK
   Version: 20.02
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: examples
  Assignee: dev@dpdk.org
  Reporter: vipin.vargh...@intel.com
  Target Milestone: ---

DPDK version: v20.02-rc2
example: l3fwd
Documentation: doc/guides/sample_app_ug/l3_forward.rst
cmd: `./build/l3fwd -l 4-7 -s 0-3 -n 4 --vdev event_sw0 -- -p 0x3
--mode=eventdev --eventq-sched=ordered`

Expected Result: Application should be running as per the documenation

Error:
```
EAL: Detected 88 lcore(s)
EAL: Detected 2 NUMA nodes
EAL: invalid service coremask

Usage: ./build/l3fwd [options]
```

Reason: the service takes HEX value mask. `-s SERVICE COREMASK Hexadecimal
bitmask of cores to be used as service cores`

Working condition: ```./build/l3fwd -l 0-7 -s 0xf -n 4 --vdev event_sw0 --
-p 0x3 --mode=eventdev --eventq-sched=ordered```

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

Re: [dpdk-dev] [PATCH v7 9/9] eal: add minimum viable code to support parsing