[dpdk-dev] [PATCH] mk: remove uio suffix from virtio pmd

[Ouyang, Changchun]

Hi Thomas,

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, April 10, 2015 3:48 PM
> To: Ouyang, Changchun
> Cc: dev at dpdk.org
> Subject: [PATCH] mk: remove uio suffix from virtio pmd
> 
> The virtio pmd is not restricted to uio anymore.
> 
> Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")
> 
> Signed-off-by: Thomas Monjalon 

Acked-by: Changchun Ouyang 

[dpdk-dev] [PATCH] doc: fix vhost guide

[Ouyang, Changchun]

Hi Igor,

Good catch, comments as below.

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Igor Ryzhov
> Sent: Thursday, April 9, 2015 12:31 AM
> To: dev at dpdk.org
> Cc: Igor Ryzhov
> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide
> 
> Guide says that a configure parameter to choose between vhost cuse and
> vhost user will be introduced in the future, but it?s already added by commit
> 28a1ccca41bf.
> 
> Signed-off-by: Igor Ryzhov 
> ---
>  doc/guides/sample_app_ug/vhost.rst | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/guides/sample_app_ug/vhost.rst
> b/doc/guides/sample_app_ug/vhost.rst
> index 8a7eb3b..df8cd8c 100644
> --- a/doc/guides/sample_app_ug/vhost.rst
> +++ b/doc/guides/sample_app_ug/vhost.rst
> @@ -309,13 +309,12 @@ Compiling the Sample Code
> 
>  CONFIG_RTE_LIBRTE_VHOST=n
> 
> -vhost user is turned on by default in the lib/librte_vhost/Makefile.
> -To enable vhost cuse, uncomment vhost cuse and comment vhost user
> manually. In future, a configure will be created for switch between two
> implementations.
> +vhost user is turned on by default in the configure file
> config/common_linuxapp.
> +To enable vhost cuse, disable vhost user.
> 
>  .. code-block:: console
> 
> -SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c
> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
> -#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c
> vhost_user/virtio-net-user.c vhost_user/fd_man.c
> +CONFIG_RTE_LIBRTE_VHOST_USER=y

If it wants to guide user how to enable vhost cuse, then I think
It makes sense to change it into:  CONFIG_RTE_LIBRTE_VHOST_USER=n

Thanks
Changchun

[dpdk-dev] [PATCH] doc: fix vhost guide

[Igor Ryzhov]

Sorry, I used wrong email address to reply from. This one is correct.

On Mon, Apr 13, 2015 at 10:11 AM, Igor Ryzhov  wrote:

> Hello, Changchun.
>
> Previous paragraph says ?To enable vhost, turn on vhost library in the
> configure file config/common_linuxapp?, but string in a code-block is
> ?CONFIG_RTE_LIBRTE_VHOST=n?. I thought that idea is to use the default
> string from the config file that user have to change, not already changed
> string. So I used the same style.
>
> Regards,
> Igor
>
> 13 ???. 2015 ?., ? 7:52, Ouyang, Changchun 
> ???(?):
>
> Hi Igor,
>
> Good catch, comments as below.
>
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org ] On Behalf
> Of Igor Ryzhov
> Sent: Thursday, April 9, 2015 12:31 AM
> To: dev at dpdk.org
> Cc: Igor Ryzhov
> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide
>
> Guide says that a configure parameter to choose between vhost cuse and
> vhost user will be introduced in the future, but it?s already added by
> commit
> 28a1ccca41bf.
>
> Signed-off-by: Igor Ryzhov 
> ---
> doc/guides/sample_app_ug/vhost.rst | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/doc/guides/sample_app_ug/vhost.rst
> b/doc/guides/sample_app_ug/vhost.rst
> index 8a7eb3b..df8cd8c 100644
> --- a/doc/guides/sample_app_ug/vhost.rst
> +++ b/doc/guides/sample_app_ug/vhost.rst
> @@ -309,13 +309,12 @@ Compiling the Sample Code
>
> CONFIG_RTE_LIBRTE_VHOST=n
>
> -vhost user is turned on by default in the lib/librte_vhost/Makefile.
> -To enable vhost cuse, uncomment vhost cuse and comment vhost user
> manually. In future, a configure will be created for switch between two
> implementations.
> +vhost user is turned on by default in the configure file
> config/common_linuxapp.
> +To enable vhost cuse, disable vhost user.
>
> .. code-block:: console
>
> -SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c
> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
> -#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c
> vhost_user/virtio-net-user.c vhost_user/fd_man.c
> +CONFIG_RTE_LIBRTE_VHOST_USER=y
>
>
> If it wants to guide user how to enable vhost cuse, then I think
> It makes sense to change it into:  CONFIG_RTE_LIBRTE_VHOST_USER=n
>
> Thanks
> Changchun
>
>
>

[dpdk-dev] [PATCH] doc: fix vhost guide

[Igor Ryzhov]

Hello, Changchun.

Previous paragraph says ?To enable vhost, turn on vhost library in the 
configure file config/common_linuxapp?, but string in a code-block is 
?CONFIG_RTE_LIBRTE_VHOST=n?. I thought that idea is to use the default string 
from the config file that user have to change, not already changed string. So I 
used the same style.

Regards,
Igor

> 13 ???. 2015 ?., ? 7:52, Ouyang, Changchun  
> ???(?):
> 
> Hi Igor,
> 
> Good catch, comments as below.
> 
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Igor Ryzhov
>> Sent: Thursday, April 9, 2015 12:31 AM
>> To: dev at dpdk.org
>> Cc: Igor Ryzhov
>> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide
>> 
>> Guide says that a configure parameter to choose between vhost cuse and
>> vhost user will be introduced in the future, but it?s already added by commit
>> 28a1ccca41bf.
>> 
>> Signed-off-by: Igor Ryzhov 
>> ---
>> doc/guides/sample_app_ug/vhost.rst | 7 +++
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>> 
>> diff --git a/doc/guides/sample_app_ug/vhost.rst
>> b/doc/guides/sample_app_ug/vhost.rst
>> index 8a7eb3b..df8cd8c 100644
>> --- a/doc/guides/sample_app_ug/vhost.rst
>> +++ b/doc/guides/sample_app_ug/vhost.rst
>> @@ -309,13 +309,12 @@ Compiling the Sample Code
>> 
>> CONFIG_RTE_LIBRTE_VHOST=n
>> 
>> -vhost user is turned on by default in the lib/librte_vhost/Makefile.
>> -To enable vhost cuse, uncomment vhost cuse and comment vhost user
>> manually. In future, a configure will be created for switch between two
>> implementations.
>> +vhost user is turned on by default in the configure file
>> config/common_linuxapp.
>> +To enable vhost cuse, disable vhost user.
>> 
>> .. code-block:: console
>> 
>> -SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c
>> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
>> -#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c
>> vhost_user/virtio-net-user.c vhost_user/fd_man.c
>> +CONFIG_RTE_LIBRTE_VHOST_USER=y
> 
> If it wants to guide user how to enable vhost cuse, then I think
> It makes sense to change it into:  CONFIG_RTE_LIBRTE_VHOST_USER=n
> 
> Thanks
> Changchun

[dpdk-dev] Crash related to virtio NICs in DPDK 2.0.0 on Freebsd 10.1 VM

[Raz Amir]

Any feedback?



From: Raz Amir [mailto:razami...@gmail.com] 
Sent: 08 April 2015 18:53
To: dev at dpdk.org; 'Thomas Monjalon'; david.marchand at 6wind.com
Subject: RE: Crash related to virtio NICs in DPDK 2.0.0 on Freebsd 10.1 VM



I found the source of the problem.

The issues happens also in dpdk 1.8.0, and related to patch
http://dpdk.org/dev/patchwork/patch/239/

Adding Thomas and David to the thread and I will appreciate your input.



The patch comes to solve a file descriptor leak in the bsdapp version of
rte_eal_iopl_init after opening the /dev/io device.

Seems like this isn't a file descriptor leak, and it should remain open - as
I wrote below, I am using virtio.

After removing it and testing the crash was resolved.



Any objection for removing the close(fd) that was added at dpdk 1.8.0?

Are there scenarios that might be impacted by removing it?



From: Raz Amir [mailto:razami...@gmail.com] 
Sent: 07 April 2015 18:41
To: dev at dpdk.org  
Subject: Crash related to virtio NICs in DPDK 2.0.0 on Freebsd 10.1 VM



I am moving from dpdk 1.7.1 to 2.0.0 and I am experiencing a crash in any
dpdk application, when rte_eal_init initializes the virtio NICs.

I have Ubuntu 14.04.02, running a Freebsd 10.1 VM with 3 virtio NICs over
qemu with kvm.

I run testpmd (for example) inside the Freebsd VM and it crashes (more info
below).

On the same setup, testpmd from dpdk 1.7.1 works without an issue.

Can you tell if you know that a Freebsd VM with virtio NICs is working for
dpdk 2.0.0?



Additional information from gdb:



user@:/dpdk/dpdk-2.0.0   # gdb testpmd

GNU gdb (GDB) 7.8.1 [GDB v7.8.1 for FreeBSD]

Copyright (C) 2014 Free Software Foundation, Inc.

License GPLv3+: GNU GPL version 3 or later


This is free software: you are free to change and redistribute it.

There is NO WARRANTY, to the extent permitted by law.  Type "show copying"

and "show warranty" for details.

This GDB was configured as "x86_64-portbld-freebsd10.1".

Type "show configuration" for configuration details.

For bug reporting instructions, please see:

.

Find the GDB manual and other documentation resources online at:

.

For help, type "help".

Type "apropos word" to search for commands related to "word"...

Reading symbols from testpmd...done.





(gdb) r -c 3 -n 2

Starting program: /root/testpmd -c 3 -n 2

EAL: Sysctl reports 2 cpus

EAL: Detected lcore 0

EAL: Detected lcore 1

EAL: Support maximum 128 logical core(s) by configuration.

EAL: Detected 2 lcore(s)

EAL: Contigmem driver has 2 buffers, each of size 64MB

EAL: Setting up physically contiguous memory...

EAL: Mapped memory segment 1 @ 0x80280: physaddr:0x1000, len
67108864

EAL: Mapped memory segment 2 @ 0x80680: physaddr:0x1400, len
67108864

EAL: TSC is not safe to use in SMP mode

EAL: TSC is not invariant

EAL: TSC frequency is ~2933513 KHz

EAL: PCI scan found 8 devices

EAL: Master lcore 0 is ready (tid=0x802406400;cpuset=[0])

PMD: ENICPMD trace: rte_enic_pmd_init

EAL: lcore 1 is ready (tid=0x802406800;cpuset=[1])

EAL: PCI device :00:03.0 on NUMA socket 0

EAL:   probe driver: 1af4:1000 rte_virtio_pmd

[New Thread 802406400 (LWP 100111)]



Program received signal SIGBUS, Bus error.

[Switching to Thread 802406400 (LWP 100111)]

0x00492f2c in outb (port=49170, data=0 '\000') at
/usr/include/machine/cpufunc.h:244

244 /usr/include/machine/cpufunc.h: No such file or directory.





(gdb) bt

#0  0x00492f2c in outb (port=49170, data=0 '\000')

at /usr/include/machine/cpufunc.h:244

#1  0x00492f7a in outb_p (data=0 '\000', port=49170)

at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211

#2  0x0049328d in vtpci_set_status (hw=0x80331f380, status=0 '\000')

at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130

#3  0x004931fe in vtpci_reset (hw=0x80331f380)

at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108

#4  0x004a175e in eth_virtio_dev_init (eth_dev=0x831b80
)

at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150

#5  0x00462c09 in rte_eth_dev_init (pci_drv=0x79d1a0
,

pci_dev=0x802417560) at
/dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326

#6  0x0046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0
,

dev=0x802417560) at
/dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487

#7  0x00475b06 in pci_probe_all_drivers (dev=0x802417560)

at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116

#8  0x00475bb9 in rte_eal_pci_probe ()

at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246

#9  0x0046cd63 in rte_eal_init (argc=5, argv=0x7fffeaf0)

at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554

#10 0x00404544 in main ()





(gdb) disassemble inb

Dump of assembl

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Gonzalez Monroy, Sergio]

On 09/04/2015 21:34, Neil Horman wrote:
> On Thu, Apr 09, 2015 at 08:00:26PM +0300, Avi Kivity wrote:
>> On 04/09/2015 02:19 PM, Neil Horman wrote:
>>> On Thu, Apr 09, 2015 at 12:06:47PM +0300, Avi Kivity wrote:
 On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
> On 08/04/2015 19:26, Stephen Hemminger wrote:
>> On Wed,  8 Apr 2015 16:07:21 +0100
>> Sergio Gonzalez Monroy  wrote:
>>
>>> Currently, the target/rules to build combined libraries is different
>>> than the one to build individual libraries.
>>>
>>> By removing the combined library option as a build configuration option
>>> we simplify the build pocess by having a single point for
>>> linking/archiving
>>> libraries in DPDK.
>>>
>>> This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option and
>>> removes the makefiles associated with building a combined library.
>>>
>>> The CONFIG_RTE_LIBNAME config option is kept as it will be use to
>>> always generate a linker script that acts as a single combined library.
>>>
>>> Signed-off-by: Sergio Gonzalez Monroy
>>> 
>> No. We use combined library and it greatly simplfies the application
>> linking process.
>>
> After all the opposition this patch had in v2, I did explain the current
> issues
> (see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this was
> the agreed solution.
>
> As I mention in the cover letter (also see patch 2/5), building DPDK
> (after applying this patch series) will always generate a very simple
> linker script that behaves as a combined library.
> I encourage you to apply this patch series and try to build your app
> (which links against combined lib).
> Your app should build without problem unless I messed up somewhere and it
> needs fixing.
 Is it possible to generate a pkgconfig file (dpdk.pc) that contains all of
 the setting needed to compile and link with dpdk?  That will greatly
 simplify usage.

 A linker script is just too esoteric.

>>> Why esoteric?  We're not talking about a linker script in the sense of a 
>>> binary
>>> layout file, we're talking about a prewritten/generated libdpdk_core.so that
>>> contains linker directives to include the appropriate libraries.  You link 
>>> it
>>> just like you do any other library, but it lets you ignore how they are 
>>> broken
>>> up.
>> You mean DT_NEEDED?  That's great, but it shouldn't be called a linker
>> script.
>>
> no, I don't mean DT_NEEDED, I mean a linker script, because thats what what
> sergio wrote is.
>
>>> We could certainly do a pkg-config file, but I don't think thats any more
>>> adventageous than this solution.
>> It solves more problems -- cflags etc.  Of course having the right DT_NEEDED
>> is a good thing regardless.
>>
> Thats a good point, pkgconfig doesn't provide that additionally.  Perhaps 
> having
> both is the best solution.  As for the DT_NEEDED issues, the earlier threads
> ennumerated all the problems that were being found with the way the libraries
> were organized (circular dependencies).
>
> Neil
I am not entirely sure of the conclusion of this thread.

Are we happy with the current linker script solution as a replacement of 
the combined lib?
Do we want to provide pkg-config file in addition or instead of linker 
script?

I think I will split the series as there seems to be no objections to 
the patches related to DT_NEEDED entries.
I'll post a series for DT_NEEDED entries and another series for dealing 
with the combined lib (once we get to an agreement).

Does it sound reasonable?

Sergio

[dpdk-dev] [PATCH] enic: disable debug traces

[Adrien Mazarguil]

On Thu, Apr 09, 2015 at 09:28:53AM -0700, Stephen Hemminger wrote:
> On Thu, 9 Apr 2015 10:32:24 +0200
> Adrien Mazarguil  wrote:
> 
> > >  
> > > +#ifdef RTE_LIBRTE_ENIC_DEBUG
> > >  #define ENICPMD_FUNC_TRACE() \
> > >   RTE_LOG(DEBUG, PMD, "ENICPMD trace: %s\n", __func__)
> > > +#else
> > > +#define ENICPMD_FUNC_TRACE() do {} while (0)  
> > 
> > How about defining it as (void)0 instead of an empty do/while block?
> > 
> > Doing so will prevent warnings if this macro happens to be used in an
> > expression. RTE_LOG() supports it.
> 
> I kind of like the Linux printk trick since it then preserves the format 
> checking
> even if compiled out.
> 
> /*
>  * Dummy printk for disabled debugging statements to use whilst maintaining
>  * gcc's format and side-effect checking.
>  */
> static inline __printf(1, 2)
> int no_printk(const char *fmt, ...)
> {
>   return 0;
> }
> 
> /* pr_devel() should produce zero code unless DEBUG is defined */
> #ifdef DEBUG
> #define pr_devel(fmt, ...) \
>   printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> #define pr_devel(fmt, ...) \
>   no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #endif

My only concern with this is that it cannot be done in a ISO C compliant
manner easily.

While "__printf()" can be defined to nothing when dealing with compilers
that do not support extensions, ", ## __VAR_ARGS__" will most likely fail
with -pedantic.

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] Crash related to virtio NICs in DPDK 2.0.0 on Freebsd 10.1 VM

[Thomas Monjalon]

2015-04-08 18:53, Raz Amir:
> The issues happens also in dpdk 1.8.0, and related to patch
> http://dpdk.org/dev/patchwork/patch/239/
> 
> Adding Thomas and David to the thread and I will appreciate your input.
> 
> The patch comes to solve a file descriptor leak in the bsdapp version of
> rte_eal_iopl_init after opening the /dev/io device.
> 
> Seems like this isn't a file descriptor leak, and it should remain open - as
> I wrote below, I am using virtio.

Thanks for the bug report.
It seems there was no validation for FreeBSD with virtio.

> After removing it and testing the crash was resolved.
> 
> Any objection for removing the close(fd) that was added at dpdk 1.8.0?

No, there was a doubt because the man page was not clear.
http://www.freebsd.org/cgi/man.cgi?query=io&sektion=4

In case you submit a patch, please add this line:
Fixes: 8a312224bcde ("eal/bsd: fix fd leak")

> Are there scenarios that might be impacted by removing it?

I don't think so.

Thanks

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Thomas Monjalon]

2015-04-13 10:52, Gonzalez Monroy, Sergio:
> On 09/04/2015 21:34, Neil Horman wrote:
> > On Thu, Apr 09, 2015 at 08:00:26PM +0300, Avi Kivity wrote:
> >> On 04/09/2015 02:19 PM, Neil Horman wrote:
> >>> On Thu, Apr 09, 2015 at 12:06:47PM +0300, Avi Kivity wrote:
>  On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
> > On 08/04/2015 19:26, Stephen Hemminger wrote:
> >> On Wed,  8 Apr 2015 16:07:21 +0100
> >> Sergio Gonzalez Monroy  wrote:
> >>
> >>> Currently, the target/rules to build combined libraries is different
> >>> than the one to build individual libraries.
> >>>
> >>> By removing the combined library option as a build configuration 
> >>> option
> >>> we simplify the build pocess by having a single point for
> >>> linking/archiving
> >>> libraries in DPDK.
> >>>
> >>> This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option 
> >>> and
> >>> removes the makefiles associated with building a combined library.
> >>>
> >>> The CONFIG_RTE_LIBNAME config option is kept as it will be use to
> >>> always generate a linker script that acts as a single combined 
> >>> library.
> >>>
> >>> Signed-off-by: Sergio Gonzalez Monroy
> >>> 
> >> No. We use combined library and it greatly simplfies the application
> >> linking process.
> >>
> > After all the opposition this patch had in v2, I did explain the current
> > issues
> > (see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and this 
> > was
> > the agreed solution.
> >
> > As I mention in the cover letter (also see patch 2/5), building DPDK
> > (after applying this patch series) will always generate a very simple
> > linker script that behaves as a combined library.
> > I encourage you to apply this patch series and try to build your app
> > (which links against combined lib).
> > Your app should build without problem unless I messed up somewhere and 
> > it
> > needs fixing.
>  Is it possible to generate a pkgconfig file (dpdk.pc) that contains all 
>  of
>  the setting needed to compile and link with dpdk?  That will greatly
>  simplify usage.
> 
>  A linker script is just too esoteric.
> 
> >>> Why esoteric?  We're not talking about a linker script in the sense of a 
> >>> binary
> >>> layout file, we're talking about a prewritten/generated libdpdk_core.so 
> >>> that
> >>> contains linker directives to include the appropriate libraries.  You 
> >>> link it
> >>> just like you do any other library, but it lets you ignore how they are 
> >>> broken
> >>> up.
> >> You mean DT_NEEDED?  That's great, but it shouldn't be called a linker
> >> script.
> >>
> > no, I don't mean DT_NEEDED, I mean a linker script, because thats what what
> > sergio wrote is.
> >
> >>> We could certainly do a pkg-config file, but I don't think thats any more
> >>> adventageous than this solution.
> >> It solves more problems -- cflags etc.  Of course having the right 
> >> DT_NEEDED
> >> is a good thing regardless.
> >>
> > Thats a good point, pkgconfig doesn't provide that additionally.  Perhaps 
> > having
> > both is the best solution.  As for the DT_NEEDED issues, the earlier threads
> > ennumerated all the problems that were being found with the way the 
> > libraries
> > were organized (circular dependencies).
> >
> > Neil
> I am not entirely sure of the conclusion of this thread.
> 
> Are we happy with the current linker script solution as a replacement of 
> the combined lib?
> Do we want to provide pkg-config file in addition or instead of linker 
> script?

Yes pkg-config should be an addition on top of shared/static split/combined
libraries (or linker script). It should be an alternative to mk/rte.app.mk.

> I think I will split the series as there seems to be no objections to 
> the patches related to DT_NEEDED entries.
> I'll post a series for DT_NEEDED entries and another series for dealing 
> with the combined lib (once we get to an agreement).
> 
> Does it sound reasonable?

Yes good idea, thanks.

[dpdk-dev] [PATCH v3 1/5] mk: remove combined library and related options

[Neil Horman]

On Mon, Apr 13, 2015 at 01:04:52PM +0200, Thomas Monjalon wrote:
> 2015-04-13 10:52, Gonzalez Monroy, Sergio:
> > On 09/04/2015 21:34, Neil Horman wrote:
> > > On Thu, Apr 09, 2015 at 08:00:26PM +0300, Avi Kivity wrote:
> > >> On 04/09/2015 02:19 PM, Neil Horman wrote:
> > >>> On Thu, Apr 09, 2015 at 12:06:47PM +0300, Avi Kivity wrote:
> >  On 04/09/2015 11:33 AM, Gonzalez Monroy, Sergio wrote:
> > > On 08/04/2015 19:26, Stephen Hemminger wrote:
> > >> On Wed,  8 Apr 2015 16:07:21 +0100
> > >> Sergio Gonzalez Monroy  wrote:
> > >>
> > >>> Currently, the target/rules to build combined libraries is different
> > >>> than the one to build individual libraries.
> > >>>
> > >>> By removing the combined library option as a build configuration 
> > >>> option
> > >>> we simplify the build pocess by having a single point for
> > >>> linking/archiving
> > >>> libraries in DPDK.
> > >>>
> > >>> This patch removes CONFIG_RTE_BUILD_COMBINE_LIB build config option 
> > >>> and
> > >>> removes the makefiles associated with building a combined library.
> > >>>
> > >>> The CONFIG_RTE_LIBNAME config option is kept as it will be use to
> > >>> always generate a linker script that acts as a single combined 
> > >>> library.
> > >>>
> > >>> Signed-off-by: Sergio Gonzalez Monroy
> > >>> 
> > >> No. We use combined library and it greatly simplfies the application
> > >> linking process.
> > >>
> > > After all the opposition this patch had in v2, I did explain the 
> > > current
> > > issues
> > > (see http://dpdk.org/ml/archives/dev/2015-March/015366.html ) and 
> > > this was
> > > the agreed solution.
> > >
> > > As I mention in the cover letter (also see patch 2/5), building DPDK
> > > (after applying this patch series) will always generate a very simple
> > > linker script that behaves as a combined library.
> > > I encourage you to apply this patch series and try to build your app
> > > (which links against combined lib).
> > > Your app should build without problem unless I messed up somewhere 
> > > and it
> > > needs fixing.
> >  Is it possible to generate a pkgconfig file (dpdk.pc) that contains 
> >  all of
> >  the setting needed to compile and link with dpdk?  That will greatly
> >  simplify usage.
> > 
> >  A linker script is just too esoteric.
> > 
> > >>> Why esoteric?  We're not talking about a linker script in the sense of 
> > >>> a binary
> > >>> layout file, we're talking about a prewritten/generated libdpdk_core.so 
> > >>> that
> > >>> contains linker directives to include the appropriate libraries.  You 
> > >>> link it
> > >>> just like you do any other library, but it lets you ignore how they are 
> > >>> broken
> > >>> up.
> > >> You mean DT_NEEDED?  That's great, but it shouldn't be called a linker
> > >> script.
> > >>
> > > no, I don't mean DT_NEEDED, I mean a linker script, because thats what 
> > > what
> > > sergio wrote is.
> > >
> > >>> We could certainly do a pkg-config file, but I don't think thats any 
> > >>> more
> > >>> adventageous than this solution.
> > >> It solves more problems -- cflags etc.  Of course having the right 
> > >> DT_NEEDED
> > >> is a good thing regardless.
> > >>
> > > Thats a good point, pkgconfig doesn't provide that additionally.  Perhaps 
> > > having
> > > both is the best solution.  As for the DT_NEEDED issues, the earlier 
> > > threads
> > > ennumerated all the problems that were being found with the way the 
> > > libraries
> > > were organized (circular dependencies).
> > >
> > > Neil
> > I am not entirely sure of the conclusion of this thread.
> > 
> > Are we happy with the current linker script solution as a replacement of 
> > the combined lib?
> > Do we want to provide pkg-config file in addition or instead of linker 
> > script?
> 
> Yes pkg-config should be an addition on top of shared/static split/combined
> libraries (or linker script). It should be an alternative to mk/rte.app.mk.
> 

Adding a pkg-config file I think makes lots of sense.

> > I think I will split the series as there seems to be no objections to 
> > the patches related to DT_NEEDED entries.
> > I'll post a series for DT_NEEDED entries and another series for dealing 
> > with the combined lib (once we get to an agreement).
> > 
> > Does it sound reasonable?
> 
yup
> 

[dpdk-dev] Crash related to virtio NICs in DPDK 2.0.0 on Freebsd 10.1 VM

[Raz Amir]

Thanks. I will submit a patch

-Original Message-
From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] 
Sent: 13 April 2015 13:46
To: Raz Amir
Cc: dev at dpdk.org; david.marchand at 6wind.com
Subject: Re: Crash related to virtio NICs in DPDK 2.0.0 on Freebsd 10.1 VM

2015-04-08 18:53, Raz Amir:
> The issues happens also in dpdk 1.8.0, and related to patch 
> http://dpdk.org/dev/patchwork/patch/239/
> 
> Adding Thomas and David to the thread and I will appreciate your input.
> 
> The patch comes to solve a file descriptor leak in the bsdapp version 
> of rte_eal_iopl_init after opening the /dev/io device.
> 
> Seems like this isn't a file descriptor leak, and it should remain 
> open - as I wrote below, I am using virtio.

Thanks for the bug report.
It seems there was no validation for FreeBSD with virtio.

> After removing it and testing the crash was resolved.
> 
> Any objection for removing the close(fd) that was added at dpdk 1.8.0?

No, there was a doubt because the man page was not clear.
http://www.freebsd.org/cgi/man.cgi?query=io&sektion=4

In case you submit a patch, please add this line:
Fixes: 8a312224bcde ("eal/bsd: fix fd leak")

> Are there scenarios that might be impacted by removing it?

I don't think so.

Thanks

[dpdk-dev] [PATCH v3] Restore support for virtio on FreeBSD

[Raz Amir]

Fixes: 8a312224bcde ("eal/bsd: fix fd leak")

Signed-off-by: Raz Amir 
---
 lib/librte_eal/bsdapp/eal/eal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 871d5f4..e20f915 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
fd = open("/dev/io", O_RDWR);
if (fd < 0)
return -1;
-   close(fd);
+   /* keep fd open for iopl */
return 0;
 }

-- 
2.1.2

[dpdk-dev] [PATCH v3] Restore support for virtio on FreeBSD

[Thomas Monjalon]

Please provide more information in the commit message.
We need to know what was the problem (crash) in the git history.
Then when doing git blame, we'll have the full explanation.

2015-04-13 15:19, Raz Amir:
> Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> 
> Signed-off-by: Raz Amir 
> ---
>  lib/librte_eal/bsdapp/eal/eal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 871d5f4..e20f915 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
>   fd = open("/dev/io", O_RDWR);
>   if (fd < 0)
>   return -1;
> - close(fd);
> + /* keep fd open for iopl */

Yes we need a comment but "for iopl" is not descriptive and
not very accurate as iopl is a Linux mechanism.

>   return 0;
>  }

Thanks

[dpdk-dev] [PATCH v9 0/3]: Add LRO support to ixgbe PMD

[Vlad Zolotarov]

On 03/31/15 14:47, Ananyev, Konstantin wrote:
>
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov
>> Sent: Monday, March 30, 2015 8:21 PM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v9 0/3]: Add LRO support to ixgbe PMD
>>
>> This series adds the missing flow for enabling the LRO in the ethdev and
>> adds a support for this feature in the ixgbe PMD. There is a big hope that 
>> this
>> initiative is going to be picked up by some Intel developer that would add 
>> the LRO support
>> to other Intel PMDs.
>>
>> The series starts with some cleanup work in the code the final patch (the 
>> actual adding of
>> the LRO support) is going to touch/use/change. There are still quite a few 
>> issues in the ixgbe
>> PMD code left but they have to be a matter of a different series and I've 
>> left a few "TODO"
>> remarks in the code.
>>
>> The LRO ("RSC" in Intel's context) PMD completion handling code follows the 
>> same design as the
>> corresponding Linux and FreeBSD implementation: pass the aggregation's 
>> cluster HEAD buffer to
>> the NEXTP entry of the software ring till EOP is met.
>>
>> HW configuration follows the corresponding specs: this feature is supported 
>> only by x540 and
>> 82599 PF devices.
>>
>> The feature has been tested with seastar TCP stack with the following 
>> configuration on Tx side:
>> - MTU: 400B
>> - 100 concurrent TCP connections.
>>
>> The results were:
>> - Without LRO: total throughput: 0.12Gbps, coefficient of variance: 1.41%
>> - With LRO:total throughput: 8.21Gbps, coefficient of variance: 0.59%
>>
>> This is an almost factor 80 improvement.
>>
>> New in v9:
>> - Move newly added IXGBE_XXX macros to ixgbe_ethdev.h.
>>
>> New in v8:
>> - Fixed the structs naming: igb_xxx -> ixgbe_xxx (some leftovers in 
>> PATCH2).
>> - Took the RSC configuration code from ixgbe_dev_rx_init() into a 
>> separate
>>   function - ixgbe_set_rsc().
>> - Added some missing macros for HW configuration.
>> - Styling adjustments:
>>- Functions names.
>>- Functions descriptions.
>> - Reworked the ixgbe_free_rsc_cluster() code to make it more readable.
>> - Kill the HEADER_SPLIT flow in ixgbe_set_rsc() since it's not supported 
>> by
>>   ixgbe PMD.
>>
>> New in v7:
>> - Free not-yet-completed RSC aggregations in rte_eth_dev_stop() flow.
>> - Fixed rx_bulk_alloc_allowed and rx_vec_allowed initialization:
>>- Don't set them to FALSE in rte_eth_dev_stop() flow - the following
>>  rte_eth_dev_start() will need them.
>>- Reset them to TRUE in rte_eth_dev_configure() and not in a probe() 
>> flow.
>>  This will ensure the proper behaviour if port is re-configured.
>> - Reset the sw_ring[].mbuf entry in a bulk allocation case.
>>   This is needed for ixgbe_rx_queue_release_mbufs().
>> - _recv_pkts_lro(): added the missing memory barrier before RDT update 
>> in a
>>   non-bulk allocation case.
>> - Don't allow RSC when device is configured in an SR-IOV mode.
>>
>> New in v6:
>> - Fix of the typo in the "bug fixes" series that broke the compilation 
>> caused a
>>   minor change in this follow-up series.
>>
>> New in v5:
>> - Split the series into "bug fixes" and "all the rest" so that the 
>> former could be
>>   integrated into a 2.0 release.
>> - Put the RTE_ETHDEV_HAS_LRO_SUPPORT definition at the beginning of 
>> rte_ethdev.h.
>> - Removed the "TODO: Remove me" comment near RTE_ETHDEV_HAS_LRO_SUPPORT.
>>
>> New in v4:
>> - Remove CONFIG_RTE_ETHDEV_LRO_SUPPORT from config/common_linuxapp.
>> - Define RTE_ETHDEV_HAS_LRO_SUPPORT in rte_ethdev.h.
>> - As a result of "ixgbe: check rxd number to avoid mbuf leak" 
>> (352078e8e) Vector Rx
>>   had to get the same treatment as Rx Bulk Alloc (see PATCH4 for more 
>> details).
>>
>> New in v3:
>> - ixgbe_rx_alloc_bufs(): Always reset refcnt of the buffers to 1. 
>> Otherwise rte_pktmbuf_free()
>>   won't free them.
>>
>> New in v2:
>> - Removed rte_eth_dev_data.lro_bulk_alloc and added 
>> ixgbe_hw.rx_bulk_alloc_allowed
>>   instead.
>> - Unified the rx_pkt_bulk callback setting (a separate new patch).
>> - Fixed a few styling and spelling issues.
>>
>> Vlad Zolotarov (3):
>>ixgbe: Cleanups
>>ixgbe: Code refactoring
>>ixgbe: Add LRO support
>>
>>   lib/librte_ether/rte_ethdev.h   |   9 +-
>>   lib/librte_net/rte_ip.h |   3 +
>>   lib/librte_pmd_ixgbe/ixgbe_ethdev.c |  11 +
>>   lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 +
>>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 765 
>> 
>>   lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |   6 +
>>   6 files changed, 738 insertions(+), 69 deletions(-)
>>
>> --
> Acked-by: Konstantin Ananyev  for 2.1 
> release.

Thomas, could u consider applying this, please.

thanks,
vlad

>
>> 2.1.0

[dpdk-dev] tools brainstorming

[Neil Horman]

On Wed, Apr 08, 2015 at 10:43:53AM +, Butler, Siobhan A wrote:
> Hi all,
> To add to the tools brainstorming - I propose we use the following Coding 
> Standards as the basis of guidelines on coding style going forward.
> The style outlined below is in alignment with the current convention used for 
> the majority of the project.
> Any thoughts/suggestions or feedback welcome.
> Thanks
> Siobhan :)
> 
> 
> 
> 
> Coding Style
> ~~
> 
> Description
> ---
> 
> This document specifies the preferred style for source files in the DPDK 
> source tree. 
> It is based on the Linux Kernel coding guidelines and the FreeBSD 7.2 Kernel 
> Developer's Manual (see man style(9)), 
> but was heavily modified for the needs of the DPDK. Many of the style rules 
> are implicit in the examples. 
> Be careful to check the examples before assuming that style is silent on an 
> issue. 
> 
> General Guidelines
> --
> 
> The rules and guidelines given in this document cannot cover every situation, 
> so the following general guidelines should be used as a fallback: 
> The code style should be consistent within each individual file, and within 
> each file in a given directory or module - in the case of creating new files 
> The primary reason for coding standards is to increase code readability and 
> comprehensibility, therefore always use whatever option will make the code 
> easiest to read. 
> 
> The following more specific recommendations apply to all sections, both for C 
> and assembly code: 
> Line length is recommended to be not more than 80 characters, including 
> comments. [Tab stop size should be assumed to be at least 4-characters wide] 
> Indentation should be to no more than 3 levels deep. 
> NOTE The above are recommendations, and not hard limits. However, it is 
> expected that the recommendations should be followed in all but the rarest 
> situations. 
> C Comment Style
> 
> Usual Comments
> --
> 
> These comments should be used in normal cases. To document a public API, a 
> doxygen-like format must be used: refer to Doxygen Documentation. 
>  /*
>   * VERY important single-line comments look like this.
>   */
>  
>  /* Most single-line comments look like this. */
>  
>  /*
>   * Multi-line comments look like this.  Make them real sentences. Fill
>   * them so they look like real paragraphs.
>   */
> 
> License Header
> --
> 
> Each file should begin with a special comment tag which will contain the 
> appropriate copyright and license for the file (Generally BSD License). 
> After any copyright header, a blank line should be left before any other 
> contents, e.g. include statements in a C file. 
> 
> C Preprocessor Directives
> -
> 
> Header Includes
> 
> In DPDK sources, the include files should be ordered as following: 
>  libc includes (system includes first) 
>  DPDK EAL includes 
>  DPDK misc libraries includes 
>  application-specific includes 
> 
> Example: 
>  #include 
>  #include 
>  
>  #include 
>  
>  #include 
>  #include 
>  
>  #include "application.h"
It doesn't really matter to me, for the sake of consistency, it might be
worthwhile mandating search path includes only (< >), and adding a -I . to the
CFLAGS in the Makefile.  That way a grep for "*.*<.*>" returns all your include
files

> 
> 
> Global pathnames are defined in . Pathnames local to the program go 
> in "pathnames.h" in the local directory. 
>  #include 

This is a design issue, not a coding style issue.  Where an application chooses
to put its pathnames are its business, and not something we should codify here.
Also, paths.h doesn't exist so it should probably not be referenced here

> 
> 
> Leave another blank line before the user include files. 
>  #include "pathnames.h" /* Local includes in double quotes. */
> 
> NOTE Please avoid, as much as possible, including headers from other headers 
> file. Doing so should be properly explained and justified. 
> Headers should be protected against multiple inclusion with the usual: 

Are you sure you want to do that?
[nhorman at hmsreliant dpdk]$ find . -name '*.h' | xargs grep include | wc -l
1300

What would the justification be?  Its common practice to do this, so I'm not
sure why you would discourage it.

>  #ifndef _FILE_H_
>  #define _FILE_H_
>  
>  /* Code */
>  
>  #endif /* _FILE_H_ */
> 
> 
> Macros
> 
> Do not ``#define`` or declare names in the implementation namespace except 
> for implementing application interfaces. 
> 

I'm not sure I understand what this means.  Can you clarify the intent here?

> The names of ``unsafe`` macros (ones that have side effects), and the names 
> of macros for manifest constants, are all in uppercase. 
> 
> The expansions of expression-like macros are either a single token or have 
> outer parentheses. If a macro is an inline expansion of a function, 
> the function name is all in lowercase and the macro has the same name all in 
> uppercase. Right-ju

[dpdk-dev] Mellanox Flow Steering

[Olga Shern]

Hi Danny, 

Please see below

Best Regards,
Olga

-Original Message-
From: Zhou, Danny [mailto:danny.z...@intel.com] 
Sent: Monday, April 13, 2015 2:30 AM
To: Olga Shern; Raghav Sethi; dev at dpdk.org
Subject: RE: [dpdk-dev] Mellanox Flow Steering

Thanks for clarification Olga. I assume when PMD is upgraded to support flow 
director, the rules should be only set by PMD while DPDK application is 
running, right?
[Olga ] Right
 Also, when DPDK application exits, the rules previously written by the PMD are 
invalid then user needs to reset rules by ethtool via mlx4_en driver.
[Olga ] Right

I think it does not make sense to allow two drivers, one in kernel and another 
in user space, to control a same NIC device simultaneously. Or a control plane 
synchronization mechanism is needed between two drivers.
[Olga ] Agree :) We are looking for a solution 

A master driver responsible for NIC control solely is expected.
[Olga ] Or there should be synchronization mechanism as you mentioned before 

> -Original Message-
> From: Olga Shern [mailto:olgas at mellanox.com]
> Sent: Monday, April 13, 2015 4:39 AM
> To: Raghav Sethi; Zhou, Danny; dev at dpdk.org
> Subject: RE: [dpdk-dev] Mellanox Flow Steering
> 
> Hi Raghav,
> 
> You are right with your observations,  Mellanox PMD and mlx4_en (kernel 
> driver) are co-exist.
> When DPDK application run, all traffic is redirected to DPDK 
> application. When DPDK application exit the traffic is received by mlx4_en 
> driver.
> 
> Regarding ethtool configuration you did, it influence only mlx4_en driver, it 
> doesn't influence Mellanox PMD queues.
> 
> Mellanox PMD doesn't support Flow Director, like you mention, and we are 
> working to add it.
> Currently the only way to spread traffic between different PMD queues is 
> using RSS.
> 
> Best Regards,
> Olga
> 
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Raghav Sethi
> Sent: Sunday, April 12, 2015 7:18 PM
> To: Zhou, Danny; dev at dpdk.org
> Subject: Re: [dpdk-dev] Mellanox Flow Steering
> 
> Hi Danny,
> 
> Thanks, that's helpful. However, Mellanox cards don't support Intel 
> Flow Director, so how would one go about installing these rules in the 
> NIC? The only technique the Mellanox User Manual (
> http://www.mellanox.com/related-docs/prod_software/Mellanox_EN_for_Lin
> ux_User_Manual_v2_0-3_0_0.pdf) lists to use Flow Steering is the 
> ethtool based method.
> 
> Additionally, the mlx4_core driver is used both by DPDK PMD and 
> otherwise (unlike the igb_uio driver, which needs to be loaded to use 
> PMD) and it seems weird that only the packets affected by the rules don't hit 
> the DPDK application. That indicates to me that the NIC is dealing with the 
> rules somehow even though a DPDK application is running.
> 
> Best,
> Raghav
> 
> On Sun, Apr 12, 2015 at 7:47 AM Zhou, Danny  wrote:
> 
> > Currently, the DPDK PMD and NIC kernel driver cannot drive a same 
> > NIC device simultaneously. When you use ethtool to setup flow 
> > director filter, the rules are written to NIC via ethtool support in 
> > kernel driver. But when DPDK PMD is loaded to drive same device, the 
> > rules previously written by ethtool/kernel_driver will be invalid, 
> > so you may have to use DPDK APIs to rewrite your rules to the NIC again.
> >
> > The bifurcated driver is designed to provide a solution to support 
> > the kernel driver and DPDK coexist scenarios, but it has security 
> > concern so netdev maintainer rejects it.
> >
> > It should not be a Mellanox hardware problem, if you try it on Intel 
> > NIC the result is same.
> >
> > > -Original Message-
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Raghav Sethi
> > > Sent: Sunday, April 12, 2015 1:10 PM
> > > To: dev at dpdk.org
> > > Subject: [dpdk-dev] Mellanox Flow Steering
> > >
> > > Hi folks,
> > >
> > > I'm trying to use the flow steering features of the Mellanox card 
> > > to effectively use a multicore server for a benchmark.
> > >
> > > The system has a single-port Mellanox ConnectX-3 EN, and I want to 
> > > use 4
> > of
> > > the 32 cores present and 4 of the 16 RX queues supported by the 
> > > hardware (i.e. one RX queue per core).
> > >
> > > I assign RX queues to each of the cores, but obviously without 
> > > flow steering (all the packets have the same IP and UDP headers, 
> > > but different dest MACs in the ethernet headers) each of the packets hits 
> > > one core.
> > I've
> > > set up the client such that it sends packets with a different 
> > > destination MAC for each RX queue (e.g. RX queue 1 should get 
> > > 10:00:00:00:00:00, RX queue 2 should get 10:00:00:00:00:01 and so on).
> > >
> > > I try to accomplish this by using ethtool to set flow steering 
> > > rules
> > (e.g.
> > > ethtool -U p7p1 flow-type ether dst 10:00:00:00:00:00 action 1 loc 
> > > 1, ethtool -U p7p1 flow-type ether dst 10:00:00:00:00:01 action 2 loc 
> > > 2..).
> > >
> > > As soon as I set up these rul

[dpdk-dev] Mellanox Flow Steering

[Raghav Sethi]

Hi Olga,

Thanks for clarifying. It appears that the mlx driver does not allow me to
modify RSS options. lib/libret_pmd_mlx4/mlx4.c file states that RSS hash
key and options cannot be modified. However, I will need to modify the hash
function to be an identity/mask function, and the key to be dst mac for my
application.

Would it be correct to conclude that I cannot route packets to cores based
on dst mac using the Mellanox?

If so, given that I have complete control over the packet headers, is there
any other way to ensure deterministic, but equal partitioning of 5-tuple
space across cores using the Mellanox card? My application uses UDP, so I'm
not really concerned about flows. I'm sure the default RSS function
attempts to do just this, but some links to documentation/code for
DPDK+mlx4 default RSS would be great.

Best,
Raghav

On Sun, Apr 12, 2015 at 4:39 PM Olga Shern  wrote:

> Hi Raghav,
>
> You are right with your observations,  Mellanox PMD and mlx4_en (kernel
> driver) are co-exist.
> When DPDK application run, all traffic is redirected to DPDK application.
> When DPDK application exit the traffic is received by mlx4_en driver.
>
> Regarding ethtool configuration you did, it influence only mlx4_en driver,
> it doesn't influence Mellanox PMD queues.
>
> Mellanox PMD doesn't support Flow Director, like you mention, and we are
> working to add it.
> Currently the only way to spread traffic between different PMD queues is
> using RSS.
>
> Best Regards,
> Olga
>
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Raghav Sethi
> Sent: Sunday, April 12, 2015 7:18 PM
> To: Zhou, Danny; dev at dpdk.org
> Subject: Re: [dpdk-dev] Mellanox Flow Steering
>
> Hi Danny,
>
> Thanks, that's helpful. However, Mellanox cards don't support Intel Flow
> Director, so how would one go about installing these rules in the NIC? The
> only technique the Mellanox User Manual (
>
> http://www.mellanox.com/related-docs/prod_software/Mellanox_EN_for_Linux_User_Manual_v2_0-3_0_0.pdf
> )
> lists to use Flow Steering is the ethtool based method.
>
> Additionally, the mlx4_core driver is used both by DPDK PMD and otherwise
> (unlike the igb_uio driver, which needs to be loaded to use PMD) and it
> seems weird that only the packets affected by the rules don't hit the DPDK
> application. That indicates to me that the NIC is dealing with the rules
> somehow even though a DPDK application is running.
>
> Best,
> Raghav
>
> On Sun, Apr 12, 2015 at 7:47 AM Zhou, Danny  wrote:
>
> > Currently, the DPDK PMD and NIC kernel driver cannot drive a same NIC
> > device simultaneously. When you use ethtool to setup flow director
> > filter, the rules are written to NIC via ethtool support in kernel
> > driver. But when DPDK PMD is loaded to drive same device, the rules
> > previously written by ethtool/kernel_driver will be invalid, so you
> > may have to use DPDK APIs to rewrite your rules to the NIC again.
> >
> > The bifurcated driver is designed to provide a solution to support the
> > kernel driver and DPDK coexist scenarios, but it has security concern
> > so netdev maintainer rejects it.
> >
> > It should not be a Mellanox hardware problem, if you try it on Intel
> > NIC the result is same.
> >
> > > -Original Message-
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Raghav Sethi
> > > Sent: Sunday, April 12, 2015 1:10 PM
> > > To: dev at dpdk.org
> > > Subject: [dpdk-dev] Mellanox Flow Steering
> > >
> > > Hi folks,
> > >
> > > I'm trying to use the flow steering features of the Mellanox card to
> > > effectively use a multicore server for a benchmark.
> > >
> > > The system has a single-port Mellanox ConnectX-3 EN, and I want to
> > > use 4
> > of
> > > the 32 cores present and 4 of the 16 RX queues supported by the
> > > hardware (i.e. one RX queue per core).
> > >
> > > I assign RX queues to each of the cores, but obviously without flow
> > > steering (all the packets have the same IP and UDP headers, but
> > > different dest MACs in the ethernet headers) each of the packets hits
> one core.
> > I've
> > > set up the client such that it sends packets with a different
> > > destination MAC for each RX queue (e.g. RX queue 1 should get
> > > 10:00:00:00:00:00, RX queue 2 should get 10:00:00:00:00:01 and so on).
> > >
> > > I try to accomplish this by using ethtool to set flow steering rules
> > (e.g.
> > > ethtool -U p7p1 flow-type ether dst 10:00:00:00:00:00 action 1 loc
> > > 1, ethtool -U p7p1 flow-type ether dst 10:00:00:00:00:01 action 2 loc
> 2..).
> > >
> > > As soon as I set up these rules though, packets matching them just
> > > stop hitting my application. All other packets go through, and
> > > removing the rules also causes the packets to go through. I'm pretty
> > > sure my
> > application
> > > is looking at all the queues, but I tried changing the rules to try
> > > a
> > rule
> > > for every single destination RX queue (0-16), and that doesn't work
> > either.
> > >

[dpdk-dev] [RFC PATCH 0/4 v2] Extending DPDK with multiple device support

[Keith Wiles]

Hi All,

Here is a set of RFC patches to update DPDK to support a generic set of
devices. The patches that follow create two files eal_common_device.h
and rte_common_device.c and then a few items in rte_ethdev.[ch] were moved
to cleanup those files by moving the device generic values and functions
into a common set of device generic files.

In the rte_common_device.h file are a couple of macros to abstract a few
common items from rte_ethdev.h which are not ethernet specific. These
generic device specific structure members are place into macros to be
placed at the top of each new device type crypto, hardware offload, dpi,
compression and possible others to be defined later. In version 2 I used
nested structures a bit cleaner from the macro design, but modified a lot
more files.

Most of the changes are to update the code to locate the new location of
the members in the nested structures. To not try and rewrite code I
used macros to help hide the changes, but these constructs are now used
in a lot of files withint DPDK.

I did not pull the Rx/Tx Routines into a common function, but it could be
done if everyone agrees. It does not mean every device will use a common
Rx/Tx routine. Without pulling Rx/Tx routines into a common file allows
the device writer to have similar or different set of routines.

These patches do not contain any new devices as these are being work on
today. I could have included the DPI (dpidev) routines we did in the PoC,
but the DPI is not open source.

More cleanup of ethdev could be done to remove NIC specific features not
supported in all devices and someone needs to do that cleanup IMO.

The code is untested and I wanted to get something our for others to poke
at today and more could be pulled out of ethdev as well. I/We will be
looking at testing the code as we get more completed.

I have not finished up the crypto APIs yet, but I am planning to work on
those additions today. The crypto code we are using is the Quick Assist
code found on 01.org, but we need to update the code to be move DPDK
friendly.

The QAT code does have a modified like API similar to Linux Kernel crypto
API and I want to review that code first.

Regards,
++Keith


Keith Wiles (4):
  Adding the common device files for multiple device support
  Add the ethdev changes for multiple device support
  Add the test file changes for common device support
  Update PMD files for new common device support

 app/test-pmd/config.c |   6 +-
 app/test-pmd/testpmd.h|   4 +-
 app/test/test_kni.c   |  12 +-
 app/test/test_link_bonding.c  |  24 +-
 app/test/virtual_pmd.c| 106 +--
 examples/link_status_interrupt/main.c |   6 +-
 lib/librte_eal/bsdapp/eal/Makefile|   1 +
 lib/librte_eal/common/Makefile|   1 +
 lib/librte_eal/common/eal_common_device.c | 185 +
 lib/librte_eal/common/include/rte_common_device.h | 674 +++
 lib/librte_eal/common/include/rte_log.h   |   1 +
 lib/librte_eal/linuxapp/eal/Makefile  |   1 +
 lib/librte_ether/rte_ethdev.c | 944 +-
 lib/librte_ether/rte_ethdev.h | 340 ++--
 lib/librte_kni/rte_kni.c  |   4 +-
 lib/librte_pmd_af_packet/rte_eth_af_packet.c  |  38 +-
 lib/librte_pmd_bond/rte_eth_bond_8023ad.c |  18 +-
 lib/librte_pmd_bond/rte_eth_bond_alb.c|  10 +-
 lib/librte_pmd_bond/rte_eth_bond_api.c| 142 ++--
 lib/librte_pmd_bond/rte_eth_bond_args.c   |   2 +-
 lib/librte_pmd_bond/rte_eth_bond_pmd.c| 164 ++--
 lib/librte_pmd_bond/rte_eth_bond_private.h|   2 +-
 lib/librte_pmd_e1000/em_ethdev.c  | 156 ++--
 lib/librte_pmd_e1000/em_rxtx.c|  94 +--
 lib/librte_pmd_e1000/igb_ethdev.c | 302 +++
 lib/librte_pmd_e1000/igb_pf.c |  86 +-
 lib/librte_pmd_e1000/igb_rxtx.c   | 168 ++--
 lib/librte_pmd_enic/enic.h|   4 +-
 lib/librte_pmd_enic/enic_ethdev.c | 140 ++--
 lib/librte_pmd_enic/enic_main.c   |  30 +-
 lib/librte_pmd_fm10k/fm10k_ethdev.c   | 154 ++--
 lib/librte_pmd_fm10k/fm10k_rxtx.c |   4 +-
 lib/librte_pmd_i40e/i40e_ethdev.c | 172 ++--
 lib/librte_pmd_i40e/i40e_ethdev_vf.c  | 194 ++---
 lib/librte_pmd_i40e/i40e_fdir.c   |  28 +-
 lib/librte_pmd_i40e/i40e_pf.c |   8 +-
 lib/librte_pmd_i40e/i40e_rxtx.c   |  88 +-
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c   | 392 -
 lib/librte_pmd_ixgbe/ixgbe_fdir.c |  50 +-
 lib/librte_pmd_ixgbe/ixgbe_pf.c   | 114 ++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 276 +++
 lib/librte_pmd_ixgbe/ixgbe_rx

[dpdk-dev] [RFC PATCH 1/4 v2] Adding the common device files for multiple device support

[Keith Wiles]

Add the eal_common_device.c and rte_common_device.h and include the
build support changes.

Signed-off-by: Keith Wiles 
---
 lib/librte_eal/bsdapp/eal/Makefile|   1 +
 lib/librte_eal/common/Makefile|   1 +
 lib/librte_eal/common/eal_common_device.c | 185 ++
 lib/librte_eal/common/include/rte_common_device.h | 674 ++
 lib/librte_eal/common/include/rte_log.h   |   1 +
 lib/librte_eal/linuxapp/eal/Makefile  |   1 +
 lib/librte_kni/rte_kni.c  |   4 +-
 7 files changed, 865 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_common_device.c
 create mode 100644 lib/librte_eal/common/include/rte_common_device.h

diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
b/lib/librte_eal/bsdapp/eal/Makefile
index 2357cfa..7bb2689 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_devargs.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_dev.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_options.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_thread.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_device.c

 CFLAGS_eal.o := -D_GNU_SOURCE
 #CFLAGS_eal_thread.o := -D_GNU_SOURCE
diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index 3ea3bbf..c4bf805 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -40,6 +40,7 @@ INC += rte_string_fns.h rte_version.h
 INC += rte_eal_memconfig.h rte_malloc_heap.h
 INC += rte_hexdump.h rte_devargs.h rte_dev.h
 INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
+INC += rte_common_device.h

 ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)
 INC += rte_warnings.h
