[dpdk-dev] [PATCH RFC] maintainers: add git tree for virtio/vhost

[Thomas Monjalon]

2016-07-19 12:17, Yuanhan Liu:
> Add a git tree line for the virtio/vhost section, to make an explicit
> statement that the developers are suggested to make patches based on
> that tree.
> 
> Signed-off-by: Yuanhan Liu 

Acked-by: Thomas Monjalon 

Applied, thanks for your work on next-virtio

[dpdk-dev] [PATCH] maintainers: split networking and crypto drivers

[Thomas Monjalon]

2016-07-20 12:38, Thomas Monjalon:
> There are now 2 different sections for drivers/net/ and drivers/crypto/.
> It makes possible to declare some dedicated git trees.
> 
> Signed-off-by: Thomas Monjalon 

Applied

[dpdk-dev] [PATCH] unify tools naming

[Thomas Monjalon]

2016-07-22 18:36, Yuanhan Liu:
> On Fri, Jul 22, 2016 at 10:04:13AM +0200, Thomas Monjalon wrote:
> > 2016-07-22 09:46, Yuanhan Liu:
> > > On Wed, Jul 20, 2016 at 04:24:30PM +0200, Thomas Monjalon wrote:
> > > > The following tools may be installed system-wise.
> > > 
> > > Yes, indeed. Following is an example from dpdk package shipped in
> > > ubutun 16.04:
> > > 
> > > dpdk: /etc/dpdk/dpdk.conf
> > > dpdk: /etc/dpdk/interfaces
> > > dpdk: /etc/init.d/dpdk
> > > dpdk: /lib/dpdk/dpdk-init
> > > dpdk: /lib/systemd/system/dpdk.service
> > > dpdk: /sbin/dpdk_nic_bind
> > > dpdk: /usr/bin/dpdk_proc_info
> > > dpdk: /usr/bin/testpmd
> > > dpdk: /usr/share/doc/dpdk/changelog.Debian.gz
> > > dpdk: /usr/share/doc/dpdk/copyright
> > > dpdk: /usr/share/dpdk/tools/cpu_layout.py
> > > dpdk: /usr/share/dpdk/tools/dpdk_nic_bind.py
> > > dpdk: /usr/share/dpdk/tools/setup.sh
> > > 
> > > > It may be cleaner and more convenient to find them with the same
> > > > dpdk- prefix (especially for autocompletion).
> > > 
> > > Agreed.
> > > 
> > > > Moreover, the script dpdk_nic_bind.py deserves a new name because it is
> > > > not restricted to NICs and can be used for e.g. crypto.
> > > 
> > > Second that. But we might need doc the name change to let user aware of
> > > that.
> > 
> > There is a note in API changes section.
> > And every references in the doc are renamed.
> > 
> > > > These files are renamed:
> > > > pmdinfogen   -> dpdk-pmdinfogen
> > > > pmdinfo.py   -> dpdk-pmdinfo.py
> > > > dpdk_pdump   -> dpdk-pdump
> > > > dpdk_proc_info   -> dpdk-procinfo
> > > > dpdk_nic_bind.py -> dpdk-devbind.py
> > > > setup.sh -> dpdk-setup.sh
> > > > 
> > > > The tools pmdinfogen, pmdinfo.py and dpdk_pdump are new in 16.07.
> > > > 
> > > > The scripts dpdk_nic_bind.py and setup.sh may have been used with
> > > > previous releases by end users. That's why a symbolic link still
> > > > provide the old name in the installed tools directory.
> > > 
> > > I was about suggesting the same thing: yes, we should keep the
> > > backward compatibility.
> > 
> > Does it mean you ack this patch?
> 
> Yes, here is an explict one:
> 
> Acked-by: Yuanhan Liu 

Applied

[dpdk-dev] [PATCH] timer: fix break list when timer_cb reset running timer

[Sanford, Robert]

On 7/17/16 2:08 PM, "Hiroyuki Mikita"  wrote:

>When timer_cb resets another running timer on the same lcore,
>the list of expired timers is chained to the pending-list.
>This commit prevents a running timer from being reset
>by not its own timer_cb.
>
>Signed-off-by: Hiroyuki Mikita 
>---
> lib/librte_timer/rte_timer.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
>diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
>index 3dcdab5..00e80c9 100644
>--- a/lib/librte_timer/rte_timer.c
>+++ b/lib/librte_timer/rte_timer.c
>@@ -69,6 +69,9 @@ struct priv_timer {
> 
>   unsigned prev_lcore;  /**< used for lcore round robin */
> 
>+  /** running timer on this lcore now */
>+  struct rte_timer *running_tim;
>+
> #ifdef RTE_LIBRTE_TIMER_DEBUG
>   /** per-lcore statistics */
>   struct rte_timer_debug_stats stats;
>@@ -135,9 +138,12 @@ timer_set_config_state(struct rte_timer *tim,
>   while (success == 0) {
>   prev_status.u32 = tim->status.u32;
> 
>-  /* timer is running on another core, exit */
>+  /* timer is running on another core
>+   * or ready to run on local core, exit
>+   */
>   if (prev_status.state == RTE_TIMER_RUNNING &&
>-  prev_status.owner != (uint16_t)lcore_id)
>+  (prev_status.owner != (uint16_t)lcore_id ||
>+   tim != priv_timer[lcore_id].running_tim))
>   return -1;
> 
>   /* timer is being configured on another core */
>@@ -580,6 +586,7 @@ void rte_timer_manage(void)
>   for (tim = run_first_tim; tim != NULL; tim = next_tim) {
>   next_tim = tim->sl_next[0];
>   priv_timer[lcore_id].updated = 0;
>+  priv_timer[lcore_id].running_tim = tim;
> 
>   /* execute callback function with list unlocked */
>   tim->f(tim, tim->arg);
>@@ -610,6 +617,7 @@ void rte_timer_manage(void)
>   rte_spinlock_unlock(_timer[lcore_id].list_lock);
>   }
>   }
>+  priv_timer[lcore_id].running_tim = NULL;
> }
> 
> /* dump statistics about timers */
>-- 
>2.7.4
>

Acked-by: Robert Sanford 


I tested the three timer patches with app/test timer_autotest and
timer_racecond_autotest, and additional private tests.

[dpdk-dev] [PATCH] timer: remove unnecessary timer add call

[Sanford, Robert]

On 7/17/16 1:35 PM, "Hiroyuki Mikita"  wrote:

>When timer_set_running_state() fails in rte_timer_manage(),
>the failed timer is put back on pending-list.
>In this case, another core tries to reset or stop the timer.
>It does not need to be on pending-list
>
>Signed-off-by: Hiroyuki Mikita 
>---
> lib/librte_timer/rte_timer.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
>index 3dcdab5..94878d3 100644
>--- a/lib/librte_timer/rte_timer.c
>+++ b/lib/librte_timer/rte_timer.c
>@@ -562,10 +562,9 @@ void rte_timer_manage(void)
>   pprev = >sl_next[0];
>   } else {
>   /* another core is trying to re-config this one,
>-   * remove it from local expired list and put it
>-   * back on the priv_timer[] skip list */
>+   * remove it from local expired list
>+   */
>   *pprev = next_tim;
>-  timer_add(tim, lcore_id, 1);
>   }
>   }
> 
>-- 
>2.7.4
>

Acked-by: Robert Sanford 

[dpdk-dev] [PATCH] timer: fix incorrect pending-list manipulation

[Sanford, Robert]

On 7/17/16 10:35 AM, "Hiroyuki Mikita"  wrote:

>This commit fixes incorrect pending-list manipulation
>when getting list of expired timers in rte_timer_manage().
>
>When timer_get_prev_entries() sets pending_head on prev,
>the pending-list is broken.
>The next of pending_head always becomes NULL.
>In this depth level, it is not need to manipulate the list.
>
>Signed-off-by: Hiroyuki Mikita 
>---
> lib/librte_timer/rte_timer.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
>index 3dcdab5..7457d32 100644
>--- a/lib/librte_timer/rte_timer.c
>+++ b/lib/librte_timer/rte_timer.c
>@@ -543,6 +543,8 @@ void rte_timer_manage(void)
>   /* break the existing list at current time point */
>   timer_get_prev_entries(cur_time, lcore_id, prev);
>   for (i = priv_timer[lcore_id].curr_skiplist_depth -1; i >= 0; i--) {
>+  if (prev[i] == _timer[lcore_id].pending_head)
>+  continue;
>   priv_timer[lcore_id].pending_head.sl_next[i] =
>   prev[i]->sl_next[i];
>   if (prev[i]->sl_next[i] == NULL)
>-- 
>2.7.4
>

Acked-by: Robert Sanford 

[dpdk-dev] [PATCH] doc: update 16.07 release notes and nic guide for enic

[Thomas Monjalon]

2016-07-21 02:11, John Daley:
> Signed-off-by: John Daley 

Applied, thanks

[dpdk-dev] [PATCH] ethdev: ensure consistent port id assignment

[Tootoonchian, Amin]

Inline:

> > This is the intended behavior with this patch. Ports are to be created
> > only by the primary process. This is required for correct operation
> > IMO, because if we allow secondary processes to create ports
> > dynamically (and locally use conflicting port ids) without any
> > synchronization mechanism, they're guaranteed to overwrite each other's
> rte_eth_dev_data.
> 
> Thanks Amin for clarification,
> I had another approach, that rte_eth_devices and rte_eth_dev_data should have
> different offset of port_id and secondary process can also add devices.

That wouldn't work without some rather intrusive changes. As of now, 
rte_eth_dev_data includes port_id and therefore should be consistent across 
processes. 

> as I now understand with this patch we will not be able do something like:
> Primary:
> ./test-pmd -c 0xf  -n 4 --socket-mem='512,0'  -w 03:00.1 -w 03:00.0
>   --proc-type=primary --file-prefix=xz1 -- -i
> Secondary:
> ./test-pmd -c 0xf0 --socket-mem='512,0' -n 4 -v -b 03:00.1 -b 03:00.0 --vdev
> 'eth_pcap0,rx_pcap=/var/log/device1.pcap,tx_pcap=/var/log/device2.pcap'
> --proc-type=secondary --file-prefix=xz1 -- -i
> 
> Because secondary processes "Ports are to be created only by the primary
> process"?

Right, that wouldn't work.

Amin

[dpdk-dev] [PATCH v1] doc: update sphinx installation instructions

[Thomas Monjalon]

2016-07-17 14:19, John McNamara:
> Update the Sphinx installation instructions in the documentation
> contributors guide to reflect the fact that in the 1.4+ versions
> of Sphinx the ReadTheDocs theme must also be installed. Previously,
> in version 1.3.x, it was installed by default.
> 
> Also change 'yum' to 'dnf' for package installations.
> 
> Signed-off-by: John McNamara 

Applied, thanks

[dpdk-dev] [PATCH v1] doc: fix sphinx highlighting warnings

[Thomas Monjalon]

2016-07-17 14:11, John McNamara:
> Fix warnings raised by Python Sphinx 1.4.5:
> 
> guides/sample_app_ug/ip_pipeline.rst:334:
> WARNING: Could not lex literal_block as "ini". Highlighting skipped.
> 
> guides/sample_app_ug/l2_forward_real_virtual.rst:467:
> WARNING: Could not lex literal_block as "c". Highlighting skipped.
> 
> guides/sample_app_ug/l3_forward.rst:293:
> WARNING: Could not lex literal_block as "c". Highlighting skipped.
> 
> guides/sample_app_ug/vm_power_management.rst:162:
> WARNING: Could not lex literal_block as "xml". Highlighting skipped.
> 
> These warnings arise from invalid syntax in code-block directives.
> 
> Fixes: f1e779ec5b50 ("doc: update ip pipeline app guide")
> Fixes: d0dff9ba445e ("doc: sample application user guide")
> Fixes: c75f4e6a7a2b ("doc: add vm power mgmt app")
> 
> Signed-off-by: John McNamara 

Applied, thanks

[dpdk-dev] [PATCH v1] doc: fix release notes for 16.07

[Thomas Monjalon]

2016-07-19 14:16, John McNamara:
> Fix grammar, spelling and formatting of DPDK 16.07 release notes.
> 
> Signed-off-by: John McNamara 

Applied, thanks

I love this commit, it means we are close to the release ;)

[dpdk-dev] [PATCH v2] doc: add section on tested platforms and nics and OSes

[Thomas Monjalon]

2016-07-22 10:21, Yulong Pei:
> --- a/doc/guides/rel_notes/release_16_07.rst
> +++ b/doc/guides/rel_notes/release_16_07.rst
> @@ -350,25 +350,120 @@ The libraries prepended with a plus sign were 
> incremented in this version.
>  Tested Platforms
>  
>  
> -.. This section should contain a list of platforms that were tested with this
> -   release.

I think we should keep this comment to ease the creation of the next
release notes.

[dpdk-dev] [PATCH] doc: announce KNI ethtool removal

[Andriy Berestovskyy]

