On 16 February 2016 at 10:07, Maxim Uvarov <[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]] On Behalf Of EXT
>> Maxim Uvarov
>> Sent: Tuesday, February 16, 2016 2:24 PM
>> To: [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]>
>>> Reviewed-by: Petri Savolainen <[email protected]>
>>> ---
>>>    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 b/configure.ac
>>> index 257f8c3..dbddbb5 100644
>>> --- a/configure.ac
>>> +++ b/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]
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
> _______________________________________________
> lng-odp mailing list
> [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