[dpdk-dev] [PATCH 3/7] test/stack: add stack test

Eads, Gage gage.eads at intel.com
Thu Feb 28 06:11:20 CET 2019



> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> Sent: Monday, February 25, 2019 4:59 AM
> To: Eads, Gage <gage.eads at intel.com>
> Cc: dev at dpdk.org; arybchenko at solarflare.com; Richardson, Bruce
> <bruce.richardson at intel.com>; Ananyev, Konstantin
> <konstantin.ananyev at intel.com>; gavin.hu at arm.com;
> Honnappa.Nagarahalli at arm.com; nd at arm.com; thomas at monjalon.net
> Subject: Re: [PATCH 3/7] test/stack: add stack test
> 
> On Fri, Feb 22, 2019 at 10:06:51AM -0600, Gage Eads wrote:
> > stack_autotest performs positive and negative testing of the stack
> > API, and exercises the push and pop datapath functions with all available
> lcores.
> >
> > Signed-off-by: Gage Eads <gage.eads at intel.com>
> 
> [...]
> 
> > --- /dev/null
> > +++ b/test/test/test_stack.c
> > @@ -0,0 +1,394 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2019 Intel Corporation  */
> > +
> > +#include <string.h>
> > +
> > +#include <rte_lcore.h>
> > +#include <rte_malloc.h>
> > +#include <rte_random.h>
> > +#include <rte_stack.h>
> > +
> > +#include "test.h"
> > +
> > +#define STACK_SIZE 4096
> > +#define MAX_BULK 32
> > +
> > +static int
> > +test_stack_push_pop(struct rte_stack *s, void **obj_table, unsigned
> > +int bulk_sz) {
> > +	void *popped_objs[STACK_SIZE];
> > +	unsigned int i, ret;
> 
> Here, a dynamic sized table is used. In test_stack_basic() below, it uses a heap-
> based allocation for the same purpose. I think it would be more consistent to
> have the same method for both. I suggest to allocate in heap to avoid a stack
> overflow if STACK_SIZE is increased in the future.
> 

Sure, I'll make popped_objs dynamically allocated.

> [...]
> 
> > +static int
> > +test_stack_basic(void)
> > +{
> > +	struct rte_stack *s = NULL;
> > +	void **obj_table = NULL;
> > +	int i, ret = -1;
> > +
> > +	obj_table = rte_calloc(NULL, STACK_SIZE, sizeof(void *), 0);
> > +	if (obj_table == NULL) {
> > +		printf("[%s():%u] failed to calloc %lu bytes\n",
> > +		       __func__, __LINE__, STACK_SIZE * sizeof(void *));
> > +		goto fail_test;
> > +	}
> > +
> > +	for (i = 0; i < STACK_SIZE; i++)
> > +		obj_table[i] = (void *)(uintptr_t)i;
> > +
> > +	s = rte_stack_create(__func__, STACK_SIZE, rte_socket_id(), 0);
> > +	if (s == NULL) {
> > +		printf("[%s():%u] failed to create a stack\n",
> > +		       __func__, __LINE__);
> > +		goto fail_test;
> > +	}
> > +
> > +	if (rte_stack_lookup(__func__) != s) {
> > +		printf("[%s():%u] failed to lookup a stack\n",
> > +		       __func__, __LINE__);
> > +		goto fail_test;
> > +	}
> > +
> > +	if (rte_stack_count(s) != 0) {
> > +		printf("[%s():%u] stack count: %u (expected 0)\n",
> > +		       __func__, __LINE__, rte_stack_count(s));
> > +		goto fail_test;
> > +	}
> > +
> > +	if (rte_stack_free_count(s) != STACK_SIZE) {
> > +		printf("[%s():%u] stack free count: %u (expected %u)\n",
> > +		       __func__, __LINE__, rte_stack_count(s), STACK_SIZE);
> > +		goto fail_test;
> > +	}
> > +
> > +	ret = test_stack_push_pop(s, obj_table, 1);
> > +	if (ret) {
> > +		printf("[%s():%u] Single object push/pop failed\n",
> > +		       __func__, __LINE__);
> > +		goto fail_test;
> > +	}
> > +
> > +	ret = test_stack_push_pop(s, obj_table, MAX_BULK);
> > +	if (ret) {
> > +		printf("[%s():%u] Bulk object push/pop failed\n",
> > +		       __func__, __LINE__);
> > +		goto fail_test;
> > +	}
> > +
> > +	ret = rte_stack_push(s, obj_table, 2 * STACK_SIZE);
> > +	if (ret != 0) {
> > +		printf("[%s():%u] Excess objects push succeeded\n",
> > +		       __func__, __LINE__);
> > +		goto fail_test;
> > +	}
> > +
> > +	ret = rte_stack_pop(s, obj_table, 1);
> > +	if (ret != 0) {
> > +		printf("[%s():%u] Empty stack pop succeeded\n",
> > +		       __func__, __LINE__);
> > +		goto fail_test;
> > +	}
> > +
> > +	ret = 0;
> > +
> > +fail_test:
> > +	rte_stack_free(s);
> > +
> > +	if (obj_table != NULL)
> > +		rte_free(obj_table);
> > +
> 
> The if can be removed.

Ah, I didn't know rte_free() checks for NULL. Will remove.

> 
> > +static int
> > +test_stack_name_length(void)
> > +{
> > +	char name[RTE_STACK_NAMESIZE + 1];
> > +	struct rte_stack *s;
> > +
> > +	memset(name, 's', sizeof(name));
> > +
> > +	s = rte_stack_create(name, STACK_SIZE, rte_socket_id(), 0);
> > +	if (s != NULL) {
> > +		printf("[%s():%u] Failed to prevent long name\n",
> > +		       __func__, __LINE__);
> > +		return -1;
> > +	}
> 
> Here, "name" is not a valid string (no \0 at the end). It does not hurt because the
> length check is properly done in the lib, but we could imagine that the wrong
> name is logged by the library on error, which would trigger a crash here. So I
> suggest to pass a valid string instead.

Good catch. Will fix.

Thanks,
Gage



More information about the dev mailing list