[dpdk-dev] DPDK-QoS- Using un-used bandwidth within a class

[Dumitrescu, Cristian]

Hi Sreenaath,

I think you are simply hitting the known and documented performance issue with 
using a single pipe. The hierarchical scheduler performance is optimized for 
many pipes (hundreds, thousands , ?), not for single pipe. The rationale is 
that if you only needs handful of queues, you can simply use a single level 
class based queuing device as opposed to the hierarchical scheduler.

Are you getting any issues when using hundreds/thousands of active (i.e. with 
traffic going through them) pipes?

Thanks,
Cristian

From: sreenaath vasudevan [mailto:sreenaat...@gmail.com]
Sent: Monday, October 24, 2016 8:05 AM
To: dev at dpdk.org; Dumitrescu, Cristian 
Subject: DPDK-QoS- Using un-used bandwidth within a class

Hi
I am using DPDK QoS and I find something strange. I am not sure if something is 
wrong with my config or my understanding of queue weights is wrong.
In my config, I am using only 1 port and 1 subport and 1 pipe. Within that 
pipe, I am using only the last class (C3). Port, subport and pipe are 
configured with 100Mbps speed.
C3 is given the entire pipe's TB rate i.e entire bandwidth in essence.
In C3, I am giving relative weights of 1:4:2:2 for the four queues q0,q1,q2,q3
When no other traffic is coming in to q0,q2,q3, I am pumping ~100Mbps in to q1. 
However, I am seeing only 40% of the traffic going through q1. In other words 
the max throughput allowed through the queue is based on its weight and the 
unused bandwidth is not used.
Cannot the unused bandwidth from q0,q2 and q3 be used for q1?

Note-
Following is the QoS config output spit out by DPDK in syslog



SCHED: Low level config for pipe profile 0:

token bucket: period = 10, credits per period = 1, size = 25000

Traffic classes: period = 125, credits per period = [0, 0, 0, 125000]

Traffic class 3 oversubscription: weight = 0

WRR cost: [1, 1, 1, 1], [1, 1, 1, 1], [1, 1, 1, 1], [4, 1, 2, 2]

SCHED: Low level config for subport 0:

Token bucket: period = 10, credits per period = 1, size = 25000

Traffic classes: period = 125, credits per period = [0, 0, 0, 125000]

Traffic class 3 oversubscription: wm min = 0, wm max = 0

--
regards
sreenaath

[dpdk-dev] [PATCH] examples/ip_pipeline: fix freeBSD build error

[Dumitrescu, Cristian]

> -Original Message-
> From: Singh, Jasvinder
> Sent: Monday, October 17, 2016 4:49 PM
> To: dev at dpdk.org
> Cc: Dumitrescu, Cristian 
> Subject: [PATCH] examples/ip_pipeline: fix freeBSD build error
> 
> Error log:
>  CC init.o
>  examples/ip_pipeline/init.c:38:22: fatal error: linux/if.h: No such file or
> directory
>  #include 
>   ^
> Fixes: 3f2c9f3bb6c6 ("examples/ip_pipeline: add TAP port")
> 
> Signed-off-by: Jasvinder Singh 
> ---

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] examples/ip_pipeline: fix gcc v6.2.1 build error

[Dumitrescu, Cristian]

> -Original Message-
> From: Singh, Jasvinder
> Sent: Monday, October 17, 2016 11:51 AM
> To: dev at dpdk.org
> Cc: Dumitrescu, Cristian 
> Subject: [PATCH] examples/ip_pipeline: fix gcc v6.2.1 build error
> 
> This patch fixes the misleading indentation error on compiling ip_pipeline
> app with gcc v6.2.1.
> 
> Fixes: 3f2c9f3bb6c6 ("examples/ip_pipeline: add TAP port")
> 
> Signed-off-by: Jasvinder Singh 
> ---
>  examples/ip_pipeline/app.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v2 1/3] lib/librte_port: enable file descriptor port support

[Dumitrescu, Cristian]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, October 12, 2016 9:33 PM
> To: Singh, Jasvinder 
> Cc: dev at dpdk.org; Dumitrescu, Cristian 
> Subject: Re: [dpdk-dev] [PATCH v2 1/3] lib/librte_port: enable file descriptor
> port support
> 
> 2016-09-04 15:38, Jasvinder Singh:
> > +#define RTE_PORT_FD_READER_STATS_PKTS_IN_ADD(port, val) \
> > +   do { port->stats.n_pkts_in += val } while (0)
> > +#define RTE_PORT_FD_READER_STATS_PKTS_DROP_ADD(port, val) \
> > +   do { port->stats.n_pkts_drop += val } while (0)
> > +
> 
> It does not compile because of missing ;
> 
> > + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> 
> The years seem outdated.

Will fix immediately.

> 
> This patchset was probably not tested as it does not compile.
> And it could be useless if a TAP PMD is integrated.
> I suggest to wait 17.02 cycle and see.

This patch was tested by me and Jasvinder as well and it works brilliantly.

We did not enable stats when testing, will sort out the missing semicolon issue 
in the stats macros and resend v3 asap. This is a trivial issue, no need to 
wait for 17.02.

This is not conflicting with TAP PMD, and as said the scope of this supersedes 
the TAP PMD.

[dpdk-dev] qos: traffic shaping at queue level

[Dumitrescu, Cristian]

From: Nikhil Jagtap [mailto:nikhil.jag...@gmail.com]
Sent: Wednesday, October 5, 2016 8:10 AM
To: Dumitrescu, Cristian 
Cc: dev at dpdk.org; users at dpdk.org
Subject: Re: qos: traffic shaping at queue level

Hi Cristian,

Thanks for the info. A few more comments/questions inline.

On 3 October 2016 at 23:42, Dumitrescu, Cristian mailto:cristian.dumitrescu at intel.com>> wrote:


From: Nikhil Jagtap [mailto:nikhil.jagtap at 
gmail.com<mailto:nikhil.jag...@gmail.com>]
Sent: Friday, September 30, 2016 7:12 AM
To: dev at dpdk.org<mailto:dev at dpdk.org>; Dumitrescu, Cristian 
mailto:cristian.dumitrescu at intel.com>>; 
users at dpdk.org<mailto:users at dpdk.org>
Subject: Re: qos: traffic shaping at queue level

Hi,
Can someone please answer my queries?
I tried using queue weights to distribute traffic-class bandwidth among the 
child queues, but did not get the desired results.
[Cristian] Can you please describe what issues you see?
[Nikhil] At the end of a 20 minute test, the total number of packets dequeued 
from the respective queues were not in the ratio 1:5.
In one other test where 4 equal-rate traffic-streams were hitting 4 different 
queues of the same TC configured with weights 1:2:4:8, I observed that the 
queue with highest weight had the least number of dequeued packets when in 
theory it should have been the one with highest packet count.

[Cristian] No idea why you get into this issue ? Please keep me posted once you 
find the root cause of your issue, maybe there is something that we can improve 
here.

Regards,
Nikhil

On 27 September 2016 at 15:34, Nikhil Jagtap mailto:nikhil.jagtap at gmail.com>> wrote:
Hi,

I have a few questions about the hierarchical scheduler. I am taking a simple 
example here to get a better understanding.

Reference example:
  pipe rate = 30 mbps
  tc 0 rate = 30 mbps
  traffic-type 0 being queued to queue 0, tc 0.
  traffic-type 1 being queued to queue 1, tc 0.
  Assume traffic-type 0 is being received at the rate of 25 mbps.
  Assume traffic-type 1 is also being received at the rate of 25 mbps.

Requirement:
  To limit traffic-type 0 to (CIR =  5 mbps, PIR = 30 mbps), AND
  limit traffic-type 1 to (CIR = 25 mbps, PIR = 30 mbps).

The questions:
1) I understand that with the scheduler, it is possible to do rate limiting 
only at the sub-port and pipe levels and not at the individual queue level.
[Cristian] Yes, correct, only subports and pipes own token buckets, with all 
the pipe traffic classes and queues sharing their pipe token bucket.

Is it possible to achieve rate limiting using the notion of queue weights? For 
the above example, will assigning weights in 1:5 ratio to the two queues help 
achieve shaping the two traffic-types at the two different rates?
[Cristian] Yes. However, getting the weight observed accurately relies on all 
the queues being backlogged (always having packets to dequeue). When a pipe and 
certain TC is examined for dequeuing, the relative weights are enforced between 
the queues that have packets at that precise moment in time, with the empty 
queues being ignored. The fully backlogged scenario is not taking place in 
practice, and the set of non-empty queues changes over time. As said it the 
past, having big relative weight ratios between queues helps (1:5 should be 
good).
[Nikhil] I see. So I guess not having fully backlogged queues could be one of 
the reasons for the observations I mentioned above where the weights-ratio does 
not directly translate into rate-ratio. I think I should also mention that 
there was no pipelining i.e. packet-processing, queueing, dequeing was all 
being done inline in a run-to-completion model.
a) Would having some kind of pipelining help achieve better rate-ratio? May be 
say atleast splitting the enqueue and dequeue operations?
b) If pipelining is not an option, what would be the recommended values for 
enqueue and dequeue packet count in the run-to-completion model? You have 
mentioned in one of your presentations to use different values for these two. 
If I go with (enqueue# > dequeue#), don't I run the risk of filling up the 
scheduler queues and failed enqueues even at rates lower than the scheduler 
pipe rates? In the other case where (dequeue# > enqueue#), we would end up 
dequeing all packets that were enqueued every time.

[Cristian]
a) In order to provide determinism for the hierarchical scheduler (e.g. 
frequent-enough calls of the enqueue and dequeue operations), I recommend 
dedicating a separate CPU core to run it, as opposed to running a lot of other 
stuff on the same core, which might result in the scheduler not being called 
regularly. This requires a pipeline of at least 2x CPU cores, i.e. one running 
your worker (run-to-completion) which feeds the second core running the 
scheduler.
b) As documented, for performance reasons, the API is not thread safe, so you 
need to run enqueue and dequeue of a given port on the same CPU core.

[dpdk-dev] [PATCH v2] examples: fix ip_pipeline to load PMD driver correctly

[Dumitrescu, Cristian]

> -Original Message-
> From: Gowrishankar [mailto:gowrishankar.m at linux.vnet.ibm.com]
> Sent: Tuesday, October 4, 2016 1:43 PM
> To: dev at dpdk.org
> Cc: Chao Zhu ; Thomas Monjalon
> ; Dumitrescu, Cristian
> ; Christian Ehrhardt
> ; Pradeep ;
> Gowrishankar Muthukrishnan 
> Subject: [PATCH v2] examples: fix ip_pipeline to load PMD driver correctly
> 
> From: Gowrishankar Muthukrishnan 
> 
> v2: minor correction in patch to avoid space between -d option and driver
> path
> 


Acked-by: Cristian Dumitrescu 

[dpdk-dev] qos: traffic shaping at queue level

[Dumitrescu, Cristian]

From: Nikhil Jagtap [mailto:nikhil.jag...@gmail.com]
Sent: Friday, September 30, 2016 7:12 AM
To: dev at dpdk.org; Dumitrescu, Cristian ; 
users at dpdk.org
Subject: Re: qos: traffic shaping at queue level

Hi,
Can someone please answer my queries?
I tried using queue weights to distribute traffic-class bandwidth among the 
child queues, but did not get the desired results.
[Cristian] Can you please describe what issues you see?

Regards,
Nikhil

On 27 September 2016 at 15:34, Nikhil Jagtap mailto:nikhil.jagtap at gmail.com>> wrote:
Hi,

I have a few questions about the hierarchical scheduler. I am taking a simple 
example here to get a better understanding.

Reference example:
  pipe rate = 30 mbps
  tc 0 rate = 30 mbps
  traffic-type 0 being queued to queue 0, tc 0.
  traffic-type 1 being queued to queue 1, tc 0.
  Assume traffic-type 0 is being received at the rate of 25 mbps.
  Assume traffic-type 1 is also being received at the rate of 25 mbps.

Requirement:
  To limit traffic-type 0 to (CIR =  5 mbps, PIR = 30 mbps), AND
  limit traffic-type 1 to (CIR = 25 mbps, PIR = 30 mbps).

The questions:
1) I understand that with the scheduler, it is possible to do rate limiting 
only at the sub-port and pipe levels and not at the individual queue level.
[Cristian] Yes, correct, only subports and pipes own token buckets, with all 
the pipe traffic classes and queues sharing their pipe token bucket.

Is it possible to achieve rate limiting using the notion of queue weights? For 
the above example, will assigning weights in 1:5 ratio to the two queues help 
achieve shaping the two traffic-types at the two different rates?
[Cristian] Yes. However, getting the weight observed accurately relies on all 
the queues being backlogged (always having packets to dequeue). When a pipe and 
certain TC is examined for dequeuing, the relative weights are enforced between 
the queues that have packets at that precise moment in time, with the empty 
queues being ignored. The fully backlogged scenario is not taking place in 
practice, and the set of non-empty queues changes over time. As said it the 
past, having big relative weight ratios between queues helps (1:5 should be 
good).

2) In continuation to previous question: if queue weights don't help, would it 
be possible to use metering to achieve rate limiting? Assume we meter 
individual traffic-types (using CIR-PIR config mentioned above) before queuing 
it to the scheduler queues. So to achieve the respective queue rates, the 
dequeuer would be expected to prioritise green packets over yellow.
Looking into the code, the packet color is used as an input to the dropper 
block, but does not seem to be used anywhere in the scheduler. So I guess it is 
not possible to prioritise green packets when dequeing?
[Cristian] Packet color is used by Weighted RED (WRED) congestion management 
scheme on the enqueue side, not on the dequeue side. Once the packet has been 
enqueued, it cannot be dropped (i.e. every enqueued packet will eventually be 
dequeued), so rate limiting cannot be enforced on the dequeue side.

Regards,
Nikhil

[dpdk-dev] [PATCH v2] sched: fix releasing enqueued packets

[Dumitrescu, Cristian]

> -Original Message-
> From: Hiroyuki Mikita [mailto:h.mikita89 at gmail.com]
> Sent: Monday, September 5, 2016 4:15 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org
> Subject: [PATCH v2] sched: fix releasing enqueued packets
> 
> rte_sched_port_free should release only enqueued packets of all queues.
> Previous behavior is that enqueued and already dequeued packets of
> only first 4 queues are released.
> 
> Fixes: 61383240 ("sched: release enqueued mbufs when freeing port")
> 
> Signed-off-by: Hiroyuki Mikita 
> ---
> v2:
> * use rte_sched_port_queues_per_port to get the number of queues
> * mask incremented qr by qsize - 1
> 
>  lib/librte_sched/rte_sched.c | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index 8696423..e6dace2 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c

Acked-by: Cristian Dumitrescu 

Thank you!

[dpdk-dev] [PATCH] app/test: fix failing packet-framework table unit-tests

[Dumitrescu, Cristian]

> -Original Message-
> From: Singh, Jasvinder
> Sent: Monday, September 12, 2016 12:06 PM
> To: dev at dpdk.org
> Cc: Dumitrescu, Cristian 
> Subject: [PATCH] app/test: fix failing packet-framework table unit-tests
> 
> The pipeline object is not freed when a particular test-case of the unit-test
> finishes. Using rte_pipeline_free() before returning the outcome for each
> test-case fixes the issue.
> 
> Fixes: 5205954791cb ("app/test: packet framework unit tests")
> 
> Signed-off-by: Jasvinder Singh 
> ---

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] app/test: decrease memory requirements for sched

[Dumitrescu, Cristian]

> -Original Message-
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> Sent: Monday, September 12, 2016 12:39 PM
> To: dev at dpdk.org; Dumitrescu, Cristian 
> Subject: [PATCH] app/test: decrease memory requirements for sched
> 
> The sched test consumes 35MB memory. When memory is too fragmented
> (with
> 2M hugepages), the test can fail.
> 
> To reduce this risk, decrease it to 4.5MB by modifying
> n_pipes_per_subport and qsize.
> 
> Signed-off-by: Olivier Matz 
> ---

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v2 1/3] lib/librte_port: enable file descriptor port support

[Dumitrescu, Cristian]

> -Original Message-
> From: Richardson, Bruce
> Sent: Monday, September 5, 2016 11:12 AM
> To: Singh, Jasvinder 
> Cc: dev at dpdk.org; Dumitrescu, Cristian 
> Subject: Re: [dpdk-dev] [PATCH v2 1/3] lib/librte_port: enable file descriptor
> port support
> 
> On Sun, Sep 04, 2016 at 03:38:39PM +0100, Jasvinder Singh wrote:
> > This patch adds File Descriptor(FD) port type (e.g. TAP port) to the
> > packet framework library that allows interface with the kernel network
> > stack. The FD port APIs are defined that allow port creation, writing
> > and reading packet from the kernel interface.
> >
> > Signed-off-by: Jasvinder Singh 
> > Acked-by: Cristian Dumitrescu 
> > ---
> > v2:
> > - fix checkpatch warnings
> 
> Rather than adding a special TAP port for use by packet framework alone,
> why not
> create a TAP PMD/ethdev and then it can be used both by regular DPDK
> apps, as well
> as by packet framework too - since packet framework already has an ethdev
> port
> type that presumably works with all ethdevs?
> 
> /Bruce

Great idea, but we don't have the bandwidth to create a TAP PMD right now. Any 
volunteers?

Please also note that the non-blocking file descriptor is actually 
significantly more generic/has broader applicability than just the TAP device, 
as it can be used to interface with any file descriptor, not just a TAP file 
descriptor, e.g. any Linux kernel character device or sockets (probably a small 
number of sockets), etc. I am sure it will prove itself useful to more people 
and we'll probably find even more places to use it going forward. 
Unfortunately, AFAIK this cannot be fitted as a PMD right now due to EAL 
limitations, as this would be virtual device and the file descriptor ID would 
have to be known before the DPDK application is started and passed to the app 
through the EAL vdev parameter, right?

This being said, I propose we go ahead with this new type of port. Whenever a 
TAP PMD becomes available, I don't mind changing the IP pipeline app to use it 
if people would prefer it. Our primary motivation for adding TAP support to IP 
pipeline app was to serve as a base of performance comparison between TAP and 
KNI when the same setup is used, but I am sure there are other potential usages 
for it.

[dpdk-dev] meter: excess token bucket update in srtcm

[Dumitrescu, Cristian]

Thanks, Nikhil, will review and get back to you in the next couple of weeks. 
Regards, Cristian

From: Nikhil Jagtap [mailto:nikhil.jag...@gmail.com]
Sent: Wednesday, September 7, 2016 7:22 AM
To: Dumitrescu, Cristian 
Cc: Ramia, Kannan Babu ; dev at dpdk.org
Subject: Re: [dpdk-dev] meter: excess token bucket update in srtcm

Hi Cristian,

Thanks for the confirmation. I have submitted a patch for the same.
http://dpdk.org/ml/archives/dev/2016-September/046226.html

Regards,
Nikhil

On 6 September 2016 at 15:26, Dumitrescu, Cristian mailto:cristian.dumitrescu at intel.com>> wrote:
Hi Nikhil,

It also looks to me that you are right. Sorry, my mistake when translating the 
RFC into code.

Challenge in fixing this is how to code it using minimal number of branches ...

Thanks,
Cristian

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org<mailto:dev-bounces at dpdk.org>] On 
> Behalf Of Nikhil Jagtap
> Sent: Tuesday, September 6, 2016 7:30 AM
> To: Ramia, Kannan Babu  intel.com<mailto:kannan.babu.ramia at intel.com>>
> Cc: dev at dpdk.org<mailto:dev at dpdk.org>
> Subject: Re: [dpdk-dev] meter: excess token bucket update in srtcm
>
> Hi Kannan,
>
> Thank you for your reply. I will submit the patch on similar lines I had
> used for my test.
>
> Regards,
> Nikhil
>
> On 6 September 2016 at 10:40, Ramia, Kannan Babu <
> kannan.babu.ramia at intel.com<mailto:kannan.babu.ramia at intel.com>> wrote:
>
> > Hi Nikhil
> >
> > You could submit a patch, something like that below logic
> >
> > If( ((n_periods * m->cir_bytes_per_period) > (m->cbs-m->tc))
> > te = m->te + ((n_periods * m->cir_bytes_per_period) -
> > (m->cbs-m->tc));
> >
> > and this should be done before m->tc update.
> >
> >
> > Regards
> > Kannan Babu
> >
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org<mailto:dev-bounces at dpdk.org>] 
> > On Behalf Of Nikhil Jagtap
> > Sent: Tuesday, September 6, 2016 9:43 AM
> > To: Dumitrescu, Cristian  > intel.com<mailto:cristian.dumitrescu at intel.com>>
> > Cc: dev at dpdk.org<mailto:dev at dpdk.org>
> > Subject: Re: [dpdk-dev] meter: excess token bucket update in srtcm
> >
> > Hi,
> > Can someone please comment on this?
> >
> > Nikhil
> >
> > On 31 August 2016 at 15:32, Nikhil Jagtap  > gmail.com<mailto:nikhil.jagtap at gmail.com>> wrote:
> >
> > > Hi,
> > >
> > > As per srTCM RFC 2697, we should be updating the E bucket only after
> > > the C bucket overflows.
> > > "Thereafter, the token counts Tc and Te are updated CIR times per
> > > second as follows:
> > >  o If Tc is less than CBS, Tc is incremented by one, else
> > >  o if Te is less then EBS, Te is incremented by one, else
> > >  o neither Tc nor Te is incremented."
> > >
> > > However in the current DPDK implementation of srTCM, we are updating
> > > both the buckets simultaneously at the same rate (CIR). This will
> > > result in a token accumulation rate of (2*CIR). This seems like a bug
> > > to me. Can you confirm this?
> > >
> > > Nikhil
> > >
> > >
> >

[dpdk-dev] [PATCH v7 0/9] enable lpm, acl and other missing libraries in ppc64le

[Dumitrescu, Cristian]

> 
> v7 changes:
> - removed enforcing cache alignment for table hash structs and
>   instead check only for multiples of 64 bytes.
> 

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v6 9/9] table: align rte table hash structs for cache line size

[Dumitrescu, Cristian]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, September 8, 2016 10:36 AM
> To: Gowrishankar Muthukrishnan 
> Cc: dev at dpdk.org; Dumitrescu, Cristian ;
> Chao Zhu ; Richardson, Bruce
> ; Ananyev, Konstantin
> ; Pradeep 
> Subject: Re: [dpdk-dev] [PATCH v6 9/9] table: align rte table hash structs for
> cache line size
> 
> 2016-08-31 17:29, Dumitrescu, Cristian:
> > From: Gowrishankar Muthukrishnan
> > > rte table hash structs rte_bucket_4_8, rte_bucket_4_16 and
> > > rte_bucket_4_32 have
> > > to be cache aligned as required by their corresponding hash create
> functions
> > > rte_table_hash_create_key8_lru etc.
> >
> > Hi Gowrishankar,
> >
> > My understanding is you are trying to work around the check invoked by
> the hash table create functions that verifies the size of the bucket header
> structure is a multiple of the cache line, right?
> >
> > Given that the size of this structure is 1x, 2x or 3x times 64 bytes, the 
> > check
> passes on IA CPUs (cache line of 64 bytes; explicit alignment to cache line 
> size
> is not needed in order to make the size of the structure a multiple of cache
> line), but not on PPC CPUs (cache line of 128 bytes), correct?
> >
> > I don't think your proposal provides the best way to fix this issue, since
> your code leads to a considerable increase in the memory consumption used
> per bucket in most cases:
> > - 100% more memory for 8-byte key hash table
> > - 0% more for 16-byte key hash table (code does not fix anything,
> explicit alignment is not needed)
> > - 50% more for 32-byte key hash table
> >
> > I suggest you simply change the check: instead of validating this data
> structure is a multiple of cache line size, validate it is a multiple of 64 
> bytes.
> 
> Any news please?
> The whole series is blocked for this patch.
> Should we expect a v7?

Yes, I think we should. Small fix for a considerable benefit.

[dpdk-dev] QoS: The difference of traffic class between subport and pipe in QoS

[Dumitrescu, Cristian]

Traffic class is really the type of traffic, e.g. voice, real-time video (like 
RTP), best effort (TCP-based video, file downloads, etc). Each traffic type has 
different requirements in terms on latency/delay, jitter/delay variation, loss 
rate, bandwidth, etc.

The levels of our scheduling hierarchy are: (1) port (output network 
interface), (2) subport (port slide, i.e. group of pipes/users), (3) pipe 
(user/subscriber), (4) traffic class (type of traffic), (5) packet queue. The 
rationale for having a TC limit at subport and pipe levels is to rate limit the 
about of that traffic type that the user (pipe level TC rate) or the group of 
users (subport level TC rate) are allowed to send. For example, you might want 
to restrict the amount of real-time video each user is sending to 1 Mbps, but 
also the amount to real-time video the group of e.g. 1000 users to 500Mbps; 
then packets might be restricted from being scheduled by either the user-level 
limit or the group-level limit, whichever is hit first.

Here is a Youtube video on DPDK QoS in case you're tired of reading the docs or 
the code: https://youtu.be/_PPklkWGugs

Regards,
Cristian

From: lvenyong at 1218.com.cn [mailto:lveny...@1218.com.cn]
Sent: Thursday, September 1, 2016 3:58 AM
To: Dumitrescu, Cristian ; dev at dpdk.org; 
users at dpdk.org
Subject: Re: RE: [dpdk-dev] QoS: The difference of traffic class between 
subport and pipe in QoS

Thanks for your answer!  But i haved not understand it.
what is the role of traffic class in subport, and the  relationship with the 
traffic class in pipe ?
The  traffic class in function of rte_sched_port_pkt_write is subport's or 
pipe's ?
void rte_sched_port_pkt_write(struct rte_mbuf *pkt,
   uint32_t subport, uint32_t pipe, uint32_t 
traffic_class,
   uint32_t queue, enum rte_meter_color color);


lvenyong at 1218.com.cn<mailto:lvenyong at 1218.com.cn>

From: Dumitrescu, Cristian<mailto:cristian.dumitre...@intel.com>
Date: 2016-09-01 01:44
To: lvenyong<mailto:lvenyong at 1218.com.cn>; dev at dpdk.org<mailto:dev at 
dpdk.org>
CC: users at dpdk.org<mailto:users at dpdk.org>
Subject: RE: [dpdk-dev] QoS: The difference of traffic class between subport 
and pipe in QoS


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of lvenyong
> Sent: Wednesday, August 31, 2016 12:34 PM
> To: dev at dpdk.org<mailto:dev at dpdk.org>
> Cc: users at dpdk.org<mailto:users at dpdk.org>
> Subject: [dpdk-dev] QoS: The difference of traffic class between subport and
> pipe in QoS
>
> HI !
>
> Is there difference of traffic class between subport and pipe in QOS ?
>
> After read prog_guide-2.2.pdf we kown that the scheduling hierarchy is port,
> subport, pipe, traffic class and queue. But the traffic class both in
> subport and pipe appeared in example of qos_sched .
>
> [subport 0]
> tb rate = 125000   ; Bytes per second
> tb size = 100  ; Bytes
> tc 0 rate = 125000 ; Bytes per second
> tc 1 rate = 125000 ; Bytes per second
> tc 2 rate = 125000 ; Bytes per second
> tc 3 rate = 125000 ; Bytes per second
> tc period = 10 ; Milliseconds
> pipe 0-4095 = 0; These pipes are configured with pipe
> profile 0
> ; Pipe configuration
> [pipe profile 0]
> tb rate = 305175   ; Bytes per second
> tb size = 100  ; Bytes
> tc 0 rate = 305175 ; Bytes per second
> tc 1 rate = 305175 ; Bytes per second
> tc 2 rate = 305175 ; Bytes per second
> tc 3 rate = 305175 ; Bytes per second
> tc period = 40 ; Milliseconds
>
> Thanks
>
>

There are 4x traffic classes. You can enforce a limit on the amount of traffic 
belonging to each traffic class at the subport level, as well as at the level 
of each pipe if you want.

[dpdk-dev] meter: excess token bucket update in srtcm

