Re: [ovs-dev] [PATCH v7 11/13] dpdk-tests: Add uni-tests for multi-seg mbufs.

2018-08-09 Thread Stokes, Ian
> In order to create a minimal environment that allows the tests to get
> mbufs from an existing mempool, the following approach is taken:
> - EAL is initialised (by using the main dpdk_init()) and a (very) small
>   mempool is instantiated (mimicking the logic in dpdk_mp_create()).
>   This mempool instance is global and used by all the tests;
> - Packets are then allocated from the instantiated mempool, and tested
>   on, by running some operations on them and manipulating data.
> 
> The tests introduced focus on testing DPDK dp_packets (where
> source=DPBUF_DPDK), linked with a single or multiple mbufs, across several
> operations, such as:
> - dp_packet_put();
> - dp_packet_shift();
> - dp_packet_reserve();
> - dp_packet_push_uninit();
> - dp_packet_clear();
> - And as a consequence of some of these, dp_packet_put_uninit() and
>   dp_packet_resize__().
> 
> Finally, this has also been integrated with the new DPDK testsuite.
> Thus, when running `$sudo make check-dpdk` one will also be running these
> tests.

Hi Tiago,

A few compilation errors flagged by sparse below.

> 
> Signed-off-by: Tiago Lam 
> Acked-by: Eelco Chaudron 
> ---
>  tests/automake.mk  |  10 +-
>  tests/dpdk-packet-mbufs.at |   7 +
>  tests/system-dpdk-testsuite.at |   1 +
>  tests/test-dpdk-mbufs.c| 513
> +
>  4 files changed, 530 insertions(+), 1 deletion(-)  create mode 100644
> tests/dpdk-packet-mbufs.at  create mode 100644 tests/test-dpdk-mbufs.c
> 
> diff --git a/tests/automake.mk b/tests/automake.mk index 8224e5a..5fe98bd
> 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -134,7 +134,8 @@ SYSTEM_DPDK_TESTSUITE_AT = \
>   tests/system-common-macros.at \
>   tests/system-dpdk-macros.at \
>   tests/system-dpdk-testsuite.at \
> - tests/system-dpdk.at
> + tests/system-dpdk.at \
> + tests/dpdk-packet-mbufs.at
> 
>  check_SCRIPTS += tests/atlocal
> 
> @@ -391,6 +392,10 @@ tests_ovstest_SOURCES = \
>   tests/test-vconn.c \
>   tests/test-aa.c \
>   tests/test-stopwatch.c
> +if DPDK_NETDEV
> +tests_ovstest_SOURCES += \
> + tests/test-dpdk-mbufs.c
> +endif
> 
>  if !WIN32
>  tests_ovstest_SOURCES += \
> @@ -403,6 +408,9 @@ tests_ovstest_SOURCES += \  endif
> 
>  tests_ovstest_LDADD = lib/libopenvswitch.la ovn/lib/libovn.la
> +if DPDK_NETDEV
> +tests_ovstest_LDFLAGS = $(AM_LDFLAGS) $(DPDK_vswitchd_LDFLAGS) endif
> 
>  noinst_PROGRAMS += tests/test-strtok_r
>  tests_test_strtok_r_SOURCES = tests/test-strtok_r.c diff --git
> a/tests/dpdk-packet-mbufs.at b/tests/dpdk-packet-mbufs.at new file mode
> 100644 index 000..f28e4fc
> --- /dev/null
> +++ b/tests/dpdk-packet-mbufs.at
> @@ -0,0 +1,7 @@
> +AT_BANNER([OVS-DPDK dp_packet unit tests])
> +
> +AT_SETUP([OVS-DPDK dp_packet - mbufs allocation])
> +AT_KEYWORDS([dp_packet, multi-seg, mbufs]) AT_CHECK(ovstest
> +test-dpdk-packet, [], [ignore], [ignore])
> +
> +AT_CLEANUP
> diff --git a/tests/system-dpdk-testsuite.at b/tests/system-dpdk-
> testsuite.at index 382f09e..f5edf58 100644
> --- a/tests/system-dpdk-testsuite.at
> +++ b/tests/system-dpdk-testsuite.at
> @@ -23,3 +23,4 @@ m4_include([tests/system-common-macros.at])
>  m4_include([tests/system-dpdk-macros.at])
> 
>  m4_include([tests/system-dpdk.at])
> +m4_include([tests/dpdk-packet-mbufs.at])
> diff --git a/tests/test-dpdk-mbufs.c b/tests/test-dpdk-mbufs.c new file
> mode 100644 index 000..8168cae
> --- /dev/null
> +++ b/tests/test-dpdk-mbufs.c
> @@ -0,0 +1,513 @@
> +/*
> + * Copyright (c) 2018 Intel Corporation
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "dp-packet.h"
> +#include "ovstest.h"
> +#include "dpdk.h"
> +#include "smap.h"
> +
> +#define N_MBUFS 1024
> +#define MBUF_DATA_LEN 2048
> +
> +int num_tests = 0;

Sparse compilation error: symbol 'num_tests' was not declared. Should it be 
static?

> +
> +/* Global var to hold a mempool instance, "test-mp", used in all of the
> +tests
> + * below. This instance is instantiated in dpdk_setup_eal_with_mp(). */
> +static struct rte_mempool *mp;
> +
> +/* Test data used to fill the packets with data. Note that this isn't a
> +string
> + * that repsents a valid packet, by any means. The pattern is generated
> +in set_
> + * testing_pattern_str() and the sole purpose is to verify the data
> +remains th

[ovs-dev] [PATCH v7 11/13] dpdk-tests: Add uni-tests for multi-seg mbufs.

2018-07-25 Thread Tiago Lam
In order to create a minimal environment that allows the tests to get
mbufs from an existing mempool, the following approach is taken:
- EAL is initialised (by using the main dpdk_init()) and a (very) small
  mempool is instantiated (mimicking the logic in dpdk_mp_create()).
  This mempool instance is global and used by all the tests;
- Packets are then allocated from the instantiated mempool, and tested
  on, by running some operations on them and manipulating data.

The tests introduced focus on testing DPDK dp_packets (where
source=DPBUF_DPDK), linked with a single or multiple mbufs, across
several operations, such as:
- dp_packet_put();
- dp_packet_shift();
- dp_packet_reserve();
- dp_packet_push_uninit();
- dp_packet_clear();
- And as a consequence of some of these, dp_packet_put_uninit() and
  dp_packet_resize__().

Finally, this has also been integrated with the new DPDK testsuite.
Thus, when running `$sudo make check-dpdk` one will also be running
these tests.

Signed-off-by: Tiago Lam 
Acked-by: Eelco Chaudron 
---
 tests/automake.mk  |  10 +-
 tests/dpdk-packet-mbufs.at |   7 +
 tests/system-dpdk-testsuite.at |   1 +
 tests/test-dpdk-mbufs.c| 513 +
 4 files changed, 530 insertions(+), 1 deletion(-)
 create mode 100644 tests/dpdk-packet-mbufs.at
 create mode 100644 tests/test-dpdk-mbufs.c

diff --git a/tests/automake.mk b/tests/automake.mk
index 8224e5a..5fe98bd 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -134,7 +134,8 @@ SYSTEM_DPDK_TESTSUITE_AT = \
tests/system-common-macros.at \
tests/system-dpdk-macros.at \
tests/system-dpdk-testsuite.at \
-   tests/system-dpdk.at
+   tests/system-dpdk.at \
+   tests/dpdk-packet-mbufs.at
 
 check_SCRIPTS += tests/atlocal
 
@@ -391,6 +392,10 @@ tests_ovstest_SOURCES = \
tests/test-vconn.c \
tests/test-aa.c \
tests/test-stopwatch.c
+if DPDK_NETDEV
+tests_ovstest_SOURCES += \
+   tests/test-dpdk-mbufs.c
+endif
 
 if !WIN32
 tests_ovstest_SOURCES += \
@@ -403,6 +408,9 @@ tests_ovstest_SOURCES += \
 endif
 
 tests_ovstest_LDADD = lib/libopenvswitch.la ovn/lib/libovn.la
+if DPDK_NETDEV
+tests_ovstest_LDFLAGS = $(AM_LDFLAGS) $(DPDK_vswitchd_LDFLAGS)
+endif
 
 noinst_PROGRAMS += tests/test-strtok_r
 tests_test_strtok_r_SOURCES = tests/test-strtok_r.c
diff --git a/tests/dpdk-packet-mbufs.at b/tests/dpdk-packet-mbufs.at
new file mode 100644
index 000..f28e4fc
--- /dev/null
+++ b/tests/dpdk-packet-mbufs.at
@@ -0,0 +1,7 @@
+AT_BANNER([OVS-DPDK dp_packet unit tests])
+
+AT_SETUP([OVS-DPDK dp_packet - mbufs allocation])
+AT_KEYWORDS([dp_packet, multi-seg, mbufs])
+AT_CHECK(ovstest test-dpdk-packet, [], [ignore], [ignore])
+
+AT_CLEANUP
diff --git a/tests/system-dpdk-testsuite.at b/tests/system-dpdk-testsuite.at
index 382f09e..f5edf58 100644
--- a/tests/system-dpdk-testsuite.at
+++ b/tests/system-dpdk-testsuite.at
@@ -23,3 +23,4 @@ m4_include([tests/system-common-macros.at])
 m4_include([tests/system-dpdk-macros.at])
 
 m4_include([tests/system-dpdk.at])
+m4_include([tests/dpdk-packet-mbufs.at])
diff --git a/tests/test-dpdk-mbufs.c b/tests/test-dpdk-mbufs.c
new file mode 100644
index 000..8168cae
--- /dev/null
+++ b/tests/test-dpdk-mbufs.c
@@ -0,0 +1,513 @@
+/*
+ * Copyright (c) 2018 Intel Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "dp-packet.h"
+#include "ovstest.h"
+#include "dpdk.h"
+#include "smap.h"
+
+#define N_MBUFS 1024
+#define MBUF_DATA_LEN 2048
+
+int num_tests = 0;
+
+/* Global var to hold a mempool instance, "test-mp", used in all of the tests
+ * below. This instance is instantiated in dpdk_setup_eal_with_mp(). */
+static struct rte_mempool *mp;
+
+/* Test data used to fill the packets with data. Note that this isn't a string
+ * that repsents a valid packet, by any means. The pattern is generated in set_
+ * testing_pattern_str() and the sole purpose is to verify the data remains the
+ * same after inserting and operating on multi-segment mbufs. */
+static char *test_str;
+
+/* Asserts a dp_packet that holds a single mbuf, where:
+ * - nb_segs must be 1;
+ * - pkt_len must be equal to data_len which in turn must equal the provided
+ *   'pkt_len';
+ * - data_off must start at the provided 'data_ofs';
+ * - next must be NULL. */
+static void
+assert_single_mbuf(struct dp_packet *pk