Hi folks,
Just to clarify. Thomas is talking about removing just the KNI ethtool
(i.e. lib/librte_eal/linuxapp/kni/ethtool/*). The major functionality
of those 45K lines of code is to get the same MAC address on the KNI
interface and the underlying igb/ixgbe NIC.

At the moment the rest of the DPDK eth devices work fine without the
KNI ethtool. The workaround is very simple: use ifconfig or ip tool to
set the same MAC you have on your NIC. Put it into your network
configuration to make it permanent.

Examples:
ifconfig vEth0_0 hw ether 
or
ip link set vEth0_0 address 
or
in /etc/network/interfaces under the "iface vEth0_0" section add the following:
hwaddress 


Andriy

On Thu, Jul 21, 2016 at 10:54 PM, Jay Rolette  wrote:
> On Thu, Jul 21, 2016 at 3:32 PM, Thomas Monjalon  6wind.com>
> wrote:
>
>> 2016-07-21 13:20, Jay Rolette:
>> > On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit 
>> > wrote:
>> > > KNI ethtool is functional and maintained, and it may have users!
>> > >
>> > > Why just removing it, specially without providing an alternative?
>> > > Is is good time to discuss KCP again?
>> >
>> > Yes, my product uses it.
>>
>> Your product uses what? KCP? KNI? KNI ethtool?
>>
>
> Sorry, that wasn't very clear. It uses KNI + ifconfig to configure the
> device/interface in Linux. I'm assuming the "ethtool" bits under discussion
> are the same things that make ifconfig work with KNI to the limited extent
> it does.
>
>> Seems like we are back to the same discussion we
>> > had a few months ago about the KNI situation...
>> >
>> > It shouldn't be removed unless there is a replacement, ideally one that
>> > works with the normal Linux tools like every other network device.
>>
>> This ethtool module works only for igb and ixgbe!
>> There is already no replacement for other drivers.
>> Who works on a replacement?
>>
>
> Ferruh submitted KCP previously, but you guys didn't like the fact that it
> was a kernel module. IIRC, one of the gains from that was simplified
> maintenance because you didn't need driver specific support for KNI.
> Assuming he's still willing to beat it into shape, we have something that
> is already most of the way there.
>
> If people are going to continue to block it because it is a kernel module,
> then IMO, it's better to leave the existing support on igx / ixgbe in place
> instead of stepping backwards to zero support for ethtool.
>
>> While the code wasn't ready at the time, it was a definite improvement
>> over what
>> > we have with KNI today.
>>



-- 
Andriy Berestovskyy

[dpdk-dev] [PATCH] doc: update release notes

[Thomas Monjalon]

2016-07-01 15:10, Bernard Iremonger:
> add release note for live migration of a VM with SRIOV VF
> 
> Signed-off-by: Bernard Iremonger 

Applied, thanks

[dpdk-dev] [PATCH v3 0/2] doc: live migration procedure with vhost_user

[Thomas Monjalon]

2016-07-18 15:30, Bernard Iremonger:
> This patchset describes the procedure to Live migrate a VM with
> Virtio PMD's with the vhost_user sample application (vhost-switch)
> running on the host.

Applied, thanks

[dpdk-dev] [PATCH] ethdev: support PCI domains

[Sinan Kaya]

On 7/22/2016 5:12 PM, Stephen Hemminger wrote:
> On Fri, 22 Jul 2016 11:34:10 -0400
> Sinan Kaya  wrote:
> 
>> The current code is enumerating devices based on bus, device and function
>> pairs. This does not work well for architectures with multiple PCI
>> segments/domains. Multiple PCI devices will have the same BDF value but
>> different segment numbers (01:01:01.0 and 02:01:01.0) for instance.
>>
>> Adding segment numbers to device naming so that we can uniquely identify
>> devices.
>>
>> Signed-off-by: Sinan Kaya 
> 
> I ran into this yes. There is a small risk of breaking some application that
> assumed something about names though.
> 
> Acked-by: Stephen Hemminger 
> 

Thanks, hopefully the change is minor and can be contained until next release.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

[dpdk-dev] [PATCH v5 0/2] doc: live migration procedure

[Thomas Monjalon]

2016-07-19 16:09, Bernard Iremonger:
> This patch set describes the procedure to Live migrate
> a VM with Virtio and VF PMD's using the bonding PMD.

Applied, thanks

[dpdk-dev] [PATCH] unify tools naming

[Yuanhan Liu]

On Fri, Jul 22, 2016 at 10:04:13AM +0200, Thomas Monjalon wrote:
> 2016-07-22 09:46, Yuanhan Liu:
> > On Wed, Jul 20, 2016 at 04:24:30PM +0200, Thomas Monjalon wrote:
> > > The following tools may be installed system-wise.
> > 
> > Yes, indeed. Following is an example from dpdk package shipped in
> > ubutun 16.04:
> > 
> > dpdk: /etc/dpdk/dpdk.conf
> > dpdk: /etc/dpdk/interfaces
> > dpdk: /etc/init.d/dpdk
> > dpdk: /lib/dpdk/dpdk-init
> > dpdk: /lib/systemd/system/dpdk.service
> > dpdk: /sbin/dpdk_nic_bind
> > dpdk: /usr/bin/dpdk_proc_info
> > dpdk: /usr/bin/testpmd
> > dpdk: /usr/share/doc/dpdk/changelog.Debian.gz
> > dpdk: /usr/share/doc/dpdk/copyright
> > dpdk: /usr/share/dpdk/tools/cpu_layout.py
> > dpdk: /usr/share/dpdk/tools/dpdk_nic_bind.py
> > dpdk: /usr/share/dpdk/tools/setup.sh
> > 
> > > It may be cleaner and more convenient to find them with the same
> > > dpdk- prefix (especially for autocompletion).
> > 
> > Agreed.
> > 
> > > Moreover, the script dpdk_nic_bind.py deserves a new name because it is
> > > not restricted to NICs and can be used for e.g. crypto.
> > 
> > Second that. But we might need doc the name change to let user aware of
> > that.
> 
> There is a note in API changes section.
> And every references in the doc are renamed.
> 
> > > These files are renamed:
> > > pmdinfogen   -> dpdk-pmdinfogen
> > > pmdinfo.py   -> dpdk-pmdinfo.py
> > > dpdk_pdump   -> dpdk-pdump
> > > dpdk_proc_info   -> dpdk-procinfo
> > > dpdk_nic_bind.py -> dpdk-devbind.py
> > > setup.sh -> dpdk-setup.sh
> > > 
> > > The tools pmdinfogen, pmdinfo.py and dpdk_pdump are new in 16.07.
> > > 
> > > The scripts dpdk_nic_bind.py and setup.sh may have been used with
> > > previous releases by end users. That's why a symbolic link still
> > > provide the old name in the installed tools directory.
> > 
> > I was about suggesting the same thing: yes, we should keep the
> > backward compatibility.
> 
> Does it mean you ack this patch?

Yes, here is an explict one:

Acked-by: Yuanhan Liu 

--yliu

[dpdk-dev] [PATCH v2 0/2] Safe tailq element removal in i40e driver

[Thomas Monjalon]

2016-07-22 15:02, Pablo de Lara:
> i40e driver was removing elements when iterating tailq lists 
> with TAILQ_FOREACH macro, which is not safe.
> Instead, TAILQ_FOREACH_SAFE macro is used when removing/freeing
> these elements, which is defined in DPDK if it is not already
> defined (in FreeBSD).

Applied, thanks

[dpdk-dev] [PATCH v2] eal: fix check number of bytes from read function

[Thomas Monjalon]

2016-07-22 17:02, Sergio Gonzalez Monroy:
> On 22/07/2016 16:24, Thomas Monjalon wrote:
> > 2016-07-22 16:33, Michal Jastrzebski:
> >> v2:
> >> -moved close(fd) just after read.
> >> -when read() from fd we expect 8 bytes, so PFN_MASK_SIZE macro
> >> was introduced instead sizeof(uint64_t).
> >> -removed errno print when read returns less than 8 bytes
> > Looks better.
[...]
> > Better title to explain the issue:
> > mem: fix check of physical address retrieval
[...]
> >> +  retval = read(fd, , sizeof(uint64_t));
> > We could use PFN_MASK_SIZE here
[...]
> > useless whitespace
[...]
> > If you and Sergio agree, I can make the minor changes before applying.
> 
> Go for it!

Applied with above changes, thanks

[dpdk-dev] [PATCH 2/2] doc: improve wording of new features section

[Thomas Monjalon]

2016-07-22 17:06, Bruce Richardson:
> Improve the wording of some text in the "new features" section of
> the release notes.

I think these patches conflict with this one:
http://dpdk.org/dev/patchwork/patch/14898/

[dpdk-dev] [PATCH 1/2] eal: add stailq safe iterator macro

[Thomas Monjalon]

2016-07-22 17:01, Sergio Gonzalez Monroy:
> Removing/freeing elements elements within a STAILQ_FOREACH loop
> is not safe. FreeBSD defines STAILQ_FOREACH_SAFE macro, which permits
> these operations safely.
> 
> This patch defines this macro for Linux systems, where it is not defined.
[...]
> +#ifndef SLIST_FOREACH_SAFE
> +#define SLIST_FOREACH_SAFE(var, head, field, tvar)  \
> + for ((var) = SLIST_FIRST((head));   \
> + (var) && ((tvar) = SLIST_NEXT((var), field), 1);\
> + (var) = (tvar))
> +#endif

The patch 2 requires STAILQ_FOREACH_SAFE, not SLIST_FOREACH_SAFE.

[dpdk-dev] [PATCH V2] doc: fix vhost setup in tep-termination app guide

[Thomas Monjalon]

2016-07-21 14:10, Mark Kavanagh:
> - Fix vhost setup flags
> - Add minor edits to improve readability and consistency
> 
> Signed-off-by: Mark Kavanagh 

Acked-by: John McNamara 

Applied, thanks

[dpdk-dev] [PATCH] doc: note a pitfall on reconnect feature

[Thomas Monjalon]

2016-07-20 09:08, Yuanhan Liu:
> On Tue, Jul 19, 2016 at 01:57:11PM +, Mcnamara, John wrote:
> > > -  Note: the "reconnect" feature requires **QEMU v2.7** (or above).
> > > +  .. Note::
> > > + * The "reconnect" feature requires **QEMU v2.7** (or above).
> > > +
> > > + * The vhost supported features must be exactly the same before and
> > > +   after the restart. For example, if TSO is disabled and then
> > > enabled,
> > > +   nothing will work and issues undefined might happen.
> > 
> > s/ issues undefined / undefined issues /
> 
> Thomas, mind to fix it while apply?

Yes

> > Apart from that:
> > 
> > Acked-by: John McNamara 
> 
> Thanks!

Applied, thanks

[dpdk-dev] [PATCH] doc: Fix incorrect mempool ops register macro name

[Thomas Monjalon]

> > Signed-off-by: Shreyansh Jain 
> 
> Acked-by: John McNamara 

Applied, thanks

[dpdk-dev] [PATCH] examples/l2fwd-ivshmem: fix icc compile error

[Thomas Monjalon]

> > icc version 16.0.2, compile error:
> > 
> > == host
> >   CC host.o
> > /root/development/dpdk/examples/l2fwd-ivshmem/host/host.c(157):
> > error #3656: variable "total_vm_packets_dropped"
> >  may be used before its value is set
> > total_vm_packets_dropped += ctrl->vm_ports[portid].stats.dropped;
> > ^
> > 
> > Fixes: 6aa497249172 ("examples/l2fwd-ivshmem: import sample
> > application")
> > 
> > Signed-off-by: Ferruh Yigit 
> 
> Acked-by: Anatoly  Burakov 

Applied, thanks

[dpdk-dev] [PATCH] app/pdump: cleanup rte rings upon failures

[Thomas Monjalon]

2016-07-22 15:19, Ferruh Yigit:
> On 7/22/2016 2:44 PM, Reshma Pattan wrote:
> > Function create_mp_ring_vdev() for failure cases exits without
> > freeing the created rte rings, because of this pdump tool cannot be
> > rerun successfully. Added rte ring cleanup logic upon failures.
> > 
> > Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> > 
> > Signed-off-by: Reshma Pattan 
> 
> Acked-by: Ferruh Yigit 

Applied, thanks

[dpdk-dev] [PATCH v2] eal: fix check number of bytes from read function

[Thomas Monjalon]

2016-07-22 16:33, Michal Jastrzebski:
> v2:
> -moved close(fd) just after read.
> -when read() from fd we expect 8 bytes, so PFN_MASK_SIZE macro 
> was introduced instead sizeof(uint64_t).
> -removed errno print when read returns less than 8 bytes

Looks better.
Note: this changelog should be below --- to avoid appearing in
the commit.

> In rte_mem_virt2phy: Value returned from a function and indicating the
> number of bytes was ignored. This could cause a wrong pfn (page frame
> number) mask read from pagemap file.
> When read returns less than the number of sizeof(uint64_t) bytes,
> function rte_mem_virt2phy returns error.

Better title to explain the issue:
mem: fix check of physical address retrieval

> +#define PFN_MASK_SIZE8
> +
>  #ifdef RTE_LIBRTE_XEN_DOM0
>  int rte_xen_dom0_supported(void)
>  {
> @@ -158,7 +160,7 @@ rte_mem_lock_page(const void *virt)
>  phys_addr_t
>  rte_mem_virt2phy(const void *virtaddr)
>  {
> - int fd;
> + int fd, retval;
>   uint64_t page, physaddr;
>   unsigned long virt_pfn;
>   int page_size;
> @@ -209,10 +211,19 @@ rte_mem_virt2phy(const void *virtaddr)
>   close(fd);
>   return RTE_BAD_PHYS_ADDR;
>   }
> - if (read(fd, , sizeof(uint64_t)) < 0) {
> +
> + retval = read(fd, , sizeof(uint64_t));

We could use PFN_MASK_SIZE here

> + close(fd);
> + if (retval < 0) {
>   RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap: %s\n",
>   __func__, strerror(errno));
> - close(fd);
> +

useless whitespace

> + return RTE_BAD_PHYS_ADDR;
> + } else if (retval != PFN_MASK_SIZE) {
> + RTE_LOG(ERR, EAL, "%s(): read %d bytes from /proc/self/pagemap "
> + "but expected %d:\n",
> + __func__, retval, PFN_MASK_SIZE);
> +

useless whitespace

>   return RTE_BAD_PHYS_ADDR;
>   }
>  
> @@ -222,7 +233,7 @@ rte_mem_virt2phy(const void *virtaddr)
>*/
>   physaddr = ((page & 0x7fULL) * page_size)
>   + ((unsigned long)virtaddr % page_size);
> - close(fd);
> +
>   return physaddr;
>  }

If you and Sergio agree, I can make the minor changes before applying.

[dpdk-dev] [PATCH] doc: announce driver name changes

[Adrien Mazarguil]

On Fri, Jul 22, 2016 at 02:15:39PM +, De Lara Guarch, Pablo wrote:
> 
> 
> > -Original Message-
> > From: Yigit, Ferruh
> > Sent: Friday, July 22, 2016 2:19 PM
> > To: De Lara Guarch, Pablo; dev at dpdk.org; Mcnamara, John
> > Subject: Re: [dpdk-dev] [PATCH] doc: announce driver name changes
> > 
> > On 7/22/2016 1:54 PM, Adrien Mazarguil wrote:
> > > Hi Pablo,
> > >
> > > On Fri, Jul 22, 2016 at 12:37:22PM +, De Lara Guarch, Pablo wrote:
> > >> Hi,
> > >>
> > >>> -Original Message-
> > >>> From: De Lara Guarch, Pablo
> > >>> Sent: Saturday, July 09, 2016 5:57 PM
> > >>> To: dev at dpdk.org
> > >>> Cc: Mcnamara, John; De Lara Guarch, Pablo
> > >>> Subject: [PATCH] doc: announce driver name changes
> > >>>
> > >>> Driver names for all the supported devices in DPDK do not have
> > >>> a naming convention. Some are using a prefix, some are not
> > >>> and some have long names. Driver names are used when creating
> > >>> virtual devices, so it is useful to have consistency in the names.
> > >>>
> > >>> Signed-off-by: Pablo de Lara 
> > >>> ---
> > >>>  doc/guides/rel_notes/deprecation.rst | 5 +
> > >>>  1 file changed, 5 insertions(+)
> > >>>
> > >>> diff --git a/doc/guides/rel_notes/deprecation.rst
> > >>> b/doc/guides/rel_notes/deprecation.rst
> > >>> index f502f86..37d65c8 100644
> > >>> --- a/doc/guides/rel_notes/deprecation.rst
> > >>> +++ b/doc/guides/rel_notes/deprecation.rst
> > >>> @@ -41,3 +41,8 @@ Deprecation Notices
> > >>>  * The mempool functions for single/multi producer/consumer are
> > >>> deprecated and
> > >>>will be removed in 16.11.
> > >>>It is replaced by rte_mempool_generic_get/put functions.
> > >>> +
> > >>> +* Driver names are quite inconsistent among each others and they will
> > be
> > >>> +  renamed to something more consistent (net_ prefix for net drivers and
> > >>> +  crypto_ for crypto drivers) in 16.11. Some of these driver names are
> > used
> > >>> +  publicly, to create virtual devices, so a deprecation notice is 
> > >>> necessary.
> > >>> --
> > >>> 2.7.4
> > >>
> > >> Any more comments on this (apart from Christian Ehrhardt's)?
> > >
> > > Yes, since you're suggesting to prefix driver names, shall 
> > > "librte_pmd_mlx5"
> > > really become "net_librte_pmd_mlx5" or shortened to "net_mlx5" instead?
> > >
> > > What about using a '/' separator instead of '_'?
> > >
> > > Will this impact directories as well ("net/mlx5" -> "net/net_mlx5")?
> > >
> 
> We will leave these untouched, although I don't think renaming the 
> directories was necessary.

My feeling as well, the depreciation notice wasn't clear about the extent of
name changes.

> > For physical net devices, driver name is same as folder name (mlnx5,
> > ixgbe ...)
> > 
> > For virtual net devices, driver name is folder name with "eth_" prefix
> > (eth_pcap, eth_ring)
> > 
> > Driver names for net devices looks consistent already, I don't know
> > about crypto devices but if crypto driver names are inconsistent what do
> > you think renaming crypto drivers only?
> 
> Sure, as long as virtual Ethernet devices are consistent, I think it is ok.
> My main intention here was to have consistent (and short) driver names,
> to call rte_eal_vdev_init (or --vdev in command line).

So what about using "net/" instead of "net_" to share names with commit
prefixes and folders?

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] [PATCH 2/2] doc: improve wording of new features section

[Bruce Richardson]

Improve the wording of some text in the "new features" section of
the release notes.

Signed-off-by: Bruce Richardson 
---
 doc/guides/rel_notes/release_16_07.rst | 50 +++---
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/doc/guides/rel_notes/release_16_07.rst 
b/doc/guides/rel_notes/release_16_07.rst
index 1606522..4630fe1 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -34,32 +34,36 @@ New Features

  Refer to the previous release notes for examples.

-* **Removed mempool cache if not needed.**
+* **Removed the mempool cache memory if caching is not being used.**

   The size of the mempool structure is reduced if the per-lcore cache is 
disabled.

 * **Added mempool external cache for non-EAL thread.**

   Added new functions to create, free or flush a user-owned mempool
-  cache for non-EAL threads. Previously, cache was always disabled
+  cache for non-EAL threads. Previously, caching was always disabled
   on these threads.

-* **Changed the memory allocation in mempool library.**
+* **Changed the memory allocation scheme in the mempool library.**

-  * Added ability to allocate a large mempool in virtually fragmented memory.
+  * Added the ability to allocate a large mempool in fragmented virtual memory.
   * Added new APIs to populate a mempool with memory.
   * Added an API to free a mempool.
   * Modified the API of rte_mempool_obj_iter() function.
-  * Dropped specific Xen Dom0 code.
-  * Dropped specific anonymous mempool code in testpmd.
+  * Dropped the specific Xen Dom0 code.
+  * Dropped the specific anonymous mempool code in testpmd.

-* **Added new driver for Broadcom NetXtreme-C devices.**
+* **Added a new driver for Broadcom NetXtreme-C devices.**

   Added the new bnxt driver for Broadcom NetXtreme-C devices. See the
   "Network Interface Controller Drivers" document for more details on this
   new driver.

-* **Added new driver for ThunderX nicvf device.**
+* **Added a new driver for ThunderX nicvf devices.**
+
+  Added the new thunderx net driver for ThunderX nicvf devices. See the
+  "Network Interface Controller Drivers" document for more details on this
+  new driver.

 * **Added mailbox interrupt support for ixgbe and igb VFs.**

@@ -84,11 +88,11 @@ New Features

   Updated the i40e base driver, which includes support for new devices IDs.

-* **Supported virtio on IBM POWER8.**
+* **Added support for virtio on IBM POWER8.**

   The ioports are mapped in memory when using Linux UIO.

-* **Virtio support for containers.**
+* **Added virtio support for containers.**

   Add a new virtual device, named virtio_user, to support virtio for 
containers.

@@ -103,22 +107,24 @@ New Features

 * **Added vhost-user client mode.**

-  DPDK vhost-user could be the server as well as the client. It supports
-  server mode only before, now it also supports client mode. Client mode
-  is enabled when ``RTE_VHOST_USER_CLIENT`` flag is set while calling
+  DPDK vhost-user can now act in client mode as well as in server mode.
+  Previously, it supported server mode only, but client mode support has
+  been added in this release. Client mode is enabled when
+  ``RTE_VHOST_USER_CLIENT`` flag is set while calling
   ``rte_vhost_driver_register``.

-  When DPDK vhost-user restarts from normal or abnormal quit (say crash),
-  the client mode would allow DPDK to establish the connect again.  Note
-  that a brand new QEMU version (v2.7 or above) is needed, otherwise, the
-  reconnect won't work.
+  When DPDK vhost-user restarts after a normal or abnormal process termination,
+  i.e. application graceful exit or an application crash,
+  client mode allows DPDK to re-establish existing connections again.  Note
+  that QEMU v2.7 or above is needed, in order for the reconnect to work.

   DPDK vhost-user will also try to reconnect by default when

-  * the first connect fails (when QEMU is not started yet)
-  * the connection is broken (when QEMU restarts)
+  * the first connect fails (e.g. when QEMU is not started yet)
+  * the connection is broken (e.g. when QEMU restarts)

-  It can be turned off if flag ``RTE_VHOST_USER_NO_RECONNECT`` is set.
+  This reconnection support can be turned off if flag
+  ``RTE_VHOST_USER_NO_RECONNECT`` is set.

 * **Added NSH packet recognition in i40e.**

@@ -127,7 +133,7 @@ New Features
   Now AESNI MB PMD supports 128/192/256-bit counter mode AES encryption and
   decryption.

-* **Added support of AES counter mode for Intel QuickAssist devices.**
+* **Added AES counter mode support for Intel QuickAssist devices.**

   Enabled support for the AES CTR algorithm for Intel QuickAssist devices.
   Provided support for algorithm-chaining operations.
@@ -160,7 +166,7 @@ New Features

 * **Added keepalive enhancements.**

-  Adds support for reporting of core states other than dead to
+  Adds support for reporting of core states other than "dead" to
   monitoring applications, 

[dpdk-dev] [PATCH 1/2] doc: fix indentation of new feature in release notes

[Bruce Richardson]

The description of the new feature for external caches on mempools was
indented too much, putting it out of alignment with the other items in the
new features list in the pdf output. Correct this by removing the extra
spaces.

Fixes: 4b5062755aa7 ("mempool: allow user-owned cache")

Signed-off-by: Bruce Richardson 
---
 doc/guides/rel_notes/release_16_07.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/release_16_07.rst 
b/doc/guides/rel_notes/release_16_07.rst
index 0740d4f..1606522 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -40,9 +40,9 @@ New Features

 * **Added mempool external cache for non-EAL thread.**

-   Added new functions to create, free or flush a user-owned mempool
-   cache for non-EAL threads. Previously, cache was always disabled
-   on these threads.
+  Added new functions to create, free or flush a user-owned mempool
+  cache for non-EAL threads. Previously, cache was always disabled
+  on these threads.

 * **Changed the memory allocation in mempool library.**

-- 
2.7.4