[Dumitrescu, Cristian]

Hi Nikhil,

It also looks to me that you are right. Sorry, my mistake when translating the 
RFC into code.

Challenge in fixing this is how to code it using minimal number of branches ...

Thanks,
Cristian

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Nikhil Jagtap
> Sent: Tuesday, September 6, 2016 7:30 AM
> To: Ramia, Kannan Babu 
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] meter: excess token bucket update in srtcm
> 
> Hi Kannan,
> 
> Thank you for your reply. I will submit the patch on similar lines I had
> used for my test.
> 
> Regards,
> Nikhil
> 
> On 6 September 2016 at 10:40, Ramia, Kannan Babu <
> kannan.babu.ramia at intel.com> wrote:
> 
> > Hi Nikhil
> >
> > You could submit a patch, something like that below logic
> >
> > If( ((n_periods * m->cir_bytes_per_period) > (m->cbs-m->tc))
> > te = m->te + ((n_periods * m->cir_bytes_per_period) -
> > (m->cbs-m->tc));
> >
> > and this should be done before m->tc update.
> >
> >
> > Regards
> > Kannan Babu
> >
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Nikhil Jagtap
> > Sent: Tuesday, September 6, 2016 9:43 AM
> > To: Dumitrescu, Cristian 
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] meter: excess token bucket update in srtcm
> >
> > Hi,
> > Can someone please comment on this?
> >
> > Nikhil
> >
> > On 31 August 2016 at 15:32, Nikhil Jagtap  
> > wrote:
> >
> > > Hi,
> > >
> > > As per srTCM RFC 2697, we should be updating the E bucket only after
> > > the C bucket overflows.
> > > "Thereafter, the token counts Tc and Te are updated CIR times per
> > > second as follows:
> > >  o If Tc is less than CBS, Tc is incremented by one, else
> > >  o if Te is less then EBS, Te is incremented by one, else
> > >  o neither Tc nor Te is incremented."
> > >
> > > However in the current DPDK implementation of srTCM, we are updating
> > > both the buckets simultaneously at the same rate (CIR). This will
> > > result in a token accumulation rate of (2*CIR). This seems like a bug
> > > to me. Can you confirm this?
> > >
> > > Nikhil
> > >
> > >
> >

[dpdk-dev] [PATCH v6 8/9] ip_pipeline: fix lcore mapping for varying SMT threads as in ppc64

[Dumitrescu, Cristian]

> -Original Message-
> From: Gowrishankar Muthukrishnan
> [mailto:gowrishankar.m at linux.vnet.ibm.com]
> Sent: Tuesday, August 16, 2016 11:28 AM
> To: dev at dpdk.org
> Cc: Chao Zhu ; Richardson, Bruce
> ; Ananyev, Konstantin
> ; Thomas Monjalon
> ; Dumitrescu, Cristian
> ; Pradeep 
> Subject: [PATCH v6 8/9] ip_pipeline: fix lcore mapping for varying SMT
> threads as in ppc64
> 
> This patch fixes ip_pipeline panic in app_init_core_map while preparing cpu
> core map in powerpc with SMT off. cpu_core_map_compute_linux currently
> prepares
> core mapping based on file existence in sysfs ie.
> 
> /sys/devices/system/cpu/cpu/topology/physical_package_id
>   /sys/devices/system/cpu/cpu/topology/core_id
> 
> These files do not exist for lcores which are offline for any reason (as in
> powerpc, while SMT is off). In this situation, this function should further
> continue preparing map for other online lcores instead of returning with -1
> for a first unavailable lcore.
> 
> Also, in SMT=off scenario for powerpc, lcore ids can not be always indexed
> from
> 0 upto 'number of cores present' (/sys/devices/system/cpu/present). For
> eg, for
> an online lcore 32, core_id returned in sysfs is 112 where online lcores are
> 10 (as in one configuration), hence sysfs lcore id can not be checked with
> indexing lcore number before positioning lcore map array.
> 
> Signed-off-by: Gowrishankar Muthukrishnan
> 
> ---
>  examples/ip_pipeline/cpu_core_map.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/examples/ip_pipeline/cpu_core_map.c
> b/examples/ip_pipeline/cpu_core_map.c
> index cb088b1..dd8f678 100644
> --- a/examples/ip_pipeline/cpu_core_map.c
> +++ b/examples/ip_pipeline/cpu_core_map.c
> @@ -351,8 +351,10 @@ cpu_core_map_compute_linux(struct
> cpu_core_map *map)
>   int lcore_socket_id =
> 
>   cpu_core_map_get_socket_id_linux(lcore_id);
> 
> +#if !defined(RTE_ARCH_PPC_64)
>   if (lcore_socket_id < 0)
>   return -1;
> +#endif
> 
>   if (((uint32_t) lcore_socket_id) == socket_id)
>   n_detected++;
> @@ -368,6 +370,7 @@ cpu_core_map_compute_linux(struct cpu_core_map
> *map)
>   cpu_core_map_get_socket_id_linux(
>   lcore_id);
> 
> +#if !defined(RTE_ARCH_PPC_64)
>   if (lcore_socket_id < 0)
>   return -1;
> 
> @@ -377,9 +380,14 @@ cpu_core_map_compute_linux(struct
> cpu_core_map *map)
> 
>   if (lcore_core_id < 0)
>   return -1;
> +#endif
> 
> +#if !defined(RTE_ARCH_PPC_64)
>   if (((uint32_t) lcore_socket_id == socket_id)
> &&
>   ((uint32_t) lcore_core_id ==
> core_id)) {
> +#else
> + if (((uint32_t) lcore_socket_id == socket_id))
> {
> +#endif
>   uint32_t pos =
> cpu_core_map_pos(map,
>   socket_id,
>   core_id_contig,
> --
> 1.9.1

This patch only changes the code for PPC CPUs, I don't have the hardware to 
check it myself, but I will take Gowrishankar's and Chao's word it is the right 
thing to do for PPC CPUs, so ...

Acked by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v6 9/9] table: align rte table hash structs for cache line size

[Dumitrescu, Cristian]

> -Original Message-
> From: Gowrishankar Muthukrishnan
> [mailto:gowrishankar.m at linux.vnet.ibm.com]
> Sent: Tuesday, August 16, 2016 11:28 AM
> To: dev at dpdk.org
> Cc: Chao Zhu ; Richardson, Bruce
> ; Ananyev, Konstantin
> ; Thomas Monjalon
> ; Dumitrescu, Cristian
> ; Pradeep 
> Subject: [PATCH v6 9/9] table: align rte table hash structs for cache line 
> size
> 
> rte table hash structs rte_bucket_4_8, rte_bucket_4_16 and
> rte_bucket_4_32 have
> to be cache aligned as required by their corresponding hash create functions
> rte_table_hash_create_key8_lru etc.
> 
> Signed-off-by: Gowrishankar Muthukrishnan
> 
> ---
>  lib/librte_table/rte_table_hash_key16.c | 4 ++--
>  lib/librte_table/rte_table_hash_key32.c | 4 ++--
>  lib/librte_table/rte_table_hash_key8.c  | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_table/rte_table_hash_key16.c
> b/lib/librte_table/rte_table_hash_key16.c
> index b7e000f..2102326 100644
> --- a/lib/librte_table/rte_table_hash_key16.c
> +++ b/lib/librte_table/rte_table_hash_key16.c
> @@ -68,10 +68,10 @@ struct rte_bucket_4_16 {
>   uint64_t next_valid;
> 
>   /* Cache line 1 */
> - uint64_t key[4][2];
> + uint64_t key[4][2] __rte_cache_aligned;
> 
>   /* Cache line 2 */
> - uint8_t data[0];
> + uint8_t data[0] __rte_cache_aligned;
>  };
> 
>  struct rte_table_hash {
> diff --git a/lib/librte_table/rte_table_hash_key32.c
> b/lib/librte_table/rte_table_hash_key32.c
> index a7aba49..619f63a 100644
> --- a/lib/librte_table/rte_table_hash_key32.c
> +++ b/lib/librte_table/rte_table_hash_key32.c
> @@ -68,10 +68,10 @@ struct rte_bucket_4_32 {
>   uint64_t next_valid;
> 
>   /* Cache lines 1 and 2 */
> - uint64_t key[4][4];
> + uint64_t key[4][4] __rte_cache_aligned;
> 
>   /* Cache line 3 */
> - uint8_t data[0];
> + uint8_t data[0] __rte_cache_aligned;
>  };
> 
>  struct rte_table_hash {
> diff --git a/lib/librte_table/rte_table_hash_key8.c
> b/lib/librte_table/rte_table_hash_key8.c
> index e2e2bdc..4d5e0cd 100644
> --- a/lib/librte_table/rte_table_hash_key8.c
> +++ b/lib/librte_table/rte_table_hash_key8.c
> @@ -68,7 +68,7 @@ struct rte_bucket_4_8 {
>   uint64_t key[4];
> 
>   /* Cache line 1 */
> - uint8_t data[0];
> + uint8_t data[0] __rte_cache_aligned;
>  };
> 
>  struct rte_table_hash {
> --
> 1.9.1

Hi Gowrishankar,

My understanding is you are trying to work around the check invoked by the hash 
table create functions that verifies the size of the bucket header structure is 
a multiple of the cache line, right?

Given that the size of this structure is 1x, 2x or 3x times 64 bytes, the check 
passes on IA CPUs (cache line of 64 bytes; explicit alignment to cache line 
size is not needed in order to make the size of the structure a multiple of 
cache line), but not on PPC CPUs (cache line of 128 bytes), correct?

I don't think your proposal provides the best way to fix this issue, since your 
code leads to a considerable increase in the memory consumption used per bucket 
in most cases:
- 100% more memory for 8-byte key hash table
- 0% more for 16-byte key hash table (code does not fix anything, 
explicit alignment is not needed)
- 50% more for 32-byte key hash table

I suggest you simply change the check: instead of validating this data 
structure is a multiple of cache line size, validate it is a multiple of 64 
bytes.

Regards,
Cristian

[dpdk-dev] [PATCH 2/2] examples/ip_pipeline: modify source port default parameter

[Dumitrescu, Cristian]

> -Original Message-
> From: Singh, Jasvinder
> Sent: Tuesday, August 9, 2016 9:31 AM
> To: dev at dpdk.org
> Cc: Dumitrescu, Cristian 
> Subject: [PATCH 2/2] examples/ip_pipeline: modify source port default
> parameter
> 
> The default value of ``file_name`` parameter of the source port structure is
> changed from ``NULL`` to ``./config/packets.pcap``.
> 
> Signed-off-by: Jasvinder Singh 
> ---
>  examples/ip_pipeline/app.h  | 4 ++--
>  examples/ip_pipeline/config_parse.c | 6 +-
>  2 files changed, 3 insertions(+), 7 deletions(-)
> 

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] qos_meter: Incorrect value check in example file.

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Sonnads,
> ShantkumarX
> Sent: Monday, August 8, 2016 7:53 AM
> To: 'dev at dpdk.org' 
> Subject: Re: [dpdk-dev] [PATCH] qos_meter: Incorrect value check in
> example file.
> 
> Sorry, Missed the attachment, resending email with attachment.
> 
> From: Sonnads, ShantkumarX
> Sent: Friday, August 05, 2016 4:21 PM
> To: dev at dpdk.org
> Subject: [PATCH] qos_meter: Incorrect value check in example file.
> 
> 
> Attached is patch for dpdk.org

Hi Sonnads,

If you have a patch, please send it using the rules documented on dpdk.org: 
http://www.dpdk.org/doc/guides/contributing/patches.html.

Regards,
Cristian

[dpdk-dev] how to design high performance QoS support for a large amount of subscribers

[Dumitrescu, Cristian]

Hi Yuyong,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Yuyong Zhang
> Sent: Tuesday, August 2, 2016 4:26 PM
> To: dev at dpdk.org; users at dpdk.org
> Subject: [dpdk-dev] how to design high performance QoS support for a large
> amount of subscribers
> 
> Hi,
> 
> I am trying to add QoS support for a high performance VNF with large
> amount of subscribers (millions).

Welcome to the world of DPDK QoS users!

It requires to support guaranteed bit rate
> for different service level of subscribers. I.e. four service levels need to 
> be
> supported:
> 
> * Diamond, 500M
> 
> * Gold, 100M
> 
> * Silver, 50M
> 
> * Bronze, 10M

Service levels translate to pipe profiles in our DPDK implementation. The set 
of pipe profiles is defined per port.

> 
> Here is the current pipeline design using DPDK:
> 
> 
> * 4 RX threads, does packet classification and load balancing
> 
> * 10-20 worker thread, does application subscriber management
> 
> * 4 TX threads, sends packets to TX NICs.
> 
> * Ring buffers used among RX threads, Worker threads, and TX threads
> 
> I read DPDK program guide for QoS framework regarding  hierarchical
> scheduler: Port, sub-port, pipe, TC and queues, I am looking for advice on
> how to design QoS scheduler to support millions of subscribers (pipes) which
> traffic are processed in tens of worker threads where subscriber
> management processing are handled?

Having millions of pipes per port poses some challenges:
1. Does it actually make sense? Assuming the port rate is 10GbE, looking at the 
smallest user rate you mention above (Bronze, 10Mbps/user), this means that 
fully provisioning all users (i.e. making sure you can fully handle each user 
in worst case scenario) results in a maximum of 1000 users per port. Assuming 
overprovisioning of 50:1, this means a maximum of 50K users per port.
2. Memory challenge. The number of pipes per port is configurable -- hey, this 
is SW! :) -- but each of these pipes has 16 queues. For 4K pipes per port, this 
is 64K queues per port; for typical value of 64 packets per queue, this is 4M 
packets per port, so worst case scenario we need to provision 4M packets in the 
buffer pool for each output port that has hierarchical scheduler enabled; for 
buffer size of ~2KB each, this means ~8GB of memory for each output port. If 
you go from 4k pipes per port to 4M pipes per port, this means 8TB of memory 
per port. Do you have enough memory in your system? :)

One thing to realize is that even for millions of users in your system, not all 
of them are active at the same time. So maybe have a smaller number of pipes 
and only map the active users (those that have any packets to send now) to them 
(a fraction of the total set of users), with the set of active users changing 
over time.

You can also consider mapping several users to the same pipe.

> 
> One design thought is as the following:
> 
> 8 ports (each one is associated with one physical port), 16-20 sub-ports (each
> is used by one Worker thread), each sub-port supports 250K pipes for
> subscribers. Each worker thread manages one sub-port and does metering
> for the sub-port to get color, and after identity subscriber flow pick a 
> unused
> pipe, and do sched enqueuer/de-queue and then put into TX rings to TX
> threads, and TX threads send the packets to TX NICs.
> 

In the current implementation, each port scheduler object has to be owned by a 
single thread, i.e. you cannot slit a port across multiple threads, therefore 
is not straightforward to have different sub-ports handled by different 
threads. The workaround is to split yourself the physical NIC port into 
multiple port scheduler objects: for example, create 8 port scheduler objects, 
set the rate of each to 1/8 of 10GbE, have each of them feed a different NIC TX 
queue of the same physical NIC port.

You can probably get this scenario (or very similar) up pretty quickly just by 
handcrafting yourself a configuration file for examples/ip_pipeline application.

> Are there functional and performance issues with above approach?
> 
> Any advice and input are appreciated.
> 
> Regards,
> 
> Yuyong
> 
> 
> 

Regards,
Cristian

[dpdk-dev] [PATCH] table: add missing exports

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Aleksey Katargin
> Sent: Monday, August 1, 2016 10:01 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] table: add missing exports
> 
> Signed-off-by: Aleksey Katargin 
> ---
>  lib/librte_table/rte_table_version.map | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/librte_table/rte_table_version.map
> b/lib/librte_table/rte_table_version.map
> index 2138698..459c2da 100644
> --- a/lib/librte_table/rte_table_version.map
> +++ b/lib/librte_table/rte_table_version.map
> @@ -3,6 +3,7 @@ DPDK_2.0 {
> 

Acked-by: Cristian Dumitrescu 


Hi Aleksey,

I checked all DPDK releases 2.0 -> 16.07 and yes, you're exactly right. Ouch!!!

Thanks very much for fixing this!

Regards,
Cristian

[dpdk-dev] [PATCH v2] examples: fix unusual-interpreter

[Dumitrescu, Cristian]

> -Original Message-
> From: Christian Ehrhardt [mailto:christian.ehrhardt at canonical.com]
> Sent: Tuesday, August 2, 2016 7:40 AM
> To: christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com;
> Dumitrescu, Cristian ; dev at dpdk.org
> Subject: [PATCH v2] examples: fix unusual-interpreter
> 
> *update in v2*
> - use #!/usr/bin/env python as usually recommended and suggested in the
>   discussion
> 
> Due to regular lintian checks in Debian packaging it surfaced that these
> two scripts had a space in their #! statement which renders it to be
> human, but not shell readable.
> 
> Fixes: 8673a3e8 ("examples/ip_pipeline: add config diagram generator")
> Fixes: fa667b46 ("examples/ip_pipeline: add core mappings script")
> 
> Signed-off-by: Christian Ehrhardt 
> ---
>  examples/ip_pipeline/config/diagram-generator.py| 2 +-
>  examples/ip_pipeline/config/pipeline-to-core-mapping.py | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] examples: fix unusual-interpreter

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Christian Ehrhardt
> Sent: Monday, August 1, 2016 1:29 PM
> To: christian.ehrhardt at canonical.com; dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] examples: fix unusual-interpreter
> 
> Due to regular lintian checks in Debian packaging it surfaced that these
> two scripts had a space in their #! statement which renders it to be
> human, but not shell readable.
> 
> Fixes: 8673a3e8 ("examples/ip_pipeline: add config diagram generator")
> Fixes: fa667b46 ("examples/ip_pipeline: add core mappings script")
> 
> Signed-off-by: Christian Ehrhardt 
> ---

Acked-by: Cristian Dumitrescu 

Christian and Thomas, 

Looking at this email thread, if there is an even better more robust solution, 
please suggest. It would not hurt to document it in the coding guidelines for 
scripts.

Thanks,
Cristian

[dpdk-dev] [PATCH v2] doc: announce ABI change of struct rte_port_source_params and rte_port_sink_params

[Dumitrescu, Cristian]

As Thomas mentioned, today is probably the last day to discuss ABI changes. 
This one is pretty small and straightforward, any issues with it?

Panu had a concern that the change from "char *" to "const char *" is too small 
to be regarded as ABI breakage and we should simply go ahead and do it. My 
conservative proposal was to put a notice anyway.

Nonetheless, what I would like to get from Thomas and Panu is a path forward 
for this now:
a) If we agree to consider this an ABI change, please merge the notice for 16.7;
b) If we agree this is too small for an ABI change, please let us agree now to 
accept our quick patch for 16.11 for this change.

Thanks,
Cristian


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Fan Zhang
> Sent: Thursday, May 19, 2016 3:19 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v2] doc: announce ABI change of struct
> rte_port_source_params and rte_port_sink_params
> 
> The ABI changes are planned for rte_port_source_params and
> rte_port_sink_params, which will be supported from release 16.11. Here
> announces that ABI changes in detail.
> 
> Signed-off-by: Fan Zhang 
> Acked-by: Cristian Dumitrescu 
> ---
>  doc/guides/rel_notes/deprecation.rst | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index fffe9c7..4f3fefe 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -74,3 +74,11 @@ Deprecation Notices
>a handle, like the way kernel exposes an fd to user for locating a
>specific file, and to keep all major structures internally, so that
>we are likely to be free from ABI violations in future.
> +
> +* ABI will change for rte_port_source_params struct. The member
> file_name
> +  data type will be changed from char * to const char *. This change targets
> +  release 16.11
> +
> +* ABI will change for rte_port_sink_params struct. The member file_name
> +  data type will be changed from char * to const char *. This change targets
> +  release 16.11
> --
> 2.5.5

[dpdk-dev] QoS config variables

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of sreenaath
> vasudevan
> Sent: Saturday, July 9, 2016 12:23 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] QoS config variables
> 
> Hi
> Can someone throw some light on the following DPDK QoS config variables?
> 
> A) In struct *rte_sched_subport_params* AND *rte_sched_pipe_params*
>1) tb_rate - Is the provisioned rate at which the TB can be filled while
> enqueuing OR the TB rate slice allotted to this subport ?
>2) tb_size
>3) tc_rate - Is tc_period the time slice given to each class within a
> subport?
>4) tc_period - Does tc_rate represent the bw rate provisioning for that
> class?
> 
> It would be nice if someone could throw the relationship between the above
> parameters and the QoS scheduling algorithm
> 
> Thanks !
> 
> --
> regards
> sreenaath

Hi Sreenaath,

Please check out the QoS documentation: 
http://www.dpdk.org/doc/guides/prog_guide/qos_framework.html. There is a 
section on traffic shaping and a section on traffic classes where there 
parameters are explained in detail. These parameters are also explained by the 
comments in file "rte_sched.h". Also this Youtube video might be useful to you: 
https://youtu.be/_PPklkWGugs

TB stands for Token Bucket, TC stands for Traffic Class. The rate parameters 
are specified in bytes per second. The traffic classes share the subport/pipe 
bandwidth and are scheduled in strict priority.

Regards,
Cristian

[dpdk-dev] [PATCH 3/3] port: remove duplicated symbols from .map

[Dumitrescu, Cristian]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 27, 2016 2:03 PM
> To: dev at dpdk.org
> Cc: Thomas Monjalon ; Olivier Matz
> ; Dumitrescu, Cristian
> 
> Subject: [PATCH 3/3] port: remove duplicated symbols from .map
> 
> Fixes: 9d41beed24b0 ("lib: provide initial versioning")
> Signed-off-by: Ferruh Yigit 
> ---
>  lib/librte_port/rte_port_version.map | 2 --
>  1 file changed, 2 deletions(-)
> 

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v2 3/7] pipeline: fix truncated dependency list

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Sunday, June 26, 2016 5:42 PM
> To: Panu Matilainen 
> Cc: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v2 3/7] pipeline: fix truncated dependency list
> 
> From: Panu Matilainen 
> 
> In other libraries, dependency list is always appended to, but
> in commit 6cbf4f75e059 it with an assignment. This causes the
> librte_eal dependency added in commit 6cbf4f75e059 to get discarded,
> resulting in missing dependency on librte_eal.
> 

Acked-by: Cristian Dumitrescu 


Hi Thomas,

As discussed in some other email thread, it would also make sense to replace 
the ':=' operator with '+=' operator in Makefile of rte_port and rte_table as 
well, do you want us to send a separate patch for this?

Thanks,
Cristian

[dpdk-dev] backtracing from within the code

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Catalin Vasile
> Sent: Friday, June 24, 2016 9:10 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] backtracing from within the code
> 
> Hi,
> 
> I'm trying to add a feature to?DPDK and I'm having a hard time printing a
> backtrace.
> I tried using this[1] functions for printing, but it does not print more than 
> one
> function. Maybe it lacks the symbols it needs.
> I tried compiling with "-rdynamic", but it breaks the compilation with an 
> error
> of "bad -rpath option".
> Can some help me out?
> 
> Catalin Vasile
> 
> [1]?https://www.gnu.org/software/libc/manual/html_node/Backtraces.html

Hi Catalin,

You can look at the implementation of function rte_panic(), which is used to 
dump the call stack and exit.

It eventually calls rte_dump_stack() in file 
lib/lirte_eal/linuxapp/eal/eal_debug.c, which calls backtrace(), which is 
probably what you are looking for. 

Regards,
Cristian

[dpdk-dev] [PATCH / RFC] sched: Correct subport calcuation

[Dumitrescu, Cristian]

> -Original Message-
> From: Simon Kagstrom [mailto:simon.kagstrom at netinsight.net]
> Sent: Friday, June 10, 2016 7:29 AM
> To: Dumitrescu, Cristian ;
> stephen at networkplumber.org; dev at dpdk.org;
> thomas.monjalon at 6wind.com
> Subject: [PATCH / RFC] sched: Correct subport calcuation
> 
> Signed-off-by: Simon Kagstrom 
> ---
> I'm a total newbie to the rte_sched design and implementation, so I've
> added the RFC.
> 
> We get crashes (at other places in the scheduler) without this code.
> 
>  lib/librte_sched/rte_sched.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index 1609ea8..b46ecfb 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -1869,7 +1869,7 @@ grinder_next_pipe(struct rte_sched_port *port,
> uint32_t pos)
> 
>   /* Install new pipe in the grinder */
>   grinder->pindex = pipe_qindex >> 4;
> - grinder->subport = port->subport + (grinder->pindex / port-
> >n_pipes_per_subport);
> + grinder->subport = port->subport + (grinder->pindex / port-
> >n_subports_per_port);
>   grinder->pipe = port->pipe + grinder->pindex;
>   grinder->pipe_params = NULL; /* to be set after the pipe structure is
> prefetched */
>   grinder->productive = 0;
> --
> 1.9.1

Hi Simon,

NACK.

Each port has an array of queues (size is port->n_queues_per_port), which are 
organized into equal size groups associated with pipes and subports:
- Each pipe is assigned the next group of RTE_SCHED_QUEUES_PER_PIPE (i.e. 16) 
queues in ascending order;
- Each subport is assigned the next group of port->n_pipes_per_subport pipes 
(congurable parameter).
The following is true:
n_queues_per_port = RTE_SCHED_QUEUES_PER_PIPE * n_pipes_per_subport * 
n_subports_per_port

Given a queue index (pipe_qindex), the following are true:
- Pipe index: pindex = pipe_qindex >> 4;
- Subport index (let's call it sindex): sindex = pindex / n_pipes_per_subport, 
right?

I don't know why you get crashes in your application, but what I do know is 
that your proposed method to compute sindex is not correct :)

Regards,
Cristian

[dpdk-dev] librte_meter compilation fails on IBM Power8

[Dumitrescu, Cristian]

> -Original Message-
> From: N?lio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
> Sent: Wednesday, June 22, 2016 1:31 PM
> To: Dumitrescu, Cristian ; Chao Zhu
> 
> Cc: dev at dpdk.org
> Subject: librte_meter compilation fails on IBM Power8
> 
> Hi Cristian, Chao,
> 
> I have encountered a compilation failure on IBM Power8 when compiling
> master branch with EXTRA_CFLAGS='-O0 -g':
> 
>   /root/nl/dpdk.org/build/lib/librte_meter.a(rte_meter.o): In function
> `rte_meter_get_tb_params':
>   /root/nl/dpdk.org/lib/librte_meter/rte_meter.c:57: undefined reference to
> `ceil'
> 
> Seems related to commit 43f4364d.
> 
> I don't have the time to search more deeply, I hope it can help.
> 
> Regards,
> 
> --
> N?lio Laranjeiro
> 6WIND

I am not sure what the problem might be for IBM Power8.

ceil() is a function defined in math library, we include math.h header file in 
rte_meter.c and we also link the library properly in the Makefile by using 
LDLIBS += -lm, therefore I do not see any issue in the library code.

Thanks,
Cristian

[dpdk-dev] [PATCH] port: fix build when KNI support is not enabled

[Dumitrescu, Cristian]

> -Original Message-
> From: Panu Matilainen [mailto:pmatilai at redhat.com]
> Sent: Wednesday, June 22, 2016 12:34 PM
> To: dev at dpdk.org
> Cc: Dumitrescu, Cristian ;
> zhuangwj at gmail.com
> Subject: [PATCH] port: fix build when KNI support is not enabled
> 
> Commit 9fc37d1c071c is missing a conditional in the dependencies,
> causing builds to fail when KNI is not enabled:
> == Build lib/librte_port
>   LD librte_port.so.3
> /usr/bin/ld: cannot find -lrte_kni
> collect2: error: ld returned 1 exit status
> 
> Fixes: 9fc37d1c071c ("port: support KNI")
> 
> Signed-off-by: Panu Matilainen 
> ---

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] ip_pipeline: fix parsing error in TM port section

[Dumitrescu, Cristian]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, June 21, 2016 9:49 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder 
> Subject: Re: [dpdk-dev] [PATCH] ip_pipeline: fix parsing error in TM port
> section
> 
> 2016-06-16 10:04, Jasvinder Singh:
> > Replace APP_PARAM_ADD_LINK_FOR_TXQ with
> APP_PARAM_ADD_LINK_FOR_TM macro
> > in TM (Traffic Manager) port section parsing function. This macro adds
> > nic ports entry specified in TM port section of the application
> > configuration file to the application parameters structure.
> >
> > Fixes: e5a1cd8a4847 ("examples/ip_pipeline: clean up configuration
> parser")
> >
> > Signed-off-by: Jasvinder Singh 
> > Acked-by: Cristian Dumitrescu 
> 
> Already fixed in 81d084dd2aa8 ("examples/ip_pipeline: support KNI")
> 
> Cristian, you should not ack a patch which do more than announced in
> the title. This pipeline KNI patch do a lot of things without any
> explanation for the fixes.
> 1 explanation = 1 patch

Yeah, there were several small issues to fix and several people spotted them at 
the same time. I agree we should do a better job at separating different fixes 
into different patches. Sorry about this.

[dpdk-dev] [PATCH v2] examples/ip_pipeline: fix build error for gcc 4.8

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrzyglod, DanielX T
> Sent: Tuesday, June 21, 2016 10:36 AM
> To: Singh, Jasvinder ; Dumitrescu, Cristian
> 
> Cc: dev at dpdk.org; Mrzyglod, DanielX T 
> Subject: [PATCH v2] examples/ip_pipeline: fix build error for gcc 4.8
> 
> This patch fixes a maybe-uninitialized warning when compiling DPDK with
> GCC 4.8
> 
> examples/ip_pipeline/pipeline/pipeline_common_fe.c: In function
> 'app_pipeline_track_pktq_out_to_link':
> examples/ip_pipeline/pipeline/pipeline_common_fe.c:66:31: error:
> 'reader' may be used uninitialized in this function [-Werror=maybe-
> uninitialized]
> 
>struct app_pktq_out_params *pktq_out =
> 
> Fixes: 760064838ec0 ("examples/ip_pipeline: link routing output ports to
> devices")
> 
> Signed-off-by: Daniel Mrzyglod 
> Acked-by: Cristian Dumitrescu 
> ---
>  examples/ip_pipeline/app.h | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
> index 7611341..242dae8 100644
> --- a/examples/ip_pipeline/app.h
> +++ b/examples/ip_pipeline/app.h
> @@ -667,11 +667,11 @@ app_swq_get_reader(struct app_params *app,
>   struct app_pktq_swq_params *swq,
>   uint32_t *pktq_in_id)
>  {
> - struct app_pipeline_params *reader;
> + struct app_pipeline_params *reader = NULL;
>   uint32_t pos = swq - app->swq_params;
>   uint32_t n_pipelines = RTE_MIN(app->n_pipelines,
>   RTE_DIM(app->pipeline_params));
> - uint32_t n_readers = 0, id, i;
> + uint32_t n_readers = 0, id = 0, i;
> 
>   for (i = 0; i < n_pipelines; i++) {
>   struct app_pipeline_params *p = >pipeline_params[i];
> @@ -727,11 +727,11 @@ app_tm_get_reader(struct app_params *app,
>   struct app_pktq_tm_params *tm,
>   uint32_t *pktq_in_id)
>  {
> - struct app_pipeline_params *reader;
> + struct app_pipeline_params *reader = NULL;
>   uint32_t pos = tm - app->tm_params;
>   uint32_t n_pipelines = RTE_MIN(app->n_pipelines,
>   RTE_DIM(app->pipeline_params));
> - uint32_t n_readers = 0, id, i;
> + uint32_t n_readers = 0, id = 0, i;
> 
>   for (i = 0; i < n_pipelines; i++) {
>   struct app_pipeline_params *p = >pipeline_params[i];
> --
> 2.5.5


No need for this patch, as these fixes have been already applied by Ethan's 
patch. 

Daniel, please check and let us know if otherwise.

[dpdk-dev] [PATCH v2] lib/table: fix wrong type of nht field

[Dumitrescu, Cristian]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, June 21, 2016 2:16 PM
> To: Dumitrescu, Cristian ; Kobylinski,
> MichalX 
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] lib/table: fix wrong type of nht field
> 
> > > Change type of nht field from uint32_t to uint8_t and increase max of
> > > next hops.
> > >
> > > Fixes: dc81ebbacaeb ("lpm: extend IPv4 next hop field")
> > > Signed-off-by: Michal Kobylinski 
> > > ---
> > > v2:
> > >  - removed changing from file: pipeline_routing_be.h
> > >  - changed macro: RTE_TABLE_LPM_MAX_NEXT_HOP
> >
> > Acked-by: Cristian Dumitrescu 
> 
> Cristian, the explanation is still missing in this commit.

Yes, you're right, sorry.

Michal K, please send updated version with the explanation and my ack included, 
thank you!

Regards,
Cristian

[dpdk-dev] [PATCH v4 1/4] port: kni interface support

[Dumitrescu, Cristian]

> -Original Message-
> From: Ethan Zhuang [mailto:zhuangwj at gmail.com]
> Sent: Tuesday, June 21, 2016 11:56 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ; Yigit,
> Ferruh ; WeiJie Zhuang 
> Subject: [PATCH v4 1/4] port: kni interface support
> 
> From: WeiJie Zhuang 
> 
> add KNI port type to the packet framework
> 
> Signed-off-by: WeiJie Zhuang 
> ---
> v2:
> * Fix check patch error.
> v3:
> * Fix code review comments.
> v4:
> * Reorganize patch sets and fix some comments
> ---

Series-acked-by: Cristian Dumitrescu 

I am acking patches 1 - 4 from v4 series on KNI support for Packet Framework 
and ip_pipeline app.

[dpdk-dev] [PATCH v3 1/3] port: add kni interface support

[Dumitrescu, Cristian]

Hi Ethan,

Thanks very much for sending the new version.

You are absolutely right about the param->parsed issue, sorry, my fault.

I think you need to use the --cover-letter flag for git format-patch command. 
You can practice by sending the patch set to your email address first before 
you send it to the list. Do you want to try sending a new revision of this 
patch set with the cover letter included (preferred) or you want to stop at v3? 
Please let us know.

Thanks,
Cristian

From: zhuangweijie at gmail.com [mailto:zhuangwei...@gmail.com] On Behalf Of 
Ethan
Sent: Tuesday, June 21, 2016 12:11 PM
To: Dumitrescu, Cristian 
Cc: dev at dpdk.org; Singh, Jasvinder ; Yigit, 
Ferruh 
Subject: Re: [PATCH v3 1/3] port: add kni interface support

Hi Cristian,

New patch has been submitted. All comments are fixed except this one:
"Here is one bug for you, you need to make sure you add the following line here:
param->parsed = 1;"
I think the new convention is to set this flag by the macro 
PARSE_CHECK_DUPLICATE_SECTION.

BTW, although I use the  --cover-letter and --annotate flags in the send-email 
command, it seems no cover letter is created.
I am not very familiar with this. So sorry!


B.R.
Ethan

2016-06-19 0:44 GMT+08:00 Dumitrescu, Cristian mailto:cristian.dumitrescu at intel.com>>:
Hi Ethan,

Thank you, here are some comments inlined below.

Please reorganize this patch in a slightly different way to look similar to 
other DPDK patch sets and also ease up the integration work for Thomas:
Patch 0: I suggest adding a cover letter;
Patch 1: all librte_port changes (rte_port_kni.h, rte_port_kni.c, 
Makefile, rte_port_version.map), including the "nodrop" KNI port version
Patch 2: all ip_pipeline app changes
Patch 3: ip_pipeline app kni.cfg file
Patch 4: Documentation changes

> -Original Message-
> From: WeiJie Zhuang [mailto:zhuangwj at gmail.com<mailto:zhuangwj at 
> gmail.com>]
> Sent: Thursday, June 16, 2016 12:27 PM
> To: Dumitrescu, Cristian  intel.com<mailto:cristian.dumitrescu at intel.com>>
> Cc: dev at dpdk.org<mailto:dev at dpdk.org>; Singh, Jasvinder 
> mailto:jasvinder.singh at intel.com>>; Yigit,
> Ferruh mailto:ferruh.yigit at intel.com>>; WeiJie 
> Zhuang mailto:zhuangwj at gmail.com>>
> Subject: [PATCH v3 1/3] port: add kni interface support
>
> 1. add KNI port type to the packet framework
> 2. add KNI support to the IP Pipeline sample Application
> 3. some bug fix
>
> Signed-off-by: WeiJie Zhuang mailto:zhuangwj at 
> gmail.com>>
> ---
> v2:
> * Fix check patch error.
> v3:
> * Fix code review comments.
> ---
>  doc/api/doxy-api-index.md<http://doxy-api-index.md>  
> |   1 +
>  examples/ip_pipeline/Makefile  |   2 +-
>  examples/ip_pipeline/app.h | 181 +++-
>  examples/ip_pipeline/config/kni.cfg|  67 +
>  examples/ip_pipeline/config_check.c|  26 +-
>  examples/ip_pipeline/config_parse.c| 166 ++-
>  examples/ip_pipeline/init.c| 132 -
>  examples/ip_pipeline/pipeline/pipeline_common_fe.c |  29 ++
>  examples/ip_pipeline/pipeline/pipeline_master_be.c |   6 +
>  examples/ip_pipeline/pipeline_be.h |  27 ++
>  lib/librte_port/Makefile   |   7 +
>  lib/librte_port/rte_port_kni.c | 325 
> +
>  lib/librte_port/rte_port_kni.h |  82 ++
>  lib/librte_port/rte_port_version.map   |   8 +
>  14 files changed, 1047 insertions(+), 12 deletions(-)
>  create mode 100644 examples/ip_pipeline/config/kni.cfg
>  create mode 100644 lib/librte_port/rte_port_kni.c
>  create mode 100644 lib/librte_port/rte_port_kni.h
>
> diff --git a/doc/api/doxy-api-index.md<http://doxy-api-index.md> 
> b/doc/api/doxy-api-index.md<http://doxy-api-index.md>
> index f626386..5e7f024 100644
> --- a/doc/api/doxy-api-index.md<http://doxy-api-index.md>
> +++ b/doc/api/doxy-api-index.md<http://doxy-api-index.md>
> @@ -118,6 +118,7 @@ There are many libraries, so their headers may be
> grouped by topics:
>  [frag] (@ref rte_port_frag.h),
>  [reass](@ref rte_port_ras.h),
>  [sched](@ref rte_port_sched.h),
> +[kni]  (@ref rte_port_kni.h),
>  [src/sink] (@ref rte_port_source_sink.h)
>* [table](@ref rte_table.h):
>  [lpm IPv4] (@ref rte_table_lpm.h),
> diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
> index 5827117..6dc3f52 100644
> --- a/examples/ip_pipeline/Makefile
> +++ b/examples

[dpdk-dev] [PATCH] lib/table: fix wrong type of nht field

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Michal Jastrzebski
> Sent: Monday, June 20, 2016 11:10 AM
> To: dev at dpdk.org
> Cc: Kobylinski, MichalX 
> Subject: [dpdk-dev] [PATCH] lib/table: fix wrong type of nht field
> 
> From: Michal Kobylinski 
> 
> Change type of nht field from uint32_t to uint8_t and increase max of
> next hops.
> 
> Fixes: dc81ebbacaeb ("lpm: extend IPv4 next hop field")
> 
> Signed-off-by: Michal Kobylinski 
> Acked-by: Cristian Dumitrescu 
> ---
>  examples/ip_pipeline/pipeline/pipeline_routing_be.h | 2 +-
>  lib/librte_table/rte_table_lpm.c| 8 
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/examples/ip_pipeline/pipeline/pipeline_routing_be.h
> b/examples/ip_pipeline/pipeline/pipeline_routing_be.h
> index 1276342..ea50896 100644
> --- a/examples/ip_pipeline/pipeline/pipeline_routing_be.h
> +++ b/examples/ip_pipeline/pipeline/pipeline_routing_be.h
> @@ -42,7 +42,7 @@
>   * Pipeline argument parsing
>   */
>  #ifndef PIPELINE_ROUTING_N_ROUTES_DEFAULT
> -#define PIPELINE_ROUTING_N_ROUTES_DEFAULT  4096
> +#define PIPELINE_ROUTING_N_ROUTES_DEFAULT  65536
>  #endif
> 

Changing the PIPELINE_ROUTING_N_ROUTES_DEFAULT  is actually not required, this 
is simply the default value which can be changed through the configuration 
file. Please remove this.

>  enum pipeline_routing_encap {
> diff --git a/lib/librte_table/rte_table_lpm.c
> b/lib/librte_table/rte_table_lpm.c
> index cdeb0f5..f2eaed5 100644
> --- a/lib/librte_table/rte_table_lpm.c
> +++ b/lib/librte_table/rte_table_lpm.c
> @@ -44,7 +44,7 @@
> 
>  #include "rte_table_lpm.h"
> 
> -#define RTE_TABLE_LPM_MAX_NEXT_HOPS256
> +#define RTE_TABLE_LPM_MAX_NEXT_HOPS65536
> 

With the next hop size of 24 bits, we can now make this configurable, so please 
use:

#ifndef RTE_TABLE_LPM_MAX_NEXT_HOPS
#define RTE_TABLE_LPM_MAX_NEXT_HOPS65536
#endif

>  #ifdef RTE_TABLE_STATS_COLLECT
> 
> @@ -74,7 +74,7 @@ struct rte_table_lpm {
> 
>   /* Next Hop Table (NHT) */
>   uint32_t nht_users[RTE_TABLE_LPM_MAX_NEXT_HOPS];
> - uint32_t nht[0] __rte_cache_aligned;
> + uint8_t nht[0] __rte_cache_aligned;
>  };
> 
>  static void *
> @@ -188,7 +188,7 @@ nht_find_existing(struct rte_table_lpm *lpm, void
> *entry, uint32_t *pos)
>   uint32_t i;
> 
>   for (i = 0; i < RTE_TABLE_LPM_MAX_NEXT_HOPS; i++) {
> - uint32_t *nht_entry = >nht[i * lpm->entry_size];
> + uint8_t *nht_entry = >nht[i * lpm->entry_size];
> 
>   if ((lpm->nht_users[i] > 0) && (memcmp(nht_entry, entry,
>   lpm->entry_unique_size) == 0)) {
> @@ -242,7 +242,7 @@ rte_table_lpm_entry_add(
> 
>   /* Find existing or free NHT entry */
>   if (nht_find_existing(lpm, entry, _pos) == 0) {
> - uint32_t *nht_entry;
> + uint8_t *nht_entry;
> 
>   if (nht_find_free(lpm, _pos) == 0) {
>   RTE_LOG(ERR, TABLE, "%s: NHT full\n", __func__);
> --
> 1.9.1

[dpdk-dev] [PATCH] lib/table: fix wrong type of nht field

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, June 20, 2016 11:14 AM
> To: Jastrzebski, MichalX K ; Kobylinski,
> MichalX 
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] lib/table: fix wrong type of nht field
> 
> 2016-06-20 12:10, Michal Jastrzebski:
> > From: Michal Kobylinski 
> >
> > Change type of nht field from uint32_t to uint8_t and increase max of
> > next hops.
> >
> > Fixes: dc81ebbacaeb ("lpm: extend IPv4 next hop field")
> 
> Why the type is wrong?

The lpm->nht is simply some raw memory allocated at the end of the table 
context using the usual pattern:
struct rte_table_lpm {
... 
uint8_t nht[0] __rte_cache_aligned;
}

Therefore, when we do: 
nht_entry = >nht[i * lpm->entry_size];
in several places, it makes big difference whether nht_entry is declared as 
uin8_t * (correct) or uint32_t * (incorrect), as the position computed by the 
latter is 4 times the position computed by the former ;(

Michal K and Michal J,
I just realized we still need to do a small change to this patch, I 
will reply to the original mail now. So Thomas, sorry, there is one small 
change, we'll send new version soon.

Thanks,
Cristian

[dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list truncation

[Dumitrescu, Cristian]

> -Original Message-
> From: Panu Matilainen [mailto:pmatilai at redhat.com]
> Sent: Tuesday, June 21, 2016 11:45 AM
> To: Richardson, Bruce 
> Cc: Dumitrescu, Cristian ; dev at dpdk.org;
> christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list
> truncation
> 
> On 06/21/2016 01:31 PM, Bruce Richardson wrote:
> > On Tue, Jun 21, 2016 at 01:25:52PM +0300, Panu Matilainen wrote:
> >> On 06/21/2016 01:01 PM, Dumitrescu, Cristian wrote:
> >>>
> >>>
> >>>> -Original Message-
> >>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Panu
> Matilainen
> >>>> Sent: Tuesday, June 21, 2016 9:12 AM
> >>>> To: dev at dpdk.org
> >>>> Cc: christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com
> >>>> Subject: [dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list
> >>>> truncation
> >>>>
> >>>> In other libraries, dependency list is always appended to, but
> >>>> in commit 6cbf4f75e059 it with an assignment. This causes the
> >>>> librte_eal dependency added in commit 6cbf4f75e059 to get discarded,
> >>>> resulting in missing dependency on librte_eal.
> >>>>
> >>>> Fixes: b3688bee81a8 ("pipeline: new packet framework logic")
> >>>> Fixes: 6cbf4f75e059 ("mk: fix missing internal dependencies")
> >>>>
> >>>> Signed-off-by: Panu Matilainen 
> >>>> ---
> >>>> lib/librte_pipeline/Makefile | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/lib/librte_pipeline/Makefile b/lib/librte_pipeline/Makefile
> >>>> index 95387aa..a8f3128 100644
> >>>> --- a/lib/librte_pipeline/Makefile
> >>>> +++ b/lib/librte_pipeline/Makefile
> >>>> @@ -53,7 +53,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_PIPELINE)-
> include +=
> >>>> rte_pipeline.h
> >>>>
> >>>> # this lib depends upon:
> >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_eal
> >>>> -DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) := lib/librte_table
> >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_table
> >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_port
> >>>>
> >>>> include $(RTE_SDK)/mk/rte.lib.mk
> >>>> --
> >>>> 2.5.5
> >>>
> >>>
> >>> In release 16.4, EAL was missing from the dependency list, now it is
> added. The librte_pipeline uses rte_malloc, therefore it depends on
> librte_eal being present.
> >>>
> >>> In the Makefile of the other Packet Framework libraries (librte_port,
> librte_table), it looks like the first dependency in the list is EAL, which 
> is listed
> with the assignment operator, followed by others that are listed with the
> append operator:
> >>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_XYZ) := lib/librte_eal
> >>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_XYZ) += lib/librte_some other lib
> >>>
> >>> Therefore, at least for cosmetic reasons, we should probably do the
> same in librte_pipeline, which requires changing both the librte_eal and the
> librte_table lines as below:
> >>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) := lib/librte_eal
> >>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_table
> >>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_port
> >>
> >> Ah, didn't notice those because the assignment is first of the
> dependencies.
> >>
> >>>
> >>> However, some other libraries e.g. librte_lpm simply add the EAL
> dependency using the append operator:
> >>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_LPM) += lib/librte_eal
> >>>
> >>> To be honest, I need to refresh my knowledge on make, I don't
> remember right now when we should use the assignment and when the
> append. Do we need to use the assign for first dependency (EAL) and
> append for others or should we use append everywhere?
> >>
> >> At least in automake, you need to assign before you can append. But in
> gmake
> >> this apparently is not the case, quoting from
> >>
> https://www.gnu.org/software/make/manual/html_node/Appending.html:
> >>
> >> "When the variable in question has not been defined before, ?+=? acts
> just
> >> like normal ?=?: it defines

[dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list truncation

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Panu Matilainen
> Sent: Tuesday, June 21, 2016 9:12 AM
> To: dev at dpdk.org
> Cc: christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com
> Subject: [dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list
> truncation
> 
> In other libraries, dependency list is always appended to, but
> in commit 6cbf4f75e059 it with an assignment. This causes the
> librte_eal dependency added in commit 6cbf4f75e059 to get discarded,
> resulting in missing dependency on librte_eal.
> 
> Fixes: b3688bee81a8 ("pipeline: new packet framework logic")
> Fixes: 6cbf4f75e059 ("mk: fix missing internal dependencies")
> 
> Signed-off-by: Panu Matilainen 
> ---
>  lib/librte_pipeline/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_pipeline/Makefile b/lib/librte_pipeline/Makefile
> index 95387aa..a8f3128 100644
> --- a/lib/librte_pipeline/Makefile
> +++ b/lib/librte_pipeline/Makefile
> @@ -53,7 +53,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_PIPELINE)-include +=
> rte_pipeline.h
> 
>  # this lib depends upon:
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_eal
> -DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) := lib/librte_table
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_table
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_port
> 
>  include $(RTE_SDK)/mk/rte.lib.mk
> --
> 2.5.5


In release 16.4, EAL was missing from the dependency list, now it is added. The 
librte_pipeline uses rte_malloc, therefore it depends on librte_eal being 
present.

In the Makefile of the other Packet Framework libraries (librte_port, 
librte_table), it looks like the first dependency in the list is EAL, which is 
listed with the assignment operator, followed by others that are listed with 
the append operator:
DEPDIRS-$(CONFIG_RTE_LIBRTE_XYZ) := lib/librte_eal
DEPDIRS-$(CONFIG_RTE_LIBRTE_XYZ) += lib/librte_some other lib

Therefore, at least for cosmetic reasons, we should probably do the same in 
librte_pipeline, which requires changing both the librte_eal and the 
librte_table lines as below:
DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) := lib/librte_eal
DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_table
DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_port

However, some other libraries e.g. librte_lpm simply add the EAL dependency 
using the append operator:
DEPDIRS-$(CONFIG_RTE_LIBRTE_LPM) += lib/librte_eal

To be honest, I need to refresh my knowledge on make, I don't remember right 
now when we should use the assignment and when the append. Do we need to use 
the assign for first dependency (EAL) and append for others or should we use 
append everywhere?

Thanks,
Cristian

[dpdk-dev] [PATCH / RFC] sched: Correct subport calcuation

[Dumitrescu, Cristian]

Hi Simon,

I am going to take a look at it this week and come back to you.

Thanks,
Cristian

> -Original Message-
> From: Simon K?gstr?m [mailto:simon.kagstrom at netinsight.net]
> Sent: Tuesday, June 21, 2016 7:41 AM
> To: Dumitrescu, Cristian ;
> stephen at networkplumber.org; dev at dpdk.org;
> thomas.monjalon at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH / RFC] sched: Correct subport calcuation
> 
> Hi again!
> 
> Any news about this patch? I'm off for parental leave starting next week
> (until january), so any comments (or simply dropping it!) would be good
> to have before that :-)
> 
> // Simon
> 
> On 2016-06-10 08:29, Simon Kagstrom wrote:
> > Signed-off-by: Simon Kagstrom 
> > ---
> > I'm a total newbie to the rte_sched design and implementation, so I've
> > added the RFC.
> >
> > We get crashes (at other places in the scheduler) without this code.
> >
> >  lib/librte_sched/rte_sched.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> > index 1609ea8..b46ecfb 100644
> > --- a/lib/librte_sched/rte_sched.c
> > +++ b/lib/librte_sched/rte_sched.c
> > @@ -1869,7 +1869,7 @@ grinder_next_pipe(struct rte_sched_port *port,
> uint32_t pos)
> >
> > /* Install new pipe in the grinder */
> > grinder->pindex = pipe_qindex >> 4;
> > -   grinder->subport = port->subport + (grinder->pindex / port-
> >n_pipes_per_subport);
> > +   grinder->subport = port->subport + (grinder->pindex / port-
> >n_subports_per_port);
> > grinder->pipe = port->pipe + grinder->pindex;
> > grinder->pipe_params = NULL; /* to be set after the pipe structure is
> prefetched */
> > grinder->productive = 0;
> >

[dpdk-dev] [PATCH v3 2/3] port: add kni nodrop writer

[Dumitrescu, Cristian]

> -Original Message-
> From: WeiJie Zhuang [mailto:zhuangwj at gmail.com]
> Sent: Thursday, June 16, 2016 12:27 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ; Yigit,
> Ferruh ; WeiJie Zhuang 
> Subject: [PATCH v3 2/3] port: add kni nodrop writer
> 
> 1. add no drop writing operations to the kni port
> 2. support dropless kni config in the ip pipeline sample application
> 
> Signed-off-by: WeiJie Zhuang 
> ---
>  examples/ip_pipeline/app.h   |   2 +
>  examples/ip_pipeline/config_parse.c  |  31 -
>  examples/ip_pipeline/init.c  |  26 -
>  examples/ip_pipeline/pipeline_be.h   |   6 +
>  lib/librte_port/rte_port_kni.c   | 220
> +++
>  lib/librte_port/rte_port_kni.h   |  13 +++
>  lib/librte_port/rte_port_version.map |   1 +
>  7 files changed, 292 insertions(+), 7 deletions(-)
> 
> diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
> index abbd6d4..6a6fdd9 100644
> --- a/examples/ip_pipeline/app.h
> +++ b/examples/ip_pipeline/app.h
> @@ -147,6 +147,8 @@ struct app_pktq_kni_params {
>   uint32_t mempool_id; /* Position in the app->mempool_params */
>   uint32_t burst_read;
>   uint32_t burst_write;
> + uint32_t dropless;
> + uint64_t n_retries;
>  };
> 
>  #ifndef APP_FILE_NAME_SIZE
> diff --git a/examples/ip_pipeline/config_parse.c
> b/examples/ip_pipeline/config_parse.c
> index c55be31..31a50c2 100644
> --- a/examples/ip_pipeline/config_parse.c
> +++ b/examples/ip_pipeline/config_parse.c
> @@ -199,6 +199,8 @@ struct app_pktq_kni_params default_kni_params = {
>   .mempool_id = 0,
>   .burst_read = 32,
>   .burst_write = 32,
> + .dropless = 0,
> + .n_retries = 0,
>  };
> 
>  struct app_pktq_source_params default_source_params = {
> @@ -1927,7 +1929,7 @@ parse_kni(struct app_params *app,
> 
>   if (strcmp(ent->name, "mempool") == 0) {
>   int status = validate_name(ent->value,
> -
> "MEMPOOL", 1);
> + "MEMPOOL", 1);
>   ssize_t idx;
> 
>   PARSE_ERROR((status == 0), section_name,
> @@ -1940,7 +1942,7 @@ parse_kni(struct app_params *app,
> 
>   if (strcmp(ent->name, "burst_read") == 0) {
>   int status = parser_read_uint32(
> >burst_read,
> -
>   ent->value);
> + ent->value);
> 
>   PARSE_ERROR((status == 0), section_name,
>   ent->name);
> @@ -1949,7 +1951,25 @@ parse_kni(struct app_params *app,
> 
>   if (strcmp(ent->name, "burst_write") == 0) {
>   int status = parser_read_uint32(
> >burst_write,
> -
>   ent->value);
> + ent->value);
> +
> + PARSE_ERROR((status == 0), section_name,
> + ent->name);
> + continue;
> + }
> +
> + if (strcmp(ent->name, "dropless") == 0) {
> + int status = parser_read_arg_bool(ent->value);
> +
> + PARSE_ERROR((status != -EINVAL), section_name,
> + ent->name);
> + param->dropless = status;
> + continue;
> + }
> +
> + if (strcmp(ent->name, "n_retries") == 0) {
> + int status = parser_read_uint64(>n_retries,
> + ent->value);
> 
>   PARSE_ERROR((status == 0), section_name,
>   ent->name);
> @@ -2794,6 +2814,11 @@ save_kni_params(struct app_params *app, FILE
> *f)
>   /* burst_write */
>   fprintf(f, "%s = %" PRIu32 "\n", "burst_write", p-
> >burst_write);
> 
> + /* dropless */
> + fprintf(f, "%s = %s\n",
> + "dropless",
> + p->dropless ? "yes" : "no");

Please also do not forget to save the number of retries parameter as well 
(struct app_pktq_kni_params::n_retries):

fprintf(f, "%s = %" PRIu64 "\n", "n_retries", p->n_retries);

I realize that we forgot to save n_retires in function save_txq_params(), can 
you please add it as p

[dpdk-dev] [PATCH v3 1/3] port: add kni interface support

[Dumitrescu, Cristian]

Hi Ethan,

Thank you, here are some comments inlined below.

Please reorganize this patch in a slightly different way to look similar to 
other DPDK patch sets and also ease up the integration work for Thomas:
Patch 0: I suggest adding a cover letter;
Patch 1: all librte_port changes (rte_port_kni.h, rte_port_kni.c, 
Makefile, rte_port_version.map), including the "nodrop" KNI port version
Patch 2: all ip_pipeline app changes
Patch 3: ip_pipeline app kni.cfg file
Patch 4: Documentation changes

> -Original Message-
> From: WeiJie Zhuang [mailto:zhuangwj at gmail.com]
> Sent: Thursday, June 16, 2016 12:27 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ; Yigit,
> Ferruh ; WeiJie Zhuang 
> Subject: [PATCH v3 1/3] port: add kni interface support
> 
> 1. add KNI port type to the packet framework
> 2. add KNI support to the IP Pipeline sample Application
> 3. some bug fix
> 
> Signed-off-by: WeiJie Zhuang 
> ---
> v2:
> * Fix check patch error.
> v3:
> * Fix code review comments.
> ---
>  doc/api/doxy-api-index.md  |   1 +
>  examples/ip_pipeline/Makefile  |   2 +-
>  examples/ip_pipeline/app.h | 181 +++-
>  examples/ip_pipeline/config/kni.cfg|  67 +
>  examples/ip_pipeline/config_check.c|  26 +-
>  examples/ip_pipeline/config_parse.c| 166 ++-
>  examples/ip_pipeline/init.c| 132 -
>  examples/ip_pipeline/pipeline/pipeline_common_fe.c |  29 ++
>  examples/ip_pipeline/pipeline/pipeline_master_be.c |   6 +
>  examples/ip_pipeline/pipeline_be.h |  27 ++
>  lib/librte_port/Makefile   |   7 +
>  lib/librte_port/rte_port_kni.c | 325 
> +
>  lib/librte_port/rte_port_kni.h |  82 ++
>  lib/librte_port/rte_port_version.map   |   8 +
>  14 files changed, 1047 insertions(+), 12 deletions(-)
>  create mode 100644 examples/ip_pipeline/config/kni.cfg
>  create mode 100644 lib/librte_port/rte_port_kni.c
>  create mode 100644 lib/librte_port/rte_port_kni.h
> 
> diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
> index f626386..5e7f024 100644
> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -118,6 +118,7 @@ There are many libraries, so their headers may be
> grouped by topics:
>  [frag] (@ref rte_port_frag.h),
>  [reass](@ref rte_port_ras.h),
>  [sched](@ref rte_port_sched.h),
> +[kni]  (@ref rte_port_kni.h),
>  [src/sink] (@ref rte_port_source_sink.h)
>* [table](@ref rte_table.h):
>  [lpm IPv4] (@ref rte_table_lpm.h),
> diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
> index 5827117..6dc3f52 100644
> --- a/examples/ip_pipeline/Makefile
> +++ b/examples/ip_pipeline/Makefile
> @@ -1,6 +1,6 @@
>  #   BSD LICENSE
>  #
> -#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> +#   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
>  #   All rights reserved.
>  #
>  #   Redistribution and use in source and binary forms, with or without
> diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
> index 7611341..abbd6d4 100644
> --- a/examples/ip_pipeline/app.h
> +++ b/examples/ip_pipeline/app.h
> @@ -44,6 +44,9 @@
>  #include 
> 
>  #include 
> +#ifdef RTE_LIBRTE_KNI
> +#include 
> +#endif
> 
>  #include "cpu_core_map.h"
>  #include "pipeline.h"
> @@ -132,6 +135,20 @@ struct app_pktq_swq_params {
>   uint32_t mempool_indirect_id;
>  };
> 
> +struct app_pktq_kni_params {
> + char *name;
> + uint32_t parsed;
> +
> + uint32_t socket_id;
> + uint32_t core_id;
> + uint32_t hyper_th_id;
> + uint32_t force_bind;
> +
> + uint32_t mempool_id; /* Position in the app->mempool_params */
> + uint32_t burst_read;
> + uint32_t burst_write;
> +};
> +
>  #ifndef APP_FILE_NAME_SIZE
>  #define APP_FILE_NAME_SIZE   256
>  #endif
> @@ -185,6 +202,7 @@ enum app_pktq_in_type {
>   APP_PKTQ_IN_HWQ,
>   APP_PKTQ_IN_SWQ,
>   APP_PKTQ_IN_TM,
> + APP_PKTQ_IN_KNI,
>   APP_PKTQ_IN_SOURCE,
>  };
> 
> @@ -197,6 +215,7 @@ enum app_pktq_out_type {
>   APP_PKTQ_OUT_HWQ,
>   APP_PKTQ_OUT_SWQ,
>   APP_PKTQ_OUT_TM,
> + APP_PKTQ_OUT_KNI,
>   APP_PKTQ_OUT_SINK,
>  };
> 
> @@ -420,6 +439,8 @@ struct app_eal_params {
> 
>  #defin

[dpdk-dev] random pkt generator PMD

[Dumitrescu, Cristian]

> -Original Message-
> From: Yerden Zhumabekov [mailto:e_zhumabekov at sts.kz]
> Sent: Wednesday, June 15, 2016 1:55 PM
> To: Dumitrescu, Cristian ; Panu Matilainen
> ; dev at dpdk.org
> Subject: Re: [dpdk-dev] random pkt generator PMD
> 
> 
> 
> On 15.06.2016 18:25, Dumitrescu, Cristian wrote:
> 
> >>>> So add a loop-mode to pcap pmd?
> >>> It would be nice to have an option like "...,rewind=1,...".
> >> As Cristian points out in
> >> http://dpdk.org/ml/archives/dev/2016-June/041589.html, the current
> pmd
> >> behavior of stopping is the odd man out in the pmd crowd.
> >>
> >> Rather than whether to rewind or not, I'd make the number of loops
> >> configurable, defaulting to forever and 1 being the equal to current
> >> behavior.
> >>
> >>- Panu -
> > +1
> 
> I'm afraid, all packets from pcap file would need to be preloaded to
> memory. Otherwise, each loop would infer pcap_open/pcap_close(), am I
> wrong?

This exactly what the code in source port is doing.

Basically, this is optimized for the case when number of packets in the PCAP 
file is relatively small, so the PCAP memory footprint when loaded into memory 
is small so it fits the L1/L2 cache. Provides traffic generation capability 
when performance measurements are not key: testing, code development on your 
laptop while on board of a plane, simulation environments, etc.

When the PCAP is large (e.g. capture of the traffic in your local cloud for 2 
mins), then PCAP memory gets swapped to disk and performance obviously drops. 
Still better than opening PCAC for each packet. Useful for e.g. IDS/IPS 
testing. 

[dpdk-dev] random pkt generator PMD

[Dumitrescu, Cristian]

> -Original Message-
> From: Panu Matilainen [mailto:pmatilai at redhat.com]
> Sent: Wednesday, June 15, 2016 1:24 PM
> To: Yerden Zhumabekov ; Dumitrescu, Cristian
> ; dev at dpdk.org
> Subject: Re: [dpdk-dev] random pkt generator PMD
> 
> On 06/15/2016 03:14 PM, Yerden Zhumabekov wrote:
> >
> >
> > On 15.06.2016 17:25, Panu Matilainen wrote:
> >> On 06/15/2016 02:10 PM, Yerden Zhumabekov wrote:
> >>>
> >>>
> >>> On 15.06.2016 16:43, Dumitrescu, Cristian wrote:
> >>>>
> >>>>>
> >>>>> Hello everybody,
> >>>>>
> >>>>> DPDK already got a number of PMDs for various eth devices, it even
> has
> >>>>> PMD emulations for backends such as pcap, sw rings etc.
> >>>>>
> >>>>> I've been thinking about the idea of having PMD which would
> generate
> >>>>> mbufs on the fly in some randomized fashion. This would serve goals
> >>>>> like, for example:
> >>>>>
> >>>>> 1) running tests for applications with network processing capabilities
> >>>>> without additional software packet generators;
> >>>>> 2) making performance measurements with no hw inteference;
> >>>>> 3) ability to run without root privileges, --no-pci, --no-huge, for CI
> >>>>> build, so on.
> >>>>>
> >>>>> Maybe there's no such need, and these goals may be achieved by
> other
> >>>>> means and this idea is flawed? Any thoughts?
> >>>> How about a Perl/Python script to generate a PCAP file with random
> >>>> packets and then feed the PCAP file to the PCAP PMD?
> >>>>
> >>>> Random can mean different requirements for different
> >>>> users/application, I think it is difficult to fit this  under a simple
> >>>> generic API. Customizing the script for different requirements if a
> >>>> far better option in my opinion.
> >>>
> >>> AFAIK, the thing about pcap pmd is that one needs to rewind pcap file
> >>> once pcap pmd reaches its end. It requires additional (non-generic)
> >>> handling in app code.
> >>
> >> So add a loop-mode to pcap pmd?
> >
> > It would be nice to have an option like "...,rewind=1,...".
> 
> As Cristian points out in
> http://dpdk.org/ml/archives/dev/2016-June/041589.html, the current pmd
> behavior of stopping is the odd man out in the pmd crowd.
> 
> Rather than whether to rewind or not, I'd make the number of loops
> configurable, defaulting to forever and 1 being the equal to current
> behavior.
> 
>   - Panu -

+1

[dpdk-dev] random pkt generator PMD

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Dumitrescu,
> Cristian
> Sent: Wednesday, June 15, 2016 12:26 PM
> To: Yerden Zhumabekov ; dev at dpdk.org
> Subject: Re: [dpdk-dev] random pkt generator PMD
> 
> 
> 
> > -Original Message-
> > From: Yerden Zhumabekov [mailto:e_zhumabekov at sts.kz]
> > Sent: Wednesday, June 15, 2016 12:11 PM
> > To: Dumitrescu, Cristian ; dev at dpdk.org
> > Subject: Re: [dpdk-dev] random pkt generator PMD
> >
> >
> >
> > On 15.06.2016 16:43, Dumitrescu, Cristian wrote:
> > >
> > >> -Original Message-
> > >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Yerden
> > >> Zhumabekov
> > >> Sent: Wednesday, June 15, 2016 10:44 AM
> > >> To: dev at dpdk.org
> > >> Subject: [dpdk-dev] random pkt generator PMD
> > >>
> > >> Hello everybody,
> > >>
> > >> DPDK already got a number of PMDs for various eth devices, it even has
> > >> PMD emulations for backends such as pcap, sw rings etc.
> > >>
> > >> I've been thinking about the idea of having PMD which would generate
> > >> mbufs on the fly in some randomized fashion. This would serve goals
> > >> like, for example:
> > >>
> > >> 1) running tests for applications with network processing capabilities
> > >> without additional software packet generators;
> > >> 2) making performance measurements with no hw inteference;
> > >> 3) ability to run without root privileges, --no-pci, --no-huge, for CI
> > >> build, so on.
> > >>
> > >> Maybe there's no such need, and these goals may be achieved by other
> > >> means and this idea is flawed? Any thoughts?
> > > How about a Perl/Python script to generate a PCAP file with random
> > packets and then feed the PCAP file to the PCAP PMD?
> > >
> > > Random can mean different requirements for different
> users/application, I
> > think it is difficult to fit this  under a simple generic API. Customizing 
> > the
> script
> > for different requirements if a far better option in my opinion.
> >
> > AFAIK, the thing about pcap pmd is that one needs to rewind pcap file
> > once pcap pmd reaches its end. It requires additional (non-generic)
> > handling in app code.
> 
> Yes, this is an obvious improvement that needs to happen to PCAP PMD.

Please note the PCAP file rewind code is already available as part of the 
source port in librte_port, it might be just straightforward to fit this code 
into the PCAP PMD.

Since the NICs generate packets forever, my recommendation is to have the 
loop/rewind mode as the default behavior for the reworked PCAP PMD, with 
potentially an option to disable the loop/rewind passed through dev args.

[dpdk-dev] random pkt generator PMD

[Dumitrescu, Cristian]

> -Original Message-
> From: Yerden Zhumabekov [mailto:e_zhumabekov at sts.kz]
> Sent: Wednesday, June 15, 2016 12:11 PM
> To: Dumitrescu, Cristian ; dev at dpdk.org
> Subject: Re: [dpdk-dev] random pkt generator PMD
> 
> 
> 
> On 15.06.2016 16:43, Dumitrescu, Cristian wrote:
> >
> >> -Original Message-
> >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Yerden
> >> Zhumabekov
> >> Sent: Wednesday, June 15, 2016 10:44 AM
> >> To: dev at dpdk.org
> >> Subject: [dpdk-dev] random pkt generator PMD
> >>
> >> Hello everybody,
> >>
> >> DPDK already got a number of PMDs for various eth devices, it even has
> >> PMD emulations for backends such as pcap, sw rings etc.
> >>
> >> I've been thinking about the idea of having PMD which would generate
> >> mbufs on the fly in some randomized fashion. This would serve goals
> >> like, for example:
> >>
> >> 1) running tests for applications with network processing capabilities
> >> without additional software packet generators;
> >> 2) making performance measurements with no hw inteference;
> >> 3) ability to run without root privileges, --no-pci, --no-huge, for CI
> >> build, so on.
> >>
> >> Maybe there's no such need, and these goals may be achieved by other
> >> means and this idea is flawed? Any thoughts?
> > How about a Perl/Python script to generate a PCAP file with random
> packets and then feed the PCAP file to the PCAP PMD?
> >
> > Random can mean different requirements for different users/application, I
> think it is difficult to fit this  under a simple generic API. Customizing 
> the script
> for different requirements if a far better option in my opinion.
> 
> AFAIK, the thing about pcap pmd is that one needs to rewind pcap file
> once pcap pmd reaches its end. It requires additional (non-generic)
> handling in app code.

Yes, this is an obvious improvement that needs to happen to PCAP PMD.

[dpdk-dev] random pkt generator PMD

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Yerden
> Zhumabekov
> Sent: Wednesday, June 15, 2016 10:44 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] random pkt generator PMD
> 
> Hello everybody,
> 
> DPDK already got a number of PMDs for various eth devices, it even has
> PMD emulations for backends such as pcap, sw rings etc.
> 
> I've been thinking about the idea of having PMD which would generate
> mbufs on the fly in some randomized fashion. This would serve goals
> like, for example:
> 
> 1) running tests for applications with network processing capabilities
> without additional software packet generators;
> 2) making performance measurements with no hw inteference;
> 3) ability to run without root privileges, --no-pci, --no-huge, for CI
> build, so on.
> 
> Maybe there's no such need, and these goals may be achieved by other
> means and this idea is flawed? Any thoughts?

How about a Perl/Python script to generate a PCAP file with random packets and 
then feed the PCAP file to the PCAP PMD?

Random can mean different requirements for different users/application, I think 
it is difficult to fit this  under a simple generic API. Customizing the script 
for different requirements if a far better option in my opinion.

Regards,
Cristian

[dpdk-dev] [PATCH] port: add kni interface support

[Dumitrescu, Cristian]

Hi Ethan,

Great, we?ll wait for your patch later this week then. I recommend you add any 
other changes that you might have on top of the latest code that I just send, 
as this will minimize your work, my work to further code reviews and number of 
future iterations to merge this patch.

Answers to your questions are inlined below.

Regards,
Cristian

From: zhuangweijie at gmail.com [mailto:zhuangwei...@gmail.com] On Behalf Of 
Ethan
Sent: Monday, June 13, 2016 11:48 AM
To: Dumitrescu, Cristian 
Cc: dev at dpdk.org; Singh, Jasvinder ; Yigit, 
Ferruh 
Subject: Re: [PATCH] port: add kni interface support

Hi Cristian,

I've got your comments. Thank you for review the code from a DPDK newbie. :-)
I plan to submit a new patch to fix all during this week hopefully.

There are four places I'd like to discuss further:

1. Dedicated lcore for kni kernel thread
First of all, it is a bug to add kni kernel core to the user space core mask. 
What I want is just to check if the kni kernel thread has a dedicated core.
The reason I prefer to allocate a dedicated core to kni kernel thread is that 
my application is latency sensitive. I worry the context switch and cache miss 
will cause the latency increasing if the kni kernel thread and application 
thread share one core.
Anyway, I think I should remove the hard coded check because it will be more 
generic. Users who has the similar usage like mine can achieve so through 
configuration file.

[Cristian] I agree with you that the user should be able to specify the core 
where the kernel thread should run, and this requirement is fully met by the 
latest code I sent, but implemented in a slightly different way, which I think 
it is a cleaner way.

In your initial solution, the application redefines the meaning of the core 
mask as the reunion of cores used by the user space application (cores running 
the pipelines) and the cores used to run the kernel space KNI threads. This 
does not make sense to me. The application is in user space and it does not 
start or manage any kernel threads itself, why should the application worry 
about the cores running kernel threads? The application should just pick up the 
user instructions from the config file and send them to the KNI kernel module 
transparently.

In the code that I just sent, the application preserves the current definition 
of the core mask, i.e. just the collection of cores running the pipelines. This 
leads to simpler code that meets all the requirements for kernel threads 
affinity:
i) The user wants to affinitize the kernel thread to a CPU core that is not 
used to run any pipeline (this core will run just KNI kernel threads): Core 
entry in KNI section is set to be different than the core entry of any PIPELINE 
section in the config file;
ii) The user affinitizes the kernel thread to a CPU core that also runs some of 
the pipelines (this core will run both user space and kernel space threads): 
Core entry in KNI section is equal to the core entry in one or several of the 
PIPELINE sections in the config file;
iii) The user does not affinitize the kernel thread to any CPU core, so the 
kernel decides the scheduling policy for the KNI threads: Core entry of the KNI 
section is not present; this results in force_bind KNI parameter to be set to 0.

Makes sense?

2. The compiler error of the Macro RTE_PORT_KNI_WRITER_STATS_PKTS_IN_ADD
Actually I implements the macro similar to 
RTE_PORT_RING_READER_STATS_PKTS_IN_ADD first. But the scripts/checkpatches.sh 
fails: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in 
parentheses
I'm not share either I have done something wrong or the checkpatches script 
need an update.

[Cristian] Let?s use the same consistent rule to create the stats macros for 
all the ports, i.e. follow the existing rule used for other ports. You can 
ignore this check patch issue.

3. KNI kernel operations callback
To be  honest, I made reference to the the KNI sample application.
Since there is very little docs tell the difference between link up call and 
device start call, I am not sure which one is better here.
Any help will be appreciate. :-)

[Cristian] I suggest you use the ones from the code that I just sent.

4. Shall I use DPDK_16.07 in the  librte_port/rte_port_version.map file?

[Cristian] Yes.


2016-06-10 7:42 GMT+08:00 Dumitrescu, Cristian mailto:cristian.dumitrescu at intel.com>>:
Hi Ethan,

Great work! There are still several comments below that need to be addressed, 
but I am confident we can close on them quickly. Thank you!

Please rebase the next version on top of the latest code on master branch.

Please also update librte_port/rte_port_version.map file.

Shall I use DPDK_16.07 in the  librte_port/rte_port_version.map file?


> -Original Message-
> From: WeiJie Zhuang [mailto:zhuangwj at gmail.com<mailto:zhuangwj at 
> gmail.com>]
> Sent: Saturday, May 28, 2016 12:26 PM
> To: Dumitrescu, Cristian  intel.com<mailto:cr

[dpdk-dev] [PATCH 1/1] ip_pipeline: fix null pointer dereference

[Dumitrescu, Cristian]

