[dpdk-dev] [PATCH] app/testpmd: fix failure of creating E-Tag and NVGRE flows

[Beilei Xing]

Application fails to create NVGRE and E_Tag flows with
current configuration, this commit fixes the issue by
adding flow items for E_TAG and NVGRE.

Fixes: e4840ef2685d ("ethdev: fix incomplete items in flow API")
Cc: sta...@dpdk.org

Signed-off-by: Beilei Xing 
---
 app/test-pmd/config.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4d873cd..83a8f52 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -968,6 +968,8 @@ static const struct {
MK_FLOW_ITEM(TCP, sizeof(struct rte_flow_item_tcp)),
MK_FLOW_ITEM(SCTP, sizeof(struct rte_flow_item_sctp)),
MK_FLOW_ITEM(VXLAN, sizeof(struct rte_flow_item_vxlan)),
+   MK_FLOW_ITEM(E_TAG, sizeof(struct rte_flow_item_e_tag)),
+   MK_FLOW_ITEM(NVGRE, sizeof(struct rte_flow_item_nvgre)),
MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)),
MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
 };
-- 
2.5.5

Re: [dpdk-dev] [PATCH v4 3/4] net: add arm64 neon version of CRC compute APIs

[Sekhar, Ashwin]

On Fri, 2017-05-12 at 13:51 +0800, Jianbo Liu wrote:
> On 9 May 2017 at 17:53, Ashwin Sekhar T K
>  wrote:
> > 
> > Added CRC compute APIs for arm64 utilizing the pmull
> > capability
> > 
> > Added new file net_crc_neon.h to hold the arm64 pmull
> > CRC implementation
> > 
> > Verified the changes with crc_autotest unit test case
> > 
> > Signed-off-by: Ashwin Sekhar T K 
> > ---
> > v2:
> > * Fixed merge conflict in MAINTAINERS
> > 
> > v3:
> > * Moved feature detection changes and GCC_VERSION definition
> >   changes to separate commit
> > * Replaced usage of assert() with RTE_ASSERT()
> > * Made the comments in rte_vect.h more positive in sense
> > 
> > v4:
> > * Rebased on top of latest commit
> > 
> >  MAINTAINERS   |   1 +
> >  lib/librte_eal/common/include/arch/arm/rte_vect.h |  28 ++
> >  lib/librte_net/net_crc_neon.h | 357
> > ++
> >  lib/librte_net/rte_net_crc.c  |  34 ++-
> >  lib/librte_net/rte_net_crc.h  |   2 +
> >  5 files changed, 416 insertions(+), 6 deletions(-)
> >  create mode 100644 lib/librte_net/net_crc_neon.h
> > 
> > 
...
> > +
> > +struct crc_pmull_ctx crc32_eth_pmull __rte_aligned(16);
> > +struct crc_pmull_ctx crc16_ccitt_pmull __rte_aligned(16);
> > +
> > +static inline uint8x16_t
> > +extract_vector(uint8x16_t v0, uint8x16_t v1, const int n)
> > +{
> > +   switch (n) {
> > +   case 0: return vextq_u8(v0, v1, 0);
> > +   case 1: return vextq_u8(v0, v1, 1);
> > +   case 2: return vextq_u8(v0, v1, 2);
> > +   case 3: return vextq_u8(v0, v1, 3);
> > +   case 4: return vextq_u8(v0, v1, 4);
> > +   case 5: return vextq_u8(v0, v1, 5);
> > +   case 6: return vextq_u8(v0, v1, 6);
> > +   case 7: return vextq_u8(v0, v1, 7);
> > +   case 8: return vextq_u8(v0, v1, 8);
> > +   case 9: return vextq_u8(v0, v1, 9);
> > +   case 10: return vextq_u8(v0, v1, 10);
> > +   case 11: return vextq_u8(v0, v1, 11);
> > +   case 12: return vextq_u8(v0, v1, 12);
> > +   case 13: return vextq_u8(v0, v1, 13);
> > +   case 14: return vextq_u8(v0, v1, 14);
> > +   case 15: return vextq_u8(v0, v1, 15);
> > +   }
> > +   return v1;
> > +}
> > +
> > +/**
> > + * Shifts right 128 bit register by specified number of bytes
> > + *
> > + * @param reg 128 bit value
> > + * @param num number of bytes to shift reg by (0-16)
> > + *
> > + * @return reg << (num * 8)
> > + */
> > +static inline uint64x2_t
> > +shift_bytes_right(uint64x2_t reg, const unsigned int num)
> > +{
> > +   /* Right Shift */
> > +   return vreinterpretq_u64_u8(extract_vector(
> > +   vreinterpretq_u8_u64(reg),
> > +   vdupq_n_u8(0),
> > +   num));
> > +}
> > +
> > +/**
> > + * Shifts left 128 bit register by specified number of bytes
> > + *
> > + * @param reg 128 bit value
> > + * @param num number of bytes to shift reg by (0-16)
> > + *
> > + * @return reg << (num * 8)
> > + */
> > +static inline uint64x2_t
> > +shift_bytes_left(uint64x2_t reg, const unsigned int num)
> > +{
> > +   /* Left Shift */
> > +   return vreinterpretq_u64_u8(extract_vector(
> > +   vdupq_n_u8(0),
> > +   vreinterpretq_u8_u64(reg),
> > +   16 - num));
> > +}
> > +
> Can you move shift_bytes_right/shift_bytes_left to rte_vect.h because
> they are common functions?
These are not really common functions. I dont think it will have a
wider usage as its shifting by bytes and not by bits.

In x86 case also, xmm_shift_left is not made a common function.

Moreover, I have not tested the behaviour of these functions when the
shift amt is (< 0) or (> 16) as these cases will never arise in the CRC
code.

Thanks
Ashwin

Re: [dpdk-dev] [PATCH] app/testpmd: fix failure of creating E-Tag and NVGRE flows

[Adrien Mazarguil]

On Fri, May 12, 2017 at 03:13:09PM +0800, Beilei Xing wrote:
> Application fails to create NVGRE and E_Tag flows with
> current configuration, this commit fixes the issue by
> adding flow items for E_TAG and NVGRE.
> 
> Fixes: e4840ef2685d ("ethdev: fix incomplete items in flow API")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Beilei Xing 

Missed that obvious issue as I could only validate the command-line parsing
side of things for these items. Thanks.

Acked-by: Adrien Mazarguil 

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] "Port 0 is not present on the board" when building DPDK libraries as shared

[Bruce Richardson]

On Thu, May 11, 2017 at 09:33:32PM +, Mastorakis, Spyridon wrote:
> Hi guys,
> 
> When I compile the DPDK libraries as shared (by modifying the base_config 
> file) and try to run an application (e.g., l3fwd), I get an error that port 0 
> is not present on board.
> 
> I have cleaned the system several times (unbinded NICs, removed igb_uio and 
> uio modules and hugepage mappings) and have configured everything again, but 
> this does not resolve the issue. I am also not able to add a virtual device 
> (specifically a virtual ring) when I compile as a shared library (no PMD 
> drivers found).
> 
> When I compile the DPDK libraries as static though, everything works 
> perfectly fine and I can run the DPDK applications and add virtual rings.
> 
> I have tried dpdk-17.02 and the latest stable dpdk version (16.11.1), but 
> none of them seems to work.
> 
> Do you have any clue why this is happening?
> 
> Thank you in advance,
> Spyros

When compiled as a shared lib, are you passing in the "-d" parameters to
make sure that the PMDs are actually loaded by the app. Unlike the
static version, they are not automatically linked in the shared lib
build.

/Bruce

Re: [dpdk-dev] [PATCH] memzone: Check socket_id value when creating memzone.

[Bruce Richardson]

On Thu, May 11, 2017 at 11:03:43PM -0700, Tonghao Zhang wrote:
> If the socket_id is invalid (e.g. -2, -3), the
> memzone_reserve_aligned_thread_unsafe should return the
> EINVAL and not ENOMEM. To avoid it, we should check the
> socket_id before calling malloc_heap_alloc.
> 
> Signed-off-by: Tonghao Zhang 
> ---
>  lib/librte_eal/common/eal_common_memzone.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_memzone.c 
> b/lib/librte_eal/common/eal_common_memzone.c
> index 64f4e0a..3026e36 100644
> --- a/lib/librte_eal/common/eal_common_memzone.c
> +++ b/lib/librte_eal/common/eal_common_memzone.c
> @@ -189,7 +189,8 @@
>   return NULL;
>   }
>  
> - if ((socket_id != SOCKET_ID_ANY) && (socket_id >= RTE_MAX_NUMA_NODES)) {
> + if ((socket_id != SOCKET_ID_ANY) &&
> + (socket_id >= RTE_MAX_NUMA_NODES || socket_id < 0)) {
>   rte_errno = EINVAL;
>   return NULL;
>   }
> -- 

Looks a sensible thing to do.

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH v4 3/4] net: add arm64 neon version of CRC compute APIs

[Jianbo Liu]

On 12 May 2017 at 15:25, Sekhar, Ashwin  wrote:
> On Fri, 2017-05-12 at 13:51 +0800, Jianbo Liu wrote:
>> On 9 May 2017 at 17:53, Ashwin Sekhar T K
>>  wrote:
>> >
>> > Added CRC compute APIs for arm64 utilizing the pmull
>> > capability
>> >
>> > Added new file net_crc_neon.h to hold the arm64 pmull
>> > CRC implementation
>> >
>> > Verified the changes with crc_autotest unit test case
>> >
>> > Signed-off-by: Ashwin Sekhar T K 
>> > ---
>> > v2:
>> > * Fixed merge conflict in MAINTAINERS
>> >
>> > v3:
>> > * Moved feature detection changes and GCC_VERSION definition
>> >   changes to separate commit
>> > * Replaced usage of assert() with RTE_ASSERT()
>> > * Made the comments in rte_vect.h more positive in sense
>> >
>> > v4:
>> > * Rebased on top of latest commit
>> >
>> >  MAINTAINERS   |   1 +
>> >  lib/librte_eal/common/include/arch/arm/rte_vect.h |  28 ++
>> >  lib/librte_net/net_crc_neon.h | 357
>> > ++
>> >  lib/librte_net/rte_net_crc.c  |  34 ++-
>> >  lib/librte_net/rte_net_crc.h  |   2 +
>> >  5 files changed, 416 insertions(+), 6 deletions(-)
>> >  create mode 100644 lib/librte_net/net_crc_neon.h
>> >
>> >
> ...
>> > +
>> > +struct crc_pmull_ctx crc32_eth_pmull __rte_aligned(16);
>> > +struct crc_pmull_ctx crc16_ccitt_pmull __rte_aligned(16);
>> > +
>> > +static inline uint8x16_t
>> > +extract_vector(uint8x16_t v0, uint8x16_t v1, const int n)
>> > +{
>> > +   switch (n) {
>> > +   case 0: return vextq_u8(v0, v1, 0);
>> > +   case 1: return vextq_u8(v0, v1, 1);
>> > +   case 2: return vextq_u8(v0, v1, 2);
>> > +   case 3: return vextq_u8(v0, v1, 3);
>> > +   case 4: return vextq_u8(v0, v1, 4);
>> > +   case 5: return vextq_u8(v0, v1, 5);
>> > +   case 6: return vextq_u8(v0, v1, 6);
>> > +   case 7: return vextq_u8(v0, v1, 7);
>> > +   case 8: return vextq_u8(v0, v1, 8);
>> > +   case 9: return vextq_u8(v0, v1, 9);
>> > +   case 10: return vextq_u8(v0, v1, 10);
>> > +   case 11: return vextq_u8(v0, v1, 11);
>> > +   case 12: return vextq_u8(v0, v1, 12);
>> > +   case 13: return vextq_u8(v0, v1, 13);
>> > +   case 14: return vextq_u8(v0, v1, 14);
>> > +   case 15: return vextq_u8(v0, v1, 15);
>> > +   }
>> > +   return v1;
>> > +}
>> > +
>> > +/**
>> > + * Shifts right 128 bit register by specified number of bytes
>> > + *
>> > + * @param reg 128 bit value
>> > + * @param num number of bytes to shift reg by (0-16)
>> > + *
>> > + * @return reg << (num * 8)
>> > + */
>> > +static inline uint64x2_t
>> > +shift_bytes_right(uint64x2_t reg, const unsigned int num)
>> > +{
>> > +   /* Right Shift */
>> > +   return vreinterpretq_u64_u8(extract_vector(
>> > +   vreinterpretq_u8_u64(reg),
>> > +   vdupq_n_u8(0),
>> > +   num));
>> > +}
>> > +
>> > +/**
>> > + * Shifts left 128 bit register by specified number of bytes
>> > + *
>> > + * @param reg 128 bit value
>> > + * @param num number of bytes to shift reg by (0-16)
>> > + *
>> > + * @return reg << (num * 8)
>> > + */
>> > +static inline uint64x2_t
>> > +shift_bytes_left(uint64x2_t reg, const unsigned int num)
>> > +{
>> > +   /* Left Shift */
>> > +   return vreinterpretq_u64_u8(extract_vector(
>> > +   vdupq_n_u8(0),
>> > +   vreinterpretq_u8_u64(reg),
>> > +   16 - num));
>> > +}
>> > +
>> Can you move shift_bytes_right/shift_bytes_left to rte_vect.h because
>> they are common functions?
> These are not really common functions. I dont think it will have a
> wider usage as its shifting by bytes and not by bits.
>