diff --git a/lib/librte_eal/common/eal_common_device.c 
b/lib/librte_eal/common/eal_common_device.c
new file mode 100644
index 000..a9ef925
--- /dev/null
+++ b/lib/librte_eal/common/eal_common_device.c
@@ -0,0 +1,185 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2014 6WIND S.A.
+ *   All rights reserved.
+ *
+ *   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 Intel Corporation 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rte_common_device.h"
+
+void *
+rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
+   void * fn, void *user_param)
+{
+   struct rte_dev_rxtx_callback *cb;
+
+   cb = rte_zmalloc(NULL, sizeof(*cb), 0);
+
+   if (cb == NULL) {
+   rte_errno = ENOMEM;
+   return NULL;
+   }
+
+   cb->fn.vp = fn;
+   cb->param = user_param;
+   cb->next = *cbp;
+   *cbp = cb;
+   return cb;
+}
+
+int
+rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
+   struct rte_dev_rxtx_callback *user_cb)
+{
+   struct rte_dev_rxtx_callback *cb = *cbp;
+   struct rte_dev_rxtx_callback *prev_cb;
+
+   /* Reset head pointer and remove user cb if first in the list. */
+   if (cb == user_cb) {
+   *cbp = user_cb->next;
+   return 0;
+   }
+
+   /* Remove the user cb from the callback list. */
+   do {
+   prev_cb = cb;
+   cb = cb->next;
+
+   if (cb == user_cb) {
+ 

[dpdk-dev] [RFC PATCH 3/4 v2] Add the test file changes for common device support

[Keith Wiles]

Signed-off-by: Keith Wiles 
---
 app/test-pmd/config.c |   6 +-
 app/test-pmd/testpmd.h|   4 +-
 app/test/test_kni.c   |  12 ++--
 app/test/test_link_bonding.c  |  24 
 app/test/virtual_pmd.c| 106 +-
 examples/link_status_interrupt/main.c |   6 +-
 6 files changed, 79 insertions(+), 79 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f788ed5..6e70add 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -415,7 +415,7 @@ port_reg_off_is_invalid(portid_t port_id, uint32_t reg_off)
   (unsigned)reg_off);
return 1;
}
-   pci_len = ports[port_id].dev_info.pci_dev->mem_resource[0].len;
+   pci_len = ports[port_id].dev_info.di.pci_dev->mem_resource[0].len;
if (reg_off >= pci_len) {
printf("Port %d: register offset %u (0x%X) out of port PCI "
   "resource (length=%"PRIu64")\n",
@@ -641,7 +641,7 @@ ring_dma_zone_lookup(const char *ring_name, uint8_t 
port_id, uint16_t q_id)
const struct rte_memzone *mz;

snprintf(mz_name, sizeof(mz_name), "%s_%s_%d_%d",
-ports[port_id].dev_info.driver_name, ring_name, port_id, q_id);
+ports[port_id].dev_info.di.driver_name, ring_name, port_id, 
q_id);
mz = rte_memzone_lookup(mz_name);
if (mz == NULL)
printf("%s ring memory zoneof (port %d, queue %d) not"
@@ -698,7 +698,7 @@ ring_rx_descriptor_display(const struct rte_memzone 
*ring_mz,

memset(&dev_info, 0, sizeof(dev_info));
rte_eth_dev_info_get(port_id, &dev_info);
-   if (strstr(dev_info.driver_name, "i40e") != NULL) {
+   if (strstr(dev_info.di.driver_name, "i40e") != NULL) {
/* 32 bytes RX descriptor, i40e only */
struct igb_ring_desc_32_bytes *ring =
(struct igb_ring_desc_32_bytes *)ring_mz->addr;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 389fc24..af4e280 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -438,7 +438,7 @@ port_pci_reg_read(struct rte_port *port, uint32_t reg_off)
uint32_t reg_v;

reg_addr = (void *)
-   ((char *)port->dev_info.pci_dev->mem_resource[0].addr +
+   ((char *)port->dev_info.di.pci_dev->mem_resource[0].addr +
reg_off);
reg_v = *((volatile uint32_t *)reg_addr);
return rte_le_to_cpu_32(reg_v);
@@ -453,7 +453,7 @@ port_pci_reg_write(struct rte_port *port, uint32_t reg_off, 
uint32_t reg_v)
void *reg_addr;

reg_addr = (void *)
-   ((char *)port->dev_info.pci_dev->mem_resource[0].addr +
+   ((char *)port->dev_info.di.pci_dev->mem_resource[0].addr +
reg_off);
*((volatile uint32_t *)reg_addr) = rte_cpu_to_le_32(reg_v);
 }
diff --git a/app/test/test_kni.c b/app/test/test_kni.c
index 608901d..456dd65 100644
--- a/app/test/test_kni.c
+++ b/app/test/test_kni.c
@@ -387,8 +387,8 @@ test_kni_processing(uint8_t port_id, struct rte_mempool *mp)
memset(&ops, 0, sizeof(ops));

rte_eth_dev_info_get(port_id, &info);
-   conf.addr = info.pci_dev->addr;
-   conf.id = info.pci_dev->id;
+   conf.addr = info.di.pci_dev->addr;
+   conf.id = info.di.pci_dev->id;
snprintf(conf.name, sizeof(conf.name), TEST_KNI_PORT);

/* core id 1 configured for kernel thread */
@@ -555,8 +555,8 @@ test_kni(void)
memset(&conf, 0, sizeof(conf));
memset(&ops, 0, sizeof(ops));
rte_eth_dev_info_get(port_id, &info);
-   conf.addr = info.pci_dev->addr;
-   conf.id = info.pci_dev->id;
+   conf.addr = info.di.pci_dev->addr;
+   conf.id = info.di.pci_dev->id;
conf.group_id = (uint16_t)port_id;
conf.mbuf_size = MAX_PACKET_SZ;

@@ -584,8 +584,8 @@ test_kni(void)
memset(&info, 0, sizeof(info));
memset(&ops, 0, sizeof(ops));
rte_eth_dev_info_get(port_id, &info);
-   conf.addr = info.pci_dev->addr;
-   conf.id = info.pci_dev->id;
+   conf.addr = info.di.pci_dev->addr;
+   conf.id = info.di.pci_dev->id;
conf.group_id = (uint16_t)port_id;
conf.mbuf_size = MAX_PACKET_SZ;

diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 8c24314..20e74c3 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -1179,7 +1179,7 @@ int test_lsc_interrupt_count;

 static void
 test_bonding_lsc_event_callback(uint8_t port_id __rte_unused,
-   enum rte_eth_event_type type  __rte_unused, void *param 
__rte_unused)
+   enum rte_dev_event_type type  __rte_unused, void *param 
__rte_unused)
 {
pthread_mutex_lock(&mutex);
test_lsc_interrupt_count++;
@@ -1231,7 +1231,7 @@ test_status_interrupt(void)

/* register link status change interrupt

[dpdk-dev] [RFC PATCH 2/4 v2] Add the ethdev changes for multiple device support

[Keith Wiles]

Signed-off-by: Keith Wiles 
---
 lib/librte_ether/rte_ethdev.c | 944 +-
 lib/librte_ether/rte_ethdev.h | 340 ---
 2 files changed, 466 insertions(+), 818 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index e20cca5..0c68d8d 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -77,38 +77,20 @@
 #define PMD_DEBUG_TRACE(fmt, args...)
 #endif

-/* Macros for checking for restricting functions to primary instance only */
-#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
-   if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
-   PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
-   return (retval); \
-   } \
-} while(0)
-#define PROC_PRIMARY_OR_RET() do { \
-   if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
-   PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
-   return; \
-   } \
-} while(0)
-
-/* Macros to check for invlaid function pointers in dev_ops structure */
-#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
-   if ((func) == NULL) { \
-   PMD_DEBUG_TRACE("Function not supported\n"); \
-   return (retval); \
-   } \
-} while(0)
-#define FUNC_PTR_OR_RET(func) do { \
-   if ((func) == NULL) { \
-   PMD_DEBUG_TRACE("Function not supported\n"); \
-   return; \
-   } \
-} while(0)
-
-static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
 struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
-static struct rte_eth_dev_data *rte_eth_dev_data = NULL;
-static uint8_t nb_ports = 0;
+
+static struct eth_dev_global eth_globals = {
+.devs   = &rte_eth_devices[0],
+.data   = NULL,
+.nb_ports   = 0,
+.max_ports  = RTE_MAX_ETHPORTS,
+.dflt_mtu   = ETHER_MTU,
+.dev_size   = sizeof(struct rte_eth_dev),
+.data_size  = sizeof(struct rte_eth_dev_data),
+.mz_dev_data= "rte_eth_dev_data"
+};
+
+struct eth_dev_global * rte_eth_globals = ð_globals;

 /* spinlock for eth device callbacks */
 static rte_spinlock_t rte_eth_dev_cb_lock = RTE_SPINLOCK_INITIALIZER;
@@ -155,49 +137,30 @@ static struct rte_eth_xstats_name_off 
rte_txq_stats_strings[] = {
sizeof(rte_txq_stats_strings[0]))


-/**
- * The user application callback description.
- *
- * It contains callback address to be registered by user application,
- * the pointer to the parameters for callback, and the event type.
- */
-struct rte_eth_dev_callback {
-   TAILQ_ENTRY(rte_eth_dev_callback) next; /**< Callbacks list */
-   rte_eth_dev_cb_fn cb_fn;/**< Callback address */
-   void *cb_arg;   /**< Parameter for callback */
-   enum rte_eth_event_type event;  /**< Interrupt event type */
-   uint32_t active;/**< Callback is executing */
-};
-
 enum {
STAT_QMAP_TX = 0,
STAT_QMAP_RX
 };

-enum {
-   DEV_DETACHED = 0,
-   DEV_ATTACHED
-};
-
 static inline void
 rte_eth_dev_data_alloc(void)
 {
const unsigned flags = 0;
const struct rte_memzone *mz;

-   if (rte_eal_process_type() == RTE_PROC_PRIMARY){
-   mz = rte_memzone_reserve(MZ_RTE_ETH_DEV_DATA,
-   RTE_MAX_ETHPORTS * sizeof(*rte_eth_dev_data),
+   if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+   mz = rte_memzone_reserve(eth_globals.mz_dev_data,
+   eth_globals.max_ports * eth_globals.data_size,
rte_socket_id(), flags);
} else
-   mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
+   mz = rte_memzone_lookup(eth_globals.mz_dev_data);
if (mz == NULL)
rte_panic("Cannot allocate memzone for ethernet port data\n");

-   rte_eth_dev_data = mz->addr;
+   eth_globals.data = mz->addr;
if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-   memset(rte_eth_dev_data, 0,
-   RTE_MAX_ETHPORTS * sizeof(*rte_eth_dev_data));
+   memset(eth_globals.data, 0,
+   eth_globals.max_ports * eth_globals.data_size);
 }

 struct rte_eth_dev *
@@ -205,9 +168,9 @@ rte_eth_dev_allocated(const char *name)
 {
unsigned i;

-   for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+   for (i = 0; i < eth_globals.max_ports; i++) {
if ((rte_eth_devices[i].attached == DEV_ATTACHED) &&
-   strcmp(rte_eth_devices[i].data->name, name) == 0)
+   strcmp(_DD(&rte_eth_devices[i], name), name) == 0)
return &rte_eth_devices[i];
}
return NULL;
@@ -218,26 +181,26 @@ rte_eth_dev_find_free_port(void)
 {
unsigned i;

-   for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+   for (i = 0; i < eth_g

[dpdk-dev] [PATCH] doc: fix code-block syntax

[Thomas Monjalon]

2015-04-07 18:06, Thomas Monjalon:
> Some blocks are not visible with some Sphinx versions because
> they are using the wrong keyword for code.
> 
> Tested with Sphinx v1.1.3.
> 
> Fixes: 1733be6d3147 ("doc: new eal multi-pthread feature")
> Fixes: ccefe752cab0 ("doc: add jobstats sample guide")
> 
> Signed-off-by: Thomas Monjalon 

Applied

[dpdk-dev] [PATCH] enic: disable debug traces

[Thomas Monjalon]

2015-04-09 10:32, Adrien Mazarguil:
> On Tue, Apr 07, 2015 at 07:40:19PM +0200, Thomas Monjalon wrote:
> > The function name is printed in each enic_ethdev function.
> > Disable it by default with a new build option.
> > 
> > Signed-off-by: Thomas Monjalon 
[...]
> > --- a/lib/librte_pmd_enic/enic_ethdev.c
> > +++ b/lib/librte_pmd_enic/enic_ethdev.c
> > @@ -48,8 +48,12 @@
> >  #include "vnic_enet.h"
> >  #include "enic.h"
> >  
> > +#ifdef RTE_LIBRTE_ENIC_DEBUG
> >  #define ENICPMD_FUNC_TRACE() \
> > RTE_LOG(DEBUG, PMD, "ENICPMD trace: %s\n", __func__)
> > +#else
> > +#define ENICPMD_FUNC_TRACE() do {} while (0)
> 
> How about defining it as (void)0 instead of an empty do/while block?
> 
> Doing so will prevent warnings if this macro happens to be used in an
> expression. RTE_LOG() supports it.

Applied with Adrien's suggestion.

[dpdk-dev] [PATCH] mk: remove uio suffix from virtio pmd

[Thomas Monjalon]

> > The virtio pmd is not restricted to uio anymore.
> > 
> > Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")
> > 
> > Signed-off-by: Thomas Monjalon 
> 
> Acked-by: Changchun Ouyang 

Applied, thanks

[dpdk-dev] [PATCH] mk: fix static linking with null pmd

[Thomas Monjalon]

> Null PMD was not found when using a statically linked application:
>   EAL: no driver found for eth_null1
>   EAL: failed to initialize eth_null1 device
> 
> Fixes: c743e50c475f ("null: new poll mode driver")
> 
> Signed-off-by: Thomas Monjalon 

Applied

[dpdk-dev] [PATCH] scripts: test null forwarding

[Thomas Monjalon]

> This script ease testing of basic initializations and Rx/Tx bursts.
> It may help to check obvious regressions.
> In order to run it on a standard development machine, it doesn't use
> neither hugepages nor real interfaces.
> 
> The optional parameters are:
> - build directory (default: build)
> - coremask (default: 3 i.e. cores 0 and 1)
> 
> Signed-off-by: Thomas Monjalon 

Added BSD header and applied.

[dpdk-dev] [RFC PATCH 4/4 v2] Update PMD files for new common device support

[Keith Wiles]

Signed-off-by: Keith Wiles 
---
 lib/librte_pmd_af_packet/rte_eth_af_packet.c |  38 +--
 lib/librte_pmd_bond/rte_eth_bond_8023ad.c|  18 +-
 lib/librte_pmd_bond/rte_eth_bond_alb.c   |  10 +-
 lib/librte_pmd_bond/rte_eth_bond_api.c   | 142 +-
 lib/librte_pmd_bond/rte_eth_bond_args.c  |   2 +-
 lib/librte_pmd_bond/rte_eth_bond_pmd.c   | 164 +--
 lib/librte_pmd_bond/rte_eth_bond_private.h   |   2 +-
 lib/librte_pmd_e1000/em_ethdev.c | 156 +--
 lib/librte_pmd_e1000/em_rxtx.c   |  94 +++
 lib/librte_pmd_e1000/igb_ethdev.c| 302 ++---
 lib/librte_pmd_e1000/igb_pf.c|  86 +++---
 lib/librte_pmd_e1000/igb_rxtx.c  | 168 ++--
 lib/librte_pmd_enic/enic.h   |   4 +-
 lib/librte_pmd_enic/enic_ethdev.c| 140 +-
 lib/librte_pmd_enic/enic_main.c  |  30 +-
 lib/librte_pmd_fm10k/fm10k_ethdev.c  | 154 +--
 lib/librte_pmd_fm10k/fm10k_rxtx.c|   4 +-
 lib/librte_pmd_i40e/i40e_ethdev.c| 172 ++--
 lib/librte_pmd_i40e/i40e_ethdev_vf.c | 194 ++---
 lib/librte_pmd_i40e/i40e_fdir.c  |  28 +-
 lib/librte_pmd_i40e/i40e_pf.c|   8 +-
 lib/librte_pmd_i40e/i40e_rxtx.c  |  88 +++---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c  | 392 +--
 lib/librte_pmd_ixgbe/ixgbe_fdir.c|  50 ++--
 lib/librte_pmd_ixgbe/ixgbe_pf.c  | 114 
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c| 276 +--
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c|   6 +-
 lib/librte_pmd_mlx4/mlx4.c   |   2 +-
 lib/librte_pmd_null/rte_eth_null.c   |  36 +--
 lib/librte_pmd_pcap/rte_eth_pcap.c   |   2 +-
 lib/librte_pmd_ring/rte_eth_ring.c   |  36 +--
 lib/librte_pmd_virtio/virtio_ethdev.c| 120 
 lib/librte_pmd_virtio/virtio_rxtx.c  |  20 +-
 lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c  |  64 ++---
 lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c|  40 +--
 lib/librte_pmd_xenvirt/rte_eth_xenvirt.c |   2 +-
 36 files changed, 1580 insertions(+), 1584 deletions(-)

diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c 
b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
index 2ac50ba..4735fb3 100644
--- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
+++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
@@ -231,7 +231,7 @@ eth_af_packet_tx(void *queue, struct rte_mbuf **bufs, 
uint16_t nb_pkts)
 static int
 eth_dev_start(struct rte_eth_dev *dev)
 {
-   dev->data->dev_link.link_status = 1;
+   ETH_DATA(dev)->dev_link.link_status = 1;
return 0;
 }

@@ -243,7 +243,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 {
unsigned i;
int sockfd;
-   struct pmd_internals *internals = dev->data->dev_private;
+   struct pmd_internals *internals = _DD_PRIVATE(dev);

for (i = 0; i < internals->nb_queues; i++) {
sockfd = internals->rx_queue[i].sockfd;
@@ -254,7 +254,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
close(sockfd);
}

-   dev->data->dev_link.link_status = 0;
+   ETH_DATA(dev)->dev_link.link_status = 0;
 }

 static int
@@ -266,16 +266,16 @@ eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 static void
 eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
-   struct pmd_internals *internals = dev->data->dev_private;
+   struct pmd_internals *internals = _DD_PRIVATE(dev);

-   dev_info->driver_name = drivername;
-   dev_info->if_index = internals->if_index;
+   dev_info->di.driver_name = drivername;
+   dev_info->di.if_index = internals->if_index;
dev_info->max_mac_addrs = 1;
dev_info->max_rx_pktlen = (uint32_t)ETH_FRAME_LEN;
dev_info->max_rx_queues = (uint16_t)internals->nb_queues;
dev_info->max_tx_queues = (uint16_t)internals->nb_queues;
dev_info->min_rx_bufsize = 0;
-   dev_info->pci_dev = NULL;
+   dev_info->di.pci_dev = NULL;
 }

 static void
@@ -283,7 +283,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats 
*igb_stats)
 {
unsigned i, imax;
unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
-   const struct pmd_internals *internal = dev->data->dev_private;
+   const struct pmd_internals *internal = _DD_PRIVATE(dev);

imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS);
@@ -310,7 +310,7 @@ static void
 eth_stats_reset(struct rte_eth_dev *dev)
 {
unsigned i;
-   struct pmd_internals *internal = dev->data->dev_private;
+   struct pmd_internals *internal = _DD_PRIVATE(dev);

for (i = 0; i < internal->nb_queues; i++)
internal->rx_queue[i].rx_pkts = 0;
@@ -346,7 +346,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
  

[dpdk-dev] How to check memory leak with dpdk application

[Marc Sune]

On 10/04/15 07:53, Linhaifeng wrote:
> Hi, all
>
> I'am trying to use valgrind to check memory leak with my dpdk application but 
> dpdk always failed to mmap hugepages.
>
> Without valgrind it works well.How to run dpdk applications with valgrind?Is 
> there any other way to check memory leak
> with dpdk applications?
>

Yes it can be used, just that 3.10 has issues with hugepages. Check this 
out:

http://article.gmane.org/gmane.comp.networking.dpdk.devel/8058/match=valgrind+hugepages

Marc

[dpdk-dev] [PATCH v2 1/6] test: remove useless memset

[Thomas Monjalon]

> > Signed-off-by: Stephen Hemminger 
> 
> For the series
> Acked-by: Neil Horman 

Applied, thanks

[dpdk-dev] [PATCH v9 0/3]: Add LRO support to ixgbe PMD

[Thomas Monjalon]

> > This series adds the missing flow for enabling the LRO in the ethdev and
> > adds a support for this feature in the ixgbe PMD. There is a big hope that 
> > this
> > initiative is going to be picked up by some Intel developer that would add 
> > the LRO support
> > to other Intel PMDs.
> > 
> > The series starts with some cleanup work in the code the final patch (the 
> > actual adding of
> > the LRO support) is going to touch/use/change. There are still quite a few 
> > issues in the ixgbe
> > PMD code left but they have to be a matter of a different series and I've 
> > left a few "TODO"
> > remarks in the code.
> > 
> > The LRO ("RSC" in Intel's context) PMD completion handling code follows the 
> > same design as the
> > corresponding Linux and FreeBSD implementation: pass the aggregation's 
> > cluster HEAD buffer to
> > the NEXTP entry of the software ring till EOP is met.
> > 
> > HW configuration follows the corresponding specs: this feature is supported 
> > only by x540 and
> > 82599 PF devices.
> > 
> > The feature has been tested with seastar TCP stack with the following 
> > configuration on Tx side:
> >- MTU: 400B
> >- 100 concurrent TCP connections.
> > 
> > The results were:
> >- Without LRO: total throughput: 0.12Gbps, coefficient of variance: 1.41%
> >- With LRO:total throughput: 8.21Gbps, coefficient of variance: 0.59%
> > 
> > This is an almost factor 80 improvement.
> > 
> > New in v9:
> >- Move newly added IXGBE_XXX macros to ixgbe_ethdev.h.
> > 
> > New in v8:
> >- Fixed the structs naming: igb_xxx -> ixgbe_xxx (some leftovers in 
> > PATCH2).
> >- Took the RSC configuration code from ixgbe_dev_rx_init() into a 
> > separate
> >  function - ixgbe_set_rsc().
> >- Added some missing macros for HW configuration.
> >- Styling adjustments:
> >   - Functions names.
> >   - Functions descriptions.
> >- Reworked the ixgbe_free_rsc_cluster() code to make it more readable.
> >- Kill the HEADER_SPLIT flow in ixgbe_set_rsc() since it's not supported 
> > by
> >  ixgbe PMD.
> > 
> > New in v7:
> >- Free not-yet-completed RSC aggregations in rte_eth_dev_stop() flow.
> >- Fixed rx_bulk_alloc_allowed and rx_vec_allowed initialization:
> >   - Don't set them to FALSE in rte_eth_dev_stop() flow - the following
> > rte_eth_dev_start() will need them.
> >   - Reset them to TRUE in rte_eth_dev_configure() and not in a probe() 
> > flow.
> > This will ensure the proper behaviour if port is re-configured.
> >- Reset the sw_ring[].mbuf entry in a bulk allocation case.
> >  This is needed for ixgbe_rx_queue_release_mbufs().
> >- _recv_pkts_lro(): added the missing memory barrier before RDT update 
> > in a
> >  non-bulk allocation case.
> >- Don't allow RSC when device is configured in an SR-IOV mode.
> > 
> > New in v6:
> >- Fix of the typo in the "bug fixes" series that broke the compilation 
> > caused a
> >  minor change in this follow-up series.
> > 
> > New in v5:
> >- Split the series into "bug fixes" and "all the rest" so that the 
> > former could be
> >  integrated into a 2.0 release.
> >- Put the RTE_ETHDEV_HAS_LRO_SUPPORT definition at the beginning of 
> > rte_ethdev.h.
> >- Removed the "TODO: Remove me" comment near RTE_ETHDEV_HAS_LRO_SUPPORT.
> > 
> > New in v4:
> >- Remove CONFIG_RTE_ETHDEV_LRO_SUPPORT from config/common_linuxapp.
> >- Define RTE_ETHDEV_HAS_LRO_SUPPORT in rte_ethdev.h.
> >- As a result of "ixgbe: check rxd number to avoid mbuf leak" 
> > (352078e8e) Vector Rx
> >  had to get the same treatment as Rx Bulk Alloc (see PATCH4 for more 
> > details).
> > 
> > New in v3:
> >- ixgbe_rx_alloc_bufs(): Always reset refcnt of the buffers to 1. 
> > Otherwise rte_pktmbuf_free()
> >  won't free them.
> > 
> > New in v2:
> >- Removed rte_eth_dev_data.lro_bulk_alloc and added 
> > ixgbe_hw.rx_bulk_alloc_allowed
> >  instead.
> >- Unified the rx_pkt_bulk callback setting (a separate new patch).
> >- Fixed a few styling and spelling issues.
> > 
> > Vlad Zolotarov (3):
> >   ixgbe: Cleanups
> >   ixgbe: Code refactoring
> >   ixgbe: Add LRO support
> > 
> >  lib/librte_ether/rte_ethdev.h   |   9 +-
> >  lib/librte_net/rte_ip.h |   3 +
> >  lib/librte_pmd_ixgbe/ixgbe_ethdev.c |  11 +
> >  lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 +
> >  lib/librte_pmd_ixgbe/ixgbe_rxtx.c   | 765 
> > 
> >  lib/librte_pmd_ixgbe/ixgbe_rxtx.h   |   6 +
> >  6 files changed, 738 insertions(+), 69 deletions(-)
> 
> Acked-by: Konstantin Ananyev  for 2.1 
> release.

Applied, thanks

[dpdk-dev] [PATCH] doc: fixed verbatim sections in vmxnet3 docs

[Yong Wang]

On 4/10/15, 7:45 AM, "John McNamara"  wrote:

>Fixed two verbatim text sections in vmxnet3 docs.
>
>Signed-off-by: John McNamara 

This patch looks good.

Just want to point out that there are some places in the same doc that
needs fixing.  For example, in the various diagrams, specific versions of
DPDK and ESXi are used.  I understand that vmxnet3 pmd was introduced in
1.6 but I prefer them to be version-agonostic since the same diagram works
for 1.7, 1.8, etc and I don?t see much benefits in spelling out 1.6 in
this case.  Same arguments to the ESXi version.

Acked-by: Yong Wang 


>---
> doc/guides/nics/vmxnet3.rst | 27 ---
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
>diff --git a/doc/guides/nics/vmxnet3.rst b/doc/guides/nics/vmxnet3.rst
>index 3aa5b40..830530d 100644
>--- a/doc/guides/nics/vmxnet3.rst
>+++ b/doc/guides/nics/vmxnet3.rst
>@@ -149,10 +149,15 @@ This section describes an example setup for
>Phy-vSwitch-VM-Phy communication.
> Other instructions on preparing to use DPDK such as, hugepage
>enabling, uio port binding are not listed here.
> Please refer to *DPDK Getting Started Guide and DPDK Sample
>Application's User Guide* for detailed instructions.
> 
>-The packet reception and transmission flow path is:
>+The packet reception and transmission flow path is::
> 
>-Packet generator -> 82576 -> VMware ESXi vSwitch -> VMXNET3 device
>-> Guest VM VMXNET3 port 0 rx burst -> Guest
>-VM 82599 VF port 0 tx burst -> 82599 VF -> Packet generator
>+Packet generator -> 82576
>+ -> VMware ESXi vSwitch
>+ -> VMXNET3 device
>+ -> Guest VM VMXNET3 port 0 rx burst
>+ -> Guest VM 82599 VF port 0 tx burst
>+ -> 82599 VF
>+ -> Packet generator
> 
> VMXNET3 Chaining VMs Connected to a vSwitch
> ---
>@@ -166,7 +171,15 @@ The following figure shows an example VM-to-VM
>communication over a Phy-VM-vSwit
> When using the L2 Forwarding or L3 Forwarding applications,
> a destination MAC address needs to be written in packets to hit the
>other VM's VMXNET3 interface.
> 
>-In this example, the packet flow path is:
>-
>-Packet generator -> 82599 VF -> Guest VM 82599 port 0 rx burst ->
>Guest VM VMXNET3 port 1 tx burst -> VMXNET3
>-device -> VMware ESXi vSwitch -> VMXNET3 device -> Guest VM VMXNET3
>port 0 rx burst -> Guest VM 82599 VF port 1 tx burst -> 82599 VF ->
>Packet generator
>+In this example, the packet flow path is::
>+
>+Packet generator -> 82599 VF
>+ -> Guest VM 82599 port 0 rx burst
>+ -> Guest VM VMXNET3 port 1 tx burst
>+ -> VMXNET3 device
>+ -> VMware ESXi vSwitch
>+ -> VMXNET3 device
>+ -> Guest VM VMXNET3 port 0 rx burst
>+ -> Guest VM 82599 VF port 1 tx burst
>+ -> 82599 VF
>+ -> Packet generator
>-- 
>1.8.1.4
>

[dpdk-dev] [PATCH] doc: fixed verbatim sections in vmxnet3 docs

[Thomas Monjalon]

> >Fixed two verbatim text sections in vmxnet3 docs.
> >
> >Signed-off-by: John McNamara 
> 
> This patch looks good.
[...]
> Acked-by: Yong Wang 

Applied, thanks

[dpdk-dev] [PATCH 0/2] eal-pci: cleanups

[Stephen Hemminger]

Part of my patch-a-day seeries

Stephen Hemminger (2):
  pci: cleanup whitespace
  pci: rearrange logic from compare loop

 lib/librte_eal/linuxapp/eal/eal_pci.c | 41 +--
 1 file changed, 20 insertions(+), 21 deletions(-)

-- 
2.1.4

[dpdk-dev] [PATCH 1/2] pci: cleanup whitespace

[Stephen Hemminger]

Fix whitespace errors reported by checkpatch, including
missing space around operators and places where tab should
be used instead of space.

Signed-off-by: Stephen Hemminger 

---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c 
b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 9cb0ffd..c98a778 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -68,22 +68,22 @@ pci_unbind_kernel_driver(struct rte_pci_device *dev)

/* open /sys/bus/pci/devices/:BB:CC.D/driver */
snprintf(filename, sizeof(filename),
-SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver/unbind",
-loc->domain, loc->bus, loc->devid, loc->function);
+SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver/unbind",
+loc->domain, loc->bus, loc->devid, loc->function);

f = fopen(filename, "w");
if (f == NULL) /* device was not bound */
return 0;

n = snprintf(buf, sizeof(buf), PCI_PRI_FMT "\n",
-loc->domain, loc->bus, loc->devid, loc->function);
+loc->domain, loc->bus, loc->devid, loc->function);
if ((n < 0) || (n >= (int)sizeof(buf))) {
RTE_LOG(ERR, EAL, "%s(): snprintf failed\n", __func__);
goto error;
}
if (fwrite(buf, n, 1, f) == 0) {
RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__,
-   filename);
+   filename);
goto error;
}

