From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gage.eads@intel.com>
Received: from mga17.intel.com (mga17.intel.com [192.55.52.151])
 by dpdk.org (Postfix) with ESMTP id 226892B87
 for <dev@dpdk.org>; Thu, 28 Feb 2019 06:11:22 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga008.fm.intel.com ([10.253.24.58])
 by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 27 Feb 2019 21:11:22 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.58,422,1544515200"; d="scan'208";a="127838101"
Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202])
 by fmsmga008.fm.intel.com with ESMTP; 27 Feb 2019 21:11:22 -0800
Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by
 fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS)
 id 14.3.408.0; Wed, 27 Feb 2019 21:11:22 -0800
Received: from fmsmsx108.amr.corp.intel.com ([169.254.9.86]) by
 FMSMSX152.amr.corp.intel.com ([169.254.6.9]) with mapi id 14.03.0415.000;
 Wed, 27 Feb 2019 21:11:21 -0800
From: "Eads, Gage" <gage.eads@intel.com>
To: Olivier Matz <olivier.matz@6wind.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "arybchenko@solarflare.com"
 <arybchenko@solarflare.com>, "Richardson, Bruce"
 <bruce.richardson@intel.com>, "Ananyev, Konstantin"
 <konstantin.ananyev@intel.com>, "gavin.hu@arm.com" <gavin.hu@arm.com>,
 "Honnappa.Nagarahalli@arm.com" <Honnappa.Nagarahalli@arm.com>, "nd@arm.com"
 <nd@arm.com>, "thomas@monjalon.net" <thomas@monjalon.net>
Thread-Topic: [PATCH 3/7] test/stack: add stack test
Thread-Index: AQHUzPk7V/iCRrR3yEKlwuphxswMqKX0kQtw
Date: Thu, 28 Feb 2019 05:11:20 +0000
Message-ID: <9184057F7FC11744A2107296B6B8EB1E541ECD63@FMSMSX108.amr.corp.intel.com>
References: <20190222160655.3346-1-gage.eads@intel.com>
 <20190222160655.3346-4-gage.eads@intel.com>
 <20190225105914.2zsmvp2ighvsbeqi@platinum>
In-Reply-To: <20190225105914.2zsmvp2ighvsbeqi@platinum>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiN2I5ZDA4ODQtMjIxNS00OWZjLTlhZWEtZGE4NDAzNTFhYjljIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUnNwN2RRRThxXC95XC9xOVoxcDlYcTQzRVlOXC9IcmZFYmFzSXVcL3hTcmhaT2xydDBkcFBPazl5WEhQZU9rcjdFc04ifQ==
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.400.15
dlp-reaction: no-action
x-originating-ip: [10.1.200.106]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 3/7] test/stack: add stack test
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 28 Feb 2019 05:11:23 -0000



> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, February 25, 2019 4:59 AM
> To: Eads, Gage <gage.eads@intel.com>
> Cc: dev@dpdk.org; arybchenko@solarflare.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com; nd@arm.com; thomas@monjalon.net
> Subject: Re: [PATCH 3/7] test/stack: add stack test
>=20
> 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 availab=
le
> lcores.
> >
> > Signed-off-by: Gage Eads <gage.eads@intel.com>
>=20
> [...]
>=20
> > --- /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;
>=20
> 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 consisten=
t to
> have the same method for both. I suggest to allocate in heap to avoid a s=
tack
> overflow if STACK_SIZE is increased in the future.
>=20

Sure, I'll make popped_objs dynamically allocated.

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

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

>=20
> > +static int
> > +test_stack_name_length(void)
> > +{
> > +	char name[RTE_STACK_NAMESIZE + 1];
> > +	struct rte_stack *s;
> > +
> > +	memset(name, 's', sizeof(name));
> > +
> > +	s =3D rte_stack_create(name, STACK_SIZE, rte_socket_id(), 0);
> > +	if (s !=3D NULL) {
> > +		printf("[%s():%u] Failed to prevent long name\n",
> > +		       __func__, __LINE__);
> > +		return -1;
> > +	}
>=20
> Here, "name" is not a valid string (no \0 at the end). It does not hurt b=
ecause the
> length check is properly done in the lib, but we could imagine that the w=
rong
> 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