> -Original Message-
> From: Kerlin, MarcinX
> Sent: Monday, June 13, 2016 10:36 AM
> To: dev at dpdk.org
> Cc: Azarewicz, PiotrX T ; Dumitrescu, 
> Cristian
> ; Kerlin, MarcinX
> 
> Subject: [PATCH 1/1] ip_pipeline: fix null pointer dereference
> 
> Return value of function app_pipeline_type_find is not checking before
> dereference. Fix this problem by adding checking condition.
> 
> Coverity issue: 127196
> Fixes: b4aee0fb9c6d ("examples/ip_pipeline: reconfigure thread binding
> dynamically")
> 
> Signed-off-by: Marcin Kerlin 
> ---
>  examples/ip_pipeline/thread_fe.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/examples/ip_pipeline/thread_fe.c
> b/examples/ip_pipeline/thread_fe.c
> index d1b72b4..6c547ca 100644
> --- a/examples/ip_pipeline/thread_fe.c
> +++ b/examples/ip_pipeline/thread_fe.c
> @@ -81,6 +81,9 @@ app_pipeline_enable(struct app_params *app,
>   p_params = >pipeline_params[pipeline_id];
>   p_type = app_pipeline_type_find(app, p_params->type);
> 
> + if (p_type == NULL)
> + return -1;
> +
>   if (p->enabled == 1)
>   return -1;
> 
> --
> 1.9.1


Hi Marcin,

Checking p_type at this point is not required, as we already validated the 
pipeline settings (including the pipeline type) as part of the init code.

But since this code is harmless I am acking this patch just to avoid having the 
same discussion about this Coverity false positive issue again.

Regards,
Cristian

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] port: add kni interface support

[Dumitrescu, Cristian]

Hi Ethan,

In the process of testing your patch, I actually had to do the rebase on the 
latest code and fix all the comments that I previously sent as part of my 
reply. I also did a few other cosmetic changes and fixes required by the latest 
code (see below), therefore I think it makes sense to send you this code for 
quick review rather than have you spend time doing the same work. It would also 
save some iterations on the email list and speed up the integration of this 
patch.

Please review this code and let us know as soon as possible if you are OK with 
us sending it as the next version of this patch keeping your initial signoff as 
well. Thank you!

Changes included in the code below:
1. Rebase on top of latest DPDK head
2. Fixing all the previous code comments
3. Fix the binding of KNI device kernel space thread to CPU core (struct 
app_pktq_kni_params::force_bind)
4. The KNI requests handled by master pipeline rather than on data path 
(pipeline_master_be.c: call to rte_kni_handle_request())
5. Fixing the tracking feature for KNI (pipeline_common_fe.c: function 
app_pipeline_track_pktq_out_to_link())
6. Enhanced config file with two KNI interfaces connected using a Linux kernel 
bridge (./config/kni.cfg)
7. Cosmetic improvements

Regards,
Cristian


diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
old mode 100644
new mode 100755
index 848244a..555c6bd
--- a/examples/ip_pipeline/app.h
+++ b/examples/ip_pipeline/app.h
@@ -44,6 +44,7 @@
 #include 

 #include 
+#include 

 #include "cpu_core_map.h"
 #include "pipeline.h"
@@ -158,6 +159,20 @@ struct app_pktq_tm_params {
uint32_t burst_write;
 };

+struct app_pktq_kni_params {
+   char *name;
+   uint32_t parsed;
+
+   uint32_t socket_id;
+   uint32_t core_id;
+   uint32_t hyper_th_id;
+   uint32_t force_bind;
+
+   uint32_t mempool_id; /* Position in the app->mempool_params */
+   uint32_t burst_read;
+   uint32_t burst_write;
+};
+
 struct app_pktq_source_params {
char *name;
uint32_t parsed;
@@ -185,6 +200,7 @@ enum app_pktq_in_type {
APP_PKTQ_IN_HWQ,
APP_PKTQ_IN_SWQ,
APP_PKTQ_IN_TM,
+   APP_PKTQ_IN_KNI,
APP_PKTQ_IN_SOURCE,
 };

@@ -197,6 +213,7 @@ enum app_pktq_out_type {
APP_PKTQ_OUT_HWQ,
APP_PKTQ_OUT_SWQ,
APP_PKTQ_OUT_TM,
+   APP_PKTQ_OUT_KNI,
APP_PKTQ_OUT_SINK,
 };

@@ -420,6 +437,8 @@ struct app_eal_params {

 #define APP_MAX_PKTQ_TM  APP_MAX_LINKS

+#define APP_MAX_PKTQ_KNI APP_MAX_LINKS
+
 #ifndef APP_MAX_PKTQ_SOURCE
 #define APP_MAX_PKTQ_SOURCE  64
 #endif
@@ -471,6 +490,7 @@ struct app_params {
struct app_pktq_hwq_out_params hwq_out_params[APP_MAX_HWQ_OUT];
struct app_pktq_swq_params swq_params[APP_MAX_PKTQ_SWQ];
struct app_pktq_tm_params tm_params[APP_MAX_PKTQ_TM];
+   struct app_pktq_kni_params kni_params[APP_MAX_PKTQ_KNI];
struct app_pktq_source_params source_params[APP_MAX_PKTQ_SOURCE];
struct app_pktq_sink_params sink_params[APP_MAX_PKTQ_SINK];
struct app_msgq_params msgq_params[APP_MAX_MSGQ];
@@ -482,6 +502,7 @@ struct app_params {
uint32_t n_pktq_hwq_out;
uint32_t n_pktq_swq;
uint32_t n_pktq_tm;
+   uint32_t n_pktq_kni;
uint32_t n_pktq_source;
uint32_t n_pktq_sink;
uint32_t n_msgq;
@@ -495,6 +516,7 @@ struct app_params {
struct app_link_data link_data[APP_MAX_LINKS];
struct rte_ring *swq[APP_MAX_PKTQ_SWQ];
struct rte_sched_port *tm[APP_MAX_PKTQ_TM];
+   struct rte_kni *kni[APP_MAX_PKTQ_KNI];
struct rte_ring *msgq[APP_MAX_MSGQ];
struct pipeline_type pipeline_type[APP_MAX_PIPELINE_TYPES];
struct app_pipeline_data pipeline_data[APP_MAX_PIPELINES];
@@ -758,6 +780,66 @@ app_tm_get_reader(struct app_params *app,
 }

 static inline uint32_t
+app_kni_get_readers(struct app_params *app, struct app_pktq_kni_params *kni)
+{
+   uint32_t pos = kni - app->kni_params;
+   uint32_t n_pipelines = RTE_MIN(app->n_pipelines,
+   RTE_DIM(app->pipeline_params));
+   uint32_t n_readers = 0, i;
+
+   for (i = 0; i < n_pipelines; i++) {
+   struct app_pipeline_params *p = >pipeline_params[i];
+   uint32_t n_pktq_in = RTE_MIN(p->n_pktq_in, RTE_DIM(p->pktq_in));
+   uint32_t j;
+
+   for (j = 0; j < n_pktq_in; j++) {
+   struct app_pktq_in_params *pktq = >pktq_in[j];
+
+   if ((pktq->type == APP_PKTQ_IN_KNI) &&
+   (pktq->id == pos))
+   n_readers++;
+   }
+   }
+
+   return n_readers;
+}
+
+static inline struct app_pipeline_params *
+app_kni_get_reader(struct app_params *app,
+   struct app_pktq_kni_params *kni,
+   uint32_t *pktq_in_id)
+{
+   struct app_pipeline_params *reader = NULL;

[dpdk-dev] [PATCH] port: add kni interface support

[Dumitrescu, Cristian]

Hi Ethan,

Great work! There are still several comments below that need to be addressed, 
but I am confident we can close on them quickly. Thank you!

Please rebase the next version on top of the latest code on master branch.

Please also update librte_port/rte_port_version.map file.


> -Original Message-
> From: WeiJie Zhuang [mailto:zhuangwj at gmail.com]
> Sent: Saturday, May 28, 2016 12:26 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; WeiJie Zhuang 
> Subject: [PATCH] port: add kni interface support
> 
> 1. add KNI port type to the packet framework
> 2. add KNI support to the IP Pipeline sample Application
> 
> Signed-off-by: WeiJie Zhuang 
> ---
> v2:
> * Fix check patch error.
> ---
>  doc/api/doxy-api-index.md   |   1 +
>  examples/ip_pipeline/Makefile   |   6 +-
>  examples/ip_pipeline/app.h  |  74 +
>  examples/ip_pipeline/config/kni.cfg |  12 ++
>  examples/ip_pipeline/config_check.c |  34 
>  examples/ip_pipeline/config_parse.c | 130 +++
>  examples/ip_pipeline/init.c |  79 +
>  examples/ip_pipeline/kni/kni.c  |  80 +
>  examples/ip_pipeline/kni/kni.h  |  16 ++
>  examples/ip_pipeline/pipeline_be.h  |  13 ++
>  examples/ip_pipeline/thread.c   |   9 +
>  lib/librte_port/Makefile|   7 +
>  lib/librte_port/rte_port_kni.c  | 316
> 
>  lib/librte_port/rte_port_kni.h  |  81 +
>  14 files changed, 856 insertions(+), 2 deletions(-)
>  create mode 100644 examples/ip_pipeline/config/kni.cfg
>  create mode 100644 examples/ip_pipeline/kni/kni.c
>  create mode 100644 examples/ip_pipeline/kni/kni.h
>  create mode 100644 lib/librte_port/rte_port_kni.c
>  create mode 100644 lib/librte_port/rte_port_kni.h
> 
> diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
> index f626386..e38a959 100644
> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -119,6 +119,7 @@ There are many libraries, so their headers may be
> grouped by topics:
>  [reass](@ref rte_port_ras.h),
>  [sched](@ref rte_port_sched.h),
>  [src/sink] (@ref rte_port_source_sink.h)
> +[kni]  (@ref rte_port_kni.h)
>* [table](@ref rte_table.h):
>  [lpm IPv4] (@ref rte_table_lpm.h),
>  [lpm IPv6] (@ref rte_table_lpm_ipv6.h),
> diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
> index 10fe1ba..848c2aa 100644
> --- a/examples/ip_pipeline/Makefile
> +++ b/examples/ip_pipeline/Makefile
> @@ -43,9 +43,10 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  # binary name
>  APP = ip_pipeline
> 
> +VPATH += $(SRCDIR)/kni
>  VPATH += $(SRCDIR)/pipeline
> 
> -INC += $(wildcard *.h) $(wildcard pipeline/*.h)
> +INC += $(wildcard *.h) $(wildcard pipeline/*.h) $(wildcard kni/*.h)
> 
>  # all source are stored in SRCS-y
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) := main.c
> @@ -56,6 +57,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += init.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += thread.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += thread_fe.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += cpu_core_map.c
> +SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += kni.c
> 
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_common_be.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_common_fe.c
> @@ -72,7 +74,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) +=
> pipeline_flow_actions.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_routing_be.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_routing.c
> 
> -CFLAGS += -I$(SRCDIR) -I$(SRCDIR)/pipeline
> +CFLAGS += -I$(SRCDIR) -I$(SRCDIR)/pipeline -I$(SRCDIR)/kni
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS) -Wno-error=unused-function -Wno-
> error=unused-variable
> 

I would like to avoid creating the kni subfolder. Please move the functions 
from kni/kni.c to init.c file, just before function app_init_kni(), where they 
should be declared as static functions.

> diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
> index e775024..a86ce57 100644
> --- a/examples/ip_pipeline/app.h
> +++ b/examples/ip_pipeline/app.h
> @@ -44,7 +44,9 @@
>  #include 
> 
>  #include 
> +#include 
> 
> +#include "kni.h"
>  #include "cpu_core_map.h"
>  #include "pipeline.h"
> 
> @@ -99,6 +101,18 @@ struct app_pktq_hwq_out_params {
>   struct rte_eth_txconf conf;
>  };
> 
> +struct app_kni_params {
> + char *name;
> + uint32_t parsed;
> +
> + uint32_t socket_id;
> + uint32_t core_id;
> + uint32_t hyper_th_id;
> +
> + uint32_t mempool_id;

Please add the usual comment on t

[dpdk-dev] [PATCH] examples/ip_pipeline: fix build error for gcc 4.8

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrzyglod, DanielX T
> Sent: Thursday, June 9, 2016 12:39 PM
> To: Singh, Jasvinder ; Dumitrescu, Cristian
> 
> Cc: dev at dpdk.org; Mrzyglod, DanielX T 
> Subject: [PATCH] examples/ip_pipeline: fix build error for gcc 4.8
> 
> This patch fixes a maybe-uninitialized warning when compiling DPDK with
> GCC 4.8
> 

Acked-by: Cristian Dumitrescu 

[dpdk-dev] QoS grinder vs pipe wrr_tokens

[Dumitrescu, Cristian]

Hi Alexey,

The WRR context is compressed to use less memory footprint in order to fit the 
entire pipe run-time context (struct rte_sched_pipe) into a single cache line 
for performance reasons. Basically we trade WRR accuracy for performance.

For some typical Telco use-cases, the WRR/WFQ accuracy for the traffic class 
queues is not that important, as usually the traffic class queue weight ratio 
is big, e.g. 1:4:20. Basically, whether the actual observed ratio at run-time 
is 1:4:20 or 1:5:18 or 1:3:22 is not that important, as the intention really is 
to source most of the traffic from the queue with the largest weight, some 
traffic from the queue with the medium weight and not starve the lowest weight 
queue; this mode is very similar to strict priority between traffic class 
queues, with the exception that the lowest priority queues are not starved for 
long time.

When WRR accuracy is more important than performance, this operation should be 
disabled.

Regards,
Cristian


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Alexey
> Bogdanenko
> Sent: Tuesday, June 7, 2016 8:29 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] QoS grinder vs pipe wrr_tokens
> 
> Hello,
> 
> I have a question regarding QoS grinder implementation, specifically,
> about the way queue WRR tokens are copied from pipe to grinder and back.
> 
> First, rte_sched_grinder uses uint16_t and rte_sched_pipe uses uint8_t
> to represent wrr_tokens. Second, instead of just copying the tokens, we
> shift bits by RTE_SCHED_WRR_SHIFT.
> 
> What does it accomplish? Can it lead to lower scheduler accuracy due to
> a round-off error?
> 
> version: v16.04
> 
> Thanks,
> 
> Alexey Bogdanenko

[dpdk-dev] [PATCH] port: add kni interface support

[Dumitrescu, Cristian]

Hi Wei Jie,

Thank you for submitting this patch. I am currently travelling, I will review 
your patch and come back to you hopefully later this week or early next week.

Regards,
Cristian

> -Original Message-
> From: WeiJie Zhuang [mailto:zhuangwj at gmail.com]
> Sent: Saturday, May 28, 2016 12:26 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; WeiJie Zhuang 
> Subject: [PATCH] port: add kni interface support
> 
> 1. add KNI port type to the packet framework
> 2. add KNI support to the IP Pipeline sample Application
> 
> Signed-off-by: WeiJie Zhuang 
> ---
> v2:
> * Fix check patch error.
> ---
>  doc/api/doxy-api-index.md   |   1 +
>  examples/ip_pipeline/Makefile   |   6 +-
>  examples/ip_pipeline/app.h  |  74 +
>  examples/ip_pipeline/config/kni.cfg |  12 ++
>  examples/ip_pipeline/config_check.c |  34 
>  examples/ip_pipeline/config_parse.c | 130 +++
>  examples/ip_pipeline/init.c |  79 +
>  examples/ip_pipeline/kni/kni.c  |  80 +
>  examples/ip_pipeline/kni/kni.h  |  16 ++
>  examples/ip_pipeline/pipeline_be.h  |  13 ++
>  examples/ip_pipeline/thread.c   |   9 +
>  lib/librte_port/Makefile|   7 +
>  lib/librte_port/rte_port_kni.c  | 316
> 
>  lib/librte_port/rte_port_kni.h  |  81 +
>  14 files changed, 856 insertions(+), 2 deletions(-)
>  create mode 100644 examples/ip_pipeline/config/kni.cfg
>  create mode 100644 examples/ip_pipeline/kni/kni.c
>  create mode 100644 examples/ip_pipeline/kni/kni.h
>  create mode 100644 lib/librte_port/rte_port_kni.c
>  create mode 100644 lib/librte_port/rte_port_kni.h
> 
> diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
> index f626386..e38a959 100644
> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -119,6 +119,7 @@ There are many libraries, so their headers may be
> grouped by topics:
>  [reass](@ref rte_port_ras.h),
>  [sched](@ref rte_port_sched.h),
>  [src/sink] (@ref rte_port_source_sink.h)
> +[kni]  (@ref rte_port_kni.h)
>* [table](@ref rte_table.h):
>  [lpm IPv4] (@ref rte_table_lpm.h),
>  [lpm IPv6] (@ref rte_table_lpm_ipv6.h),
> diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
> index 10fe1ba..848c2aa 100644
> --- a/examples/ip_pipeline/Makefile
> +++ b/examples/ip_pipeline/Makefile
> @@ -43,9 +43,10 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  # binary name
>  APP = ip_pipeline
> 
> +VPATH += $(SRCDIR)/kni
>  VPATH += $(SRCDIR)/pipeline
> 
> -INC += $(wildcard *.h) $(wildcard pipeline/*.h)
> +INC += $(wildcard *.h) $(wildcard pipeline/*.h) $(wildcard kni/*.h)
> 
>  # all source are stored in SRCS-y
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) := main.c
> @@ -56,6 +57,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += init.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += thread.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += thread_fe.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += cpu_core_map.c
> +SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += kni.c
> 
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_common_be.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_common_fe.c
> @@ -72,7 +74,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) +=
> pipeline_flow_actions.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_routing_be.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_routing.c
> 
> -CFLAGS += -I$(SRCDIR) -I$(SRCDIR)/pipeline
> +CFLAGS += -I$(SRCDIR) -I$(SRCDIR)/pipeline -I$(SRCDIR)/kni
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS) -Wno-error=unused-function -Wno-
> error=unused-variable
> 
> diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
> index e775024..a86ce57 100644
> --- a/examples/ip_pipeline/app.h
> +++ b/examples/ip_pipeline/app.h
> @@ -44,7 +44,9 @@
>  #include 
> 
>  #include 
> +#include 
> 
> +#include "kni.h"
>  #include "cpu_core_map.h"
>  #include "pipeline.h"
> 
> @@ -99,6 +101,18 @@ struct app_pktq_hwq_out_params {
>   struct rte_eth_txconf conf;
>  };
> 
> +struct app_kni_params {
> + char *name;
> + uint32_t parsed;
> +
> + uint32_t socket_id;
> + uint32_t core_id;
> + uint32_t hyper_th_id;
> +
> + uint32_t mempool_id;
> + uint32_t burst;
> +};
> +
>  struct app_pktq_swq_params {
>   char *name;
>   uint32_t parsed;
> @@ -172,6 +186,7 @@ enum app_pktq_in_type {
>   APP_PKTQ_IN_SWQ,
>   APP_PKTQ_IN_TM,
>   APP_PKTQ_IN_SOURCE,
> + APP_PKTQ_IN_KNI,
>  };
> 
>  struct app_pktq_in_params 

[dpdk-dev] [PATCH] ip_pipeline: add command for multiple execution of run

[Dumitrescu, Cristian]

> -Original Message-
> From: Singh, Jasvinder
> Sent: Friday, May 6, 2016 8:09 PM
> To: dev at dpdk.org
> Cc: Dumitrescu, Cristian ; Chokkalingam,
> SankarX 
> Subject: [PATCH] ip_pipeline: add command for multiple execution of run
> 
> 
> From: Sankar Chokkalingam 
> 
> The new command enables the execution of script-file for 'n' number of
> times in regular intervals. It takes script-file, number of times to be
> executed, interval between each execution as inputs.
> 
> Syntax: run   
> 
> Usage: This command helps to collect statistics of ports and pipelines in
> periodic interval.
> 
> Example: run port_stats 5 30
> 
> The port_stats file may contain the list of port stats commands like
> p 1 port in 0 stats
> p 1 port out 0 stats
> p 2 port in 0 stats
> p 2 port out 0 stats
> p 2 port in 1 stats
> p 2 port out stats.
> 
> The list of commands in the file will be executed 5 times with the
> interval of 30 seconds.
> 
> Signed-off-by: Sankar Chokkalingam 
> ---
>  examples/ip_pipeline/pipeline/pipeline_common_fe.c | 50
> ++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/examples/ip_pipeline/pipeline/pipeline_common_fe.c
> b/examples/ip_pipeline/pipeline/pipeline_common_fe.c
> index a691d42..9f7ef08 100644
> --- a/examples/ip_pipeline/pipeline/pipeline_common_fe.c
> +++ b/examples/ip_pipeline/pipeline/pipeline_common_fe.c
> @@ -1267,9 +1267,59 @@ cmdline_parse_inst_t cmd_run = {
>   },
>  };
> 
> +struct cmd_multirun_file_result {
> + cmdline_fixed_string_t run_string;
> + char file_name[APP_FILE_NAME_SIZE];
> + uint32_t count;
> + uint32_t interval;
> +};
> +
> +static void
> +cmd_multirun_parsed(
> + void *parsed_result,
> + struct cmdline *cl,
> + __attribute__((unused)) void *data)
> +{
> + struct cmd_multirun_file_result *params = parsed_result;
> + uint32_t i;
> +
> + for (i = 0; i < params->count; i++) {
> + app_run_file(cl->ctx, params->file_name);
> + sleep(params->interval);
> + }
> +}
> +
> +cmdline_parse_token_string_t cmd_multirun_run_string =
> + TOKEN_STRING_INITIALIZER(struct cmd_multirun_file_result,
> run_string,
> + "run");
> +
> +cmdline_parse_token_string_t cmd_multirun_file_name =
> + TOKEN_STRING_INITIALIZER(struct cmd_multirun_file_result,
> file_name, NULL);
> +
> +static cmdline_parse_token_num_t cmd_multirun_count =
> + TOKEN_NUM_INITIALIZER(struct cmd_multirun_file_result, count,
> UINT32);
> +
> +static cmdline_parse_token_num_t cmd_multirun_interval =
> + TOKEN_NUM_INITIALIZER(struct cmd_multirun_file_result, interval,
> UINT32);
> +
> +cmdline_parse_inst_t cmd_multirun = {
> + .f = cmd_multirun_parsed,
> + .data = NULL,
> + .help_str = "Run CLI script file",
> + .tokens = {
> + (void *) _multirun_run_string,
> + (void *) _multirun_file_name,
> + (void *) _multirun_count,
> + (void *) _multirun_interval,
> + NULL,
> + },
> +};
> +
> +
>  static cmdline_parse_ctx_t pipeline_common_cmds[] = {
>   (cmdline_parse_inst_t *) _quit,
>   (cmdline_parse_inst_t *) _run,
> + (cmdline_parse_inst_t *) _multirun,
> 
>   (cmdline_parse_inst_t *) _link_config,
>   (cmdline_parse_inst_t *) _link_up,
> --
> 1.8.3.1

Hi Jasvinder and Sankar,

This patch is now  reworked and included into the ip_pipeline CLI rework patch 
set that Michal and Piotr sent: 
http://www.dpdk.org/ml/archives/dev/2016-May/039429.html

So we can safely discard this patch now.

Thanks,
Cristian

[dpdk-dev] fast red autotest

[Dumitrescu, Cristian]

Hi Thomas,

>From my side, I am OK to remove RED from the fast autotest, as long as it is 
>kept available as part of the normal/full autotest of DPDK.

Some of the RED autotests need a long time to run in order to train the history 
for the average queue size stochastic variable, therefore it is difficult to 
shorten them. However, some tests are quick to execute, so those tests can 
still be included into the fast autotest. Tomazs, any comments/proposal?

Regards,
Cristian

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, May 24, 2016 4:24 PM
> To: Dumitrescu, Cristian ; Kantecki, Tomasz
> 
> Cc: dev at dpdk.org
> Subject: Re: fast red autotest
> 
> Any news Tomasz, Cristian?
> 
> 2016-05-11 10:15, Dumitrescu, Cristian:
> > CC-ing Tomasz, who is the original author of RED implementation and its
> autotest. Tomasz, what do you think?
> >
> > > -Original Message-
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > Sent: Wednesday, May 11, 2016 8:45 AM
> > > To: Dumitrescu, Cristian 
> > > Cc: dev at dpdk.org
> > > Subject: fast red autotest
> > >
> > > The autotest for librte_sched red takes more than a minute.
> > > Would it be possible to reduce it to a second please?
> > > If it is really impossible, it must be removed from fast_test.
> 

[dpdk-dev] [ovs-dev] Traffic scheduling by qos_sched library in DPDK

[Dumitrescu, Cristian]

Hi Gayathri,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of
> gayathri.manepalli at wipro.com
> Sent: Friday, May 13, 2016 7:44 AM
> To: dev at dpdk.org
> Cc: Stokes, Ian 
> Subject: [dpdk-dev] [ovs-dev] Traffic scheduling by qos_sched library in
> DPDK
> 
> Hi Team,
> 
> I started working on implementing the QoS Shaping in OVS+DPDK by making
> use of rte_sched library provided in DPDK. 

Great news, thank you!

Meanwhile to compare the
> performance, started performance test with DPDK sample scheduling
> application. Below are the configuration details of system which I am using,
> 
> Server Type : Intel ATOM
> 
> Huge page configuration: (Each page of 2M size)
> 
> [root at ATOM qos_sched]# grep -i huge /proc/meminfo
> AnonHugePages: 90112 kB
> HugePages_Total:7168
> HugePages_Free: 7168
> HugePages_Rsvd:0
> HugePages_Surp:0
> Hugepagesize:   2048 kB
> 
> Port Capacity : 1G (All four ports)
> I am able to successfully run the qos_sched with three pfc's as below,
> 
> ./build/qos_sched -c 0x3e -n 1 --socket-mem 14336 -- --pfc "0,1,2,3" --pfc
> "1,0,3,2" --pfc "2,3,4,5" --cfg ./profile.cfg
> 
> Issue :
> 
> When I am trying to add one more profile configuration flow(4th one) , I am
> getting below error
> 
> Command : ./build/qos_sched -c 0x3e -n 1 --socket-mem 14336 -- --pfc
> "0,1,2,3" --pfc "1,0,3,2" --pfc "2,3,4,5" --pfc "3,2,5,4"  --cfg ./profile.cfg
> 
> Error:
> 
> done:  Link Up - speed 1000 Mbps - full-duplex
> SCHED: Low level config for pipe profile 0:
> Token bucket: period = 1, credits per period = 1, size = 10
> Traffic classes: period = 500, credits per period = [500, 
> 500,
> 500, 500]
>Traffic class 3 oversubscription: weight = 0
> WRR cost: [1, 1, 1, 1], [1, 1, 1, 1], [1, 1, 1, 1], [1, 1, 1, 1]
> SCHED: Low level config for subport 0:
> Token bucket: period = 1, credits per period = 1, size = 10
> Traffic classes: period = 125, credits per period = [125, 
> 125,
> 125, 125]
> Traffic class 3 oversubscription: wm min = 0, wm max = 0
> EAL: Error - exiting with code: 1
>   Cause: Cannot init mbuf pool for socket 3
> 
> Analysis:
> 
> I have analyzed DPDK source code to find the root cause. I could see that in
> qos_sched, memory allocation while creating each mbug pool
> (rte_mempool_create) for corresponding RX port is as below,
> 
> 
> MBUF_SIZE = (1528 + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
> 
> mp_size  =  2*1024*1024
> 
> 
> 
> From above I understood that, for each pfc/ rx port around 4G of huge pages
> are consumed. Whereas ATOM is capable of maximum 7168 huge pages of
> 2M which is 14336M in total. So I am not able to configure beyond three pfc's.
> But I would like to measure the performance with 4 port & 6 port scenario
> which requires 4-6 pfc's configured.
> 
> 
> 
> Is there any other alternative through which I can configure more number of
> pfc's with my system configuration provided above.
> 
> 

Yes, you are probably running out of memory.

QoS hierarchical scheduler can be seen like a big reservoir of packets. Each 
rte_sched_port object basically has thousands of packet queues internally 
(number of queues is configurable). Ideally, to protect against the worst case 
scenario, , the number of buffers provisioned per each output port that has the 
hierarchical scheduler enabled needs to be at least equal to: number of 
scheduler queues x queue size. For example, for 64K queues (4K pipes with 16 
queues each) of 64 packets each, this means 4M buffers per output port, which 
(assuming 2KB buffers in the mempool) leads to 8 GB of memory per output port. 
In our examples/qos_sched sample application we decided to go mid-way instead 
of worst case scenario, therefore we use 2M buffers per output port.

So possible workarounds for you to maximize the amount of ports with 
hierarchical scheduler support given a fixed amount memory:
1. Lower the amount of buffers you use per output port, e.g. from 2M to 1M and 
restest;
2. Use less pipes per output port (configurable), so number of scheduling 
queues is less, which reduces the pressure on mempool size.

Note that you can use distinct mempools per output port (like in 
examples/qos_sched application) or a single big global mempool shared by all 
ports; this is transparent for the librte_sched library.

Regards,
Cristian

> 
> Thanks & Regards,
> 
> Gayathri
> 
> The information contained in this electronic message and any attachments to
> this message are intended for the exclusive use of the addressee(s) and may
> contain proprietary, confidential or privileged information. If you are not 
> the
> intended recipient, you should not disseminate, distribute or copy this e-
> mail. Please notify the sender immediately and destroy all copies of this
> message and any attachments. WARNING: Computer viruses can be
> transmitted via email. The 

[dpdk-dev] [PATCH] sched: fix useless call

[Dumitrescu, Cristian]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, May 13, 2016 11:12 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Yigit, Ferruh ; Mrzyglod, 
> DanielX
> T 
> Subject: Re: [dpdk-dev] [PATCH] sched: fix useless call
> 
> 2016-05-11 10:46, Ferruh Yigit:
> > On 5/10/2016 6:18 PM, Dumitrescu, Cristian wrote:
> > > As previously discussed on this email list, the rte_bitmap_free() is an 
> > > API
> function that works as a placeholder for any resource freeing that needs to
> be done for the bitmap. The API function should not be removed and also
> the call to this function from the rte_sched_port_free() should not be
> removed either.
> > >
> >
> > Right now it isn't required and doesn't do anything.
> > Why not add this function when it is required?
> 
> I don't understand why we keep a function which does nothing.

Every data type/class/object should have a constructor/create and 
destructor/free function. This is standard programming practice, right?

This API function is the free function for the bitmap object. Right now there 
are no internally allocated resources to be freed, but as code evolves, some 
other internal memory could be allocated by the bitmap, which needs to be freed 
in the bitmap free function.

This function should be kept in order to have a stable API. We should not go 
back and forth with adding / removing API functions as code evolves. It does 
not make any sense to go through the ABI change process to remove this API 
function now just to come back later on and go again through ABI change to add 
back this API function later.

I think each DPDK object should have its create and free functions clearly 
identified in the API.

[dpdk-dev] [PATCH] examples/ip_pipline: fix memory initialization in firewall bulk functions

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrzyglod, DanielX T
> Sent: Friday, May 6, 2016 6:55 PM
> To: Kerlin, MarcinX ; Dumitrescu, Cristian
> ; Singh, Jasvinder
> 
> Cc: dev at dpdk.org; Mrzyglod, DanielX T 
> Subject: [PATCH] examples/ip_pipline: fix memory initialization in firewall
> bulk functions
> 
> bulk functions expect that all memory is set with zeros
> 
> Fixes: 67ebdbef0c31 ("examples/ip_pipeline: add bulk update of firewall
> rules")
> 
> Signed-off-by: Daniel Mrzyglod 
> ---
>  examples/ip_pipeline/pipeline/pipeline_firewall_be.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/ip_pipeline/pipeline/pipeline_firewall_be.c
> b/examples/ip_pipeline/pipeline/pipeline_firewall_be.c
> index e7a8a4c..4edca66 100644
> --- a/examples/ip_pipeline/pipeline/pipeline_firewall_be.c
> +++ b/examples/ip_pipeline/pipeline/pipeline_firewall_be.c
> @@ -732,7 +732,7 @@ pipeline_firewall_msg_req_add_bulk_handler(struct
> pipeline *p, void *msg)
>   n_keys = req->n_keys;
> 
>   for (i = 0; i < n_keys; i++) {
> - entries[i] = rte_malloc(NULL,
> + entries[i] = rte_zmalloc(NULL,
>   sizeof(struct firewall_table_entry),
>   RTE_CACHE_LINE_SIZE);
>   if (entries[i] == NULL) {
> @@ -740,7 +740,7 @@ pipeline_firewall_msg_req_add_bulk_handler(struct
> pipeline *p, void *msg)
>   return rsp;
>   }
> 
> - params[i] = rte_malloc(NULL,
> + params[i] = rte_zmalloc(NULL,
>   sizeof(struct
> rte_table_acl_rule_add_params),
>   RTE_CACHE_LINE_SIZE);
>   if (params[i] == NULL) {
> @@ -814,7 +814,7 @@ pipeline_firewall_msg_req_del_bulk_handler(struct
> pipeline *p, void *msg)
>   n_keys = req->n_keys;
> 
>   for (i = 0; i < n_keys; i++) {
> - params[i] = rte_malloc(NULL,
> + params[i] = rte_zmalloc(NULL,
>   sizeof(struct
> rte_table_acl_rule_delete_params),
>   RTE_CACHE_LINE_SIZE);
>   if (params[i] == NULL) {
> --
> 2.5.5


Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v4] examples/qos_meter: fix unchecked return value

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Friday, May 13, 2016 9:35 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ;
> Mrozowicz, SlawomirX 
> Subject: [PATCH v4] examples/qos_meter: fix unchecked return value
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30693: Unchecked return value
> check_return: Calling rte_meter_srtcm_config without checking return
> value.
> 
> Fixes: e6541fdec8b2 ("meter: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_meter/main.c | 16 
>  examples/qos_meter/main.h |  2 +-
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> index b968b00..1565615 100644
> --- a/examples/qos_meter/main.c
> +++ b/examples/qos_meter/main.c
> @@ -133,14 +133,20 @@ struct rte_meter_trtcm_params
> app_trtcm_params[] = {
> 
>  FLOW_METER app_flows[APP_FLOWS_MAX];
> 
> -static void
> +static int
>  app_configure_flow_table(void)
>  {
>   uint32_t i, j;
> + int ret;
> 
> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> RTE_DIM(PARAMS)){
> - FUNC_CONFIG(_flows[i], [j]);
> + for (i = 0, j = 0; i < APP_FLOWS_MAX;
> + i ++, j = (j + 1) % RTE_DIM(PARAMS)) {
> + ret = FUNC_CONFIG(_flows[i], [j]);
> + if (ret)
> + return ret;
>   }
> +
> + return 0;
>  }
> 
>  static inline void
> @@ -381,7 +387,9 @@ main(int argc, char **argv)
>   rte_eth_promiscuous_enable(port_tx);
> 
>   /* App configuration */
> - app_configure_flow_table();
> + ret = app_configure_flow_table();
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
> 
>   /* Launch per-lcore init on every lcore */
>   rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
> diff --git a/examples/qos_meter/main.h b/examples/qos_meter/main.h
> index 530bf69..54867dc 100644
> --- a/examples/qos_meter/main.h
> +++ b/examples/qos_meter/main.h
> @@ -51,7 +51,7 @@ enum policer_action
> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] =
>  #if APP_MODE == APP_MODE_FWD
> 
>  #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len,
> time=time
> -#define FUNC_CONFIG(a,b)
> +#define FUNC_CONFIG(a, b) 0
>  #define PARAMS   app_srtcm_params
>  #define FLOW_METER int
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value

[Dumitrescu, Cristian]

> -Original Message-
> From: Dumitrescu, Cristian
> Sent: Thursday, May 12, 2016 10:41 AM
> To: Mrozowicz, SlawomirX 
> Cc: dev at dpdk.org; Singh, Jasvinder 
> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> 
> 
> 
> > -Original Message-
> > From: Mrozowicz, SlawomirX
> > Sent: Thursday, May 12, 2016 8:06 AM
> > To: Dumitrescu, Cristian 
> > Cc: dev at dpdk.org; Singh, Jasvinder 
> > Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> >
> >
> >
> > >-Original Message-
> > >From: Dumitrescu, Cristian
> > >Sent: Wednesday, May 11, 2016 7:57 PM
> > >To: Mrozowicz, SlawomirX 
> > >Cc: dev at dpdk.org; Singh, Jasvinder 
> > >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> > >
> > >
> > >
> > >> -Original Message-
> > >> From: Mrozowicz, SlawomirX
> > >> Sent: Wednesday, May 11, 2016 10:15 AM
> > >> To: Dumitrescu, Cristian 
> > >> Cc: dev at dpdk.org; Singh, Jasvinder 
> > >> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return
> value
> > >>
> > >>
> > >>
> > >> >-Original Message-
> > >> >From: Dumitrescu, Cristian
> > >> >Sent: Tuesday, May 10, 2016 7:42 PM
> > >> >To: Mrozowicz, SlawomirX 
> > >> >Cc: dev at dpdk.org; Singh, Jasvinder 
> > >> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return
> > >> >value
> > >> >
> > >> >
> > >> >
> > >> >> -Original Message-
> > >> >> From: Mrozowicz, SlawomirX
> > >> >> Sent: Monday, May 9, 2016 9:38 AM
> > >> >> To: Dumitrescu, Cristian 
> > >> >> Cc: dev at dpdk.org; Singh, Jasvinder ;
> > >> >> Mrozowicz, SlawomirX 
> > >> >> Subject: [PATCH v3] examples/qos_meter: fix unchecked return
> value
> > >> >>
> > >> >> Fix issue reported by Coverity.
> > >> >>
> > >> >> Coverity ID 30693: Unchecked return value
> > >> >> check_return: Calling rte_meter_srtcm_config without checking
> > >> >> return value.
> > >> >>
> > >> >> Fixes: e6541fdec8b2 ("meter: initial import")
> > >> >>
> > >> >> Signed-off-by: Slawomir Mrozowicz
> > 
> > >> >> ---
> > >> >>  examples/qos_meter/main.c | 15 ++-
> > >> >> examples/qos_meter/main.h |  2 +-
> > >> >>  2 files changed, 11 insertions(+), 6 deletions(-)
> > >> >>
> > >> >> diff --git a/examples/qos_meter/main.c
> > b/examples/qos_meter/main.c
> > >> >> index b968b00..7c69606 100644
> > >> >> --- a/examples/qos_meter/main.c
> > >> >> +++ b/examples/qos_meter/main.c
> > >> >> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> > >> >app_trtcm_params[]
> > >> >> = {
> > >> >>
> > >> >>  FLOW_METER app_flows[APP_FLOWS_MAX];
> > >> >>
> > >> >> -static void
> > >> >> +static int
> > >> >>  app_configure_flow_table(void)
> > >> >>  {
> > >> >>   uint32_t i, j;
> > >> >> + int ret = 0;
> > >> >>
> > >> >> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> > >> >> RTE_DIM(PARAMS)){
> > >> >> - FUNC_CONFIG(_flows[i], [j]);
> > >> >> - }
> > >> >> + for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
> > >> >> + i ++, j = (j + 1) % RTE_DIM(PARAMS))
> > >> >> + ret = FUNC_CONFIG(_flows[i], [j]);
> > >> >> +
> > >> >> + return ret;
> > >> >>  }
> > >> >
> > >> >This is only returns the configuration status for the last flow and
> > >> >leaves undetected an error for any other flow. Why not check the
> > >> >status for each flow and return an error on first occurrence?
> > >> >For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
> > >> >
> > >>
> > >> This code check status of the

[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Thursday, May 12, 2016 8:06 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder 
> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> 
> 
> 
> >-----Original Message-----
> >From: Dumitrescu, Cristian
> >Sent: Wednesday, May 11, 2016 7:57 PM
> >To: Mrozowicz, SlawomirX 
> >Cc: dev at dpdk.org; Singh, Jasvinder 
> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> >
> >
> >
> >> -Original Message-
> >> From: Mrozowicz, SlawomirX
> >> Sent: Wednesday, May 11, 2016 10:15 AM
> >> To: Dumitrescu, Cristian 
> >> Cc: dev at dpdk.org; Singh, Jasvinder 
> >> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> >>
> >>
> >>
> >> >-Original Message-
> >> >From: Dumitrescu, Cristian
> >> >Sent: Tuesday, May 10, 2016 7:42 PM
> >> >To: Mrozowicz, SlawomirX 
> >> >Cc: dev at dpdk.org; Singh, Jasvinder 
> >> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return
> >> >value
> >> >
> >> >
> >> >
> >> >> -Original Message-
> >> >> From: Mrozowicz, SlawomirX
> >> >> Sent: Monday, May 9, 2016 9:38 AM
> >> >> To: Dumitrescu, Cristian 
> >> >> Cc: dev at dpdk.org; Singh, Jasvinder ;
> >> >> Mrozowicz, SlawomirX 
> >> >> Subject: [PATCH v3] examples/qos_meter: fix unchecked return value
> >> >>
> >> >> Fix issue reported by Coverity.
> >> >>
> >> >> Coverity ID 30693: Unchecked return value
> >> >> check_return: Calling rte_meter_srtcm_config without checking
> >> >> return value.
> >> >>
> >> >> Fixes: e6541fdec8b2 ("meter: initial import")
> >> >>
> >> >> Signed-off-by: Slawomir Mrozowicz
> 
> >> >> ---
> >> >>  examples/qos_meter/main.c | 15 ++-
> >> >> examples/qos_meter/main.h |  2 +-
> >> >>  2 files changed, 11 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/examples/qos_meter/main.c
> b/examples/qos_meter/main.c
> >> >> index b968b00..7c69606 100644
> >> >> --- a/examples/qos_meter/main.c
> >> >> +++ b/examples/qos_meter/main.c
> >> >> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> >> >app_trtcm_params[]
> >> >> = {
> >> >>
> >> >>  FLOW_METER app_flows[APP_FLOWS_MAX];
> >> >>
> >> >> -static void
> >> >> +static int
> >> >>  app_configure_flow_table(void)
> >> >>  {
> >> >> uint32_t i, j;
> >> >> +   int ret = 0;
> >> >>
> >> >> -   for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> >> >> RTE_DIM(PARAMS)){
> >> >> -   FUNC_CONFIG(_flows[i], [j]);
> >> >> -   }
> >> >> +   for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
> >> >> +   i ++, j = (j + 1) % RTE_DIM(PARAMS))
> >> >> +   ret = FUNC_CONFIG(_flows[i], [j]);
> >> >> +
> >> >> +   return ret;
> >> >>  }
> >> >
> >> >This is only returns the configuration status for the last flow and
> >> >leaves undetected an error for any other flow. Why not check the
> >> >status for each flow and return an error on first occurrence?
> >> >For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
> >> >
> >>
> >> This code check status of the function FUNC_CONFIG for each flow and
> >> return an error on first occurrence. Rest of functions FUNC_CONFIG are
> >> not called. See terminate condition of the loop.
> >>
> >
> >Where is the status of FUNC_CONFIG checked exactly? I cannot see any
> check
> >in your code. I can only see returning the status code for the last call of 
> >this
> >function in the for loop. I was expecting a check such as: if (ret) return 
> >ret.
> >
> 
> Look at the loop terminate conditions:
> i < APP_FLOWS_MAX && ret == 0
> Program terminate the loop if the ret variable is differ then zero.
> It means that program terminate i

[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Wednesday, May 11, 2016 10:15 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder 
> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> 
> 
> 
> >-----Original Message-----
> >From: Dumitrescu, Cristian
> >Sent: Tuesday, May 10, 2016 7:42 PM
> >To: Mrozowicz, SlawomirX 
> >Cc: dev at dpdk.org; Singh, Jasvinder 
> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> >
> >
> >
> >> -Original Message-
> >> From: Mrozowicz, SlawomirX
> >> Sent: Monday, May 9, 2016 9:38 AM
> >> To: Dumitrescu, Cristian 
> >> Cc: dev at dpdk.org; Singh, Jasvinder ;
> >> Mrozowicz, SlawomirX 
> >> Subject: [PATCH v3] examples/qos_meter: fix unchecked return value
> >>
> >> Fix issue reported by Coverity.
> >>
> >> Coverity ID 30693: Unchecked return value
> >> check_return: Calling rte_meter_srtcm_config without checking return
> >> value.
> >>
> >> Fixes: e6541fdec8b2 ("meter: initial import")
> >>
> >> Signed-off-by: Slawomir Mrozowicz 
> >> ---
> >>  examples/qos_meter/main.c | 15 ++-
> >> examples/qos_meter/main.h |  2 +-
> >>  2 files changed, 11 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> >> index b968b00..7c69606 100644
> >> --- a/examples/qos_meter/main.c
> >> +++ b/examples/qos_meter/main.c
> >> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> >app_trtcm_params[]
> >> = {
> >>
> >>  FLOW_METER app_flows[APP_FLOWS_MAX];
> >>
> >> -static void
> >> +static int
> >>  app_configure_flow_table(void)
> >>  {
> >>uint32_t i, j;
> >> +  int ret = 0;
> >>
> >> -  for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> >> RTE_DIM(PARAMS)){
> >> -  FUNC_CONFIG(_flows[i], [j]);
> >> -  }
> >> +  for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
> >> +  i ++, j = (j + 1) % RTE_DIM(PARAMS))
> >> +  ret = FUNC_CONFIG(_flows[i], [j]);
> >> +
> >> +  return ret;
> >>  }
> >
> >This is only returns the configuration status for the last flow and leaves
> >undetected an error for any other flow. Why not check the status for each
> >flow and return an error on first occurrence?
> >For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
> >
> 
> This code check status of the function FUNC_CONFIG for each flow and
> return an error on first occurrence. Rest of functions FUNC_CONFIG are  not
> called. See terminate condition of the loop.
> 

Where is the status of FUNC_CONFIG checked exactly? I cannot see any check in 
your code. I can only see returning the status code for the last call of this 
function in the for loop. I was expecting a check such as: if (ret) return ret.

> >>
> >>  static inline void
> >> @@ -381,7 +384,9 @@ main(int argc, char **argv)
> >>rte_eth_promiscuous_enable(port_tx);
> >>
> >>/* App configuration */
> >> -  app_configure_flow_table();
> >> +  ret = app_configure_flow_table();
> >> +  if (ret < 0)
> >> +  rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
> >>
> >>/* Launch per-lcore init on every lcore */
> >>rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); diff -
> -
> >git
> >> a/examples/qos_meter/main.h b/examples/qos_meter/main.h index
> >> 530bf69..54867dc 100644
> >> --- a/examples/qos_meter/main.h
> >> +++ b/examples/qos_meter/main.h
> >> @@ -51,7 +51,7 @@ enum policer_action
> >> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] =  #if
> >APP_MODE
> >> == APP_MODE_FWD
> >>
> >>  #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len,
> >> time=time -#define FUNC_CONFIG(a,b)
> >> +#define FUNC_CONFIG(a, b) 0
> >>  #define PARAMSapp_srtcm_params
> >>  #define FLOW_METER int
> >>
> >> --
> >> 1.9.1

[dpdk-dev] [PATCH v4] examples/qos_sched: fix bad bit shift operation

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Wednesday, May 11, 2016 9:48 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ;
> Mrozowicz, SlawomirX 
> Subject: [PATCH v4] examples/qos_sched: fix bad bit shift operation
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30690: Bad bit shift operation
> large_shift: In expression 1ULL << i, left shifting by more than 63 bits
> has undefined behavior. The shift amount, i, is as much as 127.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 4 ++--
>  examples/qos_sched/main.h | 5 -
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..354372d 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -123,7 +123,7 @@ app_eal_core_mask(void)
>   uint64_t cm = 0;
>   struct rte_config *cfg = rte_eal_get_configuration();
> 
> - for (i = 0; i < RTE_MAX_LCORE; i++) {
> + for (i = 0; i < APP_MAX_LCORE; i++) {
>   if (cfg->lcore_role[i] == ROLE_RTE)
>   cm |= (1ULL << i);
>   }
> @@ -142,7 +142,7 @@ app_cpu_core_count(void)
>   char path[PATH_MAX];
>   uint32_t ncores = 0;
> 
> - for(i = 0; i < RTE_MAX_LCORE; i++) {
> + for (i = 0; i < APP_MAX_LCORE; i++) {
>   len = snprintf(path, sizeof(path), SYS_CPU_DIR, i);
>   if (len <= 0 || (unsigned)len >= sizeof(path))
>   continue;
> diff --git a/examples/qos_sched/main.h b/examples/qos_sched/main.h
> index 82aa0fa..c7490c6 100644
> --- a/examples/qos_sched/main.h
> +++ b/examples/qos_sched/main.h
> @@ -68,7 +68,10 @@ extern "C" {
> 
>  #define BURST_TX_DRAIN_US 100
> 
> -#define MAX_DATA_STREAMS (RTE_MAX_LCORE/2)
> +#ifndef APP_MAX_LCORE
> +#define APP_MAX_LCORE 64
> +#endif
> +#define MAX_DATA_STREAMS (APP_MAX_LCORE/2)
>  #define MAX_SCHED_SUBPORTS   8
>  #define MAX_SCHED_PIPES  4096
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] fast red autotest

[Dumitrescu, Cristian]

CC-ing Tomasz, who is the original author of RED implementation and its 
autotest. Tomasz, what do you think?

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, May 11, 2016 8:45 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org
> Subject: fast red autotest
> 
> The autotest for librte_sched red takes more than a minute.
> Would it be possible to reduce it to a second please?
> If it is really impossible, it must be removed from fast_test.

[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Monday, May 9, 2016 9:38 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ;
> Mrozowicz, SlawomirX 
> Subject: [PATCH v3] examples/qos_meter: fix unchecked return value
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30693: Unchecked return value
> check_return: Calling rte_meter_srtcm_config without checking return
> value.
> 
> Fixes: e6541fdec8b2 ("meter: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_meter/main.c | 15 ++-
>  examples/qos_meter/main.h |  2 +-
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> index b968b00..7c69606 100644
> --- a/examples/qos_meter/main.c
> +++ b/examples/qos_meter/main.c
> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> app_trtcm_params[] = {
> 
>  FLOW_METER app_flows[APP_FLOWS_MAX];
> 
> -static void
> +static int
>  app_configure_flow_table(void)
>  {
>   uint32_t i, j;
> + int ret = 0;
> 
> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> RTE_DIM(PARAMS)){
> - FUNC_CONFIG(_flows[i], [j]);
> - }
> + for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
> + i ++, j = (j + 1) % RTE_DIM(PARAMS))
> + ret = FUNC_CONFIG(_flows[i], [j]);
> +
> + return ret;
>  }

This is only returns the configuration status for the last flow and leaves 
undetected an error for any other flow. Why not check the status for each flow 
and return an error on first occurrence?
For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}

> 
>  static inline void
> @@ -381,7 +384,9 @@ main(int argc, char **argv)
>   rte_eth_promiscuous_enable(port_tx);
> 
>   /* App configuration */
> - app_configure_flow_table();
> + ret = app_configure_flow_table();
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
> 
>   /* Launch per-lcore init on every lcore */
>   rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
> diff --git a/examples/qos_meter/main.h b/examples/qos_meter/main.h
> index 530bf69..54867dc 100644
> --- a/examples/qos_meter/main.h
> +++ b/examples/qos_meter/main.h
> @@ -51,7 +51,7 @@ enum policer_action
> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] =
>  #if APP_MODE == APP_MODE_FWD
> 
>  #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len,
> time=time
> -#define FUNC_CONFIG(a,b)
> +#define FUNC_CONFIG(a, b) 0
>  #define PARAMS   app_srtcm_params
>  #define FLOW_METER int
> 
> --
> 1.9.1

[dpdk-dev] [PATCH v3] examples/qos_sched: fix bad bit shift operation

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Tuesday, May 10, 2016 1:21 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ;
> Mrozowicz, SlawomirX 
> Subject: [PATCH v3] examples/qos_sched: fix bad bit shift operation
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30690: Bad bit shift operation
> large_shift: In expression 1ULL << i, left shifting by more than 63 bits
> has undefined behavior. The shift amount, i, is as much as 127.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 4 ++--
>  examples/qos_sched/main.h | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..354372d 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -123,7 +123,7 @@ app_eal_core_mask(void)
>   uint64_t cm = 0;
>   struct rte_config *cfg = rte_eal_get_configuration();
> 
> - for (i = 0; i < RTE_MAX_LCORE; i++) {
> + for (i = 0; i < APP_MAX_LCORE; i++) {
>   if (cfg->lcore_role[i] == ROLE_RTE)
>   cm |= (1ULL << i);
>   }
> @@ -142,7 +142,7 @@ app_cpu_core_count(void)
>   char path[PATH_MAX];
>   uint32_t ncores = 0;
> 
> - for(i = 0; i < RTE_MAX_LCORE; i++) {
> + for (i = 0; i < APP_MAX_LCORE; i++) {
>   len = snprintf(path, sizeof(path), SYS_CPU_DIR, i);
>   if (len <= 0 || (unsigned)len >= sizeof(path))
>   continue;
> diff --git a/examples/qos_sched/main.h b/examples/qos_sched/main.h
> index 82aa0fa..e0517d1 100644
> --- a/examples/qos_sched/main.h
> +++ b/examples/qos_sched/main.h
> @@ -68,7 +68,8 @@ extern "C" {
> 
>  #define BURST_TX_DRAIN_US 100
> 
> -#define MAX_DATA_STREAMS (RTE_MAX_LCORE/2)
> +#define APP_MAX_LCORE 64

Please allow this parameter to be configured from Makefile:

#ifndef APP_MAX_LCORE
#define APP_MAX_LCORE 64
#endif

> +#define MAX_DATA_STREAMS (APP_MAX_LCORE/2)
>  #define MAX_SCHED_SUBPORTS   8
>  #define MAX_SCHED_PIPES  4096
> 
> --
> 1.9.1

Yep, looks good. Thanks, Slawomir!

Regards,
Cristian

[dpdk-dev] [PATCH] sched: fix useless call

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrzyglod, DanielX T
> Sent: Tuesday, May 10, 2016 11:11 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Mrzyglod, DanielX T 
> Subject: [PATCH] sched: fix useless call
> 
> Fix issue reported by Coverity.
> Coverity ID 13338
> 
> A function call that seems to have an intended effect has no actual effect
> on the logic of the program.
> 
> In rte_sched_port_free: A function is called that is only useful for its
> return value, and this value is ignored.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Daniel Mrzyglod 
> ---
>  lib/librte_sched/rte_sched.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index 1609ea8..9b962a6 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -749,7 +749,6 @@ rte_sched_port_free(struct rte_sched_port *port)
>   rte_pktmbuf_free(mbufs[i]);
>   }
> 
> - rte_bitmap_free(port->bmp);
>   rte_free(port);
>  }
> 
> --
> 2.5.5

NAK.

This needs to be flagged out as a false positive to Coverity.

As previously discussed on this email list, the rte_bitmap_free() is an API 
function that works as a placeholder for any resource freeing that needs to be 
done for the bitmap. The API function should not be removed and also the call 
to this function from the rte_sched_port_free() should not be removed either.

Thanks,
Cristian

[dpdk-dev] [PATCH v3] examples/qos_sched: fix bad bit shift operation

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Tuesday, May 10, 2016 10:40 AM
> To: Dumitrescu, Cristian ; Jastrzebski,
> MichalX K ; Zhang, Roy Fan
> ; Singh, Jasvinder 
> Cc: dev at dpdk.org
> Subject: RE: [PATCH v3] examples/qos_sched: fix bad bit shift operation
> 
> 
> >-Original Message-
> >From: Dumitrescu, Cristian
> >Sent: Thursday, April 28, 2016 1:16 PM
> >To: Jastrzebski, MichalX K ; Zhang, Roy
> Fan
> >; Singh, Jasvinder 
> >Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> >Subject: RE: [PATCH v3] examples/qos_sched: fix bad bit shift operation
> >
> >
> >
> >> -Original Message-
> >> From: Jastrzebski, MichalX K
> >> Sent: Thursday, April 21, 2016 2:08 PM
> >> To: Dumitrescu, Cristian ; Zhang, Roy
> >> Fan ; Singh, Jasvinder
> >> 
> >> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> >
> >> Subject: [PATCH v3] examples/qos_sched: fix bad bit shift operation
> >>
> >> From: Slawomir Mrozowicz 
> >>
> >> Fix issue reported by Coverity.
> >>
> >> Coverity ID 30690: Bad bit shift operation
> >> large_shift: In expression 1ULL << i, left shifting by more than 63
> >> bits has undefined behavior. The shift amount, i, is as much as 127.
> >>
> >> Fixes: de3cfa2c9823 ("sched: initial import")
> >>
> >> Signed-off-by: Slawomir Mrozowicz 
> >> ---
> >>  examples/qos_sched/args.c | 84 +--
> ---
> >-
> >> 
> >>  1 file changed, 52 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> >> index 3e7fd08..cd077ba 100644
> >> --- a/examples/qos_sched/args.c
> >> +++ b/examples/qos_sched/args.c
> >> @@ -53,7 +53,7 @@
> >>
> >>  static uint32_t app_master_core = 1;
> >>  static uint32_t app_numa_mask;
> >> -static uint64_t app_used_core_mask = 0;
> >> +static int app_used_core_mask[RTE_MAX_LCORE];
> 
> Changed type of the app_used_core_mask variable to store up to
> RTE_MAX_LCORE cores information.
> 
> >>  static uint64_t app_used_port_mask = 0;  static uint64_t
> >> app_used_rx_port_mask = 0;  static uint64_t app_used_tx_port_mask =
> 0;
> >> @@ -115,22 +115,23 @@ static inline int str_is(const char *str, const char
> >*is)
> >>return strcmp(str, is) == 0;
> >>  }
> >>
> >> -/* returns core mask used by DPDK */
> >> -static uint64_t
> >> -app_eal_core_mask(void)
> >> +/* compare used core with eal configuration,
> >> +  returns:
> >> +  1 if equal
> >> +  0 if differ */
> >> +static int
> >> +app_eal_core_check(void)
> >>  {
> >> -  uint32_t i;
> >> -  uint64_t cm = 0;
> >> +  uint16_t i;
> >> +  int ret = 1;
> >>struct rte_config *cfg = rte_eal_get_configuration();
> >>
> >> -  for (i = 0; i < RTE_MAX_LCORE; i++) {
> >> -  if (cfg->lcore_role[i] == ROLE_RTE)
> >> -  cm |= (1ULL << i);
> >> +  for (i = 0; i < RTE_MAX_LCORE && ret; i++) {
> >> +  if ((cfg->lcore_role[i] == ROLE_RTE) !=
> >> app_used_core_mask[i])
> >> +  ret = 0;
> >>}
> >>
> >> -  cm |= (1ULL << cfg->master_lcore);
> >> -
> >> -  return cm;
> >> +  return ret;
> >>  }
> >>
> 
> Added tool function app_eal_core_check() to check compatibility used cores
> with information stored in configuration file. The function is used below.
> Removed not used function app_eal_core_mask()
> 
> >>
> >> @@ -292,14 +293,9 @@ app_parse_flow_conf(const char *conf_str)
> >>app_used_tx_port_mask |= mask;
> >>app_used_port_mask |= mask;
> >>
> >> -  mask = 1lu << pconf->rx_core;
> >> -  app_used_core_mask |= mask;
> >> -
> >> -  mask = 1lu << pconf->wt_core;
> >> -  app_used_core_mask |= mask;
> >> -
> >> -  mask = 1lu << pconf->tx_core;
> >> -  app_used_core_mask |= mask;
> >> +  app_used_core_mask[pconf->rx_core] = 1;
> >> +  app_used_core_mask[pconf->wt_core] = 1;
> >> +  app_used_core_mask[pconf->tx_core] = 1;
> >>
> 
> Change method of set the mask on each used core according to change mask
> type.
> 
>

[dpdk-dev] Old oversubscription related checkin

[Dumitrescu, Cristian]

Hi Sridhar,

I think this patch is simply the implementation of the oversubscription 
feature, as introduced by DPDK release 1.4.1. Most likely, this patch is the 
way Thomas added this Intel release to dpdk.org.

The oversubscription feature was never implemented for all 4 traffic classes. 
The patch you are pointing to simply introduced this feature for TC3 only. 
Before this patch, there might have been just a placeholder in the code for the 
oversubscription feature that was to be developed later on.

Again, the reason for introducing this feature for TC3 only is that the higher 
priority traffic classes TC0 .. TC2 are typically fully provisioned, as the 
amount of TC0 .. TC2 traffic is much less than TC3 (Best Effort) traffic, which 
is usually hugely overprovisioned. This feature can potentially be extended to 
TC0 .. TC2 as well, as the way this feature basically works is deciding the 
quanta to be applied for all pipes in the subport during the next scheduling 
decision based on the amount of unused credits found at the end of the current 
scheduling period.

Regards,
Cristian

From: Sridhar.V.Iyer [mailto:sridhari...@versa-networks.com]
Sent: Wednesday, May 4, 2016 10:15 PM
To: dev at dpdk.org; Dumitrescu, Cristian 
Cc: Ananda 
Subject: Old oversubscription related checkin

Hi Cristian,

 I stumbled into an old from 2013 
(http://dpdk.org/browse/dpdk/patch/lib/librte_sched/rte_sched.c?id=835c5409a7bac3055b82bebee65d8ada7f20d332)

I couldn?t find any context for the patch from the mail archives. Could you 
please let me know why oversubscription was removed from all the other traffic 
classes and just given to tc3?
Were there huge performance penalties? Would there be any issues if I add this 
patch back again locally (enable/disabled via config).

Regards,

Sridhar V Iyer

<http://www.versa-networks.com/>






<http://www.versa-networks.com/>


<http://www.versa-networks.com/>


<http://www.versa-networks.com/>

[dpdk-dev] [PATCH v2] examples/qos_meter: fix unchecked return value

[Dumitrescu, Cristian]

> -Original Message-
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 12:48 PM
> To: Dumitrescu, Cristian ; Zhang, Roy Fan
> ; Singh, Jasvinder 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH v2] examples/qos_meter: fix unchecked return value
> 
> From: Slawomir Mrozowicz 
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30693: Unchecked return value
> check_return: Calling rte_meter_srtcm_config without checking return
> value.
> 
> Fixes: e6541fdec8b2 ("meter: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_meter/main.c | 15 ++-
>  examples/qos_meter/main.h |  2 +-
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> index b968b00..16b0b87 100644
> --- a/examples/qos_meter/main.c
> +++ b/examples/qos_meter/main.c
> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> app_trtcm_params[] = {
> 
>  FLOW_METER app_flows[APP_FLOWS_MAX];
> 
> -static void
> +static int
>  app_configure_flow_table(void)
>  {
>   uint32_t i, j;
> + int ret = 0;
> 
> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> RTE_DIM(PARAMS)){
> - FUNC_CONFIG(_flows[i], [j]);
> - }
> + for (i = 0, j = 0; i < APP_FLOWS_MAX;
> + i ++, j = (j + 1) % RTE_DIM(PARAMS))
> + ret |= FUNC_CONFIG(_flows[i], [j]);
> +
> + return ret;
>  }


You should probably return of the first error rather than carry on. How would 
you know which is the flow that produced the configuration error?

for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) % RTE_DIM(PARAMS)){
ret = FUNC_CONFIG(_flows[i], [j]);
if (ret)
return;
}

> 
>  static inline void
> @@ -381,7 +384,9 @@ main(int argc, char **argv)
>   rte_eth_promiscuous_enable(port_tx);
> 
>   /* App configuration */
> - app_configure_flow_table();
> + ret = app_configure_flow_table();
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
> 
>   /* Launch per-lcore init on every lcore */
>   rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
> diff --git a/examples/qos_meter/main.h b/examples/qos_meter/main.h
> index 530bf69..54867dc 100644
> --- a/examples/qos_meter/main.h
> +++ b/examples/qos_meter/main.h
> @@ -51,7 +51,7 @@ enum policer_action
> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] =
>  #if APP_MODE == APP_MODE_FWD
> 
>  #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len,
> time=time
> -#define FUNC_CONFIG(a,b)
> +#define FUNC_CONFIG(a, b) 0
>  #define PARAMS   app_srtcm_params
>  #define FLOW_METER int
> 
> --
> 1.9.1

[dpdk-dev] [PATCH v3] examples/qos_sched: fix bad bit shift operation

[Dumitrescu, Cristian]

> -Original Message-
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 2:08 PM
> To: Dumitrescu, Cristian ; Zhang, Roy Fan
> ; Singh, Jasvinder 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH v3] examples/qos_sched: fix bad bit shift operation
> 
> From: Slawomir Mrozowicz 
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30690: Bad bit shift operation
> large_shift: In expression 1ULL << i, left shifting by more than 63 bits
> has undefined behavior. The shift amount, i, is as much as 127.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 84 +--
> 
>  1 file changed, 52 insertions(+), 32 deletions(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..cd077ba 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -53,7 +53,7 @@
> 
>  static uint32_t app_master_core = 1;
>  static uint32_t app_numa_mask;
> -static uint64_t app_used_core_mask = 0;
> +static int app_used_core_mask[RTE_MAX_LCORE];
>  static uint64_t app_used_port_mask = 0;
>  static uint64_t app_used_rx_port_mask = 0;
>  static uint64_t app_used_tx_port_mask = 0;
> @@ -115,22 +115,23 @@ static inline int str_is(const char *str, const char 
> *is)
>   return strcmp(str, is) == 0;
>  }
> 
> -/* returns core mask used by DPDK */
> -static uint64_t
> -app_eal_core_mask(void)
> +/* compare used core with eal configuration,
> + returns:
> + 1 if equal
> + 0 if differ */
> +static int
> +app_eal_core_check(void)
>  {
> - uint32_t i;
> - uint64_t cm = 0;
> + uint16_t i;
> + int ret = 1;
>   struct rte_config *cfg = rte_eal_get_configuration();
> 
> - for (i = 0; i < RTE_MAX_LCORE; i++) {
> - if (cfg->lcore_role[i] == ROLE_RTE)
> - cm |= (1ULL << i);
> + for (i = 0; i < RTE_MAX_LCORE && ret; i++) {
> + if ((cfg->lcore_role[i] == ROLE_RTE) !=
> app_used_core_mask[i])
> + ret = 0;
>   }
> 
> - cm |= (1ULL << cfg->master_lcore);
> -
> - return cm;
> + return ret;
>  }
> 
> 
> @@ -292,14 +293,9 @@ app_parse_flow_conf(const char *conf_str)
>   app_used_tx_port_mask |= mask;
>   app_used_port_mask |= mask;
> 
> - mask = 1lu << pconf->rx_core;
> - app_used_core_mask |= mask;
> -
> - mask = 1lu << pconf->wt_core;
> - app_used_core_mask |= mask;
> -
> - mask = 1lu << pconf->tx_core;
> - app_used_core_mask |= mask;
> + app_used_core_mask[pconf->rx_core] = 1;
> + app_used_core_mask[pconf->wt_core] = 1;
> + app_used_core_mask[pconf->tx_core] = 1;
> 
>   nb_pfc++;
> 
> @@ -335,7 +331,7 @@ app_parse_args(int argc, char **argv)
>   int option_index;
>   const char *optname;
>   char *prgname = argv[0];
> - uint32_t i, nb_lcores;
> + uint16_t i, j, k, nb_lcores;
> 
>   static struct option lgopts[] = {
>   { "pfc", 1, 0, 0 },
> @@ -349,6 +345,9 @@ app_parse_args(int argc, char **argv)
>   { NULL,  0, 0, 0 }
>   };
> 
> + for (i = 0; i < RTE_MAX_LCORE; i++)
> + app_used_core_mask[i] = 0;
> +
>   /* initialize EAL first */
>   ret = rte_eal_init(argc, argv);
>   if (ret < 0)
> @@ -436,19 +435,40 @@ app_parse_args(int argc, char **argv)
>   }
> 
>   /* check master core index validity */
> - for(i = 0; i <= app_master_core; i++) {
> - if (app_used_core_mask & (1u << app_master_core)) {
> - RTE_LOG(ERR, APP, "Master core index is not
> configured properly\n");
> - app_usage(prgname);
> - return -1;
> - }
> + if (app_used_core_mask[app_master_core] == 1) {
> + RTE_LOG(ERR, APP,
> + "Master core index is not configured properly\n");
> + app_usage(prgname);
> + return -1;
>   }
> - app_used_core_mask |= 1u << app_master_core;
> + app_used_core_mask[app_master_core] = 1;
> +
> + if ((app_eal_core_check() == 0) ||
> + (app_master_core != rte_get_master_lcore())) {
> +
> + char used_hexstr[RTE_MAX_LCORE/4+1];
> + char conf_hexstr[RTE_MAX_LCORE/4+1];
> + int used_byte, conf_byte;
> +

[dpdk-dev] [PATCH] cfgfile: fix integer overflow

[Dumitrescu, Cristian]

> -Original Message-
> From: Kobylinski, MichalX
> Sent: Friday, April 22, 2016 11:41 AM
> To: Dumitrescu, Cristian ; dev at dpdk.org
> Cc: Kobylinski, MichalX 
> Subject: [PATCH] cfgfile: fix integer overflow
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 13289: Integer overflowed argument: The argument will be too
> small or even negative, likely resulting in unexpected behavior (for
> example, under-allocation in a memory allocation function).
> In rte_cfgfile_load: An integer overflow occurs, with the overflowed
> value used as an argument to a function
> 
> Fixes: eaafbad419bf ("cfgfile: library to interpret config files")
> 
> Signed-off-by: Michal Kobylinski 
> ---
>  lib/librte_cfgfile/rte_cfgfile.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cfgfile/rte_cfgfile.c 
> b/lib/librte_cfgfile/rte_cfgfile.c
> index 75625a2..0a5a279 100644
> --- a/lib/librte_cfgfile/rte_cfgfile.c
> +++ b/lib/librte_cfgfile/rte_cfgfile.c
> @@ -135,7 +135,7 @@ rte_cfgfile_load(const char *filename, int flags)
>   goto error1;
>   }
>   *end = '\0';
> - _strip([1], end - [1]);
> + _strip([1], (unsigned)(end - [1]));
> 
>   /* close off old section and add start new one */
>   if (curr_section >= 0)
> --
> 1.9.1

I don't understand the root issue here, can you please explain?

It looks to me that "end" is always going to point to a location bigger or 
equal to [1]. So the second parameter of _strip function is always going 
to be a positive number (0 included).

[dpdk-dev] [PATCH v3] examples/qos_sched: fix copy-paste error

[Dumitrescu, Cristian]

> -Original Message-
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 2:08 PM
> To: Dumitrescu, Cristian ; Zhang, Roy Fan
> ; Singh, Jasvinder 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH v3] examples/qos_sched: fix copy-paste error
> 
> From: Slawomir Mrozowicz 
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30699: Copy-paste error;
> rx_port in pconf->rx_port looks like a copy-paste error.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..1916790 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -270,7 +270,7 @@ app_parse_flow_conf(const char *conf_str)
>   }
>   if (pconf->tx_port >= RTE_MAX_ETHPORTS) {
>   RTE_LOG(ERR, APP, "pfc %u: invalid tx port %"PRIu8"
> index\n",
> - nb_pfc, pconf->rx_port);
> + nb_pfc, pconf->tx_port);
>   return -1;
>   }
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v3] examples/qos_sched: fix negative loop bound

[Dumitrescu, Cristian]

> -Original Message-
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 2:08 PM
> To: Dumitrescu, Cristian ; Zhang, Roy Fan
> ; Singh, Jasvinder 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH v3] examples/qos_sched: fix negative loop bound
> 
> From: Slawomir Mrozowicz 
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30704: Negative loop bound
> negative_returns: Using unsigned variable n_tokens in a loop exit condition.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..7a98e5c 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -162,7 +162,7 @@ static int
>  app_parse_opt_vals(const char *conf_str, char separator, uint32_t n_vals,
> uint32_t *opt_vals)
>  {
>   char *string;
> - uint32_t i, n_tokens;
> + int i, n_tokens;
>   char *tokens[MAX_OPT_VALUES];
> 
>   if (conf_str == NULL || opt_vals == NULL || n_vals == 0 || n_vals >
> MAX_OPT_VALUES)
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v2] examples/qos_sched: fix out-of-bounds read

[Dumitrescu, Cristian]

> -Original Message-
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 12:48 PM
> To: Dumitrescu, Cristian ; Zhang, Roy Fan
> ; Singh, Jasvinder 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH v2] examples/qos_sched: fix out-of-bounds read
> 
> From: Slawomir Mrozowicz 
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30708: Out-of-bounds read
> overrun-local: Overrunning array tokens of 8 8-byte elements
> at element index 4294967294 (byte offset 34359738352)
> using index i (which evaluates to 4294967294).
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..d819269 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -175,9 +175,11 @@ app_parse_opt_vals(const char *conf_str, char
> separator, uint32_t n_vals, uint32
> 
>   n_tokens = rte_strsplit(string, strnlen(string, 32), tokens, n_vals,
> separator);
> 
> - for(i = 0; i < n_tokens; i++) {
> + if (n_tokens > MAX_OPT_VALUES)
> + return -1;
> +
> + for (i = 0; i < n_tokens; i++)
>   opt_vals[i] = (uint32_t)atol(tokens[i]);
> - }
> 
>   free(string);
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v2] examples/qos_sched: fix negative loop bound

[Dumitrescu, Cristian]

> -Original Message-
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 12:48 PM
> To: Dumitrescu, Cristian ; Zhang, Roy Fan
> ; Singh, Jasvinder 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH v2] examples/qos_sched: fix negative loop bound
> 
> From: Slawomir Mrozowicz 
> 
> Coverity ID 30704: Negative loop bound
> negative_returns: Using unsigned variable n_tokens in a loop exit condition.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..7a98e5c 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -162,7 +162,7 @@ static int
>  app_parse_opt_vals(const char *conf_str, char separator, uint32_t n_vals,
> uint32_t *opt_vals)
>  {
>   char *string;
> - uint32_t i, n_tokens;
> + int i, n_tokens;
>   char *tokens[MAX_OPT_VALUES];
> 
>   if (conf_str == NULL || opt_vals == NULL || n_vals == 0 || n_vals >
> MAX_OPT_VALUES)
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v2] examples/qos_sched: fix copy-paste error

[Dumitrescu, Cristian]

> -Original Message-
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 12:48 PM
> To: Dumitrescu, Cristian ; Zhang, Roy Fan
> ; Singh, Jasvinder 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH v2] examples/qos_sched: fix copy-paste error
> 
> From: Slawomir Mrozowicz 
> 
> Coverity ID 30699: Copy-paste error;
> rx_port in pconf->rx_port looks like a copy-paste error.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..1916790 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -270,7 +270,7 @@ app_parse_flow_conf(const char *conf_str)
>   }
>   if (pconf->tx_port >= RTE_MAX_ETHPORTS) {
>   RTE_LOG(ERR, APP, "pfc %u: invalid tx port %"PRIu8"
> index\n",
> - nb_pfc, pconf->rx_port);
> + nb_pfc, pconf->tx_port);
>   return -1;
>   }
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] examples: fix CID 30704 negative loop bound

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Thursday, April 14, 2016 12:16 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH] examples: fix CID 30704 negative loop bound
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..7a98e5c 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -162,7 +162,7 @@ static int
>  app_parse_opt_vals(const char *conf_str, char separator, uint32_t n_vals,
> uint32_t *opt_vals)
>  {
>   char *string;
> - uint32_t i, n_tokens;
> + int i, n_tokens;
>   char *tokens[MAX_OPT_VALUES];
> 
>   if (conf_str == NULL || opt_vals == NULL || n_vals == 0 || n_vals >
> MAX_OPT_VALUES)
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] examples: fix CID 30708 out-of-bounds read

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Thursday, April 14, 2016 10:53 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH] examples: fix CID 30708 out-of-bounds read
> 
> It fix coverity issue:
> CID 30708 (#1 of 1): Out-of-bounds read (OVERRUN)
> 12. overrun-local: Overrunning array tokens of 8 8-byte elements
> at element index 4294967294 (byte offset 34359738352)
> using index i (which evaluates to 4294967294).
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..d819269 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -175,9 +175,11 @@ app_parse_opt_vals(const char *conf_str, char
> separator, uint32_t n_vals, uint32
> 
>   n_tokens = rte_strsplit(string, strnlen(string, 32), tokens, n_vals,
> separator);
> 
> - for(i = 0; i < n_tokens; i++) {
> + if (n_tokens > MAX_OPT_VALUES)
> + return -1;
> +
> + for (i = 0; i < n_tokens; i++)
>   opt_vals[i] = (uint32_t)atol(tokens[i]);
> - }
> 
>   free(string);
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] examples/ip_pipeline: fix out-of-bounds write

[Dumitrescu, Cristian]

> -Original Message-
> From: Kerlin, MarcinX
> Sent: Thursday, April 14, 2016 10:54 AM
> To: dev at dpdk.org
> Cc: Dumitrescu, Cristian ; Kerlin, MarcinX
> 
> Subject: [PATCH] examples/ip_pipeline: fix out-of-bounds write
> 
> CID 124567:
> In the function app_init_eal(struct app params * app) number of
> entries into array exceeds the size of the array if the conditions
> are fulfilled.
> 
> Fixes: 7f64b9c004aa ("examples/ip_pipeline: rework config file syntax")
> 
> Signed-off-by: Marcin Kerlin 
> ---
>  examples/ip_pipeline/app.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
> index 55a9841..e775024 100644
> --- a/examples/ip_pipeline/app.h
> +++ b/examples/ip_pipeline/app.h
> @@ -415,7 +415,7 @@ struct app_eal_params {
>  #endif
> 
>  #ifndef APP_EAL_ARGC
> -#define APP_EAL_ARGC 32
> +#define APP_EAL_ARGC 64
>  #endif
> 
>  #ifndef APP_MAX_PIPELINE_TYPES
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] cfgfile: fix uninitialized scalar variable

[Dumitrescu, Cristian]

> -Original Message-
> From: Kobylinski, MichalX
> Sent: Wednesday, April 13, 2016 1:15 PM
> To: dev at dpdk.org; Dumitrescu, Cristian 
> Cc: Kobylinski, MichalX 
> Subject: [PATCH] cfgfile: fix uninitialized scalar variable
> 
> CID 13323:
> Uninitialized scalar variable. Using uninitialized value
> cfg->num_sections when calling rte_cfgfile_close.
> 
> Fixes: eaafbad419bf ("cfgfile: library to interpret config files")
> 
> Signed-off-by: Michal Kobylinski 
> ---
>  lib/librte_cfgfile/rte_cfgfile.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_cfgfile/rte_cfgfile.c 
> b/lib/librte_cfgfile/rte_cfgfile.c
> index 75625a2..d72052a 100644
> --- a/lib/librte_cfgfile/rte_cfgfile.c
> +++ b/lib/librte_cfgfile/rte_cfgfile.c
> @@ -232,6 +232,7 @@ rte_cfgfile_load(const char *filename, int flags)
>   return cfg;
> 
>  error1:
> + cfg->num_sections = curr_section + 1;
>   rte_cfgfile_close(cfg);
>  error2:
>   fclose(f);
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] examples: fix CID 30699 copy-paste error

