date:20160825

Re: [lng-odp] [PATCH v8] example: introducing l3fwd

2016-08-25 Thread forrest.shi

Hi Maxim,

I will send another patch to resolve the hash collision issue.

BTW: did you received my resend v8, this time is TO: lng-odp@ lists.linaro.org?
I received it but not see on patch list.

Thanks,
Forrest

> -Original Message-
> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
> Sent: Thursday, August 25, 2016 16:16
> To: forrest.shi 
> Cc: lng-odp 
> Subject: Re: [lng-odp] [PATCH v8] example: introducing l3fwd
> 
> On 08/25/16 04:53, forrest.shi wrote:
> > Hi Maxim,
> >
> > I CCed lng-odp@lists.linaro.org, but not show on
> > http://patches.opendataplane.org/project/lng-odp/list/
> >
> 
> Might me list can not be in CC. Does hash collistion needs to be
> resolved before inclusion?
> 
> Maxim.
> 
> 
> > I will resend it.
> >
> > thanks,
> >
> > Forrest
> >
> >
> > 在 8/24/2016 6:03 PM, Maxim Uvarov 写道:
> >> was v8 send to mailing list?
> >>
> >> I do not see it here:
> >> http://patches.opendataplane.org/project/lng-odp/list/
> >>
> >>
> >> On 08/23/16 19:09, Mike Holmes wrote:
> >>> On 23 August 2016 at 09:28, Elo, Matias (Nokia - FI/Espoo) <
> >>> matias@nokia-bell-labs.com> wrote:
> >>>
> > Would you please have a review on this version?
> >
> > It should fix the TX queue binding issue.
> >
> > Each thread has one tx queue for each port.
> >
>    Hi Forrest,
> 
>  Now the throughput scales as it should. The hash IP lookup
>  performance is
>  still bad
>  (~3K packets per second) but in my opinion this can be fixed in
>  another
>  commit.
> 
> >>> If we take this into the repo, which sounds like a good plan so we can
> >>> establish it in CI, we should make a bug for the IP lookup
> >>> performance to
> >>> ensure that gets addressed.
> >>>
> >>>
>  Reviewed-and-tested-by: Matias Elo 
> 
> > thanks,
> >
> > Forrest
> >
> >
> > 在 8/22/2016 3:32 PM, forrest@linaro.org 写道:
> >> From: Xuelin Shi 
> >>
> >> multi-thread, multi-queues and bi-directional forwarding.
> >>
> >> support (port, queue, thread) arguments in cmdline which specify how
> >> the threads handle which rx queue at which port, if no this
> >> argument,
> >> default specification used.
> >>
> >> both hash and lpm based lookup methods are supported, default lpm.
> >>
> >> Signed-off-by: Xuelin Shi 
> >> ---
> >>example/Makefile.am   |   12 +-
> >>example/l3fwd/.gitignore  |1 +
> >>example/l3fwd/Makefile.am |   12 +
> >>example/l3fwd/odp_l3fwd.c | 1089
> > +
> >>example/l3fwd/odp_l3fwd_db.c  | 432 
> >>example/l3fwd/odp_l3fwd_db.h  |  136 +
> >>example/l3fwd/odp_l3fwd_lpm.c |  224 +
> >>example/l3fwd/odp_l3fwd_lpm.h |   20 +
> >>example/m4/configure.m4   |9 +-
> >>9 files changed, 1930 insertions(+), 5 deletions(-)
> >>create mode 100644 example/l3fwd/.gitignore
> >>create mode 100644 example/l3fwd/Makefile.am
> >>create mode 100644 example/l3fwd/odp_l3fwd.c
> >>create mode 100644 example/l3fwd/odp_l3fwd_db.c
> >>create mode 100644 example/l3fwd/odp_l3fwd_db.h
> >>create mode 100644 example/l3fwd/odp_l3fwd_lpm.c
> >>create mode 100644 example/l3fwd/odp_l3fwd_lpm.h
> >>
> >> diff --git a/example/Makefile.am b/example/Makefile.am
> >> index 37542af..dfc07b6 100644
> >> --- a/example/Makefile.am
> >> +++ b/example/Makefile.am
> >> @@ -1 +1,11 @@
> >> -SUBDIRS = classifier generator ipsec packet time timer traffic_mgmt
> > l2fwd_simple switch hello
> >> +SUBDIRS = classifier \
> >> + generator \
> >> + hello \
> >> + ipsec \
> >> + l2fwd_simple \
> >> + l3fwd \
> >> + packet \
> >> + switch \
> >> + time \
> >> + timer \
> >> + traffic_mgmt
> >> diff --git a/example/l3fwd/.gitignore b/example/l3fwd/.gitignore
> >> new file mode 100644
> >> index 000..74e501f
> >> --- /dev/null
> >> +++ b/example/l3fwd/.gitignore
> >> @@ -0,0 +1 @@
> >> +odp_l3fwd
> >> diff --git a/example/l3fwd/Makefile.am
> b/example/l3fwd/Makefile.am
> >> new file mode 100644
> >> index 000..f926717
> >> --- /dev/null
> >> +++ b/example/l3fwd/Makefile.am
> >> @@ -0,0 +1,12 @@
> >> +include $(top_srcdir)/example/Makefile.inc
> >> +
> >> +bin_PROGRAMS = odp_l3fwd$(EXEEXT)
> >> +odp_l3fwd_LDFLAGS = $(AM_LDFLAGS) -static
> >> +odp_l3fwd_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example -
> > I${top_srcdir}/test
> >> +
> >> +noinst_HEADERS = \
> >> + $(top_srcdir)/example/l3fwd/odp_l3fwd_db.h \
> >> +