[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17

[Thomas Monjalon]

2016-07-22 14:47, Azarewicz, PiotrX T:
> > > I was trying rc3 + fix and latest (today) dpdk version. The same fail
> > message:
> > >
> > > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o): In function
> > `get_tsc_freq':
> > > eal_timer.c:(.text+0x128): undefined reference to `clock_gettime'
> > > eal_timer.c:(.text+0x166): undefined reference to `clock_gettime'
> > > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function
> > `eal_alarm_callback':
> > > eal_alarm.c:(.text+0xda): undefined reference to `clock_gettime'
> > > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function
> > `rte_eal_alarm_set':
> > > eal_alarm.c:(.text+0x211): undefined reference to `clock_gettime'
> > 
> > Interesting.
> > Could check the command line in verbose mode to see where is -lrt please?
> 
> Here you are.
> -lrt is in separate line:
> 
> gcc -o test -m64 -pthread  -march=native -DRTE_MACHINE_CPUFLAG_SSE 
> -DRTE_MACHINE_CPUFLAG_SSE2
[...]
> test_cryptodev_aes.o test_cryptodev_perf.o test_cryptodev.o test_kvargs.o -Wl,
> -lrt 
> -Wl,-lm -L/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/lib 
> -Wl,-lrte_kni -Wl,-lrte_pipeline -Wl,-lrte_table -Wl,-lrte_port 
> -Wl,-lrte_pdump -Wl,-lrte_distributor -Wl,-lrte_reorder -Wl,-lrte_ip_frag 
> -Wl,-lrte_meter -Wl,-lrte_sched -Wl,-lrte_lpm -Wl,--whole-archive 
> -Wl,-lrte_acl -Wl,--no-whole-archive -Wl,-lrte_jobstats -Wl,-lrte_power 
> -Wl,--whole-archive -Wl,-lrte_timer -Wl,-lrte_hash -Wl,-lrte_vhost 
> -Wl,-lrte_kvargs -Wl,-lrte_mbuf -Wl,-lethdev -Wl,-lrte_cryptodev 
> -Wl,-lrte_mempool -Wl,-lrte_ring -Wl,-lrte_eal -Wl,-lrte_cmdline 
> -Wl,-lrte_cfgfile -Wl,-lrte_pmd_bond -
[...]

The problem is that -lrt appears before -lrte_eal.
The question is: where does it come from?
It is even before _LDLIBS-y += -L$(RTE_SDK_BIN)/lib... mystery

[dpdk-dev] [PATCH v2] eal: fix check number of bytes from read function

[Sergio Gonzalez Monroy]

On 22/07/2016 16:24, Thomas Monjalon wrote:
> 2016-07-22 16:33, Michal Jastrzebski:
>> v2:
>> -moved close(fd) just after read.
>> -when read() from fd we expect 8 bytes, so PFN_MASK_SIZE macro
>> was introduced instead sizeof(uint64_t).
>> -removed errno print when read returns less than 8 bytes
> Looks better.
> Note: this changelog should be below --- to avoid appearing in
> the commit.
>
>> In rte_mem_virt2phy: Value returned from a function and indicating the
>> number of bytes was ignored. This could cause a wrong pfn (page frame
>> number) mask read from pagemap file.
>> When read returns less than the number of sizeof(uint64_t) bytes,
>> function rte_mem_virt2phy returns error.
> Better title to explain the issue:
> mem: fix check of physical address retrieval
>
>> +#define PFN_MASK_SIZE   8
>> +
>>   #ifdef RTE_LIBRTE_XEN_DOM0
>>   int rte_xen_dom0_supported(void)
>>   {
>> @@ -158,7 +160,7 @@ rte_mem_lock_page(const void *virt)
>>   phys_addr_t
>>   rte_mem_virt2phy(const void *virtaddr)
>>   {
>> -int fd;
>> +int fd, retval;
>>  uint64_t page, physaddr;
>>  unsigned long virt_pfn;
>>  int page_size;
>> @@ -209,10 +211,19 @@ rte_mem_virt2phy(const void *virtaddr)
>>  close(fd);
>>  return RTE_BAD_PHYS_ADDR;
>>  }
>> -if (read(fd, , sizeof(uint64_t)) < 0) {
>> +
>> +retval = read(fd, , sizeof(uint64_t));
> We could use PFN_MASK_SIZE here
>
>> +close(fd);
>> +if (retval < 0) {
>>  RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap: %s\n",
>>  __func__, strerror(errno));
>> -close(fd);
>> +
> useless whitespace
>
>> +return RTE_BAD_PHYS_ADDR;
>> +} else if (retval != PFN_MASK_SIZE) {
>> +RTE_LOG(ERR, EAL, "%s(): read %d bytes from /proc/self/pagemap "
>> +"but expected %d:\n",
>> +__func__, retval, PFN_MASK_SIZE);
>> +
> useless whitespace
>
>>  return RTE_BAD_PHYS_ADDR;
>>  }
>>   
>> @@ -222,7 +233,7 @@ rte_mem_virt2phy(const void *virtaddr)
>>   */
>>  physaddr = ((page & 0x7fULL) * page_size)
>>  + ((unsigned long)virtaddr % page_size);
>> -close(fd);
>> +
>>  return physaddr;
>>   }
> If you and Sergio agree, I can make the minor changes before applying.

Go for it!

Sergio

[dpdk-dev] [PATCH 2/2] mempool: fix unsafe tailq element removal

[Sergio Gonzalez Monroy]

Potentially user provided function could remove/free tailq elements.
Doing so within a TAILQ_FOREACH loop is not safe.

Use _SAFE versions of _FOREACH macros.

Signed-off-by: Sergio Gonzalez Monroy 
---
 lib/librte_mempool/rte_mempool.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 8806633..394154a 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -157,10 +157,10 @@ rte_mempool_obj_iter(struct rte_mempool *mp,
rte_mempool_obj_cb_t *obj_cb, void *obj_cb_arg)
 {
struct rte_mempool_objhdr *hdr;
-   void *obj;
+   void *obj, *temp;
unsigned n = 0;

-   STAILQ_FOREACH(hdr, >elt_list, next) {
+   STAILQ_FOREACH_SAFE(hdr, >elt_list, next, temp) {
obj = (char *)hdr + sizeof(*hdr);
obj_cb(mp, obj_cb_arg, obj, n);
n++;
@@ -176,8 +176,9 @@ rte_mempool_mem_iter(struct rte_mempool *mp,
 {
struct rte_mempool_memhdr *hdr;
unsigned n = 0;
+   void *temp;

-   STAILQ_FOREACH(hdr, >mem_list, next) {
+   STAILQ_FOREACH_SAFE(hdr, >mem_list, next, temp) {
mem_cb(mp, mem_cb_arg, hdr, n);
n++;
}
@@ -1283,12 +1284,13 @@ void rte_mempool_walk(void (*func)(struct rte_mempool 
*, void *),
 {
struct rte_tailq_entry *te = NULL;
struct rte_mempool_list *mempool_list;
+   void *temp;

mempool_list = RTE_TAILQ_CAST(rte_mempool_tailq.head, rte_mempool_list);

rte_rwlock_read_lock(RTE_EAL_MEMPOOL_RWLOCK);

-   TAILQ_FOREACH(te, mempool_list, next) {
+   TAILQ_FOREACH_SAFE(te, mempool_list, next, temp) {
(*func)((struct rte_mempool *) te->data, arg);
}

-- 
2.4.11

[dpdk-dev] [PATCH 1/2] eal: add stailq safe iterator macro

[Sergio Gonzalez Monroy]

Removing/freeing elements elements within a STAILQ_FOREACH loop
is not safe. FreeBSD defines STAILQ_FOREACH_SAFE macro, which permits
these operations safely.

This patch defines this macro for Linux systems, where it is not defined.

Signed-off-by: Sergio Gonzalez Monroy 
---

NOTE: This patch is based on top of:
http://dpdk.org/dev/patchwork/patch/14995/

 lib/librte_eal/common/include/rte_tailq.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_tailq.h 
b/lib/librte_eal/common/include/rte_tailq.h
index cc3c0f1..bba2835 100644
--- a/lib/librte_eal/common/include/rte_tailq.h
+++ b/lib/librte_eal/common/include/rte_tailq.h
@@ -163,6 +163,13 @@ void __attribute__((constructor, used)) tailqinitfn_ 
##t(void) \
(var) = (tvar))
 #endif

+#ifndef SLIST_FOREACH_SAFE
+#define SLIST_FOREACH_SAFE(var, head, field, tvar)  \
+   for ((var) = SLIST_FIRST((head));   \
+   (var) && ((tvar) = SLIST_NEXT((var), field), 1);\
+   (var) = (tvar))
+#endif
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.4.11

[dpdk-dev] [PATCH v2 2/2] net/i40e: fix unsafe tailq element removal

[Thomas Monjalon]

2016-07-22 15:02, Pablo de Lara:
> i40e driver was removing elements when iterating tailq lists
> with TAILQ_FOREACH macro, which is not safe.
> Instead, TAILQ_FOREACH_SAFE macro is used when removing/freeing
> these elements.

Pablo,
Maybe we should add a note to explain that the bug of freeing
while iterating is seen since the memory is zeroed on free:
ea0bddbd14e6 ("mem: zero out memory on free")

[dpdk-dev] [PATCH v2] eal: fix check number of bytes from read function

[Michal Jastrzebski]

v2:
-moved close(fd) just after read.
-when read() from fd we expect 8 bytes, so PFN_MASK_SIZE macro 
was introduced instead sizeof(uint64_t).
-removed errno print when read returns less than 8 bytes

In rte_mem_virt2phy: Value returned from a function and indicating the
number of bytes was ignored. This could cause a wrong pfn (page frame
number) mask read from pagemap file.
When read returns less than the number of sizeof(uint64_t) bytes,
function rte_mem_virt2phy returns error.

Coverity issue: 13212
Fixes: 40b966a211ab ("ivshmem: library changes for mmaping using
ivshmem").

Signed-off-by: Michal Jastrzebski 
---
 lib/librte_eal/linuxapp/eal/eal_memory.c |   19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c 
b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 42a29fa..e20a38c 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -99,6 +99,8 @@
 #include "eal_filesystem.h"
 #include "eal_hugepages.h"

+#define PFN_MASK_SIZE  8
+
 #ifdef RTE_LIBRTE_XEN_DOM0
 int rte_xen_dom0_supported(void)
 {
@@ -158,7 +160,7 @@ rte_mem_lock_page(const void *virt)
 phys_addr_t
 rte_mem_virt2phy(const void *virtaddr)
 {
-   int fd;
+   int fd, retval;
uint64_t page, physaddr;
unsigned long virt_pfn;
int page_size;
@@ -209,10 +211,19 @@ rte_mem_virt2phy(const void *virtaddr)
close(fd);
return RTE_BAD_PHYS_ADDR;
}
-   if (read(fd, , sizeof(uint64_t)) < 0) {
+
+   retval = read(fd, , sizeof(uint64_t));
+   close(fd);
+   if (retval < 0) {
RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap: %s\n",
__func__, strerror(errno));
-   close(fd);
+
+   return RTE_BAD_PHYS_ADDR;
+   } else if (retval != PFN_MASK_SIZE) {
+   RTE_LOG(ERR, EAL, "%s(): read %d bytes from /proc/self/pagemap "
+   "but expected %d:\n",
+   __func__, retval, PFN_MASK_SIZE);
+
return RTE_BAD_PHYS_ADDR;
}

@@ -222,7 +233,7 @@ rte_mem_virt2phy(const void *virtaddr)
 */
physaddr = ((page & 0x7fULL) * page_size)
+ ((unsigned long)virtaddr % page_size);
-   close(fd);
+
return physaddr;
 }

-- 
1.7.9.5

[dpdk-dev] [RFC] Generic flow director/filtering/classification API

[Chandran, Sugesh]

HI Adrien,
Thank you for your effort and considering the inputs and comments.
The design looks fine for me now.


Regards
_Sugesh


> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Thursday, July 21, 2016 2:37 PM
> To: Chandran, Sugesh 
> Cc: dev at dpdk.org; Thomas Monjalon ;
> Zhang, Helin ; Wu, Jingjing
> ; Rasesh Mody ; Ajit
> Khaparde ; Rahul Lakkireddy
> ; Lu, Wenzhuo ;
> Jan Medala ; John Daley ; Chen,
> Jing D ; Ananyev, Konstantin
> ; Matej Vido ;
> Alejandro Lucero ; Sony Chacko
> ; Jerin Jacob
> ; De Lara Guarch, Pablo
> ; Olga Shern ;
> Chilikin, Andrey 
> Subject: Re: [dpdk-dev] [RFC] Generic flow director/filtering/classification
> API
> 
> Hi Sugesh,
> 
> I do not have much to add, please see below.
> 
> On Thu, Jul 21, 2016 at 11:06:52AM +, Chandran, Sugesh wrote:
> [...]
> > > > RSS hashing support :- Just to confirm, the RSS flow action allows
> > > > application to decide the header fields to produce the hash. This
> > > > gives programmability on load sharing across different queues. The
> > > > application can program the NIC to calculate the RSS hash only
> > > > using mac or mac+ ip or ip only using this.
> > >
> > > I'd say yes but from your summary, I'm not sure we share the same
> > > idea of what the RSS action is supposed to do, so here is mine.
> > >
> > > Like all flow rules, the pattern part of the RSS action only filters
> > > the packets on which the action will be performed.
> > >
> > > The rss_conf parameter (struct rte_eth_rss_conf) only provides a key
> > > and a RSS hash function to use (ETH_RSS_IPV4,
> > > ETH_RSS_NONFRAG_IPV6_UDP, etc).
> > >
> > > Nothing prevents the RSS hash function from being applied to
> > > protocol headers which are not necessarily present in the flow rule
> > > pattern. These are two independent things, e.g. you could have a
> > > pattern matching IPv4 packets yet perform RSS hashing only on UDP
> headers.
> > >
> > > Finally, the RSS action configuration only affects packets coming
> > > from this flow rule. It is not performed on the device globally so
> > > packets which are not matched are not affected by RSS processing. As
> > > a result it might not be possible to configure two flow rules
> > > specifying incompatible RSS actions simultaneously if the underlying
> > > device supports only a single global RSS context.
> > >
> > [Sugesh] thank you for the explanation. This means I can have a rule
> > that matches on Every incoming packets(all field wild card rule) and
> > does RSS hash on selected fields, MAC only, IP only or IP & MAC?
> 
> Yes, I guess it could even replace the current method for configuring RSS on a
> device in a more versatile fashion, but this is a topic for another debate.
> 
> Let's implement this API first!
> 
> > This can be useful to do a packet lookup in software by just using
> > Only hash.
> 
> Not sure to fully understand your idea, but I'm positive it could be done
> somehow :)
> 
> --
> Adrien Mazarguil
> 6WIND

[dpdk-dev] [PATCH v2] eal: fix check number of bytes from read function

[Jastrzebski, MichalX K]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, July 22, 2016 5:25 PM
> To: Jastrzebski, MichalX K 
> Cc: Richardson, Bruce ; dev at dpdk.org;
> Kobylinski, MichalX ; Gonzalez Monroy,
> Sergio ; david.marchand at 6wind.com
> Subject: Re: [PATCH v2] eal: fix check number of bytes from read function
> 
> 2016-07-22 16:33, Michal Jastrzebski:
> > v2:
> > -moved close(fd) just after read.
> > -when read() from fd we expect 8 bytes, so PFN_MASK_SIZE macro
> > was introduced instead sizeof(uint64_t).
> > -removed errno print when read returns less than 8 bytes
> 
> Looks better.
> Note: this changelog should be below --- to avoid appearing in
> the commit.
> 
> > In rte_mem_virt2phy: Value returned from a function and indicating the
> > number of bytes was ignored. This could cause a wrong pfn (page frame
> > number) mask read from pagemap file.
> > When read returns less than the number of sizeof(uint64_t) bytes,
> > function rte_mem_virt2phy returns error.
> 
> Better title to explain the issue:
> mem: fix check of physical address retrieval
> 
> > +#define PFN_MASK_SIZE  8
> > +
> >  #ifdef RTE_LIBRTE_XEN_DOM0
> >  int rte_xen_dom0_supported(void)
> >  {
> > @@ -158,7 +160,7 @@ rte_mem_lock_page(const void *virt)
> >  phys_addr_t
> >  rte_mem_virt2phy(const void *virtaddr)
> >  {
> > -   int fd;
> > +   int fd, retval;
> > uint64_t page, physaddr;
> > unsigned long virt_pfn;
> > int page_size;
> > @@ -209,10 +211,19 @@ rte_mem_virt2phy(const void *virtaddr)
> > close(fd);
> > return RTE_BAD_PHYS_ADDR;
> > }
> > -   if (read(fd, , sizeof(uint64_t)) < 0) {
> > +
> > +   retval = read(fd, , sizeof(uint64_t));
> 
> We could use PFN_MASK_SIZE here
> 
> > +   close(fd);
> > +   if (retval < 0) {
> > RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap:
> %s\n",
> > __func__, strerror(errno));
> > -   close(fd);
> > +
> 
> useless whitespace
> 
> > +   return RTE_BAD_PHYS_ADDR;
> > +   } else if (retval != PFN_MASK_SIZE) {
> > +   RTE_LOG(ERR, EAL, "%s(): read %d bytes from
> /proc/self/pagemap "
> > +   "but expected %d:\n",
> > +   __func__, retval, PFN_MASK_SIZE);
> > +
> 
> useless whitespace
> 
> > return RTE_BAD_PHYS_ADDR;
> > }
> >
> > @@ -222,7 +233,7 @@ rte_mem_virt2phy(const void *virtaddr)
> >  */
> > physaddr = ((page & 0x7fULL) * page_size)
> > + ((unsigned long)virtaddr % page_size);
> > -   close(fd);
> > +
> > return physaddr;
> >  }
> 
> If you and Sergio agree, I can make the minor changes before applying.

Thanks Thomas. Please apply.

[dpdk-dev] [PATCH] doc: announce driver name changes

[Thomas Monjalon]

2016-07-22 14:15, De Lara Guarch, Pablo:
> From: Yigit, Ferruh
> > For physical net devices, driver name is same as folder name (mlnx5,
> > ixgbe ...)
> > 
> > For virtual net devices, driver name is folder name with "eth_" prefix
> > (eth_pcap, eth_ring)
> > 
> > Driver names for net devices looks consistent already, I don't know
> > about crypto devices but if crypto driver names are inconsistent what do
> > you think renaming crypto drivers only?
> 
> Sure, as long as virtual Ethernet devices are consistent, I think it is ok.
> My main intention here was to have consistent (and short) driver names,
> to call rte_eal_vdev_init (or --vdev in command line).

It is important to rename every drivers with net and crypto prefixes.
The long term goal is to have a consistent command line specification
to pass parameters to any device/driver (virtual or physical).

[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17

[Thomas Monjalon]

2016-07-22 14:07, Azarewicz, PiotrX T:
> > > > > Tested-by: Yongjie Gu 
> > > >
> > > > Applied
> > >
> > > OS: UB1204
> > > GCC: 4.6.4
> > > Kernel: 3.13.0-45
> > > glibc 2.15
> > >
> > > x86_64-native-linuxapp-gcc: FAIL
> > 
> > Please don't be a bot and explain us the error you see.
> 
> I was trying rc3 + fix and latest (today) dpdk version. The same fail message:
> 
> /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o): In function 
> `get_tsc_freq':
> eal_timer.c:(.text+0x128): undefined reference to `clock_gettime'
> eal_timer.c:(.text+0x166): undefined reference to `clock_gettime'
> /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function 
> `eal_alarm_callback':
> eal_alarm.c:(.text+0xda): undefined reference to `clock_gettime'
> /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function 
> `rte_eal_alarm_set':
> eal_alarm.c:(.text+0x211): undefined reference to `clock_gettime'

Interesting.
Could check the command line in verbose mode to see where is -lrt please?