[Dumitrescu, Cristian]

> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Wednesday, April 13, 2016 12:47 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Mrozowicz, SlawomirX
> 
> Subject: [PATCH] examples: fix CID 30699 copy-paste error
> 
> It fix coverity issue:
> CID 30699 (#1 of 1): Copy-paste error (COPY_PASTE_ERROR)
> copy_paste_error: rx_port in pconf->rx_port looks like a copy-paste error.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  examples/qos_sched/args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..1916790 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -270,7 +270,7 @@ app_parse_flow_conf(const char *conf_str)
>   }
>   if (pconf->tx_port >= RTE_MAX_ETHPORTS) {
>   RTE_LOG(ERR, APP, "pfc %u: invalid tx port %"PRIu8"
> index\n",
> - nb_pfc, pconf->rx_port);
> + nb_pfc, pconf->tx_port);
>   return -1;
>   }
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] KNI port type in IP pipeline

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Moon-Sang Lee
> Sent: Friday, April 15, 2016 8:13 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] KNI port type in IP pipeline
> 
> According to pp. 145 of DPDK programmer's guide 2.2.0, the KNI port type is
> described in
> table 23.1. But, I cannot find any material about how to specify the KNI
> port type in pipeline
> configuration file.
> 
> Unfortunately, it seems there is no related source file in
> $DPDK_TOP/lib/librte_port.
> 
> Does packet framework already implement the KNI port type somewhere
> or should I implement that KNI port type by myself?
> 
> regards,
> 
> 
> 
> --
> Moon-Sang Lee, SW Engineer
> Email: sang0627 at gmail.com
> Wisdom begins in wonder. *Socrates*

Hi,

There is no KNI or TUN/TAP port implemented in librte_port currently, hence 
please consider contributing this to DPDK. If the throughput of this device is 
not the first priority for you, I would actually recommend the TUN/TAP approach 
rather than KNI, as KNI seem to require significant maintenance due to constant 
changes in kernel header files, while TUN/TAP is very stable.

KNI and TUN/TAP could also be leveraged straight away by librte_port as 
rte_ethdev type of port if they would be made available as PMDs. I know there 
were some plans to make this happen, but for some reason looks like the plans 
did not become reality.

Thanks,
Cristian

[dpdk-dev] [PATCH] cfgfile: fix return value comment

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Dmitriy Yakovlev
> Sent: Friday, April 15, 2016 11:59 PM
> To: dev at dpdk.org
> Cc: Dmitriy Yakovlev 
> Subject: [dpdk-dev] [PATCH] cfgfile: fix return value comment
> 
> Function rte_cfgfile_load can return NULL value, when something goes
> wrong.
> 
> Signed-off-by: Dmitriy Yakovlev 
> ---
>  lib/librte_cfgfile/rte_cfgfile.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cfgfile/rte_cfgfile.h 
> b/lib/librte_cfgfile/rte_cfgfile.h
> index 834f828..f649836 100644
> --- a/lib/librte_cfgfile/rte_cfgfile.h
> +++ b/lib/librte_cfgfile/rte_cfgfile.h
> @@ -72,7 +72,7 @@ struct rte_cfgfile_entry {
>  * @param flags
>  *   Config file flags, Reserved for future use. Must be set to 0.
>  * @return
> -*   Handle to configuration file
> +*   Handle to configuration file on success, NULL otherwise
>  */
>  struct rte_cfgfile *rte_cfgfile_load(const char *filename, int flags);
> 
> --
> 2.6.2.windows.1

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH] port: bump ABI for pcap file support

[Dumitrescu, Cristian]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, April 14, 2016 7:34 PM
> To: Zhang, Roy Fan 
> Cc: dev at dpdk.org; Dumitrescu, Cristian ;
> Singh, Jasvinder 
> Subject: [PATCH] port: bump ABI for pcap file support
> 
> Support of PCAP file has been added to rte_port in release 16.04
> as NEXT_ABI. It is in the standard ABI of the release 16.07.
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  doc/guides/rel_notes/deprecation.rst   |  5 -
>  doc/guides/rel_notes/release_16_07.rst |  5 -
>  examples/ip_pipeline/init.c|  4 
>  lib/librte_port/Makefile   |  2 +-
>  lib/librte_port/rte_port_source_sink.c | 14 --
>  lib/librte_port/rte_port_source_sink.h |  3 ---
>  6 files changed, 5 insertions(+), 28 deletions(-)
> 


Acked-by: Cristian Dumitrescu 

[dpdk-dev] Packet drops at lower tc transmit-rates.

[Dumitrescu, Cristian]

> -Original Message-
> From: Sridhar.V.Iyer [mailto:sridhariyer at versa-networks.com]
> Sent: Wednesday, April 13, 2016 12:39 AM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] Packet drops at lower tc transmit-rates.
> 
> Hi Cristian,
> 
> Thanks for the response.
> 
> >
> > Another potential workaround could be to change the pipe TC credit
> update logic from straightforward re-initialization to a slightly more tuned
> strategy that, in some cases, keeps some of the existing credits, so that the
> existing credits are not completely lost but some of them (value capped to 1x
> MTU) are carried forward:
> >
> > pipe->tc_credits[i] = (params->tc_credits_per_period[i] < MTU)?
> > ((pipe->tc_credits[i] % MTU) + params-
> >tc_credits_per_period[i]) :
> > params->tc_credits_per_period[i];
> >
> > This would give the chance to the pipe TC credits to accumulate and
> become greater than the MTU every few periods and a packet to be
> transmitted for this pipe TC. Of course, this strategy needs to be further
> developed.
> 
> This approach seemed to give the apparent rate closest to the configured
> rate, irrespective of the MTU, the packet size, or the min packet size. I?ll 
> use
> the port->mtu to influence the tc_credits_per_period accumulation.
> 
> Is there any particular reason why a token bucket was not used for traffic
> classes?

Yes, all the pipe traffic classes are sharing the rate of their pipe, i.e. 
credits from the same pipe token bucket. Traffic classes are there just to 
describe how to divide the pipe rate amongst different types of traffic of the 
same user.

> 
> Regards,
> Sridhar

[dpdk-dev] [PATCH 4/4] port: fix ethdev writer burst too big

[Dumitrescu, Cristian]

> -Original Message-
> From: Sanford, Robert [mailto:rsanford at akamai.com]
> Sent: Monday, April 11, 2016 9:37 PM
> To: Dumitrescu, Cristian ; dev at dpdk.org
> Cc: Liang, Cunming ; Venkatesan, Venky
> ; Richardson, Bruce
> 
> Subject: Re: [dpdk-dev] [PATCH 4/4] port: fix ethdev writer burst too big
> 
> Hi Cristian,
> 
> Yes, I mostly agree with your suggestions:
> 1. We should fix the two obvious bugs (1a and 1b) right away. Jasvinder's
> patches look fine.
> 2. We should take no immediate action on the issue I raised about PMDs
> (vector IXGBE) not enqueuing more than 32 packets. We can discuss and
> debate; no patch for 16.04, perhaps something in 16.07.
> 
> 
> Let's start the discussion now, regarding vector IXGBE. You state
> "Internally it handles packets in batches [of] 32 (as your code snippets
> suggest), but there is no drop of excess packets taking place." I guess it
> depends on your definition of "drop". If I pass 33 packets to
> ixgbe_xmit_pkts_vec(), it will enqueue 32 packets, and return a value of
> 32. Can we agree on that?
> 

Yes, Steve Liang and I looked at the latest IXGBE vector code and it looks like 
you are right. The number of packets that get accepted is the minimum between 
number of packets provided by the user (33 in our case) and two thresholds, 
txq->tx_rs_thresh and txq->nb_tx_free, which are by default set to 32, which is 
the value that yields the best performance, hence only 32 packets get accepted.

It also looks virtually impossible to change this behaviour of IXGBE vector 
driver. As an example, let's say 33 packets are presented by the user, IXGBE 
picks the first 32 and tries to send them, but only 17 make it, so the other 15 
have to be returned back to the user; then there is the 33rd packet that is 
picked, and this packet makes it. Since return value is a number (not a mask), 
how do you tell the user that packets 0 .. 16 and 32 made it, while the packets 
17 .. 31 in the middle of the burst did not make it?

So the only real place to improve this is the port_ethdev_writer. I wonder 
whether there is nice way to combine existing behavior (burst treated as 
minimal requirement) with your proposal (burst treated as constant requirement) 
under the same code, and then pick between the two behaviors based on an input 
parameter provided when port is created?

> --
> Regards,
> Robert
> 
> 
> On 4/11/16 3:21 PM, "Dumitrescu, Cristian" 
> wrote:
> 
> >Hi Robert,
> >
> >I am doing a quick summary below on the changes proposed by these
> patches:
> >
> >1. [PRIORITY 1] Bug fixing:
> >a) Fix buffer overflow issue in rte_port_ring.c (writer, writer_nodrop):
> >double the tx_buf buffer size (applicable for current code approach)
> >b) Fix issue with handling burst sizes bigger than 32: replace all
> >declarations of local variable bsz_size from uint32_t to uint64_t
> >
> >2. [PRIORITY 2] Treat burst size as a fixed/exact value for the TX burst
> >(Approach 2) instead of minimal value (current code, Approach 1) for
> >ethdev ports. Rationale is that some PMDs (like vector IXGBE) _might_
> >drop the excess packets in the burst.
> >
> >Additional input:
> >1. Bruce and I looked together at the code, it looks that vector IXGBE is
> >not doing this (anymore). Internally it handles packets in batches on 32
> >(as your code snippets suggest), but there is no drop of excess packets
> >taking place.
> >
> >2. Venky also suggested to keep a larger burst as a single burst
> >(Approach 1) rather than break the larger burst into a fixed/constant
> >size burst while buffering the excess packets until complete burst is met
> >in the future.
> >
> >Given this input and also the timing of the release, we think the best
> >option is:
> >- urgently send a quick patch to handle the bug fixes now
> >- keep the existing code (burst size used as minimal burst size
> >requirement, not constant) as is, at least for now, and if you feel it is
> >not the best choice, we can continue to debate it for 16.7 release.
> >What do you think?
> >
> >Jasvinder just send the bug fixing patches, hopefully they will make it
> >into the 16.4 release:
> >http://www.dpdk.org/ml/archives/dev/2016-April/037392.html
> >http://www.dpdk.org/ml/archives/dev/2016-April/037393.html
> >
> >Many thanks for your work on this, Robert!
> >
> >Regards,
> >Cristian

[dpdk-dev] Packet drops at lower tc transmit-rates.

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Sridhar.V.Iyer
> Sent: Thursday, April 7, 2016 8:24 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] Packet drops at lower tc transmit-rates.
> 
> Hi all,
> 
> We are using DPDK 1.7 in our application.
> We are running into an issue where a lower transmit-rate configured at the
> traffic class of a subport is causing complete packet drops.
> Here are few parameters to clear up some context:
> 
> Packet length  = 728 byte
> Port rate  = 1Gbps   = 1250 bytes/s
> Subport tc_period   = 10 ms
> Configured TC0 rate   = 500 kbps   = 62500 bytes/s
> 
> This means that for the given subport tc0_credits_per_period = 10 * 62500 /
> 1000 = 625 (from rte_sched_subport_config)
> 
> Now, there is no token bucket at the subport-tc level, so there are no credits
> to accrue. The tc0_credits are just initialized to 625.
> - This means that we?ll never have ?enough_credits? in
> grinder_credits_check to process a 728 byte packet.
>- grinder_schedule will then return 0.
>  - grinder_handle will return 0.
>- which implies that the rte_sched_port_dequeue will never dequeue
> any packet.
> - After port->time exceeds the subport->tc_time, tc0_credit will be re-
> initialized back to 625 again.
> 
> Is this a bug in the logic?
> What are some of the viable workarounds?
> 
> Is this issue taken care of in the later releases?
> 
> Regards,
> Sridhar V Iyer
> 
>   


Hi Sridhar,

This case seems to take place only when the pipe TC is configured with 
relatively low rate.

One potential workaround could be to detect the case when the pipe TC credits 
per period is less than MTU size and either flag it as error or round up the 
pipe TC credits per period to at least 1x MTU:

if (pipe_params->tc_credits_per_period[i] < MTU) error(?);
if (pipe_params->tc_credits_per_period[i] < MTU) 
pipe_params->tc_credits_per_period[i] = MTU;

Another potential workaround could be to change the pipe TC credit update logic 
from straightforward re-initialization to a slightly more tuned strategy that, 
in some cases, keeps some of the existing credits, so that the existing credits 
are not completely lost but some of them (value capped to 1x MTU) are carried 
forward:

pipe->tc_credits[i] = (params->tc_credits_per_period[i] < MTU)?
((pipe->tc_credits[i] % MTU) + 
params->tc_credits_per_period[i]) : 
params->tc_credits_per_period[i];

This would give the chance to the pipe TC credits to accumulate and become 
greater than the MTU every few periods and a packet to be transmitted for this 
pipe TC. Of course, this strategy needs to be further developed.