@@ -205,8 +205,7 @@ pci_parse_sysfs_resource(const char *filename, struct 
rte_pci_device *dev)
return -1;
}

-   for (i = 0; idrv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
-  rte_eal_process_type() == RTE_PROC_PRIMARY) {
+  rte_eal_process_type() == RTE_PROC_PRIMARY) {
/* unbind current driver */
if (pci_unbind_kernel_driver(dev) < 0)
return -1;
-- 
2.1.4

[dpdk-dev] [PATCH 2/2] pci: rearrange logic from compare loop

[Stephen Hemminger]

Do some cleanup of pci scan loop.
  * check errors first
  * don't initialize variables where not necessary
  * cuddle else (follow existing style)
  * chop off conditional after return

Signed-off-by: Stephen Hemminger 

---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c 
b/lib/librte_eal/linuxapp/eal/eal_pci.c
index c98a778..d96b1c4 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -337,6 +337,12 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t 
bus,
/* parse driver */
snprintf(filename, sizeof(filename), "%s/driver", dirname);
ret = pci_get_kernel_driver_by_path(filename, driver);
+   if (ret < 0) {
+   RTE_LOG(ERR, EAL, "Fail to get kernel driver\n");
+   free(dev);
+   return -1;
+   }
+
if (!ret) {
if (!strcmp(driver, "vfio-pci"))
dev->kdrv = RTE_KDRV_VFIO;
@@ -346,37 +352,31 @@ pci_scan_one(const char *dirname, uint16_t domain, 
uint8_t bus,
dev->kdrv = RTE_KDRV_UIO_GENERIC;
else
dev->kdrv = RTE_KDRV_UNKNOWN;
-   } else if (ret < 0) {
-   RTE_LOG(ERR, EAL, "Fail to get kernel driver\n");
-   free(dev);
-   return -1;
} else
dev->kdrv = RTE_KDRV_UNKNOWN;

/* device is valid, add in list (sorted) */
if (TAILQ_EMPTY(&pci_device_list)) {
TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
-   }
-   else {
-   struct rte_pci_device *dev2 = NULL;
+   } else {
+   struct rte_pci_device *dev2;
int ret;

TAILQ_FOREACH(dev2, &pci_device_list, next) {
ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
if (ret > 0)
continue;
-   else if (ret < 0) {
+
+   if (ret < 0) {
TAILQ_INSERT_BEFORE(dev2, dev, next);
-   return 0;
} else { /* already registered */
dev2->kdrv = dev->kdrv;
dev2->max_vfs = dev->max_vfs;
-   memmove(dev2->mem_resource,
-   dev->mem_resource,
+   memmove(dev2->mem_resource, dev->mem_resource,
sizeof(dev->mem_resource));
free(dev);
-   return 0;
}
+   return 0;
}
TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
}
-- 
2.1.4

[dpdk-dev] tools brainstorming

[Stephen Hemminger]

One other policy from Linux that would be worth enforcing is that
the default config value for every new feature should be NO.

The problem is too often developers refuse/forget to test if the
code still builds without their new feature.

[dpdk-dev] [PATCH] Fixed spam from kni_allocate_mbufs() when no mbufs are free. If mbufs exhausted, 'out of memory' message logged at EXTREMELY high rates. Now logs no more than once per 10 mins

[Stephen Hemminger]

On Wed, 17 Dec 2014 07:57:02 -0600
Jay Rolette  wrote:

> Signed-off-by: Jay Rolette 
> ---
>  lib/librte_kni/rte_kni.c | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> index fdb7509..f89319c 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -40,6 +40,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -61,6 +62,9 @@
>  
>  #define KNI_MEM_CHECK(cond) do { if (cond) goto kni_fail; } while (0)
>  
> +// Configure how often we log "out of memory" messages (in seconds)
> +#define KNI_SPAM_SUPPRESSION_PERIOD 60*10
> +
>  /**
>   * KNI context
>   */
> @@ -592,6 +596,10 @@ kni_free_mbufs(struct rte_kni *kni)
>  static void
>  kni_allocate_mbufs(struct rte_kni *kni)
>  {
> + static uint64_t no_mbufs = 0;
> + static uint64_t spam_filter = 0;
> + static uint64_t delayPeriod = 0;
> +
>   int i, ret;
>   struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM];
>  
> @@ -620,7 +628,18 @@ kni_allocate_mbufs(struct rte_kni *kni)
>   pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
>   if (unlikely(pkts[i] == NULL)) {
>   /* Out of memory */
> - RTE_LOG(ERR, KNI, "Out of memory\n");
> + no_mbufs++;
> +
> + // Memory leak or need to tune? Regardless, if we get 
> here once,
> + // we will get here a *lot*. Don't spam the logs!
> + now = rte_get_tsc_cycles();
> + if (!delayPeriod)
> + delayPeriod = rte_get_tsc_hz() * 
> KNI_SPAM_SUPPRESSION_PERIOD;
> +
> + if (!spam_filter || (now - spam_filter) > delayPeriod) {
> + RTE_LOG(ERR, KNI, "No mbufs available 
> (%llu)\n", (unsigned long long)no_mbufs);
> + spam_filter = now;
> + }
>   break;
>   }
>   }

I agree whole completely with the intent of this.
But just remove the log message completely. It doesn't
help at all, use a statistic instead.

If you want to do ratelimiting, then it is better to create
a common function (see net_ratelimit() in Linux kernel) to
have all code use the same method, rather than reinventing private
code to do it.

Minor style complaints:
  * don't use camelCase, it goes against the style of the rest of the code.
  * don't use C++ style comments.
  * always use rte_cycles() not TSC for things like this.


Please resubmit removing the log message and adding a statistic.