[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17

[Thomas Monjalon]

2016-07-22 13:38, Azarewicz, PiotrX T:
> > > Tested-by: Yongjie Gu 
> > 
> > Applied
> 
> OS: UB1204
> GCC: 4.6.4
> Kernel: 3.13.0-45
> glibc 2.15
> 
> x86_64-native-linuxapp-gcc: FAIL

Please don't be a bot and explain us the error you see.

[dpdk-dev] em1000 driver lockup in KVM

[Stephen Hemminger]

On Fri, 22 Jul 2016 14:19:29 -0700
Prabahar Radhakrishnan  wrote:

> Hi,
>I was running a dpdk application with e1000 driver and I am facing a
> rare driver lockup condition.  Under this condition, what I am seeing is
> that the Receive side locks up.  The transmit side is fine.  When I execute
> "rte_eth_dev_stop()/rte_eth_dev_start()" from within the VM, the dpdk
> application recovers.  The lockup happens once in 2 or 3 days if I
> continuously ping an IP address serviced by the dpdk application.  Once it
> locks up, what I am noticing is that even though packets are coming in, the
> driver is not populating them and signaling the event.  In other words, in
> "eth_em_recv_pkts" call, I am not seeing "E1000_RXD_STAT_DD" being set for
> status, and the driver bails out.  If anyone has come across such
> condition, please let me know?  Appreciate the help.
> 
> Environment:  I have a VM running on top of Ubuntu/KVM.
> Application: Running dpdk-2.0.0.
> VM: uses e1000 driver.
> On the KVM side, our port is sitting in a bridge.  There are other VNFs
> that we use for testing and some of them are using virtio driver.
> 
> Thank You
> regards Prab

Save yourself lots of pain, use virtio not e1000 with DPDK.
The emulation of E1000 in QEMU is very limited and DPDK expects real hardware.
Lots of features and control are missing.

[dpdk-dev] [PATCH] app/pdump: cleanup rte rings upon failures

[Ferruh Yigit]

On 7/22/2016 2:44 PM, Reshma Pattan wrote:
> Function create_mp_ring_vdev() for failure cases exits without
> freeing the created rte rings, because of this pdump tool cannot be
> rerun successfully. Added rte ring cleanup logic upon failures.
> 
> Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")
> 
> Signed-off-by: Reshma Pattan 

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH] examples/l2fwd-ivshmem: fix icc compile error

[Ferruh Yigit]

icc version 16.0.2, compile error:

== host
  CC host.o
/root/development/dpdk/examples/l2fwd-ivshmem/host/host.c(157):
error #3656: variable "total_vm_packets_dropped"
 may be used before its value is set
total_vm_packets_dropped += ctrl->vm_ports[portid].stats.dropped;
^

Fixes: 6aa497249172 ("examples/l2fwd-ivshmem: import sample application")

Signed-off-by: Ferruh Yigit 
---
 examples/l2fwd-ivshmem/host/host.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/examples/l2fwd-ivshmem/host/host.c 
b/examples/l2fwd-ivshmem/host/host.c
index cd284b7..da7b00d 100644
--- a/examples/l2fwd-ivshmem/host/host.c
+++ b/examples/l2fwd-ivshmem/host/host.c
@@ -110,7 +110,8 @@ static void
 print_stats(void)
 {
uint64_t total_packets_dropped, total_packets_tx, total_packets_rx;
-   uint64_t total_vm_packets_dropped, total_vm_packets_tx, 
total_vm_packets_rx;
+   uint64_t total_vm_packets_dropped = 0;
+   uint64_t total_vm_packets_tx, total_vm_packets_rx;
unsigned portid;

total_packets_dropped = 0;
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/2] net/i40e: fix unsafe tailq element removal

[Pablo de Lara]

i40e driver was removing elements when iterating tailq lists
with TAILQ_FOREACH macro, which is not safe.
Instead, TAILQ_FOREACH_SAFE macro is used when removing/freeing
these elements.

Fixes: 4861cde46116 ("i40e: new poll mode driver")
Fixes: 440499cf5376 ("net/i40e: support floating VEB")

Signed-off-by: Pablo de Lara 
---
 drivers/net/i40e/i40e_ethdev.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 84c86aa..11a5804 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -31,7 +31,6 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */

-#include 
 #include 
 #include 
 #include 
@@ -51,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 

 #include "i40e_logs.h"
 #include "base/i40e_prototype.h"
@@ -4094,6 +4094,7 @@ i40e_vsi_release(struct i40e_vsi *vsi)
struct i40e_pf *pf;
struct i40e_hw *hw;
struct i40e_vsi_list *vsi_list;
+   void *temp;
int ret;
struct i40e_mac_filter *f;
uint16_t user_param = vsi->user_param;
@@ -4106,7 +4107,7 @@ i40e_vsi_release(struct i40e_vsi *vsi)