[lng-odp] [Bug 2449] Packet queue pool causes out-of-bounds accesses

2016-08-25 Thread bugzilla-daemon

https://bugs.linaro.org/show_bug.cgi?id=2449

Bill Fischofer  changed:

   What|Removed |Added

 Status|IN_PROGRESS |RESOLVED
 Resolution|--- |FIXED

--- Comment #5 from Bill Fischofer  ---
Merged. Commit ID 6e95b8923cd013f9db4d7198a2add5cc4cdc9e28

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [lng-odp] [PATCH] linux-generic: tm: fix off by 1 bug accessing queue_num_tbl

2016-08-25 Thread Maxim Uvarov


Merged,
Maxim.

On 08/24/16 19:17, Bill Fischofer wrote:

On Tue, Aug 23, 2016 at 4:51 PM, Barry Spinney  wrote:


Both odp_pkt_queue.c and odp_traffic_mngr.c has similar bugs where
an array was malloc'd but the valid indicies run from 1 to the number
of entries malloc'd.  This is because queue_num == 0 is used as a special
value indicating an invalid queue_num.  The value 0 is used for this
same special purpose throughout the traffic mngr.
The fix involved using "queue_num - 1" as the array index, but only
after checking that queue_num is not 0.  Also in a few cases a common
function used to get and check the queue_num was used in a few more
cases.

Signed-off-by: Barry Spinney 


Reviewed-and-tested-by: Bill Fischofer 



---
  platform/linux-generic/odp_pkt_queue.c| 12 ++--
  platform/linux-generic/odp_traffic_mngr.c | 26 +++---
  2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/platform/linux-generic/odp_pkt_queue.c
