Hi,

I guess it should be reasonably simple to implement a test that doesn’t check 
the packet rate statistics but merely checks for segfault. My testing with 
killing processes in the background showed that “wait” returns 143 for 
processes killed with “kill”, and 139 for processes that segfaulted. Such a 
test would add no additional lines to l2fwd_simple. The test implemented as a 
shell script unfortunately would depend on the signal name to signal number 
mappings, but I guess those are stable.

I’ll see if I have time to implement such test. Might take a while. If I do, it 
will be a separate patch in the patch set.

Anyway, thanks for the review comments!

From: EXT Mike Holmes [mailto:[email protected]]
Sent: Tuesday, February 16, 2016 5:56 PM
To: Maxim Uvarov
Cc: Tilli, Juha-Matti (Nokia - FI/Espoo); [email protected]
Subject: Re: [lng-odp] [API-NEXT PATCH] example: l2fwd_simple: add 
single-threaded l2fwd example



On 16 February 2016 at 10:07, Maxim Uvarov 
<[email protected]<mailto:[email protected]>> wrote:
1. That can be in separate patch.

2. I don't think that we need to add time. I think it might be something 
simple. Or example seg. faults on traffic processing or it works.
I don't like having examples which might be not working.

This is a key issue, we need to extend make check to run these. Examples that 
go stale are not helping us and CI is not checking that they are working.


As I remember odp_generator can send and receive packets. Then test might be
a) run l2fwd_simple in background;
b) run odp_generator to send and receive packets
c) kill l2wfd_simple. So pass is packets were transmitted, fail on any other 
error (like process died before kill.).

Maxim.



On 02/16/16 17:16, Tilli, Juha-Matti (Nokia - FI/Espoo) wrote:
Hi,

I briefly considered whether there should be a test case for that example.

The main trouble is that the current l2fwd test case uses the option of l2fwd 
to run for only 30 seconds and also uses the output of l2fwd to parse the 
packets per second information.

If the current program should have a test, I can't use the l2fwd test as an 
example, because my program doesn't have the option to run for only 30 seconds 
and also doesn't print the packets per second information to stdout.

So, the test infrastructure either needs a major overhaul (i.e. a generator 
program that can actually receive the generated packets too and figure out the 
packets per second statistics from those) or alternatively the proposed program 
should be modified to have an option of running for only 30 seconds and also 
statistics printing to stdout. If those features really need to be added to the 
proposed program, I don't see the point of having a simple version of l2fwd 
anymore. It would become yet another l2fwd, and having two copies of the same 
program in the repository is obviously a no-no.

The whole point of having l2fwd_simple is to keep it simple to get new ODP 
users up to speed quickly. I'm not going to add another hundred lines or two 
just to make it possible to test it with a similar test.

Sadly, this means that l2fwd_simple is untested as of now. If somebody want to 
do the effort of improving the test infrastructure to figure out the packets 
per second rates from the generator program, that should probably be done in a 
separate patch. And the test for the original l2fwd could be improved, too: it 
could be tested with a similar external PPS observation instead of parsing its 
output. But that sounds like a project for another day.

-----Original Message-----
From: lng-odp 
[mailto:[email protected]<mailto:[email protected]>]
 On Behalf Of EXT Maxim Uvarov
Sent: Tuesday, February 16, 2016 2:24 PM
To: [email protected]<mailto:[email protected]>
Subject: Re: [lng-odp] [API-NEXT PATCH] example: l2fwd_simple: add 
single-threaded l2fwd example

it will be good to add test case for that example. The same as we do for
original l2fwd.
make check is very useful for examples.

Maxim.

On 02/12/16 16:54, Juha-Matti Tilli wrote:
The ODP distribution is lacking a simple example how to receive and send
packets. The l2fwd is overly complicated because it supports an arbitrary
number of interfaces, bi-directional operation and an arbitrary number of
threads.

To remedy this situation, add l2fwd_simple which is a single-threaded
unidirectional variant of l2fwd for exactly two interfaces. l2fwd_simple
can be used as an example application when learning how to use ODP. The
focus when developing l2fwd_simple was to reduce the code line count to as
small value as possible, and it turned out it requires 170 lines of code.

For unidirectional traffic in single-threaded use cases, l2fwd_simple
performs actually better than l2fwd with single thread, because
l2fwd_simple does not need to poll two interfaces in a single thread.

Signed-off-by: Juha-Matti Tilli 
<[email protected]<mailto:[email protected]>>
Reviewed-by: Petri Savolainen 
<[email protected]<mailto:[email protected]>>
---
   configure.ac<http://configure.ac>                            |   1 +
   example/Makefile.am                     |   2 +-
   example/l2fwd_simple/.gitignore         |   1 +
   example/l2fwd_simple/Makefile.am        |  10 ++
   example/l2fwd_simple/odp_l2fwd_simple.c | 170 