/* VSI has child to attach, release child first */
if (vsi->veb) {
-   TAILQ_FOREACH(vsi_list, >veb->head, list) {
+   TAILQ_FOREACH_SAFE(vsi_list, >veb->head, list, temp) {
if (i40e_vsi_release(vsi_list->vsi) != I40E_SUCCESS)
return -1;
TAILQ_REMOVE(>veb->head, vsi_list, list);
@@ -4115,7 +4116,7 @@ i40e_vsi_release(struct i40e_vsi *vsi)
}

if (vsi->floating_veb) {
-   TAILQ_FOREACH(vsi_list, >floating_veb->head, list) {
+   TAILQ_FOREACH_SAFE(vsi_list, >floating_veb->head, list, 
temp) {
if (i40e_vsi_release(vsi_list->vsi) != I40E_SUCCESS)
return -1;
TAILQ_REMOVE(>floating_veb->head, vsi_list, list);
@@ -4124,7 +4125,7 @@ i40e_vsi_release(struct i40e_vsi *vsi)

/* Remove all macvlan filters of the VSI */
i40e_vsi_remove_all_macvlan_filter(vsi);
-   TAILQ_FOREACH(f, >mac_list, next)
+   TAILQ_FOREACH_SAFE(f, >mac_list, next, temp)
rte_free(f);

if (vsi->type != I40E_VSI_MAIN &&
@@ -4682,6 +4683,7 @@ i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on)
 {
int i, num;
struct i40e_mac_filter *f;
+   void *temp;
struct i40e_mac_filter_info *mac_filter;
enum rte_mac_filter_type desired_filter;
int ret = I40E_SUCCESS;
@@ -4706,7 +4708,7 @@ i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on)
i = 0;

/* Remove all existing mac */
-   TAILQ_FOREACH(f, >mac_list, next) {
+   TAILQ_FOREACH_SAFE(f, >mac_list, next, temp) {
mac_filter[i] = f->mac_info;
ret = i40e_vsi_delete_mac(vsi, >mac_info.mac_addr);
if (ret) {
-- 
2.7.4

[dpdk-dev] [PATCH v2 1/2] eal: add tailq safe iterator macro

[Pablo de Lara]

Removing/freeing elements elements within a TAILQ_FOREACH loop is not safe.
FreeBSD defines TAILQ_FOREACH_SAFE macro, which permits
these operations safely.
This patch defines this macro for Linux systems, where it is not defined.

Signed-off-by: Pablo de Lara 
---
 lib/librte_eal/common/include/rte_tailq.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_tailq.h 
b/lib/librte_eal/common/include/rte_tailq.h
index 4a686e6..cc3c0f1 100644
--- a/lib/librte_eal/common/include/rte_tailq.h
+++ b/lib/librte_eal/common/include/rte_tailq.h
@@ -155,6 +155,14 @@ void __attribute__((constructor, used)) tailqinitfn_ 
##t(void) \
rte_panic("Cannot initialize tailq: %s\n", t.name); \
 }

+/* This macro permits both remove and free var within the loop safely.*/
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+   for ((var) = TAILQ_FIRST((head));   \
+   (var) && ((tvar) = TAILQ_NEXT((var), field), 1);\
+   (var) = (tvar))
+#endif
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.7.4

[dpdk-dev] [PATCH v2 0/2] Safe tailq element removal in i40e driver

[Pablo de Lara]

i40e driver was removing elements when iterating tailq lists 
with TAILQ_FOREACH macro, which is not safe.
Instead, TAILQ_FOREACH_SAFE macro is used when removing/freeing
these elements, which is defined in DPDK if it is not already
defined (in FreeBSD).

Changes in v2:
- Modified second commit title

Pablo de Lara (2):
  eal: add tailq safe iterator macro
  net/i40e: fix unsafe tailq element removal

 drivers/net/i40e/i40e_ethdev.c| 12 +++-
 lib/librte_eal/common/include/rte_tailq.h |  8 
 2 files changed, 15 insertions(+), 5 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH] doc: announce driver name changes

[Adrien Mazarguil]

Hi Pablo,

On Fri, Jul 22, 2016 at 12:37:22PM +, De Lara Guarch, Pablo wrote:
> Hi,
> 
> > -Original Message-
> > From: De Lara Guarch, Pablo
> > Sent: Saturday, July 09, 2016 5:57 PM
> > To: dev at dpdk.org
> > Cc: Mcnamara, John; De Lara Guarch, Pablo
> > Subject: [PATCH] doc: announce driver name changes
> > 
> > Driver names for all the supported devices in DPDK do not have
> > a naming convention. Some are using a prefix, some are not
> > and some have long names. Driver names are used when creating
> > virtual devices, so it is useful to have consistency in the names.
> > 
> > Signed-off-by: Pablo de Lara 
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index f502f86..37d65c8 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -41,3 +41,8 @@ Deprecation Notices
> >  * The mempool functions for single/multi producer/consumer are
> > deprecated and
> >will be removed in 16.11.
> >It is replaced by rte_mempool_generic_get/put functions.
> > +
> > +* Driver names are quite inconsistent among each others and they will be
> > +  renamed to something more consistent (net_ prefix for net drivers and
> > +  crypto_ for crypto drivers) in 16.11. Some of these driver names are used
> > +  publicly, to create virtual devices, so a deprecation notice is 
> > necessary.
> > --
> > 2.7.4
> 
> Any more comments on this (apart from Christian Ehrhardt's)?

Yes, since you're suggesting to prefix driver names, shall "librte_pmd_mlx5"
really become "net_librte_pmd_mlx5" or shortened to "net_mlx5" instead?

What about using a '/' separator instead of '_'?

Will this impact directories as well ("net/mlx5" -> "net/net_mlx5")?

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] [PATCH] examples/l2fwd-ivshmem: fix icc compile error

[Burakov, Anatoly]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Friday, July 22, 2016 3:14 PM
> To: dev at dpdk.org
> Cc: Burakov, Anatoly 
> Subject: [PATCH] examples/l2fwd-ivshmem: fix icc compile error
> 
> icc version 16.0.2, compile error:
> 
> == host
>   CC host.o
> /root/development/dpdk/examples/l2fwd-ivshmem/host/host.c(157):
> error #3656: variable "total_vm_packets_dropped"
>  may be used before its value is set
> total_vm_packets_dropped += ctrl->vm_ports[portid].stats.dropped;
> ^
> 
> Fixes: 6aa497249172 ("examples/l2fwd-ivshmem: import sample
> application")
> 
> Signed-off-by: Ferruh Yigit 
> ---
>  examples/l2fwd-ivshmem/host/host.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/l2fwd-ivshmem/host/host.c b/examples/l2fwd-
> ivshmem/host/host.c
> index cd284b7..da7b00d 100644
> --- a/examples/l2fwd-ivshmem/host/host.c
> +++ b/examples/l2fwd-ivshmem/host/host.c
> @@ -110,7 +110,8 @@ static void
>  print_stats(void)
>  {
>   uint64_t total_packets_dropped, total_packets_tx, total_packets_rx;
> - uint64_t total_vm_packets_dropped, total_vm_packets_tx,
> total_vm_packets_rx;
> + uint64_t total_vm_packets_dropped = 0;
> + uint64_t total_vm_packets_tx, total_vm_packets_rx;
>   unsigned portid;
> 
>   total_packets_dropped = 0;
> --
> 2.7.4

Acked-by: Anatoly  Burakov 

[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17

[Azarewicz, PiotrX T]

> > > > > > Tested-by: Yongjie Gu 
> > > > >
> > > > > Applied
> > > >
> > > > OS: UB1204
> > > > GCC: 4.6.4
> > > > Kernel: 3.13.0-45
> > > > glibc 2.15
> > > >
> > > > x86_64-native-linuxapp-gcc: FAIL
> > >
> > > Please don't be a bot and explain us the error you see.
> >
> > I was trying rc3 + fix and latest (today) dpdk version. The same fail
> message:
> >
> > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o): In function
> `get_tsc_freq':
> > eal_timer.c:(.text+0x128): undefined reference to `clock_gettime'
> > eal_timer.c:(.text+0x166): undefined reference to `clock_gettime'
> > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function
> `eal_alarm_callback':
> > eal_alarm.c:(.text+0xda): undefined reference to `clock_gettime'
> > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function
> `rte_eal_alarm_set':
> > eal_alarm.c:(.text+0x211): undefined reference to `clock_gettime'
> 
> Interesting.
> Could check the command line in verbose mode to see where is -lrt please?

Here you are.
-lrt is in separate line:

gcc -o test -m64 -pthread  -march=native -DRTE_MACHINE_CPUFLAG_SSE 
-DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 
-DRTE_MACHINE_CPUFLAG_SSSE3 -DRTE_MACHINE_CPUFLAG_SSE4_1 
-DRTE_MACHINE_CPUFLAG_SSE4_2 -DRTE_MACHINE_CPUFLAG_AES 
-DRTE_MACHINE_CPUFLAG_PCLMULQDQ -DRTE_MACHINE_CPUFLAG_AVX  
-I/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/include -include 
/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/include/rte_config.h -O3 
-W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations 
-Wold-style-definition -Wpointer-arith -Wcast-align -Wnested-externs 
-Wcast-qual -Wformat-nonliteral -Wformat-security -Wundef -Wwrite-strings 
-Werror -Wno-missing-field-initializers -Wno-uninitialized -D_GNU_SOURCE 
commands.o test.o resource.o test_resource.o test_resource_c.res.o 
test_prefetch.o test_byteorder.o test_per_lcore.o test_atomic.o test_malloc.o 
test_cycles.o test_spinlock.o test_memory.o test_memzone.o test_ring.o 
test_ring_perf.o test_pmd_perf.o test_table.o test_table_pipeline.o 
test_table_tables.o test_table_ports.o test_table_combined.o test_table_acl.o 
test_rwlock.o test_timer.o test_timer_perf.o test_timer_racecond.o 
test_mempool.o test_mempool_perf.o test_mbuf.o test_logs.o test_memcpy.o 
test_memcpy_perf.o test_hash.o test_thash.o test_hash_perf.o 
test_hash_functions.o test_hash_scaling.o test_hash_multiwriter.o test_lpm.o 
test_lpm_perf.o test_lpm6.o test_lpm6_perf.o test_debug.o test_errno.o 
test_tailq.o test_string_fns.o test_cpuflags.o test_mp_secondary.o 
test_eal_flags.o test_eal_fs.o test_alarm.o test_interrupts.o test_version.o 
test_func_reentrancy.o test_cmdline.o test_cmdline_num.o 
test_cmdline_etheraddr.o test_cmdline_portlist.o test_cmdline_ipaddr.o 
test_cmdline_cirbuf.o test_cmdline_string.o test_cmdline_lib.o test_red.o 
test_sched.o test_meter.o test_kni.o test_power.o test_power_acpi_cpufreq.o 
test_power_kvm_vm.o test_common.o test_distributor.o test_distributor_perf.o 
test_reorder.o test_devargs.o virtual_pmd.o packet_burst_generator.o test_acl.o 
test_link_bonding.o test_link_bonding_mode4.o test_link_bonding_rssconf.o 
test_pmd_ring.o test_pmd_ring_perf.o test_cryptodev_aes.o test_cryptodev_perf.o 
test_cryptodev.o test_kvargs.o -Wl,
-lrt 
-Wl,-lm -L/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/lib 
-Wl,-lrte_kni -Wl,-lrte_pipeline -Wl,-lrte_table -Wl,-lrte_port -Wl,-lrte_pdump 
-Wl,-lrte_distributor -Wl,-lrte_reorder -Wl,-lrte_ip_frag -Wl,-lrte_meter 
-Wl,-lrte_sched -Wl,-lrte_lpm -Wl,--whole-archive -Wl,-lrte_acl 
-Wl,--no-whole-archive -Wl,-lrte_jobstats -Wl,-lrte_power -Wl,--whole-archive 
-Wl,-lrte_timer -Wl,-lrte_hash -Wl,-lrte_vhost -Wl,-lrte_kvargs -Wl,-lrte_mbuf 
-Wl,-lethdev -Wl,-lrte_cryptodev -Wl,-lrte_mempool -Wl,-lrte_ring -Wl,-lrte_eal 
-Wl,-lrte_cmdline -Wl,-lrte_cfgfile -Wl,-lrte_pmd_bond -Wl,-lrte_pmd_af_packet 
-Wl,-lrte_pmd_bnxt -Wl,-lrte_pmd_cxgbe -Wl,-lrte_pmd_e1000 -Wl,-lrte_pmd_ena 
-Wl,-lrte_pmd_enic -Wl,-lrte_pmd_fm10k -Wl,-lrte_pmd_i40e -Wl,-lrte_pmd_ixgbe 
-Wl,-lrte_pmd_null -Wl,-lrte_pmd_ring -Wl,-lrte_pmd_virtio -Wl,-lrte_pmd_vhost 
-Wl,-lrte_pmd_vmxnet3_uio -Wl,-lrte_pmd_null_crypto -Wl,--no-whole-archive 
-Wl,-ldl -Wl,-export-dynamic -Wl,-export-dynamic -Wl,-export-dynamic 
-L/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/lib -Wl,--as-needed 
-Wl,-Map=test.map -Wl,--cref
/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o):
 In function `get_tsc_freq':
eal_timer.c:(.text+0x128): undefined reference to `clock_gettime'

[dpdk-dev] [PATCH] app/pdump: cleanup rte rings upon failures

[Reshma Pattan]

Function create_mp_ring_vdev() for failure cases exits without
freeing the created rte rings, because of this pdump tool cannot be
rerun successfully. Added rte ring cleanup logic upon failures.

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan 
---
 app/pdump/main.c | 67 +---
 1 file changed, 49 insertions(+), 18 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index e0ff8be..b76cfd0 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -525,6 +525,26 @@ free_ring_data(struct rte_ring *ring, uint8_t vdev_id,
 }

 static void
+cleanup_rings(void)
+{
+   int i;
+   struct pdump_tuples *pt;
+
+   for (i = 0; i < num_tuples; i++) {
+   pt = _t[i];
+
+   if (pt->device_id)
+   free(pt->device_id);
+
+   /* free the rings */
+   if (pt->rx_ring)
+   rte_ring_free(pt->rx_ring);
+   if (pt->tx_ring)
+   rte_ring_free(pt->tx_ring);
+   }
+}
+
+static void
 cleanup_pdump_resources(void)
 {
int i;
@@ -545,16 +565,8 @@ cleanup_pdump_resources(void)
free_ring_data(pt->rx_ring, pt->rx_vdev_id, >stats);
if (pt->dir & RTE_PDUMP_FLAG_TX)
free_ring_data(pt->tx_ring, pt->tx_vdev_id, >stats);
-
-   if (pt->device_id)
-   free(pt->device_id);
-
-   /* free the rings */
-   if (pt->rx_ring)
-   rte_ring_free(pt->rx_ring);
-   if (pt->tx_ring)
-   rte_ring_free(pt->tx_ring);
}
+   cleanup_rings();
 }

 static void
@@ -630,10 +642,12 @@ create_mp_ring_vdev(void)
MBUF_POOL_CACHE_SIZE, 0,
pt->mbuf_data_size,
rte_socket_id());
-   if (mbuf_pool == NULL)
+   if (mbuf_pool == NULL) {
+   cleanup_rings();
rte_exit(EXIT_FAILURE,
"Mempool creation failed: %s\n",
rte_strerror(rte_errno));
+   }
}
pt->mp = mbuf_pool;

@@ -643,19 +657,23 @@ create_mp_ring_vdev(void)
snprintf(ring_name, SIZE, RX_RING, i);
pt->rx_ring = rte_ring_create(ring_name, pt->ring_size,
rte_socket_id(), 0);
-   if (pt->rx_ring == NULL)
+   if (pt->rx_ring == NULL) {
+   cleanup_rings();
rte_exit(EXIT_FAILURE, "%s:%s:%d\n",
rte_strerror(rte_errno),
__func__, __LINE__);
+   }

/* create tx_ring */
snprintf(ring_name, SIZE, TX_RING, i);
pt->tx_ring = rte_ring_create(ring_name, pt->ring_size,
rte_socket_id(), 0);
-   if (pt->tx_ring == NULL)
+   if (pt->tx_ring == NULL) {
+   cleanup_rings();
rte_exit(EXIT_FAILURE, "%s:%s:%d\n",
rte_strerror(rte_errno),
__func__, __LINE__);
+   }

/* create vdevs */
(pt->rx_vdev_stream_type == IFACE) ?
@@ -663,10 +681,12 @@ create_mp_ring_vdev(void)
pt->rx_dev) :
snprintf(vdev_args, SIZE, VDEV_PCAP, RX_STR, i,
pt->rx_dev);
-   if (rte_eth_dev_attach(vdev_args, ) < 0)
+   if (rte_eth_dev_attach(vdev_args, ) < 0) {
+   cleanup_rings();
rte_exit(EXIT_FAILURE,
"vdev creation failed:%s:%d\n",
__func__, __LINE__);
+   }
pt->rx_vdev_id = portid;

/* configure vdev */
@@ -680,10 +700,13 @@ create_mp_ring_vdev(void)
pt->tx_dev) :
snprintf(vdev_args, SIZE, VDEV_PCAP, TX_STR, i,
pt->tx_dev);
-   if (rte_eth_dev_attach(vdev_args, ) < 0)
+   if (rte_eth_dev_attach(vdev_args,
+   ) < 0) {
+   cleanup_rings();

[dpdk-dev] [PATCH] eal: fix check number of bytes from read function

[Jastrzebski, MichalX K]

> -Original Message-
> From: Gonzalez Monroy, Sergio
> Sent: Thursday, July 21, 2016 4:37 PM
> To: Jastrzebski, MichalX K ; Richardson,
> Bruce 
> Cc: dev at dpdk.org; Kobylinski, MichalX ;
> david.marchand at 6wind.com
> Subject: Re: [PATCH] eal: fix check number of bytes from read function
> 
> On 20/07/2016 15:24, Michal Jastrzebski wrote:
> > In rte_mem_virt2phy: Value returned from a function and indicating the
> > number of bytes was ignored. This could cause a wrong pfn (page frame
> > number) mask read from pagemap file.
> > When read returns less than the number of sizeof(uint64_t) bytes,
> > function rte_mem_virt2phy returns error.
> >
> > Coverity issue: 13212
> > Fixes: 40b966a211ab ("ivshmem: library changes for mmaping using
> > ivshmem").
> >
> > Signed-off-by: Michal Jastrzebski 
> > ---
> >   lib/librte_eal/linuxapp/eal/eal_memory.c |   12 ++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c
> b/lib/librte_eal/linuxapp/eal/eal_memory.c
> > index 42a29fa..05769fb 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> > @@ -158,7 +158,7 @@ rte_mem_lock_page(const void *virt)
> >   phys_addr_t
> >   rte_mem_virt2phy(const void *virtaddr)
> >   {
> > -   int fd;
> > +   int fd, retval;
> > uint64_t page, physaddr;
> > unsigned long virt_pfn;
> > int page_size;
> > @@ -209,11 +209,19 @@ rte_mem_virt2phy(const void *virtaddr)
> > close(fd);
> > return RTE_BAD_PHYS_ADDR;
> > }
> > -   if (read(fd, , sizeof(uint64_t)) < 0) {
> > +
> > +   retval = read(fd, , sizeof(uint64_t));
> > +   if (retval < 0) {
> > RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap:
> %s\n",
> > __func__, strerror(errno));
> > close(fd);
> > return RTE_BAD_PHYS_ADDR;
> > +   }   else if (retval >= 0 && retval < (int)sizeof(uint64_t)) {
> 
> Just a couple of nits, retval >= 0 it's already implicit, no need to do
> that check.
> 
> > +   RTE_LOG(ERR, EAL, "%s(): read %d bytes from
> /proc/self/pagemap "
> > +   "but expected %d: %s\n",
> > +   __func__, retval, (int)sizeof(uint64_t),
> strerror(errno));
> > +   close(fd);
> 
> Another nit, we could just close(fd) right after read, regardless of
> read being success or error as
> we close(fd) also on success just before exiting the function.
> 
> Other than that:
> 
> Acked-by: Sergio Gonzalez Monroy 
> 
> > +   return RTE_BAD_PHYS_ADDR;
> > }
> >
> > /*

Thanks Sergio,
I have sent v2 with the changes that You suggest

Michal.

[dpdk-dev] [dpdk-users] RSS Hash not working for XL710/X710 NICs for some RX mbuf sizes

[Take Ceara]

On Fri, Jul 22, 2016 at 2:31 PM, Take Ceara  wrote:
> Hi Beilei,
>
> On Fri, Jul 22, 2016 at 11:04 AM, Xing, Beilei  
> wrote:
>> Hi Ceara,
>>
>>> -Original Message-
>>> From: Take Ceara [mailto:dumitru.ceara at gmail.com]
>>> Sent: Thursday, July 21, 2016 6:58 PM
>>> To: Xing, Beilei 
>>> Cc: Zhang, Helin ; Wu, Jingjing
>>> ; dev at dpdk.org
>>> Subject: Re: [dpdk-dev] [dpdk-users] RSS Hash not working for XL710/X710
>>> NICs for some RX mbuf sizes
>>>
>>>
>>> Following your testpmd example run I managed to replicate the problem on
>>> my dpdk 16.04 setup like this:
>>>
>>> I have two X710 adapters connected back to back:
>>> $ ./tools/dpdk_nic_bind.py -s
>>>
>>> Network devices using DPDK-compatible driver
>>> 
>>> :01:00.3 'Ethernet Controller X710 for 10GbE SFP+' drv=igb_uio unused=
>>> :81:00.3 'Ethernet Controller X710 for 10GbE SFP+' drv=igb_uio unused=
>>>
>>> The firmware of the two adapters is up to date with the latest
>>> version: 5.04 (f5.0.40043 a1.5 n5.04 e24cd)
>>>
>>> I run testpmd with mbuf-size 1152 and txpktsize 1024 such that upon receival
>>> the whole mbuf (except headroom) is filled.
>>> I enabled RX IP checksum in hw and RX RSS hashing for UDP.
>>> With test-pmd forward mode "rxonly" and verbose 1 I see that incoming
>>> packets have PKT_RX_RSS_HASH set but the hash value is 0.
>>>
>>> ./testpmd -c 1 -n 4 -w :01:00.3 -w :81:00.3 -- -i
>>> --coremask=0x0 --rxq=16 --txq=16 --mbuf-size 1152 --txpkts 1024 --
>>> enable-rx-cksum --rss-udp [...]
>>> testpmd> set verbose 1
>>> Change verbose level from 0 to 1
>>> testpmd> set fwd rxonly
>>> Set rxonly packet forwarding mode
>>> testpmd> start tx_first
>>>   rxonly packet forwarding - CRC stripping disabled - packets/burst=32
>>>   nb forwarding cores=16 - nb forwarding ports=2
>>>   RX queues=16 - RX desc=128 - RX free threshold=32
>>>   RX threshold registers: pthresh=8 hthresh=8 wthresh=0
>>>   TX queues=16 - TX desc=512 - TX free threshold=32
>>>   TX threshold registers: pthresh=32 hthresh=0 wthresh=0
>>>   TX RS bit threshold=32 - TXQ flags=0xf01 port 0/queue 1: received 32
>>> packets
>>>   src=68:05:CA:38:6D:63 - dst=02:00:00:00:00:01 - type=0x0800 -
>>> length=1024 - nb_segs=1 - RSS hash=0x0 - RSS queue=0x1 - (outer) L2
>>> type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP
>>> - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type:
>>> Unknown - Inner L4 type: Unknown
>>>  - Receive queue=0x1
>>>   PKT_RX_RSS_HASH
>>>   src=68:05:CA:38:6D:63 - dst=02:00:00:00:00:01 - type=0x0800 -
>>> length=1024 - nb_segs=1 - RSS hash=0x0 - RSS queue=0x1 - (outer) L2
>>> type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP
>>> - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type:
>>> Unknown - Inner L4 type: Unknown
>>>  - Receive queue=0x1
>>>   PKT_RX_RSS_HASH
>>
>> What's the source ip address and destination ip address of the packet you 
>> sent to port 0? Could you try to change ip address or port number to observe 
>> if hash value changes? I remember I saw hash value was 0 before, but with 
>> different ip address, there'll be different hash values.
>
> I was using the test-pmd "txonly" implementation which sends fixed UDP
> packets from 192.168.0.1:1024 -> 192.168.0.2:1024.
>
> I changed the test-pmd tx_only code so that it sends traffic with
> incremental destination IP: 192.168.0.1:1024 -> [192.168.0.2,
> 192.168.0.12]:1024
> I also dumped the source and destination IPs in the "rxonly"
> pkt_burst_receive function.
> Then I see that packets are indeed sent to different queues but the
> mbuf->hash.rss value is still 0.
>
> ./testpmd -c 1 -n 4 -w :01:00.3 -w :81:00.3 -- -i
> --coremask=0x0 --rxq=16 --txq=16 --mbuf-size 1152 --txpkts 1024
> --enable-rx-cksum --rss-udp
>
> [...]
>
>  - Receive queue=0xf
>   PKT_RX_RSS_HASH
>   src=68:05:CA:38:6D:63 - dst=02:00:00:00:00:01 - type=0x0800 -
> length=1024 - nb_segs=1 - RSS queue=0xa - (outer) L2 type: ETHER -
> (outer) L3 type: IPV4_EXT_UNKNOWN SIP=C0A80001 DIP=C0A80006 - (outer)
> L4 type: UDP - Tunnel type: Unknown - RSS hash=0x0 - Inner L2 type:
> Unknown - RSS queue=0xf - RSS queue=0x7 - (outer) L2 type: ETHER -
> (outer) L3 type: IPV4_EXT_UNKNOWN SIP=C0A80001 DIP=C0A80007 - (outer)
> L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner
> L3 type: Unknown - Inner L4 type: Unknown
>  - Receive queue=0x7
>   PKT_RX_RSS_HASH
>   src=68:05:CA:38:6D:63 - dst=02:00:00:00:00:01 - (outer) L2 type:
> ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN SIP=C0A80001 DIP=C0A80009 -
> type=0x0800 - length=1024 - nb_segs=1 - Inner L3 type: Unknown - Inner
> L4 type: Unknown - RSS hash=0x0 - (outer) L4 type: UDP - Tunnel type:
> Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - RSS
> queue=0x7 - Inner L4 type: Unknown
>
> [...]
>
> testpmd> stop
>   --- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 ---
>   RX-packets: 

[dpdk-dev] [dpdk-users] RSS Hash not working for XL710/X710 NICs for some RX mbuf sizes

[Take Ceara]

Hi Beilei,

On Fri, Jul 22, 2016 at 11:04 AM, Xing, Beilei  wrote:
> Hi Ceara,
>
>> -Original Message-
>> From: Take Ceara [mailto:dumitru.ceara at gmail.com]
>> Sent: Thursday, July 21, 2016 6:58 PM
>> To: Xing, Beilei 
>> Cc: Zhang, Helin ; Wu, Jingjing
>> ; dev at dpdk.org
>> Subject: Re: [dpdk-dev] [dpdk-users] RSS Hash not working for XL710/X710
>> NICs for some RX mbuf sizes
>>
>>
>> Following your testpmd example run I managed to replicate the problem on
>> my dpdk 16.04 setup like this:
>>
>> I have two X710 adapters connected back to back:
>> $ ./tools/dpdk_nic_bind.py -s
>>
>> Network devices using DPDK-compatible driver
>> 
>> :01:00.3 'Ethernet Controller X710 for 10GbE SFP+' drv=igb_uio unused=
>> :81:00.3 'Ethernet Controller X710 for 10GbE SFP+' drv=igb_uio unused=
>>
>> The firmware of the two adapters is up to date with the latest
>> version: 5.04 (f5.0.40043 a1.5 n5.04 e24cd)
>>
>> I run testpmd with mbuf-size 1152 and txpktsize 1024 such that upon receival
>> the whole mbuf (except headroom) is filled.
>> I enabled RX IP checksum in hw and RX RSS hashing for UDP.
>> With test-pmd forward mode "rxonly" and verbose 1 I see that incoming
>> packets have PKT_RX_RSS_HASH set but the hash value is 0.
>>
>> ./testpmd -c 1 -n 4 -w :01:00.3 -w :81:00.3 -- -i
>> --coremask=0x0 --rxq=16 --txq=16 --mbuf-size 1152 --txpkts 1024 --
>> enable-rx-cksum --rss-udp [...]
>> testpmd> set verbose 1
>> Change verbose level from 0 to 1
>> testpmd> set fwd rxonly
>> Set rxonly packet forwarding mode
>> testpmd> start tx_first
>>   rxonly packet forwarding - CRC stripping disabled - packets/burst=32
>>   nb forwarding cores=16 - nb forwarding ports=2
>>   RX queues=16 - RX desc=128 - RX free threshold=32
>>   RX threshold registers: pthresh=8 hthresh=8 wthresh=0
>>   TX queues=16 - TX desc=512 - TX free threshold=32
>>   TX threshold registers: pthresh=32 hthresh=0 wthresh=0
>>   TX RS bit threshold=32 - TXQ flags=0xf01 port 0/queue 1: received 32
>> packets
>>   src=68:05:CA:38:6D:63 - dst=02:00:00:00:00:01 - type=0x0800 -
>> length=1024 - nb_segs=1 - RSS hash=0x0 - RSS queue=0x1 - (outer) L2
>> type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP
>> - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type:
>> Unknown - Inner L4 type: Unknown
>>  - Receive queue=0x1
>>   PKT_RX_RSS_HASH
>>   src=68:05:CA:38:6D:63 - dst=02:00:00:00:00:01 - type=0x0800 -
>> length=1024 - nb_segs=1 - RSS hash=0x0 - RSS queue=0x1 - (outer) L2
>> type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP
>> - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type:
>> Unknown - Inner L4 type: Unknown
>>  - Receive queue=0x1
>>   PKT_RX_RSS_HASH
>
> What's the source ip address and destination ip address of the packet you 
> sent to port 0? Could you try to change ip address or port number to observe 
> if hash value changes? I remember I saw hash value was 0 before, but with 
> different ip address, there'll be different hash values.

I was using the test-pmd "txonly" implementation which sends fixed UDP
packets from 192.168.0.1:1024 -> 192.168.0.2:1024.

I changed the test-pmd tx_only code so that it sends traffic with
incremental destination IP: 192.168.0.1:1024 -> [192.168.0.2,
192.168.0.12]:1024
I also dumped the source and destination IPs in the "rxonly"
pkt_burst_receive function.
Then I see that packets are indeed sent to different queues but the
mbuf->hash.rss value is still 0.

./testpmd -c 1 -n 4 -w :01:00.3 -w :81:00.3 -- -i
--coremask=0x0 --rxq=16 --txq=16 --mbuf-size 1152 --txpkts 1024
--enable-rx-cksum --rss-udp

[...]

 - Receive queue=0xf
  PKT_RX_RSS_HASH
  src=68:05:CA:38:6D:63 - dst=02:00:00:00:00:01 - type=0x0800 -
length=1024 - nb_segs=1 - RSS queue=0xa - (outer) L2 type: ETHER -
(outer) L3 type: IPV4_EXT_UNKNOWN SIP=C0A80001 DIP=C0A80006 - (outer)
L4 type: UDP - Tunnel type: Unknown - RSS hash=0x0 - Inner L2 type:
Unknown - RSS queue=0xf - RSS queue=0x7 - (outer) L2 type: ETHER -
(outer) L3 type: IPV4_EXT_UNKNOWN SIP=C0A80001 DIP=C0A80007 - (outer)
L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner
L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x7
  PKT_RX_RSS_HASH
  src=68:05:CA:38:6D:63 - dst=02:00:00:00:00:01 - (outer) L2 type:
ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN SIP=C0A80001 DIP=C0A80009 -
type=0x0800 - length=1024 - nb_segs=1 - Inner L3 type: Unknown - Inner
L4 type: Unknown - RSS hash=0x0 - (outer) L4 type: UDP - Tunnel type:
Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - RSS
queue=0x7 - Inner L4 type: Unknown

[...]

testpmd> stop
  --- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 ---
  RX-packets: 0  TX-packets: 32 TX-dropped: 0
  --- Forward Stats for RX Port= 0/Queue= 1 -> TX Port= 1/Queue= 1 ---
  RX-packets: 59 TX-packets: 32 TX-dropped: 

[dpdk-dev] em1000 driver lockup in KVM

[Prabahar Radhakrishnan]

Hi,
   I was running a dpdk application with e1000 driver and I am facing a
rare driver lockup condition.  Under this condition, what I am seeing is
that the Receive side locks up.  The transmit side is fine.  When I execute
"rte_eth_dev_stop()/rte_eth_dev_start()" from within the VM, the dpdk
application recovers.  The lockup happens once in 2 or 3 days if I
continuously ping an IP address serviced by the dpdk application.  Once it
locks up, what I am noticing is that even though packets are coming in, the
driver is not populating them and signaling the event.  In other words, in
"eth_em_recv_pkts" call, I am not seeing "E1000_RXD_STAT_DD" being set for
status, and the driver bails out.  If anyone has come across such
condition, please let me know?  Appreciate the help.

Environment:  I have a VM running on top of Ubuntu/KVM.
Application: Running dpdk-2.0.0.
VM: uses e1000 driver.
On the KVM side, our port is sitting in a bridge.  There are other VNFs
that we use for testing and some of them are using virtio driver.

Thank You
regards Prab

[dpdk-dev] [PATCH] doc: announce driver name changes

[Ferruh Yigit]

On 7/22/2016 1:54 PM, Adrien Mazarguil wrote:
> Hi Pablo,
> 
> On Fri, Jul 22, 2016 at 12:37:22PM +, De Lara Guarch, Pablo wrote:
>> Hi,
>>
>>> -Original Message-
>>> From: De Lara Guarch, Pablo
>>> Sent: Saturday, July 09, 2016 5:57 PM
>>> To: dev at dpdk.org
>>> Cc: Mcnamara, John; De Lara Guarch, Pablo
>>> Subject: [PATCH] doc: announce driver name changes
>>>
>>> Driver names for all the supported devices in DPDK do not have
>>> a naming convention. Some are using a prefix, some are not
>>> and some have long names. Driver names are used when creating
>>> virtual devices, so it is useful to have consistency in the names.
>>>
>>> Signed-off-by: Pablo de Lara 
>>> ---
>>>  doc/guides/rel_notes/deprecation.rst | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>> b/doc/guides/rel_notes/deprecation.rst
>>> index f502f86..37d65c8 100644
>>> --- a/doc/guides/rel_notes/deprecation.rst
>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>> @@ -41,3 +41,8 @@ Deprecation Notices
>>>  * The mempool functions for single/multi producer/consumer are
>>> deprecated and
>>>will be removed in 16.11.
>>>It is replaced by rte_mempool_generic_get/put functions.
>>> +
>>> +* Driver names are quite inconsistent among each others and they will be
>>> +  renamed to something more consistent (net_ prefix for net drivers and
>>> +  crypto_ for crypto drivers) in 16.11. Some of these driver names are used
>>> +  publicly, to create virtual devices, so a deprecation notice is 
>>> necessary.
>>> --
>>> 2.7.4
>>
>> Any more comments on this (apart from Christian Ehrhardt's)?
> 
> Yes, since you're suggesting to prefix driver names, shall "librte_pmd_mlx5"
> really become "net_librte_pmd_mlx5" or shortened to "net_mlx5" instead?
> 
> What about using a '/' separator instead of '_'?
> 
> Will this impact directories as well ("net/mlx5" -> "net/net_mlx5")?
> 

For physical net devices, driver name is same as folder name (mlnx5,
ixgbe ...)

For virtual net devices, driver name is folder name with "eth_" prefix
(eth_pcap, eth_ring)

Driver names for net devices looks consistent already, I don't know
about crypto devices but if crypto driver names are inconsistent what do
you think renaming crypto drivers only?

Thanks,
ferruh

[dpdk-dev] [PATCH] doc: announce driver name changes

[De Lara Guarch, Pablo]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Friday, July 22, 2016 2:19 PM
> To: De Lara Guarch, Pablo; dev at dpdk.org; Mcnamara, John
> Subject: Re: [dpdk-dev] [PATCH] doc: announce driver name changes
> 
> On 7/22/2016 1:54 PM, Adrien Mazarguil wrote:
> > Hi Pablo,
> >
> > On Fri, Jul 22, 2016 at 12:37:22PM +, De Lara Guarch, Pablo wrote:
> >> Hi,
> >>
> >>> -Original Message-
> >>> From: De Lara Guarch, Pablo
> >>> Sent: Saturday, July 09, 2016 5:57 PM
> >>> To: dev at dpdk.org
> >>> Cc: Mcnamara, John; De Lara Guarch, Pablo
> >>> Subject: [PATCH] doc: announce driver name changes
> >>>
> >>> Driver names for all the supported devices in DPDK do not have
> >>> a naming convention. Some are using a prefix, some are not
> >>> and some have long names. Driver names are used when creating
> >>> virtual devices, so it is useful to have consistency in the names.
> >>>
> >>> Signed-off-by: Pablo de Lara 
> >>> ---
> >>>  doc/guides/rel_notes/deprecation.rst | 5 +
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>> b/doc/guides/rel_notes/deprecation.rst
> >>> index f502f86..37d65c8 100644
> >>> --- a/doc/guides/rel_notes/deprecation.rst
> >>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>> @@ -41,3 +41,8 @@ Deprecation Notices
> >>>  * The mempool functions for single/multi producer/consumer are
> >>> deprecated and
> >>>will be removed in 16.11.
> >>>It is replaced by rte_mempool_generic_get/put functions.
> >>> +
> >>> +* Driver names are quite inconsistent among each others and they will
> be
> >>> +  renamed to something more consistent (net_ prefix for net drivers and
> >>> +  crypto_ for crypto drivers) in 16.11. Some of these driver names are
> used
> >>> +  publicly, to create virtual devices, so a deprecation notice is 
> >>> necessary.
> >>> --
> >>> 2.7.4
> >>
> >> Any more comments on this (apart from Christian Ehrhardt's)?
> >
> > Yes, since you're suggesting to prefix driver names, shall "librte_pmd_mlx5"
> > really become "net_librte_pmd_mlx5" or shortened to "net_mlx5" instead?
> >
> > What about using a '/' separator instead of '_'?
> >
> > Will this impact directories as well ("net/mlx5" -> "net/net_mlx5")?
> >

We will leave these untouched, although I don't think renaming the directories 
was necessary.

> 
> For physical net devices, driver name is same as folder name (mlnx5,
> ixgbe ...)
> 
> For virtual net devices, driver name is folder name with "eth_" prefix
> (eth_pcap, eth_ring)
> 
> Driver names for net devices looks consistent already, I don't know
> about crypto devices but if crypto driver names are inconsistent what do
> you think renaming crypto drivers only?

Sure, as long as virtual Ethernet devices are consistent, I think it is ok.
My main intention here was to have consistent (and short) driver names,
to call rte_eal_vdev_init (or --vdev in command line).

Thanks,
Pablo

> 
> Thanks,
> ferruh

[dpdk-dev] [PATCH] ethdev: support PCI domains

[Stephen Hemminger]

On Fri, 22 Jul 2016 11:34:10 -0400
Sinan Kaya  wrote:

> The current code is enumerating devices based on bus, device and function
> pairs. This does not work well for architectures with multiple PCI
> segments/domains. Multiple PCI devices will have the same BDF value but
> different segment numbers (01:01:01.0 and 02:01:01.0) for instance.
> 
> Adding segment numbers to device naming so that we can uniquely identify
> devices.
> 
> Signed-off-by: Sinan Kaya 

I ran into this yes. There is a small risk of breaking some application that
assumed something about names though.

Acked-by: Stephen Hemminger 

[dpdk-dev] [PATCH 2/2] net/i40e: avoid unsafe tailq element removal

[Pablo de Lara]

i40e driver was removing elements when iterating tailq lists
with TAILQ_FOREACH macro, which is not safe.
Instead, TAILQ_FOREACH_SAFE macro is used when removing/freeing
these elements.

Fixes: 4861cde46116 ("i40e: new poll mode driver")
Fixes: 440499cf5376 ("net/i40e: support floating VEB")

Signed-off-by: Pablo de Lara 
---
 drivers/net/i40e/i40e_ethdev.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 84c86aa..11a5804 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -31,7 +31,6 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */

-#include 
 #include 
 #include 
 #include 
@@ -51,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 

 #include "i40e_logs.h"
 #include "base/i40e_prototype.h"
@@ -4094,6 +4094,7 @@ i40e_vsi_release(struct i40e_vsi *vsi)
struct i40e_pf *pf;
struct i40e_hw *hw;
struct i40e_vsi_list *vsi_list;
+   void *temp;
int ret;
struct i40e_mac_filter *f;
uint16_t user_param = vsi->user_param;
@@ -4106,7 +4107,7 @@ i40e_vsi_release(struct i40e_vsi *vsi)

/* VSI has child to attach, release child first */
if (vsi->veb) {
-   TAILQ_FOREACH(vsi_list, >veb->head, list) {
+   TAILQ_FOREACH_SAFE(vsi_list, >veb->head, list, temp) {
if (i40e_vsi_release(vsi_list->vsi) != I40E_SUCCESS)
return -1;
TAILQ_REMOVE(>veb->head, vsi_list, list);
@@ -4115,7 +4116,7 @@ i40e_vsi_release(struct i40e_vsi *vsi)
}

if (vsi->floating_veb) {
-   TAILQ_FOREACH(vsi_list, >floating_veb->head, list) {
+   TAILQ_FOREACH_SAFE(vsi_list, >floating_veb->head, list, 
temp) {
if (i40e_vsi_release(vsi_list->vsi) != I40E_SUCCESS)
return -1;
TAILQ_REMOVE(>floating_veb->head, vsi_list, list);
@@ -4124,7 +4125,7 @@ i40e_vsi_release(struct i40e_vsi *vsi)

/* Remove all macvlan filters of the VSI */
i40e_vsi_remove_all_macvlan_filter(vsi);
-   TAILQ_FOREACH(f, >mac_list, next)
+   TAILQ_FOREACH_SAFE(f, >mac_list, next, temp)
rte_free(f);

if (vsi->type != I40E_VSI_MAIN &&
@@ -4682,6 +4683,7 @@ i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on)
 {
int i, num;
struct i40e_mac_filter *f;
+   void *temp;
struct i40e_mac_filter_info *mac_filter;
enum rte_mac_filter_type desired_filter;
int ret = I40E_SUCCESS;
@@ -4706,7 +4708,7 @@ i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on)
i = 0;

/* Remove all existing mac */
-   TAILQ_FOREACH(f, >mac_list, next) {
+   TAILQ_FOREACH_SAFE(f, >mac_list, next, temp) {
mac_filter[i] = f->mac_info;
ret = i40e_vsi_delete_mac(vsi, >mac_info.mac_addr);
if (ret) {
-- 
2.7.4

[dpdk-dev] [PATCH 1/2] eal: add tailq safe iterator macro

[Pablo de Lara]

Removing/freeing elements elements within a TAILQ_FOREACH loop is not safe.
FreeBSD defines TAILQ_FOREACH_SAFE macro, which permits
these operations safely.
This patch defines this macro for Linux systems, where it is not defined.

Signed-off-by: Pablo de Lara 
---
 lib/librte_eal/common/include/rte_tailq.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_tailq.h 
b/lib/librte_eal/common/include/rte_tailq.h
index 4a686e6..cc3c0f1 100644
--- a/lib/librte_eal/common/include/rte_tailq.h
+++ b/lib/librte_eal/common/include/rte_tailq.h
@@ -155,6 +155,14 @@ void __attribute__((constructor, used)) tailqinitfn_ 
##t(void) \
rte_panic("Cannot initialize tailq: %s\n", t.name); \
 }

+/* This macro permits both remove and free var within the loop safely.*/
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+   for ((var) = TAILQ_FIRST((head));   \
+   (var) && ((tvar) = TAILQ_NEXT((var), field), 1);\
+   (var) = (tvar))
+#endif
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.7.4

[dpdk-dev] [PATCH 0/2] Safe tailq element removal in i40e driver

[Pablo de Lara]

i40e driver was removing elements when iterating tailq lists 
with TAILQ_FOREACH macro, which is not safe.
Instead, TAILQ_FOREACH_SAFE macro is used when removing/freeing
these elements, which is defined in DPDK if it is not already
defined (in FreeBSD).

Pablo de Lara (2):
  eal: add tailq safe iterator macro
  net/i40e: avoid unsafe tailq element removal

 drivers/net/i40e/i40e_ethdev.c| 12 +++-
 lib/librte_eal/common/include/rte_tailq.h | 11 +++
 2 files changed, 18 insertions(+), 5 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17

[Azarewicz, PiotrX T]

> > > > Tested-by: Yongjie Gu 
> > >
> > > Applied
> >
> > OS: UB1204
> > GCC: 4.6.4
> > Kernel: 3.13.0-45
> > glibc 2.15
> >
> > x86_64-native-linuxapp-gcc: FAIL
> 
> Please don't be a bot and explain us the error you see.

I was trying rc3 + fix and latest (today) dpdk version. The same fail message:

/x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o): In function 
`get_tsc_freq':
eal_timer.c:(.text+0x128): undefined reference to `clock_gettime'
eal_timer.c:(.text+0x166): undefined reference to `clock_gettime'
/x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function 
`eal_alarm_callback':
eal_alarm.c:(.text+0xda): undefined reference to `clock_gettime'
/x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function 
`rte_eal_alarm_set':
eal_alarm.c:(.text+0x211): undefined reference to `clock_gettime'

[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17

[Azarewicz, PiotrX T]

> > Tested-by: Yongjie Gu 
> 
> Applied

OS: UB1204
GCC: 4.6.4
Kernel: 3.13.0-45
glibc 2.15

x86_64-native-linuxapp-gcc: FAIL

[dpdk-dev] [PATCH v2] validate_abi: build faster by augmenting make with job count

[Thomas Monjalon]

2016-07-20 15:02, Neil Horman:
> John Mcnamara and I were discussing enhacing the validate_abi script to build
> the dpdk tree faster with multiple jobs.  Theres no reason not to do it, so 
> this
> implements that requirement.  It uses a MAKE_JOBS variable that can be set by
> the user to limit the job count.  By default the job count is set to the 
> number
> of online cpus.
> 
> Signed-off-by: Neil Horman 

Applied, thanks

[dpdk-dev] [PATCH v2 0/2] cryptodev_start fixes

[Thomas Monjalon]

2016-07-22 10:20, De Lara Guarch, Pablo:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > It seems cryptodev_start() calls empty driver functions,
> > and is not called in the example applications.
> > 
> > This a v2 of patches from Hemant Agrawal and Akhil Goyal.
> > There are some improvements to make error messages more
> > consistent with existing ones and keep the existing error return.
> > 
> > Are they important fixes for 16.07? Please advise.
> > 
> > Hemant Agrawal (2):
> >   examples/l2fwd-crypto: call start function
> >   examples/ipsec-secgw: call start function
> 
> Series-acked-by: Pablo de Lara 

Applied, thanks

[dpdk-dev] [PATCH] doc: announce driver name changes

[De Lara Guarch, Pablo]

Hi,

> -Original Message-
> From: De Lara Guarch, Pablo
> Sent: Saturday, July 09, 2016 5:57 PM
> To: dev at dpdk.org
> Cc: Mcnamara, John; De Lara Guarch, Pablo
> Subject: [PATCH] doc: announce driver name changes
> 
> Driver names for all the supported devices in DPDK do not have
> a naming convention. Some are using a prefix, some are not
> and some have long names. Driver names are used when creating
> virtual devices, so it is useful to have consistency in the names.
> 
> Signed-off-by: Pablo de Lara 
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index f502f86..37d65c8 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -41,3 +41,8 @@ Deprecation Notices
>  * The mempool functions for single/multi producer/consumer are
> deprecated and
>will be removed in 16.11.
>It is replaced by rte_mempool_generic_get/put functions.
> +
> +* Driver names are quite inconsistent among each others and they will be
> +  renamed to something more consistent (net_ prefix for net drivers and
> +  crypto_ for crypto drivers) in 16.11. Some of these driver names are used
> +  publicly, to create virtual devices, so a deprecation notice is necessary.
> --
> 2.7.4

Any more comments on this (apart from Christian Ehrhardt's)?

[dpdk-dev] [PATCH v2] net/i40e: remove weak symbols

[Zoltan Kiss]

On 21/07/16 19:58, bynes adam wrote:
> On Wed, Jul 20, 2016 at 06:11:16PM +0100, Zoltan Kiss wrote:
> Hi, Kiss
>> Using weak symbols have a few issues with static linking:
>>
>> - normally the linker searches the .o files already linked, if your weak
>>   one is there, it won't check if there is a strong version
>> - unless the symbol is directly referred, but it's not the right thing to
>>   rely on
>> - or --whole-archive specified in the command line, which pulls in a lot
>>   of unwanted stuff
> --whole-archive on the other hand can ensure all the object files are linked,
> and the strong symbol will take precedence over weak symbol. So we don't need 
> to
> take care of the sequence of the object files in the ar or between ar.

--whole-archive is primarily for shared libraries, just as weak symbols. 
When people do static linking, their reason is often to avoid carrying 
around a big library while they only use a fraction of it.

>> - whole-archive also makes libtool dropping the library from the command
>>   line, which is what should happen with dynamic linking, but not with
>>   static (observed on Ubuntu 14.04). This is an important bug if you
>>   build a static library depending on DPDK
> you mean the libtool bug for --whole-archive?
> if it does, you shouldn't using the libtool,

GNU libtool is a quite common tool for building libraries, it's a bit 
painful sometimes, but it works. What else do you recommend? I mean, 
something which is proven to be better?

> BTW what's the circumstance you must use the libtool. using you own makefile 
> for
> the library or APP.

Building libraries which depend on DPDK

>>
>> This patch merges the two version and make the behaviour rely on the
>> config.
>>
>> If we can agree in the concept, I can send a series to fix this for the
>> other weak functions.
>>
>> Signed-off-by: Zoltan Kiss 
>> ---
>>
>> Notes:
>> v2: fix commit message
>>
>>  drivers/net/i40e/i40e_rxtx.c | 36 +++-
>>  drivers/net/i40e/i40e_rxtx_vec.c | 36 
>>  2 files changed, 35 insertions(+), 37 deletions(-)
>>
> From the original design, we follow the rule, we don't want the Macro in the 
> file
> to seperate the different Rx/Tx path for disabled/enabled vector 
> configuration.

And why is it better to compile two versions of the same function when 
you already know at the time of compilation which one do you want to use?

> Adam Bynes
>

[dpdk-dev] [PATCH] ip_pipeline: Fix for performance issue in downstream configuration

[Thomas Monjalon]

2016-07-18 11:23, Chokkalingam, SankarX:
> In TM, the read size should be lesser than the write size to improve 
> performance.
> This enables the TM ports to push maximum packets to the output port.
> 
> This fix changes the burst_read value from 64 to 24 in default_tm_params.
> 
> Signed-off-by: Chokkalingam, SankarX 
> Acked-by: Cristian Dumitrescu 

Applied, thanks

[dpdk-dev] [PATCH] ip_pipeline: Fix for Flow Classifcation IPv6 configuration error

[Thomas Monjalon]

2016-07-18 11:15, Chokkalingam, SankarX:
> IP Pipeline application with the configuration for Flow Classification IPV6 
> did not instantiate.
> Parse error in section "PIPELINE1": entry "dma_src_mask" too long
> 
> The dma_src_mask check in pipeline_passthrough_parse_args() is wrong.
> 
> This fix increases the length of dma_src_mask by 1 for NULL termination
> and corrected the validation of dma_src_mask length.
> This fix is also propagated to pipeline_fc_parse_args() for key_mask_str 
> validation.
> 
> Signed-off-by: Chokkalingam, SankarX 
> Acked-by: Cristian Dumitrescu 

Applied, thanks

[dpdk-dev] [PATCH] examples/performance-thread: add missing braces

[Thomas Monjalon]

2016-07-18 12:39, Mcnamara, John:
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Pablo de Lara
> > pthread_detach() function was returning 0 even when not calling
> > lthread_detach(), due to missing braces in conditional (extra indentation
> > was applied, giving a hint this is the correct fix).
> > 
> > Fixes: 433ba6228f9a ("examples/performance-thread: add pthread_shim app")
> > 
> > Signed-off-by: Pablo de Lara 
> 
> Tested-by: John McNamara 
> Acked-by: John McNamara 

Applied, thanks

[dpdk-dev] [PATCH v3] examples/vhost: fix perf regression

[Thomas Monjalon]

2016-07-21 09:34, Yuanhan Liu:
> On Thu, Jul 21, 2016 at 12:42:45AM +, Jianfeng Tan wrote:
> > We find significant perfermance drop introduced by below commit,
> > when vhost example is started with --mergeable 0 and inside vm,
> > kernel virtio-net driver is used to do ip based forwarding.
> > 
> > The commit, 859b480d5afd ("vhost: add guest offload setting"), adds
> > support for VIRTIO_NET_F_GUEST_TSO4 and VIRTIO_NET_F_GUEST_TSO6,
> > in vhost lib. But inside vhost example, the way to disable tso only
> > excludes the direction from virtio to vhost, but not the opposite
> > direction. When mergeable is disabled, it triggers big_packets path
> > of virtio-net driver to prepare to receive possible big packets with
> > size of 64K. Because mergeable is off, for each entry of avail ring,
> > virtio driver uses 19 desc chained together, with one desc pointing
> > to header, other 18 desc pointing to 4K-sized pages. But QEMU only
> > creates 256 desc entries for each vq, which results in that only 13
> > packets can be received. VM kernel can quickly handle those packets
> > and go to sleep (HLT).
> > 
> > As QEMU has no option to set the desc entries of a vq, so here,
> > we disable VIRTIO_NET_F_GUEST_TSO4 and VIRTIO_NET_F_GUEST_TSO6
> > with VIRTIO_NET_F_HOST_TSO4 and VIRTIO_NET_F_HOST_TSO6 when we
> > disable tso of vhost example, to avoid VM kernel virtio driver
> > go into big_packets path.
> > 
> > Fixes: 9fd72e3cbd29 ("examples/vhost: add virtio offload")
> > 
> > Reported-by: Qian Xu 
> > Signed-off-by: Jianfeng Tan 
> > ---
> > v3: reword commit log.
> 
> Yes, much better. One minor nit: you forgot to carry the Tested-by from
> Qian.
> 
> Acked-by: Yuanhan Liu 

Applied, thanks

[dpdk-dev] [PATCH] net/fm10k: fix RSS hash config

[Thomas Monjalon]

2016-07-21 16:24, Xiao Wang:
> Sometimes app just wants to update the RSS hash function and no RSS key
> update is needed, but fm10k pmd will return EINVAL for this case.
> 
> If the rss_key is NULL, we don't need to check the rss_key_len.
> 
> Fixes: 57033cdf8fdc ("fm10k: add PF RSS")
> 
> Reported-by: Xueqin Lin 
> Signed-off-by: Xiao Wang 

Applied, thanks

[dpdk-dev] [PATCH] examples/ipsec-secgw: fix GCC 4.5.x build error

[Thomas Monjalon]

> GCC 4.5.x does not handle well initializing anonymous union and/or
> structs.
> 
> To make the compiler happy we name those anonymous union/struct.
> 
> Fixes: 906257e965b7 ("examples/ipsec-secgw: support IPv6")
> 
> Signed-off-by: Sergio Gonzalez Monroy 

Applied, thanks

[dpdk-dev] [PATCH v2 2/2] examples/ipsec-secgw: call start function

[Thomas Monjalon]

From: Hemant Agrawal 

The usual device sequence is configure, queue setup and start.
Crypto device should be started before use.

Signed-off-by: Akhil Goyal 
Signed-off-by: Hemant Agrawal 
Signed-off-by: Thomas Monjalon 
---
 examples/ipsec-secgw/ipsec-secgw.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c 
b/examples/ipsec-secgw/ipsec-secgw.c
index 1ca144b..5d04eb3 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -1273,6 +1273,10 @@ cryptodevs_init(void)
_conf, dev_conf.socket_id))
rte_panic("Failed to setup queue %u for "
"cdev_id %u\n", 0, cdev_id);
+
+   if (rte_cryptodev_start(cdev_id))
+   rte_panic("Failed to start cryptodev %u\n",
+   cdev_id);
}

printf("\n");
-- 
2.7.0

[dpdk-dev] [PATCH v2 1/2] examples/l2fwd-crypto: call start function

[Thomas Monjalon]

From: Hemant Agrawal 

The usual device sequence is configure, queue setup and start.
Crypto device should be started before use.

Signed-off-by: Akhil Goyal 
Signed-off-by: Hemant Agrawal 
Signed-off-by: Thomas Monjalon 
---
 examples/l2fwd-crypto/main.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index dd39cc1..66397a0 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -1796,6 +1796,13 @@ initialize_cryptodevs(struct l2fwd_crypto_options 
*options, unsigned nb_ports,
return -1;
}

+   retval = rte_cryptodev_start(cdev_id);
+   if (retval < 0) {
+   printf("Failed to start device %u: error %d\n",
+   cdev_id, retval);
+   return -1;
+   }
+
l2fwd_enabled_crypto_mask |= (1 << cdev_id);

enabled_cdevs[cdev_id] = 1;
-- 
2.7.0

[dpdk-dev] [PATCH] ethdev: support PCI domains

[Sinan Kaya]

The current code is enumerating devices based on bus, device and function
pairs. This does not work well for architectures with multiple PCI
segments/domains. Multiple PCI devices will have the same BDF value but
different segment numbers (01:01:01.0 and 02:01:01.0) for instance.

Adding segment numbers to device naming so that we can uniquely identify
devices.

Signed-off-by: Sinan Kaya 
---
 lib/librte_ether/rte_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 0a6e3f1..929240f 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -226,7 +226,8 @@ rte_eth_dev_create_unique_device_name(char *name, size_t 
size,
 {
int ret;

-   ret = snprintf(name, size, "%d:%d.%d",
+   ret = snprintf(name, size, "%d:%d:%d.%d",
+   pci_dev->addr.domain,
pci_dev->addr.bus, pci_dev->addr.devid,
pci_dev->addr.function);
if (ret < 0)
-- 
1.8.2.1

[dpdk-dev] [PATCH v3] crypto: fix memory leak

[Thomas Monjalon]

2016-07-18 14:21, Pablo de Lara:
> When parsing the parameters for virtual device initialization,
> rte_kvargs structure was being freed only if there was an error,
> not when parsing was successful.
> 
> Coverity issue: 124568
> 
> Fixes: f3e764fa2fb7 ("cryptodev: uninline parameter parsing")
> 
> Signed-off-by: Pablo de Lara 
> Acked-by: Reshma Pattan 

Applied, thanks

[dpdk-dev] [PATCH] net/virtio_user: fix inconsistent name

[Thomas Monjalon]

2016-07-22 10:33, Yuanhan Liu:
> On Fri, Jul 22, 2016 at 02:24:47AM +, Jianfeng Tan wrote:
> > The commit cb6696d22023 ("drivers: update registration macro usage")
> > changes the name from virtio-user to virtio_user, because hyphen
> > cannot be used in a C symbol name. However, this commit does not
> > update the strings in docs and source code, which could lead to
> > failure to start this device as per the docs.
> > 
> > This patch updates related strings in the docs and source code.
> > 
> > Fixes: cb6696d22023 ("drivers: update registration macro usage")
> > 
> > Reported-by: Tiwei Bie 
> > Signed-off-by: Jianfeng Tan 
> 
> Acked-by: Yuanhan Liu 

Applied, thanks

[dpdk-dev] [PATCH] net/virtio_user: fix inconsistent name

[Yuanhan Liu]

On Fri, Jul 22, 2016 at 02:24:47AM +, Jianfeng Tan wrote:
> The commit cb6696d22023 ("drivers: update registration macro usage")
> changes the name from virtio-user to virtio_user, because hyphen
> cannot be used in a C symbol name. However, this commit does not
> update the strings in docs and source code, which could lead to
> failure to start this device as per the docs.
> 
> This patch updates related strings in the docs and source code.
> 
> Fixes: cb6696d22023 ("drivers: update registration macro usage")
> 
> Reported-by: Tiwei Bie 
> Signed-off-by: Jianfeng Tan 

Acked-by: Yuanhan Liu 

--yliu

[dpdk-dev] [PATCH] doc: add cryptodev shared library version to release notes

[Thomas Monjalon]

2016-07-20 13:31, Pablo de Lara:
> Signed-off-by: Pablo de Lara 

Applied, thanks

[dpdk-dev] [PATCH] net/fm10k: fix RSS hash config

[Thomas Monjalon]

2016-07-22 08:23, Chen, Jing D:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2016-07-21 09:35, Wang, Xiao W:
> > > From: Chen, Jing D
> > > > > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > > > > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > > > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev,
> > > > >
> > > > >   PMD_INIT_FUNC_TRACE();
> > > > >
> > > > > - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE *
> > > > > - FM10K_RSSRK_ENTRIES_PER_REG)
> > > > > + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE *
> > > > > + FM10K_RSSRK_ENTRIES_PER_REG))
> > > > >   return -EINVAL;
> > > > >
> > > > >   if (hf == 0)
> > > >
> > > > It's also possible that app wants to update rss key and not expect to 
> > > > update hash
> > > > function.
> > > > Is that indicate we shouldn't return error in case hf == 0?
> > > >
> > >
> > > If the app just wants to update RSS key, it needs to read out the RSS 
> > > config first,
> > then
> > > change only the key field. This is what testpmd does for this operation.
> > >
> > > hf == 0 will disable RSS feature, I think we should return error to 
> > > protect multi-
> > queue.
> > 
> > Jing, do you confirm we can apply this patch, please?
> I think we need some rework or more explanations here.

It is not reasonnable to wait RC5 for such a fix.
Either it is not important and postponed to 16.11 or you submit
a v2 very shortly for 16.07.
Please advise

[dpdk-dev] [PATCH 04/12] mbuf: add function to calculate a checksum

[Olivier Matz]

Hi Konstantin,

On 07/21/2016 12:51 PM, Ananyev, Konstantin wrote:
> Hi Olivier,
> 
>>
>> This function can be used to calculate the checksum of data embedded in 
>> mbuf, that can be composed of several segments.
>>
>> This function will be used by the virtio pmd in next commits to calculate 
>> the checksum in software in case the protocol is not recognized.
>>
>> Signed-off-by: Olivier Matz 
>> ---
>>  doc/guides/rel_notes/release_16_11.rst |  5 
>>  lib/librte_mbuf/rte_mbuf.c | 55 
>> --
>>  lib/librte_mbuf/rte_mbuf.h | 13 
>>  lib/librte_mbuf/rte_mbuf_version.map   |  1 +
>>  4 files changed, 72 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/release_16_11.rst 
>> b/doc/guides/rel_notes/release_16_11.rst
>> index 6a591e2..da70f3b 100644
>> --- a/doc/guides/rel_notes/release_16_11.rst
>> +++ b/doc/guides/rel_notes/release_16_11.rst
>> @@ -53,6 +53,11 @@ New Features
>>Added two new functions ``rte_get_rx_ol_flag_list()`` and
>>``rte_get_tx_ol_flag_list()`` to dump offload flags as a string.
>>
>> +* **Added a functions to calculate the checksum of data in a mbuf.**
>> +
>> +  Added a new function ``rte_pktmbuf_cksum()`` to process the checksum
>> + of  data embedded in an mbuf chain.
>> +
>>  Resolved Issues
>>  ---
>>
>> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index 
>> 56f37e6..0304245 100644
>> --- a/lib/librte_mbuf/rte_mbuf.c
>> +++ b/lib/librte_mbuf/rte_mbuf.c
>> @@ -60,6 +60,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
> 
> As a nit, do we need to introduce a dependency for librte_mbuf on librte_net?
> Might be better to put this functionality into librte_net?

I tried to have this code in librte_net, also when working on the
software packet type parser, and it did not really convince me, mainly
because librte_net is just header files as of today (it's not a real
library). But I can give it a try and post a patch so we can compare,
probably not in the coming days, but I keep a note on it.

Also, as I answered to Don, it would make less sense to move software
packet type parser in librte_net, since it's not a network feature but
more a dpdk mbuf feature. But software packet type needs network headers
definitions... so the cat is eating its tail ;)

Regards,
Olivier

[dpdk-dev] [PATCH v4] doc: flow bifurcation guide on Linux

[Thomas Monjalon]

> > Flow Bifurcation is a mechanism which uses features of advanced Ethernet
> > devices to split traffic between queues. It provides the capability to let
> > the kernel driver and DPDK driver co-exist and take advantage of both.
> > 
> > It is achieved by using SR-IOV and the NIC's advanced filtering. This
> > patch describes Flow Bifurcation and adds the user guide for ixgbe and
> > i40e NICs.
> > 
> > Signed-off-by: Jingjing Wu 
> 
> Acked-by: John McNamara 

Applied, thanks

[dpdk-dev] [PATCH] net/fm10k: fix RSS hash config

[Thomas Monjalon]

2016-07-21 09:35, Wang, Xiao W:
> From: Chen, Jing D
> > > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev,
> > >
> > >   PMD_INIT_FUNC_TRACE();
> > >
> > > - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE *
> > > - FM10K_RSSRK_ENTRIES_PER_REG)
> > > + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE *
> > > + FM10K_RSSRK_ENTRIES_PER_REG))
> > >   return -EINVAL;
> > >
> > >   if (hf == 0)
> > 
> > It's also possible that app wants to update rss key and not expect to 
> > update hash
> > function.
> > Is that indicate we shouldn't return error in case hf == 0?
> > 
> 
> If the app just wants to update RSS key, it needs to read out the RSS config 
> first, then
> change only the key field. This is what testpmd does for this operation.
> 
> hf == 0 will disable RSS feature, I think we should return error to protect 
> multi-queue.

Jing, do you confirm we can apply this patch, please?

[dpdk-dev] [PATCH v2] doc: add section on tested platforms and nics and OSes

[Yulong Pei]

Add new section on tested platforms and nics and OSes to the release notes.

Signed-off-by: Yulong Pei 
---
 doc/guides/rel_notes/release_16_07.rst | 117 +
 1 file changed, 106 insertions(+), 11 deletions(-)

diff --git a/doc/guides/rel_notes/release_16_07.rst 
b/doc/guides/rel_notes/release_16_07.rst
index d3a144f..b61fe07 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -350,25 +350,120 @@ The libraries prepended with a plus sign were 
incremented in this version.
 Tested Platforms
 

-.. This section should contain a list of platforms that were tested with this
-   release.
+#. SuperMicro 1U

-   The format is:
+   - BIOS: 1.0c
+   - Processor: Intel(R) Atom(TM) CPU C2758 @ 2.40GHz

-   #. Platform name.
+#. SuperMicro 1U

-  - Platform details.
-  - Platform details.
+   - BIOS: 1.0a
+   - Processor: Intel(R) Xeon(R) CPU D-1540 @ 2.00GHz
+   - Onboard NIC: Intel(R) X552/X557-AT (2x10G)
+
+ - Firmware-version: 0x81cf
+ - Device ID (PF/VF): 8086:15ad /8086:15a8
+
+   - kernel driver version: 4.2.5 (ixgbe)
+
+#. SuperMicro 2U
+
+   - BIOS: 1.0a
+   - Processor: Intel(R) Xeon(R) CPU E5-4667 v3 @ 2.00GHz
+
+#. Intel(R) Server board S2600GZ
+
+   - BIOS: SE5C600.86B.02.02.0002.122320131210
+   - Processor: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
+
+#. Intel(R) Server board W2600CR
+
+   - BIOS: SE5C600.86B.02.01.0002.082220131453
+   - Processor: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
+
+#. Intel(R) Server board S2600CWT
+
+   - BIOS: SE5C610.86B.01.01.0009.060120151350
+   - Processor: Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz
+
+#. Intel(R) Server board S2600WTT
+
+   - BIOS: SE5C610.86B.01.01.0005.101720141054
+   - Processor: Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz
+
+#. Intel(R) Server board S2600WTT
+
+   - BIOS: SE5C610.86B.11.01.0044.090120151156
+   - Processor: Intel(R) Xeon(R) CPU E5-2695 v4 @ 2.10GHz


 Tested NICs
 ---

-.. This section should contain a list of NICs that were tested with this 
release.
+#. Intel(R) Ethernet Controller X540-AT2
+
+   - Firmware version: 0x8389
+   - Device id (pf): 8086:1528
+   - Driver version: 3.23.2 (ixgbe)
+
+#. Intel(R) 82599ES 10 Gigabit Ethernet Controller

-   The format is:
+   - Firmware version: 0x61bf0001
+   - Device id (pf/vf): 8086:10fb / 8086:10ed
+   - Driver version: 4.0.1-k (ixgbe)

-   #. NIC name.
+#. Intel(R) Corporation Ethernet Connection X552/X557-AT 10GBASE-T
+
+   - Firmware version: 0x81cf
+   - Device id (pf/vf): 8086:15ad / 8086:15a8
+   - Driver version: 4.2.5 (ixgbe)
+
+#. Intel(R) Ethernet Converged Network Adapter X710-DA4 (4x10G)
+
+   - Firmware version: 5.04
+   - Device id (pf/vf): 8086:1572 / 8086:154c
+   - Driver version: 1.4.26 (i40e)
+
+#. Intel(R) Ethernet Converged Network Adapter X710-DA2 (2x10G)
+
+   - Firmware version: 5.04
+   - Device id (pf/vf): 8086:1572 / 8086:154c
+   - Driver version: 1.4.25 (i40e)
+
+#. Intel(R) Ethernet Converged Network Adapter XL710-QDA1 (1x40G)
+
+   - Firmware version: 5.04
+   - Device id (pf/vf): 8086:1584 / 8086:154c
+   - Driver version: 1.4.25 (i40e)
+
+#. Intel(R) Ethernet Converged Network Adapter XL710-QDA2 (2X40G)
+
+   - Firmware version: 5.04
+   - Device id (pf/vf): 8086:1583 / 8086:154c
+   - Driver version: 1.4.25 (i40e)
+
+#. Intel(R) Corporation I350 Gigabit Network Connection
+
+   - Firmware version: 1.48, 0x86e7
+   - Device id (pf/vf): 8086:1521 / 8086:1520
+   - Driver version: 5.2.13-k (igb)
+
+#. Intel(R) Ethernet Multi-host Controller FM1
+
+   - Firmware version: N/A
+   - Device id (pf/vf): 8086:15d0
+   - Driver version: 0.17.0.9 (fm10k)
+
+
+Tested OSes
+---

-  - NIC details.
-  - NIC details.
+   - CentOS 7.0
+   - Fedora 23
+   - Fedora 24
+   - FreeBSD 10.3
+   - Red Hat Enterprise Linux 7.2
+   - SUSE Enterprise Linux 12
+   - Ubuntu 15.10
+   - Ubuntu 16.04 LTS
+   - Wind River Linux 8
-- 
2.1.0

[dpdk-dev] [PATCH v2 0/2] cryptodev_start fixes

[De Lara Guarch, Pablo]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, July 22, 2016 10:44 AM
> To: Akhil Goyal; Hemant Agrawal; Doherty, Declan; De Lara Guarch, Pablo
> Cc: dev at dpdk.org
> Subject: [PATCH v2 0/2] cryptodev_start fixes
> 
> It seems cryptodev_start() calls empty driver functions,
> and is not called in the example applications.
> 
> This a v2 of patches from Hemant Agrawal and Akhil Goyal.
> There are some improvements to make error messages more
> consistent with existing ones and keep the existing error return.
> 
> Are they important fixes for 16.07? Please advise.
> 
> Hemant Agrawal (2):
>   examples/l2fwd-crypto: call start function
>   examples/ipsec-secgw: call start function
> 
>  examples/ipsec-secgw/ipsec-secgw.c | 4 
>  examples/l2fwd-crypto/main.c   | 7 +++
>  2 files changed, 11 insertions(+)
> 
> --
> 2.7.0

Series-acked-by: Pablo de Lara 

[dpdk-dev] [PATCH] unify tools naming

[Thomas Monjalon]

2016-07-22 09:46, Yuanhan Liu:
> On Wed, Jul 20, 2016 at 04:24:30PM +0200, Thomas Monjalon wrote:
> > The following tools may be installed system-wise.
> 
> Yes, indeed. Following is an example from dpdk package shipped in
> ubutun 16.04:
> 
> dpdk: /etc/dpdk/dpdk.conf
> dpdk: /etc/dpdk/interfaces
> dpdk: /etc/init.d/dpdk
> dpdk: /lib/dpdk/dpdk-init
> dpdk: /lib/systemd/system/dpdk.service
> dpdk: /sbin/dpdk_nic_bind
> dpdk: /usr/bin/dpdk_proc_info
> dpdk: /usr/bin/testpmd
> dpdk: /usr/share/doc/dpdk/changelog.Debian.gz
> dpdk: /usr/share/doc/dpdk/copyright
> dpdk: /usr/share/dpdk/tools/cpu_layout.py
> dpdk: /usr/share/dpdk/tools/dpdk_nic_bind.py
> dpdk: /usr/share/dpdk/tools/setup.sh
> 
> > It may be cleaner and more convenient to find them with the same
> > dpdk- prefix (especially for autocompletion).
> 
> Agreed.
> 
> > Moreover, the script dpdk_nic_bind.py deserves a new name because it is
> > not restricted to NICs and can be used for e.g. crypto.
> 
> Second that. But we might need doc the name change to let user aware of
> that.

There is a note in API changes section.
And every references in the doc are renamed.

> > These files are renamed:
> > pmdinfogen   -> dpdk-pmdinfogen
> > pmdinfo.py   -> dpdk-pmdinfo.py
> > dpdk_pdump   -> dpdk-pdump
> > dpdk_proc_info   -> dpdk-procinfo
> > dpdk_nic_bind.py -> dpdk-devbind.py
> > setup.sh -> dpdk-setup.sh
> > 
> > The tools pmdinfogen, pmdinfo.py and dpdk_pdump are new in 16.07.
> > 
> > The scripts dpdk_nic_bind.py and setup.sh may have been used with
> > previous releases by end users. That's why a symbolic link still
> > provide the old name in the installed tools directory.
> 
> I was about suggesting the same thing: yes, we should keep the
> backward compatibility.

Does it mean you ack this patch?

[dpdk-dev] [PATCH 05/12] mbuf: add new Rx checksum mbuf flags

[Olivier Matz]

Hi Stephen,

On 07/21/2016 11:22 PM, Stephen Hemminger wrote:
> On Thu, 21 Jul 2016 10:08:23 +0200
> Olivier Matz  wrote:
> 
>> +/**
>> + * Deprecated.
>> + * Checking this flag alone is deprecated: check the 2 bits of
>> + * PKT_RX_L4_CKSUM_MASK.
>> + * This flag was set when the L4 checksum of a packet was detected as
>> + * wrong by the hardware.
>> + */
>> +#define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)
>> +
>> +/**
>> + * Deprecated.
>> + * Checking this flag alone is deprecated: check the 2 bits of
>> + * PKT_RX_IP_CKSUM_MASK.
>> + * This flag was set when the IP checksum of a packet was detected as
>> + * wrong by the hardware.
>> + */
>> +#define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)
> 
> I think you should use the GCC deprecated attribute, not sure how though
> 

The reason why I did not use a macro poisoning here is because this flag
is still valid when used with the mask. Actually, checking this flag
alone still works and does the same as before but I wanted to highlight
that it should now be used with the mask.

Your comment makes me think that maybe the new flags could have
different names to avoid to keep old-style tests on this flag. On the
other hand, I think the name is already the good one, and doing this
would break the API and affect large pieces of code in dpdk.

Opinions are welcome here :)

Thanks for commenting
Olivier

[dpdk-dev] [PATCH 02/12] virtio: setup and start cq in configure callback

[Olivier Matz]

On 07/21/2016 11:15 PM, Stephen Hemminger wrote:
> On Thu, 21 Jul 2016 10:08:20 +0200
> Olivier Matz  wrote:
> 
>> +dev_info->max_rx_queues = (uint16_t)
>> +((VIRTIO_MAX_RX_QUEUES < hw->max_queue_pairs) ?
>> +VIRTIO_MAX_RX_QUEUES : hw->max_queue_pairs);
>> +dev_info->max_tx_queues = (uint16_t)
>> +((VIRTIO_MAX_TX_QUEUES < hw->max_queue_pairs) ?
>> +VIRTIO_MAX_TX_QUEUES : hw->max_queue_pairs);
> 
> cast here was always unnecessary.
> Why not use RTE_MIN()
> 

Yes, good idea, I'll do that for the v2.

Thanks,
Olivier

[dpdk-dev] [PATCH] unify tools naming

[Yuanhan Liu]

On Wed, Jul 20, 2016 at 04:24:30PM +0200, Thomas Monjalon wrote:
> The following tools may be installed system-wise.

Yes, indeed. Following is an example from dpdk package shipped in
ubutun 16.04:

dpdk: /etc/dpdk/dpdk.conf
dpdk: /etc/dpdk/interfaces
dpdk: /etc/init.d/dpdk
dpdk: /lib/dpdk/dpdk-init
dpdk: /lib/systemd/system/dpdk.service
dpdk: /sbin/dpdk_nic_bind
dpdk: /usr/bin/dpdk_proc_info
dpdk: /usr/bin/testpmd
dpdk: /usr/share/doc/dpdk/changelog.Debian.gz
dpdk: /usr/share/doc/dpdk/copyright
dpdk: /usr/share/dpdk/tools/cpu_layout.py
dpdk: /usr/share/dpdk/tools/dpdk_nic_bind.py
dpdk: /usr/share/dpdk/tools/setup.sh

> It may be cleaner and more convenient to find them with the same
> dpdk- prefix (especially for autocompletion).

Agreed.

> Moreover, the script dpdk_nic_bind.py deserves a new name because it is
> not restricted to NICs and can be used for e.g. crypto.

Second that. But we might need doc the name change to let user aware of
that.

> These files are renamed:
> pmdinfogen   -> dpdk-pmdinfogen
> pmdinfo.py   -> dpdk-pmdinfo.py
> dpdk_pdump   -> dpdk-pdump
> dpdk_proc_info   -> dpdk-procinfo
> dpdk_nic_bind.py -> dpdk-devbind.py
> setup.sh -> dpdk-setup.sh
> 
> The tools pmdinfogen, pmdinfo.py and dpdk_pdump are new in 16.07.
> 
> The scripts dpdk_nic_bind.py and setup.sh may have been used with
> previous releases by end users. That's why a symbolic link still
> provide the old name in the installed tools directory.

I was about suggesting the same thing: yes, we should keep the
backward compatibility.

--yliu

[dpdk-dev] [PATCH] net/fm10k: fix RSS hash config

[Chen, Jing D]

Hi, Thomas,


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, July 22, 2016 4:29 PM
> To: Chen, Jing D 
> Cc: dev at dpdk.org; Wang, Xiao W ; Lin, Xueqin
> 
> Subject: Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
> 
> 2016-07-22 08:23, Chen, Jing D:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-07-21 09:35, Wang, Xiao W:
> > > > From: Chen, Jing D
> > > > > > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > > > > > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > > > > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev
> *dev,
> > > > > >
> > > > > > PMD_INIT_FUNC_TRACE();
> > > > > >
> > > > > > -   if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE *
> > > > > > -   FM10K_RSSRK_ENTRIES_PER_REG)
> > > > > > +   if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE *
> > > > > > +   FM10K_RSSRK_ENTRIES_PER_REG))
> > > > > > return -EINVAL;
> > > > > >
> > > > > > if (hf == 0)
> > > > >
> > > > > It's also possible that app wants to update rss key and not expect to 
> > > > > update
> hash
> > > > > function.
> > > > > Is that indicate we shouldn't return error in case hf == 0?
> > > > >
> > > >
> > > > If the app just wants to update RSS key, it needs to read out the RSS 
> > > > config first,
> > > then
> > > > change only the key field. This is what testpmd does for this operation.
> > > >
> > > > hf == 0 will disable RSS feature, I think we should return error to 
> > > > protect
> multi-
> > > queue.
> > >
> > > Jing, do you confirm we can apply this patch, please?
> > I think we need some rework or more explanations here.
> 
> It is not reasonnable to wait RC5 for such a fix.
> Either it is not important and postponed to 16.11 or you submit
> a v2 very shortly for 16.07.
> Please advise