I think these shifting may be used by other functions.
For example, to replace  _mm_srli_si128.

> In x86 case also, xmm_shift_left is not made a common function.
>

But its counterpart right shifting (_mm_srli_si128) is...

> Moreover, I have not tested the behaviour of these functions when the
> shift amt is (< 0) or (> 16) as these cases will never arise in the CRC
> code.
>

You can define thee functions according to current requirement.
And I don't think this parameter can be <0 or > 16.

Re: [dpdk-dev] [PATCH v4 3/4] net: add arm64 neon version of CRC compute APIs

[Sekhar, Ashwin]

On Fri, 2017-05-12 at 16:49 +0800, Jianbo Liu wrote:
> On 12 May 2017 at 15:25, Sekhar, Ashwin 
> wrote:
> > 
> > On Fri, 2017-05-12 at 13:51 +0800, Jianbo Liu wrote:
> > > 
> > > On 9 May 2017 at 17:53, Ashwin Sekhar T K
> > >  wrote:
> > > > 
> > > > 
> > > > Added CRC compute APIs for arm64 utilizing the pmull
> > > > capability
> > > > 
> > > > Added new file net_crc_neon.h to hold the arm64 pmull
> > > > CRC implementation
> > > > 
> > > > Verified the changes with crc_autotest unit test case
> > > > 
> > > > Signed-off-by: Ashwin Sekhar T K  > > > com>
> > > > ---
> > > > v2:
> > > > * Fixed merge conflict in MAINTAINERS
> > > > 
> > > > v3:
> > > > * Moved feature detection changes and GCC_VERSION definition
> > > >   changes to separate commit
> > > > * Replaced usage of assert() with RTE_ASSERT()
> > > > * Made the comments in rte_vect.h more positive in sense
> > > > 
> > > > v4:
> > > > * Rebased on top of latest commit
> > > > 
> > > >  MAINTAINERS   |   1 +
> > > >  lib/librte_eal/common/include/arch/arm/rte_vect.h |  28 ++
> > > >  lib/librte_net/net_crc_neon.h | 357
> > > > ++
> > > >  lib/librte_net/rte_net_crc.c  |  34 ++-
> > > >  lib/librte_net/rte_net_crc.h  |   2 +
> > > >  5 files changed, 416 insertions(+), 6 deletions(-)
> > > >  create mode 100644 lib/librte_net/net_crc_neon.h
> > > > 
> > > > 
> > ...
> > > 
> > > > 
> > > > +
> > > > +struct crc_pmull_ctx crc32_eth_pmull __rte_aligned(16);
> > > > +struct crc_pmull_ctx crc16_ccitt_pmull __rte_aligned(16);
> > > > +
> > > > +static inline uint8x16_t
> > > > +extract_vector(uint8x16_t v0, uint8x16_t v1, const int n)
> > > > +{
> > > > +   switch (n) {
> > > > +   case 0: return vextq_u8(v0, v1, 0);
> > > > +   case 1: return vextq_u8(v0, v1, 1);
> > > > +   case 2: return vextq_u8(v0, v1, 2);
> > > > +   case 3: return vextq_u8(v0, v1, 3);
> > > > +   case 4: return vextq_u8(v0, v1, 4);
> > > > +   case 5: return vextq_u8(v0, v1, 5);
> > > > +   case 6: return vextq_u8(v0, v1, 6);
> > > > +   case 7: return vextq_u8(v0, v1, 7);
> > > > +   case 8: return vextq_u8(v0, v1, 8);
> > > > +   case 9: return vextq_u8(v0, v1, 9);
> > > > +   case 10: return vextq_u8(v0, v1, 10);
> > > > +   case 11: return vextq_u8(v0, v1, 11);
> > > > +   case 12: return vextq_u8(v0, v1, 12);
> > > > +   case 13: return vextq_u8(v0, v1, 13);
> > > > +   case 14: return vextq_u8(v0, v1, 14);
> > > > +   case 15: return vextq_u8(v0, v1, 15);
> > > > +   }
> > > > +   return v1;
> > > > +}
> > > > +
> > > > +/**
> > > > + * Shifts right 128 bit register by specified number of bytes
> > > > + *
> > > > + * @param reg 128 bit value
> > > > + * @param num number of bytes to shift reg by (0-16)
> > > > + *
> > > > + * @return reg << (num * 8)
> > > > + */
> > > > +static inline uint64x2_t
> > > > +shift_bytes_right(uint64x2_t reg, const unsigned int num)
> > > > +{
> > > > +   /* Right Shift */
> > > > +   return vreinterpretq_u64_u8(extract_vector(
> > > > +   vreinterpretq_u8_u64(reg),
> > > > +   vdupq_n_u8(0),
> > > > +   num));
> > > > +}
> > > > +
> > > > +/**
> > > > + * Shifts left 128 bit register by specified number of bytes
> > > > + *
> > > > + * @param reg 128 bit value
> > > > + * @param num number of bytes to shift reg by (0-16)
> > > > + *
> > > > + * @return reg << (num * 8)
> > > > + */
> > > > +static inline uint64x2_t
> > > > +shift_bytes_left(uint64x2_t reg, const unsigned int num)
> > > > +{
> > > > +   /* Left Shift */
> > > > +   return vreinterpretq_u64_u8(extract_vector(
> > > > +   vdupq_n_u8(0),
> > > > +   vreinterpretq_u8_u64(reg),
> > > > +   16 - num));
> > > > +}
> > > > +
> > > Can you move shift_bytes_right/shift_bytes_left to rte_vect.h
> > > because
> > > they are common functions?
> > These are not really common functions. I dont think it will have a
> > wider usage as its shifting by bytes and not by bits.
> > 
> I think these shifting may be used by other functions.
> For example, to replace  _mm_srli_si128.
> 
> > 
> > In x86 case also, xmm_shift_left is not made a common function.
> > 
> But its counterpart right shifting (_mm_srli_si128) is...
> 
> > 
> > Moreover, I have not tested the behaviour of these functions when
> > the
> > shift amt is (< 0) or (> 16) as these cases will never arise in the
> > CRC
> > code.
> > 
> You can define thee functions according to current requirement.
> And I don't think this parameter can be <0 or > 16.

Okay. In that case, I will move it to rte_vect.h.

Ashwin

Re: [dpdk-dev] [PATCH v1] doc: change doc line length limit in contributors guide

[Mcnamara, John]

> -Original Message-
> From: Iremonger, Bernard
> Sent: Thursday, May 11, 2017 6:31 PM
> To: Thomas Monjalon ; Mcnamara, John
> 
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v1] doc: change doc line length limit in
> contributors guide
> 
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Thursday, May 11, 2017 6:18 PM
> > To: Mcnamara, John 
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1] doc: change doc line length limit
> > in contributors guide
> >
> > 11/05/2017 18:11, Mcnamara, John:
> > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > >
> > > > ...
> > > > > -* The recommended style for the DPDK documentation is to put
> > > > > sentences
> > > > on separate lines.
> > > > > -  This allows for easier reviewing of patches.
> > > > > -  Multiple sentences which are not separated by a blank line
> > > > > are joined
> > > > automatically into paragraphs, for example::
> > > > > +* Lines in sentences should be less than 80 characters and
> > > > > +wrapped at
> > > > > +  words. Multiple sentences which are not separated by a blank
> > > > > +line are joined
> > > > > +  automatically into paragraphs.
> > > >
> > > > Why not keep the recommendation of separating sentences?
> > >
> > > This isn't a recommendation. It is just pointing out that lines and
> > > sentences are joined into paragraphs. Maybe that is obvious and
> > > doesn't need to be stated.
> >
> > I'm talking about "The recommended style for the DPDK documentation is
> > to put sentences on separate lines."
> > I like this recommendation.
> 
> +1 for this recommendation
> 

The problem is that almost no-one follows this recommendation.

An 80 character margin is a simple rule that most programming
editors can enforce or handle automatically.

It is also what is recommended in OpenStack:


https://docs.openstack.org/contributor-guide/rst-conv/general-guidelines.html#lines-length

The kernel doc guidelines don't have a length rule but their docs
are wrapped at 80:

https://www.kernel.org/doc/html/latest/_sources/doc-guide/sphinx.rst.txt

The current DPDK "single sentence per line plus wrap at ~120 characters"
guideline is unusual, not supported by editors and, with rare exceptions, not
followed by anyone.

As such I think the guidelines should reflect how people actually
write docs and submit patches, which is wrapping at 80 characters.

John

Re: [dpdk-dev] [PATCH v5 19/26] app/testpmd: add item raw to flow command

[Adrien Mazarguil]

Hi Wei,

On Thu, May 11, 2017 at 06:53:52AM +, Zhao1, Wei wrote:
> Hi, Adrien
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Wednesday, December 21, 2016 10:52 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v5 19/26] app/testpmd: add item raw to flow
> > command
> > 
> > Matches arbitrary byte strings with properties:
> > 
> > - relative: look for pattern after the previous item.
> > - search: search pattern from offset (see also limit).
> > - offset: absolute or relative offset for pattern.
> > - limit: search area limit for start of pattern.
> > - length: pattern length.
> > - pattern: byte string to look for.
> > 
> > Signed-off-by: Adrien Mazarguil 
> > Acked-by: Olga Shern 
[...]
> #define  ITEM_RAW_PATTERN_SIZE 36
> 
> The size of NIC i350 flex byte filter can accommodate the max length size of 
> 128 byte, and the reason to 
> Define it as 36 is ?If it is the max length of pattern, maybe 128  is more 
> appropriate? 
> Maybe I have not understand your purpose.
> 
> Thank you.

It's more or less an arbitrary compromise due to various limitations.

Once parsed, the result of an entire command is stored in a fixed buffer of
size CMDLINE_PARSE_RESULT_BUFSIZE (8192). Each parsed token ends up
somewhere in that buffer.

Each flow item always consumes sizeof(struct rte_flow_item) + sizeof(struct
rte_flow_item_xxx) * 3 (spec, last and mask) + alignment constraints.

For the raw item, this makes at least:

 (sizeof(rte_flow_item) +
  (sizeof(rte_flow_item_raw) + ITEM_RAW_PATTERN_SIZE) * 3)
 /* (32 + (12 + 36) * 3) => 176 bytes */

Because space is always consumed regardless of the size of the byte string
to match for implementation reasons, there is a chance to fill the buffer
too quickly with a larger ITEM_RAW_PATTERN_SIZE.

Also, this does not prevent users from specifying larger raw patterns (even
larger than 128) by combining them, e.g.:

 flow create 0
pattern eth / raw relative is 1 pattern is foobar /
   raw relative is 1 pattern is barbaz / end
actions queue index 42 / end

Such a pattern ends up matching a single "foobarbarbaz" string.

To summarize, it is only due to testpmd limitations. Even without PMD
support for combination, the current ability to provide 36 bytes of raw data
to match per specified item is plenty to validate basic functionality. We'll
improve testpmd eventually.

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH v1] doc: change doc line length limit in contributors guide

[Thomas Monjalon]