b/platform/linux-generic/odp_pkt_queue.c
index 7734ee9..7c6cd87 100644
--- a/platform/linux-generic/odp_pkt_queue.c
+++ b/platform/linux-generic/odp_pkt_queue.c
@@ -92,7 +92,7 @@ static int pkt_queue_free_list_add(queue_pool_t *pool,
 queue_blks_t *queue_blks;
 queue_blk_t *queue_blk;
 uint32_t which_region, blks_added, num_blks, start_idx;
-   uint32_t malloc_len, blks_to_add, cnt;
+   uint32_t malloc_len, blks_to_add, cnt, i;

 which_region = pool->current_region;
 blks_added = 0;
@@ -102,7 +102,6 @@ static int pkt_queue_free_list_add(queue_pool_t *pool,
 num_blks = region_desc->num_blks;
 queue_blks = region_desc->queue_blks;
 if (!queue_blks) {
-   uint32_t i;
 malloc_len = num_blks * sizeof(queue_blk_t);
 queue_blks = malloc(malloc_len);

@@ -263,13 +262,13 @@ int _odp_pkt_queue_append(_odp_int_queue_pool_t
queue_pool,
 return -3;

 pool->total_pkt_appends++;
-   first_blk_idx = pool->queue_num_tbl[queue_num];
+   first_blk_idx = pool->queue_num_tbl[queue_num - 1];
 if (first_blk_idx == 0) {
 first_blk = queue_blk_alloc(pool, _blk_idx);
 if (!first_blk)
 return -1;

-   pool->queue_num_tbl[queue_num] = first_blk_idx;
+   pool->queue_num_tbl[queue_num - 1] = first_blk_idx;
 init_queue_blk(first_blk);
 first_blk->pkts[0] = pkt;
 return 0;
@@ -316,7 +315,7 @@ int _odp_pkt_queue_remove(_odp_int_queue_pool_t
queue_pool,
 if ((queue_num == 0) || (pool->max_queue_num < queue_num))
 return -2;

-   first_blk_idx = pool->queue_num_tbl[queue_num];
+   first_blk_idx = pool->queue_num_tbl[queue_num - 1];
 if (first_blk_idx == 0)
 return 0; /* pkt queue is empty. */

@@ -344,7 +343,8 @@ int _odp_pkt_queue_remove(_odp_int_queue_pool_t
queue_pool,

first_blk->tail_queue_blk_idx;
 }

-   pool->queue_num_tbl[queue_num] =
next_blk_idx;
+   pool->queue_num_tbl[queue_num - 1] =
+   next_blk_idx;
 queue_blk_free(pool, first_blk,
first_blk_idx);
 }

diff --git a/platform/linux-generic/odp_traffic_mngr.c
b/platform/linux-generic/odp_traffic_mngr.c
index a5271ed..4fe07ef 100644
--- a/platform/linux-generic/odp_traffic_mngr.c
+++ b/platform/linux-generic/odp_traffic_mngr.c
@@ -109,7 +109,7 @@ static tm_queue_obj_t *get_tm_queue_obj(tm_system_t
*tm_system,
 return NULL;

 queue_num= pkt_desc->queue_num;
-   tm_queue_obj = tm_system->queue_num_tbl[queue_num];
+   tm_queue_obj = tm_system->queue_num_tbl[queue_num - 1];
 return tm_queue_obj;
  }

@@ -1596,11 +1596,10 @@ static odp_bool_t tm_consume_sent_pkt(tm_system_t
*tm_system,
 tm_queue_obj_t *tm_queue_obj;
 odp_packet_t pkt;
 pkt_desc_t *new_pkt_desc;
-   uint32_t queue_num, pkt_len;
+   uint32_t pkt_len;
 int rc;

-   queue_num = sent_pkt_desc->queue_num;
-   tm_queue_obj = tm_system->queue_num_tbl[queue_num];
+   tm_queue_obj = get_tm_queue_obj(tm_system, sent_pkt_desc);
 if (!tm_queue_obj)
 return false;

@@ -2088,15 +2087,11 @@ static void tm_send_pkt(tm_system_t *tm_system,
uint32_t max_sends)
 tm_queue_obj_t *tm_queue_obj;
 odp_packet_t odp_pkt;
 pkt_desc_t *pkt_desc;
-   uint32_t cnt, queue_num;
+   uint32_t cnt;

 for (cnt = 1; cnt <= max_sends; cnt++) {
 pkt_desc = _system->egress_pkt_desc;
-   queue_num = pkt_desc->queue_num;
-   if (queue_num == 0)
-

[lng-odp] [Bug 2444] Can not use two PKTIOs with Traffic Manager

2016-08-25 Thread bugzilla-daemon

https://bugs.linaro.org/show_bug.cgi?id=2444

Bill Fischofer  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|IN_PROGRESS |RESOLVED

--- Comment #8 from Bill Fischofer  ---
Oriol confirms this is fixed, so closing.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[lng-odp] [Bug 2449] Packet queue pool causes out-of-bounds accesses

2016-08-25 Thread bugzilla-daemon

https://bugs.linaro.org/show_bug.cgi?id=2449

Bill Fischofer  changed:

   What|Removed |Added

   Assignee|lng-odp@lists.linaro.org|bill.fischo...@linaro.org

--- Comment #4 from Bill Fischofer  ---
Patch has been reviewed and is awaiting merge.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are on the CC list for the bug.

Re: [lng-odp] [PATCHv2] test: linux-gen: add pcap playback test

2016-08-25 Thread Bill Fischofer

On Thu, Aug 25, 2016 at 3:14 AM, Maxim Uvarov 
wrote:

> On 08/25/16 00:34, Anders Roxell wrote:
>
>> +#include 
>
 > >why do we include an internal header file?
 > >

>>> >
>>> >because we need access to classifier bits, that is why this test is
>>> under
>>> >linux-generic.
>>>
>> OK, but do we really have to fiddle with internal classifier bits?
>>
>>
> That is good question. For linux-generic it's bit's for other platform it
> might be something else.
> I would like to change input_flags_t (linux-generic internal) to something
> else which might be common
> for all platforms. From other point of view if bit field will be changed
> in linux-generic we should not
> loose testing functionality. I'm not sure that all platforms support now
> pcap so that removing internal
> header might be not needed as this is internal platform test. When we will
> move to common then
> of course we need to remove internals. How about make it separate patch?


As I recall, Matias reorganized those bits a couple of months back to get
some measurable performance gains, so I'd be cautious about making changes
just for 'tidiness' without measuring any impact


>
>
> Maxim.
>
>
>

Re: [lng-odp] [PATCH v8] example: introducing l3fwd

2016-08-25 Thread Maxim Uvarov


On 08/25/16 04:53, forrest.shi wrote:

Hi Maxim,

I CCed lng-odp@lists.linaro.org, but not show on 
http://patches.opendataplane.org/project/lng-odp/list/




Might me list can not be in CC. Does hash collistion needs to be 
resolved before inclusion?


Maxim.



I will resend it.

thanks,

Forrest


在 8/24/2016 6:03 PM, Maxim Uvarov 写道:

was v8 send to mailing list?

I do not see it here:
http://patches.opendataplane.org/project/lng-odp/list/


On 08/23/16 19:09, Mike Holmes wrote:

On 23 August 2016 at 09:28, Elo, Matias (Nokia - FI/Espoo) <
matias@nokia-bell-labs.com> wrote:


Would you please have a review on this version?

It should fix the TX queue binding issue.

Each thread has one tx queue for each port.


  Hi Forrest,

Now the throughput scales as it should. The hash IP lookup 
performance is

still bad
(~3K packets per second) but in my opinion this can be fixed in 
another

commit.


If we take this into the repo, which sounds like a good plan so we can
establish it in CI, we should make a bug for the IP lookup 
performance to

ensure that gets addressed.



Reviewed-and-tested-by: Matias Elo 


thanks,

Forrest


在 8/22/2016 3:32 PM, forrest@linaro.org 写道:

From: Xuelin Shi 

multi-thread, multi-queues and bi-directional forwarding.

support (port, queue, thread) arguments in cmdline which specify how
the threads handle which rx queue at which port, if no this 
argument,

default specification used.

both hash and lpm based lookup methods are supported, default lpm.

Signed-off-by: Xuelin Shi 
---
   example/Makefile.am   |   12 +-
   example/l3fwd/.gitignore  |1 +
   example/l3fwd/Makefile.am |   12 +
   example/l3fwd/odp_l3fwd.c | 1089

+

   example/l3fwd/odp_l3fwd_db.c  | 432 
   example/l3fwd/odp_l3fwd_db.h  |  136 +
   example/l3fwd/odp_l3fwd_lpm.c |  224 +
   example/l3fwd/odp_l3fwd_lpm.h |   20 +
   example/m4/configure.m4   |9 +-
   9 files changed, 1930 insertions(+), 5 deletions(-)
   create mode 100644 example/l3fwd/.gitignore
   create mode 100644 example/l3fwd/Makefile.am
   create mode 100644 example/l3fwd/odp_l3fwd.c
   create mode 100644 example/l3fwd/odp_l3fwd_db.c
   create mode 100644 example/l3fwd/odp_l3fwd_db.h
   create mode 100644 example/l3fwd/odp_l3fwd_lpm.c
   create mode 100644 example/l3fwd/odp_l3fwd_lpm.h

diff --git a/example/Makefile.am b/example/Makefile.am
index 37542af..dfc07b6 100644
--- a/example/Makefile.am
+++ b/example/Makefile.am
@@ -1 +1,11 @@
-SUBDIRS = classifier generator ipsec packet time timer traffic_mgmt

l2fwd_simple switch hello

+SUBDIRS = classifier \
+ generator \
+ hello \
+ ipsec \
+ l2fwd_simple \
+ l3fwd \
+ packet \
+ switch \
+ time \
+ timer \
+ traffic_mgmt
diff --git a/example/l3fwd/.gitignore b/example/l3fwd/.gitignore
new file mode 100644
index 000..74e501f
--- /dev/null
+++ b/example/l3fwd/.gitignore
@@ -0,0 +1 @@
+odp_l3fwd
diff --git a/example/l3fwd/Makefile.am b/example/l3fwd/Makefile.am
new file mode 100644
index 000..f926717
--- /dev/null
+++ b/example/l3fwd/Makefile.am
@@ -0,0 +1,12 @@
+include $(top_srcdir)/example/Makefile.inc
+
+bin_PROGRAMS = odp_l3fwd$(EXEEXT)
+odp_l3fwd_LDFLAGS = $(AM_LDFLAGS) -static
+odp_l3fwd_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example -

I${top_srcdir}/test

+
+noinst_HEADERS = \
+ $(top_srcdir)/example/l3fwd/odp_l3fwd_db.h \
+ $(top_srcdir)/example/l3fwd/odp_l3fwd_lpm.h \
+ $(top_srcdir)/example/example_debug.h
+
+dist_odp_l3fwd_SOURCES = odp_l3fwd.c odp_l3fwd_db.c odp_l3fwd_lpm.c
diff --git a/example/l3fwd/odp_l3fwd.c b/example/l3fwd/odp_l3fwd.c
new file mode 100644
index 000..efe0382
--- /dev/null
+++ b/example/l3fwd/odp_l3fwd.c
@@ -0,0 +1,1089 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "odp_l3fwd_db.h"
+#include "odp_l3fwd_lpm.h"
+
+#define POOL_NUM_PKT   8192
+#define POOL_SEG_LEN   1856
+#define MAX_PKT_BURST  32
+
+#define MAX_NB_WORKER  32
+#define MAX_NB_PKTIO   32
+#define MAX_NB_QUEUE   32
+#define MAX_NB_QCONFS  1024
+#define MAX_NB_ROUTE   32
+
+#define INVALID_ID (-1)
+#define PRINT_INTERVAL 10  /* interval seconds of printing

stats */

+
+/** Get rid of path in filename - only for unix-type paths using 
'/'

*/

+#define NO_PATH(file_name) (strrchr((file_name), '/') ? \
+   strrchr((file_name), '/') + 1 : (file_name))
+
+struct l3fwd_pktio_s {
+   odp_pktio_t pktio;
+   odph_ethaddr_t mac_addr;
+   odp_pktin_queue_t ifin[MAX_NB_QUEUE];
+   odp_pktout_queue_t ifout[MAX_NB_QUEUE];
+   int nb_rxq; /* capa max */
+   int nb_txq;

Re: [lng-odp] [PATCHv2] test: linux-gen: add pcap playback test

2016-08-25 Thread Maxim Uvarov

On 08/25/16 00:34, Anders Roxell wrote:

+#include 

> >why do we include an internal header file?
> >

>
>because we need access to classifier bits, that is why this test is under
>linux-generic.

OK, but do we really have to fiddle with internal classifier bits?

That is good question. For linux-generic it's bit's for other platform 
it might be something else.
I would like to change input_flags_t (linux-generic internal) to 
something else which might be common
for all platforms. From other point of view if bit field will be changed 
in linux-generic we should not
loose testing functionality. I'm not sure that all platforms support now 
pcap so that removing internal
header might be not needed as this is internal platform test. When we 
will move to common then

of course we need to remove internals. How about make it separate patch?

Maxim.

Re: [lng-odp] [PATCH v8] example: introducing l3fwd

[lng-odp] [Bug 2449] Packet queue pool causes out-of-bounds accesses

Re: [lng-odp] [PATCH] linux-generic: tm: fix off by 1 bug accessing queue_num_tbl

[lng-odp] [Bug 2444] Can not use two PKTIOs with Traffic Manager

[lng-odp] [Bug 2449] Packet queue pool causes out-of-bounds accesses

Re: [lng-odp] [PATCHv2] test: linux-gen: add pcap playback test

Re: [lng-odp] [PATCH v8] example: introducing l3fwd

Re: [lng-odp] [PATCHv2] test: linux-gen: add pcap playback test

8 matches

Site Navigation

Mail list logo

Footer information