Please kindly merge.

[dpdk-dev] [PATCH] net/fm10k: fix RSS hash config

[Chen, Jing D]

Hi, Thomas,

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, July 22, 2016 4:22 PM
> To: Chen, Jing D 
> Cc: dev at dpdk.org; Wang, Xiao W ; Lin, Xueqin
> 
> Subject: Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
> 
> 2016-07-21 09:35, Wang, Xiao W:
> > From: Chen, Jing D
> > > > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > > > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev,
> > > >
> > > > PMD_INIT_FUNC_TRACE();
> > > >
> > > > -   if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE *
> > > > -   FM10K_RSSRK_ENTRIES_PER_REG)
> > > > +   if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE *
> > > > +   FM10K_RSSRK_ENTRIES_PER_REG))
> > > > return -EINVAL;
> > > >
> > > > if (hf == 0)
> > >
> > > It's also possible that app wants to update rss key and not expect to 
> > > update hash
> > > function.
> > > Is that indicate we shouldn't return error in case hf == 0?
> > >
> >
> > If the app just wants to update RSS key, it needs to read out the RSS 
> > config first,
> then
> > change only the key field. This is what testpmd does for this operation.
> >
> > hf == 0 will disable RSS feature, I think we should return error to protect 
> > multi-
> queue.
> 
> Jing, do you confirm we can apply this patch, please?
I think we need some rework or more explanations here.