12/05/2017 11:10, Mcnamara, John:
> From: Iremonger, Bernard
> > From: Thomas Monjalon
> > > 11/05/2017 18:11, Mcnamara, John:
> > > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > > >
> > > > > ...
> > > > > > -* The recommended style for the DPDK documentation is to put
> > > > > > sentences
> > > > > on separate lines.
> > > > > > -  This allows for easier reviewing of patches.
> > > > > > -  Multiple sentences which are not separated by a blank line
> > > > > > are joined
> > > > > automatically into paragraphs, for example::
> > > > > > +* Lines in sentences should be less than 80 characters and
> > > > > > +wrapped at
> > > > > > +  words. Multiple sentences which are not separated by a blank
> > > > > > +line are joined
> > > > > > +  automatically into paragraphs.
> > > > >
> > > > > Why not keep the recommendation of separating sentences?
> > > >
> > > > This isn't a recommendation. It is just pointing out that lines and
> > > > sentences are joined into paragraphs. Maybe that is obvious and
> > > > doesn't need to be stated.
> > >
> > > I'm talking about "The recommended style for the DPDK documentation is
> > > to put sentences on separate lines."
> > > I like this recommendation.
> > 
> > +1 for this recommendation
> > 
> 
> The problem is that almost no-one follows this recommendation.
> 
> An 80 character margin is a simple rule that most programming
> editors can enforce or handle automatically.
> 
> It is also what is recommended in OpenStack:
> 
> 
> https://docs.openstack.org/contributor-guide/rst-conv/general-guidelines.html#lines-length
> 
> The kernel doc guidelines don't have a length rule but their docs
> are wrapped at 80:
> 
> https://www.kernel.org/doc/html/latest/_sources/doc-guide/sphinx.rst.txt
> 
> The current DPDK "single sentence per line plus wrap at ~120 characters"
> guideline is unusual, not supported by editors and, with rare exceptions, not
> followed by anyone.
> 
> As such I think the guidelines should reflect how people actually
> write docs and submit patches, which is wrapping at 80 characters.

I am OK with 80 characters.
However, I think we should keep trying to explain that it is better
to wrap at the end of a sentence.

Example:
This long sentence with a lot of words which does not mean anything will wrap
at 80 characters and continue on the second line. Then a new sentence starts
and ends on the third line.

It would be better like that:
This long sentence with a lot of words which does not mean anything will wrap
at 80 characters and continue on the second line.
Then a new sentence starts and ends on the third line.

[dpdk-dev] [PATCH v5 0/4] add arm64 neon version of CRC compute APIs

[Ashwin Sekhar T K]

This patch series adds arm64 neon version of CRC compute APIs utilizing
the pmull capability (which is available as part of crypto extensions).

 * Patch 1 adds crypto capability in compilation of generic armv8a
   and thunderx targets.
 * Patch 2 moves GCC_VERSION defintion to a more common location as
   it will be used in the Patch 3.
 * Patch 3 adds the arm64 neon implementation of the CRC compute APIs.
 * Patch 4 adds the test case for testing arm64 neon implementation of the
   CRC compute APIs.

v5:
* Moved APIs shift_bytes_left, shift_bytes_right and extract_vector from
  net_crc_neon.h to rte_vect.h and renamed them to vshift_bytes_left,
  vshift_bytes_right and vextract respectively.

v4:
* Rebased on top of latest commit
* Edited the Patch 2 commit message body according to comments
* Moved definition and usage of GCC_VERSION under RTE_TOOLCHAIN_GCC flag

v3:
* Moved feature detection changes and GCC_VERSION definition changes
  to separate commits.
* Replaced usage of assert() with RTE_ASSERT()
* Made the comments in rte_vect.h more positive in sense
* Moved GCC_VERSION definition to common header and removed the same from
  rte_lru.h

v2:
* Fixed merge conflict in MAINTAINERS
* Fixed checkpatch errors/warnings

Ashwin Sekhar T K (4):
  mk: add crypto capability for generic armv8a and thunderx
  eal: move gcc version definition to common header
  net: add arm64 neon version of CRC compute APIs
  test: add tests for arm64 CRC neon versions

 MAINTAINERS   |   1 +
 lib/librte_eal/common/include/arch/arm/rte_vect.h |  88 +++
 lib/librte_eal/common/include/rte_common.h|   6 +
 lib/librte_net/net_crc_neon.h | 297 ++
 lib/librte_net/rte_net_crc.c  |  34 ++-
 lib/librte_net/rte_net_crc.h  |   2 +
 lib/librte_table/rte_lru.h|  10 +-
 mk/machine/armv8a/rte.vars.mk |   2 +-
 mk/machine/thunderx/rte.vars.mk   |   2 +-
 mk/rte.cpuflags.mk|   6 +
 mk/toolchain/gcc/rte.toolchain-compat.mk  |   1 +
 test/test/test_crc.c  |   9 +
 12 files changed, 442 insertions(+), 16 deletions(-)
 create mode 100644 lib/librte_net/net_crc_neon.h

-- 
2.12.2

[dpdk-dev] [PATCH v5 1/4] mk: add crypto capability for generic armv8a and thunderx

[Ashwin Sekhar T K]

armv8-a has optional CRYPTO extension which adds the
AES, PMULL, SHA1 and SHA2 capabilities. -march=armv8-a+crypto
enables code generation for the ARMv8-A architecture together
with the optional CRYPTO extensions.

Added the following flags to detect the corresponding
capability at compile time.
 * RTE_MACHINE_CPUFLAG_AES
 * RTE_MACHINE_CPUFLAG_PMULL
 * RTE_MACHINE_CPUFLAG_SHA1
 * RTE_MACHINE_CPUFLAG_SHA2

At run-time, the following flags can be used to detect the
capabilities.
 * RTE_CPUFLAG_AES
 * RTE_CPUFLAG_PMULL
 * RTE_CPUFLAG_SHA1
 * RTE_CPUFLAG_SHA2

Signed-off-by: Ashwin Sekhar T K 
Reviewed-by: Jan Viktorin 
---
 mk/machine/armv8a/rte.vars.mk| 2 +-
 mk/machine/thunderx/rte.vars.mk  | 2 +-
 mk/rte.cpuflags.mk   | 6 ++
 mk/toolchain/gcc/rte.toolchain-compat.mk | 1 +
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk
index d5049e1f1..51966a5b6 100644
--- a/mk/machine/armv8a/rte.vars.mk
+++ b/mk/machine/armv8a/rte.vars.mk
@@ -55,4 +55,4 @@
 # CPU_LDFLAGS =
 # CPU_ASFLAGS =
 
-MACHINE_CFLAGS += -march=armv8-a+crc
+MACHINE_CFLAGS += -march=armv8-a+crc+crypto
diff --git a/mk/machine/thunderx/rte.vars.mk b/mk/machine/thunderx/rte.vars.mk
index ad5a379b0..678410581 100644
--- a/mk/machine/thunderx/rte.vars.mk
+++ b/mk/machine/thunderx/rte.vars.mk
@@ -55,4 +55,4 @@
 # CPU_LDFLAGS =
 # CPU_ASFLAGS =
 
-MACHINE_CFLAGS += -march=armv8-a+crc -mcpu=thunderx
+MACHINE_CFLAGS += -march=armv8-a+crc+crypto -mcpu=thunderx
diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
index 4288c1470..a813c91f4 100644
--- a/mk/rte.cpuflags.mk
+++ b/mk/rte.cpuflags.mk
@@ -125,6 +125,12 @@ ifneq ($(filter $(AUTO_CPUFLAGS),__ARM_FEATURE_CRC32),)
 CPUFLAGS += CRC32
 endif
 
+ifneq ($(filter $(AUTO_CPUFLAGS),__ARM_FEATURE_CRYPTO),)
+CPUFLAGS += AES
+CPUFLAGS += PMULL
+CPUFLAGS += SHA1
+CPUFLAGS += SHA2
+endif
 
 MACHINE_CFLAGS += $(addprefix -DRTE_MACHINE_CPUFLAG_,$(CPUFLAGS))
 
diff --git a/mk/toolchain/gcc/rte.toolchain-compat.mk 
b/mk/toolchain/gcc/rte.toolchain-compat.mk
index 280dde2a6..01ac7e232 100644
--- a/mk/toolchain/gcc/rte.toolchain-compat.mk
+++ b/mk/toolchain/gcc/rte.toolchain-compat.mk
@@ -60,6 +60,7 @@ else
 #
ifeq ($(shell test $(GCC_VERSION) -le 49 && echo 1), 1)
MACHINE_CFLAGS := $(patsubst 
-march=armv8-a+crc,-march=armv8-a+crc -D__ARM_FEATURE_CRC32=1,$(MACHINE_CFLAGS))
+   MACHINE_CFLAGS := $(patsubst 
-march=armv8-a+crc+crypto,-march=armv8-a+crc+crypto 
-D__ARM_FEATURE_CRC32=1,$(MACHINE_CFLAGS))
endif
ifeq ($(shell test $(GCC_VERSION) -le 47 && echo 1), 1)
MACHINE_CFLAGS := $(patsubst 
-march=core-avx-i,-march=corei7-avx,$(MACHINE_CFLAGS))
-- 
2.12.2

[dpdk-dev] [PATCH v5 2/4] eal: move gcc version definition to common header

[Ashwin Sekhar T K]

Moved the definition of GCC_VERSION from lib/librte_table/rte_lru.h
to lib/librte_eal/common/include/rte_common.h.

Tested compilation on:
 * arm64 with gcc
 * x86 with gcc and clang

Signed-off-by: Ashwin Sekhar T K 
Reviewed-by: Jan Viktorin 
---
 lib/librte_eal/common/include/rte_common.h |  6 ++
 lib/librte_table/rte_lru.h | 10 ++
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_common.h 
b/lib/librte_eal/common/include/rte_common.h
index e057f6e21..ff4a12bbe 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -66,6 +66,12 @@ extern "C" {
 #define RTE_STD_C11
 #endif
 
+/** Define GCC_VERSION **/
+#ifdef RTE_TOOLCHAIN_GCC
+#define GCC_VERSION (__GNUC__ * 1 + __GNUC_MINOR__ * 100 + \
+   __GNUC_PATCHLEVEL__)
+#endif
+
 #ifdef RTE_ARCH_STRICT_ALIGN
 typedef uint64_t unaligned_uint64_t __attribute__ ((aligned(1)));
 typedef uint32_t unaligned_uint32_t __attribute__ ((aligned(1)));
