Re: [dpdk-dev] [PATCH 00/31] Support VFD and DPDK PF + kernel VF on i40e

2016-12-02 Thread Thomas Monjalon
2016-12-02 14:25, Iremonger, Bernard:
> Hi Thomas,
> 
> From: Thomas Monjalon [mailto:thomas.monja...@6wind.com]
> > 
> > Do you agree to copy the remaining VF-specific functions from the generic
> > ethdev API to ixgbe in 17.02?
> > After a deprecation notice, we could remove them from the generic API in
> > 17.05.
> > So we will have a consistent status in 17.05 regarding VF functions.
> > 
> > After some time (and experience) we will be able to discuss wether we need
> > something more to access to these specific features.
> 
> I had intended to copy the remaining VF-specific functions from the generic 
> ethdev API to ixgbe in 17.02 and a deprecation notice was sent in 16.11. 
> 
> This work was parked in order work on the VF-specfic functionsfor the i40e.  
> Is it ok to submit patches for this work after the V1 deadline (today)?

Yes, it is a kind of cleanup and it has been discussed earlier.
So yes you can submit it later. Sooner is better though :)
Thanks


Re: [dpdk-dev] [PATCH 00/31] Support VFD and DPDK PF + kernel VF on i40e

2016-12-02 Thread Thomas Monjalon
Do you agree to copy the remaining VF-specific functions from the
generic ethdev API to ixgbe in 17.02?
After a deprecation notice, we could remove them from the generic API in 17.05.
So we will have a consistent status in 17.05 regarding VF functions.

After some time (and experience) we will be able to discuss wether
we need something more to access to these specific features.


Re: [dpdk-dev] [PATCH 4/4] lib/librte_vhost: improve vhost perf using rte_memset

2016-12-02 Thread Thomas Monjalon
2016-12-05 16:26, Zhiyong Yang:
> +* **Introduced rte_memset and related test on IA platform.**
> +
> +  Performance drop had been caused in some cases on Ivybridge when DPDK code 
> calls glibc
> +  function memset. It was necessary to introduce more high efficient 
> function to fix it.
> +  The function rte_memset supported three types of instruction sets 
> including sse & avx(128 bits),
> +  avx2(256 bits) and avx512(512bits).
> +
> +  * Added rte_memset support on IA platform.
> +  * Added functional autotest support for rte_memset.
> +  * Added performance autotest support for rte_memset.

No need to reference autotests in the release notes.

> +  * Improved performance to use rte_memset instead of copy_virtio_net_hdr in 
> lib/librte_vhost.

Please describe this change at a higher level. Which case it is improving?



[dpdk-dev] [PATCH] pmdinfogen: Fix pmdinfogen to select proper endianess on cross-compile

2016-12-01 Thread Thomas Monjalon
2016-11-21 10:11, Bruce Richardson:
> On Fri, Nov 18, 2016 at 01:47:52PM -0500, Neil Horman wrote:
> > pmdinfogen has a bug in which, during build, it pulls in rte_byteorder.h to
> > obtain the rte macros for byteswapping between the cpu byte order and big or
> > little endian.  Unfortunately, pmdinfogen is a tool that is only meant to 
> > be run
> > during the build of dpdk components, and so, it runs on the host.  In cross
> > compile environments however, the rte_byteorder.h is configured using a 
> > target
> > cpu, who's endianess may differ from that of the host, leading to improper
> > swapping.
> > 
> > The fix is to use host system defined byte swapping routines rather than the
> > dpdk provided routines.  Note that we are using non posix compliant 
> > routines, as
> > the posix compliant api only addresses 16 and 32 bit swaps, and we also 
> > need 64
> > bit swaps.  Those macros exist (via endian.h), but BSD and Linux put that 
> > header
> > in different locations so some ifdeffery is required.
> > 
> > Tested successfully by myself on Linux and BSD systems.
> > 
> > Signed-off-by: Neil Horman 
> > CC: Hemant Agrawal 
> > CC: Jerin Jacob 
> > CC: Bruce Richardson 
> > CC: Thomas Monjalon 
> > ---
> >  buildtools/pmdinfogen/pmdinfogen.h | 10 +++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Compiles fine on FreeBSD with clang.
> 
> Tested-by: Bruce Richardson 

Fixed "endianness" typo, headline, added Fixes:, CC: stable at dpdk.org and 
removed a trailing whitespace, then
applied, thanks


[dpdk-dev] [PATCH 2/3] maintainers: update virtio section name

2016-12-01 Thread Thomas Monjalon
2016-12-01 15:06, Yuanhan Liu:
> Signed-off-by: Yuanhan Liu 
> ---
> 
> hmm.., maybe we could seperate lib vhost and virtio pmd, into two
> different sections?

Yes we can :)

> -RedHat virtio
> +Virtio PMD and vhost lib
>  M: Yuanhan Liu 
>  T: git://dpdk.org/next/dpdk-next-virtio
>  F: drivers/net/virtio/
> 




[dpdk-dev] [PATCH v2] scripts: check cc stable mailing list in commit

2016-12-01 Thread Thomas Monjalon
2016-12-01 15:00, Ferruh Yigit:
> On 12/1/2016 1:43 PM, Thomas Monjalon wrote:
> > Add a check for commits fixing a released bug.
> > Such commits are found thanks to scripts/git-log-fixes.sh.
> > They must be sent CC: stable at dpdk.org.
> > In order to avoid forgetting CC, this mail header can be written
> > in the git commit message.
> > 
> > Signed-off-by: Thomas Monjalon 
> 
> Tested-by: Ferruh Yigit 

Applied


Re: [dpdk-dev] [PATCH v12 1/6] ethdev: add Tx preparation

2016-12-01 Thread Thomas Monjalon
2016-12-01 22:31, Kulasek, TomaszX:
> From: Thomas Monjalon [mailto:thomas.monja...@6wind.com]
> > 2016-12-01 19:20, Kulasek, TomaszX:
> > > Hi Thomas,
> > >
> > > Sorry, I have answered for this question in another thread and I missed
> > about this one. Detailed answer is below.
> > 
> > Yes you already gave this answer.
> > And I will continue asking the question until you understand it.
> > 
> > > > 2016-11-28 11:54, Thomas Monjalon:
> > > > > Hi,
> > > > >
> > > > > 2016-11-23 18:36, Tomasz Kulasek:
> > > > > > --- a/config/common_base
> > > > > > +++ b/config/common_base
> > > > > > @@ -120,6 +120,7 @@ CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> > > > > >  CONFIG_RTE_LIBRTE_IEEE1588=n
> > > > > >  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > > > > >  CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y
> > > > > > +CONFIG_RTE_ETHDEV_TX_PREPARE=y
> > > > >
> > > > > Please, remind me why is there a configuration here.
> > > > > It should be the responsibility of the application to call
> > > > > tx_prepare or not. If the application choose to use this new API
> > > > > but it is disabled, then the packets won't be prepared and there is
> > no error code:
> > > > >
> > > > > > +#else
> > > > > > +
> > > > > > +static inline uint16_t
> > > > > > +rte_eth_tx_prepare(__rte_unused uint8_t port_id, __rte_unused
> > > > uint16_t queue_id,
> > > > > > +   __rte_unused struct rte_mbuf **tx_pkts, uint16_t
> > > > > > +nb_pkts) {
> > > > > > +   return nb_pkts;
> > > > > > +}
> > > > > > +
> > > > > > +#endif
> > > > >
> > > > > So the application is not aware of the issue and it will not use
> > > > > any fallback.
> > >
> > > tx_prepare mechanism can be turned off by compilation flag (as discussed
> > with Jerin in http://dpdk.org/dev/patchwork/patch/15770/) to provide real
> > NOOP functionality (e.g. for low-end CPUs, where even unnecessary memory
> > dereference and check can have significant impact on performance).
> > >
> > > Jerin observed that on some architectures (e.g. low-end ARM with
> > embedded NIC), just reading and comparing 'dev->tx_pkt_prepare' may cause
> > significant performance drop, so he proposed to introduce this
> > configuration flag to provide real NOOP when tx_prepare functionality is
> > not required, and can be turned on based on the _target_ configuration.
> > >
> > > For other cases, when this flag is turned on (by default), and
> > tx_prepare is not implemented, functional NOOP is used based on comparison
> > (dev->tx_pkt_prepare == NULL).
> > 
> > So if the application call this function and if it is disabled, it simply
> > won't work. Packets won't be prepared, checksum won't be computed.
> > 
> > I give up, I just NACK.
> 
> It is not to be turned on/off whatever someone wants, but only and only for 
> the case, when platform developer knows, that his platform doesn't need this 
> callback, so, he may turn off it and then save some performance (this option 
> is per target).

How may he know? There is no comment in the config file, no documentation.

> For this case, the behavior of tx_prepare will be exactly the same when it is 
> turned on or off. If is not the same, there's no sense to turn it off. There 
> were long topic, where we've tried to convince you, that it should be turned 
> on for all devices.

Really? You tried to convince me to turn it on?
No you were trying to convince Jerin.
I think it is a wrong idea to allow disabling this function.
I didn't comment in first discussion because Jerin told it was really
important for small hardware with fixed NIC, and I thought it would
be implemented in a way the application cannot be misleaded.

The only solution I see here is to add some comments in the configuration
file, below the #else and in the doc.
Have you checked doc/guides/prog_guide/poll_mode_drv.rst?


[dpdk-dev] [PATCH v2] scripts: check cc stable mailing list in commit

2016-12-01 Thread Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
They must be sent CC: stable at dpdk.org.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.

Signed-off-by: Thomas Monjalon 
---
v2: fix option -N
---
 scripts/check-git-log.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index 5f8a9fc..f79f0a2 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -47,12 +47,18 @@ if [ "$1" = '-h' -o "$1" = '--help' ] ; then
exit
 fi

+selfdir=$(dirname $(readlink -e $0))
 range=${1:-origin/master..}
+# convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
+if printf -- $range | grep -q '^-[0-9]\+' ; then
+   range="HEAD$(printf -- $range | sed 's,^-,~,').."
+fi

 commits=$(git log --format='%h' --reverse $range)
 headlines=$(git log --format='%s' --reverse $range)
 bodylines=$(git log --format='%b' --reverse $range)
 fixes=$(git log --format='%h %s' --reverse $range | grep -i ': *fix' | cut -d' 
' -f1)
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d'  | cut -d' ' 
-f2)
 tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e 'fix.*:')
 bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by:'

@@ -191,3 +197,10 @@ bad=$(for fixtag in $fixtags ; do
printf "$fixtag" | grep -v "^$good$"
 done | sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+   git log --format='%b' -1 $fix | grep -qi '^CC: *stable at dpdk.org' ||
+   git log --format='\t%s' -1 $fix
+done)
+[ -z "$bad" ] || printf "Should CC: stable at dpdk.org\n$bad\n"
-- 
2.7.0



[dpdk-dev] Hyper-v support

2016-12-01 Thread Thomas Monjalon
(fixed the email, sorry)

2016-12-01 12:21, Thomas Monjalon:
> 2016-11-30 14:34, Varun:
> > Hi,
> > 
> > I would like to know if the latest DPDK (16.11) supports hyper-v?
> > 
> > I couldn't find any conclusive evidence online or in dpdk roadmap. Is it
> > likely that we see it in 17.05?
> 
> Stephen did a presentation at the last DPDK userspace summit:
> https://dpdksummit.com/Archive/pdf/2016Userspace/Day01-Session03-StephenHemminger-Userspace2016.pdf
> 
> Stephen, please, could you confirm the expected release for Hyper-V support?
> A patch for the roadmap page would be great:
>   http://dpdk.org/dev/roadmap




[dpdk-dev] Hyper-v support

2016-12-01 Thread Thomas Monjalon
2016-11-30 14:34, Varun:
> Hi,
> 
> I would like to know if the latest DPDK (16.11) supports hyper-v?
> 
> I couldn't find any conclusive evidence online or in dpdk roadmap. Is it
> likely that we see it in 17.05?

Stephen did a presentation at the last DPDK userspace summit:
https://dpdksummit.com/Archive/pdf/2016Userspace/Day01-Session03-StephenHemminger-Userspace2016.pdf

Stephen, please, could you confirm the expected release for Hyper-V support?
A patch for the roadmap page would be great:
http://dpdk.org/dev/roadmap


[dpdk-dev] [PATCH v12 0/6] add Tx preparation

2016-11-30 Thread Thomas Monjalon
2016-11-30 17:42, Ananyev, Konstantin:
> > >Please, we need a comment for each driver saying
> > >"it is OK, we do not need any checksum preparation for TSO"
> > >or
> > >"yes we have to implement tx_prepare or TSO will not work in this mode"
> > >
> > 
> > qede PMD doesn?t currently support TSO yet, it only supports Tx TCP/UDP/IP
> > csum offloads.
> > So Tx preparation isn?t applicable. So as of now -
> > "it is OK, we do not need any checksum preparation for TSO"
> 
> Thanks for the answer.
> Though please note that it not only for TSO.

Oh yes, sorry, my wording was incorrect.
We need to know if any checksum preparation is needed prior
offloading its final computation to the hardware or driver.
So the question applies to TSO and simple checksum offload.

We are still waiting answers for
bnxt, cxgbe, ena, nfp, thunderx, virtio and vmxnet3.

> This is for any TX offload for which the upper layer SW would have
> to modify the contents of the packet.
> Though as I can see for qede neither PKT_TX_IP_CKSUM or PKT_TX_TCP_CKSUM
> exhibits any extra requirements for the user.
> Is that correct?



[dpdk-dev] [PATCH] scripts: check cc stable mailing list in commit

2016-11-30 Thread Thomas Monjalon
2016-11-30 15:26, Bruce Richardson:
> On Wed, Nov 30, 2016 at 04:09:47PM +0100, Thomas Monjalon wrote:
> > 2016-11-30 14:54, Ferruh Yigit:
> > > On 11/21/2016 10:43 PM, Thomas Monjalon wrote:
> > > > +stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d'  | 
> > > > cut -d' ' -f2)
> > > 
> > > This breaks the "check-git-log.sh -N" usage, since "-N" is not a valid
> > > range for git-log-fixes.sh.
> > > Generates warning:
> > > .../scripts/git-log-fixes.sh: illegal option -- 6
> > > usage: git-log-fixes.sh [-h] 
> > 
> > Yes, good catch.
> > I'm trying to fix it by converting -N to HEAD~N..
> > 
> > if printf -- $range | grep -q '^-[0-9]\+' ; then
> > range="HEAD$(printf -- $range | sed 's,^-,~,').."
> > fi
> > 
> > > > +# check CC:stable for fixes
> > > > +bad=$(for fix in $stablefixes ; do
> > > > +   git log --format='%b' -1 $fix | grep -qi '^CC: *stable at 
> > > > dpdk.org' ||
> > > > +   git log --format='\t%s' -1 $fix
> > > > +done)
> > > > +[ -z "$bad" ] || printf "Should CC: stable at dpdk.org\n$bad\n"
> > > 
> > > This is good for developer, but since "CC: xx" tags removed when patch
> > > applied, this will generate warnings when run against existing history.
> > 
> > I do not think it is a problem.
> > Who runs this tool against existing history?
> >
> 
> Me for one. I prefer to run the script against the commits in the repo
> before I generate the patches, rather than manually hand-editing the
> patches afterward - or having to fix the repo and then regenerate them.
> Also, when I was maintaining the next-net tree, I used to use pwclient git-am
> to apply a patch, and then check-got-log.sh -1 to sanity check it once
> build checks had passed.

I am not sure to understand.
You explain that you run the script for the commits you are going to send
or going to push. That's the normal usage.
In your cases you should have the CC: stable or you will have the warning.



[dpdk-dev] [PATCH] scripts: check cc stable mailing list in commit

2016-11-30 Thread Thomas Monjalon
2016-11-30 14:54, Ferruh Yigit:
> On 11/21/2016 10:43 PM, Thomas Monjalon wrote:
> > +stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d'  | cut 
> > -d' ' -f2)
> 
> This breaks the "check-git-log.sh -N" usage, since "-N" is not a valid
> range for git-log-fixes.sh.
> Generates warning:
> .../scripts/git-log-fixes.sh: illegal option -- 6
> usage: git-log-fixes.sh [-h] 

Yes, good catch.
I'm trying to fix it by converting -N to HEAD~N..

if printf -- $range | grep -q '^-[0-9]\+' ; then
range="HEAD$(printf -- $range | sed 's,^-,~,').."
fi

> > +# check CC:stable for fixes
> > +bad=$(for fix in $stablefixes ; do
> > +   git log --format='%b' -1 $fix | grep -qi '^CC: *stable at dpdk.org' ||
> > +   git log --format='\t%s' -1 $fix
> > +done)
> > +[ -z "$bad" ] || printf "Should CC: stable at dpdk.org\n$bad\n"
> 
> This is good for developer, but since "CC: xx" tags removed when patch
> applied, this will generate warnings when run against existing history.

I do not think it is a problem.
Who runs this tool against existing history?

> I don't know what can be done for this.
> 
> Or should we keep CC: tags in commit log perhaps?

I do not see the value of keeping the CC: in the git history.


[dpdk-dev] [PATCH] scripts: fix checkpatch from standard input