[dpdk-dev] [PATCH] net/virtio_user: fix inconsistent name

[Jianfeng Tan]

The commit cb6696d22023 ("drivers: update registration macro usage")
changes the name from virtio-user to virtio_user, because hyphen
cannot be used in a C symbol name. However, this commit does not
update the strings in docs and source code, which could lead to
failure to start this device as per the docs.

This patch updates related strings in the docs and source code.

Fixes: cb6696d22023 ("drivers: update registration macro usage")

Reported-by: Tiwei Bie 
Signed-off-by: Jianfeng Tan 
---
 doc/guides/rel_notes/release_16_07.rst  |  2 +-
 doc/guides/sample_app_ug/vhost.rst  | 12 ++--
 drivers/net/virtio/virtio_ethdev.c  |  4 ++--
 drivers/net/virtio/virtio_user_ethdev.c |  6 +++---
 drivers/net/virtio/virtqueue.h  |  2 +-
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/doc/guides/rel_notes/release_16_07.rst 
b/doc/guides/rel_notes/release_16_07.rst
index d3a144f..0740d4f 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -90,7 +90,7 @@ New Features

 * **Virtio support for containers.**

-  Add a new virtual device, named virtio-user, to support virtio for 
containers.
+  Add a new virtual device, named virtio_user, to support virtio for 
containers.

   Known limitations:

diff --git a/doc/guides/sample_app_ug/vhost.rst 
b/doc/guides/sample_app_ug/vhost.rst
index a93e54d..2b7defc 100644
--- a/doc/guides/sample_app_ug/vhost.rst
+++ b/doc/guides/sample_app_ug/vhost.rst
@@ -834,19 +834,19 @@ The above message indicates that device 0 has been 
registered with MAC address c
 Any packets received on the NIC with these values is placed on the devices 
receive queue.
 When a virtio-net device transmits packets, the VLAN tag is added to the 
packet by the DPDK vhost sample code.

-Running virtio-user with vhost-switch
+Running virtio_user with vhost-switch
 -

-We can also use virtio-user with vhost-switch now.
-Virtio-user is a virtual device that can be run in a application (container) 
parallelly with vhost in the same OS,
+We can also use virtio_user with vhost-switch now.
+Virtio_user is a virtual device that can be run in a application (container) 
parallelly with vhost in the same OS,
 aka, there is no need to start a VM. We just run it with a different 
--file-prefix to avoid startup failure.

 .. code-block:: console

 cd ${RTE_SDK}/x86_64-native-linuxapp-gcc/app
-./testpmd -c 0x3 -n 4 --socket-mem 1024 --no-pci 
--file-prefix=virtio-user-testpmd \
---vdev=virtio-user0,mac=00:01:02:03:04:05,path=$path_vhost \
+./testpmd -c 0x3 -n 4 --socket-mem 1024 --no-pci 
--file-prefix=virtio_user-testpmd \
+--vdev=virtio_user0,mac=00:01:02:03:04:05,path=$path_vhost \
 -- -i --txqflags=0xf01 --disable-hw-vlan

 There is no difference on the vhost side.
-Pleae note that there are some limitations (see release note for more 
information) in the usage of virtio-user.
+Pleae note that there are some limitations (see release note for more 
information) in the usage of virtio_user.
diff --git a/drivers/net/virtio/virtio_ethdev.c 
b/drivers/net/virtio/virtio_ethdev.c
index 850e3ba..c1ff2ac 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -452,7 +452,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
*pvq = cvq;
}

-   /* For virtio-user case (that is when dev->pci_dev is NULL), we use
+   /* For virtio_user case (that is when dev->pci_dev is NULL), we use
 * virtual address. And we need properly set _offset_, please see
 * MBUF_DATA_DMA_ADDR in virtqueue.h for more information.
 */
@@ -1541,7 +1541,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
if (dev->pci_dev)
dev_info->driver_name = dev->driver->pci_drv.name;
else
-   dev_info->driver_name = "virtio-user PMD";
+   dev_info->driver_name = "virtio_user PMD";
dev_info->max_rx_queues = (uint16_t)hw->max_rx_queues;
dev_info->max_tx_queues = (uint16_t)hw->max_tx_queues;
dev_info->min_rx_bufsize = VIRTIO_MIN_RX_BUFSIZE;
diff --git a/drivers/net/virtio/virtio_user_ethdev.c 
b/drivers/net/virtio/virtio_user_ethdev.c
index 6b4f66e..daef09b 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -331,7 +331,7 @@ virtio_user_pmd_devinit(const char *name, const char 
*params)
int ret = -1;

if (!params || params[0] == '\0') {
-   PMD_INIT_LOG(ERR, "arg %s is mandatory for virtio-user",
+   PMD_INIT_LOG(ERR, "arg %s is mandatory for virtio_user",
  VIRTIO_USER_ARG_QUEUE_SIZE);
goto end;
}
@@ -351,7 +351,7 @@ virtio_user_pmd_devinit(const char *name, const char 
*params)
goto end;
}
} else {
-   PMD_INIT_LOG(ERR, "arg %s is 

[dpdk-dev] l3fwd can't launch on VF when we use i40e

[Xu, Qian Q]

+ dpdk.org mailing list. I also thought the patch to check port CRC strip then 
make the port fail to start is not proper.
For testpmd, we can use -crc-strip, but for l3fwd, we may need change code to 
enable crc-strip, and for all application, we need change the code; this is not 
acceptable.
Users may think it's a bug of R16.07 since R16.04 didn't check that. From my 
view, crc-strip is not a "MUST" check for the port start. We can configure the 
value after
the port start.
Any thoughts?

From: Yao, Lei A
Sent: Friday, July 22, 2016 9:36 AM
To: Topel, Bjorn 
Cc: Xu, Qian Q 
Subject: l3fwd can't launch on VF when we use i40e

Hi, Bjorn

This is lei who is running DPDK testing on VMware with 16.07 RC3. Now I met one 
issue, after I enable SR-IOV on VMware with i40e related NIC, I can't launch 
l3fwd example on VF because of this error info:
i40evf_dev_configure(): VF can't disable HW CRC Strip.

I see that you have submit following patch related to this scenario
commit 1bbcc5d21129168a212e7d755751b0d4742d20d9
Author: Bj?rn T?pel 
Date:   Fri Apr 22 07:39:22 2016 +0200

i40evf: report error for unsupported CRC stripping config

On hosts running a non-DPDK PF driver, the VF has no means of changing
the HW CRC strip setting for a RX queue. It's implicitly enabled.

This patch checks if the host is running a non-DPDK PF kernel driver,
and returns an error, if HW CRC stripping was not requested in the port
configuration.

Signed-off-by: Bj?rn T?pel 
Acked-by: Helin Zhang 

Do you know is there any way I can bypass this when I use l3fwd? I know when we 
use testpmd, we can add --crc-strip enable to bypass this blocker. But I don't 
know the method on l3fwd. Thanks.

Best Regards
Lei

[dpdk-dev] [RFC] Generic flow director/filtering/classification API

[Lu, Wenzhuo]

Hi Adrien,


> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Thursday, July 21, 2016 8:48 PM
> To: Lu, Wenzhuo
> Cc: dev at dpdk.org; Thomas Monjalon; Zhang, Helin; Wu, Jingjing; Rasesh Mody;
> Ajit Khaparde; Rahul Lakkireddy; Jan Medala; John Daley; Chen, Jing D; 
> Ananyev,
> Konstantin; Matej Vido; Alejandro Lucero; Sony Chacko; Jerin Jacob; De Lara
> Guarch, Pablo; Olga Shern
> Subject: Re: [RFC] Generic flow director/filtering/classification API
> 
> Hi Wenzhuo,
> 
> It seems that we agree on about everything now, just a few more comments
> below after snipping the now irrelevant parts.
Yes, I think we?re agree with each other now. And thanks for the additional 
explanation :)

[dpdk-dev] [PATCH] net/i40e: fix out-of-bounds writes during vector Rx

[Thomas Monjalon]

2016-07-21 14:03, Ilya Maximets:
> From: Sergey Dyasly 
> 
> Rx loop inside _recv_raw_pkts_vec() ignores nb_pkts argument and always
> tries to receive RTE_I40E_VPMD_RX_BURST (32) packets. This is a violation
> of rte_eth_rx_burst() API and can lead to memory corruption (out-of-bounds
> writes to struct rte_mbuf **rx_pkts) if nb_pkts is less than 32.
> 
> Fix this by actually using nb_pkts inside the loop.
> 
> Fixes: 9ed94e5bb04e ("i40e: add vector Rx")
> 
> Signed-off-by: Sergey Dyasly 
> Acked-by: Ilya Maximets 

Applied, thanks

[dpdk-dev] [PATCH] ixgbe:Prevent redefinition of bool if compiling using c++

[Thomas Monjalon]

2016-07-18 13:39, Ido Barnea:
> Signed-off-by: Ido Barnea 

Patch reconstructed (format was not applicable),
and applied, thanks

[dpdk-dev] [PATCH] eal: fix check number of bytes from read function

[Thomas Monjalon]

2016-07-21 20:50, Jastrzebski, MichalX K:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > - (int)sizeof(uint64_t) can be replaced by 8 but it's shorter ;)
> 
> I didn't want to change all invokes of read() function here. 
> I can use some macro:
> #define PFN_MASK_SIZE 8
> How do You think?

Yes may be an idea.

[dpdk-dev] [PATCH] net/enic: heed VLAN strip flag in device configure function

[Thomas Monjalon]

> > The configure function enicpmd_dev_configure() was not paying attention to
> > the rxmode VLAN strip bit. Set the VLAN strip mode according to the bit.
> > 
> > Fixes: fefed3d1e62c ("enic: new driver")
> > 
> > Signed-off-by: John Daley 
> 
> Reviewed-by: David Harton 
> Tested-by: David Harton 

Applied, thanks