diff --git a/lib/librte_table/rte_lru.h b/lib/librte_table/rte_lru.h
index e87e062d0..5cc596613 100644
--- a/lib/librte_table/rte_lru.h
+++ b/lib/librte_table/rte_lru.h
@@ -40,12 +40,6 @@ extern "C" {
 
 #include 
 
-#ifdef __INTEL_COMPILER
-#define GCC_VERSION (0)
-#else
-#define GCC_VERSION (__GNUC__ * 1+__GNUC_MINOR__*100 + __GNUC_PATCHLEVEL__)
-#endif
-
 #ifndef RTE_TABLE_HASH_LRU_STRATEGY
 #ifdef __SSE4_2__
 #define RTE_TABLE_HASH_LRU_STRATEGY2
@@ -120,7 +114,7 @@ do {
\
 
 #elif RTE_TABLE_HASH_LRU_STRATEGY == 2
 
-#if GCC_VERSION > 40306
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION > 40306)
 #include 
 #else
 #include 
@@ -166,7 +160,7 @@ do {
\
 
 #elif RTE_TABLE_HASH_LRU_STRATEGY == 3
 
-#if GCC_VERSION > 40306
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION > 40306)
 #include 
 #else
 #include 
-- 
2.12.2

[dpdk-dev] [PATCH v5 3/4] net: add arm64 neon version of CRC compute APIs

[Ashwin Sekhar T K]

Added CRC compute APIs for arm64 utilizing the pmull
capability.

Added new file net_crc_neon.h to hold the arm64 pmull
CRC implementation.

Added wrappers in rte_vect.h for those neon intrinsics
which are not supported in GCC version < 7.

Verified the changes with crc_autotest unit test case

Signed-off-by: Ashwin Sekhar T K 
---
 MAINTAINERS   |   1 +
 lib/librte_eal/common/include/arch/arm/rte_vect.h |  88 +++
 lib/librte_net/net_crc_neon.h | 297 ++
 lib/librte_net/rte_net_crc.c  |  34 ++-
 lib/librte_net/rte_net_crc.h  |   2 +
 5 files changed, 416 insertions(+), 6 deletions(-)
 create mode 100644 lib/librte_net/net_crc_neon.h

diff --git a/MAINTAINERS b/MAINTAINERS
index b6495d2b9..66d64c2c9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -147,6 +147,7 @@ F: lib/librte_eal/common/include/arch/arm/*_64.h
 F: lib/librte_acl/acl_run_neon.*
 F: lib/librte_lpm/rte_lpm_neon.h
 F: lib/librte_hash/rte*_arm64.h
+F: lib/librte_net/net_crc_neon.h
 F: drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
 F: drivers/net/i40e/i40e_rxtx_vec_neon.c
 F: drivers/net/virtio/virtio_rxtx_simple_neon.c
diff --git a/lib/librte_eal/common/include/arch/arm/rte_vect.h 
b/lib/librte_eal/common/include/arch/arm/rte_vect.h
index 4107c9988..55e228a77 100644
--- a/lib/librte_eal/common/include/arch/arm/rte_vect.h
+++ b/lib/librte_eal/common/include/arch/arm/rte_vect.h
@@ -35,6 +35,7 @@
 
 #include 
 #include "generic/rte_vect.h"
+#include "rte_debug.h"
 #include "arm_neon.h"
 
 #ifdef __cplusplus
@@ -78,6 +79,93 @@ vqtbl1q_u8(uint8x16_t a, uint8x16_t b)
 }
 #endif
 
+#if defined(RTE_ARCH_ARM64)
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 7)
+/* NEON intrinsic vreinterpretq_u64_p128() is supported since GCC version 7 */
+static inline uint64x2_t
+vreinterpretq_u64_p128(poly128_t x)
+{
+   return (uint64x2_t)x;
+}
+
+/* NEON intrinsic vreinterpretq_p64_u64() is supported since GCC version 7 */
+static inline poly64x2_t
+vreinterpretq_p64_u64(uint64x2_t x)
+{
+   return (poly64x2_t)x;
+}
+
+/* NEON intrinsic vgetq_lane_p64() is supported since GCC version 7 */
+static inline poly64_t
+vgetq_lane_p64(poly64x2_t x, const int lane)
+{
+   RTE_ASSERT(lane >= 0 && lane <= 1);
+
+   poly64_t *p = (poly64_t *)&x;
+
+   return p[lane];
+}
+#endif
+#endif
+
+/*
+ * If (0 <= index <= 15), then call the ASIMD ext intruction on the
+ * 128 bit regs v0 and v1 with the appropriate index.
+ *
+ * Else returns a zero vector.
+ */
+static inline uint8x16_t
+vextract(uint8x16_t v0, uint8x16_t v1, const int index)
+{
+   switch (index) {
+   case 0: return vextq_u8(v0, v1, 0);
+   case 1: return vextq_u8(v0, v1, 1);
+   case 2: return vextq_u8(v0, v1, 2);
+   case 3: return vextq_u8(v0, v1, 3);
+   case 4: return vextq_u8(v0, v1, 4);
+   case 5: return vextq_u8(v0, v1, 5);
+   case 6: return vextq_u8(v0, v1, 6);
+   case 7: return vextq_u8(v0, v1, 7);
+   case 8: return vextq_u8(v0, v1, 8);
+   case 9: return vextq_u8(v0, v1, 9);
+   case 10: return vextq_u8(v0, v1, 10);
+   case 11: return vextq_u8(v0, v1, 11);
+   case 12: return vextq_u8(v0, v1, 12);
+   case 13: return vextq_u8(v0, v1, 13);
+   case 14: return vextq_u8(v0, v1, 14);
+   case 15: return vextq_u8(v0, v1, 15);
+   }
+   return vdupq_n_u8(0);
+}
+
+/**
+ * Shifts right 128 bit register by specified number of bytes
+ *
+ * Value of shift parameter must be in range 0 - 16
+ */
+static inline uint64x2_t
+vshift_bytes_right(uint64x2_t reg, const unsigned int shift)
+{
+   return vreinterpretq_u64_u8(vextract(
+   vreinterpretq_u8_u64(reg),
+   vdupq_n_u8(0),
+   shift));
+}
+
+/**
+ * Shifts left 128 bit register by specified number of bytes
+ *
+ * Value of shift parameter must be in range 0 - 16
+ */
+static inline uint64x2_t
+vshift_bytes_left(uint64x2_t reg, const unsigned int shift)
+{
+   return vreinterpretq_u64_u8(vextract(
+   vdupq_n_u8(0),
+   vreinterpretq_u8_u64(reg),
+   16 - shift));
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_net/net_crc_neon.h b/lib/librte_net/net_crc_neon.h
new file mode 100644
index 0..2be579d6b
--- /dev/null
+++ b/lib/librte_net/net_crc_neon.h
@@ -0,0 +1,297 @@
+/*
+ *   BSD LICENSE
+ *
+ *   Copyright (C) Cavium networks Ltd. 2017.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of condi

[dpdk-dev] [PATCH v5 4/4] test: add tests for arm64 CRC neon versions

[Ashwin Sekhar T K]

Verified the changes with crc_autotest unit test case

Signed-off-by: Ashwin Sekhar T K 
---
 test/test/test_crc.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/test/test/test_crc.c b/test/test/test_crc.c
index cd5af69a2..9f2a17d49 100644
--- a/test/test/test_crc.c
+++ b/test/test/test_crc.c
@@ -178,6 +178,15 @@ test_crc(void)
return ret;
}
 
+   /* set CRC neon mode */
+   rte_net_crc_set_alg(RTE_NET_CRC_NEON);
+
+   ret = test_crc_calc();
+   if (ret < 0) {
+   printf("test crc (arm64 neon pmull): failed (%d)\n", ret);
+   return ret;
+   }
+
return 0;
 }
 
-- 
2.12.2

[dpdk-dev] [PATCH] vfio: fix array bounds check

[Alejandro Lucero]

Checking against VFIO_MAX_GROUPS goes beyond the maximum array
index which should be (VFIO_MAX_GROUPS - 1).

Fixes: 94c0776b1bad("support hotplug")
Coverity issue: 144555
Coverity issue: 144556
Coverity issue: 144557

Signed-off-by: Alejandro Lucero 
---
 lib/librte_eal/linuxapp/eal/eal_vfio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c 
b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index 5486dca..4816afc 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -189,7 +189,7 @@
int i;
 
i = get_vfio_group_idx(vfio_group_fd);
-   if (i < 0 || i > VFIO_MAX_GROUPS)
+   if (i < 0 || i > (VFIO_MAX_GROUPS - 1))
RTE_LOG(ERR, EAL, "  wrong vfio_group index (%d)\n", i);
else
vfio_cfg.vfio_groups[i].devices++;
@@ -201,7 +201,7 @@
int i;
 
i = get_vfio_group_idx(vfio_group_fd);
-   if (i < 0 || i > VFIO_MAX_GROUPS)
+   if (i < 0 || i > (VFIO_MAX_GROUPS - 1))
RTE_LOG(ERR, EAL, "  wrong vfio_group index (%d)\n", i);
else
vfio_cfg.vfio_groups[i].devices--;
@@ -213,7 +213,7 @@
int i;
 
i = get_vfio_group_idx(vfio_group_fd);
-   if (i < 0 || i > VFIO_MAX_GROUPS) {
+   if (i < 0 || i > (VFIO_MAX_GROUPS - 1)) {
RTE_LOG(ERR, EAL, "  wrong vfio_group index (%d)\n", i);
return -1;
}
-- 
1.9.1

[dpdk-dev] [PATCH] driver/net: remove unnecessary macro for unused variables

[Ferruh Yigit]

remove __rte_unused instances that are not required.

Signed-off-by: Ferruh Yigit 
---
 drivers/net/af_packet/rte_eth_af_packet.c  |  7 ++-
 drivers/net/avp/avp_ethdev.c   |  3 +--
 drivers/net/bnx2x/bnx2x_ethdev.c   |  4 ++--
 drivers/net/bnxt/bnxt_ethdev.c |  2 +-
 drivers/net/bnxt/bnxt_rxq.c|  2 +-
 drivers/net/bonding/rte_eth_bond_args.c|  2 +-
 drivers/net/bonding/rte_eth_bond_private.h | 14 +++---
 drivers/net/e1000/igb_ethdev.c |  2 +-
 drivers/net/ena/ena_ethdev.c   |  4 ++--
 drivers/net/enic/enic.h|  8 
 drivers/net/fm10k/fm10k_ethdev.c   |  2 +-
 drivers/net/i40e/i40e_ethdev.h |  3 +--
 drivers/net/i40e/i40e_ethdev_vf.c  |  9 -
 drivers/net/i40e/i40e_flow.c   |  2 +-
 drivers/net/i40e/i40e_pf.h |  2 +-
 drivers/net/i40e/i40e_rxtx_vec_sse.c   |  2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c   | 12 ++--
 drivers/net/ixgbe/ixgbe_flow.c |  2 +-
 drivers/net/ixgbe/ixgbe_pf.c   |  2 +-
 drivers/net/qede/qede_ethdev.c |  2 +-
 drivers/net/qede/qede_rxtx.h   |  6 +++---
 drivers/net/sfc/sfc_ev.c   |  3 +--
 drivers/net/tap/rte_eth_tap.c  |  2 +-
 drivers/net/virtio/virtio_ethdev.c | 12 ++--
 drivers/net/vmxnet3/vmxnet3_rxtx.c |  2 +-
 25 files changed, 56 insertions(+), 55 deletions(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c 
b/drivers/net/af_packet/rte_eth_af_packet.c
index 68de45c..a03966a 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -565,7 +565,12 @@ rte_pmd_init_internals(struct rte_vdev_device *dev,
int rc, tpver, discard;
int qsockfd = -1;
unsigned int i, q, rdsize;
-   int fanout_arg __rte_unused, bypass __rte_unused;
+#if defined(PACKET_FANOUT)
+   int fanout_arg;
+#endif
+#if defined(PACKET_QDISC_BYPASS)
+   int bypass;
+#endif
 
for (k_idx = 0; k_idx < kvlist->count; k_idx++) {
pair = &kvlist->pairs[k_idx];
diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
index fe6849f..b08dbaa 100644
--- a/drivers/net/avp/avp_ethdev.c
+++ b/drivers/net/avp/avp_ethdev.c
@@ -71,8 +71,7 @@ static void avp_dev_close(struct rte_eth_dev *dev);
 static void avp_dev_info_get(struct rte_eth_dev *dev,
 struct rte_eth_dev_info *dev_info);
 static void avp_vlan_offload_set(struct rte_eth_dev *dev, int mask);
-static int avp_dev_link_update(struct rte_eth_dev *dev,
-  __rte_unused int wait_to_complete);
+static int avp_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete);
 static void avp_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static void avp_dev_promiscuous_disable(struct rte_eth_dev *dev);
 
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index b79cfdb..90cbb6c 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -119,7 +119,7 @@ bnx2x_interrupt_action(struct rte_eth_dev *dev)
bnx2x_link_update(dev);
 }
 
-static __rte_unused void
+static void
 bnx2x_interrupt_handler(void *param)
 {
struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
@@ -439,7 +439,7 @@ bnx2x_dev_xstats_get(struct rte_eth_dev *dev, struct 
rte_eth_xstat *xstats,
 }
 
 static void
-bnx2x_dev_infos_get(struct rte_eth_dev *dev, __rte_unused struct 
rte_eth_dev_info *dev_info)
+bnx2x_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
struct bnx2x_softc *sc = dev->data->dev_private;
dev_info->pci_dev = RTE_DEV_TO_PCI(dev->device);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index bb87361..e659c57 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -918,7 +918,7 @@ static int bnxt_rss_hash_conf_get_op(struct rte_eth_dev 
*eth_dev,
 }
 
 static int bnxt_flow_ctrl_get_op(struct rte_eth_dev *dev,
-  struct rte_eth_fc_conf *fc_conf __rte_unused)
+  struct rte_eth_fc_conf *fc_conf)
 {
struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
struct rte_eth_link link_info;
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index cddf17d..7625fb1 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -213,7 +213,7 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
return rc;
 }
 
-static void bnxt_rx_queue_release_mbufs(struct bnxt_rx_queue *rxq __rte_unused)
+static void bnxt_rx_queue_release_mbufs(struct bnxt_rx_queue *rxq)
 {
struct bnxt_sw_rx_bd *sw_ring;
uint16_t i;
diff --git a/drivers/net/bonding/rte_eth_bond_args.c 
b/drivers/net/bonding/rte_eth_bond_args.c
index e3bdad9..3155fca 100644
--- a/drivers/net/bond

Re: [dpdk-dev] [PATCH] drivers/net: document missing speed capabilities feature

[Ferruh Yigit]

On 5/10/2017 2:51 PM, Thomas Monjalon wrote:
> 10/05/2017 15:10, Ferruh Yigit:
>> --- a/doc/guides/nics/features/bnx2x.ini
>> +++ b/doc/guides/nics/features/bnx2x.ini
>> @@ -4,6 +4,7 @@
>>  ; Refer to default.ini for the full list of available PMD features.
>>  ;
>>  [Features]
>> +Speed capabilities   = Y
> 
> We should validate this feature only if the driver advertise the
> right speeds for the device.

Hi Thomas,

Can you please clarify more, what is expected implementation in PMD?
And perhaps a good and a bad sample can be helpful.

Thanks,
ferruh


> Most of drivers advertise every possible speeds for the driver
> without considering device limitations.
> 

Re: [dpdk-dev] [PATCH] vfio: fix array bounds check

[Burakov, Anatoly]

> From: Alejandro Lucero [mailto:alejandro.luc...@netronome.com]
> Sent: Friday, May 12, 2017 11:18 AM
> To: dev@dpdk.org
> Cc: Burakov, Anatoly 
> Subject: [PATCH] vfio: fix array bounds check
> 
> Checking against VFIO_MAX_GROUPS goes beyond the maximum array
> index which should be (VFIO_MAX_GROUPS - 1).
> 
> Fixes: 94c0776b1bad("support hotplug")
> Coverity issue: 144555
> Coverity issue: 144556
> Coverity issue: 144557
> 
> Signed-off-by: Alejandro Lucero 


Acked-by: Anatoly  Burakov 

Re: [dpdk-dev] [PATCH] net/ark: fix for Coverity issues

[Ferruh Yigit]

On 5/11/2017 12:02 PM, John Miller wrote:
> Fixes: 9c7188a68d7b ("net/ark: provide API for hardware modules pktchkr and 
> pktgen")
> Coverity issue: 144513
> 
> Fixes: 727b3fe292bc ("net/ark: integrate PMD")
> Coverity issue: 144514
> 
> Fixes: 9c7188a68d7b ("net/ark: provide API for hardware modules pktchkr and 
> pktgen")
> Coverity issue: 144512
> 
> Fixes: 1131cbf0fb2b ("net/ark: stub PMD for Atomic Rules Arkville")
> Coverity issue: 144517

The convention is Coverity line first, Fixes line later.

> 
> Fixes: 727b3fe292bc ("net/ark: integrate PMD")
> Coverity issue: 144520

Hi John,

Thanks for fixing coverity issues.

Can you please split patch into a patchset with multiple patches,
grouped to same kind of fixes?

And instead of having "coverity fix" in patch title, can you please
describe what is really fixed, like "fix not null terminated buffer" or
"fix missing function return check" etc ...

Thanks,
ferruh

> Signed-off-by: John Miller 

<...>

> --- a/drivers/net/ark/ark_pktgen.c
> +++ b/drivers/net/ark/ark_pktgen.c
> @@ -354,7 +354,8 @@ struct OPTIONS {
>   o->v.INT = atoll(val);
>   break;
>   case OTSTRING:
> - strncpy(o->v.STR, val, ARK_MAX_STR_LEN);
> + strncpy(o->v.STR, val, ARK_MAX_STR_LEN - 1);
> + o->v.STR[ARK_MAX_STR_LEN - 1] = 0;

This also works, but you can prefer to switch snprintf(), which
guaranties the null termination.

>   break;
>   }
>   return 1;
> 

Re: [dpdk-dev] [PATCH 0/4] Rel 17.08: add compile time checks to vpmds

[Ferruh Yigit]

On 4/28/2017 5:21 PM, Bruce Richardson wrote:
> As previously discussed*, add some compile time checks to the vpmds to
> help sanity-check their dependencies on the mbuf layout.
> 
> * http://dpdk.org/ml/archives/dev/2017-April/064988.html
> 
> Bruce Richardson (4):
>   net/ixgbe: add compile-time checks to vector driver
>   net/ixgbe: enable ixgbe vector PMD for i686
>   net/i40e: add compile-time checks to vector driver
>   net/fm10k: add compile-time checks to vector driver

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

Re: [dpdk-dev] [PATCH] net/thunderx: add compile-time checks

[Ferruh Yigit]

On 5/1/2017 7:40 AM, Jerin Jacob wrote:
> The thunderx PMD is sensitive to the layout of the mbuf on
> the RX path. Add in some compile-time checks to make sure the mbuf layout
> assumptions are valid, and to provide hints to anyone changing the mbuf
> where things may need to be updated.
> 
> Signed-off-by: Jerin Jacob 

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

Re: [dpdk-dev] [PATCH] net/ixgbe: do not touch mbuf initialized fields

[Ferruh Yigit]

On 5/5/2017 1:57 AM, Lu, Wenzhuo wrote:
> Hi,
> 
>> -Original Message-
>> From: Yigit, Ferruh
>> Sent: Thursday, May 4, 2017 11:59 PM
>> To: Lu, Wenzhuo; Ananyev, Konstantin
>> Cc: dev@dpdk.org; Olivier Matz
>> Subject: [PATCH] net/ixgbe: do not touch mbuf initialized fields
>>
>> See: 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool")
>>
>> Signed-off-by: Ferruh Yigit 
> Acked-by: Wenzhuo Lu 

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

Re: [dpdk-dev] [PATCH] driver/net: remove unnecessary macro for unused variables

[Legacy, Allain]

> -Original Message-
> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
> Sent: Friday, May 12, 2017 6:33 AM
> To: John W. Linville; Legacy, Allain; Peters, Matt; Harish Patil; Rasesh Mody;
> Stephen Hurd; Ajit Khaparde; DOHERTY, DECLAN; LU, WENZHUO; Marcin
> Wojtas; Michal Krawczyk; Guy Tzalik; Evgeny Schemeilin; John Daley; Nelson
> Escobar; CHEN, JING; ZHANG, HELIN; WU, JINGJING; ANANYEV,
> KONSTANTIN; Andrew Rybchenko; Pascal Mazon; Yuanhan Liu; Maxime
> Coquelin; Shrikrishna Khare
> Cc: dev@dpdk.org; YIGIT, FERRUH
> Subject: [PATCH] driver/net: remove unnecessary macro for unused
> variables
> 
> remove __rte_unused instances that are not required.
> 
> Signed-off-by: Ferruh Yigit 
> ---
Acked-by:  Allain Legacy 

Re: [dpdk-dev] [PATCH] net/null: do not touch mbuf next or nb segs on Rx

[Ferruh Yigit]

On 5/9/2017 8:24 AM, Olivier Matz wrote:
> On Thu,  4 May 2017 16:43:58 +0100, Ferruh Yigit  
> wrote:
>> mbuf next and nb_segs fields already have the default values when get
>> from mempool, no need to update them in PMD.
>>
>> See: 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool")
>>
>> Signed-off-by: Ferruh Yigit 

> Reviewed-by: Olivier Matz 

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

Re: [dpdk-dev] [DPDK] net/i40e: add return value checks

[Ferruh Yigit]

On 5/11/2017 2:04 AM, Zhang, Helin wrote:
> 
> 
>> -Original Message-
>> From: Lipiec, Herakliusz
>> Sent: Tuesday, May 9, 2017 10:38 PM
>> To: Zhang, Helin; Wu, Jingjing
>> Cc: dev@dpdk.org
>> Subject: [DPDK] net/i40e: add return value checks
>>
>> Coverity issue: 1379362
>> Coverity issue: 1379365
>> Fixes: 71d35259ff67 ("i40e: tear down flow director")
>>
>> Signed-off-by: Herakliusz Lipiec 
> Acked-by: Helin Zhang 

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

Re: [dpdk-dev] [PATCH v2 7/7] net/enic: flow API documentation

[Ferruh Yigit]

On 3/31/2017 3:06 AM, John Daley wrote:
> Update enic NIC guide, release notes and add flow API to the
> supported features list.
> 
> Signed-off-by: John Daley 
> ---
>  doc/guides/nics/enic.rst   | 44 
> ++
>  doc/guides/nics/features/enic.ini  |  1 +
>  doc/guides/rel_notes/release_17_05.rst |  6 +

Patch needs to be rebased on top of latest next-net, to switch release
file 17.08, also enic.rst merge conflicts.

Thanks,
ferruh

>  3 files changed, 51 insertions(+)
<...>

Re: [dpdk-dev] [RFC PATCH v2 0/3] specifications for asymmetric crypto algorithms

[Neil Horman]

On Thu, May 11, 2017 at 06:05:29PM +0530, Umesh Kartha wrote:
> This RFC contains specifications for asymmetric crypto algorithms.
> Asymmetric crypto algorithms are essential part of protocols such as
> SSL/TLS. As the current DPDK crypto library lacks support for asymmetric
> crypto algorithms, this RFC is an attempt to address it.
> 
> Cavium offers  PCI hardware accelerators that supports symmetric and
> asymmetric crypto algorithms, of which a few are  addressed in this RFC.
> Once specifications are agreed upon, I can submit a patch for the same.
> We will develop a poll mode driver which can offload to OpenSSL crypto
> library and to Cavium crypto accelerator.
> 
> The asymmetric crypto algorithms supported in this version are:
> 
This all appears to modify the cryptodev api, but I don't see where said
modification was announced.

Additionally, I don't see modifications to a map file to export the api symbols.
Have you tested this in a shared library build?

Neil

> 1 RSA
>   - RSA Sign
>   - RSA Verify
>   - RSA Public Encrypt
>   - RSA Private Decrypt
> 
>   Padding schemes supported for RSA operations are
> * RSA PKCS#1 BT1
> * RSA PKCS#1 BT2
> * RSA PKCS#1 OAEP
> * RSA PKCS#1 PSS
> 
> 2 DH
>   - DH generate key
>   - DH compute key
> 
> 3 ECDH
>   - ECDH generate key
>   - ECDH check key
>   - ECDH compute key
> 
> 4 DSA
>   - DSA Sign
>   - DSA Verify
> 
> 5  ECDSA
>   - ECDSA Sign
>   - ECDSA Verify
> 
> 6  MODEXP
> 
> 7  FUNDAMENTAL ECC
>   - Point Addition
>   - Point Multiplication
>   - Point Doubling
> 
> 8 MODULAR INVERSE
> 
> 
>  Asymmetric crypto transform operations support both session oriented
> mode  and session less mode. If the operation is sessionless, an
> asymmetric crypto transform structure, containing immutable parameters,
> is passed along with per-operation mutable parameters in the structure.
> Specific structures were written to contain immutable parameters
> depending on algorithm used for crypto transform operation. The
> parameters and type of transform is distinguished by the algorithm for
> which the transform structure is filled. For a particular asymmetric
> algorithm, not all parameters will be used and hence not required to be
> filled.
> 
> Changes from RFC v1:
> 
> Added additional algorithms : DH/ECDH/MODINVERSE/DSA
> Added additional curves for ECC operations: All cuves supported by libcrypto.
> As per the comments received for RFC v1:
>  - removed mbufs from asymmetric crypto operation structure.
>  - added separate queue pair in device structure to handle asymmetric crypto
>operations.
>  - added APIs to start/stop/initialize queue pairs to handle asymmetric crypto
>operations.
>  - added asymmetric session structure and related APIs to handle session
>operations (initialize/allocate/free) etc.
> 
> RFC v1: http://dpdk.org/ml/archives/dev/2017-March/060869.html
> 
> Umesh Kartha (3):
>   cryptodev: added asymmetric algorithms
>   cryptodev: asymmetric algorithm capability definitions
>   cryptodev: added asym queue pair, session apis
> 
>  lib/librte_cryptodev/rte_crypto.h|  135 +++-
>  lib/librte_cryptodev/rte_crypto_asym.h   | 1124 
> ++
>  lib/librte_cryptodev/rte_cryptodev.c |  782 -
>  lib/librte_cryptodev/rte_cryptodev.h |  414 +++
>  lib/librte_cryptodev/rte_cryptodev_pmd.h |  113 +++
>  5 files changed, 2564 insertions(+), 4 deletions(-)
>  create mode 100644 lib/librte_cryptodev/rte_crypto_asym.h
> 
> -- 
> 1.8.3.1
> 
> 

Re: [dpdk-dev] [PATCH v2] net/tap: add support for fixed mac addresses

[Ferruh Yigit]

On 4/12/2017 8:30 AM, Pascal Mazon wrote:
> Support for a fixed MAC address for testing with the last octet
> incrementing by one for each interface defined with the new 'mac=fixed'
> string on the --vdev option. The default option is still to randomize
> the MAC address for each tap interface.
> 
> Signed-off-by: Keith Wiles 
> Signed-off-by: Pascal Mazon 
> Acked-by: Keith Wiles 

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

Re: [dpdk-dev] [PATCH v1] doc: change doc line length limit in contributors guide

[Shreyansh Jain]

Sorry for the triviality, once again:

On Thursday 11 May 2017 07:39 PM, John McNamara wrote:

The DPDK documentation guidelines state that lines should be wrapped as
follows:



[..]



- Here is an example sentence.
- Long sentences over the limit shown below can be wrapped onto
- a new line.
- These three sentences will be joined into the same paragraph.
+* Lines in literal blocks **must** by less than 80 characters since

^
"...blocks **must** be less than ..."


+  they aren't wrapped by the document formatters and can exceed the page width
+  in PDF documents.


[...]

From [1]:
"Quis custodiet ipsos custodes?".

"DPDK custodes" :) (Sorry for my pathetic latin)

[1] http://dpdk.org/ml/archives/dev/2017-May/065655.html

-
Shreyansh

Re: [dpdk-dev] [PATCH 0/5] improve tap behavior

[Ferruh Yigit]

On 4/18/2017 9:17 AM, Pascal Mazon wrote:
> The tap does not behave properly in some cases.
> 
> It is generally expected that a real device should be available once the
> probing has been done.
> 
> It is also better to check if an operation (here, setting MAC) is
> mandatory before performing it. Typically in cases where the remote
> netdevice is a VF with limited capabilities.
> 
> This series ensures that the tap works more logically.
> 
> v2 changes:
>   - fix uninitialized fd variable
> 
> Pascal Mazon (5):
>   net/tap: add debug messages
>   net/tap: remove unnecessary functions
>   net/tap: drop unnecessary nested block
>   net/tap: create netdevice during probing
>   net/tap: do not set remote MAC if not necessary

Hi Pascal,

Can you please rebase the patchset on top of latest next-net?

Thanks,
ferruh

[dpdk-dev] [PATCH v3 0/5] improve tap behavior

[Pascal Mazon]

The tap does not behave properly in some cases.

It is generally expected that a real device should be available once the
probing has been done.

It is also better to check if an operation (here, setting MAC) is
mandatory before performing it. Typically in cases where the remote
netdevice is a VF with limited capabilities.

This series ensures that the tap works more logically.

v3 changes:
  - rebase on top of next-net/master

v2 changes:
  - fix uninitialized fd variable

Pascal Mazon (5):
  net/tap: add debug messages
  net/tap: remove unnecessary functions
  net/tap: drop unnecessary nested block
  net/tap: create netdevice during probing
  net/tap: do not set remote MAC if not necessary

 drivers/net/tap/rte_eth_tap.c | 301 +-
 1 file changed, 154 insertions(+), 147 deletions(-)

-- 
2.12.0.306.g4a9b9b3

[dpdk-dev] [PATCH v3 1/5] net/tap: add debug messages

[Pascal Mazon]

Print a detailed debug message inside tap_ioctl() directly. The caller
now only needs to check for return value.

Signed-off-by: Pascal Mazon 
---
 drivers/net/tap/rte_eth_tap.c | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index f1bb55e9769b..3d08ef2ca4d4 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -445,6 +446,24 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t 
nb_pkts)
return num_tx;
 }
 
+static const char *
+tap_ioctl_req2str(unsigned long request)
+{
+   switch (request) {
+   case SIOCSIFFLAGS:
+   return "SIOCSIFFLAGS";
+   case SIOCGIFFLAGS:
+   return "SIOCGIFFLAGS";
+   case SIOCGIFHWADDR:
+   return "SIOCGIFHWADDR";
+   case SIOCSIFHWADDR:
+   return "SIOCSIFHWADDR";
+   case SIOCSIFMTU:
+   return "SIOCSIFMTU";
+   }
+   return "UNKNOWN";
+}
+
 static int
 tap_ioctl(struct pmd_internals *pmd, unsigned long request,
  struct ifreq *ifr, int set, enum ioctl_mode mode)
@@ -480,9 +499,7 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request,
case SIOCSIFMTU:
break;
default:
-   RTE_LOG(WARNING, PMD, "%s: ioctl() called with wrong arg\n",
-   pmd->name);
-   return -EINVAL;
+   RTE_ASSERT(!"unsupported request type: must not happen");
}
if (ioctl(pmd->ioctl_sock, request, ifr) < 0)
goto error;
@@ -491,8 +508,8 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request,
return 0;
 
 error:
-   RTE_LOG(ERR, PMD, "%s: ioctl(%lu) failed with error: %s\n",
-   ifr->ifr_name, request, strerror(errno));
+   RTE_LOG(DEBUG, PMD, "%s: %s(%s) failed: %s(%d)\n", ifr->ifr_name,
+   __func__, tap_ioctl_req2str(request), strerror(errno), errno);
return -errno;
 }
 
@@ -774,12 +791,8 @@ tap_mac_set(struct rte_eth_dev *dev, struct ether_addr 
*mac_addr)
return;
}
/* Check the actual current MAC address on the tap netdevice */
-   if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY) != 0) {
-   RTE_LOG(ERR, PMD,
-   "%s: couldn't check current tap MAC address\n",
-   dev->data->name);
+   if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY) != 0)
return;
-   }
if (is_same_ether_addr((struct ether_addr *)&ifr.ifr_hwaddr.sa_data,
   mac_addr))
