From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f178.google.com (mail-wr0-f178.google.com [209.85.128.178]) by dpdk.org (Postfix) with ESMTP id EC8BA108D for ; Fri, 31 Mar 2017 15:59:56 +0200 (CEST) Received: by mail-wr0-f178.google.com with SMTP id w11so106454771wrc.3 for ; Fri, 31 Mar 2017 06:59:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=turOE44X6q/sWKNQR3LLP/e3MKMXDH2WBnLaZAYX2xk=; b=jsiHjYRkI6hX/kUvXv+3KdXQVzRHsNggvcxVgkJR08EJJreU2TxXKeZXF1IcZobztP 7rKjtF4GnhF251QDzr7mLkwGvWUUXmaDW+ScvDjRfl5amUgIJVl4zv088qFNhrBHdS9c f8+6sbrN7Xjp2F7NyxIGWamX/anSqctExPla7uPVvt59F5lgeWTsnIbvwKd46HddHd3S 3cmmy5kEEXK28ALyAjryokOUpl83rtOHYh1iqO+OqWiMaBiVJBRxetDlWmthc0fn8t7t PTA8IxXNIXPURNYsyLUcMV4L393HWypa1u7i3JC5htPDq5aMwwMXSu+LGJiEDQg2K1l6 V+Pg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=turOE44X6q/sWKNQR3LLP/e3MKMXDH2WBnLaZAYX2xk=; b=hP/y7cpqKsWrHO2+9kOdNUkFmC+/OlDjBVl2fT8fAB02iVQIBRrWN5wfPGVA2MvZvx JUrlqHHwrZ0slQzKWCyRVHgsj3Je3Bf8eBwaHIYVpgUxwSUZPMGJ3HZcU3s+BBykhCfR Moymbvc5e+AetVpG0w/k1o/9QwVejeM+4YzS43ISntDjiYyMOJJDCnQy0LUTWoSFKqSG SZ/ixMbecDKLI/Jq1EaCwrS+llhDYhCorgL4WUk2ohornKyqfz/r3fkDjN8EVwP9kSej pKjovHBkI7GvuN6lGR2X5sihtTeOSBdTm6QuVs5sW4s/eRUVSRcQEzZbvbdjD/Df5anP Xk3Q== X-Gm-Message-State: AFeK/H06Zu+g8+9/QNUZ+pv/nm5PBWt9xXMcZNa7zSomSjzNCpJ1nZGAIxrHnrPuVYupqKdt X-Received: by 10.223.155.130 with SMTP id d2mr3216416wrc.67.1490968796484; Fri, 31 Mar 2017 06:59:56 -0700 (PDT) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id y22sm6850281wry.51.2017.03.31.06.59.56 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 31 Mar 2017 06:59:56 -0700 (PDT) Date: Fri, 31 Mar 2017 15:59:53 +0200 From: Olivier Matz To: Shreyansh Jain Cc: , Message-ID: <20170331155953.65198108@platinum> In-Reply-To: <1490955469-16216-1-git-send-email-shreyansh.jain@nxp.com> References: <1490955469-16216-1-git-send-email-shreyansh.jain@nxp.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC PATCH] test/test: support default mempool autotest 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: Fri, 31 Mar 2017 13:59:57 -0000 Hi Shreyansh, On Fri, 31 Mar 2017 15:47:49 +0530, Shreyansh Jain wrote: > Mempool test currently supports: > * ring_mp_mc > * stack > > In case a new mempool handler is added, there are multiple options > for supporting that in the mempool autotest: > 1. Like the patch below, adding a new default pool options > So, ring* + stack + default (which can be 'stack' or 'ring') > * This way, whatever the value of RTE_MBUF_DEFAULT_MEMPOOL_OPS is set, > it would be verified. > * even if that means duplicating some test (for example when "stack" is > set as default and it already part of standard test) > > 2. Removing stack handler as standard, and moving only to one specified > by RTE_MBUF_DEFAULT_MEMPOOL_OPS (+ existing ring*) > * It still leaves space for duplication of ring_mp_mc in case that is > set to default (as in case of master tree) > > 3. Iterating over the list of mempool handlers and performing a set > or predefined tests > * reqiures quite a lot of rewrite of mempool autotest > * specially, allowing some special tests (cache/no-cache) cases when > a set of variables in loop are being used, would be tricky > > 4. only checking the default pool set by RTE_* macro > * In case a user has build DPDK using a configured value, probably it > expected that application (or custom applications) would use that > default handler. > * would also mean that non-default (non RTE_* value) would not be tested > even though they are being used. > > The decision above would impact how new mempool handlers are added and > how their testing (API verification) can be done without impacting the > mempool_autotest file everytime. > > Signed-off-by: Shreyansh Jain I'd prefer 3-, it looks to be the most complete. But I think 1- is ok for now. > > --- > test/test/test_mempool.c | 35 +++++++++++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/test/test/test_mempool.c b/test/test/test_mempool.c > index b9880b3..aefbf80 100644 > --- a/test/test/test_mempool.c > +++ b/test/test/test_mempool.c > @@ -509,9 +509,11 @@ walk_cb(struct rte_mempool *mp, void *userdata __rte_unused) > static int > test_mempool(void) > { > + int ret = -1; > struct rte_mempool *mp_cache = NULL; > struct rte_mempool *mp_nocache = NULL; > struct rte_mempool *mp_stack = NULL; > + struct rte_mempool *default_pool = NULL; > > rte_atomic32_init(&synchro); > > @@ -561,6 +563,30 @@ test_mempool(void) > } > rte_mempool_obj_iter(mp_stack, my_obj_init, NULL); > > + /* Create a mempool based on Default handler, if not "stack" */ > + printf("Testing %s mempool handler\n", > + RTE_MBUF_DEFAULT_MEMPOOL_OPS); > + default_pool = rte_mempool_create_empty("default_pool", > + MEMPOOL_SIZE, > + MEMPOOL_ELT_SIZE, > + RTE_MEMPOOL_CACHE_MAX_SIZE, 0, > + SOCKET_ID_ANY, 0); > + > + if (default_pool == NULL) { > + printf("cannot allocate default mempool\n"); > + goto err; > + } > + if (rte_mempool_set_ops_byname(default_pool, > + RTE_MBUF_DEFAULT_MEMPOOL_OPS, NULL) < 0) { > + printf("cannot set default handler\n"); > + goto err; > + } > + if (rte_mempool_populate_default(default_pool) < 0) { > + printf("cannot populate default mempool\n"); > + goto err; > + } > + rte_mempool_obj_iter(default_pool, my_obj_init, NULL); > + > /* retrieve the mempool from its name */ > if (rte_mempool_lookup("test_nocache") != mp_nocache) { > printf("Cannot lookup mempool from its name\n"); > @@ -605,15 +631,20 @@ test_mempool(void) > if (test_mempool_basic(mp_stack, 1) < 0) > goto err; > > + if (test_mempool_basic(default_pool, 1) < 0) > + goto err; > + > rte_mempool_list_dump(stdout); > > - return 0; > + ret = 0; looks it also fixes a memory leak :) Can you please move the fix in another patch? Thanks, Olivier