++++++++++++++++++++++++++++++++
   5 files changed, 183 insertions(+), 1 deletion(-)
   create mode 100644 example/l2fwd_simple/.gitignore
   create mode 100644 example/l2fwd_simple/Makefile.am
   create mode 100644 example/l2fwd_simple/odp_l2fwd_simple.c

diff --git a/configure.ac<http://configure.ac> 
b/configure.ac<http://configure.ac>
index 257f8c3..dbddbb5 100644
--- a/configure.ac<http://configure.ac>
+++ b/configure.ac<http://configure.ac>
@@ -331,6 +331,7 @@ AC_CONFIG_FILES([Makefile
                 example/time/Makefile
                 example/timer/Makefile
                 example/traffic_mgmt/Makefile
+                example/l2fwd_simple/Makefile
                 helper/Makefile
                 helper/test/Makefile
                 pkgconfig/libodp.pc
diff --git a/example/Makefile.am b/example/Makefile.am
index 906d907..2d425c6 100644
--- a/example/Makefile.am
+++ b/example/Makefile.am
@@ -1 +1 @@
-SUBDIRS = classifier generator ipsec packet time timer traffic_mgmt
+SUBDIRS = classifier generator ipsec packet time timer traffic_mgmt 
l2fwd_simple
diff --git a/example/l2fwd_simple/.gitignore b/example/l2fwd_simple/.gitignore
new file mode 100644
index 0000000..1326732
--- /dev/null
+++ b/example/l2fwd_simple/.gitignore
@@ -0,0 +1 @@
+odp_l2fwd_simple
diff --git a/example/l2fwd_simple/Makefile.am b/example/l2fwd_simple/Makefile.am
new file mode 100644
index 0000000..88d2915
--- /dev/null
+++ b/example/l2fwd_simple/Makefile.am
@@ -0,0 +1,10 @@
+include $(top_srcdir)/example/Makefile.inc
+
+bin_PROGRAMS = odp_l2fwd_simple$(EXEEXT)
+odp_l2fwd_simple_LDFLAGS = $(AM_LDFLAGS) -static
+odp_l2fwd_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
+
+noinst_HEADERS = \
+                 $(top_srcdir)/example/example_debug.h
+
+dist_odp_l2fwd_simple_SOURCES = odp_l2fwd_simple.c
diff --git a/example/l2fwd_simple/odp_l2fwd_simple.c 
b/example/l2fwd_simple/odp_l2fwd_simple.c
new file mode 100644
index 0000000..f57eaab
--- /dev/null
+++ b/example/l2fwd_simple/odp_l2fwd_simple.c
@@ -0,0 +1,170 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include <odp_api.h>
+#include <odp/helper/linux.h>
+#include <odp/helper/eth.h>
+#include <odp/helper/ip.h>
+
+#define POOL_NUM_PKT 8192
+#define POOL_SEG_LEN 1856
+#define MAX_PKT_BURST 32
+
+struct {
+       odp_pktio_t if0, if1;
+       odp_pktin_queue_t if0in, if1in;
+       odp_pktout_queue_t if0out, if1out;
+       odph_ethaddr_t src, dst;
+} global;
+
+static odp_pktio_t create_pktio(const char *name, odp_pool_t pool,
+                               odp_pktin_queue_t *pktin,
+                               odp_pktout_queue_t *pktout)
+{
+       odp_pktio_param_t pktio_param;
+       odp_pktin_queue_param_t in_queue_param;
+       odp_pktout_queue_param_t out_queue_param;
+       odp_pktio_t pktio;
+
+       odp_pktio_param_init(&pktio_param);
+
+       pktio_param.in_mode = ODP_PKTIN_MODE_DIRECT;
+       pktio_param.out_mode = ODP_PKTOUT_MODE_DIRECT;
+       pktio = odp_pktio_open(name, pool, &pktio_param);
+       if (pktio == ODP_PKTIO_INVALID) {
+               printf("Failed to open %s\n", name);
+               exit(1);
+       }
+
+       odp_pktin_queue_param_init(&in_queue_param);
+       odp_pktout_queue_param_init(&out_queue_param);
+
+       in_queue_param.op_mode = ODP_PKTIO_OP_MT_UNSAFE;
+       in_queue_param.num_queues = 1;
+
+       if (odp_pktin_queue_config(pktio, &in_queue_param)) {
+               printf("Failed to config input queue for %s\n", name);
+               exit(1);
+       }
+
+       out_queue_param.op_mode = ODP_PKTIO_OP_MT_UNSAFE;
+       out_queue_param.num_queues = 1;
+
+       if (odp_pktout_queue_config(pktio, &out_queue_param)) {
+               printf("Failed to config output queue for %s\n", name);
+               exit(1);
+       }
+
+       if (odp_pktin_queue(pktio, pktin, 1) != 1) {
+               printf("pktin queue query failed for %s\n", name);
+               exit(1);
+       }
+       if (odp_pktout_queue(pktio, pktout, 1) != 1) {
+               printf("pktout queue query failed for %s\n", name);
+               exit(1);
+       }
+       return pktio;
+}
+
+static void *run_worker(void *arg ODP_UNUSED)
+{
+       odp_packet_t pkt_tbl[MAX_PKT_BURST];
+       int pkts, sent, tx_drops, i;
+
+       if (odp_pktio_start(global.if0)) {
+               printf("unable to start input interface\n");
+               exit(1);
+       }
+       printf("started input interface\n");
+       if (odp_pktio_start(global.if1)) {
+               printf("unable to start output interface\n");
+               exit(1);
+       }
+       printf("started output interface\n");
+       printf("started all\n");
+
+       for (;;) {
+               pkts = odp_pktio_recv_queue(global.if0in, pkt_tbl,
+                                           MAX_PKT_BURST);
+               if (odp_unlikely(pkts <= 0))
+                       continue;
+               for (i = 0; i < pkts; i++) {
+                       odp_packet_t pkt = pkt_tbl[i];
+                       odph_ethhdr_t *eth;
+
+                       if (odp_unlikely(!odp_packet_has_eth(pkt))) {
+                               printf("warning: packet has no eth header\n");
+                               return NULL;
+                       }
+                       eth = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, NULL);
+                       eth->src = global.src;
+                       eth->dst = global.dst;
+               }
+               sent = odp_pktio_send_queue(global.if1out, pkt_tbl, pkts);
+               if (sent < 0)
+                       sent = 0;
+               tx_drops = pkts - sent;
+               if (odp_unlikely(tx_drops))
+                       odp_packet_free_multi(&pkt_tbl[sent], tx_drops);
+       }
+       return NULL;
+}
+
+int main(int argc, char **argv)
+{
+       odp_pool_t pool;
+       odp_pool_param_t params;
+       odp_cpumask_t cpumask;
+       odph_linux_pthread_t thd;
+
+       if (argc != 5 ||
+           odph_eth_addr_parse(&global.dst, argv[3]) != 0 ||
+           odph_eth_addr_parse(&global.src, argv[4]) != 0) {
+               printf("Usage: odp_l2fwd_simple eth0 eth1 01:02:03:04:05:06"
+                      " 07:08:09:0a:0b:0c\n");
+               printf("Where eth0 and eth1 are the used interfaces"
+                      " (must have 2 of them)\n");
+               printf("And the hexadecimal numbers are destination MAC address"
+                      " and source MAC address\n");
+               exit(1);
+       }
+
+       if (odp_init_global(NULL, NULL)) {
+               printf("Error: ODP global init failed.\n");
+               exit(1);
+       }
+
+       if (odp_init_local(ODP_THREAD_CONTROL)) {
+               printf("Error: ODP local init failed.\n");
+               exit(1);
+       }
+
+       /* Create packet pool */
+       odp_pool_param_init(&params);
+       params.pkt.seg_len = POOL_SEG_LEN;
+       params.pkt.len     = POOL_SEG_LEN;
+       params.pkt.num     = POOL_NUM_PKT;
+       params.type        = ODP_POOL_PACKET;
+
+       pool = odp_pool_create("packet pool", &params);
+
+       if (pool == ODP_POOL_INVALID) {
+               printf("Error: packet pool create failed.\n");
+               exit(1);
+       }
+
+       global.if0 = create_pktio(argv[1], pool, &global.if0in, &global.if0out);
+       global.if1 = create_pktio(argv[2], pool, &global.if1in, &global.if1out);
+
+       odp_cpumask_default_worker(&cpumask, 1);
+       odph_linux_pthread_create(&thd, &cpumask, run_worker, NULL,
+                                 ODP_THREAD_WORKER);
+       odph_linux_pthread_join(&thd, 1);
+       return 0;
+}
_______________________________________________
lng-odp mailing list
[email protected]<mailto:[email protected]>
https://lists.linaro.org/mailman/listinfo/lng-odp

_______________________________________________
lng-odp mailing list
[email protected]<mailto:[email protected]>
https://lists.linaro.org/mailman/listinfo/lng-odp



--
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org<http://www.linaro.org/> │ Open source software for ARM SoCs
"Work should be fun and collborative, the rest follows"

_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to