return;
@@ -1230,10 +1243,8 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char 
*tap_name,
remote_iface);
return 0;
}
-   if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY) < 0) {
-   RTE_LOG(ERR, PMD, "Could not get remote MAC address\n");
+   if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY) < 0)
goto error_exit;
-   }
rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data,
   ETHER_ADDR_LEN);
}
-- 
2.12.0.306.g4a9b9b3

[dpdk-dev] [PATCH v3 3/5] net/tap: drop unnecessary nested block

[Pascal Mazon]

This is cosmetic; the code is functionally equivalent.

Signed-off-by: Pascal Mazon 
---
 drivers/net/tap/rte_eth_tap.c | 34 +++---
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 5b99a812fda0..91a957edb333 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -825,30 +825,26 @@ tap_setup_queue(struct rte_eth_dev *dev,
struct pmd_internals *pmd = dev->data->dev_private;
struct rx_queue *rx = &internals->rxq[qid];
struct tx_queue *tx = &internals->txq[qid];
-   int fd;
+   int fd = rx->fd == -1 ? tx->fd : rx->fd;
 
-   fd = rx->fd;
-   if (fd < 0) {
-   fd = tx->fd;
+   if (fd == -1) {
+   RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
+   pmd->name, qid);
+   fd = tun_alloc(pmd, qid);
if (fd < 0) {
-   RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
+   RTE_LOG(ERR, PMD, "tun_alloc(%s, %d) failed\n",
pmd->name, qid);
-   fd = tun_alloc(pmd, qid);
-   if (fd < 0) {
-   RTE_LOG(ERR, PMD, "tun_alloc(%s, %d) failed\n",
-   pmd->name, qid);
+   return -1;
+   }
+   if (qid == 0) {
+   struct ifreq ifr;
+
+   ifr.ifr_mtu = dev->data->mtu;
+   if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1,
+ LOCAL_AND_REMOTE) < 0) {
+   close(fd);
return -1;
}
-   if (qid == 0) {
-   struct ifreq ifr;
-
-   ifr.ifr_mtu = dev->data->mtu;
-   if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1,
- LOCAL_AND_REMOTE) < 0) {
-   close(fd);
-   return -1;
-   }
-   }
}
}
 
