* [dpdk-dev] [PATCH v4] test: fix missing NULL pointer checks
@ 2015-01-30 11:14 Daniel Mrzyglod
2015-02-10 11:45 ` Bruce Richardson
0 siblings, 1 reply; 2+ messages in thread
From: Daniel Mrzyglod @ 2015-01-30 11:14 UTC (permalink / raw)
To: dev
In test_sched, we are missing NULL pointer checks after create_mempool()
and rte_pktmbuf_alloc(). Add in these checks using TEST_ASSERT_NOT_NULL
macros.
VERIFY macro was removed and replaced by standard test ASSERTS from
"test.h" header.
This provides additional information to track when the failure occured.
v4 changes:
-If test fails add cleanup routines
-Change Magic Numbers to dedicated Macro RTE_DIM
v3 changes:
- remove VERIFY macro
- fix spelling error.
- change unproper comment
v2 changes:
- Replace all VERIFY macros instances by proper TEST_ASSERT* macros.
- fix description
v1 changes:
- first iteration of patch using VERIFY macro.
Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
app/test/test_sched.c | 120 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 83 insertions(+), 37 deletions(-)
diff --git a/app/test/test_sched.c b/app/test/test_sched.c
index c957d80..6a97e2c 100644
--- a/app/test/test_sched.c
+++ b/app/test/test_sched.c
@@ -44,14 +44,7 @@
#include <rte_ip.h>
#include <rte_byteorder.h>
#include <rte_sched.h>
-
-
-#define VERIFY(exp,fmt,args...) \
- if (!(exp)) { \
- printf(fmt, ##args); \
- return -1; \
- }
-
+#include <rte_common.h>
#define SUBPORT 0
#define PIPE 1
@@ -92,6 +85,10 @@ static struct rte_sched_port_params port_param = {
.n_pipe_profiles = 1,
};
+static struct rte_sched_port *port = NULL;
+static struct rte_mbuf *in_mbufs[10];
+static struct rte_mempool *mp = NULL;
+
#define NB_MBUF 32
#define MAX_PACKET_SZ 2048
#define MBUF_SZ (MAX_PACKET_SZ + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
@@ -128,7 +125,6 @@ prepare_pkt(struct rte_mbuf *mbuf)
struct ether_hdr *eth_hdr;
struct vlan_hdr *vlan1, *vlan2;
struct ipv4_hdr *ip_hdr;
-
/* Simulate a classifier */
eth_hdr = rte_pktmbuf_mtod(mbuf, struct ether_hdr *);
vlan1 = (struct vlan_hdr *)(ð_hdr->ether_type );
@@ -150,64 +146,71 @@ prepare_pkt(struct rte_mbuf *mbuf)
}
+static void
+clean_environment(void)
+{
+ uint32_t i;
+
+ port = NULL;
+ for (i = 0; i < RTE_DIM(in_mbufs); i++) {
+ if (in_mbufs[i] != NULL) {
+ rte_pktmbuf_free(in_mbufs[i]);
+ in_mbufs[i] = NULL;
+ }
+ }
+}
+
/**
* test main entrance for library sched
*/
static int
test_sched(void)
{
- struct rte_mempool *mp = NULL;
- struct rte_sched_port *port = NULL;
uint32_t pipe;
- struct rte_mbuf *in_mbufs[10];
+ uint32_t i;
struct rte_mbuf *out_mbufs[10];
- int i;
-
int err;
- mp = create_mempool();
-
port_param.socket = 0;
port_param.rate = (uint64_t) 10000 * 1000 * 1000 / 8;
port = rte_sched_port_config(&port_param);
- VERIFY(port != NULL, "Error config sched port\n");
-
+ TEST_ASSERT_NOT_NULL(port, "Error config sched port\n");
err = rte_sched_subport_config(port, SUBPORT, subport_param);
- VERIFY(err == 0, "Error config sched, err=%d\n", err);
+ TEST_ASSERT_SUCCESS(err, "Error config sched, err=%d\n", err);
for (pipe = 0; pipe < port_param.n_pipes_per_subport; pipe ++) {
err = rte_sched_pipe_config(port, SUBPORT, pipe, 0);
- VERIFY(err == 0, "Error config sched pipe %u, err=%d\n", pipe, err);
+ TEST_ASSERT_SUCCESS(err, "Error config sched pipe %u, err=%d\n", pipe, err);
}
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < RTE_DIM(in_mbufs); i++) {
in_mbufs[i] = rte_pktmbuf_alloc(mp);
+ TEST_ASSERT_NOT_NULL(in_mbufs[i], "Packet allocation failed\n");
prepare_pkt(in_mbufs[i]);
}
+ err = rte_sched_port_enqueue(port, in_mbufs, RTE_DIM(in_mbufs));
+ TEST_ASSERT_EQUAL(err, RTE_DIM(in_mbufs), "Wrong enqueue, err=%d\n", err);
- err = rte_sched_port_enqueue(port, in_mbufs, 10);
- VERIFY(err == 10, "Wrong enqueue, err=%d\n", err);
-
- err = rte_sched_port_dequeue(port, out_mbufs, 10);
- VERIFY(err == 10, "Wrong dequeue, err=%d\n", err);
+ err = rte_sched_port_dequeue(port, out_mbufs, RTE_DIM(out_mbufs));
+ TEST_ASSERT_EQUAL(err, RTE_DIM(out_mbufs), "Wrong dequeue, err=%d\n", err);
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < RTE_DIM(out_mbufs); i++) {
enum rte_meter_color color;
uint32_t subport, traffic_class, queue;
color = rte_sched_port_pkt_read_color(out_mbufs[i]);
- VERIFY(color == e_RTE_METER_YELLOW, "Wrong color\n");
+ TEST_ASSERT_EQUAL(color, e_RTE_METER_YELLOW, "Wrong color\n");
rte_sched_port_pkt_read_tree_path(out_mbufs[i],
&subport, &pipe, &traffic_class, &queue);
- VERIFY(subport == SUBPORT, "Wrong subport\n");
- VERIFY(pipe == PIPE, "Wrong pipe\n");
- VERIFY(traffic_class == TC, "Wrong traffic_class\n");
- VERIFY(queue == QUEUE, "Wrong queue\n");
+ TEST_ASSERT_EQUAL(subport, SUBPORT, "Wrong subport\n");
+ TEST_ASSERT_EQUAL(pipe, PIPE, "Wrong pipe\n");
+ TEST_ASSERT_EQUAL(traffic_class, TC, "Wrong traffic_class\n");
+ TEST_ASSERT_EQUAL(queue, QUEUE, "Wrong queue\n");
}
@@ -215,20 +218,63 @@ test_sched(void)
struct rte_sched_subport_stats subport_stats;
uint32_t tc_ov;
rte_sched_subport_read_stats(port, SUBPORT, &subport_stats, &tc_ov);
- //VERIFY(subport_stats.n_pkts_tc[TC-1] == 10, "Wrong subport stats\n");
-
+#if 0
+ TEST_ASSERT_EQUAL(subport_stats.n_pkts_tc[TC-1], 10, "Wrong subport stats\n");
+#endif
struct rte_sched_queue_stats queue_stats;
uint16_t qlen;
rte_sched_queue_read_stats(port, QUEUE, &queue_stats, &qlen);
- //VERIFY(queue_stats.n_pkts == 10, "Wrong queue stats\n");
-
+#if 0
+ TEST_ASSERT_EQUAL(queue_stats.n_pkts, 10, "Wrong queue stats\n");
+#endif
+ /* Test cleanning */
rte_sched_port_free(port);
+ port = NULL;
+ for (i = 0; i < RTE_DIM(in_mbufs); i++) {
+ rte_pktmbuf_free(in_mbufs[i]);
+ in_mbufs[i] = NULL;
+ }
+
+ return 0;
+}
+
+static int
+check_environment(void)
+{
+ uint32_t i;
+
+ TEST_ASSERT_NULL(port, "Port not cleaned by test\n");
+
+ for (i = 0; i < RTE_DIM(in_mbufs); i++) {
+ TEST_ASSERT_NULL(in_mbufs[i], "in_mbuff not cleaned by test\n");
+ }
return 0;
}
+
+
+static int
+test_sched_start(void)
+{
+ int ret;
+
+ mp = create_mempool();
+ TEST_ASSERT_NOT_NULL(mp, "Error creating mempool\n");
+
+ if (check_environment() < 0)
+ return -1;
+ ret = test_sched();
+ if (ret == 0)
+ TEST_ASSERT_SUCCESS(check_environment(), "Bad Values after test execution");
+ else
+ clean_environment();
+
+ return ret;
+}
+
static struct test_command sched_cmd = {
.command = "sched_autotest",
- .callback = test_sched,
+ .callback = test_sched_start,
};
REGISTER_TEST_COMMAND(sched_cmd);
--
2.1.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [dpdk-dev] [PATCH v4] test: fix missing NULL pointer checks
2015-01-30 11:14 [dpdk-dev] [PATCH v4] test: fix missing NULL pointer checks Daniel Mrzyglod
@ 2015-02-10 11:45 ` Bruce Richardson
0 siblings, 0 replies; 2+ messages in thread
From: Bruce Richardson @ 2015-02-10 11:45 UTC (permalink / raw)
To: Daniel Mrzyglod; +Cc: dev
On Fri, Jan 30, 2015 at 12:14:53PM +0100, Daniel Mrzyglod wrote:
> In test_sched, we are missing NULL pointer checks after create_mempool()
> and rte_pktmbuf_alloc(). Add in these checks using TEST_ASSERT_NOT_NULL
> macros.
>
> VERIFY macro was removed and replaced by standard test ASSERTS from
> "test.h" header.
> This provides additional information to track when the failure occured.
>
> v4 changes:
> -If test fails add cleanup routines
> -Change Magic Numbers to dedicated Macro RTE_DIM
This V4 adds in significant new changes to those described in the patch title.
I think it needs to be split into separate patches, so that we can have
a single simple patch adding in the additional null tests. The rework of the
macros and the switch to have global variables and clean-up routines should be
in separate patches.
Check spelling of mbuf as "mbuff" a few times below. Also, can you please
post follow-up patches within the original thread to make tracking
the discussion easier. It allows us to quickly see how this patch has evolved
from the original proposal.
/Bruce
>
> v3 changes:
> - remove VERIFY macro
> - fix spelling error.
> - change unproper comment
>
> v2 changes:
> - Replace all VERIFY macros instances by proper TEST_ASSERT* macros.
> - fix description
>
> v1 changes:
> - first iteration of patch using VERIFY macro.
>
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> ---
> app/test/test_sched.c | 120 ++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 83 insertions(+), 37 deletions(-)
>
> diff --git a/app/test/test_sched.c b/app/test/test_sched.c
> index c957d80..6a97e2c 100644
> --- a/app/test/test_sched.c
> +++ b/app/test/test_sched.c
> @@ -44,14 +44,7 @@
> #include <rte_ip.h>
> #include <rte_byteorder.h>
> #include <rte_sched.h>
> -
> -
> -#define VERIFY(exp,fmt,args...) \
> - if (!(exp)) { \
> - printf(fmt, ##args); \
> - return -1; \
> - }
> -
> +#include <rte_common.h>
>
> #define SUBPORT 0
> #define PIPE 1
> @@ -92,6 +85,10 @@ static struct rte_sched_port_params port_param = {
> .n_pipe_profiles = 1,
> };
>
> +static struct rte_sched_port *port = NULL;
> +static struct rte_mbuf *in_mbufs[10];
> +static struct rte_mempool *mp = NULL;
> +
> #define NB_MBUF 32
> #define MAX_PACKET_SZ 2048
> #define MBUF_SZ (MAX_PACKET_SZ + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
> @@ -128,7 +125,6 @@ prepare_pkt(struct rte_mbuf *mbuf)
> struct ether_hdr *eth_hdr;
> struct vlan_hdr *vlan1, *vlan2;
> struct ipv4_hdr *ip_hdr;
> -
> /* Simulate a classifier */
> eth_hdr = rte_pktmbuf_mtod(mbuf, struct ether_hdr *);
> vlan1 = (struct vlan_hdr *)(ð_hdr->ether_type );
> @@ -150,64 +146,71 @@ prepare_pkt(struct rte_mbuf *mbuf)
> }
>
>
> +static void
> +clean_environment(void)
> +{
> + uint32_t i;
> +
> + port = NULL;
> + for (i = 0; i < RTE_DIM(in_mbufs); i++) {
> + if (in_mbufs[i] != NULL) {
> + rte_pktmbuf_free(in_mbufs[i]);
> + in_mbufs[i] = NULL;
> + }
> + }
> +}
> +
> /**
> * test main entrance for library sched
> */
> static int
> test_sched(void)
> {
> - struct rte_mempool *mp = NULL;
> - struct rte_sched_port *port = NULL;
> uint32_t pipe;
> - struct rte_mbuf *in_mbufs[10];
> + uint32_t i;
> struct rte_mbuf *out_mbufs[10];
> - int i;
> -
> int err;
>
> - mp = create_mempool();
> -
> port_param.socket = 0;
> port_param.rate = (uint64_t) 10000 * 1000 * 1000 / 8;
>
> port = rte_sched_port_config(&port_param);
> - VERIFY(port != NULL, "Error config sched port\n");
> -
> + TEST_ASSERT_NOT_NULL(port, "Error config sched port\n");
>
> err = rte_sched_subport_config(port, SUBPORT, subport_param);
> - VERIFY(err == 0, "Error config sched, err=%d\n", err);
> + TEST_ASSERT_SUCCESS(err, "Error config sched, err=%d\n", err);
>
> for (pipe = 0; pipe < port_param.n_pipes_per_subport; pipe ++) {
> err = rte_sched_pipe_config(port, SUBPORT, pipe, 0);
> - VERIFY(err == 0, "Error config sched pipe %u, err=%d\n", pipe, err);
> + TEST_ASSERT_SUCCESS(err, "Error config sched pipe %u, err=%d\n", pipe, err);
> }
>
> - for (i = 0; i < 10; i++) {
> + for (i = 0; i < RTE_DIM(in_mbufs); i++) {
> in_mbufs[i] = rte_pktmbuf_alloc(mp);
> + TEST_ASSERT_NOT_NULL(in_mbufs[i], "Packet allocation failed\n");
> prepare_pkt(in_mbufs[i]);
> }
>
> + err = rte_sched_port_enqueue(port, in_mbufs, RTE_DIM(in_mbufs));
> + TEST_ASSERT_EQUAL(err, RTE_DIM(in_mbufs), "Wrong enqueue, err=%d\n", err);
>
> - err = rte_sched_port_enqueue(port, in_mbufs, 10);
> - VERIFY(err == 10, "Wrong enqueue, err=%d\n", err);
> -
> - err = rte_sched_port_dequeue(port, out_mbufs, 10);
> - VERIFY(err == 10, "Wrong dequeue, err=%d\n", err);
> + err = rte_sched_port_dequeue(port, out_mbufs, RTE_DIM(out_mbufs));
> + TEST_ASSERT_EQUAL(err, RTE_DIM(out_mbufs), "Wrong dequeue, err=%d\n", err);
>
> - for (i = 0; i < 10; i++) {
> + for (i = 0; i < RTE_DIM(out_mbufs); i++) {
> enum rte_meter_color color;
> uint32_t subport, traffic_class, queue;
>
> color = rte_sched_port_pkt_read_color(out_mbufs[i]);
> - VERIFY(color == e_RTE_METER_YELLOW, "Wrong color\n");
> + TEST_ASSERT_EQUAL(color, e_RTE_METER_YELLOW, "Wrong color\n");
>
> rte_sched_port_pkt_read_tree_path(out_mbufs[i],
> &subport, &pipe, &traffic_class, &queue);
>
> - VERIFY(subport == SUBPORT, "Wrong subport\n");
> - VERIFY(pipe == PIPE, "Wrong pipe\n");
> - VERIFY(traffic_class == TC, "Wrong traffic_class\n");
> - VERIFY(queue == QUEUE, "Wrong queue\n");
> + TEST_ASSERT_EQUAL(subport, SUBPORT, "Wrong subport\n");
> + TEST_ASSERT_EQUAL(pipe, PIPE, "Wrong pipe\n");
> + TEST_ASSERT_EQUAL(traffic_class, TC, "Wrong traffic_class\n");
> + TEST_ASSERT_EQUAL(queue, QUEUE, "Wrong queue\n");
>
> }
>
> @@ -215,20 +218,63 @@ test_sched(void)
> struct rte_sched_subport_stats subport_stats;
> uint32_t tc_ov;
> rte_sched_subport_read_stats(port, SUBPORT, &subport_stats, &tc_ov);
> - //VERIFY(subport_stats.n_pkts_tc[TC-1] == 10, "Wrong subport stats\n");
> -
> +#if 0
> + TEST_ASSERT_EQUAL(subport_stats.n_pkts_tc[TC-1], 10, "Wrong subport stats\n");
> +#endif
> struct rte_sched_queue_stats queue_stats;
> uint16_t qlen;
> rte_sched_queue_read_stats(port, QUEUE, &queue_stats, &qlen);
> - //VERIFY(queue_stats.n_pkts == 10, "Wrong queue stats\n");
> -
> +#if 0
> + TEST_ASSERT_EQUAL(queue_stats.n_pkts, 10, "Wrong queue stats\n");
> +#endif
> + /* Test cleanning */
> rte_sched_port_free(port);
> + port = NULL;
> + for (i = 0; i < RTE_DIM(in_mbufs); i++) {
> + rte_pktmbuf_free(in_mbufs[i]);
> + in_mbufs[i] = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +check_environment(void)
> +{
> + uint32_t i;
> +
> + TEST_ASSERT_NULL(port, "Port not cleaned by test\n");
> +
> + for (i = 0; i < RTE_DIM(in_mbufs); i++) {
> + TEST_ASSERT_NULL(in_mbufs[i], "in_mbuff not cleaned by test\n");
> + }
>
> return 0;
> }
>
> +
> +
> +static int
> +test_sched_start(void)
> +{
> + int ret;
> +
> + mp = create_mempool();
> + TEST_ASSERT_NOT_NULL(mp, "Error creating mempool\n");
> +
> + if (check_environment() < 0)
> + return -1;
> + ret = test_sched();
> + if (ret == 0)
> + TEST_ASSERT_SUCCESS(check_environment(), "Bad Values after test execution");
> + else
> + clean_environment();
> +
> + return ret;
> +}
> +
> static struct test_command sched_cmd = {
> .command = "sched_autotest",
> - .callback = test_sched,
> + .callback = test_sched_start,
> };
> REGISTER_TEST_COMMAND(sched_cmd);
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-02-10 11:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-30 11:14 [dpdk-dev] [PATCH v4] test: fix missing NULL pointer checks Daniel Mrzyglod
2015-02-10 11:45 ` Bruce Richardson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).