Regards,
Cristian

[dpdk-dev] [PATCH 4/4] port: fix ethdev writer burst too big

[Dumitrescu, Cristian]

Hi Robert,

I am doing a quick summary below on the changes proposed by these patches:

1. [PRIORITY 1] Bug fixing:
a) Fix buffer overflow issue in rte_port_ring.c (writer, writer_nodrop): double 
the tx_buf buffer size (applicable for current code approach)
b) Fix issue with handling burst sizes bigger than 32: replace all declarations 
of local variable bsz_size from uint32_t to uint64_t

2. [PRIORITY 2] Treat burst size as a fixed/exact value for the TX burst 
(Approach 2) instead of minimal value (current code, Approach 1) for ethdev 
ports. Rationale is that some PMDs (like vector IXGBE) _might_ drop the excess 
packets in the burst.

Additional input:
1. Bruce and I looked together at the code, it looks that vector IXGBE is not 
doing this (anymore). Internally it handles packets in batches on 32 (as your 
code snippets suggest), but there is no drop of excess packets taking place.

2. Venky also suggested to keep a larger burst as a single burst (Approach 1) 
rather than break the larger burst into a fixed/constant size burst while 
buffering the excess packets until complete burst is met in the future.

Given this input and also the timing of the release, we think the best option 
is:
- urgently send a quick patch to handle the bug fixes now
- keep the existing code (burst size used as minimal burst size requirement, 
not constant) as is, at least for now, and if you feel it is not the best 
choice, we can continue to debate it for 16.7 release.
What do you think?

Jasvinder just send the bug fixing patches, hopefully they will make it into 
the 16.4 release:
http://www.dpdk.org/ml/archives/dev/2016-April/037392.html
http://www.dpdk.org/ml/archives/dev/2016-April/037393.html

Many thanks for your work on this, Robert!

Regards,
Cristian


> -Original Message-
> From: Sanford, Robert [mailto:rsanford at akamai.com]
> Sent: Friday, April 1, 2016 5:22 PM
> To: Dumitrescu, Cristian ; dev at dpdk.org
> Cc: Liang, Cunming 
> Subject: Re: [dpdk-dev] [PATCH 4/4] port: fix ethdev writer burst too big
> 
> Hi Cristian,
> 
> Please see my comments inline.
> 
> >
> >
> >> -Original Message-
> >> From: Robert Sanford [mailto:rsanford2 at gmail.com]
> >> Sent: Monday, March 28, 2016 9:52 PM
> >> To: dev at dpdk.org; Dumitrescu, Cristian  >> intel.com>
> >> Subject: [PATCH 4/4] port: fix ethdev writer burst too big
> >>
> >> For f_tx_bulk functions in rte_port_ethdev.c, we may unintentionally
> >> send bursts larger than tx_burst_sz to the underlying ethdev.
> >> Some PMDs (e.g., ixgbe) may truncate this request to their maximum
> >> burst size, resulting in unnecessary enqueuing failures or ethdev
> >> writer retries.
> >
> >Sending bursts larger than tx_burst_sz is actually intentional. The
> >assumption is that NIC performance benefits from larger burst size. So
> >the tx_burst_sz is used as a minimal burst size requirement, not as a
> >maximal or fixed burst size requirement.
> >
> >I agree with you that a while ago the vector version of IXGBE driver used
> >to work the way you describe it, but I don't think this is the case
> >anymore. As an example, if TX burst size is set to 32 and 48 packets are
> >transmitted, than the PMD will TX all the 48 packets (internally it can
> >work in batches of 4, 8, 32, etc, should not matter) rather than TXing
> >just 32 packets out of 48 and user having to either discard or retry with
> >the remaining 16 packets. I am CC-ing Steve Liang for confirming this.
> >
> >Is there any PMD that people can name that currently behaves the
> >opposite, i.e. given a burst of 48 pkts for TX, accept 32 pkts and
> >discard the other 16?
> >
> >>
> 
> Yes, I believe that IXGBE *still* truncates. What am I missing? :) My
> interpretation of the latest vector TX burst function is that it truncates
> bursts longer than txq->tx_rs_thresh. Here are relevant code snippets that
> show it lowering the number of packets (nb_pkts) to enqueue (apologies in
> advance for the email client mangling the indentation):
> 
> ---
> 
> #define IXGBE_DEFAULT_TX_RSBIT_THRESH 32
> 
> static void
> ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info
> *dev_info)
> {
>   ...
>   dev_info->default_txconf = (struct rte_eth_txconf) {
> ...
> .tx_rs_thresh = IXGBE_DEFAULT_TX_RSBIT_THRESH,
> ...
>   };
>   ...
> }
> 
> 
> uint16_t
> ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
>   uint16_t nb_pkts)
> {
>   ...
>   /* cross rx_thresh boundary is not allowed */
>   nb_pkts = RTE_MIN(nb_pkts, txq->tx_rs_thresh);
> 
>   if (txq->nb_tx_free < txq->tx_free_thresh)
> ixgbe_

[dpdk-dev] [PATCH 1/4] app/test: enhance test_port_ring_writer

[Dumitrescu, Cristian]

Hi Robert,

Sorry for my delay, I am traveling this week, I will reply as soon as I find a 
slot to focus on this, hopefully in the next couple of days, thanks for your 
patience.

Regards,
Cristian

> -Original Message-
> From: Sanford, Robert [mailto:rsanford at akamai.com]
> Sent: Friday, April 1, 2016 12:43 PM
> To: dev at dpdk.org; Dumitrescu, Cristian 
> Subject: Re: [dpdk-dev] [PATCH 1/4] app/test: enhance
> test_port_ring_writer
> 
> We don't need to change this line, because we never access more than
> RTE_PORT_IN_BURST_SIZE_MAX (64) elements in this array:
> 
> - struct rte_mbuf *mbuf[RTE_PORT_IN_BURST_SIZE_MAX];
> + struct rte_mbuf *mbuf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
> 
> 
> --
> Robert
> 
> >Add code to send two 60-packet bursts to a ring port_out.
> >This tests a ring writer buffer overflow problem and fix
> >(in patch 2/4).
> >
> >Signed-off-by: Robert Sanford 
> >---
> > app/test/test_table_ports.c |   27 +--
> > 1 files changed, 25 insertions(+), 2 deletions(-)
> >
> >diff --git a/app/test/test_table_ports.c b/app/test/test_table_ports.c
> >index 2532367..0c0ec0a 100644
> >--- a/app/test/test_table_ports.c
> >+++ b/app/test/test_table_ports.c
> >@@ -149,8 +149,8 @@ test_port_ring_writer(void)
> >
> > /* -- Traffic TX -- */
> > int expected_pkts, received_pkts;
> >-struct rte_mbuf *mbuf[RTE_PORT_IN_BURST_SIZE_MAX];
> >-struct rte_mbuf *res_mbuf[RTE_PORT_IN_BURST_SIZE_MAX];
> >+struct rte_mbuf *mbuf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
> >+struct rte_mbuf *res_mbuf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
> >
> > port_ring_writer_params.ring = RING_TX;
> > port_ring_writer_params.tx_burst_sz =
> RTE_PORT_IN_BURST_SIZE_MAX;
> >@@ -216,5 +216,28 @@ test_port_ring_writer(void)
> > for (i = 0; i < RTE_PORT_IN_BURST_SIZE_MAX; i++)
> > rte_pktmbuf_free(res_mbuf[i]);
> >
> >+/* TX Bulk - send two 60-packet bursts */
> >+uint64_t pkt_mask = 0xfff0ULL;
> >+
> >+for (i = 0; i < 4; i++)
> >+mbuf[i] = NULL;
> >+for (i = 4; i < 64; i++)
> >+mbuf[i] = rte_pktmbuf_alloc(pool);
> >+rte_port_ring_writer_ops.f_tx_bulk(port, mbuf, pkt_mask);
> >+for (i = 4; i < 64; i++)
> >+mbuf[i] = rte_pktmbuf_alloc(pool);
> >+rte_port_ring_writer_ops.f_tx_bulk(port, mbuf, pkt_mask);
> >+rte_port_ring_writer_ops.f_flush(port);
> >+
> >+expected_pkts = 2 * 60;
> >+received_pkts =
> rte_ring_sc_dequeue_burst(port_ring_writer_params.ring,
> >+(void **)res_mbuf, 2 * RTE_PORT_IN_BURST_SIZE_MAX);
> >+
> >+if (received_pkts != expected_pkts)
> >+return -10;
> >+
> >+for (i = 0; i < received_pkts; i++)
> >+rte_pktmbuf_free(res_mbuf[i]);
> >+
> > return 0;
> > }
> >--
> >1.7.1
> 
> 

[dpdk-dev] [PATCH 3/4] port: fix full burst checks in f_tx_bulk ops

[Dumitrescu, Cristian]

> -Original Message-
> From: Robert Sanford [mailto:rsanford2 at gmail.com]
> Sent: Monday, March 28, 2016 9:52 PM
> To: dev at dpdk.org; Dumitrescu, Cristian 
> Subject: [PATCH 3/4] port: fix full burst checks in f_tx_bulk ops
> 
> For several f_tx_bulk functions in rte_port_{ethdev,ring,sched}.c,
> it appears that the intent of the bsz_mask logic is to test whether
> pkts_mask contains a full burst (i.e., the  least
> significant bits are set).
> 
> There are two problems with the bsz_mask code: 1) It truncates
> by using the wrong size for local variable uint32_t bsz_mask, and

This is indeed a bug: although port->bsz_mask is always defined as uint64_t, 
there are several places where we cache it to a local variable which is defined 
as 32-bit by mistake: uint32_t bsz = p->bsz_mask. Thanks, Robert!

> 2) We may pass oversized bursts to the underlying ethdev/ring/sched,
> e.g., tx_burst_sz=16, bsz_mask=0x8000, and pkts_mask=0x1
> (17 packets), results in expr==0, and we send a burst larger than
> desired (and non-power-of-2) to the underlying tx burst interface.
> 

As stated in another related email, this is done by design, with the key 
assumption being that larger TX burst sizes will always be beneficial. So 
tx_burst_size is, by design, a requirement for the *minimal* value of the TX 
burst size rather than the *exact* value for the burst size.
As an example, when the TX burst size of 32 is set, then larger burst sizes of 
33, 34, ..., 40, 41, ..., 48, ..., 64 are welcomed and sent out as a single 
burst rather than breaking in into multiple fixed size 32-packet bursts. 
For PMDs, burst size (smaller than 64) is typically much lower than the TX ring 
size (typical value for IXGBE: 512). Same for rte_ring.

So what we are debating here is which of the following two approaches is better:
Approach 1: Consider tx_burst_sz as the minimal burst size, welcome larger 
bursts and send them as a single burst (i.e. do not break them into fixed 
tx_burst_sz bursts). This is the existing approach used consistently everywhere 
in librte_port.
Approach 2: Consider tx_burst_sz as an exact burst size requirement, any larger 
incoming burst is broken into fixed size bursts of exactly tx_burst_sz packets 
before send. This is the approach suggested by Robert.

I think we should go for the approach that gives the best performance. 
Personally, I think Approach 1 (existing) is doing this, but I would like to 
get more fact-based opinions from the people on this mail list (CC-ing a few 
key folks), especially PMD and ring maintainers. What is your experience, guys?

> We propose to effectively set bsz_mask = (1 << tx_burst_sz) - 1
> (while avoiding truncation for tx_burst_sz=64), to cache the mask
> value of a full burst, and then do a simple compare with pkts_mask
> in each f_tx_bulk.
> 
> Signed-off-by: Robert Sanford 
> ---
>  lib/librte_port/rte_port_ethdev.c |   15 ---
>  lib/librte_port/rte_port_ring.c   |   16 
>  lib/librte_port/rte_port_sched.c  |7 ++-
>  3 files changed, 10 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/librte_port/rte_port_ethdev.c
> b/lib/librte_port/rte_port_ethdev.c
> index 1c34602..3fb4947 100644
> --- a/lib/librte_port/rte_port_ethdev.c
> +++ b/lib/librte_port/rte_port_ethdev.c
> @@ -188,7 +188,7 @@ rte_port_ethdev_writer_create(void *params, int
> socket_id)
>   port->queue_id = conf->queue_id;
>   port->tx_burst_sz = conf->tx_burst_sz;
>   port->tx_buf_count = 0;
> - port->bsz_mask = 1LLU << (conf->tx_burst_sz - 1);
> + port->bsz_mask = UINT64_MAX >> (64 - conf->tx_burst_sz);

Another way to write this is: port->bsz_mask = RTE_LEN2MASK(conf->tx_burst_sz, 
uint64_t);

> 
>   return port;
>  }
> @@ -229,12 +229,9 @@ rte_port_ethdev_writer_tx_bulk(void *port,
>  {
>   struct rte_port_ethdev_writer *p =
>   (struct rte_port_ethdev_writer *) port;
> - uint32_t bsz_mask = p->bsz_mask;
>   uint32_t tx_buf_count = p->tx_buf_count;
> - uint64_t expr = (pkts_mask & (pkts_mask + 1)) |
> - ((pkts_mask & bsz_mask) ^ bsz_mask);
> 
> - if (expr == 0) {
> + if (pkts_mask == p->bsz_mask) {
>   uint64_t n_pkts = __builtin_popcountll(pkts_mask);
>   uint32_t n_pkts_ok;
> 
> @@ -369,7 +366,7 @@ rte_port_ethdev_writer_nodrop_create(void
> *params, int socket_id)
>   port->queue_id = conf->queue_id;
>   port->tx_burst_sz = conf->tx_burst_sz;
>   port->tx_buf_count = 0;
> - port->bsz_mask = 1LLU << (conf->tx_burst_sz - 1);
> + port->bsz_mask = UINT64_MAX >> (64 - conf->tx_burst_sz);
> 
>   /*
>* When 

[dpdk-dev] [PATCH 4/4] port: fix ethdev writer burst too big

[Dumitrescu, Cristian]

> -Original Message-
> From: Robert Sanford [mailto:rsanford2 at gmail.com]
> Sent: Monday, March 28, 2016 9:52 PM
> To: dev at dpdk.org; Dumitrescu, Cristian 
> Subject: [PATCH 4/4] port: fix ethdev writer burst too big
> 
> For f_tx_bulk functions in rte_port_ethdev.c, we may unintentionally
> send bursts larger than tx_burst_sz to the underlying ethdev.
> Some PMDs (e.g., ixgbe) may truncate this request to their maximum
> burst size, resulting in unnecessary enqueuing failures or ethdev
> writer retries.

Sending bursts larger than tx_burst_sz is actually intentional. The assumption 
is that NIC performance benefits from larger burst size. So the tx_burst_sz is 
used as a minimal burst size requirement, not as a maximal or fixed burst size 
requirement.

I agree with you that a while ago the vector version of IXGBE driver used to 
work the way you describe it, but I don't think this is the case anymore. As an 
example, if TX burst size is set to 32 and 48 packets are transmitted, than the 
PMD will TX all the 48 packets (internally it can work in batches of 4, 8, 32, 
etc, should not matter) rather than TXing just 32 packets out of 48 and user 
having to either discard or retry with the remaining 16 packets. I am CC-ing 
Steve Liang for confirming this.

Is there any PMD that people can name that currently behaves the opposite, i.e. 
given a burst of 48 pkts for TX, accept 32 pkts and discard the other 16?

> 
> We propose to fix this by moving the tx buffer flushing logic from
> *after* the loop that puts all packets into the tx buffer, to *inside*
> the loop, testing for a full burst when adding each packet.
> 

The issue I have with this approach is the introduction of a branch that has to 
be tested for each iteration of the loop rather than once for the entire loop.

The code branch where you add this is actually the slow(er) code path (where 
local variable expr != 0), which is used for non-contiguous or bursts smaller 
than tx_burst_sz. Is there a particular reason you are only interested of 
enabling this strategy (of using tx_burst_sz as a fixed burst size requirement) 
only on this code path? The reason I am asking is the other fast(er) code path 
(where expr == 0) also uses tx_burst_sz as a minimal requirement and therefore 
it can send burst sizes bigger than tx_burst_sz.


> Signed-off-by: Robert Sanford 
> ---
>  lib/librte_port/rte_port_ethdev.c |   20 ++--
>  1 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/librte_port/rte_port_ethdev.c
> b/lib/librte_port/rte_port_ethdev.c
> index 3fb4947..1283338 100644
> --- a/lib/librte_port/rte_port_ethdev.c
> +++ b/lib/librte_port/rte_port_ethdev.c
> @@ -151,7 +151,7 @@ static int rte_port_ethdev_reader_stats_read(void
> *port,
>  struct rte_port_ethdev_writer {
>   struct rte_port_out_stats stats;
> 
> - struct rte_mbuf *tx_buf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
> + struct rte_mbuf *tx_buf[RTE_PORT_IN_BURST_SIZE_MAX];
>   uint32_t tx_burst_sz;
>   uint16_t tx_buf_count;
>   uint64_t bsz_mask;
> @@ -257,11 +257,11 @@ rte_port_ethdev_writer_tx_bulk(void *port,
>   p->tx_buf[tx_buf_count++] = pkt;
> 
>   RTE_PORT_ETHDEV_WRITER_STATS_PKTS_IN_ADD(p, 1);
>   pkts_mask &= ~pkt_mask;
> - }
> 
> - p->tx_buf_count = tx_buf_count;
> - if (tx_buf_count >= p->tx_burst_sz)
> - send_burst(p);
> + p->tx_buf_count = tx_buf_count;
> + if (tx_buf_count >= p->tx_burst_sz)
> + send_burst(p);
> + }
>   }

One observation here: if we enable this proposal (which I have an issue with 
due to the executing the branch per loop iteration rather than once per entire 
loop), it also eliminates the buffer overflow issue flagged by you in the other 
email :), so no need to e.g. doble the size of the port internal buffer 
(tx_buf).

> 
>   return 0;
> @@ -328,7 +328,7 @@ static int rte_port_ethdev_writer_stats_read(void
> *port,
>  struct rte_port_ethdev_writer_nodrop {
>   struct rte_port_out_stats stats;
> 
> - struct rte_mbuf *tx_buf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
> + struct rte_mbuf *tx_buf[RTE_PORT_IN_BURST_SIZE_MAX];
>   uint32_t tx_burst_sz;
>   uint16_t tx_buf_count;
>   uint64_t bsz_mask;
> @@ -466,11 +466,11 @@ rte_port_ethdev_writer_nodrop_tx_bulk(void
> *port,
>   p->tx_buf[tx_buf_count++] = pkt;
> 
>   RTE_PORT_ETHDEV_WRITER_NODROP_STATS_PKTS_IN_ADD(p, 1);
>   pkts_mask &= ~pkt_mask;
> - }
> 
> - p->tx_buf_count = tx_buf_count;
> -

[dpdk-dev] [PATCH 2/4] port: fix ring writer buffer overflow

[Dumitrescu, Cristian]

> -Original Message-
> From: Robert Sanford [mailto:rsanford2 at gmail.com]
> Sent: Monday, March 28, 2016 9:52 PM
> To: dev at dpdk.org; Dumitrescu, Cristian 
> Subject: [PATCH 2/4] port: fix ring writer buffer overflow
> 
> Ring writer tx_bulk functions may write past the end of tx_buf[].
> Solution is to double the size of tx_buf[].
> 
> Signed-off-by: Robert Sanford 
> ---
>  lib/librte_port/rte_port_ring.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_port/rte_port_ring.c b/lib/librte_port/rte_port_ring.c
> index b847fea..765ecc5 100644
> --- a/lib/librte_port/rte_port_ring.c
> +++ b/lib/librte_port/rte_port_ring.c
> @@ -179,7 +179,7 @@ rte_port_ring_reader_stats_read(void *port,
>  struct rte_port_ring_writer {
>   struct rte_port_out_stats stats;
> 
> - struct rte_mbuf *tx_buf[RTE_PORT_IN_BURST_SIZE_MAX];
> + struct rte_mbuf *tx_buf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
>   struct rte_ring *ring;
>   uint32_t tx_burst_sz;
>   uint32_t tx_buf_count;
> @@ -447,7 +447,7 @@ rte_port_ring_writer_stats_read(void *port,
>  struct rte_port_ring_writer_nodrop {
>   struct rte_port_out_stats stats;
> 
> - struct rte_mbuf *tx_buf[RTE_PORT_IN_BURST_SIZE_MAX];
> + struct rte_mbuf *tx_buf[2 * RTE_PORT_IN_BURST_SIZE_MAX];
>   struct rte_ring *ring;
>   uint32_t tx_burst_sz;
>   uint32_t tx_buf_count;
> --
> 1.7.1

Hi Robert,

How is the buffer overflow taking place?

After looking long and hard, I spotted that buffer overflow can potentially 
take place when the following conditions are met:
1. The input packet burst does not meet the conditions of (a) being contiguous 
(first n bits set in pkts_mask, all the other bits cleared) and (b) containing 
a full burst, i.e. at least tx_burst_sz packets (n >= tx_burst_size). This is 
the slow(er) code path taken when local variable expr != 0.
2. There are some packets already in the buffer.
3. The number of packets in the incoming burst (i.e. popcount(pkts_mask)) plus 
the number of packets already in the buffer exceeds the buffer size 
(RTE_PORT_IN_BURST_SIZE_MAX, i.e. 64).

Is this the buffer overflow scenario that you detected?

Thanks,
Cristian

[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

[Dumitrescu, Cristian]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, March 30, 2016 4:50 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ; Zhang,
> Roy Fan ; Hunt, David 
> Subject: Re: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> 
> 2016-03-30 14:15, Dumitrescu, Cristian:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-03-30 13:57, Dumitrescu, Cristian:
> > > > I think the correct fix is:
> > > > #if defined(__x86_64__) &&
> (defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> > > || defined(RTE_MACHINE_CPUFLAG_CRC32))
> > > >
> > > > We'll test it and send a patch asap.
> > >
> > > I had prepared this patch. Please be inspired:
> > >
> > > examples/ip_pipeline: fix SSE4.2 optimization branch
> > >
> > > The branch was disabled because of a typo in the SSE4.2 flag.
> > > Change also the x86_64 flag to use a DPDK one.
> > >
> > > Fixes: 28377375c6c0 ("examples/ip_pipeline: fix build for x86_64
> without
> > > SSE4.2")
> > >
> > > -#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> > > +#if defined(RTE_ARCH_X86_64) &&
> > > defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> >
> > Acked-by: Cristian Dumitrescu 
> 
> I thought you wanted to send a patch with another CPUFLAG (CRC32)?

The extra flag is good, but not absolutely required, as SSE4.2 implies support 
for CRC32 instruction.

The CRC32 flag might be useful when a CPU architecture other than Intel 
supports the CRC32 instruction, but I am not sure whether such CPU architecture 
exists. Anyway, the SSE4.2 || CRC32 pattern is already present in several DPDK 
files, so I was looking to observe it as well.

I thought you considered the CRC32 flag to be redundant and decided to remove 
it on purpose.

I am OK if you want to go ahead with your patch right now, otherwise we can 
send a patch tomorrow?

Thanks,
Cristian

[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

[Dumitrescu, Cristian]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, March 30, 2016 3:07 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ; Zhang,
> Roy Fan ; Hunt, David 
> Subject: Re: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> 
> 2016-03-30 13:57, Dumitrescu, Cristian:
> > I think the correct fix is:
> > #if defined(__x86_64__) && (defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> || defined(RTE_MACHINE_CPUFLAG_CRC32))
> >
> > We'll test it and send a patch asap.
> 
> I had prepared this patch. Please be inspired:
> 
> examples/ip_pipeline: fix SSE4.2 optimization branch
> 
> The branch was disabled because of a typo in the SSE4.2 flag.
> Change also the x86_64 flag to use a DPDK one.
> 
> Fixes: 28377375c6c0 ("examples/ip_pipeline: fix build for x86_64 without
> SSE4.2")
> 
> -#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> +#if defined(RTE_ARCH_X86_64) &&
> defined(RTE_MACHINE_CPUFLAG_SSE4_2)

Acked-by: Cristian Dumitrescu 

[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

[Dumitrescu, Cristian]

> -Original Message-
> From: Dumitrescu, Cristian
> Sent: Wednesday, March 30, 2016 2:24 PM
> To: 'Thomas Monjalon' ; dev at dpdk.org
> Cc: Singh, Jasvinder ; Zhang, Roy Fan
> ; Hunt, David 
> Subject: RE: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> Importance: High
> 
> 
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Tuesday, February 16, 2016 6:46 AM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> > x86_64 without SSE4.2
> >
> > The compiler cannot use _mm_crc32_u64:
> >
> > examples/ip_pipeline/pipeline/hash_func.h:165:9:
> > error: implicit declaration of function '_mm_crc32_u64' is invalid in C99
> >
> > Fixes: 947024a26df7 ("examples/ip_pipeline: rework passthrough
> pipeline")
> >
> > Signed-off-by: Thomas Monjalon 
> > ---
> >  examples/ip_pipeline/pipeline/hash_func.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/examples/ip_pipeline/pipeline/hash_func.h
> > b/examples/ip_pipeline/pipeline/hash_func.h
> > index 7846300..1953ad4 100644
> > --- a/examples/ip_pipeline/pipeline/hash_func.h
> > +++ b/examples/ip_pipeline/pipeline/hash_func.h
> > @@ -152,7 +152,7 @@ hash_xor_key64(void *key, __rte_unused uint32_t
> > key_size, uint64_t seed)
> > return (xor0 >> 32) ^ xor0;
> >  }
> >
> > -#if defined(__x86_64__)
> > +#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> >
> >  #include 
> >
> > --
> > 2.7.0
> 
> Hi Thomas,
> 
> This is not the correct fix, as RTE_CPUFLAG_SSE4_2 is a flag that can only be
> tested at run-time (as result of calling function rte_cpu_get_flag_enabled()),
> not at build-time.
> 
> The reason it appears to fix the build issue you are mentioning is the fact 
> that
> this change results in disabling the  __x86_64__ code branch.
> 
> We need to revert this patch and look for a better option.
> 
> What is the compiler that generates the build issue you are mentioning? We
> could not reproduce it with gcc 5 (gcc 5.3.1).
> 
> Thanks,
> Cristian

I think the correct fix is:
#if defined(__x86_64__) && (defined(RTE_MACHINE_CPUFLAG_SSE4_2) || 
defined(RTE_MACHINE_CPUFLAG_CRC32))

We'll test it and send a patch asap.

Thanks,
Cristian

[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

[Dumitrescu, Cristian]

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, February 16, 2016 6:46 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> 
> The compiler cannot use _mm_crc32_u64:
> 
> examples/ip_pipeline/pipeline/hash_func.h:165:9:
> error: implicit declaration of function '_mm_crc32_u64' is invalid in C99
> 
> Fixes: 947024a26df7 ("examples/ip_pipeline: rework passthrough pipeline")
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  examples/ip_pipeline/pipeline/hash_func.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/ip_pipeline/pipeline/hash_func.h
> b/examples/ip_pipeline/pipeline/hash_func.h
> index 7846300..1953ad4 100644
> --- a/examples/ip_pipeline/pipeline/hash_func.h
> +++ b/examples/ip_pipeline/pipeline/hash_func.h
> @@ -152,7 +152,7 @@ hash_xor_key64(void *key, __rte_unused uint32_t
> key_size, uint64_t seed)
>   return (xor0 >> 32) ^ xor0;
>  }
> 
> -#if defined(__x86_64__)
> +#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> 
>  #include 
> 
> --
> 2.7.0

Hi Thomas,

This is not the correct fix, as RTE_CPUFLAG_SSE4_2 is a flag that can only be 
tested at run-time (as result of calling function rte_cpu_get_flag_enabled()), 
not at build-time.

The reason it appears to fix the build issue you are mentioning is the fact 
that this change results in disabling the  __x86_64__ code branch.

We need to revert this patch and look for a better option.

What is the compiler that generates the build issue you are mentioning? We 
could not reproduce it with gcc 5 (gcc 5.3.1).

Thanks,
Cristian