-- 
2.12.0.306.g4a9b9b3

[dpdk-dev] [PATCH v3 2/5] net/tap: remove unnecessary functions

[Pascal Mazon]

These functions are only two lines each and are used only once.

Signed-off-by: Pascal Mazon 
---
 drivers/net/tap/rte_eth_tap.c | 26 --
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 3d08ef2ca4d4..5b99a812fda0 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -861,26 +861,6 @@ tap_setup_queue(struct rte_eth_dev *dev,
 }
 
 static int
-rx_setup_queue(struct rte_eth_dev *dev,
-   struct pmd_internals *internals,
-   uint16_t qid)
-{
-   dev->data->rx_queues[qid] = &internals->rxq[qid];
-
-   return tap_setup_queue(dev, internals, qid);
-}
-
-static int
-tx_setup_queue(struct rte_eth_dev *dev,
-   struct pmd_internals *internals,
-   uint16_t qid)
-{
-   dev->data->tx_queues[qid] = &internals->txq[qid];
-
-   return tap_setup_queue(dev, internals, qid);
-}
-
-static int
 tap_rx_queue_setup(struct rte_eth_dev *dev,
   uint16_t rx_queue_id,
   uint16_t nb_rx_desc,
@@ -920,7 +900,8 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
}
rxq->iovecs = iovecs;
 
-   fd = rx_setup_queue(dev, internals, rx_queue_id);
+   dev->data->rx_queues[rx_queue_id] = rxq;
+   fd = tap_setup_queue(dev, internals, rx_queue_id);
if (fd == -1) {
ret = fd;
goto error;
@@ -971,7 +952,8 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
if (tx_queue_id >= internals->nb_queues)
return -1;
 
-   ret = tx_setup_queue(dev, internals, tx_queue_id);
+   dev->data->tx_queues[tx_queue_id] = &internals->txq[tx_queue_id];
+   ret = tap_setup_queue(dev, internals, tx_queue_id);
if (ret == -1)
return -1;
 
-- 
2.12.0.306.g4a9b9b3

[dpdk-dev] [PATCH v3 4/5] net/tap: create netdevice during probing

[Pascal Mazon]

This has three main benefits:
 - tun_alloc is now generic again for any queue,
 - mtu no longer needs to be handled in tap_setup_queue(),
 - an actual netdevice is created as soon as the device is probed.

On top of it, code in eth_dev_tap_create() has been reworked to have a
more logical behavior; initialization can now fail if a remote is
requested but cannot be set up.

Fixes: 2bc06869cd94 ("net/tap: add remote netdevice traffic capture")

Signed-off-by: Pascal Mazon 
---
 drivers/net/tap/rte_eth_tap.c | 217 ++
 1 file changed, 115 insertions(+), 102 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 91a957edb333..26a7f84d4f6b 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -114,10 +114,6 @@ enum ioctl_mode {
REMOTE_ONLY,
 };
 
-static int
-tap_ioctl(struct pmd_internals *pmd, unsigned long request,
- struct ifreq *ifr, int set, enum ioctl_mode mode);
-
 static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
 
 /* Tun/Tap allocation routine
@@ -126,7 +122,7 @@ static int tap_intr_handle_set(struct rte_eth_dev *dev, int 
set);
  * supplied name.
  */
 static int
-tun_alloc(struct pmd_internals *pmd, uint16_t qid)
+tun_alloc(struct pmd_internals *pmd)
 {
struct ifreq ifr;
 #ifdef IFF_MULTI_QUEUE
@@ -225,75 +221,6 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
strerror(errno));
}
 
-   if (qid == 0) {
-   struct ifreq ifr;
-
-   /*
-* pmd->eth_addr contains the desired MAC, either from remote
-* or from a random assignment. Sync it with the tap netdevice.
-*/
-   ifr.ifr_hwaddr.sa_family = AF_LOCAL;
-   rte_memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
-  ETHER_ADDR_LEN);
-   if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
-   goto error;
-
-   pmd->if_index = if_nametoindex(pmd->name);
-   if (!pmd->if_index) {
-   RTE_LOG(ERR, PMD,
-   "Could not find ifindex for %s: rte_flow won't 
be usable.\n",
-   pmd->name);
-   return fd;
-   }
-   if (!pmd->flower_support)
-   return fd;
-   if (qdisc_create_multiq(pmd->nlsk_fd, pmd->if_index) < 0) {
-   RTE_LOG(ERR, PMD,
-   "Could not create multiq qdisc for %s: rte_flow 
won't be usable.\n",
-   pmd->name);
-   return fd;
-   }
-   if (qdisc_create_ingress(pmd->nlsk_fd, pmd->if_index) < 0) {
-   RTE_LOG(ERR, PMD,
-   "Could not create multiq qdisc for %s: rte_flow 
won't be usable.\n",
-   pmd->name);
-   return fd;
-   }
-   if (pmd->remote_if_index) {
-   /*
-* Flush usually returns negative value because it tries
-* to delete every QDISC (and on a running device, one
-* QDISC at least is needed). Ignore negative return
-* value.
-*/
-   qdisc_flush(pmd->nlsk_fd, pmd->remote_if_index);
-   if (qdisc_create_ingress(pmd->nlsk_fd,
-pmd->remote_if_index) < 0)
-   goto remote_fail;
-   LIST_INIT(&pmd->implicit_flows);
-   if (tap_flow_implicit_create(
-   pmd, TAP_REMOTE_LOCAL_MAC) < 0)
-   goto remote_fail;
-   if (tap_flow_implicit_create(
-   pmd, TAP_REMOTE_BROADCAST) < 0)
-   goto remote_fail;
-   if (tap_flow_implicit_create(
-   pmd, TAP_REMOTE_BROADCASTV6) < 0)
-   goto remote_fail;
-   if (tap_flow_implicit_create(
-   pmd, TAP_REMOTE_TX) < 0)
-   goto remote_fail;
-   }
-   }
-
-   return fd;
-
-remote_fail:
-   RTE_LOG(ERR, PMD,
-   "Could not set up remote flow rules for %s: remote disabled.\n",
-   pmd->name);
-   pmd->remote_if_index = 0;
-   tap_flow_implicit_flush(pmd, NULL);
return fd;
 
 error:
@@ -830,22 +757,12 @@ tap_setup_queue(struct rte_eth_dev *dev,
if (fd == -1) {
RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
pmd->name, qid);
-   fd = tun_alloc(pmd, qid);
+ 

[dpdk-dev] [PATCH v3 5/5] net/tap: do not set remote MAC if not necessary

[Pascal Mazon]

Check for the current MAC address on both the remote and the tap
netdevices before setting a new value.

While there, remove wrong empty lines and ensure tap_ioctl() return
value is negative, just like what is done throughout this code.

Fixes: 2bc06869cd94 ("net/tap: add remote netdevice traffic capture")

Signed-off-by: Pascal Mazon 
---
 drivers/net/tap/rte_eth_tap.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 26a7f84d4f6b..49549b4f1822 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -705,11 +705,11 @@ tap_allmulti_disable(struct rte_eth_dev *dev)
tap_flow_implicit_destroy(pmd, TAP_REMOTE_ALLMULTI);
 }
 
-
 static void
 tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
struct pmd_internals *pmd = dev->data->dev_private;
+   enum ioctl_mode mode = LOCAL_ONLY;
struct ifreq ifr;
 
if (is_zero_ether_addr(mac_addr)) {
@@ -718,15 +718,20 @@ tap_mac_set(struct rte_eth_dev *dev, struct ether_addr 
*mac_addr)
return;
}
/* Check the actual current MAC address on the tap netdevice */
-   if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY) != 0)
+   if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
return;
if (is_same_ether_addr((struct ether_addr *)&ifr.ifr_hwaddr.sa_data,
   mac_addr))
return;
-
+   /* Check the current MAC address on the remote */
+   if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY) < 0)
+   return;
+   if (!is_same_ether_addr((struct ether_addr *)&ifr.ifr_hwaddr.sa_data,
+  mac_addr))
+   mode = LOCAL_AND_REMOTE;
ifr.ifr_hwaddr.sa_family = AF_LOCAL;
rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, ETHER_ADDR_LEN);
-   if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1, LOCAL_AND_REMOTE) < 0)
+   if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1, mode) < 0)
return;
rte_memcpy(&pmd->eth_addr, mac_addr, ETHER_ADDR_LEN);
if (pmd->remote_if_index) {
-- 
2.12.0.306.g4a9b9b3

Re: [dpdk-dev] [PATCH 4/4] net/dpaa2: support parallel recv mode

[Ferruh Yigit]

On 4/19/2017 2:09 PM, Hemant Agrawal wrote:
> Typically when the PMD issues a RX command to DPAA2 hardware,
> the hw writes the available descriptors into the given memory.
> The RX function then processes the frames and prepare them as
> mbufs.
> 
> This patch adds support to issue another pull request to hardware
> in another memory location, before we start processing the output
> of the first request. This help in controlling the cpu cycles
> wasted during the wait for the hardware to write the descriptors.
> 
> During hw debugging, it may be desired to keep the original
> mode, so the original mode is also preserved and can be controlled
> with an env flag.
> 
> Signed-off-by: Hemant Agrawal 

<...>

>  
> + /*If no prefetch is configured. */
> + if (getenv("DPAA2_RX_NO_PREFETCH")) {

Instead of getting configuration option from environment variable, can
you please make it argument to the driver?
This will be more consistent to the DPDK usage.

> + eth_dev->rx_pkt_burst = dpaa2_dev_rx;
> + PMD_INIT_LOG(INFO, "No Prefetch enabled");
> + }
> +

<...>

Re: [dpdk-dev] [PATCH 2/4] net/dpaa2: improve the error handling in dev init

[Ferruh Yigit]

On 4/19/2017 2:09 PM, Hemant Agrawal wrote:
> Signed-off-by: Hemant Agrawal 

<...>

> - /*Close the device at underlying layer*/
> - ret = dpni_close(dpni, CMD_PRI_LOW, priv->token);
> - if (ret) {
> - PMD_INIT_LOG(ERR, "Failure closing dpni device with"
> - " error code %d\n", ret);
> - }
> -
> - /*Free the allocated memory for ethernet private data and dpni*/
> - priv->hw = NULL;
> - free(dpni);

Where this free operation done when it is removed from dpaa2_dev_uninit() ?

> -
>   eth_dev->dev_ops = NULL;
>   eth_dev->rx_pkt_burst = NULL;
>   eth_dev->tx_pkt_burst = NULL;
> 

Re: [dpdk-dev] [PATCH 3/4] bus/fslmc: support for multiple parallel dq requests

[Ferruh Yigit]

On 4/19/2017 2:09 PM, Hemant Agrawal wrote:

Can you please note what dq stands for in commit log, and if it is an
abbreviation can you please use it uppercase in patch title?

> Signed-off-by: Hemant Agrawal 

<...>

Re: [dpdk-dev] [PATCH v3 0/5] improve tap behavior

[Ferruh Yigit]

On 5/12/2017 2:01 PM, Pascal Mazon wrote:
> The tap does not behave properly in some cases.
> 
> It is generally expected that a real device should be available once the
> probing has been done.
> 
> It is also better to check if an operation (here, setting MAC) is
> mandatory before performing it. Typically in cases where the remote
> netdevice is a VF with limited capabilities.
> 
> This series ensures that the tap works more logically.
> 
> v3 changes:
>   - rebase on top of next-net/master
> 
> v2 changes:
>   - fix uninitialized fd variable
> 
> Pascal Mazon (5):
>   net/tap: add debug messages
>   net/tap: remove unnecessary functions
>   net/tap: drop unnecessary nested block
>   net/tap: create netdevice during probing
>   net/tap: do not set remote MAC if not necessary

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

Re: [dpdk-dev] active_backup link bonding and mac address

[Kyle Larose]

I'm adding the dev mailing list/link bonding maintainer, because I've done some 
more investigation and I'm beginning to think something is wrong.

> -Original Message-
> From: Kyle Larose
> Sent: Thursday, May 11, 2017 4:55 PM
> To: us...@dpdk.org
> Subject: active_backup link bonding and mac address
> 
> Hey fellow DPDK users,
> 
> I have a question about the link bond pmd.
> 
> I am running  4 X710 interfaces in a link bond pmd for my application. In
> LACP mode, everything works fine. But, in active_backup mode, if the primary
> link fails, my application stops working. The reason is that I'm still
> sending packets with the original MAC address of the link bond pmd, which is
> that of the original primary slave. However, the new primary is not in
> promiscuous mode, so traffic coming back with that MAC address drops.
> 
> What should I be doing here:
> 
> 1) Should I be listening for the changes in the state of the primary, and
> updating the MAC address I use to send? (I have it cached for efficiency)
> 2) Should the driver be placing the interface into promiscuous mode to allow
> for this, similar to what LACP does?
> 3) Should the driver be overwriting the MAC on egress, similar to what the
> tlb driver seems to do (in bond_ethdev_tx_burst_tlb)
> 
> I'm fine with #1, but it seems to break the goal of having the link bond pmd
> be transparent to the application.
> 

I checked the mac address of the link bond interface after the failover, and it 
did not change.
It still had the MAC address of the first slave that was added. This seems 
incompatible with
solution number 1 that I suggested above, which means either it the link bond 
device should
update its address, or it should be promiscuous at the slave level.

FWIW, I'm using 16.07. I have reproduced this on testpmd by looking at port 
state. (with some
fiddling -- needed to prevent it from starting the slave interfaces, and turn 
off its default
promiscuous mode.)

Does anyone have any input on this problem?

Thanks,

Kyle

Re: [dpdk-dev] [RFC PATCH v2 0/3] specifications for asymmetric crypto algorithms

[Umesh Kartha]

Hi Neil,

On Fri, May 12, 2017 at 08:15:57AM -0400, Neil Horman wrote:
> On Thu, May 11, 2017 at 06:05:29PM +0530, Umesh Kartha wrote:
> > This RFC contains specifications for asymmetric crypto algorithms.
> > Asymmetric crypto algorithms are essential part of protocols such as
> > SSL/TLS. As the current DPDK crypto library lacks support for asymmetric
> > crypto algorithms, this RFC is an attempt to address it.
> > 
> > Cavium offers  PCI hardware accelerators that supports symmetric and
> > asymmetric crypto algorithms, of which a few are  addressed in this RFC.
> > Once specifications are agreed upon, I can submit a patch for the same.
> > We will develop a poll mode driver which can offload to OpenSSL crypto
> > library and to Cavium crypto accelerator.
> > 
> > The asymmetric crypto algorithms supported in this version are:
> > 
> This all appears to modify the cryptodev api, but I don't see where said
> modification was announced.
> 
> Additionally, I don't see modifications to a map file to export the api 
> symbols.
> Have you tested this in a shared library build?
> 
> Neil
> 

This is just an RFC for asymmetric crypto operation specifications. The
specifications are not finalised. Once the specifications are finalised,
support for asymmetric algorithms will be added to OpenSSL PMD.

> > 1 RSA
> >   - RSA Sign
> >   - RSA Verify
> >   - RSA Public Encrypt
> >   - RSA Private Decrypt
> > 
> >   Padding schemes supported for RSA operations are
> > * RSA PKCS#1 BT1
> > * RSA PKCS#1 BT2
> > * RSA PKCS#1 OAEP
> > * RSA PKCS#1 PSS
> > 
> > 2 DH
> >   - DH generate key
> >   - DH compute key
> > 
> > 3 ECDH
> >   - ECDH generate key
> >   - ECDH check key
> >   - ECDH compute key
> > 
> > 4 DSA
> >   - DSA Sign
> >   - DSA Verify
> > 
> > 5  ECDSA
> >   - ECDSA Sign
> >   - ECDSA Verify
> > 
> > 6  MODEXP
> > 
> > 7  FUNDAMENTAL ECC
> >   - Point Addition
> >   - Point Multiplication
> >   - Point Doubling
> > 
> > 8 MODULAR INVERSE
> > 
> > 
> >  Asymmetric crypto transform operations support both session oriented
> > mode  and session less mode. If the operation is sessionless, an
> > asymmetric crypto transform structure, containing immutable parameters,
> > is passed along with per-operation mutable parameters in the structure.
> > Specific structures were written to contain immutable parameters
> > depending on algorithm used for crypto transform operation. The
> > parameters and type of transform is distinguished by the algorithm for
> > which the transform structure is filled. For a particular asymmetric
> > algorithm, not all parameters will be used and hence not required to be
> > filled.
> > 
> > Changes from RFC v1:
> > 
> > Added additional algorithms : DH/ECDH/MODINVERSE/DSA
> > Added additional curves for ECC operations: All cuves supported by 
> > libcrypto.
> > As per the comments received for RFC v1:
> >  - removed mbufs from asymmetric crypto operation structure.
> >  - added separate queue pair in device structure to handle asymmetric crypto
> >operations.
> >  - added APIs to start/stop/initialize queue pairs to handle asymmetric 
> > crypto
> >operations.
> >  - added asymmetric session structure and related APIs to handle session
> >operations (initialize/allocate/free) etc.
> > 
> > RFC v1: http://dpdk.org/ml/archives/dev/2017-March/060869.html
> > 
> > Umesh Kartha (3):
> >   cryptodev: added asymmetric algorithms
> >   cryptodev: asymmetric algorithm capability definitions
> >   cryptodev: added asym queue pair, session apis
> > 
> >  lib/librte_cryptodev/rte_crypto.h|  135 +++-
> >  lib/librte_cryptodev/rte_crypto_asym.h   | 1124 
> > ++
> >  lib/librte_cryptodev/rte_cryptodev.c |  782 -
> >  lib/librte_cryptodev/rte_cryptodev.h |  414 +++
> >  lib/librte_cryptodev/rte_cryptodev_pmd.h |  113 +++
> >  5 files changed, 2564 insertions(+), 4 deletions(-)
> >  create mode 100644 lib/librte_cryptodev/rte_crypto_asym.h
> > 
> > -- 
> > 1.8.3.1
> > 
> > 
Regards,
Umesh

