From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 226892B87 for ; 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" To: Olivier Matz CC: "dev@dpdk.org" , "arybchenko@solarflare.com" , "Richardson, Bruce" , "Ananyev, Konstantin" , "gavin.hu@arm.com" , "Honnappa.Nagarahalli@arm.com" , "nd@arm.com" , "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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 > Cc: dev@dpdk.org; arybchenko@solarflare.com; Richardson, Bruce > ; Ananyev, Konstantin > ; 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 >=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 > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#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