2016-11-30 Thread Thomas Monjalon
2016-11-28 16:21, Olivier Matz:
> On Mon, 21 Nov 2016 23:42:41 +0100, Thomas Monjalon
>  wrote:
> > When checking a valid patch from standard input,
> > the footer lines of the report are not filtered out.
> > 
> > The function check is called outside of any loop,
> > so the statement continue has no effect and the footer is printed.
> > 
> > Fixes: 8005feef421d ("scripts: add standard input to checkpatch")
> > 
> > Signed-off-by: Thomas Monjalon 
> 
> The 'continue' statement is not always without effect. On my machine
> (but it looks it's not the same everywhere):
> - with dash, the 'continue' acts like a return in that case
> - with bash, it displays an error:
>   "continue: only meaningful in a `for', `while', or `until' loop"
> - with bash --posix, the 'continue' is ignored...
> 
> In my case, checkpatches.sh was displaying "0/1 valid" although there
> was no error. This patch solves the issue, thanks.
> 
> 
> Acked-by: Olivier Matz 

I've amended with your explanations and applied, thanks


[dpdk-dev] [PATCH v2] maintainers: update pcap pmd maintainers

2016-11-30 Thread Thomas Monjalon
2016-11-29 14:39, John McNamara:
> Remove Nico Pernas Maradei as a PCAP PMD maintainer.
> 
> Signed-off-by: John McNamara 

Yes he is not active anymore.
Note he is welcome to come back at any time.

Acked-by: Thomas Monjalon 

Applied, thanks


[dpdk-dev] [PATCH v12 0/6] add Tx preparation

2016-11-30 Thread Thomas Monjalon
2016-11-30 08:40, Adrien Mazarguil:
[...]
> I understand tx_prep() automates this process, however I'm wondering why
> isn't the TX burst function doing that itself. Using nb_mtu_seg_max as an
> example, tx_prep() has an extra check in case of TSO that the TX burst
> function does not perform. This ends up being much more expensive to
> applications due to the additional loop doing redundant testing on each
> mbuf.
> 
> If, say as a performance improvement, we decided to leave the validation
> part to the TX burst function; what remains in tx_prep() is basically heavy
> "preparation" requiring mbuf changes (i.e. erasing checksums, for now).
> 
> Following the same logic, why can't such a thing be made part of the TX
> burst function as well (through a direct call to rte_phdr_cksum_fix()
> whenever necessary). From an application standpoint, what are the advantages
> of having to:
> 
>  if (tx_prep()) // iterate and update mbufs as needed
>  tx_burst(); // iterate and send
> 
> Compared to:
> 
>  tx_burst(); // iterate, update as needed and send
> 
> Note that PMDs could still provide different TX callbacks depending on the
> set of enabled offloads so performance is not unnecessarily impacted.
> 
> In my opinion the second approach is both faster to applications and more
> friendly from a usability perspective, am I missing something obvious?

I think it was not clearly explained in this patchset, but this is
my understanding:
tx_prepare and tx_burst can be called at different stages of a pipeline,
on different cores.



[dpdk-dev] [PATCH] doc: remove wrong document description

2016-11-29 Thread Thomas Monjalon
2016-11-29 12:54, Baruch Siach:
> On Tue, Nov 29, 2016 at 09:54:58AM +, Mcnamara, John wrote:
> > Acked-by: John McNamara 
> 
> Thanks. For some reason patchwork didn't get your ack.

That's because you are not registered to the mailing list so your email
was in a moderation queue while John sent his ack.
You should register to avoid moderation and such issues:
http://dpdk.org/ml/listinfo/dev


[dpdk-dev] [PATCH v1] maintainers: update lthreads maintainer

2016-11-29 Thread Thomas Monjalon
> Signed-off-by: John McNamara 
[...]
> -M: Ian Betts 
> +M: John McNamara 

Acked-by: Thomas Monjalon 

Applied, thanks

We need to talk about the status of this library.



[dpdk-dev] [PATCH v1] maintainers: update procinfo maintainer

2016-11-29 Thread Thomas Monjalon
2016-11-18 17:09, John McNamara:
> Update procinfo maintainer and name of the application.
> 
> Signed-off-by: John McNamara 

Acked-by: Thomas Monjalon 

Applied, thanks


[dpdk-dev] [PATCH v3] maintainers: update testpmd maintainer

2016-11-29 Thread Thomas Monjalon
> > Signed-off-by: Jingjing Wu 
> 
> Acked-by: Pablo de Lara 
> 
> Thanks Jingjing!

Acked-by: Thomas Monjalon 

Applied, thanks


[dpdk-dev] [PATCH 00/16] e1000 base code update

2016-11-29 Thread Thomas Monjalon
2016-11-29 00:30, Lu, Wenzhuo:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2016-11-25 12:58, Ferruh Yigit:
> > > Can you also please send another patch to:
> > > 1- add I219 to supported nics list
> > > 2- announce new supported nic in release notes.
> > 
> > Please update also the web site:
> > http://dpdk.org/doc/nics
> Didn't know that. How to update it? Thanks.

Please check this repository:
http://dpdk.org/browse/tools/dpdk-web/
You can send a patch to this mailing list:
http://dpdk.org/ml/listinfo/web


[dpdk-dev] dpdk/vpp and cross-version migration for vhost

2016-11-28 Thread Thomas Monjalon
2016-11-28 16:28, Maxime Coquelin:
> On 11/24/2016 04:24 PM, Kavanagh, Mark B wrote:
> > DPDK v16.04 added support for vHost User TSO; as such, by default,
> > TSO is advertised to guest devices as an available feature during
> > feature negotiation with QEMU.
> > However, while the vHost user backend sets up the majority of the
> > mbuf fields that are required for TSO, there is still a reliance
> > on the associated DPDK application (i.e. in this case OvS-DPDK)
> > to set the remaining flags and/or offsets.
> > Since OvS-DPDK doesn't currently provide that functionality, it is
> > necessary to explicitly disable TSO; otherwise, undefined behaviour
> > will ensue.
> 
> Thanks Mark for the clarification.
> 
> In this case, maybe we could add a DPDK build option to disable Vhost's
> TSO support, that would be selected for OVS packages?

Why do you prefer a build-time option rather than the run-time config
with rte_vhost_feature_disable()? Because we need to lock the features?

Reminder: build-time configuration options are forbidden in DPDK for
such usage. It would prevent other applications from using the feature
in a given distribution, just because it is not implemented in OVS.

> Does that sound reasonable?

Maybe I'm missing something but I feel it is more reasonnable to implement
the missing code in OVS.
If something is missing in DPDK, do not hesitate to request or add more
helper functions.



[dpdk-dev] [PATCH] doc: introduce PVP reference benchmark

2016-11-28 Thread Thomas Monjalon
2016-11-28 15:02, Maxime Coquelin:
> 
> On 11/28/2016 12:22 PM, Thomas Monjalon wrote:
> > 2016-11-23 22:00, Maxime Coquelin:
> >> +You can use this qmp-vcpu-pin script to pin vCPUs:
> >> +
> >> +   .. code-block:: python
> >> +
> >> +#!/usr/bin/python
> >> +# QEMU vCPU pinning tool
> >> +#
> >> +# Copyright (C) 2016 Red Hat Inc.
> >> +#
> >> +# Authors:
> >> +#  Maxime Coquelin 
> >> +#
> >> +# This work is licensed under the terms of the GNU GPL, version 2.  
> >> See
> >> +# the COPYING file in the top-level directory
> >> +import argparse
> >> +import json
> >> +import os
> >> +
> >> +from subprocess import call
> >> +from qmp import QEMUMonitorProtocol
> >> +
> >> +pinned = []
> >> +
> >> +parser = argparse.ArgumentParser(description='Pin QEMU vCPUs to 
> >> physical CPUs')
> >> +parser.add_argument('-s', '--server', type=str, required=True,
> >> +help='QMP server path or address:port')
> >> +parser.add_argument('cpu', type=int, nargs='+',
> >> +help='Physical CPUs IDs')
> >> +args = parser.parse_args()
> >> +
> >> +devnull = open(os.devnull, 'w')
> >> +
> >> +srv = QEMUMonitorProtocol(args.server)
> >> +srv.connect()
> >> +
> >> +for vcpu in srv.command('query-cpus'):
> >> +vcpuid = vcpu['CPU']
> >> +tid = vcpu['thread_id']
> >> +if tid in pinned:
> >> +print 'vCPU{}\'s tid {} already pinned, 
> >> skipping'.format(vcpuid, tid)
> >> +continue
> >> +
> >> +cpuid = args.cpu[vcpuid % len(args.cpu)]
> >> +print 'Pin vCPU {} (tid {}) to physical CPU {}'.format(vcpuid, 
> >> tid, cpuid)
> >> +try:
> >> +call(['taskset', '-pc', str(cpuid), str(tid)], stdout=devnull)
> >> +pinned.append(tid)
> >> +except OSError:
> >> +print 'Failed to pin vCPU{} to CPU{}'.format(vcpuid, cpuid)
> >>
> >
> >
> > No please do not introduce such useful script in a doc.
> > I think it must be a separate file in the DPDK repository or
> > in the QEMU repository.
> 
> Ok. The patch is under review on Qemu ML.
> While it gets merged, I can add a link to its patchwork ID.
> Ok for you?

Perfect, thanks


[dpdk-dev] [PATCH 00/16] e1000 base code update

2016-11-28 Thread Thomas Monjalon
2016-11-25 12:58, Ferruh Yigit:
> Can you also please send another patch to:
> 1- add I219 to supported nics list
> 2- announce new supported nic in release notes.

Please update also the web site:
http://dpdk.org/doc/nics



[dpdk-dev] [PATCH 1/4] eventdev: introduce event driven programming model

2016-11-28 Thread Thomas Monjalon
2016-11-28 09:16, Bruce Richardson:
> On Sat, Nov 26, 2016 at 08:24:55AM +0530, Jerin Jacob wrote:
> > On Fri, Nov 25, 2016 at 11:00:53AM +, Bruce Richardson wrote:
> > > On Fri, Nov 25, 2016 at 05:53:34AM +0530, Jerin Jacob wrote:
> > > > On Thu, Nov 24, 2016 at 04:35:56PM +0100, Thomas Monjalon wrote:
> > > > > 2016-11-24 07:29, Jerin Jacob:
> > > > > > On Wed, Nov 23, 2016 at 07:39:09PM +0100, Thomas Monjalon wrote:
> > > > > > > 2016-11-18 11:14, Jerin Jacob:
> > > > > > > > +Eventdev API - EXPERIMENTAL
> > > > > > > > +M: Jerin Jacob 
> > > > > > > > +F: lib/librte_eventdev/
> > > > > > > 
> > > > 
> > > > I don't think there is any portability issue here, I can explain.
> > > > 
> > > > The application level, we have two more use case to deal with non burst
> > > > variant
> > > > 
> > > > - latency critical work
> > > > - on dequeue, if application wants to deal with only one flow(i.e to
> > > >   avoid processing two different application flows to avoid cache 
> > > > trashing)
> > > > 
> > > > Selection of the burst variants will be based on
> > > > rte_event_dev_info_get() and rte_event_dev_configure()(see, 
> > > > max_event_port_dequeue_depth,
> > > > max_event_port_enqueue_depth, nb_event_port_dequeue_depth, 
> > > > nb_event_port_enqueue_depth )
> > > > So I don't think their is portability issue here and I don't want to 
> > > > waste my
> > > > CPU cycles on the for loop if application known to be working with non
> > > > bursts variant like below
> > > > 
> > > 
> > > If the application is known to be working on non-burst varients, then
> > > they always request a burst-size of 1, and skip the loop completely.
> > > There is no extra performance hit in that case in either the app or the
> > > driver (since the non-burst driver always returns 1, irrespective of the
> > > number requested).
> > 
> > Hmm. I am afraid, There is.
> > On the app side, the const "1" can not be optimized by the compiler as
> > on downside it is function pointer based driver interface
> > On the driver side, the implementation would be for loop based instead
> > of plain access.
> > (compiler never can see the const "1" in driver interface)
> > 
> > We are planning to implement burst mode as kind of emulation mode and
> > have a different scheme for burst and nonburst. The similar approach we have
> > taken in introducing rte_event_schedule() and split the responsibility so
> > that SW driver can work without additional performance overhead and neat
> > driver interface.
> > 
> > If you are concerned about the usability part and regression on the SW
> > driver, then it's not the case, application will use nonburst variant only 
> > if
> > dequeue_depth == 1 and/or explicit case where latency matters.
> > 
> > On the portability side, we support both case and application if written 
> > based
> > on dequeue_depth it will perform well in both implementations.IMO, There is
> > no another shortcut for performance optimized application running on 
> > different
> > set of model.I think it is not an issue as, in event model as each cores
> > identical and main loop can be changed based on dequeue_depth
> > if needs performance(anyway mainloop will be function pointer based).
> > 
> 
> Ok, I think I see your point now. Here is an alternative suggestion.
> 
> 1. Keep the single user API.
> 2. Have both single and burst function pointers in the driver
> 3. Call appropriately in the eventdev layer based on parameters. For
> example:
> 
> rte_event_dequeue_burst(..., int num)
> {
>   if (num == 1 && single_dequeue_fn != NULL)
>   return single_dequeue_fn(...);
>   return burst_dequeue_fn(...);
> }
> 
> This way drivers can optionally special-case the single dequeue case -
> the function pointer check will definitely be predictable in HW making
> that a near-zero-cost check - while not forcing all drivers to do so.
> It also reduces the public API surface, and gives us a single enqueue
> and dequeue function.

+1


[dpdk-dev] [PATCH] doc: introduce PVP reference benchmark

2016-11-28 Thread Thomas Monjalon
2016-11-23 22:00, Maxime Coquelin:
> +You can use this qmp-vcpu-pin script to pin vCPUs:
> +
> +   .. code-block:: python
> +
> +#!/usr/bin/python
> +# QEMU vCPU pinning tool
> +#
> +# Copyright (C) 2016 Red Hat Inc.
> +#
> +# Authors:
> +#  Maxime Coquelin 
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.  See
> +# the COPYING file in the top-level directory
> +import argparse
> +import json
> +import os
> +
> +from subprocess import call
> +from qmp import QEMUMonitorProtocol
> +
> +pinned = []
> +
> +parser = argparse.ArgumentParser(description='Pin QEMU vCPUs to physical 
> CPUs')
> +parser.add_argument('-s', '--server', type=str, required=True,
> +help='QMP server path or address:port')
> +parser.add_argument('cpu', type=int, nargs='+',
> +help='Physical CPUs IDs')
> +args = parser.parse_args()
> +
> +devnull = open(os.devnull, 'w')
> +
> +srv = QEMUMonitorProtocol(args.server)
> +srv.connect()
> +
> +for vcpu in srv.command('query-cpus'):
> +vcpuid = vcpu['CPU']
> +tid = vcpu['thread_id']
> +if tid in pinned:
> +print 'vCPU{}\'s tid {} already pinned, skipping'.format(vcpuid, 
> tid)
> +continue
> +
> +cpuid = args.cpu[vcpuid % len(args.cpu)]
> +print 'Pin vCPU {} (tid {}) to physical CPU {}'.format(vcpuid, tid, 
> cpuid)
> +try:
> +call(['taskset', '-pc', str(cpuid), str(tid)], stdout=devnull)
> +pinned.append(tid)
> +except OSError:
> +print 'Failed to pin vCPU{} to CPU{}'.format(vcpuid, cpuid)
> 


No please do not introduce such useful script in a doc.
I think it must be a separate file in the DPDK repository or
in the QEMU repository.


[dpdk-dev] [PATCH v2] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

2016-11-28 Thread Thomas Monjalon
2016-11-24 17:59, Olivier Matz:
> Hi,
> 
> On Mon, 2016-11-21 at 09:59 +, Alejandro Lucero wrote:
> > From: Bert van Leeuwen 
> > 
> > Arrays inside rte_eth_stats have size=RTE_ETHDEV_QUEUE_STAT_CNTRS.
> > Some devices report more queues than that and this code blindly uses
> > the reported number of queues by the device to fill those arrays up.
> > This patch fixes the problem using MIN between the reported number of
> > queues and RTE_ETHDEV_QUEUE_STAT_CNTRS.
> > 
> > Signed-off-by: Alejandro Lucero 
> > 
> 
> Reviewed-by: Olivier Matz 
> 
> 
> As a next step, I'm wondering if it would be possible to remove
> this limitation. We could replace the tables in struct rte_eth_stats
> by a pointer to an array allocated dynamically at pmd setup.

Yes that's definitely the right way to handle these statistics.

> It would break the API, so it should be announced first. I'm thinking
> of something like:
> 
> struct rte_eth_generic_stats {
> uint64_t ipackets;
> uint64_t opackets;
> uint64_t ibytes;
> uint64_t obytes;
> uint64_t imissed;
> uint64_t ierrors;
> uint64_t oerrors;
> uint64_t rx_nombuf
> };
> 
> struct rte_eth_stats {
>   struct rte_eth_generic_stats port_stats;
>   struct rte_eth_generic_stats *queue_stats;
> };
> 
> The queue_stats array would always be indexed by queue_id.
> The xstats would continue to report the generic stats per-port and
> per-queue.
> 
> About the mapping API, either we keep it as-is, or it could
> become a driver-specific API.

Yes I agree to remove the queue statistics mapping which is very specific.
I will send a patch with a deprecation notice to move the mapping API
to a driver-specific API.

Any objection?


[dpdk-dev] [PATCH v12 0/6] add Tx preparation

2016-11-28 Thread Thomas Monjalon
We need attention of every PMD developers on this thread.

Reminder of what Konstantin suggested:
"
- if the PMD supports TX offloads AND
- if to be able use any of these offloads the upper layer SW would have to:
* modify the contents of the packet OR
* obey HW specific restrictions
then it is a PMD developer responsibility to provide tx_prep() that would 
implement
expected modifications of the packet contents and restriction checks.
Otherwise, tx_prep() implementation is not required and can be safely set to 
NULL.  
"

I copy/paste also my previous conclusion:

Before txprep, there is only one API: the application must prepare the
packets checksum itself (get_psd_sum in testpmd).
With txprep, the application have 2 choices: keep doing the job itself
or call txprep which calls a PMD-specific function.
The question is: does non-Intel drivers need a checksum preparation for TSO?
Will it behave well if txprep does nothing in these drivers?

When looking at the code, most of drivers handle the TSO flags.
But it is hard to know whether they rely on the pseudo checksum or not.

git grep -l 'PKT_TX_UDP_CKSUM\|PKT_TX_TCP_CKSUM\|PKT_TX_TCP_SEG' drivers/net/

drivers/net/bnxt/bnxt_txr.c
drivers/net/cxgbe/sge.c
drivers/net/e1000/em_rxtx.c
drivers/net/e1000/igb_rxtx.c
drivers/net/ena/ena_ethdev.c
drivers/net/enic/enic_rxtx.c
drivers/net/fm10k/fm10k_rxtx.c
drivers/net/i40e/i40e_rxtx.c
drivers/net/ixgbe/ixgbe_rxtx.c
drivers/net/mlx4/mlx4.c
drivers/net/mlx5/mlx5_rxtx.c
drivers/net/nfp/nfp_net.c
drivers/net/qede/qede_rxtx.c
drivers/net/thunderx/nicvf_rxtx.c
drivers/net/virtio/virtio_rxtx.c
drivers/net/vmxnet3/vmxnet3_rxtx.c

Please, we need a comment for each driver saying
"it is OK, we do not need any checksum preparation for TSO"
or
"yes we have to implement tx_prepare or TSO will not work in this mode"


[dpdk-dev] [PATCH v12 1/6] ethdev: add Tx preparation

2016-11-28 Thread Thomas Monjalon
Hi,

2016-11-23 18:36, Tomasz Kulasek:
> --- a/config/common_base
> +++ b/config/common_base
> @@ -120,6 +120,7 @@ CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
>  CONFIG_RTE_LIBRTE_IEEE1588=n
>  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
>  CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y
> +CONFIG_RTE_ETHDEV_TX_PREPARE=y

Please, remind me why is there a configuration here.
It should be the responsibility of the application to call tx_prepare
or not. If the application choose to use this new API but it is
disabled, then the packets won't be prepared and there is no error code:

> +#else
> +
> +static inline uint16_t
> +rte_eth_tx_prepare(__rte_unused uint8_t port_id, __rte_unused uint16_t 
> queue_id,
> +   __rte_unused struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> +{
> +   return nb_pkts;
> +}
> +
> +#endif

So the application is not aware of the issue and it will not use
any fallback.


[dpdk-dev] [PATCH 1/4] eventdev: introduce event driven programming model

2016-11-25 Thread Thomas Monjalon
2016-11-25 11:00, Bruce Richardson:
> On Fri, Nov 25, 2016 at 05:53:34AM +0530, Jerin Jacob wrote:
> > On Thu, Nov 24, 2016 at 04:35:56PM +0100, Thomas Monjalon wrote:
> > > 2016-11-24 07:29, Jerin Jacob:
> > > > On Wed, Nov 23, 2016 at 07:39:09PM +0100, Thomas Monjalon wrote:
> > > > > 2016-11-18 11:14, Jerin Jacob:
> > > > > > +#define EVENTDEV_NAME_SKELETON_PMD event_skeleton
> > > > > > +/**< Skeleton event device PMD name */
> > > > > 
> > > > > I do not understand this #define.
> > > > 
> > > > Applications can explicitly request the a specific driver though driver
> > > > name. This will go as argument to rte_event_dev_get_dev_id(const char 
> > > > *name).
> > > > The reason for keeping this #define in rte_eventdev.h is that,
> > > > application needs to include only rte_eventdev.h not rte_eventdev_pmd.h.
> > > 
> > > So each driver must register its name in the API?
> > > Is it really needed?
> > 
> > Otherwise how application knows the name of the driver.
> > The similar scheme used in cryptodev.
> > http://dpdk.org/browse/dpdk/tree/lib/librte_cryptodev/rte_cryptodev.h#n53
> > No strong opinion here. Open for suggestions.
> > 
> 
> I like having a name registered. I think we need a scheme where an app
> can find and use an implementation using a specific driver.

I do not like having the driver names in the API.
An API should not know its drivers.
If an application do some driver-specific processing, it knows
the driver name as well. The driver name is written in the driver.


[dpdk-dev] [PATCH 00/56] Solarflare libefx-based PMD

2016-11-25 Thread Thomas Monjalon
2016-11-25 12:43, Ferruh Yigit:
> On 11/25/2016 12:02 PM, Andrew Rybchenko wrote:
> > On 11/25/2016 01:24 PM, Ferruh Yigit wrote:
> >> On 11/23/2016 7:49 AM, Andrew Rybchenko wrote:
> >>> On 11/23/2016 03:02 AM, Ferruh Yigit wrote:
>  Also folder structure is drivers/net/sfc/efx/, why /sfc/
>  layer is created?
>  sfc is company name (solarflare communications), right? Other driver
>  folders not structured based on company, what about using
>  drivers/net/efx/* ?
> >>> I've tried to explain it above in item (2):
> >>>
> >>>  >>>
> >>>
> >>>   2. Another Solarflare PMD with in-kernel part (for control operations)
> >>>  is considered and could be added in the future. Code for data path
> >>>  should be shared by these two drivers. libefx-based PMD is put into
> >>>  'efx' subdirectory to have a space for another PMD and shared code.
> >>>
> >>> <<<
> >>>
> >>> So, main reason is to have location for the code shared by two Solarflare
> >>> network PMDs. May be it better to relocate when we really have it.
> >>> I'm open for other ideas/suggestions.
> >> If there will be another PMD that shares code with current one, the
> >> logic seems good, but I am not sure about start using company names, I
> >> am not against it, just I don't know.
> > 
> > I think that mlx4 and mlx5 are tightly bound to the company name (plus
> > adapter generation, I guess).
> > 
> >> Let's relocate later, this buys some time to think / get feedback on issue.
> > 
> > Do I understand correctly that you suggest to avoid extra level inside
> > for now
> > and relocate later if required? If so, that's fine for me.
> > 
> > As for naming, we think that just "efx" is a bad idea. The prefix is
> > occupied by
> > the libefx functions and driver should use something else. We have chosen
> > "sfc" mainly to follow naming used in Linux kernel for Solarflare driver
> > (the first level of Ethernet driver names is company bound in the Linux
> > kernel).
> > If we avoid extra level as discussed above, I think "sfc_efx", "sfcefx"
> > (may be it
> > will look better nearby other drivers) or just "sfc" are fine for us.
> > 
> 
> Thomas, Bruce, any comment on this?

You can add multiple drivers in the same library. As an example, ixgbe
directory have several drivers for PF/VF, scalar/vector, etc.
If you really want separate directories for your drivers while sharing some
code, you can link some files from the other directory.

About the name of this directory, I have no strong opinion.
sfcefx looks good.


[dpdk-dev] [PATCH 1/4] eventdev: introduce event driven programming model

2016-11-24 Thread Thomas Monjalon
2016-11-24 07:29, Jerin Jacob:
> On Wed, Nov 23, 2016 at 07:39:09PM +0100, Thomas Monjalon wrote:
> > 2016-11-18 11:14, Jerin Jacob:
> > > +Eventdev API - EXPERIMENTAL
> > > +M: Jerin Jacob 
> > > +F: lib/librte_eventdev/
> > 
> > OK to mark it experimental.
> > What is the plan to remove the experimental word?
> 
> IMO, EXPERIMENTAL status can be changed when
> - At least two event drivers available(Intel and Cavium are working on
>   SW and HW event drivers)
> - Functional test applications are fine with at least two drivers
> - Portable example application to showcase the features of the library
> - eventdev integration with another dpdk subsystem such as ethdev
> 
> Thoughts?. I am not sure the criteria used in cryptodev case.

Sounds good.
We will be more confident when drivers and tests will be implemented.

I think the roadmap for the SW driver targets the release 17.05.
Do you still plan 17.02 for this API and the Cavium driver?

> > > +#define EVENTDEV_NAME_SKELETON_PMD event_skeleton
> > > +/**< Skeleton event device PMD name */
> > 
> > I do not understand this #define.
> 
> Applications can explicitly request the a specific driver though driver
> name. This will go as argument to rte_event_dev_get_dev_id(const char *name).
> The reason for keeping this #define in rte_eventdev.h is that,
> application needs to include only rte_eventdev.h not rte_eventdev_pmd.h.

So each driver must register its name in the API?
Is it really needed?

> > > +struct rte_event_dev_config {
> > > + uint32_t dequeue_wait_ns;
> > > + /**< rte_event_dequeue() wait for *dequeue_wait_ns* ns on this device.
> > 
> > Please explain exactly when the wait occurs and why.
> 
> Here is the explanation from rte_event_dequeue() API definition,
> -
> @param wait
> 0 - no-wait, returns immediately if there is no event.
> >0 - wait for the event, if the device is configured with
> RTE_EVENT_DEV_CFG_PER_DEQUEUE_WAIT then this function will wait until
> the event available or *wait* time.
> if the device is not configured with RTE_EVENT_DEV_CFG_PER_DEQUEUE_WAIT
> then this function will wait until the event available or *dequeue_wait_ns*
>   ^^
> ns which was previously supplied to rte_event_dev_configure()
> -
> This is provides the application to have control over, how long the
> implementation should wait if event is not available.
> 
> Let me know what exact changes are required if details are not enough in
> rte_event_dequeue() API definition.

Maybe that timeout would be a better name.
It waits only if there is nothing in the queue.
It can be interesting to highlight in this comment that this parameter
makes the dequeue function a blocking call.

> > > +/** Event port configuration structure */
> > > +struct rte_event_port_conf {
> > > + int32_t new_event_threshold;
> > > + /**< A backpressure threshold for new event enqueues on this port.
> > > +  * Use for *closed system* event dev where event capacity is limited,
> > > +  * and cannot exceed the capacity of the event dev.
> > > +  * Configuring ports with different thresholds can make higher priority
> > > +  * traffic less likely to  be backpressured.
> > > +  * For example, a port used to inject NIC Rx packets into the event dev
> > > +  * can have a lower threshold so as not to overwhelm the device,
> > > +  * while ports used for worker pools can have a higher threshold.
> > > +  * This value cannot exceed the *nb_events_limit*
> > > +  * which previously supplied to rte_event_dev_configure()
> > > +  */
> > > + uint8_t dequeue_depth;
> > > + /**< Configure number of bulk dequeues for this event port.
> > > +  * This value cannot exceed the *nb_event_port_dequeue_depth*
> > > +  * which previously supplied to rte_event_dev_configure()
> > > +  */
> > > + uint8_t enqueue_depth;
> > > + /**< Configure number of bulk enqueues for this event port.
> > > +  * This value cannot exceed the *nb_event_port_enqueue_depth*
> > > +  * which previously supplied to rte_event_dev_configure()
> > > +  */
> > > +};
> > 
> > The depth configuration is not clear to me.
> 
> Basically the maximum number of events can be enqueued/dequeued at time
> from a given event port. depth of one == non burst mode.

OK so depth is the queue size. Please could you reword?

> > > +/* Event types to classify the event source */
> > 
> > Why this classification is needed?
> 
> This for application pipeling and the cases like, if appl

[dpdk-dev] Proposal for a new Committer model

2016-11-24 Thread Thomas Monjalon
2016-11-23 15:13, Neil Horman:
> Can either you or thomas provide some detail as to how you are doing patch
> management between trees (details of the commands you use are what I would be
> interested in). It sounds to me like there may be some optimization to be made
> here before we even make changes to the subtrees.

Until now, we preferred avoiding merge commits.
That's why I rebase sub-trees to integrate them in the mainline.
As Ferruh explained, it does not require more work because sub-trees are
regularly rebased anyway.
And I use a script to keep original committer name and date.

Hope it's clear now


[dpdk-dev] [PATCH 2/4] eventdev: implement the northbound APIs

2016-11-23 Thread Thomas Monjalon
2016-11-18 11:15, Jerin Jacob:
> This patch set defines the southbound driver interface
> and implements the common code required for northbound
> eventdev API interface.

Please make two separate patches.

> +#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> +#define RTE_PMD_DEBUG_TRACE(...) \
> + rte_pmd_debug_trace(__func__, __VA_ARGS__)
> +#else
> +#define RTE_PMD_DEBUG_TRACE(...)
> +#endif

I would like to discuss the need for a debug option as there is
already a log level.

> +/* Logging Macros */
> +#define EDEV_LOG_ERR(fmt, args...) \

Every symbols and macros in an exported header must be prefixed by RTE_.

> +/* Macros to check for valid device */
> +#define RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, retval) do { \

Sometimes you use RTE_EVENT_DEV_ and sometimes RTE_EVENTDEV.
(I prefer the latter).

> +struct rte_eventdev_driver {
> + struct rte_pci_driver pci_drv;  /**< The PMD is also a PCI driver. */

It must not be directly linked to the underlying bus.



[dpdk-dev] [PATCH 1/4] eventdev: introduce event driven programming model

2016-11-23 Thread Thomas Monjalon
Hi Jerin,

Thanks for bringing a big new piece in DPDK.

I made some comments below.

2016-11-18 11:14, Jerin Jacob:
> +Eventdev API - EXPERIMENTAL
> +M: Jerin Jacob 
> +F: lib/librte_eventdev/

OK to mark it experimental.
What is the plan to remove the experimental word?

> + * RTE event device drivers do not use interrupts for enqueue or dequeue
> + * operation. Instead, Event drivers export Poll-Mode enqueue and dequeue
> + * functions to applications.

To the question "what makes DPDK different" it could be answered
that DPDK event drivers implement polling functions :)

> +#include 
> +
> +#include 
> +#include 
> +#include 

Is it possible to remove some of these includes from the API?

> +
> +#define EVENTDEV_NAME_SKELETON_PMD event_skeleton
> +/**< Skeleton event device PMD name */

I do not understand this #define.
And it is not properly prefixed.

> +struct rte_event_dev_info {
> + const char *driver_name;/**< Event driver name */
> + struct rte_pci_device *pci_dev; /**< PCI information */

There is some work in progress to remove PCI information from ethdev.
Please do not add any PCI related structure in eventdev.
The generic structure is rte_device.

> +struct rte_event_dev_config {
> + uint32_t dequeue_wait_ns;
> + /**< rte_event_dequeue() wait for *dequeue_wait_ns* ns on this device.

Please explain exactly when the wait occurs and why.

> +  * This value should be in the range of *min_dequeue_wait_ns* and
> +  * *max_dequeue_wait_ns* which previously provided in
> +  * rte_event_dev_info_get()
> +  * \see RTE_EVENT_DEV_CFG_PER_DEQUEUE_WAIT

I think the @see syntax would be more consistent than \see.

> + uint8_t nb_event_port_dequeue_depth;
> + /**< Number of dequeue queue depth for any event port on this device.

I think it deserves more explanations.

> + uint32_t event_dev_cfg;
> + /**< Event device config flags(RTE_EVENT_DEV_CFG_)*/

How this field differs from others in the struct?
Should it be named flags?

> + uint32_t event_queue_cfg; /**< Queue config flags(EVENT_QUEUE_CFG_) */

Same comment about the naming of this field for event_queue config sruct.

> +/** Event port configuration structure */
> +struct rte_event_port_conf {
> + int32_t new_event_threshold;
> + /**< A backpressure threshold for new event enqueues on this port.
> +  * Use for *closed system* event dev where event capacity is limited,
> +  * and cannot exceed the capacity of the event dev.
> +  * Configuring ports with different thresholds can make higher priority
> +  * traffic less likely to  be backpressured.
> +  * For example, a port used to inject NIC Rx packets into the event dev
> +  * can have a lower threshold so as not to overwhelm the device,
> +  * while ports used for worker pools can have a higher threshold.
> +  * This value cannot exceed the *nb_events_limit*
> +  * which previously supplied to rte_event_dev_configure()
> +  */
> + uint8_t dequeue_depth;
> + /**< Configure number of bulk dequeues for this event port.
> +  * This value cannot exceed the *nb_event_port_dequeue_depth*
> +  * which previously supplied to rte_event_dev_configure()
> +  */
> + uint8_t enqueue_depth;
> + /**< Configure number of bulk enqueues for this event port.
> +  * This value cannot exceed the *nb_event_port_enqueue_depth*
> +  * which previously supplied to rte_event_dev_configure()
> +  */
> +};

The depth configuration is not clear to me.

> +/* Event types to classify the event source */

Why this classification is needed?

> +#define RTE_EVENT_TYPE_ETHDEV   0x0
> +/**< The event generated from ethdev subsystem */
> +#define RTE_EVENT_TYPE_CRYPTODEV0x1
> +/**< The event generated from crypodev subsystem */
> +#define RTE_EVENT_TYPE_TIMERDEV 0x2
> +/**< The event generated from timerdev subsystem */
> +#define RTE_EVENT_TYPE_CORE 0x3
> +/**< The event generated from core.

What is core?

> +/* Event enqueue operations */

I feel a longer explanation is needed here to describe
what is an operation and where this data is useful.

> +#define RTE_EVENT_OP_NEW0
> +/**< New event without previous context */
> +#define RTE_EVENT_OP_FORWARD1
> +/**< Re-enqueue previously dequeued event */
> +#define RTE_EVENT_OP_RELEASE2

There is no comment for the release operation.

> +/**
> + * Release the flow context associated with the schedule type.
> + *
[...]
> + */

There is no function declaration below this comment.

> +/**
> + * The generic *rte_event* structure to hold the event attributes
> + * for dequeue and enqueue operation
> + */
> +struct rte_event {
> + /** WORD0 */
> + RTE_STD_C11
> + union {
> + uint64_t event;
[...]
> + };
> + /** WORD1 */
> + RTE_STD_C11
> + union {
> + uintptr_t event_ptr;

I wonder if it can be a problem to have the size 

[dpdk-dev] [PATCH 00/56] Solarflare libefx-based PMD

2016-11-23 Thread Thomas Monjalon
2016-11-23 09:57, Mcnamara, John:
> From: Andrew Rybchenko
> > Yes, I have no ICC compilers. I'll try to fix these warnings, but I can't
> > be sure without checking it.
> > Also we cannot claim ICC supported without building and testing the
> > generated binary.
> 
> Hi,
> 
> You can get a copy of ICC with a 30 day evaluation license here: 
> https://software.intel.com/en-us/try-buy-tools

I think we should stop having some ICC requirements.
That's OK to support it on best effort base but it cannot be
a responsibility of developers outside of Intel.


[dpdk-dev] [PATCH] eal: postpone vdev initialization

2016-11-23 Thread Thomas Monjalon
2016-11-23 05:37, Jerin Jacob:
> On Mon, Nov 21, 2016 at 05:35:58PM +, Ferruh Yigit wrote:
> > On 11/21/2016 5:02 PM, Jerin Jacob wrote:
> > > On Mon, Nov 21, 2016 at 09:54:57AM +, Ferruh Yigit wrote:
> > >> This changes the port id assignments to the devices, right?
> > >>
> > >> Previously virtual devices get first available port ids (0..N1), later
> > >> physical devices (N1..N2). Now this becomes reverse.
> > >>
> > >> Can this change break some existing user applications?
> > > 
> > > I guess it may be effected only to ethdev bond pmd based application,
> > > which is broken anyway.
> > 
> > My concern is, this may effect the applications that use "--vdev" eal
> > parameter and has an assumption about port assignment.
> 
> Not sure. Application expectation on specific port assignment is bad anyway.
> But in any event, what we do with exiting ethdev bond pmd failure.
> 
> > 
> > And if this breaks any userspace application, does it require a
> > deprecation notice?
> 
> I am not sure. Thomas, Any input on this ?

Is the expectation thought by Ferruh, written somewhere?
If not, we can accept this change as is in this release.


[dpdk-dev] [PATCH] maintainers: update testpmd maintainers

2016-11-23 Thread Thomas Monjalon
2016-11-23 20:34, Wei Dai:
> add Jingjing Wu and Wei Dai as new maintainers
> of test-pmd.

Thanks for proposing yourself.
I think it is a bit strange for you Wei Dai to become maintainer now,
as you are a newcomer and never sent a patch for testpmd yet.


[dpdk-dev] [PATCH v2 1/8] eal: define container_of macro

2016-11-22 Thread Thomas Monjalon
2016-11-22 15:33, Shreyansh Jain:
> On Monday 21 November 2016 10:25 PM, Jan Blunck wrote:
> > This macro is based on Jan Viktorin's original patch but also checks the
> > type of the passed pointer against the type of the member.
> >
> > Signed-off-by: Jan Viktorin 
> > Signed-off-by: Shreyansh Jain 
> > [jblunck at infradead.org: add type checking and __extension__]
> > Signed-off-by: Jan Blunck 
> 
> I will start using this in my patchset.
> 
> Acked-by: Shreyansh Jain 

It is a bit strange to have this patch in a series which do
not use it. I am in favor of getting it when it is used
(and included) in another series.


[dpdk-dev] [PATCH] mk: remove make target for examples

2016-11-22 Thread Thomas Monjalon
2016-11-22 00:34, Ferruh Yigit:
> On 11/21/2016 11:47 PM, Thomas Monjalon wrote:
> > The command
> >   make examples
> > works only if target directories have the exact name of configs.
> > 
> > It is more flexible to use
> >   make -C examples RTE_SDK=$(pwd) RTE_TARGET=build
> > 
> > Signed-off-by: Thomas Monjalon 
> 
> Instead of removing examples & examples_clean targets, what do you think
> keeping them as wrapper to suggested usage, for backward compatibility.
> 
> Something like:
> "
> BUILDING_RTE_SDK :=
> export BUILDING_RTE_SDK
> 
> # Build directory is given with O=
> O ?= $(RTE_SDK)/examples
> 
> # Target for which examples should be built.
> T ?= build
> 
> .PHONY: examples
> examples:
> @echo == Build examples for $(T)
> $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T);
> 
> .PHONY: examples_clean
> examples_clean:
> @echo == Clean examples for $(T)
> $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T) clean;
> "

What is the benefit of this makefile? Just remove -C ?
It is not compatible with the old behaviour, so I'm afraid it would be
confusing for no real benefit.


[dpdk-dev] [PATCH 0/4] libeventdev API and northbound implementation

2016-11-22 Thread Thomas Monjalon
2016-11-21 09:57, Bruce Richardson:
> On Mon, Nov 21, 2016 at 10:40:50AM +0100, Thomas Monjalon wrote:
> > Are you asking for a temporary tree?
> > If yes, please tell its name and its committers, it will be done.
> 
> Yes, we are asking for a new tree, but I would not assume it is
> temporary - it might be, but it also might not be, given how other
> threads are discussing having an increasing number of subtrees giving
> pull requests. :-)
> 
> Name: dpdk-eventdev-next

Named dpdk-next-eventdev for consistency.

> Committers: Bruce Richardson & Jerin Jacob

Access granted. Jerin could you send me a public SSH key please?


[dpdk-dev] [PATCH] mk: remove make target for examples

2016-11-22 Thread Thomas Monjalon
The command
  make examples
works only if target directories have the exact name of configs.

It is more flexible to use
  make -C examples RTE_SDK=$(pwd) RTE_TARGET=build

Signed-off-by: Thomas Monjalon 
---
 mk/rte.sdkexamples.mk | 77 ---
 mk/rte.sdkroot.mk |  4 ---
 2 files changed, 81 deletions(-)
 delete mode 100644 mk/rte.sdkexamples.mk

diff --git a/mk/rte.sdkexamples.mk b/mk/rte.sdkexamples.mk
deleted file mode 100644
index 111ce91..000
--- a/mk/rte.sdkexamples.mk
+++ /dev/null
@@ -1,77 +0,0 @@
-#   BSD LICENSE
-#
-#   Copyright(c) 2014 6WIND S.A.
-#
-#   Redistribution and use in source and binary forms, with or without
-#   modification, are permitted provided that the following conditions
-#   are met:
-#
-# * Redistributions of source code must retain the above copyright
-#   notice, this list of conditions and the following disclaimer.
-# * Redistributions in binary form must reproduce the above copyright
-#   notice, this list of conditions and the following disclaimer in
-#   the documentation and/or other materials provided with the
-#   distribution.
-# * Neither the name of 6WIND S.A. nor the names of its
-#   contributors may be used to endorse or promote products derived
-#   from this software without specific prior written permission.
-#
-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-# examples application are seen as external applications which are
-# not part of SDK.
-BUILDING_RTE_SDK :=
-export BUILDING_RTE_SDK
-
-# Build directory is given with O=
-O ?= $(RTE_SDK)/examples
-
-# Target for which examples should be built.
-T ?= *
-
-# list all available configurations
-EXAMPLES_CONFIGS := $(patsubst $(RTE_SRCDIR)/config/defconfig_%,%,\
-   $(wildcard $(RTE_SRCDIR)/config/defconfig_$(T)))
-EXAMPLES_TARGETS := $(addsuffix _examples,\
-   $(filter-out %~,$(EXAMPLES_CONFIGS)))
-
-.PHONY: examples
-examples: $(EXAMPLES_TARGETS)
-
-%_examples:
-   @echo == Build examples for $*
-   $(Q)if [ ! -d "${RTE_SDK}/${*}" ]; then \
-   echo "Target ${*} does not exist in ${RTE_SDK}/${*}." ; \
-   echo -n "Please install DPDK first (make install) or use 
another " ; \
-   echo "target argument (T=target)." ; \
-   false ; \
-   else \
-   $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(*); \
-   fi
-
-EXAMPLES_CLEAN_TARGETS := $(addsuffix _examples_clean,\
-   $(filter-out %~,$(EXAMPLES_CONFIGS)))
-
-.PHONY: examples_clean
-examples_clean: $(EXAMPLES_CLEAN_TARGETS)
-
-%_examples_clean:
-   @echo == Clean examples for $*
-   $(Q)if [ ! -d "${RTE_SDK}/${*}" ]; then \
-   echo "Target ${*} does not exist in ${RTE_SDK}/${*}." ; \
-   echo -n "Please install DPDK first (make install) or use 
another " ; \
-   echo "target argument (T=target)." ; \
-   false ; \
-   else \
-   $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(*) clean; \
-   fi
diff --git a/mk/rte.sdkroot.mk b/mk/rte.sdkroot.mk
index 04ad523..81233ed 100644
--- a/mk/rte.sdkroot.mk
+++ b/mk/rte.sdkroot.mk
@@ -117,10 +117,6 @@ depdirs depgraph:
 gcov gcovclean:
$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkgcov.mk $@

-.PHONY: examples examples_clean
-examples examples_clean:
-   $(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkexamples.mk $@
-
 # all other build targets
 %:
$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkconfig.mk checkconfig
-- 
2.7.0



[dpdk-dev] [PATCH] scripts: check cc stable mailing list in commit

2016-11-21 Thread Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
They must be sent CC: stable at dpdk.org.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.

Signed-off-by: Thomas Monjalon 
---
 scripts/check-git-log.sh | 9 +
 1 file changed, 9 insertions(+)

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index 5f8a9fc..4f98a7a 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -47,12 +47,14 @@ if [ "$1" = '-h' -o "$1" = '--help' ] ; then
exit
 fi

+selfdir=$(dirname $(readlink -e $0))
 range=${1:-origin/master..}

 commits=$(git log --format='%h' --reverse $range)
 headlines=$(git log --format='%s' --reverse $range)
 bodylines=$(git log --format='%b' --reverse $range)
 fixes=$(git log --format='%h %s' --reverse $range | grep -i ': *fix' | cut -d' 
' -f1)
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d'  | cut -d' ' 
-f2)
 tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e 'fix.*:')
 bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by:'

@@ -191,3 +193,10 @@ bad=$(for fixtag in $fixtags ; do
printf "$fixtag" | grep -v "^$good$"
 done | sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+   git log --format='%b' -1 $fix | grep -qi '^CC: *stable at dpdk.org' ||
+   git log --format='\t%s' -1 $fix
+done)
+[ -z "$bad" ] || printf "Should CC: stable at dpdk.org\n$bad\n"
-- 
2.7.0



[dpdk-dev] [PATCH] scripts: fix checkpatch from standard input

2016-11-21 Thread Thomas Monjalon
When checking a valid patch from standard input,
the footer lines of the report are not filtered out.

The function check is called outside of any loop,
so the statement continue has no effect and the footer is printed.

Fixes: 8005feef421d ("scripts: add standard input to checkpatch")

Signed-off-by: Thomas Monjalon 
---
 scripts/checkpatches.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatches.sh b/scripts/checkpatches.sh
index 336cc7b..cfe262b 100755
--- a/scripts/checkpatches.sh
+++ b/scripts/checkpatches.sh
@@ -94,7 +94,7 @@ check () { #   
else
report=$($DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
fi
-   [ $? -ne 0 ] || continue
+   [ $? -ne 0 ] || return 0
$verbose || printf '\n### %s\n\n' "$3"
printf '%s\n' "$report" | sed -n '1,/^total:.*lines checked$/p'
status=$(($status + 1))
-- 
2.7.0



[dpdk-dev] [RFC PATCH 0/6] Restructure EAL device model for bus support

2016-11-21 Thread Thomas Monjalon
2016-11-20 16:30, David Marchand:
> For a first patchset, I would see:
> - introduce the rte_bus object. In rte_eal_init, for each bus, we call
> the scan method. Then, for each bus, we find the appropriate
> rte_driver using the bus match method then call the probe method. If
> the probe succeeds, the rte_device points to the associated
> rte_driver,
> - migrate the pci scan code to a pci bus (scan looks at sysfs for
> linux / ioctl for bsd + devargs for blacklist / whitelist ?), match is
> the same at what is done in rte_eal_pci_probe_one_driver() at the
> moment,
> - migrate the vdev init code to a vdev bus (scan looks at devargs):
> this is new, we must create rte_device objects for vdev drivers to use
> later

I think it can be 3 patchsets.
Who can work on the vdev part please?

> Then we can talk about the next steps once the bus is in place.

Yes


[dpdk-dev] Proposal for a new Committer model

2016-11-21 Thread Thomas Monjalon
2016-11-18 13:09, Neil Horman:
> A) Further promote subtree maintainership.  This was a conversation that I
> proposed some time ago, but my proposed granularity was discarded in favor
> of something that hasn't worked as well (in my opinion).  That is to say a
> few driver pmds (i40e and fm10k come to mind) have their own tree that
> send pull requests to Thomas.

Yes we tried this fine granularity and stated that it was not working well.
We are now using the bigger granularity that you describe below.

> We should be sharding that at a much higher
> granularity and using it much more consistently.  That is to say, that we
> should have a maintainer for all the ethernet pmds, and another for the
> crypto pmds, another for the core eal layer, another for misc libraries
> that have low patch volumes, etc.

Yes we could open a tree for EAL and another one for the core libraries.

> Each of those subdivisions should have
> their own list to communicate on, and each should have a tree that
> integrates patches for their own subsystem, and they should on a regular
> cycle send pull requests to Thomas.

Yes I think it is now a good idea to split the mailing list traffic,
at least for netdev and cryptodev.

> Thomas in turn should by and large,
> only be integrating pull requests.  This should address our high-
> throughput issue, in that it will allow multiple maintainers to share the
> workload, and integration should be relatively easy.

Yes in an ideal organization, the last committer does only a last check
that technical plan and fairness are respected.
So it gives more time to coordinate the plans :)

> B) Designate alternates to serve as backups for the maintainer when they
> are unavailable.  This provides high-availablility, and sounds very much
> like your proposal, but in the interests of clarity, there is still a
> single maintainer at any one time, it just may change to ensure the
> continued merging of patches, if the primary maintainer isn't available.
> Ideally however, those backup alternates arent needed, because most of the
> primary maintainers work in merging pull requests, which are done based on
> the trust of the submaintainer, and done during a very limited window of
> time.  This also partially addreses multi-vendor fairness if your subtree
> maintainers come from multiple participating companies.

About the merge window, I do not have a strong opinion about how it can be
improved. However, I know that closing the window too early makes developer
unhappy because it makes wait - between development start and its release -
longer.


[dpdk-dev] Solarflare PMD submission question

2016-11-21 Thread Thomas Monjalon
2016-11-18 19:50, Andrew Rybchenko:
> Now we have a split of the base driver import in big feature steps. The 
> base driver is split into 28 patches. Just only 1 patch exceeds 300K 
> boundary (which add MCDI definitions header).

Good

> Before submitting 56 patches I'd like to double-check that checkpatch.pl 
> errors (for example, because of assignments in the 'if' condition, 
> parenthesis around return value) is not a show-stopper for base driver 
> import.

You can run checkpatches.sh or send the patches to checkpatch at dpdk.org.
The script check-git-log.sh can also guide you for the expected formatting.


[dpdk-dev] Solarflare PMD submission question

2016-11-21 Thread Thomas Monjalon
2016-11-21 11:46, Andrew Rybchenko:
> On 11/21/2016 11:19 AM, Thomas Monjalon wrote:
> >> Before submitting 56 patches I'd like to double-check that checkpatch.pl
> >> errors (for example, because of assignments in the 'if' condition,
> >> parenthesis around return value) is not a show-stopper for base driver
> >> import.
> > You can run checkpatches.sh or send the patches to checkpatch at dpdk.org.
> > The script check-git-log.sh can also guide you for the expected formatting.
> 
> Yes, I did it and it helped me to find and fix some coding standard 
> violations.
> 
> The problem with libefx (base driver) is that it is existing code which 
> follows FreeBSD and illumos coding conventions which contradict to 
> checkpatches.sh sometimes (e.g. require parenthesis around return 
> value). Other example of error produced by checkpatches.sh is assign in 
> if. It is widely used in the code to assign return code value and 
> compare it vs 0 in one line. It is not a coding standard conflict, but 
> it is very wide-spread in the code (so changing it will produce too many 
> changes not strictly required/useful).
> 
> So, may I repeat my question if it is a show-stopper for base driver or 
> acceptable.

I would vote to accept these minor style warnings for the base driver.
Ferruh, any comment?


[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info

2016-11-18 Thread Thomas Monjalon
2016-11-18 16:06, Alejandro Lucero:
> Previous reported speed was hardcoded.
> 
> Signed-off-by: Alejandro Lucero 
> ---
>  drivers/net/nfp/nfp_net.c  | 28 ++--
>  drivers/net/nfp/nfp_net_ctrl.h | 13 +
>  2 files changed, 39 insertions(+), 2 deletions(-)

You should update the doc in the same patch:
doc/guides/nics/features/nfp.ini
It will be the first feature as the file appears to be empty.
So you will need another patch to fill other existing features.

I have an unrelated question: why nfp is disabled in the default build?



[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Thomas Monjalon
2016-11-18 15:31, Alejandro Lucero:
> On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit 
> wrote:
> 
> > On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > > Hi Thomas,
> > >
> > > I got this email when sending a patch some minutes ago.
> > >
> > > The point is I trusted script/checkpatches.sh which did not report those
> > > warnings.
> > > Am I doing anything wrong when using checkpatches.sh?
> >
> > I am also getting same warnings as below, this can be related to the
> > checkpatch.pl version.
> >
> > I have: Version: 0.32
> > (./scripts/checkpatch.pl --version)
> >
> Uhmm, I got same one.

The last update of this version number is from 2011...
I guess we have to live without checkpatch versioning.


[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Thomas Monjalon
2016-11-18 15:24, Ferruh Yigit:
> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > Hi Thomas,
> > 
> > I got this email when sending a patch some minutes ago.
> > 
> > The point is I trusted script/checkpatches.sh which did not report those
> > warnings.
> > Am I doing anything wrong when using checkpatches.sh?
> 
> I am also getting same warnings as below, this can be related to the
> checkpatch.pl version.
> 
> I have: Version: 0.32
> (./scripts/checkpatch.pl --version)

Yes checkpatch at dpdk.org uses the version 0.32.
I could try to add it in the mail report.


[dpdk-dev] Clarification for - SoC specific driver based common sub component placing

2016-11-18 Thread Thomas Monjalon
2016-11-18 14:38, Thomas Monjalon:
> 2016-11-18 12:44, Hemant Agrawal:
> > We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) 
> > Poll mode drivers into the DPDK.
> > 
> > We need some clarification w.r.t the right placing of some dependent 
> > components, which can be common across drivers. E.g. We have hardware queue 
> > and buffer manager driver. This will be used by both network driver and 
> > crypto driver. But it is specific to NXP platform only.
> > 
> > What is the right place for such common hardware specific components in 
> > DPDK? 
> > 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For 
> > each soc configuration only the required components will be compiled-in. 
> > 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. 
> > And link the network and crypto drivers to it.
> > 3. Add it to main network driver and make the crypto driver dependent 
> > on it.
> 
> Your question is more generic than SoC context.
> You just want to share some code between drivers, right?
> What about building a library located in drivers/common/nxp/ ?

I'm a bit reluctant to have company name in file hierarchy,
as it not something stable. And especially for NXP/Qualcomm...

In this case would it be better to name the directory
drivers/common/dpaa2/ ?


[dpdk-dev] Clarification for - SoC specific driver based common sub component placing

2016-11-18 Thread Thomas Monjalon
2016-11-18 12:44, Hemant Agrawal:
> We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) 
> Poll mode drivers into the DPDK.
> 
> We need some clarification w.r.t the right placing of some dependent 
> components, which can be common across drivers. E.g. We have hardware queue 
> and buffer manager driver. This will be used by both network driver and 
> crypto driver. But it is specific to NXP platform only.
> 
> What is the right place for such common hardware specific components in DPDK? 
>   1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For 
> each soc configuration only the required components will be compiled-in. 
>   2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. 
> And link the network and crypto drivers to it.
>   3. Add it to main network driver and make the crypto driver dependent 
> on it.

Your question is more generic than SoC context.
You just want to share some code between drivers, right?
What about building a library located in drivers/common/nxp/ ?


[dpdk-dev] [dpdk-announce] DPDK 16.11 released

2016-11-17 Thread Thomas Monjalon
2016-11-17 03:51, Xu, Qian Q:
> It is good that RC1 on Jan.11th is a hard deadline. We also need ensure that 
> RC1 is a complete package with all features. If RC1 is out but missing some 
> big features, then the test results on RC1 vs RC2 may have big difference. 
> So, could we ensure that RC1 is a complete feature package, if any feature 
> missing RC1, then we may postpone the feature to 17.05? Does it make sense? 

Absolutely, yes.


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, November 15, 2016 5:06 PM
> To: Liu, Yong 
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-announce] DPDK 16.11 released
> 
> Hi and thanks for sharing your time constraints,
> 
> 2016-11-15 01:46, Liu, Yong:
> > As prospect for 17.02, our intel validation team have some concern about 
> > the release date.
> > The official day off for Chinese Sprint Festival holiday will be from 27th 
> > Jan to 3th Feb.
> > Most of our members may ask for more days leave either before or after the 
> > official day off.
> > From our previous experience, it will take 3~4 weeks to do the full 
> > function and performance test.
> > If the first candidate release in the middle of Jan, we can do first 
> > round of validation and raise issues to developers.
> 
> The integration deadline is January 5.
> So we can target/expect a RC1 on January 11.
> 
> > And after the holiday, we can keep on the validation process and finish in 
> > two weeks.
> > If release date is after Feb, it will be hard for us to cover all cases in 
> > release window.
> 
> Yes, we must remind that mid-January is a hard deadline for RC1.
> Then the release will be in mid-February to make sure you have some time 
> after the holidays.
> What about Valentine's day? :)




[dpdk-dev] [PATCH 1/5] ethdev: add firmware version get

2016-11-17 Thread Thomas Monjalon
2016-11-17 17:42, Qiming Yang:
> This patch added API for 'rte_eth_dev_fwver_get'
> 
> void rte_eth_dev_fwver_get(uint8_t port_id,
> char *fw_version, int fw_length);

Copying some code here doesn't help really help.
Could you describe what we can expect in this string?
How can we compare this version number across different
devices of the same driver?
How does it apply to FPGA devices?
What is the potential use?

[...]
>  /**
> + * Retrieve the firmware version of an Ethernet device.

We should stop talking about Ethernet device.
Networking device is more appropriate.
And in this case, "device" is enough.

> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param fw_version
> + *   A pointer the firmware version of an Ethernet device
> + * @param fw_length
> + *   The size of the firmware version, which should be large enough to store
> + *   the firmware version of the device.

How do we know that the length is too small?

> + */
> +void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int fw_length);

Why not returning some errors?

You forgot to remove the deprecation notice in this patch.

PS: the series is broken as some patches are not numbered and this one is
not the first one. Please use a cover-letter to introduce such series.


[dpdk-dev] [PATCH] doc: add pdump library to API doxygen

2016-11-15 Thread Thomas Monjalon
2016-11-15 14:41, Reshma Pattan:
> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -140,7 +140,8 @@ There are many libraries, so their headers may be grouped 
> by topics:
>[debug]  (@ref rte_debug.h),
>[log](@ref rte_log.h),
>[warnings]   (@ref rte_warnings.h),
> -  [errno]  (@ref rte_errno.h)
> +  [errno]  (@ref rte_errno.h),
> +  [pdump]  (@ref rte_pdump.h)

The end of this list are trivial headers, what pdump is not.
I think it should be at the top of the list, between jobstats and hexdump.


[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler

2016-11-15 Thread Thomas Monjalon
2016-11-15 09:27, Neil Horman:
> On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote:
> > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote:
> > > > > Hi Neil,
> > > > >Pmdinfogen compiles with host compiler. It usages rte_byteorder.h
> > > > >of the target platform.
[...]
> > > Yeah, so what we need is a way to get to the host version of 
> > > rte_byteorder.h
> > > when building in a cross environment
> > > 
> > +1 
> > 
> Actually, looking at this, I think we're 90% there anyway.  The buildtools
> already uses the hostapp.mk file (as it should), to target the host build
> system, rather than any cross target, so we're good there.  The only question
> is, should we be using rte_byteorder.h at all, and I think the answer is no, 
> we
> shouldn't.  I assert that because the byteorder file is configured for the
> target, not the host, and so we shouldn't be touching it at all.  Instead we
> should just be using the posix htobe*/htole*/letoh*/betoh* variants to do the
> endian conversion, as those are always configured to work on the local build
> host.

Yes there are 2 possible fixes:
- get a host version of EAL
- do not use EAL for host applications

As Neil, I think there is no point in using EAL for a host application.


[dpdk-dev] [dpdk-announce] DPDK 16.11 released

2016-11-15 Thread Thomas Monjalon
Hi and thanks for sharing your time constraints,

2016-11-15 01:46, Liu, Yong:
> As prospect for 17.02, our intel validation team have some concern about the 
> release date.
> The official day off for Chinese Sprint Festival holiday will be from 27th 
> Jan to 3th Feb.
> Most of our members may ask for more days leave either before or after the 
> official day off.
> From our previous experience, it will take 3~4 weeks to do the full function 
> and performance test.
> If the first candidate release in the middle of Jan,
> we can do first round of validation and raise issues to developers.

The integration deadline is January 5.
So we can target/expect a RC1 on January 11.

> And after the holiday, we can keep on the validation process and finish in 
> two weeks.
> If release date is after Feb, it will be hard for us to cover all cases in 
> release window.

Yes, we must remind that mid-January is a hard deadline for RC1.
Then the release will be in mid-February to make sure you have some time
after the holidays.
What about Valentine's day? :)


[dpdk-dev] [PATCH v1] doc: add template release notes for 17.02

2016-11-14 Thread Thomas Monjalon
> > Add template release notes for DPDK 17.02 with inline
> > comments and explanations of the various sections.
> >
> > Signed-off-by: John McNamara 
> 
> Acked-by: Remy Horton 

Applied, thanks
We are ready to start a new release cycle.
The version in the master branch is now 17.02-rc0.


[dpdk-dev] [PATCH v2] doc: add sub-repositories information

2016-11-14 Thread Thomas Monjalon
2016-11-11 13:34, Ferruh Yigit:
> DPDK switched to main and sub-repositories approach, this patch
> documents new approach and updates development process according.
> 
> Signed-off-by: Ferruh Yigit 
> Acked-by: John McNamara 

Applied, thanks




[dpdk-dev] [PATCH] maintainers: add staging tree for network drivers

2016-11-14 Thread Thomas Monjalon
> >  Networking Drivers
> >  --
> > +M: Ferruh Yigit 
> > +T: git://dpdk.org/next/dpdk-next-net
> 
> Acked-by: Thomas Monjalon 
> 
> It will be applied at the beginning of 17.02 cycle to reflect the change.

Applied, thanks.

So from now, Ferruh will be responsible of the next-net tree.
Thank you for your commitment.


[dpdk-dev] [dpdk-announce] DPDK 16.11 released

2016-11-13 Thread Thomas Monjalon
A new major release is available:
http://fast.dpdk.org/rel/dpdk-16.11.tar.xz

It has been built by an important community:
728 patches from 119 authors
773 files changed, 60728 insertions(+), 1220384 deletions(-)

There are 56 new contributors
(including authors, reviewers and testers):
Thanks to Alain Leon, Aleksey Katargin, Alex Zelezniak, Ali Volkan Atli,
Amine Kherbouche, Ananda Sathyanarayana, Ben Walker, Byron Marohn,
Daniele Di Proietto, Daniel Verkamp, Deirdre O'Connor, Elad Persiko,
E. Scott Daniels, Frederico Cadete, Gary Mussar, Gowrishankar Muthukrishnan,
Guolin Yang, Guruprasad Rao, James Poole, Jason Wang, Jean Tourrilhes,
Jiayu Hu, John Carney, John Ousterhout, Jon Loeliger, Jozef Martiniak,
Juho Snellman, Karmarkar Suyash, Luca Boccassi, Masoud Hasanifard,
Matthias Gatto, Mohammad Abdul Awal, Morten Br?rup, Nikhil Jagtap,
Ning Li, Nipun Gupta, Olivier Gournet, Patrick Kutch, Pierre Pfister,
Qiming Yang, Qi Zhang, Rahul R Shah, Sagi Grimberg, Saikrishna Edupuganti,
Sarath Somasekharan, Souvik Dey, Tal Avraham, Tao Y Yang, Vincent Guo,
Wang Wei, Weiliang Luo, Wei Zhao, Xuekun Hu, Yangchao Zhou, Yunjian Wang,
Zhiyong Yang.

These new contributors are associated with these domain names:
163.com, 6wind.com, annapurnalabs.com, argela.com.tr, att.com,
broadcom.com, brocade.com, ciena.com, cisco.com, corp.free.fr,
cs.stanford.edu, gmail.com, grimberg.me, huawei.com, iki.fi,
intel.com, jd.com, labs.hpe.com, linux.vnet.ibm.com, mellanox.com,
netgate.com, nxp.com, oneaccess-net.com, outscale.com, redhat.com,
research.att.com, smartsharesystems.com, sonusnet.com,
versa-networks.com, vmware.com, 

Some highlights:
- new device object model
- more networking offloads
- improved vhost
- virtio for NEON on ARM
- new crypto libraries (ZUC and OpenSSL)
More details in the release notes:
http://dpdk.org/doc/guides/rel_notes/release_16_11.html

The new features for the 17.02 cycle must be submitted before December 4.
There is a long list of expected works:
http://dpdk.org/dev/roadmap
It means we will have a huge workload to properly review all the new stuff
before the end of the year. Do not forget to help reviewing patches from
others if we want to have a chance to integrate everything in time.

Other works in progress:
- A structure in Linux Foundation is being discussed at moving at dpdk.org
http://dpdk.org/ml/listinfo/ci
- A CI process is being discussed at ci at dpdk.org
http://dpdk.org/ml/listinfo/moving

Thanks everyone


[dpdk-dev] [PATCH] improve git diff

2016-11-13 Thread Thomas Monjalon
2016-11-09 16:44, Thomas Monjalon:
> Sometimes git does not print the name of the function being changed
> after @@. It happens especially after a goto label which is not indented.
> Giving a hint about the languages of files .c, .h and .py
> will improve hunk headers of "git diff" rendering.
> 
> Signed-off-by: Thomas Monjalon 

Applied


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

2016-11-13 Thread Thomas Monjalon
2016-11-11 12:04, John McNamara:
> Fix grammar, spelling and formatting of DPDK 16.11 release notes.
> 
> Signed-off-by: John McNamara 

Applied, thanks


[dpdk-dev] [PATCH] doc: announce ABI change for ethtool app enhance

2016-11-13 Thread Thomas Monjalon
> > This patch adds a notice that the ABI change for ethtool app to get the NIC
> > firmware version in the 17.02 release.
> > 
> > Signed-off-by: Qiming Yang 
Acked-by: Jingjing Wu 
Acked-by: Beilei Xing 
> Acked-by: Helin Zhang 



[dpdk-dev] [PATCH v1] doc: announce API and ABI change for librte_ether

2016-11-13 Thread Thomas Monjalon
> >> In 17.02 five rte_eth_dev_set_vf_*** functions will be removed from
> >> librte_ether, renamed and moved to the ixgbe PMD.
> >>
> >> Signed-off-by: Bernard Iremonger 
> > 
> > Acked-by: John McNamara 
Acked-by: Reshma Pattan 
> Acked-by: Ferruh Yigit 

Applied


[dpdk-dev] [PATCH v1] doc: announce API change for ethdev function

2016-11-13 Thread Thomas Monjalon
> > The _rte_eth_dev_call_process function will change to return "int"
> > and a fourth parameter "void* ret_param" will be added. This change
> > targets release 17.02.
> > 
> > Signed-off-by: Bernard Iremonger 
> 
> Acked-by: John McNamara 

The real API change is in rte_eth_dev_cb_fn but we understand the idea.

Acked-by: Thomas Monjalon 

Applied


[dpdk-dev] [PATCH] doc: announce API and ABI changes for librte_eal

2016-11-13 Thread Thomas Monjalon
> > >> Signed-off-by: Shreyansh Jain 
> > >
> > > Acked-by: David Marchand 
> > 
> > Acked-by: Ferruh Yigit 
> 
> Acked-by: Reshma Pattan 

Applied


[dpdk-dev] [PATCH] doc: remove iomem and ioport handling in igb_uio

2016-11-13 Thread Thomas Monjalon
> > Suggested-by: Yigit, Ferruh 
> > Signed-off-by: Jianfeng Tan 
> 
> Acked-by: Remy Horton 

Applied


[dpdk-dev] [PATCH v1] doc: rearrange the high level documentation index

2016-11-13 Thread Thomas Monjalon
2016-11-11 13:45, John McNamara:
> Rearrange the order of the high level documenation index into
> a more logical sequence for a new user.
> 
> Also, improve some of the high-level document names.
> 
> Signed-off-by: John McNamara 

Applied, thanks


[dpdk-dev] [PATCH] doc: add more tested platforms and nics and OSes

2016-11-12 Thread Thomas Monjalon
> > Add more tested platforms and nics and OSes to the release notes.
> > 
> > Signed-off-by: Yulong Pei 
> 
> Acked-by: John McNamara 

Applied, thanks


[dpdk-dev] [PATCH v2] doc: add known issue on QAT PMD into release notes

2016-11-12 Thread Thomas Monjalon
> > Issue is with the digest appended feature on QAT PMD.
> > A workaround is also documented.
> >
> > Signed-off-by: Fiona Trahe 
> > Acked-by: John McNamara 
> Acked-by: John Griffin 

Applied, thanks


[dpdk-dev] [PATCH] pdump: fix log message to display correct error number

2016-11-12 Thread Thomas Monjalon
> > The ethdev Rx/Tx remove callback apis doesn't set rte_errno during
> > failures, instead they just return negative error number, so using
> > that number in logs instead of rte_errno upon Rx and Tx callback
> > removal failures.
> > 
> > Fixes: 278f9454 ("pdump: add new library for packet capture")
> > 
> > Signed-off-by: Reshma Pattan 
> 
> Acked-by: Pablo de Lara 

Applied, thanks


[dpdk-dev] [PATCH] app/test: fix crash of lpm test

2016-11-12 Thread Thomas Monjalon
> > The test recently added accesses to lpm->tbl8[ip >> 8] with is much larger 
> > than
> > the size of the table, causing a crash of the test application.
> > 
> > Fix this typo by replacing tbl8 by tbl24.
> > 
> > Fixes: 231fa88ed522 ("app/test: verify LPM tbl8 recycle")
> > 
> > Signed-off-by: Olivier Matz 
> Acked-by: Wei Dai 

Applied, thanks


[dpdk-dev] [PATCH v2] mempool: Free memzone if mempool populate phys fails

2016-11-12 Thread Thomas Monjalon
> > This patch fixes the issue of memzone not being freed incase the
> > rte_mempool_populate_phys fails in the rte_mempool_populate_default
> > 
> > This issue was identified when testing with OVS ~2.6
> > - configure the system with low memory (e.g. < 500 MB)
> > - add bridge and dpdk interfaces
> > - delete brigde
> > - keep on repeating the above sequence.
> > 
> > Fixes: d1d914ebbc25 ("mempool: allocate in several memory chunks by 
> > default")
> > 
> > Signed-off-by: Nipun Gupta 
> 
> Acked-by: Olivier Matz 

Applied, thanks


[dpdk-dev] [PATCH v2] net/qede: fix unknown speed errmsg for 25G link

2016-11-12 Thread Thomas Monjalon
2016-11-11 09:41, Harish Patil:
> - Fix to use bitmapped values in NVM configuration for speed capability
>   advertisement. This issue is specific to 25G NIC since it is capable
>   of 25G and 10G speeds.
> 
> - Update feature list.
> 
> Fixes: 64c239b7f8b7 ("net/qede: fix advertising link speed capability")
> 
> Signed-off-by: Harish Patil 

Applied, thanks


[dpdk-dev] [PATCH] improve git diff

2016-11-12 Thread Thomas Monjalon
2016-11-11 17:28, Ferruh Yigit:
> On 11/11/2016 4:21 PM, Thomas Monjalon wrote:
> > 2016-11-11 11:22, Ferruh Yigit:
> >> On 11/9/2016 3:44 PM, Thomas Monjalon wrote:
> >>> Sometimes git does not print the name of the function being changed
> >>> after @@. It happens especially after a goto label which is not indented.
> >>> Giving a hint about the languages of files .c, .h and .py
> >>> will improve hunk headers of "git diff" rendering.
> >>>
> >>> Signed-off-by: Thomas Monjalon 
> > [...]
> >>> --- /dev/null
> >>> +++ b/.gitattributes
> >>> @@ -0,0 +1,3 @@
> >>> +*.c   diff=cpp
> >>> +*.h   diff=cpp
> >>
> >> Can't git auto detect to use C/C++ language diff use for .c/.h files?
> > 
> > No
> > 
> >> Do you have a sample that generates bad hunk header, just to test?
> > 
> > Yes, you'll find a lot of them with "git log -p | grep '@@.*:'"
> > Example:
> > git show bb6722f | grep '@@.*:'
> > Without the patch, it is a goto label in the hunk header.
> > 
> 
> You are right, I was expecting better from git J

Sometimes, less is more :)


[dpdk-dev] [PATCH v1] doc: rearrange the high level documentation index

2016-11-11 Thread Thomas Monjalon
2016-11-11 13:53, Mcnamara, John:
> I would prefer not to have some of these are highest level items such as
> "Xen Guide", and possibly "Crypto Device Drivers". Perhaps we could push
> them down a level with "Network Interface Controller Drivers" under a
> "Devices and Drivers" (or similar) section like:
> 
> * ...
> * Programmer's Guide
> * HowTo Guides
> * DPDK Tools User Guides
> * Devices and Drivers  
> * Network Interface Controller Drivers
> * Crypto Device Drivers
> * Xen Guide

Yes it may be an idea.
But as we will continue to support Xen Dom0, I think Xen should have
a special place somewhere (in Linux guide or separated)?


[dpdk-dev] [PATCH] improve git diff

2016-11-11 Thread Thomas Monjalon
2016-11-11 11:22, Ferruh Yigit:
> On 11/9/2016 3:44 PM, Thomas Monjalon wrote:
> > Sometimes git does not print the name of the function being changed
> > after @@. It happens especially after a goto label which is not indented.
> > Giving a hint about the languages of files .c, .h and .py
> > will improve hunk headers of "git diff" rendering.
> > 
> > Signed-off-by: Thomas Monjalon 
[...]
> > --- /dev/null
> > +++ b/.gitattributes
> > @@ -0,0 +1,3 @@
> > +*.c   diff=cpp
> > +*.h   diff=cpp
> 
> Can't git auto detect to use C/C++ language diff use for .c/.h files?

No

> Do you have a sample that generates bad hunk header, just to test?

Yes, you'll find a lot of them with "git log -p | grep '@@.*:'"
Example:
git show bb6722f | grep '@@.*:'
Without the patch, it is a goto label in the hunk header.


[dpdk-dev] [PATCH] mempool: Free memzone if mempool populate phys fails

2016-11-11 Thread Thomas Monjalon
2016-11-11 20:12, Hemant Agrawal:
> From: Nipun Gupta 
> 
> This fixes the issue of memzone not being freed, if the
> rte_mempool_populate_phys fails in the rte_mempool_populate_default
> 
> This issue was identified when testing with OVS ~2.6
> - configure the system with low memory (e.g. < 500 MB)
> - add bridge and dpdk interfaces
> - delete brigde
> - keep on repeating the above sequence.
> 
> Signed-off-by: Nipun Gupta 

You forgot the "Fixes:" line to give a tip on the release introducing this bug.


[dpdk-dev] [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

2016-11-11 Thread Thomas Monjalon
2016-11-11 09:16, Alejandro Lucero:
> Thomas,
> 
> We are wondering if you realize this patch fixes a bug with current ethdev
> code as a device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS.
> 
> Maybe the commit message is giving the wrong impression and as you
> commented, it should just focus on the bug it fixes and to leave for
> another email thread the discussion of how to solve the
> RTE_ETHDEV_QUEUE_STAT_CNTRS
> problem.
> 
> Should we remove this from patchwork and to send another patch that way?

Yes please. It was my first comment, we don't understand the exact issue
you are fixing.
And I have a bad feeling it could break something else (really just a feeling).
It is not the kind of patch we can apply the last day of a release.
That's why I think it should wait 17.02.

Of course you can try to convince me and others to apply it as a last minute
patch. But why are you sending a patch on the generic API in the last days?

Last argument: it is not fixing a regression of 16.11, so it is not so urgent.


[dpdk-dev] [PATCH] net/ixgbe: fix link never come up problem with x552

2016-11-11 Thread Thomas Monjalon
> > From: zhao wei 
> > 
> > The links never coming up with DPDK16.11 when bring up x552 NIC, device id 
> > is
> > 15ac.This is caused by delete some code which casing removes X550em SFP iXFI
> > setup for the drivers in function ixgbe_setup_mac_link_sfp_x550em().Fix
> > methord is recover the deleted code.
> > 
> > Fixes: 1726b9cd9c40 ("net/ixgbe/base: remove X550em SFP iXFI setup")
> > 
> > Signed-off-by: zhao wei 
> Acked-by: Wenzhuo Lu 
> Thanks for the patch. It's a critical issue. We need to fix it. I'll report 
> it to our kernel driver developers.

Applied, thanks


[dpdk-dev] [PATCH 0/2] net/thunderx: add 83xx SoC support

2016-11-11 Thread Thomas Monjalon
2016-11-08 12:01, Jerin Jacob:
> CN83xx is 24 core version of ThunderX ARMv8 SoC with integrated
> octeon style packet and crypto accelerators.
> 
> The standard NIC block used in 88xx/81xx also included in 83xx.
> This patchset adds support for existing standard NIC block on 83xx by
> adding new HW capability flag to select the difference in runtime.
> 
> Jerin Jacob (2):
>   net/thunderx: disable l3 alignment pad feature
>   net/thunderx: add cn83xx support

Applied, thanks


[dpdk-dev] [PATCH v3] doc: arm64: document DPDK application profiling methods

2016-11-11 Thread Thomas Monjalon
> > Signed-off-by: Jerin Jacob 
> > Signed-off-by: John McNamara 
> 
> Acked-by: Jianbo Liu 

Applied, thanks


[dpdk-dev] [PATCH] doc: postpone ABI changes for Tx prepare

2016-11-11 Thread Thomas Monjalon
> > The changes for the feature "Tx prepare" should be made in version 17.02.
> > 
> > Signed-off-by: Thomas Monjalon 
> 
> Acked-by: Tomasz Kulasek 

Applied


[dpdk-dev] [PATCH] doc: move testpmd guide with other tools

2016-11-11 Thread Thomas Monjalon
2016-11-10 23:18, Wiles, Keith:
> > On Nov 10, 2016, at 5:02 PM, Thomas Monjalon  
> > wrote:
> > 2016-11-10 16:11, Mcnamara, John:
> >> The problem is that TestPMD is a bit of an outlier. It isn't a sample 
> >> application and it isn't really a test application despite the name (it is 
> >> more of a tester application). Also I don't think that it is a 
> >> tool/utility like the other apps in the target directory (if it is seen as 
> >> a tool then it should be renamed to something like dpdk-tester for 
> >> consistency). Testpmd also has quite a lot of documentation, more than any 
> >> of our other apps or utilities, which again makes it an outlier.
> > 
> > Yes testpmd is not the same kind of tool as others. It helps for tests,
> > debugging and demos.
> > 
> > About the name, as it is packaged as part of the runtime, I think we should
> > find a better name. As you said it should start with "dpdk-" and it should
> > contain "net" as it does not test the crypto drivers.
> > Something like dpdk-testpmd-net.
> 
> To me the name dpdk-testpmd-net is a bit long and does testpmd really just 
> test PMDs. I was thinking of the name dpdk-tester is really pretty short and 
> descriptive IMO. Adding net or pmd to the name does not really add anything 
> as dpdk is kind of networking. Just my $0.04 worth. 

I tend to agree. dpdk-tester is not so bad.
So this application could be able to test the crypto engines?


[dpdk-dev] [PATCH] doc: fix l3fwd mode selection from compile to run time

2016-11-11 Thread Thomas Monjalon
> > The l3fwd application route lookup mode can be selected at run time but
> > not at compile time. This patch corrects the statement in the doc.
> > 
> > Fixes: d0dff9ba ("doc: sample application user guide")
> > 
> > Signed-off-by: Reshma Pattan 
> 
> Acked-by: John McNamara 


Applied, thanks


[dpdk-dev] [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

2016-11-10 Thread Thomas Monjalon
2016-11-10 15:43, Alejandro Lucero:
> On Thu, Nov 10, 2016 at 2:42 PM, Thomas Monjalon  6wind.com>
> wrote:
> 
> > 2016-11-10 14:00, Alejandro Lucero:
> > > From: Bert van Leeuwen 
> > >
> > > A device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS queues which
> > > is used inside struct rte_eth_stats. Ideally, DPDK should be built with
> > > RTE_ETHDEV_QUEUE_STAT_CNTRS to the maximum number of queues a device
> > > can support, 65536, as uint16_t is used for keeping those values for
> > > RX and TX. But of course, having such big arrays inside struct
> > rte_eth_stats
> > > is not a good idea.
> >
> > RTE_ETHDEV_QUEUE_STAT_CNTRS come from a limitation in Intel devices.
> > They have limited number of registers to store the stats per queue.
> >
> > > Current default value is 16, which could likely be changed to 32 or 64
> > > without too much opposition. And maybe it would be a good idea to modify
> > > struct rte_eth_stats for allowing dynamically allocated arrays and maybe
> > > some extra fields for keeping the array sizes.
> >
> > Yes
> > and? what is your issue exactly? with which device?
> > Please explain the idea brought by your patch.
> >
> 
> Netronome NFP devices support 128 queues and future version will support
> 1024.
> 
> A particular VF, our PMD just supports VFs, could get as much as 128.
> Although that is not likely, that could be an option for some client.
> 
> Clients want to use a DPDK coming with a distribution, so changing the
> RTE_ETHDEV_QUEUE_STAT_CNTRS depending on the present devices is not an
> option.
> 
> We would be happy if RTE_ETHDEV_QUEUE_STAT_CNTRS could be set to 1024,
> covering current and future requirements for our cards, but maybe having
> such big arrays inside struct rte_eth_stats is something people do not want
> to have.
> 
> A solution could be to create such arrays dynamically based on the device
> to get the stats from. For example, call to rte_eth_dev_configure could
> have ax extra field for allocating a rte_eth_stats struct, which will be
> based on nb_rx_q and nb_tx_q params already given to that function.
> 
> Maybe the first thing to know is what people think about just incrementing
> RTE_ETHDEV_QUEUE_STAT_CNTRS to 1024.
> 
> So Thomas, what do you think about this?

I think this patch is doing something else :)

I'm not sure what is better between big arrays and variable size.
I think you must explain these 2 options in another thread,
because I'm not sure you will have enough attention in a thread starting with
"check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS".


[dpdk-dev] [PATCH] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS

2016-11-10 Thread Thomas Monjalon
2016-11-10 14:00, Alejandro Lucero:
> From: Bert van Leeuwen 
> 
> A device can have more than RTE_ETHDEV_QUEUE_STAT_CNTRS queues which
> is used inside struct rte_eth_stats. Ideally, DPDK should be built with
> RTE_ETHDEV_QUEUE_STAT_CNTRS to the maximum number of queues a device
> can support, 65536, as uint16_t is used for keeping those values for
> RX and TX. But of course, having such big arrays inside struct rte_eth_stats
> is not a good idea.

RTE_ETHDEV_QUEUE_STAT_CNTRS come from a limitation in Intel devices.
They have limited number of registers to store the stats per queue.

> Current default value is 16, which could likely be changed to 32 or 64
> without too much opposition. And maybe it would be a good idea to modify
> struct rte_eth_stats for allowing dynamically allocated arrays and maybe
> some extra fields for keeping the array sizes.

Yes
and? what is your issue exactly? with which device?
Please explain the idea brought by your patch.


[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

2016-11-10 Thread Thomas Monjalon
2016-11-10 13:50, Mori, Naoyuki:
> Hi,
> 
> >Thomas wrote:
> > > Just to make it sure, you mean returning an error in the driver when
> > > a configuration cannot be applied, right?
> >
> >Yes, as in 1bbcc5d21129 ("i40evf: report error for unsupported CRC
> >stripping config"), where -EINVAL is returned.
> >
> >Bj?rn
> 
> On my experience, OvS+DPDK has same issue.
> You cannot run OvS on i40evf due to this CRC config mismatch, while OvS on 
> ixgbevf works fine.
> So, changing on i40evf PMD side would have more benefit, I believe.

No I think OVS should handle the configuration error.
We could also add a function to query this capability before configuring.


[dpdk-dev] [PATCH] maintainers: add staging tree for network drivers

2016-11-10 Thread Thomas Monjalon
>  Networking Drivers
>  --
> +M: Ferruh Yigit 
> +T: git://dpdk.org/next/dpdk-next-net

Acked-by: Thomas Monjalon 

It will be applied at the beginning of 17.02 cycle to reflect the change.


[dpdk-dev] Clarification for eth_driver changes

2016-11-10 Thread Thomas Monjalon
Hi Stephen,

2016-11-10 02:51, Stephen Hemminger:
> I also think drv_flags should part of device not PCI. Most of the flags
> there like link state support are generic. If it isn't changed for this
> release will probably have to break ABI to fully support VMBUS

When do you plan to send VMBUS patches?
Could you send a deprecation notice for this change?
Are you aware of the work started by Shreyansh to have a generic bus model?
Could you help in 17.02 timeframe to have a solid bus model?

Thanks


[dpdk-dev] [PATCH v7 11/21] eal/soc: implement probing of drivers

2016-11-10 Thread Thomas Monjalon
2016-11-10 14:40, Shreyansh Jain:
> On Thursday 10 November 2016 01:11 PM, Jianbo Liu wrote:
> > On 10 November 2016 at 14:10, Shreyansh Jain  
> > wrote:
> >> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
> >>> I'm still not sure about the purpose of soc_scan, and how to use it.
> >>
> >>
> >> For each device to be used by DPDK, which cannot be scanned/identified 
> >> using
> >> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
> >> for driver to make those devices part of device lists.
> >>
> >> Ideally, 'scan' is not a function of a driver. It is a bus function - which
> >> is missing in this case.
> >>
> >>> If it's for each driver, it should at least struct rte_soc_driver * as
> >>> its parameter.
> >>
> >>
> >> Its for each driver - assuming that each non-PCI driver which implements it
> >> knows how to find devices which it can control (for example, special area 
> >> in
> >> sysfs, or even platform bus).
> >>
> >
> > Considering there are several drivers in a platform bus, each driver
> > call the scan function, like the rte_eal_soc_scan_platform_bus() you
> > implemented.
> > The first will add soc devices to the list, but the remaining calls
> > are redundant.
> 
> Indeed. This is exactly the issue we will face if we try and move this 
> scan/match logic to PCI - all devices are identified in one step.
> 
> There is a difference in principle here:
> A SoC device/driver combination is essentially focused towards a single 
> type of bus<->devices. For example, a NXP PMD would implement a scan 
> function which would scan for all devices on NXP's bus. This would not 
> conflict with another XYZ SoC PMD which scans its specific bus.
> 
> There is caveat to this - the platform bus. There can be multiple 
> drivers which can serve platform bus compliant devices. First 
> PMD->scan() initiated for such a bus/device would leave all other scans 
> redundant.
> 
> More similar caveats will come if we consider somewhat generic buses. At 
> least I couldn't find any interest for such devices in the ML when I 
> picked this series (from where Jan left it).
> 
> Probably when more common type of PMDs come in, some default scan 
> implementation can check for skipping those devices which are already 
> added. It would be redundant but harmless.

If several drivers use the same bus, it means the bus is standard enough
to be implemented in EAL. So the scan function of this bus should be
called only once when calling the generic EAL scan function.


[dpdk-dev] Clarification for eth_driver changes

2016-11-10 Thread Thomas Monjalon
2016-11-10 15:51, Jianbo Liu:
> On 10 November 2016 at 15:26, Shreyansh Jain  
> wrote:
> > This is what the current outline of eth_driver is:
> >
> > ++
> > | eth_driver |
> > | +-+|
> > | | rte_pci_driver  ||
> > | | +--+||
> > | | | rte_driver   |||
> > | | |  name[]  |||
> > | | |  ... |||
> > | | +--+||
> > | |  .probe ||
> > | |  .remove||
> > | |  ...||
> > | +-+|
> > |  .eth_dev_init |
> > |  .eth_dev_uninit   |
> > ++
> >
> > This is what I was thinking:
> >
> > +-++--+
> > | rte_pci_driver  ||eth_driver|
> > | +--+|   _|_struct rte_driver *p |
> > | | rte_driver   <---/ | .eth_dev_init|
> > | |  ... ||| .eth_dev_uninit  |
> > | |  name||+--+
> > | |||
> > | +--+|
> > |  |
> > +-+
> >
> > ::Impact::
> > Various drivers use the rte_pci_driver embedded in the eth_driver object for
> > device initialization.
> >  == They assume that rte_pci_driver is directly embedded and hence simply
> > dereference.
> >  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
> >
> > With the above change, such drivers would have to access rte_driver and then
> > perform container_of to obtain their respective rte_xxx_driver.
> >  == this would be useful in case there is a non-PCI driver
> >
> > ::Problem::
> > I am not sure of reason as to why eth_driver embedded rte_pci_driver in
> > first place - other than a convenient way to define it before PCI driver
> > registration.
> >
> > As all the existing PMDs are impacted - am I missing something here in
> > making the above change?
> >
> 
> How do you know eth_driver->p is pointing to a rte_pci_driver or 
> rte_soc_driver?
> Maybe you need to add a type/flag in rte_driver.

Why do you need any bus information at ethdev level?


[dpdk-dev] [PATCH] examples/l3fwd: force CRC stripping for i40evf

2016-11-10 Thread Thomas Monjalon
2016-11-10 07:17, Bj?rn T?pel:
> As discussed in the thread, it might be better to just change the
> default in l3fwd from .hw_strip_crc = 0 to 1.
> 
> I'll be looking into changing igbvf and ixgbevf to match the semantics
> of i40evf.

Just to make it sure, you mean returning an error in the driver when
a configuration cannot be applied, right?


[dpdk-dev] [PATCH] net/qede: fix unknown speed errmsg for 25G link

2016-11-10 Thread Thomas Monjalon
2016-11-09 18:26, Rasesh Mody:
> From: Harish Patil 
> 
> Fix to use bitmapped values in NVM configuration for speed capability
> advertisement. This issue is specific to 25G NIC since it is capable
> of 25G and 10G speeds.
> 
> Fixes: 64c239b7f8b7 ("net/qede: fix advertising link speed capability")
> 
> Signed-off-by: Harish Patil 

Now that the feature seems well implemented, I think you can add
it to the feature matrix (doc/guides/nics/features/qede.ini).


[dpdk-dev] Clarification for eth_driver changes

2016-11-10 Thread Thomas Monjalon
2016-11-10 14:12, Shreyansh Jain:
> On Thursday 10 November 2016 01:33 PM, Thomas Monjalon wrote:
> > 2016-11-10 15:51, Jianbo Liu:
> >> On 10 November 2016 at 15:26, Shreyansh Jain  
> >> wrote:
> >>> This is what the current outline of eth_driver is:
> >>>
> >>> ++
> >>> | eth_driver |
> >>> | +-+|
> >>> | | rte_pci_driver  ||
> >>> | | +--+||
> >>> | | | rte_driver   |||
> >>> | | |  name[]  |||
> >>> | | |  ... |||
> >>> | | +--+||
> >>> | |  .probe ||
> >>> | |  .remove||
> >>> | |  ...||
> >>> | +-+|
> >>> |  .eth_dev_init |
> >>> |  .eth_dev_uninit   |
> >>> ++
> >>>
> >>> This is what I was thinking:
> >>>
> >>> +-++--+
> >>> | rte_pci_driver  ||eth_driver|
> >>> | +--+|   _|_struct rte_driver *p |
> >>> | | rte_driver   <---/ | .eth_dev_init|
> >>> | |  ... ||| .eth_dev_uninit  |
> >>> | |  name||+--+
> >>> | |||
> >>> | +--+|
> >>> |  |
> >>> +-+
> >>>
> >>> ::Impact::
> >>> Various drivers use the rte_pci_driver embedded in the eth_driver object 
> >>> for
> >>> device initialization.
> >>>  == They assume that rte_pci_driver is directly embedded and hence simply
> >>> dereference.
> >>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
> >>>
> >>> With the above change, such drivers would have to access rte_driver and 
> >>> then
> >>> perform container_of to obtain their respective rte_xxx_driver.
> >>>  == this would be useful in case there is a non-PCI driver
> >>>
> >>> ::Problem::
> >>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
> >>> first place - other than a convenient way to define it before PCI driver
> >>> registration.
> >>>
> >>> As all the existing PMDs are impacted - am I missing something here in
> >>> making the above change?
> >>>
> >>
> >> How do you know eth_driver->p is pointing to a rte_pci_driver or 
> >> rte_soc_driver?
> >> Maybe you need to add a type/flag in rte_driver.
> >
> > Why do you need any bus information at ethdev level?
> 
> AFAIK, we don't need it. Above text is not stating anything on that 
> grounds either, I think. Isn't it?

No, I was replying to Jianbo.
Anyway, David made a more interesting comment.


[dpdk-dev] [PATCH] doc: postpone ABI changes for Tx prepare

2016-11-09 Thread Thomas Monjalon
The changes for the feature "Tx prepare" should be made in version 17.02.

Signed-off-by: Thomas Monjalon 
---
 doc/guides/rel_notes/deprecation.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 1a9e1ae..ab6014d 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -8,8 +8,8 @@ API and ABI deprecation notices are to be posted here.
 Deprecation Notices
 ---

-* In 16.11 ABI changes are planned: the ``rte_eth_dev`` structure will be
-  extended with new function pointer ``tx_pkt_prep`` allowing verification
+* In 17.02 ABI changes are planned: the ``rte_eth_dev`` structure will be
+  extended with new function pointer ``tx_pkt_prepare`` allowing verification
   and processing of packet burst to meet HW specific requirements before
   transmit. Also new fields will be added to the ``rte_eth_desc_lim`` 
structure:
   ``nb_seg_max`` and ``nb_mtu_seg_max`` providing information about number of
-- 
2.7.0



[dpdk-dev] [PATCH] doc: postpone ABI changes for mbuf

2016-11-09 Thread Thomas Monjalon
2016-11-09 17:12, Olivier Matz:
> Mbuf modifications are not ready for 16.11, postpone them to 17.02.
> 
> Signed-off-by: Olivier Matz 

Applied, thanks


[dpdk-dev] [PATCH] maintainers: claim responsability for xen

2016-11-09 Thread Thomas Monjalon
2016-11-07 07:38, Jianfeng Tan:
> As some users are still using xen as the hypervisor, I suggest to
> continue support for xen in DPDK. And from 16.11, I will be the
> maintainer of all xen-related files.
> 
> Signed-off-by: Jianfeng Tan 

Applied

Please Jianfeng, could you start your new role by sending a status
of Xen dom0 support in DPDK? I think there are some issues in latest releases.



  1   2   3   4   5   6   7   8   9   10   >