Re: [dpdk-dev] active_backup link bonding and mac address

[Declan Doherty]

On 12/05/2017 3:31 PM, Kyle Larose wrote:

I'm adding the dev mailing list/link bonding maintainer, because I've done some 
more investigation and I'm beginning to think something is wrong.


-Original Message-
From: Kyle Larose
Sent: Thursday, May 11, 2017 4:55 PM
To: us...@dpdk.org
Subject: active_backup link bonding and mac address

Hey fellow DPDK users,

I have a question about the link bond pmd.

I am running  4 X710 interfaces in a link bond pmd for my application. In
LACP mode, everything works fine. But, in active_backup mode, if the primary
link fails, my application stops working. The reason is that I'm still
sending packets with the original MAC address of the link bond pmd, which is
that of the original primary slave. However, the new primary is not in
promiscuous mode, so traffic coming back with that MAC address drops.

What should I be doing here:

1) Should I be listening for the changes in the state of the primary, and
updating the MAC address I use to send? (I have it cached for efficiency)
2) Should the driver be placing the interface into promiscuous mode to allow
for this, similar to what LACP does?
3) Should the driver be overwriting the MAC on egress, similar to what the
tlb driver seems to do (in bond_ethdev_tx_burst_tlb)

I'm fine with #1, but it seems to break the goal of having the link bond pmd
be transparent to the application.



I checked the mac address of the link bond interface after the failover, and it 
did not change.
It still had the MAC address of the first slave that was added. This seems 
incompatible with
solution number 1 that I suggested above, which means either it the link bond 
device should
update its address, or it should be promiscuous at the slave level.

FWIW, I'm using 16.07. I have reproduced this on testpmd by looking at port 
state. (with some
fiddling -- needed to prevent it from starting the slave interfaces, and turn 
off its default
promiscuous mode.)

Does anyone have any input on this problem?

Thanks,

Kyle




Kyle, sorry I didn't see the post in the users list. I think the issue 
is that the new primary is missing the bond MAC address on it's valid 
MACs list, hence it is dropping the ingress packets after a fail-over 
event, placing the all the slave devices into promiscuous mode as you 
suggest in option 2 would probably make the issue go away but I don't 
think it's the correct solution. I think we should just be adding the 
bond MAC to each slaves devices valid MAC list. As only one bond slave 
is only active at any time this won't cause any issues to the network, 
and will mean that fail over is transparent to your application and 
there is no need for MAC rewrites, which would invalidate existing ARP 
table entries at downstream end points.

Re: [dpdk-dev] "Port 0 is not present on the board" when building DPDK libraries as shared

[Mastorakis, Spyridon]

Hi Bruce,

Thank you for your response.

This seems to have resolved the issue.

Spyros

-Original Message-
From: Richardson, Bruce 
Sent: Friday, May 12, 2017 1:41 AM
To: Mastorakis, Spyridon 
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] "Port 0 is not present on the board" when building DPDK 
libraries as shared

On Thu, May 11, 2017 at 09:33:32PM +, Mastorakis, Spyridon wrote:
> Hi guys,
> 
> When I compile the DPDK libraries as shared (by modifying the base_config 
> file) and try to run an application (e.g., l3fwd), I get an error that port 0 
> is not present on board.
> 
> I have cleaned the system several times (unbinded NICs, removed igb_uio and 
> uio modules and hugepage mappings) and have configured everything again, but 
> this does not resolve the issue. I am also not able to add a virtual device 
> (specifically a virtual ring) when I compile as a shared library (no PMD 
> drivers found).
> 
> When I compile the DPDK libraries as static though, everything works 
> perfectly fine and I can run the DPDK applications and add virtual rings.
> 
> I have tried dpdk-17.02 and the latest stable dpdk version (16.11.1), but 
> none of them seems to work.
> 
> Do you have any clue why this is happening?
> 
> Thank you in advance,
> Spyros

When compiled as a shared lib, are you passing in the "-d" parameters to make 
sure that the PMDs are actually loaded by the app. Unlike the static version, 
they are not automatically linked in the shared lib build.

/Bruce

Re: [dpdk-dev] active_backup link bonding and mac address

[Kyle Larose]

> -Original Message-
> From: Declan Doherty [mailto:declan.dohe...@intel.com]
> Sent: Friday, May 12, 2017 10:56 AM
> To: Kyle Larose; us...@dpdk.org; dev@dpdk.org
> Subject: Re: active_backup link bonding and mac address
> 
> On 12/05/2017 3:31 PM, Kyle Larose wrote:
> > I'm adding the dev mailing list/link bonding maintainer, because I've done
> some more investigation and I'm beginning to think something is wrong.
> >
> 
> Kyle, sorry I didn't see the post in the users list. I think the issue is
> that the new primary is missing the bond MAC address on it's valid MACs
> list, hence it is dropping the ingress packets after a fail-over event,
> placing the all the slave devices into promiscuous mode as you suggest in
> option 2 would probably make the issue go away but I don't think it's the
> correct solution. I think we should just be adding the bond MAC to each
> slaves devices valid MAC list. As only one bond slave is only active at any
> time this won't cause any issues to the network, and will mean that fail
> over is transparent to your application and there is no need for MAC
> rewrites, which would invalidate existing ARP table entries at downstream
> end points.

Hey Declan,

Thanks for the prompt response.

I agree with your suggestion. Does this MAC list propagate to the slave NICs' 
hardware layers?
That is, even if a slave isn't in promiscuous mode, if it receives a frame 
addressed to any
MAC in its list, it will accept it and deliver it to the software? Or, does it 
mean we need to
put the slave into promiscuous mode, but filter any MACs not in its list 
(unless the bond
interface itself is in promiscuous mode)?

Thanks,

Kyle

Re: [dpdk-dev] active_backup link bonding and mac address

[Declan Doherty]

On 12/05/2017 4:34 PM, Kyle Larose wrote:




-Original Message-
From: Declan Doherty [mailto:declan.dohe...@intel.com]
Sent: Friday, May 12, 2017 10:56 AM
To: Kyle Larose; us...@dpdk.org; dev@dpdk.org
Subject: Re: active_backup link bonding and mac address

On 12/05/2017 3:31 PM, Kyle Larose wrote:

I'm adding the dev mailing list/link bonding maintainer, because I've done

some more investigation and I'm beginning to think something is wrong.




Kyle, sorry I didn't see the post in the users list. I think the issue is
that the new primary is missing the bond MAC address on it's valid MACs
list, hence it is dropping the ingress packets after a fail-over event,
placing the all the slave devices into promiscuous mode as you suggest in
option 2 would probably make the issue go away but I don't think it's the
correct solution. I think we should just be adding the bond MAC to each
slaves devices valid MAC list. As only one bond slave is only active at any
time this won't cause any issues to the network, and will mean that fail
over is transparent to your application and there is no need for MAC
rewrites, which would invalidate existing ARP table entries at downstream
end points.


Hey Declan,

Thanks for the prompt response.

I agree with your suggestion. Does this MAC list propagate to the slave NICs' 
hardware layers?
That is, even if a slave isn't in promiscuous mode, if it receives a frame 
addressed to any
MAC in its list, it will accept it and deliver it to the software? Or, does it 
mean we need to
put the slave into promiscuous mode, but filter any MACs not in its list 
(unless the bond
interface itself is in promiscuous mode)?

Thanks,

Kyle



Yes it would be into the hw tables, rte_eth_dev_mac_addr_add() on each 
slave port should achieve this, so there will be no need to run in 
promiscuous mode. I'll try and setup a test for this on Monday morning 
in our lab.


Declan

Re: [dpdk-dev] [PATCH] drivers/net: document missing speed capabilities feature

[Thomas Monjalon]

12/05/2017 12:49, Ferruh Yigit:
> On 5/10/2017 2:51 PM, Thomas Monjalon wrote:
> > 10/05/2017 15:10, Ferruh Yigit:
> >> --- a/doc/guides/nics/features/bnx2x.ini
> >> +++ b/doc/guides/nics/features/bnx2x.ini
> >> @@ -4,6 +4,7 @@
> >>  ; Refer to default.ini for the full list of available PMD features.
> >>  ;
> >>  [Features]
> >> +Speed capabilities   = Y
> > 
> > We should validate this feature only if the driver advertise the
> > right speeds for the device.
> 
> Hi Thomas,
> 
> Can you please clarify more, what is expected implementation in PMD?

It is expected to advertise only the speeds that the device is
capable to offer.

> And perhaps a good and a bad sample can be helpful.

Good example:
drivers/net/i40e/i40e_ethdev.c
if (I40E_PHY_TYPE_SUPPORT_40G(hw->phy.phy_types))
/* For XL710 */
dev_info->speed_capa = ETH_LINK_SPEED_40G;
else if (I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
/* For XXV710 */
dev_info->speed_capa = ETH_LINK_SPEED_25G;
else
/* For X710 */
dev_info->speed_capa = ETH_LINK_SPEED_1G | ETH_LINK_SPEED_10G;

Bad example:
drivers/net/bnx2x/bnx2x_ethdev.c
dev_info->speed_capa = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_20G;
Looking at qlogic.com, only some 57840 adapters are capable of 20G.

Re: [dpdk-dev] active_backup link bonding and mac address

[Kyle Larose]

> -Original Message-
> From: Declan Doherty [mailto:declan.dohe...@intel.com]
> Sent: Friday, May 12, 2017 11:47 AM
> To: Kyle Larose; us...@dpdk.org; dev@dpdk.org
> Subject: Re: active_backup link bonding and mac address
> 
> 
> Yes it would be into the hw tables, rte_eth_dev_mac_addr_add() on each slave
> port should achieve this, so there will be no need to run in promiscuous
> mode. I'll try and setup a test for this on Monday morning in our lab.
> 
> Declan


Thanks for the suggestion. I modified mac_address_slaves_update to add the link
bond's mac address to all slaves in addition to setting their mac. Previously it
would only set the link bond's mac on the primary, and not add it to the hw.
This has solved my problem. On failure of the primary, my application starts
working via the backup seamlessly.

Now, I'm not sure if what I have done is ideal -- should we only be installing 
it
on the primary, and removing it from the old primary on failure? Either way, let
me know if you want my change as a patch, or whether you've come up with one of
your own. :)

Thanks,

Kyle

[dpdk-dev] [PATCH] eventdev: clarify atomic and ordered queue config

[Gage Eads]

The nb_atomic_flows and nb_atomic_order_sequences fields are only inspected
if the queue is configured for atomic or ordered scheduling, respectively.
This commit updates the documentation to reflect that.

Signed-off-by: Gage Eads 
---
 lib/librte_eventdev/rte_eventdev.h | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.h 
b/lib/librte_eventdev/rte_eventdev.h
index 20e7293..32ffcd1 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -521,9 +521,11 @@ rte_event_dev_configure(uint8_t dev_id,
 struct rte_event_queue_conf {
uint32_t nb_atomic_flows;
/**< The maximum number of active flows this queue can track at any
-* given time. The value must be in the range of
-* [1 - nb_event_queue_flows)] which previously provided in
-* rte_event_dev_info_get().
+* given time. If the queue is configured for atomic scheduling (by
+* applying the RTE_EVENT_QUEUE_CFG_ALL_TYPES or
+* RTE_EVENT_QUEUE_CFG_ATOMIC_ONLY flags to event_queue_cfg), then the
+* value must be in the range of [1 - nb_event_queue_flows)], which was
+* previously provided in rte_event_dev_configure().
 */
uint32_t nb_atomic_order_sequences;
/**< The maximum number of outstanding events waiting to be
@@ -533,8 +535,11 @@ struct rte_event_queue_conf {
 * scheduler cannot schedule the events from this queue and invalid
 * event will be returned from dequeue until one or more entries are
 * freed up/released.
-* The value must be in the range of [1 - nb_event_queue_flows)]
-* which previously supplied to rte_event_dev_configure().
+* If the queue is configured for ordered scheduling (by applying the
+* RTE_EVENT_QUEUE_CFG_ALL_TYPES or RTE_EVENT_QUEUE_CFG_ORDERED_ONLY
+* flags to event_queue_cfg), then the value must be in the range of [1
+* - nb_event_queue_flows)], which was previously supplied to
+* rte_event_dev_configure().
 */
uint32_t event_queue_cfg; /**< Queue cfg flags(EVENT_QUEUE_CFG_) */
uint8_t priority;
-- 
2.7.4