* Re: [PATCH v2 01/12] eal: define container_of macro
From: Adrien Mazarguil @ 2016-12-16 11:21 UTC (permalink / raw)
To: Jan Blunck
Cc: Shreyansh Jain, dev, David Marchand, Thomas Monjalon,
Ferruh Yigit, jianbo.liu, Jan Viktorin
In-Reply-To: <CALe+Z02WkXzbz+wA=ysvjf5SvU2bMCEY0vHTm0oGxXG=Jhs34Q@mail.gmail.com>
On Fri, Dec 16, 2016 at 11:47:14AM +0100, Jan Blunck wrote:
> On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
> <adrien.mazarguil@6wind.com> wrote:
> > On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
> >> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> >> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
> >> >>
> >> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
> >> >> wrote:
> >> >>>
> >> >>> From: Jan Blunck <jblunck@infradead.org>
> >> >>>
> >> >>> This macro is based on Jan Viktorin's original patch but also checks the
> >> >>> type of the passed pointer against the type of the member.
> >> >>>
> >> >>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> >> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
> >> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >> >>> [jblunck@infradead.org: add type checking and __extension__]
> >> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> >> >>>
> >> >>> --
> >> >>> v2:
> >> >>> - fix checkpatch error
> >> >>> ---
> >> >>> lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
> >> >>> 1 file changed, 21 insertions(+)
> >> >>>
> >> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
> >> >>> b/lib/librte_eal/common/include/rte_common.h
> >> >>> index db5ac91..3eb8d11 100644
> >> >>> --- a/lib/librte_eal/common/include/rte_common.h
> >> >>> +++ b/lib/librte_eal/common/include/rte_common.h
> >> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
> >> >>> #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
> >> >>> #endif
> >> >>>
> >> >>> +/**
> >> >>> + * Return pointer to the wrapping struct instance.
> >> >>> + *
> >> >>> + * Example:
> >> >>> + *
> >> >>> + * struct wrapper {
> >> >>> + * ...
> >> >>> + * struct child c;
> >> >>> + * ...
> >> >>> + * };
> >> >>> + *
> >> >>> + * struct child *x = obtain(...);
> >> >>> + * struct wrapper *w = container_of(x, struct wrapper, c);
> >> >>> + */
> >> >>> +#ifndef container_of
> >> >>> +#define container_of(ptr, type, member) (__extension__ ({
> >> >>> \
> >> >>> + typeof(((type *)0)->member) * _ptr = (ptr); \
> >> >>> + (type *)(((char *)_ptr) - offsetof(type,
> >> >>> member));\
> >> >>> + }))
> >> >>
> >> >>
> >> >> This is a checkpatch false positive. It should be fine to ignore this.
> >> >> IIRC we already discussed this before.
> >> >
> >> >
> >> > I too thought something similar was discussed. I tried searching the
> >> > archives but couldn't find anything - thus, I thought probably I was
> >> > hallucinating :P
> >> >
> >> > So, you want me to revert back the '()' change? Does it impact the expansion
> >> > of this macro?
> >>
> >> We haven't added this on any other usage of the __extension__ keyword
> >> in the existing code. From my perspective it is more consistent to
> >> revert it.
> >>
> >> Anyone else with an opinion here? David? Thomas?
> >
> > As an exported header, rte_common.h must pass check-includes.sh. Both
> > typeof() and the ({ ... }) construct are non-standard GCC extensions and
> > would fail to compile with pedantic options.
> >
>
> Thanks Adrien. These extensions are already in use by rte_common.h and
> other headers. I don't believe we can remove the usage of typeof()
> that easily without making the code really ugly.
Sure, no problem with that, I misread and thought you wanted to drop
__extension__ as well.
Parentheses may perhaps cause more accurate warnings in case of syntax
errors. That is not significant so either way is fine by me.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v2] crypto/qat: fix to avoid buffer overwrite in OOP case
From: Griffin, John @ 2016-12-16 11:20 UTC (permalink / raw)
To: Trahe, Fiona, dev@dpdk.org
Cc: De Lara Guarch, Pablo, Trahe, Fiona, stable@dpdk.org
In-Reply-To: <1481297945-20649-1-git-send-email-fiona.trahe@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> Sent: Friday, December 9, 2016 3:39 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] crypto/qat: fix to avoid buffer overwrite in
> OOP case
>
> In out-of-place operation, data is DMAed from source mbuf to destination
> mbuf. To avoid header data in dest mbuf being overwritten, the minimal data-
> set should be DMAed.
>
> Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for
> performance")
>
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
Acked-by: John Griffin <john.griffin@intel.com>
^ permalink raw reply
* Re: [PATCH v2 01/12] eal: define container_of macro
From: Jan Blunck @ 2016-12-16 10:47 UTC (permalink / raw)
To: Adrien Mazarguil
Cc: Shreyansh Jain, dev, David Marchand, Thomas Monjalon,
Ferruh Yigit, jianbo.liu, Jan Viktorin
In-Reply-To: <20161216092306.GD10340@6wind.com>
On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
<adrien.mazarguil@6wind.com> wrote:
> On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
>> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
>> >>
>> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
>> >> wrote:
>> >>>
>> >>> From: Jan Blunck <jblunck@infradead.org>
>> >>>
>> >>> This macro is based on Jan Viktorin's original patch but also checks the
>> >>> type of the passed pointer against the type of the member.
>> >>>
>> >>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> >>> [jblunck@infradead.org: add type checking and __extension__]
>> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>> >>>
>> >>> --
>> >>> v2:
>> >>> - fix checkpatch error
>> >>> ---
>> >>> lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
>> >>> 1 file changed, 21 insertions(+)
>> >>>
>> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
>> >>> b/lib/librte_eal/common/include/rte_common.h
>> >>> index db5ac91..3eb8d11 100644
>> >>> --- a/lib/librte_eal/common/include/rte_common.h
>> >>> +++ b/lib/librte_eal/common/include/rte_common.h
>> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
>> >>> #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
>> >>> #endif
>> >>>
>> >>> +/**
>> >>> + * Return pointer to the wrapping struct instance.
>> >>> + *
>> >>> + * Example:
>> >>> + *
>> >>> + * struct wrapper {
>> >>> + * ...
>> >>> + * struct child c;
>> >>> + * ...
>> >>> + * };
>> >>> + *
>> >>> + * struct child *x = obtain(...);
>> >>> + * struct wrapper *w = container_of(x, struct wrapper, c);
>> >>> + */
>> >>> +#ifndef container_of
>> >>> +#define container_of(ptr, type, member) (__extension__ ({
>> >>> \
>> >>> + typeof(((type *)0)->member) * _ptr = (ptr); \
>> >>> + (type *)(((char *)_ptr) - offsetof(type,
>> >>> member));\
>> >>> + }))
>> >>
>> >>
>> >> This is a checkpatch false positive. It should be fine to ignore this.
>> >> IIRC we already discussed this before.
>> >
>> >
>> > I too thought something similar was discussed. I tried searching the
>> > archives but couldn't find anything - thus, I thought probably I was
>> > hallucinating :P
>> >
>> > So, you want me to revert back the '()' change? Does it impact the expansion
>> > of this macro?
>>
>> We haven't added this on any other usage of the __extension__ keyword
>> in the existing code. From my perspective it is more consistent to
>> revert it.
>>
>> Anyone else with an opinion here? David? Thomas?
>
> As an exported header, rte_common.h must pass check-includes.sh. Both
> typeof() and the ({ ... }) construct are non-standard GCC extensions and
> would fail to compile with pedantic options.
>
Thanks Adrien. These extensions are already in use by rte_common.h and
other headers. I don't believe we can remove the usage of typeof()
that easily without making the code really ugly.
^ permalink raw reply
* Re: [PATCH v2 09/13] PMD/af_packet: guard against buffer overruns in TX path
From: Ferruh Yigit @ 2016-12-16 10:32 UTC (permalink / raw)
To: John W. Linville, Michał Mirosław; +Cc: dev
In-Reply-To: <20161213160605.GB27914@tuxdriver.com>
On 12/13/2016 4:06 PM, John W. Linville wrote:
> On Tue, Dec 13, 2016 at 02:28:34AM +0100, Michał Mirosław wrote:
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
>
> Acked-by: John W. Linville <linville@tuxdriver.com>
>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH v2 08/13] PMD/af_packet: guard against buffer overruns in RX path
From: Ferruh Yigit @ 2016-12-16 10:32 UTC (permalink / raw)
To: John W. Linville, Michał Mirosław; +Cc: dev, test-report
In-Reply-To: <20161213160550.GA27914@tuxdriver.com>
On 12/13/2016 4:05 PM, John W. Linville wrote:
> On Tue, Dec 13, 2016 at 02:28:34AM +0100, Michał Mirosław wrote:
>>
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
>
> Acked-by: John W. Linville <linville@tuxdriver.com>
>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH] crypto/openssl: fix extra bytes being written at end of data
From: De Lara Guarch, Pablo @ 2016-12-16 10:30 UTC (permalink / raw)
To: Azarewicz, PiotrX T; +Cc: dev@dpdk.org, stable@dpdk.org
In-Reply-To: <1481107554-206879-1-git-send-email-piotrx.t.azarewicz@intel.com>
> -----Original Message-----
> From: Azarewicz, PiotrX T
> Sent: Wednesday, December 07, 2016 10:46 AM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] crypto/openssl: fix extra bytes being written at end of
> data
>
> Extra bytes are being written at end of data while process standard
> openssl cipher encryption. This behaviour is unexpected.
>
> This patch disable the padding feature in openssl library, which is
> causing the problem.
>
> Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL library")
>
> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
^ permalink raw reply
* Re: [PATCH v2 4/4] doc: add ixgbe specific APIs
From: Mcnamara, John @ 2016-12-16 10:29 UTC (permalink / raw)
To: Bie, Tiwei, dev@dpdk.org
Cc: Lu, Wenzhuo, Dai, Wei, Wang, Xiao W, Ananyev, Konstantin,
Zhang, Helin
In-Reply-To: <1481852611-103254-5-git-send-email-tiwei.bie@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tiwei Bie
> Sent: Friday, December 16, 2016 1:44 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Dai, Wei <wei.dai@intel.com>;
> Wang, Xiao W <xiao.w.wang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [dpdk-dev] [PATCH v2 4/4] doc: add ixgbe specific APIs
>
> Add information about the new ixgbe PMD APIs in the release note.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH] doc: Fix a typo in testpmd application user guide.
From: Mcnamara, John @ 2016-12-16 10:27 UTC (permalink / raw)
To: Rosen, Rami, dev@dpdk.org; +Cc: Rosen, Rami
In-Reply-To: <1481835800-6241-1-git-send-email-rami.rosen@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rami Rosen
> Sent: Thursday, December 15, 2016 9:03 PM
> To: dev@dpdk.org
> Cc: Rosen, Rami <rami.rosen@intel.com>
> Subject: [dpdk-dev] [PATCH] doc: Fix a typo in testpmd application user
> guide.
>
> This patch fixes a trivial typo in testpmd application user guide.
>
> Signed-off-by: Rami Rosen <rami.rosen@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH 13/28] eal/arm64: override I/O device read/write access for arm64
From: Jerin Jacob @ 2016-12-16 10:25 UTC (permalink / raw)
To: Jianbo Liu
Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
Jan Viktorin
In-Reply-To: <CAP4Qi38_5zKe6a=v3W5wzcDgeB9Un97O8UAybucgPjhucHsE8Q@mail.gmail.com>
On Fri, Dec 16, 2016 at 06:12:13PM +0800, Jianbo Liu wrote:
> On 15 December 2016 at 19:08, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
> >> On 15 December 2016 at 18:04, Jerin Jacob
> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
> >> >> On 14 December 2016 at 09:55, Jerin Jacob
> >> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> >> > Override the generic I/O device memory read/write access and implement it
> >> >> > using armv8 instructions for arm64.
> >> >> >
> >> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >> >> > ---
> >> >> > lib/librte_eal/common/include/arch/arm/rte_io.h | 4 +
> >> >> > lib/librte_eal/common/include/arch/arm/rte_io_64.h | 183 +++++++++++++++++++++
> >> >> > 2 files changed, 187 insertions(+)
> >> >> > create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> >
> >> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > index 74c1f2c..9593b42 100644
> >> >> > --- a/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > @@ -38,7 +38,11 @@
> >> >> > extern "C" {
> >> >> > #endif
> >> >> >
> >> >> > +#ifdef RTE_ARCH_64
> >> >> > +#include "rte_io_64.h"
> >> >> > +#else
> >> >> > #include "generic/rte_io.h"
> >> >> > +#endif
> >> >> >
> >> >> > #ifdef __cplusplus
> >> >> > }
> >> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> > new file mode 100644
> >> >> > index 0000000..09e7a89
> >> >> > --- /dev/null
> >> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> > @@ -0,0 +1,183 @@
> >> >> > +/*
> >> >> > + * BSD LICENSE
> >> >> > + *
> >> >> > + * Copyright (C) Cavium networks Ltd. 2016.
> >> >> > + *
> >> >> > + * Redistribution and use in source and binary forms, with or without
> >> >> > + * modification, are permitted provided that the following conditions
> >> >> > + * are met:
> >> >> > + *
> >> >> > + * * Redistributions of source code must retain the above copyright
> >> >> > + * notice, this list of conditions and the following disclaimer.
> >> >> > + * * Redistributions in binary form must reproduce the above copyright
> >> >> > + * notice, this list of conditions and the following disclaimer in
> >> >> > + * the documentation and/or other materials provided with the
> >> >> > + * distribution.
> >> >> > + * * Neither the name of Cavium networks nor the names of its
> >> >> > + * contributors may be used to endorse or promote products derived
> >> >> > + * from this software without specific prior written permission.
> >> >> > + *
> >> >> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> >> >> > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >> >> > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> >> >> > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> >> >> > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> >> >> > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >> >> > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> >> >> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> >> >> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> >> >> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> >> >> > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >> >> > + */
> >> >> > +
> >> >> > +#ifndef _RTE_IO_ARM64_H_
> >> >> > +#define _RTE_IO_ARM64_H_
> >> >> > +
> >> >> > +#ifdef __cplusplus
> >> >> > +extern "C" {
> >> >> > +#endif
> >> >> > +
> >> >> > +#include <stdint.h>
> >> >> > +
> >> >> > +#define RTE_OVERRIDE_IO_H
> >> >> > +
> >> >> > +#include "generic/rte_io.h"
> >> >> > +#include "rte_atomic_64.h"
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint8_t
> >> >> > +__rte_arm64_readb(const volatile void *addr)
> >> >> > +{
> >> >> > + uint8_t val;
> >> >> > +
> >> >> > + asm volatile(
> >> >> > + "ldrb %w[val], [%x[addr]]"
> >> >> > + : [val] "=r" (val)
> >> >> > + : [addr] "r" (addr));
> >> >> > + return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint16_t
> >> >> > +__rte_arm64_readw(const volatile void *addr)
> >> >> > +{
> >> >> > + uint16_t val;
> >> >> > +
> >> >> > + asm volatile(
> >> >> > + "ldrh %w[val], [%x[addr]]"
> >> >> > + : [val] "=r" (val)
> >> >> > + : [addr] "r" (addr));
> >> >> > + return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint32_t
> >> >> > +__rte_arm64_readl(const volatile void *addr)
> >> >> > +{
> >> >> > + uint32_t val;
> >> >> > +
> >> >> > + asm volatile(
> >> >> > + "ldr %w[val], [%x[addr]]"
> >> >> > + : [val] "=r" (val)
> >> >> > + : [addr] "r" (addr));
> >> >> > + return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint64_t
> >> >> > +__rte_arm64_readq(const volatile void *addr)
> >> >> > +{
> >> >> > + uint64_t val;
> >> >> > +
> >> >> > + asm volatile(
> >> >> > + "ldr %x[val], [%x[addr]]"
> >> >> > + : [val] "=r" (val)
> >> >> > + : [addr] "r" (addr));
> >> >> > + return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writeb(uint8_t val, volatile void *addr)
> >> >> > +{
> >> >> > + asm volatile(
> >> >> > + "strb %w[val], [%x[addr]]"
> >> >> > + :
> >> >> > + : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writew(uint16_t val, volatile void *addr)
> >> >> > +{
> >> >> > + asm volatile(
> >> >> > + "strh %w[val], [%x[addr]]"
> >> >> > + :
> >> >> > + : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writel(uint32_t val, volatile void *addr)
> >> >> > +{
> >> >> > + asm volatile(
> >> >> > + "str %w[val], [%x[addr]]"
> >> >> > + :
> >> >> > + : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writeq(uint64_t val, volatile void *addr)
> >> >> > +{
> >> >> > + asm volatile(
> >> >> > + "str %x[val], [%x[addr]]"
> >> >> > + :
> >> >> > + : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >>
> >> >> I'm not quite sure about these overridings. Can you explain the
> >> >> benefit to do so?
> >> >
> >> > Better to be native if there is option. That all. Do you see any issue?
> >> > or what is the real concern?
> >> >
> >>
> >> I think it's the same as the generic c version after compiling. Am I right?
> >
> > I really don't that is the case for all the scenarios like compiler may
> > combine two 16bit reads one 32bit read etc and which will impact on IO
>
> I wonder which compiler will do that as armv8 is 32/64 bit system?
Not specific to armv8.
Two consecutive continues 16bits reads one 32bit read for optimization.
Any idea why Linux kernel doing explicit instructions for readl/writel?
obviously not for fun.
>
> > register access.
> >
> > But, I am sure the proposed scheme generates correct instruction in all the cases.
^ permalink raw reply
* Re: [PATCH 1/4] eal/common: introduce rte_memset on IA platform
From: Yang, Zhiyong @ 2016-12-16 10:19 UTC (permalink / raw)
To: Richardson, Bruce
Cc: Ananyev, Konstantin, Thomas Monjalon, dev@dpdk.org,
yuanhan.liu@linux.intel.com, De Lara Guarch, Pablo
In-Reply-To: <20161215101242.GA125588@bricha3-MOBL3.ger.corp.intel.com>
Hi, Bruce:
> -----Original Message-----
> From: Richardson, Bruce
> Sent: Thursday, December 15, 2016 6:13 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> Monjalon <thomas.monjalon@6wind.com>; dev@dpdk.org;
> yuanhan.liu@linux.intel.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/4] eal/common: introduce rte_memset on
> IA platform
>
> On Thu, Dec 15, 2016 at 06:51:08AM +0000, Yang, Zhiyong wrote:
> > Hi, Thomas, Konstantin:
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yang, Zhiyong
> > > Sent: Sunday, December 11, 2016 8:33 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> > > Monjalon <thomas.monjalon@6wind.com>
> > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > <pablo.de.lara.guarch@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> rte_memset
> > > on IA platform
> > >
> > > Hi, Konstantin, Bruce:
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Thursday, December 8, 2016 6:31 PM
> > > > To: Yang, Zhiyong <zhiyong.yang@intel.com>; Thomas Monjalon
> > > > <thomas.monjalon@6wind.com>
> > > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > > <pablo.de.lara.guarch@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> > > > rte_memset on IA platform
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Yang, Zhiyong
> > > > > Sent: Thursday, December 8, 2016 9:53 AM
> > > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> > > > > Monjalon <thomas.monjalon@6wind.com>
> > > > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > > > <pablo.de.lara.guarch@intel.com>
> > > > > Subject: RE: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> > > > > rte_memset on IA platform
> > > > >
> > > > extern void *(*__rte_memset_vector)( (void *s, int c, size_t n);
> > > >
> > > > static inline void*
> > > > rte_memset_huge(void *s, int c, size_t n) {
> > > > return __rte_memset_vector(s, c, n); }
> > > >
> > > > static inline void *
> > > > rte_memset(void *s, int c, size_t n) {
> > > > If (n < XXX)
> > > > return rte_memset_scalar(s, c, n);
> > > > else
> > > > return rte_memset_huge(s, c, n); }
> > > >
> > > > XXX could be either a define, or could also be a variable, so it
> > > > can be setuped at startup, depending on the architecture.
> > > >
> > > > Would that work?
> > > > Konstantin
> > > >
> > I have implemented the code for choosing the functions at run time.
> > rte_memcpy is used more frequently, So I test it at run time.
> >
> > typedef void *(*rte_memcpy_vector_t)(void *dst, const void *src,
> > size_t n); extern rte_memcpy_vector_t rte_memcpy_vector; static inline
> > void * rte_memcpy(void *dst, const void *src, size_t n) {
> > return rte_memcpy_vector(dst, src, n); } In order to reduce
> > the overhead at run time, I assign the function address to var
> > rte_memcpy_vector before main() starts to init the var.
> >
> > static void __attribute__((constructor))
> > rte_memcpy_init(void)
> > {
> > if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
> > {
> > rte_memcpy_vector = rte_memcpy_avx2;
> > }
> > else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
> > {
> > rte_memcpy_vector = rte_memcpy_sse;
> > }
> > else
> > {
> > rte_memcpy_vector = memcpy;
> > }
> >
> > }
> > I run the same virtio/vhost loopback tests without NIC.
> > I can see the throughput drop when running choosing functions at run
> > time compared to original code as following on the same platform(my
> machine is haswell)
> > Packet size perf drop
> > 64 -4%
> > 256 -5.4%
> > 1024 -5%
> > 1500 -2.5%
> > Another thing, I run the memcpy_perf_autotest, when N= <128, the
> > rte_memcpy perf gains almost disappears When choosing functions at run
> > time. For N=other numbers, the perf gains will become narrow.
> >
> How narrow. How significant is the improvement that we gain from having to
> maintain our own copy of memcpy. If the libc version is nearly as good we
> should just use that.
>
> /Bruce
Zhihong sent a patch about rte_memcpy, From the patch,
we can see the optimization job for memcpy will bring obvious perf improvements
than glibc for DPDK.
http://www.dpdk.org/dev/patchwork/patch/17753/
git log as following:
This patch is tested on Ivy Bridge, Haswell and Skylake, it provides
up to 20% gain for Virtio Vhost PVP traffic, with packet size ranging
from 64 to 1500 bytes.
thanks
Zhiyong
^ permalink raw reply
* Re: [PATCH 13/28] eal/arm64: override I/O device read/write access for arm64
From: Jianbo Liu @ 2016-12-16 10:12 UTC (permalink / raw)
To: Jerin Jacob
Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
Jan Viktorin
In-Reply-To: <20161215110807.GA10881@localhost.localdomain>
On 15 December 2016 at 19:08, Jerin Jacob
<jerin.jacob@caviumnetworks.com> wrote:
> On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
>> On 15 December 2016 at 18:04, Jerin Jacob
>> <jerin.jacob@caviumnetworks.com> wrote:
>> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
>> >> On 14 December 2016 at 09:55, Jerin Jacob
>> >> <jerin.jacob@caviumnetworks.com> wrote:
>> >> > Override the generic I/O device memory read/write access and implement it
>> >> > using armv8 instructions for arm64.
>> >> >
>> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> >> > ---
>> >> > lib/librte_eal/common/include/arch/arm/rte_io.h | 4 +
>> >> > lib/librte_eal/common/include/arch/arm/rte_io_64.h | 183 +++++++++++++++++++++
>> >> > 2 files changed, 187 insertions(+)
>> >> > create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> >
>> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > index 74c1f2c..9593b42 100644
>> >> > --- a/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > @@ -38,7 +38,11 @@
>> >> > extern "C" {
>> >> > #endif
>> >> >
>> >> > +#ifdef RTE_ARCH_64
>> >> > +#include "rte_io_64.h"
>> >> > +#else
>> >> > #include "generic/rte_io.h"
>> >> > +#endif
>> >> >
>> >> > #ifdef __cplusplus
>> >> > }
>> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> > new file mode 100644
>> >> > index 0000000..09e7a89
>> >> > --- /dev/null
>> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> > @@ -0,0 +1,183 @@
>> >> > +/*
>> >> > + * BSD LICENSE
>> >> > + *
>> >> > + * Copyright (C) Cavium networks Ltd. 2016.
>> >> > + *
>> >> > + * Redistribution and use in source and binary forms, with or without
>> >> > + * modification, are permitted provided that the following conditions
>> >> > + * are met:
>> >> > + *
>> >> > + * * Redistributions of source code must retain the above copyright
>> >> > + * notice, this list of conditions and the following disclaimer.
>> >> > + * * Redistributions in binary form must reproduce the above copyright
>> >> > + * notice, this list of conditions and the following disclaimer in
>> >> > + * the documentation and/or other materials provided with the
>> >> > + * distribution.
>> >> > + * * Neither the name of Cavium networks nor the names of its
>> >> > + * contributors may be used to endorse or promote products derived
>> >> > + * from this software without specific prior written permission.
>> >> > + *
>> >> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> >> > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> >> > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>> >> > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>> >> > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>> >> > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> >> > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> >> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> >> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> >> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>> >> > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> >> > + */
>> >> > +
>> >> > +#ifndef _RTE_IO_ARM64_H_
>> >> > +#define _RTE_IO_ARM64_H_
>> >> > +
>> >> > +#ifdef __cplusplus
>> >> > +extern "C" {
>> >> > +#endif
>> >> > +
>> >> > +#include <stdint.h>
>> >> > +
>> >> > +#define RTE_OVERRIDE_IO_H
>> >> > +
>> >> > +#include "generic/rte_io.h"
>> >> > +#include "rte_atomic_64.h"
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint8_t
>> >> > +__rte_arm64_readb(const volatile void *addr)
>> >> > +{
>> >> > + uint8_t val;
>> >> > +
>> >> > + asm volatile(
>> >> > + "ldrb %w[val], [%x[addr]]"
>> >> > + : [val] "=r" (val)
>> >> > + : [addr] "r" (addr));
>> >> > + return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint16_t
>> >> > +__rte_arm64_readw(const volatile void *addr)
>> >> > +{
>> >> > + uint16_t val;
>> >> > +
>> >> > + asm volatile(
>> >> > + "ldrh %w[val], [%x[addr]]"
>> >> > + : [val] "=r" (val)
>> >> > + : [addr] "r" (addr));
>> >> > + return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint32_t
>> >> > +__rte_arm64_readl(const volatile void *addr)
>> >> > +{
>> >> > + uint32_t val;
>> >> > +
>> >> > + asm volatile(
>> >> > + "ldr %w[val], [%x[addr]]"
>> >> > + : [val] "=r" (val)
>> >> > + : [addr] "r" (addr));
>> >> > + return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint64_t
>> >> > +__rte_arm64_readq(const volatile void *addr)
>> >> > +{
>> >> > + uint64_t val;
>> >> > +
>> >> > + asm volatile(
>> >> > + "ldr %x[val], [%x[addr]]"
>> >> > + : [val] "=r" (val)
>> >> > + : [addr] "r" (addr));
>> >> > + return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writeb(uint8_t val, volatile void *addr)
>> >> > +{
>> >> > + asm volatile(
>> >> > + "strb %w[val], [%x[addr]]"
>> >> > + :
>> >> > + : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writew(uint16_t val, volatile void *addr)
>> >> > +{
>> >> > + asm volatile(
>> >> > + "strh %w[val], [%x[addr]]"
>> >> > + :
>> >> > + : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writel(uint32_t val, volatile void *addr)
>> >> > +{
>> >> > + asm volatile(
>> >> > + "str %w[val], [%x[addr]]"
>> >> > + :
>> >> > + : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writeq(uint64_t val, volatile void *addr)
>> >> > +{
>> >> > + asm volatile(
>> >> > + "str %x[val], [%x[addr]]"
>> >> > + :
>> >> > + : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >>
>> >> I'm not quite sure about these overridings. Can you explain the
>> >> benefit to do so?
>> >
>> > Better to be native if there is option. That all. Do you see any issue?
>> > or what is the real concern?
>> >
>>
>> I think it's the same as the generic c version after compiling. Am I right?
>
> I really don't that is the case for all the scenarios like compiler may
> combine two 16bit reads one 32bit read etc and which will impact on IO
I wonder which compiler will do that as armv8 is 32/64 bit system?
> register access.
>
> But, I am sure the proposed scheme generates correct instruction in all the cases.
^ permalink raw reply
* Re: [PATCH 07/13] pcap: fix timestamps in output pcap file
From: Ferruh Yigit @ 2016-12-16 10:06 UTC (permalink / raw)
To: Michał Mirosław, dev; +Cc: "CC: stable"
In-Reply-To: <b8fb833e-634e-f559-12ce-1e71d7740ce5@intel.com>
On 12/14/2016 5:45 PM, Ferruh Yigit wrote:
> On 12/13/2016 1:08 AM, Michał Mirosław wrote:
>> From: Piotr Bartosiewicz <piotr.bartosiewicz@atendesoftware.pl>
>>
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
>> ---
>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
CC: stable@dpdk.org
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH 1/4] rte_mbuf: add rte_pktmbuf_coalesce
From: Olivier Matz @ 2016-12-16 10:06 UTC (permalink / raw)
To: Tomasz Kulasek; +Cc: dev
In-Reply-To: <1480698466-17620-2-git-send-email-tomaszx.kulasek@intel.com>
Hi Tomasz,
On Fri, 2 Dec 2016 18:07:43 +0100, Tomasz Kulasek
<tomaszx.kulasek@intel.com> wrote:
> This patch adds function rte_pktmbuf_coalesce to let crypto PMD
> coalesce chained mbuf before crypto operation and extend their
> capabilities to support segmented mbufs when device cannot handle
> them natively.
>
>
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> ---
> lib/librte_mbuf/rte_mbuf.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index ead7c6e..f048681 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1647,6 +1647,40 @@ static inline int rte_pktmbuf_chain(struct
> rte_mbuf *head, struct rte_mbuf *tail }
>
> /**
> + * Coalesce data from mbuf to the continuous buffer.
> + *
> + * @param mbuf_dst
> + * Contiguous destination mbuf
> + * @param mbuf_src
> + * Uncontiguous source mbuf
> + *
> + * @return
> + * - 0, on success
> + * - -EINVAL, on error
> + */
I think the API should be clarified. In your case, it is expected that the
destination mbuf is already filled with uninitialized data (i.e. that
rte_pktmbuf_append() has been called).
We could wonder if a better API wouldn't be to allocate the dst mbuf in
the function, call append()/prepend(), and do the copy.
Even better, we could have:
int rte_pktmbuf_linearize(struct rte_mbuf *m)
It will reuse the same mbuf (maybe moving the data).
> +
> +#include <rte_hexdump.h>
This should be removed.
> +
> +static inline int
> +rte_pktmbuf_coalesce(struct rte_mbuf *mbuf_dst, struct rte_mbuf *mbuf_src) {
Source mbuf should be const.
> + char *dst;
> +
> + if (!rte_pktmbuf_is_contiguous(mbuf_dst) ||
> + rte_pktmbuf_data_len(mbuf_dst) >=
> + rte_pktmbuf_pkt_len(mbuf_src))
> + return -EINVAL;
Why >= ?
> +
> + dst = rte_pktmbuf_mtod(mbuf_dst, char *);
> +
> + if (!__rte_pktmbuf_read(mbuf_src, 0, rte_pktmbuf_pkt_len(mbuf_src),
> + dst))
When a function returns a pointer, I think it is clearer to do:
if (func() == NULL)
than:
if (!func())
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> * Dump an mbuf structure to a file.
> *
> * Dump all fields for the given packet mbuf and all its associated
One more question, I don't see where this function is used in your
patchset. What is your use-case?
Regards,
Olivier
^ permalink raw reply
* Re: [PATCH v2 00/32] Support more features in Solarflare PMD
From: Ferruh Yigit @ 2016-12-16 9:57 UTC (permalink / raw)
To: Andrew Rybchenko, dev
In-Reply-To: <1481806283-10387-1-git-send-email-arybchenko@solarflare.com>
On 12/15/2016 12:50 PM, Andrew Rybchenko wrote:
> The patch series adds a number of features to Solarflare libefx-based
> PMD. Basically one patch per feature.
>
> The patches are grouped into one series since they touch nearby lines
> in either PMD feature list, or dev_ops structure, or documentation.
> So, patches cannot be applied in arbitrary order.
>
> ---
>
> v2:
> * Fix ICC and clang warnings
> * Slightly change sfc_tso_{alloc,free}_tsoh_objs() prototypes
>
>
> Andrew Rybchenko (17):
> net/sfc: implement MCDI logging callback
> net/sfc: support parameter to choose performance profile
> net/sfc: implement ethdev hook to get basic statistics
> net/sfc: support extended statistics
> net/sfc: support flow control settings get/set
> net/sfc: support link status change interrupt
> net/sfc: implement device operation to change MTU
> net/sfc: support link speed and duplex settings
> net/sfc: support checksum offloads on receive
> net/sfc: handle received packet type info provided by HW
> net/sfc: support callback to get receive queue information
> net/sfc: support Rx free threshold
> net/sfc: add callback to get RxQ pending descriptors count
> net/sfc: add RxQ descriptor done callback
> net/sfc: support scattered Rx DMA
> net/sfc: support deferred start of receive queues
> net/sfc/base: do not use enum type when values are bitmask
>
> Artem Andreev (1):
> net/sfc: support link up/down
>
> Ivan Malov (14):
> net/sfc: support promiscuous and all-multicast control
> net/sfc: support main (the first) MAC address change
> net/sfc: support multicast addresses list controls
> net/sfc: add callback to get transmit queue information
> net/sfc: support Tx free threshold
> net/sfc: support deferred start of transmit queues
> net/sfc: support VLAN offload on transmit path
> net/sfc: add basic stubs for RSS support on driver attach
> net/sfc: support RSS hash offload
> net/sfc: add callback to query RSS key and hash types config
> net/sfc: add callback to set RSS key and hash types config
> net/sfc: add callback to query RSS redirection table
> net/sfc: add callback to update RSS redirection table
> net/sfc: support firmware-assisted TSOv2
>
<...>
Series applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* [PATCH 2/2] ethdev: clarify xstats Api documentation
From: Olivier Matz @ 2016-12-16 9:44 UTC (permalink / raw)
To: dev, thomas.monjalon; +Cc: remy.horton, stable
In-Reply-To: <1481881454-17382-1-git-send-email-olivier.matz@6wind.com>
Reword the Api documentation of xstats ethdev.
CC: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_ether/rte_ethdev.h | 45 ++++++++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..4479553 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -938,23 +938,26 @@ struct rte_eth_txq_info {
/**
* An Ethernet device extended statistic structure
*
- * This structure is used by ethdev->eth_xstats_get() to provide
- * statistics that are not provided in the generic rte_eth_stats
+ * This structure is used by rte_eth_xstats_get() to provide
+ * statistics that are not provided in the generic *rte_eth_stats*
* structure.
+ * It maps a name id, corresponding to an index in the array returned
+ * by rte_eth_xstats_get_names(), to a statistic value.
*/
struct rte_eth_xstat {
- uint64_t id;
- uint64_t value;
+ uint64_t id; /**< The index in xstats name array. */
+ uint64_t value; /**< The statistic counter value. */
};
/**
- * A name-key lookup element for extended statistics.
+ * A name element for extended statistics.
*
- * This structure is used to map between names and ID numbers
- * for extended ethernet statistics.
+ * An array of this structure is returned by rte_eth_xstats_get_names().
+ * It lists the names of extended statistics for a PMD. The *rte_eth_xstat*
+ * structure references these names by their array index.
*/
struct rte_eth_xstat_name {
- char name[RTE_ETH_XSTATS_NAME_SIZE];
+ char name[RTE_ETH_XSTATS_NAME_SIZE]; /**< The statistic name. */
};
#define ETH_DCB_NUM_TCS 8
@@ -2272,18 +2275,19 @@ void rte_eth_stats_reset(uint8_t port_id);
* @param port_id
* The port identifier of the Ethernet device.
* @param xstats_names
- * Block of memory to insert names into. Must be at least size in capacity.
- * If set to NULL, function returns required capacity.
+ * An rte_eth_xstat_name array of at least *size* elements to
+ * be filled. If set to NULL, the function returns the required number
+ * of elements.
* @param size
- * Capacity of xstats_names (number of names).
+ * The size of the xstats_names array (number of elements).
* @return
- * - positive value lower or equal to size: success. The return value
+ * - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
- * - positive value higher than size: error, the given statistics table
+ * - A positive value higher than size: error, the given statistics table
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - negative value on error (invalid port id)
+ * - A negative value on error (invalid port id).
*/
int rte_eth_xstats_get_names(uint8_t port_id,
struct rte_eth_xstat_name *xstats_names,
@@ -2296,19 +2300,20 @@ int rte_eth_xstats_get_names(uint8_t port_id,
* The port identifier of the Ethernet device.
* @param xstats
* A pointer to a table of structure of type *rte_eth_xstat*
- * to be filled with device statistics ids and values.
+ * to be filled with device statistics ids and values: id is the
+ * index of the name string in xstats_names (@see rte_eth_xstats_get_names),
+ * and value is the statistic counter.
* This parameter can be set to NULL if n is 0.
* @param n
- * The size of the stats table, which should be large enough to store
- * all the statistics of the device.
+ * The size of the xstats array (number of elements).
* @return
- * - positive value lower or equal to n: success. The return value
+ * - A positive value lower or equal to n: success. The return value
* is the number of entries filled in the stats table.
- * - positive value higher than n: error, the given statistics table
+ * - A positive value higher than n: error, the given statistics table
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - negative value on error (invalid port id)
+ * - A negative value on error (invalid port id).
*/
int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
unsigned n);
--
2.8.1
^ permalink raw reply related
* [PATCH 1/2] ethdev: fix name index in xstats Api
From: Olivier Matz @ 2016-12-16 9:44 UTC (permalink / raw)
To: dev, thomas.monjalon; +Cc: remy.horton, stable
The function rte_eth_xstats_get() return an array of tuples (id,
value). The value is the statistic counter, while the id references a
name in the array returned by rte_eth_xstats_get_name().
Today, each 'id' returned by rte_eth_xstats_get() is equal to the index
in the returned array, making this value useless. It also prevents a
driver from having different indexes for names and value, like in the
example below:
rte_eth_xstats_get_name() returns:
0: "rx0_stat"
1: "rx1_stat"
2: ...
7: "rx7_stat"
8: "tx0_stat"
9: "tx1_stat"
...
15: "tx7_stat"
rte_eth_xstats_get() returns:
0: id=0, val=<stat> ("rx0_stat")
1: id=1, val=<stat> ("rx1_stat")
2: id=8, val=<stat> ("tx0_stat")
3: id=9, val=<stat> ("tx1_stat")
This patch fixes the drivers to set the 'id' in their ethdev->xstats_get()
(except e1000 which was already doing it), and fixes ethdev by not setting
the 'id' field to the index of the table for pmd-specific stats: instead,
they should just be shifted by the max number of generic statistics.
CC: stable@dpdk.org
Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer ids")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/bnx2x/bnx2x_ethdev.c | 1 +
drivers/net/fm10k/fm10k_ethdev.c | 3 +++
drivers/net/i40e/i40e_ethdev.c | 4 ++++
drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
drivers/net/qede/qede_ethdev.c | 2 ++
drivers/net/vhost/rte_eth_vhost.c | 2 ++
drivers/net/virtio/virtio_ethdev.c | 2 ++
lib/librte_ether/rte_ethdev.c | 5 ++++-
8 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 0f1e4a2..f01047f 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -422,6 +422,7 @@ bnx2x_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[num].value =
*(uint64_t *)((char *)&sc->eth_stats +
bnx2x_xstats_strings[num].offset_lo);
+ xstats[num].id = num;
}
return num;
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index fe74f6d..0d3098e 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1315,6 +1315,7 @@ fm10k_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < FM10K_NB_HW_XSTATS; i++) {
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
fm10k_hw_stats_strings[count].offset);
+ xstats[count].id = count;
count++;
}
@@ -1324,12 +1325,14 @@ fm10k_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[count].value =
*(uint64_t *)(((char *)&hw_stats->q[q]) +
fm10k_hw_stats_rx_q_strings[i].offset);
+ xstats[count].id = count;
count++;
}
for (i = 0; i < FM10K_NB_TX_Q_XSTATS; i++) {
xstats[count].value =
*(uint64_t *)(((char *)&hw_stats->q[q]) +
fm10k_hw_stats_tx_q_strings[i].offset);
+ xstats[count].id = count;
count++;
}
}
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index b0c0fbf..f01455e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2533,6 +2533,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < I40E_NB_ETH_XSTATS; i++) {
xstats[count].value = *(uint64_t *)(((char *)&hw_stats->eth) +
rte_i40e_stats_strings[i].offset);
+ xstats[count].id = count;
count++;
}
@@ -2540,6 +2541,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < I40E_NB_HW_PORT_XSTATS; i++) {
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
rte_i40e_hw_port_strings[i].offset);
+ xstats[count].id = count;
count++;
}
@@ -2549,6 +2551,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
*(uint64_t *)(((char *)hw_stats) +
rte_i40e_rxq_prio_strings[i].offset +
(sizeof(uint64_t) * prio));
+ xstats[count].id = count;
count++;
}
}
@@ -2559,6 +2562,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
*(uint64_t *)(((char *)hw_stats) +
rte_i40e_txq_prio_strings[i].offset +
(sizeof(uint64_t) * prio));
+ xstats[count].id = count;
count++;
}
}
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index baffc71..cf68fa9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2902,6 +2902,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
rte_ixgbe_stats_strings[i].offset);
+ xstats[count].id = count;
count++;
}
@@ -2911,6 +2912,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
rte_ixgbe_rxq_strings[stat].offset +
(sizeof(uint64_t) * i));
+ xstats[count].id = count;
count++;
}
}
@@ -2921,6 +2923,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
rte_ixgbe_txq_strings[stat].offset +
(sizeof(uint64_t) * i));
+ xstats[count].id = count;
count++;
}
}
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 001166a..6f3b10c 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -973,6 +973,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < RTE_DIM(qede_xstats_strings); i++) {
xstats[stat_idx].value = *(uint64_t *)(((char *)&stats) +
qede_xstats_strings[i].offset);
+ xstats[stat_idx].id = stat_idx;
stat_idx++;
}
@@ -982,6 +983,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[stat_idx].value = *(uint64_t *)(
((char *)(qdev->fp_array[(qid)].rxq)) +
qede_rxq_xstats_strings[i].offset);
+ xstats[stat_idx].id = stat_idx;
stat_idx++;
}
}
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 766d4ef..dd79ccf 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -324,6 +324,7 @@ vhost_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
*(uint64_t *)(((char *)vq)
+ vhost_rxport_stat_strings[t].offset);
}
+ xstats[count].id = count;
count++;
}
for (t = 0; t < VHOST_NB_XSTATS_TXPORT; t++) {
@@ -336,6 +337,7 @@ vhost_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
*(uint64_t *)(((char *)vq)
+ vhost_txport_stat_strings[t].offset);
}
+ xstats[count].id = count;
count++;
}
return count;
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 1bd60e9..f6335aa 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -893,6 +893,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (t = 0; t < VIRTIO_NB_RXQ_XSTATS; t++) {
xstats[count].value = *(uint64_t *)(((char *)rxvq) +
rte_virtio_rxq_stat_strings[t].offset);
+ xstats[count].id = count;
count++;
}
}
@@ -908,6 +909,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (t = 0; t < VIRTIO_NB_TXQ_XSTATS; t++) {
xstats[count].value = *(uint64_t *)(((char *)txvq) +
rte_virtio_txq_stat_strings[t].offset);
+ xstats[count].id = count;
count++;
}
}
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 1e0f206..45d8286 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1491,8 +1491,11 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
}
}
- for (i = 0; i < count + xcount; i++)
+ for (i = 0; i < count; i++)
xstats[i].id = i;
+ /* add an offset to driver-specific stats */
+ for ( ; i < count + xcount; i++)
+ xstats[i].id += count;
return count + xcount;
}
--
2.8.1
^ permalink raw reply related
* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Olivier Matz @ 2016-12-16 9:36 UTC (permalink / raw)
To: Ferruh Yigit
Cc: dev, nhorman, thomas.monjalon, vido, fiona.trahe, stephen,
adrien.mazarguil
In-Reply-To: <ba0a18d0-d4db-3c65-f11c-10465924082d@intel.com>
Hi Ferruh,
On Thu, 15 Dec 2016 14:52:02 +0000, Ferruh Yigit
<ferruh.yigit@intel.com> wrote:
> Hi Olivier, Thomas,
>
> On 12/15/2016 1:46 PM, Olivier Matz wrote:
> > Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver to
> > declare the list of kernel modules required to run properly.
> >
> > Today, most PCI drivers require uio/vfio.
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > Acked-by: Fiona Trahe <fiona.trahe@intel.com>
>
> This patch is for master branch, what do you think targeting it to
> next-net tree?
> So that new PMDs also can be included into patch?
That makes sense, I can rebase on next-net.
Thomas, do you agree?
From what I see, the 2 new PMDs are sfc (uio) and tap (nothing).
Regards,
Olivier
^ permalink raw reply
* Re: [PATCH v2 01/12] eal: define container_of macro
From: Adrien Mazarguil @ 2016-12-16 9:23 UTC (permalink / raw)
To: Jan Blunck
Cc: Shreyansh Jain, dev, David Marchand, Thomas Monjalon,
Ferruh Yigit, jianbo.liu, Jan Viktorin
In-Reply-To: <CALe+Z02=wKpjadYrA6JUHaoG5WH=MG7Y581fFeTry7K0mbBL1Q@mail.gmail.com>
On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
> >>
> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
> >> wrote:
> >>>
> >>> From: Jan Blunck <jblunck@infradead.org>
> >>>
> >>> This macro is based on Jan Viktorin's original patch but also checks the
> >>> type of the passed pointer against the type of the member.
> >>>
> >>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >>> [jblunck@infradead.org: add type checking and __extension__]
> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> >>>
> >>> --
> >>> v2:
> >>> - fix checkpatch error
> >>> ---
> >>> lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
> >>> 1 file changed, 21 insertions(+)
> >>>
> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
> >>> b/lib/librte_eal/common/include/rte_common.h
> >>> index db5ac91..3eb8d11 100644
> >>> --- a/lib/librte_eal/common/include/rte_common.h
> >>> +++ b/lib/librte_eal/common/include/rte_common.h
> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
> >>> #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
> >>> #endif
> >>>
> >>> +/**
> >>> + * Return pointer to the wrapping struct instance.
> >>> + *
> >>> + * Example:
> >>> + *
> >>> + * struct wrapper {
> >>> + * ...
> >>> + * struct child c;
> >>> + * ...
> >>> + * };
> >>> + *
> >>> + * struct child *x = obtain(...);
> >>> + * struct wrapper *w = container_of(x, struct wrapper, c);
> >>> + */
> >>> +#ifndef container_of
> >>> +#define container_of(ptr, type, member) (__extension__ ({
> >>> \
> >>> + typeof(((type *)0)->member) * _ptr = (ptr); \
> >>> + (type *)(((char *)_ptr) - offsetof(type,
> >>> member));\
> >>> + }))
> >>
> >>
> >> This is a checkpatch false positive. It should be fine to ignore this.
> >> IIRC we already discussed this before.
> >
> >
> > I too thought something similar was discussed. I tried searching the
> > archives but couldn't find anything - thus, I thought probably I was
> > hallucinating :P
> >
> > So, you want me to revert back the '()' change? Does it impact the expansion
> > of this macro?
>
> We haven't added this on any other usage of the __extension__ keyword
> in the existing code. From my perspective it is more consistent to
> revert it.
>
> Anyone else with an opinion here? David? Thomas?
As an exported header, rte_common.h must pass check-includes.sh. Both
typeof() and the ({ ... }) construct are non-standard GCC extensions and
would fail to compile with pedantic options.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Olivier Matz @ 2016-12-16 9:22 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Neil Horman, dev, thomas.monjalon, vido, fiona.trahe,
adrien.mazarguil
In-Reply-To: <20161215092207.168ba141@xeon-e3>
Hi,
On Thu, 15 Dec 2016 09:22:07 -0800, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Thu, 15 Dec 2016 11:09:12 -0500
> Neil Horman <nhorman@tuxdriver.com> wrote:
>
> > On Thu, Dec 15, 2016 at 02:46:39PM +0100, Olivier Matz wrote:
> > > Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver
> > > to declare the list of kernel modules required to run properly.
> > >
> > > Today, most PCI drivers require uio/vfio.
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> > > ---
> > >
> > > v2 -> v3:
> > > - fix kmods deps advertised by mellanox drivers as pointed out
> > > by Adrien
> > >
> > > v1 ->
> > > v2:
> > > - do not advertise uio_pci_generic for vf drivers
> > > - rebase on top of head: use new driver names and prefix
> > > macro with
> > > RTE_
> > >
> > > rfc -> v1:
> > > - the kmod information can be per-device using a modalias-like
> > > pattern
> > > - change syntax to use '&' and '|' instead of ',' and ':'
> > > - remove useless prerequisites in kmod lis: no need to
> > > specify both uio and uio_pci_generic, only the latter is
> > > required
> > > - update kmod list in szedata2 driver
> > > - remove kmod list in qat driver: it requires more than just
> > > loading a kmod, which is described in documentation
> > >
> > > buildtools/pmdinfogen/pmdinfogen.c | 1 +
> > > buildtools/pmdinfogen/pmdinfogen.h | 1 +
> > > drivers/net/bnx2x/bnx2x_ethdev.c | 2 ++
> > > drivers/net/bnxt/bnxt_ethdev.c | 1 +
> > > drivers/net/cxgbe/cxgbe_ethdev.c | 1 +
> > > drivers/net/e1000/em_ethdev.c | 1 +
> > > drivers/net/e1000/igb_ethdev.c | 2 ++
> > > drivers/net/ena/ena_ethdev.c | 1 +
> > > drivers/net/enic/enic_ethdev.c | 1 +
> > > drivers/net/fm10k/fm10k_ethdev.c | 1 +
> > > drivers/net/i40e/i40e_ethdev.c | 1 +
> > > drivers/net/i40e/i40e_ethdev_vf.c | 1 +
> > > drivers/net/ixgbe/ixgbe_ethdev.c | 2 ++
> > > drivers/net/mlx4/mlx4.c | 2 ++
> > > drivers/net/mlx5/mlx5.c | 1 +
> > > drivers/net/nfp/nfp_net.c | 1 +
> > > drivers/net/qede/qede_ethdev.c | 2 ++
> > > drivers/net/szedata2/rte_eth_szedata2.c | 2 ++
> > > drivers/net/thunderx/nicvf_ethdev.c | 1 +
> > > drivers/net/virtio/virtio_ethdev.c | 1 +
> > > drivers/net/vmxnet3/vmxnet3_ethdev.c | 1 +
> > > lib/librte_eal/common/include/rte_dev.h | 25
> > > +++++++++++++++++++++++++ tools/dpdk-pmdinfo.py
> > > | 5 ++++- 23 files changed, 56 insertions(+), 1 deletion(-)
> > >
> > Its odd that all devices, regardless of vendor should depend on the
> > igb_uio module. It seems to me that depending on uio_pci_generic
> > or vfio is sufficient.
igb_uio is the historical uio module of dpdk. Although it is called
igb_uio, it is not specific to Intel drivers.
Most drivers declare "igb_uio | uio_pci_generic | vfio", which means
that any of the 3 kernel modules can be used.
I think there are some cases where people will prefer using igb_uio,
for instance to use a vf pmd in a vm where there is no iommu,
and where the kernel vfio module does not support the no-iommu mode.
>
> Yes it seems just a special case extension for Mellanox drivers.
Kmod deps are different whether it's a vf driver or not.
Mellanox drivers are not the only drivers that do not require uio,
there is also szedata2.
Is it an argument for not including this patch?
Regards,
Olivier
^ permalink raw reply
* Re: [PATCH 12/22] app/testpmd: add rte_flow item spec handler
From: Adrien Mazarguil @ 2016-12-16 9:17 UTC (permalink / raw)
To: Pei, Yulong
Cc: dev@dpdk.org, Thomas Monjalon, De Lara Guarch, Pablo,
Olivier Matz, Xing, Beilei
In-Reply-To: <188971FCDA171749BED5DA74ABF3E6F03B6625B9@shsmsx102.ccr.corp.intel.com>
Hi Yulong,
On Fri, Dec 16, 2016 at 03:01:15AM +0000, Pei, Yulong wrote:
> Hi Adrien,
>
> I try to setup the following rule, but it seems that after set 'spec' param, can not set 'mask' param, is it an issue here or am I wrong to use it ?
>
> testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00
> dst [TOKEN]: destination MAC
> src [TOKEN]: source MAC
> type [TOKEN]: EtherType
> / [TOKEN]: specify next pattern item
You need to re-specify dst with "mask" instead of "spec". You can specify it
as many times you like to update each structure in turn, e.g.:
testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask 00:00:00:00:ff:ff
If you want to specify both spec and mask at once assuming you want it full,
these commands yield the same result:
testpmd> flow create 0 ingress pattern eth dst fix 00:00:00:00:09:00
testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask ff:ff:ff:ff:ff:ff
testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst prefix 48
You are even allowed to change your mind:
testpmd> flow create 0 ingress pattern eth dst fix 00:00:2a:2a:2a:2a dst fix 00:00:00:00:09:00
All these will be properly documented in the v2 patchset. Note, this version
will replace the "fix" keyword with "is" ("fix" made no sense according to
feedback).
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Adrien Mazarguil @ 2016-12-16 8:23 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, nhorman, thomas.monjalon, vido, fiona.trahe, stephen
In-Reply-To: <1481809599-27896-1-git-send-email-olivier.matz@6wind.com>
On Thu, Dec 15, 2016 at 02:46:39PM +0100, Olivier Matz wrote:
> Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver to
> declare the list of kernel modules required to run properly.
>
> Today, most PCI drivers require uio/vfio.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
>
> v2 -> v3:
> - fix kmods deps advertised by mellanox drivers as pointed out
> by Adrien
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH 00/22] Generic flow API (rte_flow)
From: Adrien Mazarguil @ 2016-12-16 8:22 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Thomas Monjalon, Pablo de Lara, Olivier Matz
In-Reply-To: <57fdc1d0-1d2b-da61-481b-358aed8d91a2@intel.com>
On Thu, Dec 15, 2016 at 12:20:36PM +0000, Ferruh Yigit wrote:
> On 12/8/2016 3:19 PM, Adrien Mazarguil wrote:
> > Hi Ferruh,
> >
> > On Fri, Dec 02, 2016 at 04:58:53PM +0000, Ferruh Yigit wrote:
> >> Hi Adrien,
> >>
> >> On 11/16/2016 4:23 PM, Adrien Mazarguil wrote:
> >>> As previously discussed in RFC v1 [1], RFC v2 [2], with changes
> >>> described in [3] (also pasted below), here is the first non-draft series
> >>> for this new API.
> >>>
> >>> Its capabilities are so generic that its name had to be vague, it may be
> >>> called "Generic flow API", "Generic flow interface" (possibly shortened
> >>> as "GFI") to refer to the name of the new filter type, or "rte_flow" from
> >>> the prefix used for its public symbols. I personally favor the latter.
> >>>
> >>> While it is currently meant to supersede existing filter types in order for
> >>> all PMDs to expose a common filtering/classification interface, it may
> >>> eventually evolve to cover the following ideas as well:
> >>>
> >>> - Rx/Tx offloads configuration through automatic offloads for specific
> >>> packets, e.g. performing checksum on TCP packets could be expressed with
> >>> an egress rule with a TCP pattern and a kind of checksum action.
> >>>
> >>> - RSS configuration (already defined actually). Could be global or per rule
> >>> depending on hardware capabilities.
> >>>
> >>> - Switching configuration for devices with many physical ports; rules doing
> >>> both ingress and egress could even be used to completely bypass software
> >>> if supported by hardware.
> >>>
> >>> [1] http://dpdk.org/ml/archives/dev/2016-July/043365.html
> >>> [2] http://dpdk.org/ml/archives/dev/2016-August/045383.html
> >>> [3] http://dpdk.org/ml/archives/dev/2016-November/050044.html
> >>>
> >>> Changes since RFC v2:
> >>>
> >>> - New separate VLAN pattern item (previously part of the ETH definition),
> >>> found to be much more convenient.
> >>>
> >>> - Removed useless "any" field from VF pattern item, the same effect can be
> >>> achieved by not providing a specification structure.
> >>>
> >>> - Replaced bit-fields from the VXLAN pattern item to avoid endianness
> >>> conversion issues on 24-bit fields.
> >>>
> >>> - Updated struct rte_flow_item with a new "last" field to create inclusive
> >>> ranges. They are defined as the interval between (spec & mask) and
> >>> (last & mask). All three parameters are optional.
> >>>
> >>> - Renamed ID action MARK.
> >>>
> >>> - Renamed "queue" fields in actions QUEUE and DUP to "index".
> >>>
> >>> - "rss_conf" field in RSS action is now const.
> >>>
> >>> - VF action now uses a 32 bit ID like its pattern item counterpart.
> >>>
> >>> - Removed redundant struct rte_flow_pattern, API functions now expect
> >>> struct
> >>> rte_flow_item lists terminated by END items.
> >>>
> >>> - Replaced struct rte_flow_actions for the same reason, with struct
> >>> rte_flow_action lists terminated by END actions.
> >>>
> >>> - Error types (enum rte_flow_error_type) have been updated and the cause
> >>> pointer in struct rte_flow_error is now const.
> >>>
> >>> - Function prototypes (rte_flow_create, rte_flow_validate) have also been
> >>> updated for clarity.
> >>>
> >>> Additions:
> >>>
> >>> - Public wrapper functions rte_flow_{validate|create|destroy|flush|query}
> >>> are now implemented in rte_flow.c, with their symbols exported and
> >>> versioned. Related filter type RTE_ETH_FILTER_GENERIC has been added.
> >>>
> >>> - A separate header (rte_flow_driver.h) has been added for driver-side
> >>> functionality, in particular struct rte_flow_ops which contains PMD
> >>> callbacks returned by RTE_ETH_FILTER_GENERIC query.
> >>>
> >>> - testpmd now exposes most of this API through the new "flow" command.
> >>>
> >>> What remains to be done:
> >>>
> >>> - Using endian-aware integer types (rte_beX_t) where necessary for clarity.
> >>>
> >>> - API documentation (based on RFC).
> >>>
> >>> - testpmd flow command documentation (although context-aware command
> >>> completion should already help quite a bit in this regard).
> >>>
> >>> - A few pattern item / action properties cannot be configured yet
> >>> (e.g. rss_conf parameter for RSS action) and a few completions
> >>> (e.g. possible queue IDs) should be added.
> >>>
> >>
> >> <...>
> >>
> >> I was trying to check driver filter API patches, but hit a few compiler
> >> errors with this patchset.
> >>
> >> [1] clang complains about variable bitfield value changed from -1 to 1.
> >> Which is correct, but I guess that is intentional, but I don't know how
> >> to tell this to clang?
> >>
> >> [2] shred library compilation error, because of missing rte_flow_flush
> >> in rte_ether_version.map file
> >>
> >> [3] bunch of icc compilation errors, almost all are same type:
> >> error #188: enumerated type mixed with another type
> >
> > Thanks for the report, I'll attempt to address them all in v2.
>
> Hi Adrien,
>
> I would like to remind that there are driver patch sets depends to this
> patch.
>
> New version of this patch should give some time to drivers to re-do (if
> required) the patchsets before integration deadline.
Hi Ferruh,
I intend to send v2 (including all the requested changes and fixes) shortly,
most likely today.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v2 01/12] eal: define container_of macro
From: Jan Blunck @ 2016-12-16 8:14 UTC (permalink / raw)
To: Shreyansh Jain
Cc: dev, David Marchand, Thomas Monjalon, Ferruh Yigit, jianbo.liu,
Jan Viktorin
In-Reply-To: <3310c320-fa39-cd8c-ab77-ced20daa5073@nxp.com>
On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
>>
>> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
>> wrote:
>>>
>>> From: Jan Blunck <jblunck@infradead.org>
>>>
>>> This macro is based on Jan Viktorin's original patch but also checks the
>>> type of the passed pointer against the type of the member.
>>>
>>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> [jblunck@infradead.org: add type checking and __extension__]
>>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>>>
>>> --
>>> v2:
>>> - fix checkpatch error
>>> ---
>>> lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>>
>>> diff --git a/lib/librte_eal/common/include/rte_common.h
>>> b/lib/librte_eal/common/include/rte_common.h
>>> index db5ac91..3eb8d11 100644
>>> --- a/lib/librte_eal/common/include/rte_common.h
>>> +++ b/lib/librte_eal/common/include/rte_common.h
>>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
>>> #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
>>> #endif
>>>
>>> +/**
>>> + * Return pointer to the wrapping struct instance.
>>> + *
>>> + * Example:
>>> + *
>>> + * struct wrapper {
>>> + * ...
>>> + * struct child c;
>>> + * ...
>>> + * };
>>> + *
>>> + * struct child *x = obtain(...);
>>> + * struct wrapper *w = container_of(x, struct wrapper, c);
>>> + */
>>> +#ifndef container_of
>>> +#define container_of(ptr, type, member) (__extension__ ({
>>> \
>>> + typeof(((type *)0)->member) * _ptr = (ptr); \
>>> + (type *)(((char *)_ptr) - offsetof(type,
>>> member));\
>>> + }))
>>
>>
>> This is a checkpatch false positive. It should be fine to ignore this.
>> IIRC we already discussed this before.
>
>
> I too thought something similar was discussed. I tried searching the
> archives but couldn't find anything - thus, I thought probably I was
> hallucinating :P
>
> So, you want me to revert back the '()' change? Does it impact the expansion
> of this macro?
We haven't added this on any other usage of the __extension__ keyword
in the existing code. From my perspective it is more consistent to
revert it.
Anyone else with an opinion here? David? Thomas?
>
>>
>>
>>> +#endif
>>> +
>>> #define _RTE_STR(x) #x
>>> /** Take a macro value and get a string version of it */
>>> #define RTE_STR(x) _RTE_STR(x)
>>> --
>>> 2.7.4
>>>
>>
>
^ permalink raw reply
* Re: [PATCH 23/28] net/ixgbe: use eal I/O device memory read/write API
From: Santosh Shukla @ 2016-12-16 4:40 UTC (permalink / raw)
To: Jianbo Liu
Cc: Jerin Jacob, dev, Ananyev, Konstantin, Thomas Monjalon,
Bruce Richardson, Jan Viktorin, Helin Zhang
In-Reply-To: <CAP4Qi38Bp6oL7uQcOoUqFHk53ARS3Yd+RKw4w1XeHUC+aveEMw@mail.gmail.com>
On Thu, Dec 15, 2016 at 04:37:12PM +0800, Jianbo Liu wrote:
> On 14 December 2016 at 09:55, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > From: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> >
> > Replace the raw I/O device memory read/write access with eal
> > abstraction for I/O device memory read/write access to fix
> > portability issues across different architectures.
> >
> > Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > CC: Helin Zhang <helin.zhang@intel.com>
> > CC: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > drivers/net/ixgbe/base/ixgbe_osdep.h | 13 +++++++++----
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_osdep.h b/drivers/net/ixgbe/base/ixgbe_osdep.h
> > index 77f0af5..9d16c21 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_osdep.h
> > +++ b/drivers/net/ixgbe/base/ixgbe_osdep.h
> > @@ -44,6 +44,7 @@
> > #include <rte_cycles.h>
> > #include <rte_log.h>
> > #include <rte_byteorder.h>
> > +#include <rte_io.h>
> >
> > #include "../ixgbe_logs.h"
> > #include "../ixgbe_bypass_defines.h"
> > @@ -121,16 +122,20 @@ typedef int bool;
> >
> > #define prefetch(x) rte_prefetch0(x)
> >
> > -#define IXGBE_PCI_REG(reg) (*((volatile uint32_t *)(reg)))
> > +#define IXGBE_PCI_REG(reg) ({ \
> > + uint32_t __val; \
> > + __val = rte_readl(reg); \
> > + __val; \
> > +})
> >
> > static inline uint32_t ixgbe_read_addr(volatile void* addr)
> > {
> > return rte_le_to_cpu_32(IXGBE_PCI_REG(addr));
> > }
> >
> > -#define IXGBE_PCI_REG_WRITE(reg, value) do { \
> > - IXGBE_PCI_REG((reg)) = (rte_cpu_to_le_32(value)); \
> > -} while(0)
> > +#define IXGBE_PCI_REG_WRITE(reg, value) ({ \
> > + rte_writel(rte_cpu_to_le_32(value), reg); \
> > +})
> >
>
> memory barrier operation is put inside IXGBE_PCI_REG_READ/WRITE in
> your change, but I found rte_*mb is called before these macros in some
> places.
> Can you remove all these redundant calls? And please do the same
> checking for other drivers.
>
Ok.
Thinking of adding _relaxed_rd/wr style macro agnostic to arch for ixgbe case
in particular. Such that for those code incident:
x86 case> first default barrier + relaxed call.
arm case> first default barrier + relaxed call.
Does that make sense to you? If so then will take care in v2.
Santosh.
> > #define IXGBE_PCI_REG_ADDR(hw, reg) \
> > ((volatile uint32_t *)((char *)(hw)->hw_addr + (reg)))
> > --
> > 2.5.5
> >
^ permalink raw reply
* [PATCH v3 29/29] net/i40e: set/clear VF stats from PF
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Qi Zhang
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
This patch add support to get/clear VF statistics
from PF side.
Two APIs are added:
rte_pmd_i40e_get_vf_stats.
rte_pmd_i40e_reset_vf_stats.
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 76 +++++++++++++++++++++++++++++++
drivers/net/i40e/rte_pmd_i40e.h | 41 +++++++++++++++++
drivers/net/i40e/rte_pmd_i40e_version.map | 2 +
3 files changed, 119 insertions(+)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5ee010e..080eff2 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -10462,3 +10462,79 @@ int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
return ret;
}
+
+int
+rte_pmd_i40e_get_vf_stats(uint8_t port,
+ uint16_t vf_id,
+ struct rte_eth_stats *stats)
+{
+ struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info;
+ struct i40e_pf *pf;
+ struct i40e_vsi *vsi;
+ int ret = 0;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ dev = &rte_eth_devices[port];
+ rte_eth_dev_info_get(port, &dev_info);
+
+ if (vf_id >= dev_info.max_vfs)
+ return -EINVAL;
+
+ pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+ if (vf_id > pf->vf_num - 1 || !pf->vfs) {
+ PMD_DRV_LOG(ERR, "Invalid argument.");
+ return -EINVAL;
+ }
+
+ vsi = pf->vfs[vf_id].vsi;
+ i40e_update_vsi_stats(vsi);
+
+ stats->ipackets = vsi->eth_stats.rx_unicast +
+ vsi->eth_stats.rx_multicast +
+ vsi->eth_stats.rx_broadcast;
+ stats->opackets = vsi->eth_stats.tx_unicast +
+ vsi->eth_stats.tx_multicast +
+ vsi->eth_stats.tx_broadcast;
+ stats->ibytes = vsi->eth_stats.rx_bytes;
+ stats->obytes = vsi->eth_stats.tx_bytes;
+ stats->ierrors = vsi->eth_stats.rx_discards;
+ stats->oerrors = vsi->eth_stats.tx_errors + vsi->eth_stats.tx_discards;
+
+ return ret;
+}
+
+int
+rte_pmd_i40e_reset_vf_stats(uint8_t port,
+ uint16_t vf_id)
+{
+ struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info;
+ struct i40e_pf *pf;
+ struct i40e_vsi *vsi;
+ int ret = 0;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ dev = &rte_eth_devices[port];
+ rte_eth_dev_info_get(port, &dev_info);
+
+ if (vf_id >= dev_info.max_vfs)
+ return -EINVAL;
+
+ pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+ if (vf_id > pf->vf_num - 1 || !pf->vfs) {
+ PMD_DRV_LOG(ERR, "Invalid argument.");
+ return -EINVAL;
+ }
+
+ vsi = pf->vfs[vf_id].vsi;
+
+ vsi->offset_loaded = false;
+ i40e_update_vsi_stats(vsi);
+
+ return ret;
+}
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index 96cda89..2bda36d 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -284,4 +284,45 @@ int rte_pmd_i40e_set_vf_vlan_tag(uint8_t port, uint16_t vf_id, uint8_t on);
int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
uint64_t vf_mask, uint8_t on);
+/**
+ * Get VF's statistics
+ *
+ * @param port
+ * The port identifier of the Ethernet device.
+ * @param vf_id
+ * VF on which to get.
+ * @param stats
+ * A pointer to a structure of type *rte_eth_stats* to be filled with
+ * the values of device counters for the following set of statistics:
+ * - *ipackets* with the total of successfully received packets.
+ * - *opackets* with the total of successfully transmitted packets.
+ * - *ibytes* with the total of successfully received bytes.
+ * - *obytes* with the total of successfully transmitted bytes.
+ * - *ierrors* with the total of erroneous received packets.
+ * - *oerrors* with the total of failed transmitted packets.
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if *port* invalid.
+ * - (-EINVAL) if bad parameter.
+ */
+
+int rte_pmd_i40e_get_vf_stats(uint8_t port,
+ uint16_t vf_id,
+ struct rte_eth_stats *stats);
+
+/**
+ * Clear VF's statistics
+ *
+ * @param port
+ * The port identifier of the Ethernet device.
+ * @param vf_id
+ * VF on which to get.
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if *port* invalid.
+ * - (-EINVAL) if bad parameter.
+ */
+int rte_pmd_i40e_reset_vf_stats(uint8_t port,
+ uint16_t vf_id);
+
#endif /* _PMD_I40E_H_ */
diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
index 818ff9c..e4dff0b 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -18,5 +18,7 @@ DPDK_17.02 {
rte_pmd_i40e_set_vf_broadcast;
rte_pmd_i40e_set_vf_vlan_tag;
rte_pmd_i40e_set_vf_vlan_filter;
+ rte_pmd_i40e_get_vf_stats;
+ rte_pmd_i40e_reset_vf_stats;
} DPDK_2.0;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox