* [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var
@ 2017-06-08 14:28 Santosh Shukla
2017-06-16 14:35 ` Olivier Matz
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Santosh Shukla @ 2017-06-08 14:28 UTC (permalink / raw)
To: olivier.matz, dev; +Cc: thomas, shreyansh.jain, Santosh Shukla, stable
Let test_mbuf alloc and free mempool.
Cc: stable@dpdk.org
Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
---
v1 --> v2:
- Clubed v1 two patch into 1 patch per Olivier review comment [1]
[1] http://dpdk.org/dev/patchwork/patch/24237/
test/test/test_mbuf.c | 148 ++++++++++++++++++++++++++++----------------------
1 file changed, 82 insertions(+), 66 deletions(-)
diff --git a/test/test/test_mbuf.c b/test/test/test_mbuf.c
index d3ea812e5..d6cf4d611 100644
--- a/test/test/test_mbuf.c
+++ b/test/test/test_mbuf.c
@@ -82,13 +82,8 @@
#define MAKE_STRING(x) # x
-static struct rte_mempool *pktmbuf_pool = NULL;
-static struct rte_mempool *pktmbuf_pool2 = NULL;
-
#ifdef RTE_MBUF_REFCNT_ATOMIC
-static struct rte_mempool *refcnt_pool = NULL;
-static struct rte_ring *refcnt_mbuf_ring = NULL;
static volatile uint32_t refcnt_stop_slaves;
static unsigned refcnt_lcore[RTE_MAX_LCORE];
@@ -144,7 +139,7 @@ static unsigned refcnt_lcore[RTE_MAX_LCORE];
* test data manipulation in mbuf with non-ascii data
*/
static int
-test_pktmbuf_with_non_ascii_data(void)
+test_pktmbuf_with_non_ascii_data(struct rte_mempool *pktmbuf_pool)
{
struct rte_mbuf *m = NULL;
char *data;
@@ -182,7 +177,7 @@ test_pktmbuf_with_non_ascii_data(void)
* test data manipulation in mbuf
*/
static int
-test_one_pktmbuf(void)
+test_one_pktmbuf(struct rte_mempool *pktmbuf_pool)
{
struct rte_mbuf *m = NULL;
char *data, *data2, *hdr;
@@ -328,7 +323,7 @@ test_one_pktmbuf(void)
}
static int
-testclone_testupdate_testdetach(void)
+testclone_testupdate_testdetach(struct rte_mempool *pktmbuf_pool)
{
struct rte_mbuf *m = NULL;
struct rte_mbuf *clone = NULL;
@@ -432,7 +427,8 @@ testclone_testupdate_testdetach(void)
}
static int
-test_attach_from_different_pool(void)
+test_attach_from_different_pool(struct rte_mempool *pktmbuf_pool,
+ struct rte_mempool *pktmbuf_pool2)
{
struct rte_mbuf *m = NULL;
struct rte_mbuf *clone = NULL;
@@ -542,7 +538,7 @@ test_attach_from_different_pool(void)
* test allocation and free of mbufs
*/
static int
-test_pktmbuf_pool(void)
+test_pktmbuf_pool(struct rte_mempool *pktmbuf_pool)
{
unsigned i;
struct rte_mbuf *m[NB_MBUF];
@@ -583,7 +579,7 @@ test_pktmbuf_pool(void)
* test that the pointer to the data on a packet mbuf is set properly
*/
static int
-test_pktmbuf_pool_ptr(void)
+test_pktmbuf_pool_ptr(struct rte_mempool *pktmbuf_pool)
{
unsigned i;
struct rte_mbuf *m[NB_MBUF];
@@ -636,7 +632,7 @@ test_pktmbuf_pool_ptr(void)
}
static int
-test_pktmbuf_free_segment(void)
+test_pktmbuf_free_segment(struct rte_mempool *pktmbuf_pool)
{
unsigned i;
struct rte_mbuf *m[NB_MBUF];
@@ -680,10 +676,11 @@ test_pktmbuf_free_segment(void)
#ifdef RTE_MBUF_REFCNT_ATOMIC
static int
-test_refcnt_slave(__attribute__((unused)) void *arg)
+test_refcnt_slave(void *arg)
{
unsigned lcore, free;
void *mp = 0;
+ struct rte_ring *refcnt_mbuf_ring = arg;
lcore = rte_lcore_id();
printf("%s started at lcore %u\n", __func__, lcore);
@@ -704,7 +701,9 @@ test_refcnt_slave(__attribute__((unused)) void *arg)
}
static void
-test_refcnt_iter(unsigned lcore, unsigned iter)
+test_refcnt_iter(unsigned int lcore, unsigned int iter,
+ struct rte_mempool *refcnt_pool,
+ struct rte_ring *refcnt_mbuf_ring)
{
uint16_t ref;
unsigned i, n, tref, wn;
@@ -760,7 +759,8 @@ test_refcnt_iter(unsigned lcore, unsigned iter)
}
static int
-test_refcnt_master(void)
+test_refcnt_master(struct rte_mempool *refcnt_pool,
+ struct rte_ring *refcnt_mbuf_ring)
{
unsigned i, lcore;
@@ -768,7 +768,7 @@ test_refcnt_master(void)
printf("%s started at lcore %u\n", __func__, lcore);
for (i = 0; i != REFCNT_MAX_ITER; i++)
- test_refcnt_iter(lcore, i);
+ test_refcnt_iter(lcore, i, refcnt_pool, refcnt_mbuf_ring);
refcnt_stop_slaves = 1;
rte_wmb();
@@ -783,9 +783,10 @@ static int
test_refcnt_mbuf(void)
{
#ifdef RTE_MBUF_REFCNT_ATOMIC
-
unsigned lnum, master, slave, tref;
-
+ int ret = -1;
+ struct rte_mempool *refcnt_pool = NULL;
+ struct rte_ring *refcnt_mbuf_ring = NULL;
if ((lnum = rte_lcore_count()) == 1) {
printf("skipping %s, number of lcores: %u is not enough\n",
@@ -797,31 +798,31 @@ test_refcnt_mbuf(void)
/* create refcnt pool & ring if they don't exist */
- if (refcnt_pool == NULL &&
- (refcnt_pool = rte_pktmbuf_pool_create(
- MAKE_STRING(refcnt_pool),
- REFCNT_MBUF_NUM, 0, 0, 0,
- SOCKET_ID_ANY)) == NULL) {
+ refcnt_pool = rte_pktmbuf_pool_create(MAKE_STRING(refcnt_pool),
+ REFCNT_MBUF_NUM, 0, 0, 0,
+ SOCKET_ID_ANY);
+ if (refcnt_pool == NULL) {
printf("%s: cannot allocate " MAKE_STRING(refcnt_pool) "\n",
__func__);
return -1;
}
- if (refcnt_mbuf_ring == NULL &&
- (refcnt_mbuf_ring = rte_ring_create("refcnt_mbuf_ring",
+ refcnt_mbuf_ring = rte_ring_create("refcnt_mbuf_ring",
rte_align32pow2(REFCNT_RING_SIZE), SOCKET_ID_ANY,
- RING_F_SP_ENQ)) == NULL) {
+ RING_F_SP_ENQ);
+ if (refcnt_mbuf_ring == NULL) {
printf("%s: cannot allocate " MAKE_STRING(refcnt_mbuf_ring)
"\n", __func__);
- return -1;
+ goto err;
}
refcnt_stop_slaves = 0;
memset(refcnt_lcore, 0, sizeof (refcnt_lcore));
- rte_eal_mp_remote_launch(test_refcnt_slave, NULL, SKIP_MASTER);
+ rte_eal_mp_remote_launch(test_refcnt_slave, refcnt_mbuf_ring,
+ SKIP_MASTER);
- test_refcnt_master();
+ test_refcnt_master(refcnt_pool, refcnt_mbuf_ring);
rte_eal_mp_wait_lcore();
@@ -839,8 +840,15 @@ test_refcnt_mbuf(void)
rte_mempool_dump(stdout, refcnt_pool);
rte_ring_dump(stdout, refcnt_mbuf_ring);
-#endif
+ ret = 0;
+
+err:
+ rte_mempool_free(refcnt_pool);
+ rte_ring_free(refcnt_mbuf_ring);
+ return ret;
+#else
return 0;
+#endif
}
#include <unistd.h>
@@ -870,7 +878,7 @@ verify_mbuf_check_panics(struct rte_mbuf *buf)
}
static int
-test_failing_mbuf_sanity_check(void)
+test_failing_mbuf_sanity_check(struct rte_mempool *pktmbuf_pool)
{
struct rte_mbuf *buf;
struct rte_mbuf badbuf;
@@ -931,7 +939,9 @@ test_failing_mbuf_sanity_check(void)
}
static int
-test_mbuf_linearize(int pkt_len, int nb_segs) {
+test_mbuf_linearize(struct rte_mempool *pktmbuf_pool, int pkt_len,
+ int nb_segs)
+{
struct rte_mbuf *m = NULL, *mbuf = NULL;
uint8_t *data;
@@ -1022,7 +1032,7 @@ test_mbuf_linearize(int pkt_len, int nb_segs) {
}
static int
-test_mbuf_linearize_check(void)
+test_mbuf_linearize_check(struct rte_mempool *pktmbuf_pool)
{
struct test_mbuf_array {
int size;
@@ -1039,7 +1049,7 @@ test_mbuf_linearize_check(void)
printf("Test mbuf linearize API\n");
for (i = 0; i < RTE_DIM(mbuf_array); i++)
- if (test_mbuf_linearize(mbuf_array[i].size,
+ if (test_mbuf_linearize(pktmbuf_pool, mbuf_array[i].size,
mbuf_array[i].nb_segs)) {
printf("Test failed for %d, %d\n", mbuf_array[i].size,
mbuf_array[i].nb_segs);
@@ -1052,53 +1062,54 @@ test_mbuf_linearize_check(void)
static int
test_mbuf(void)
{
+ int ret = -1;
+ struct rte_mempool *pktmbuf_pool = NULL;
+ struct rte_mempool *pktmbuf_pool2 = NULL;
+
+
RTE_BUILD_BUG_ON(sizeof(struct rte_mbuf) != RTE_CACHE_LINE_MIN_SIZE * 2);
/* create pktmbuf pool if it does not exist */
- if (pktmbuf_pool == NULL) {
- pktmbuf_pool = rte_pktmbuf_pool_create("test_pktmbuf_pool",
+ pktmbuf_pool = rte_pktmbuf_pool_create("test_pktmbuf_pool",
NB_MBUF, 32, 0, MBUF_DATA_SIZE, SOCKET_ID_ANY);
- }
if (pktmbuf_pool == NULL) {
printf("cannot allocate mbuf pool\n");
- return -1;
+ goto err;
}
/* create a specific pktmbuf pool with a priv_size != 0 and no data
* room size */
- if (pktmbuf_pool2 == NULL) {
- pktmbuf_pool2 = rte_pktmbuf_pool_create("test_pktmbuf_pool2",
+ pktmbuf_pool2 = rte_pktmbuf_pool_create("test_pktmbuf_pool2",
NB_MBUF, 32, MBUF2_PRIV_SIZE, 0, SOCKET_ID_ANY);
- }
if (pktmbuf_pool2 == NULL) {
printf("cannot allocate mbuf pool\n");
- return -1;
+ goto err;
}
/* test multiple mbuf alloc */
- if (test_pktmbuf_pool() < 0) {
+ if (test_pktmbuf_pool(pktmbuf_pool) < 0) {
printf("test_mbuf_pool() failed\n");
- return -1;
+ goto err;
}
/* do it another time to check that all mbufs were freed */
- if (test_pktmbuf_pool() < 0) {
+ if (test_pktmbuf_pool(pktmbuf_pool) < 0) {
printf("test_mbuf_pool() failed (2)\n");
- return -1;
+ goto err;
}
/* test that the pointer to the data on a packet mbuf is set properly */
- if (test_pktmbuf_pool_ptr() < 0) {
+ if (test_pktmbuf_pool_ptr(pktmbuf_pool) < 0) {
printf("test_pktmbuf_pool_ptr() failed\n");
- return -1;
+ goto err;
}
/* test data manipulation in mbuf */
- if (test_one_pktmbuf() < 0) {
+ if (test_one_pktmbuf(pktmbuf_pool) < 0) {
printf("test_one_mbuf() failed\n");
- return -1;
+ goto err;
}
@@ -1106,47 +1117,52 @@ test_mbuf(void)
* do it another time, to check that allocation reinitialize
* the mbuf correctly
*/
- if (test_one_pktmbuf() < 0) {
+ if (test_one_pktmbuf(pktmbuf_pool) < 0) {
printf("test_one_mbuf() failed (2)\n");
- return -1;
+ goto err;
}
- if (test_pktmbuf_with_non_ascii_data() < 0) {
+ if (test_pktmbuf_with_non_ascii_data(pktmbuf_pool) < 0) {
printf("test_pktmbuf_with_non_ascii_data() failed\n");
- return -1;
+ goto err;
}
/* test free pktmbuf segment one by one */
- if (test_pktmbuf_free_segment() < 0) {
+ if (test_pktmbuf_free_segment(pktmbuf_pool) < 0) {
printf("test_pktmbuf_free_segment() failed.\n");
- return -1;
+ goto err;
}
- if (testclone_testupdate_testdetach()<0){
+ if (testclone_testupdate_testdetach(pktmbuf_pool) < 0) {
printf("testclone_and_testupdate() failed \n");
- return -1;
+ goto err;
}
- if (test_attach_from_different_pool() < 0) {
+ if (test_attach_from_different_pool(pktmbuf_pool, pktmbuf_pool2) < 0) {
printf("test_attach_from_different_pool() failed\n");
- return -1;
+ goto err;
}
if (test_refcnt_mbuf()<0){
printf("test_refcnt_mbuf() failed \n");
- return -1;
+ goto err;
}
- if (test_failing_mbuf_sanity_check() < 0) {
+ if (test_failing_mbuf_sanity_check(pktmbuf_pool) < 0) {
printf("test_failing_mbuf_sanity_check() failed\n");
- return -1;
+ goto err;
}
- if (test_mbuf_linearize_check() < 0) {
+ if (test_mbuf_linearize_check(pktmbuf_pool) < 0) {
printf("test_mbuf_linearize_check() failed\n");
- return -1;
+ goto err;
}
- return 0;
+ ret = 0;
+
+err:
+ rte_mempool_free(pktmbuf_pool);
+ rte_mempool_free(pktmbuf_pool2);
+ return ret;
}
REGISTER_TEST_COMMAND(mbuf_autotest, test_mbuf);
--
2.13.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var
2017-06-08 14:28 [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var Santosh Shukla
@ 2017-06-16 14:35 ` Olivier Matz
2017-06-19 20:37 ` Thomas Monjalon
2017-06-26 10:04 ` [dpdk-dev] [PATCH v3] " Santosh Shukla
2 siblings, 0 replies; 10+ messages in thread
From: Olivier Matz @ 2017-06-16 14:35 UTC (permalink / raw)
To: Santosh Shukla; +Cc: dev, thomas, shreyansh.jain, stable
On Thu, 8 Jun 2017 14:28:31 +0000, Santosh Shukla <santosh.shukla@caviumnetworks.com> wrote:
> Let test_mbuf alloc and free mempool.
>
> Cc: stable@dpdk.org
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var
2017-06-08 14:28 [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var Santosh Shukla
2017-06-16 14:35 ` Olivier Matz
@ 2017-06-19 20:37 ` Thomas Monjalon
2017-06-20 4:14 ` santosh
2017-06-26 10:04 ` [dpdk-dev] [PATCH v3] " Santosh Shukla
2 siblings, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2017-06-19 20:37 UTC (permalink / raw)
To: Santosh Shukla; +Cc: dev, olivier.matz, shreyansh.jain, stable
08/06/2017 16:28, Santosh Shukla:
> Let test_mbuf alloc and free mempool.
>
> Cc: stable@dpdk.org
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Why Cc stable?
Is it fixing something?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var
2017-06-19 20:37 ` Thomas Monjalon
@ 2017-06-20 4:14 ` santosh
2017-06-20 7:35 ` Thomas Monjalon
0 siblings, 1 reply; 10+ messages in thread
From: santosh @ 2017-06-20 4:14 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev, olivier.matz, shreyansh.jain, stable
On Tuesday 20 June 2017 02:07 AM, Thomas Monjalon wrote:
> 08/06/2017 16:28, Santosh Shukla:
>> Let test_mbuf alloc and free mempool.
>>
>> Cc: stable@dpdk.org
>> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Why Cc stable?
> Is it fixing something?
>
w/o this fix, application can't run more than once.
Reason: Static allocation of resources and exiting w/o freeing so leak.
Patch makes app resource handling dynamic and Now user could run
test more than once and app exits gracefully. thats why Cc: stable a need (IHMO).
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var
2017-06-20 4:14 ` santosh
@ 2017-06-20 7:35 ` Thomas Monjalon
2017-06-20 8:22 ` Yuanhan Liu
2017-06-22 5:29 ` santosh
0 siblings, 2 replies; 10+ messages in thread
From: Thomas Monjalon @ 2017-06-20 7:35 UTC (permalink / raw)
To: santosh; +Cc: dev, olivier.matz, shreyansh.jain, stable
20/06/2017 06:14, santosh:
> On Tuesday 20 June 2017 02:07 AM, Thomas Monjalon wrote:
>
> > 08/06/2017 16:28, Santosh Shukla:
> >> Let test_mbuf alloc and free mempool.
> >>
> >> Cc: stable@dpdk.org
> >> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> > Why Cc stable?
> > Is it fixing something?
> >
> w/o this fix, application can't run more than once.
> Reason: Static allocation of resources and exiting w/o freeing so leak.
>
> Patch makes app resource handling dynamic and Now user could run
> test more than once and app exits gracefully. thats why Cc: stable a need (IHMO).
> Thanks.
OK
So we need a Fixes: tag in order to be able to guess which
release it should be backported to.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var
2017-06-20 7:35 ` Thomas Monjalon
@ 2017-06-20 8:22 ` Yuanhan Liu
2017-06-22 5:31 ` santosh
2017-06-22 5:29 ` santosh
1 sibling, 1 reply; 10+ messages in thread
From: Yuanhan Liu @ 2017-06-20 8:22 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: santosh, dev, olivier.matz, shreyansh.jain, stable
On Tue, Jun 20, 2017 at 09:35:07AM +0200, Thomas Monjalon wrote:
> 20/06/2017 06:14, santosh:
> > On Tuesday 20 June 2017 02:07 AM, Thomas Monjalon wrote:
> >
> > > 08/06/2017 16:28, Santosh Shukla:
> > >> Let test_mbuf alloc and free mempool.
> > >>
> > >> Cc: stable@dpdk.org
> > >> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> > > Why Cc stable?
> > > Is it fixing something?
> > >
> > w/o this fix, application can't run more than once.
> > Reason: Static allocation of resources and exiting w/o freeing so leak.
> >
> > Patch makes app resource handling dynamic and Now user could run
> > test more than once and app exits gracefully. thats why Cc: stable a need (IHMO).
> > Thanks.
>
> OK
> So we need a Fixes: tag in order to be able to guess which
> release it should be backported to.
Also, I would suggest you to include above info (the real issue) in
the commit log.
--yliu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var
2017-06-20 8:22 ` Yuanhan Liu
@ 2017-06-22 5:31 ` santosh
0 siblings, 0 replies; 10+ messages in thread
From: santosh @ 2017-06-22 5:31 UTC (permalink / raw)
To: Yuanhan Liu, Thomas Monjalon; +Cc: dev, olivier.matz, shreyansh.jain, stable
Hi Yuanhan,
On Tuesday 20 June 2017 01:52 PM, Yuanhan Liu wrote:
> On Tue, Jun 20, 2017 at 09:35:07AM +0200, Thomas Monjalon wrote:
>> 20/06/2017 06:14, santosh:
>>> On Tuesday 20 June 2017 02:07 AM, Thomas Monjalon wrote:
>>>
>>>> 08/06/2017 16:28, Santosh Shukla:
>>>>> Let test_mbuf alloc and free mempool.
>>>>>
>>>>> Cc: stable@dpdk.org
>>>>> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
>>>> Why Cc stable?
>>>> Is it fixing something?
>>>>
>>> w/o this fix, application can't run more than once.
>>> Reason: Static allocation of resources and exiting w/o freeing so leak.
>>>
>>> Patch makes app resource handling dynamic and Now user could run
>>> test more than once and app exits gracefully. thats why Cc: stable a need (IHMO).
>>> Thanks.
>> OK
>> So we need a Fixes: tag in order to be able to guess which
>> release it should be backported to.
> Also, I would suggest you to include above info (the real issue) in
> the commit log.
>
> --yliu
Yes, We'll reword and post v3 but won't be a stable candidate.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var
2017-06-20 7:35 ` Thomas Monjalon
2017-06-20 8:22 ` Yuanhan Liu
@ 2017-06-22 5:29 ` santosh
1 sibling, 0 replies; 10+ messages in thread
From: santosh @ 2017-06-22 5:29 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev, olivier.matz, shreyansh.jain, stable
On Tuesday 20 June 2017 01:05 PM, Thomas Monjalon wrote:
> 20/06/2017 06:14, santosh:
>> On Tuesday 20 June 2017 02:07 AM, Thomas Monjalon wrote:
>>
>>> 08/06/2017 16:28, Santosh Shukla:
>>>> Let test_mbuf alloc and free mempool.
>>>>
>>>> Cc: stable@dpdk.org
>>>> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
>>> Why Cc stable?
>>> Is it fixing something?
>>>
>> w/o this fix, application can't run more than once.
>> Reason: Static allocation of resources and exiting w/o freeing so leak.
>>
>> Patch makes app resource handling dynamic and Now user could run
>> test more than once and app exits gracefully. thats why Cc: stable a need (IHMO).
>> Thanks.
> OK
> So we need a Fixes: tag in order to be able to guess which
> release it should be backported to.
Hmmm, By git blame, Its a very early commit(af75078fe : first public release). And
I think this patch can't be back-ported. IMO, I would prefer to remove Cc: stable@
from git description area, suggestion? is it fine.
Yes, I will reword the git description.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [dpdk-dev] [PATCH v3] test/test_mbuf: remove mempool global var
2017-06-08 14:28 [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var Santosh Shukla
2017-06-16 14:35 ` Olivier Matz
2017-06-19 20:37 ` Thomas Monjalon
@ 2017-06-26 10:04 ` Santosh Shukla
2017-06-27 17:09 ` Thomas Monjalon
2 siblings, 1 reply; 10+ messages in thread
From: Santosh Shukla @ 2017-06-26 10:04 UTC (permalink / raw)
To: olivier.matz, thomas.monjalon, dev; +Cc: yuanhan.liu, Santosh Shukla
Currently, pool resources are allocated statically
and are not freed. Results of that test can not run more than once.
Fix removes static dependency from test application and
now allocating and freeing resources dynamically.
Test runs for more than once.
Fixes: af75078fece3 ("first public release")
Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
v1 --> v2:
- Clubed v1 two patch into 1 patch per Olivier review comment [1]
v2 --> v3:
- Reword the description. refer [2].
- Removed Cc: stable@dpdk.org.
[1] http://dpdk.org/dev/patchwork/patch/24237/
[2] http://dpdk.org/dev/patchwork/patch/25202/
test/test/test_mbuf.c | 148 ++++++++++++++++++++++++++++----------------------
1 file changed, 82 insertions(+), 66 deletions(-)
diff --git a/test/test/test_mbuf.c b/test/test/test_mbuf.c
index 758402d6b..2057a5182 100644
--- a/test/test/test_mbuf.c
+++ b/test/test/test_mbuf.c
@@ -82,13 +82,8 @@
#define MAKE_STRING(x) # x
-static struct rte_mempool *pktmbuf_pool = NULL;
-static struct rte_mempool *pktmbuf_pool2 = NULL;
-
#ifdef RTE_MBUF_REFCNT_ATOMIC
-static struct rte_mempool *refcnt_pool = NULL;
-static struct rte_ring *refcnt_mbuf_ring = NULL;
static volatile uint32_t refcnt_stop_slaves;
static unsigned refcnt_lcore[RTE_MAX_LCORE];
@@ -144,7 +139,7 @@ static unsigned refcnt_lcore[RTE_MAX_LCORE];
* test data manipulation in mbuf with non-ascii data
*/
static int
-test_pktmbuf_with_non_ascii_data(void)
+test_pktmbuf_with_non_ascii_data(struct rte_mempool *pktmbuf_pool)
{
struct rte_mbuf *m = NULL;
char *data;
@@ -182,7 +177,7 @@ test_pktmbuf_with_non_ascii_data(void)
* test data manipulation in mbuf
*/
static int
-test_one_pktmbuf(void)
+test_one_pktmbuf(struct rte_mempool *pktmbuf_pool)
{
struct rte_mbuf *m = NULL;
char *data, *data2, *hdr;
@@ -328,7 +323,7 @@ test_one_pktmbuf(void)
}
static int
-testclone_testupdate_testdetach(void)
+testclone_testupdate_testdetach(struct rte_mempool *pktmbuf_pool)
{
struct rte_mbuf *m = NULL;
struct rte_mbuf *clone = NULL;
@@ -432,7 +427,8 @@ testclone_testupdate_testdetach(void)
}
static int
-test_attach_from_different_pool(void)
+test_attach_from_different_pool(struct rte_mempool *pktmbuf_pool,
+ struct rte_mempool *pktmbuf_pool2)
{
struct rte_mbuf *m = NULL;
struct rte_mbuf *clone = NULL;
@@ -542,7 +538,7 @@ test_attach_from_different_pool(void)
* test allocation and free of mbufs
*/
static int
-test_pktmbuf_pool(void)
+test_pktmbuf_pool(struct rte_mempool *pktmbuf_pool)
{
unsigned i;
struct rte_mbuf *m[NB_MBUF];
@@ -583,7 +579,7 @@ test_pktmbuf_pool(void)
* test that the pointer to the data on a packet mbuf is set properly
*/
static int
-test_pktmbuf_pool_ptr(void)
+test_pktmbuf_pool_ptr(struct rte_mempool *pktmbuf_pool)
{
unsigned i;
struct rte_mbuf *m[NB_MBUF];
@@ -636,7 +632,7 @@ test_pktmbuf_pool_ptr(void)
}
static int
-test_pktmbuf_free_segment(void)
+test_pktmbuf_free_segment(struct rte_mempool *pktmbuf_pool)
{
unsigned i;
struct rte_mbuf *m[NB_MBUF];
@@ -680,10 +676,11 @@ test_pktmbuf_free_segment(void)
#ifdef RTE_MBUF_REFCNT_ATOMIC
static int
-test_refcnt_slave(__attribute__((unused)) void *arg)
+test_refcnt_slave(void *arg)
{
unsigned lcore, free;
void *mp = 0;
+ struct rte_ring *refcnt_mbuf_ring = arg;
lcore = rte_lcore_id();
printf("%s started at lcore %u\n", __func__, lcore);
@@ -704,7 +701,9 @@ test_refcnt_slave(__attribute__((unused)) void *arg)
}
static void
-test_refcnt_iter(unsigned lcore, unsigned iter)
+test_refcnt_iter(unsigned int lcore, unsigned int iter,
+ struct rte_mempool *refcnt_pool,
+ struct rte_ring *refcnt_mbuf_ring)
{
uint16_t ref;
unsigned i, n, tref, wn;
@@ -760,7 +759,8 @@ test_refcnt_iter(unsigned lcore, unsigned iter)
}
static int
-test_refcnt_master(void)
+test_refcnt_master(struct rte_mempool *refcnt_pool,
+ struct rte_ring *refcnt_mbuf_ring)
{
unsigned i, lcore;
@@ -768,7 +768,7 @@ test_refcnt_master(void)
printf("%s started at lcore %u\n", __func__, lcore);
for (i = 0; i != REFCNT_MAX_ITER; i++)
- test_refcnt_iter(lcore, i);
+ test_refcnt_iter(lcore, i, refcnt_pool, refcnt_mbuf_ring);
refcnt_stop_slaves = 1;
rte_wmb();
@@ -783,9 +783,10 @@ static int
test_refcnt_mbuf(void)
{
#ifdef RTE_MBUF_REFCNT_ATOMIC
-
unsigned lnum, master, slave, tref;
-
+ int ret = -1;
+ struct rte_mempool *refcnt_pool = NULL;
+ struct rte_ring *refcnt_mbuf_ring = NULL;
if ((lnum = rte_lcore_count()) == 1) {
printf("skipping %s, number of lcores: %u is not enough\n",
@@ -797,31 +798,31 @@ test_refcnt_mbuf(void)
/* create refcnt pool & ring if they don't exist */
- if (refcnt_pool == NULL &&
- (refcnt_pool = rte_pktmbuf_pool_create(
- MAKE_STRING(refcnt_pool),
- REFCNT_MBUF_NUM, 0, 0, 0,
- SOCKET_ID_ANY)) == NULL) {
+ refcnt_pool = rte_pktmbuf_pool_create(MAKE_STRING(refcnt_pool),
+ REFCNT_MBUF_NUM, 0, 0, 0,
+ SOCKET_ID_ANY);
+ if (refcnt_pool == NULL) {
printf("%s: cannot allocate " MAKE_STRING(refcnt_pool) "\n",
__func__);
return -1;
}
- if (refcnt_mbuf_ring == NULL &&
- (refcnt_mbuf_ring = rte_ring_create("refcnt_mbuf_ring",
+ refcnt_mbuf_ring = rte_ring_create("refcnt_mbuf_ring",
rte_align32pow2(REFCNT_RING_SIZE), SOCKET_ID_ANY,
- RING_F_SP_ENQ)) == NULL) {
+ RING_F_SP_ENQ);
+ if (refcnt_mbuf_ring == NULL) {
printf("%s: cannot allocate " MAKE_STRING(refcnt_mbuf_ring)
"\n", __func__);
- return -1;
+ goto err;
}
refcnt_stop_slaves = 0;
memset(refcnt_lcore, 0, sizeof (refcnt_lcore));
- rte_eal_mp_remote_launch(test_refcnt_slave, NULL, SKIP_MASTER);
+ rte_eal_mp_remote_launch(test_refcnt_slave, refcnt_mbuf_ring,
+ SKIP_MASTER);
- test_refcnt_master();
+ test_refcnt_master(refcnt_pool, refcnt_mbuf_ring);
rte_eal_mp_wait_lcore();
@@ -839,8 +840,15 @@ test_refcnt_mbuf(void)
rte_mempool_dump(stdout, refcnt_pool);
rte_ring_dump(stdout, refcnt_mbuf_ring);
-#endif
+ ret = 0;
+
+err:
+ rte_mempool_free(refcnt_pool);
+ rte_ring_free(refcnt_mbuf_ring);
+ return ret;
+#else
return 0;
+#endif
}
#include <unistd.h>
@@ -870,7 +878,7 @@ verify_mbuf_check_panics(struct rte_mbuf *buf)
}
static int
-test_failing_mbuf_sanity_check(void)
+test_failing_mbuf_sanity_check(struct rte_mempool *pktmbuf_pool)
{
struct rte_mbuf *buf;
struct rte_mbuf badbuf;
@@ -931,7 +939,9 @@ test_failing_mbuf_sanity_check(void)
}
static int
-test_mbuf_linearize(int pkt_len, int nb_segs) {
+test_mbuf_linearize(struct rte_mempool *pktmbuf_pool, int pkt_len,
+ int nb_segs)
+{
struct rte_mbuf *m = NULL, *mbuf = NULL;
uint8_t *data;
@@ -1022,7 +1032,7 @@ test_mbuf_linearize(int pkt_len, int nb_segs) {
}
static int
-test_mbuf_linearize_check(void)
+test_mbuf_linearize_check(struct rte_mempool *pktmbuf_pool)
{
struct test_mbuf_array {
int size;
@@ -1039,7 +1049,7 @@ test_mbuf_linearize_check(void)
printf("Test mbuf linearize API\n");
for (i = 0; i < RTE_DIM(mbuf_array); i++)
- if (test_mbuf_linearize(mbuf_array[i].size,
+ if (test_mbuf_linearize(pktmbuf_pool, mbuf_array[i].size,
mbuf_array[i].nb_segs)) {
printf("Test failed for %d, %d\n", mbuf_array[i].size,
mbuf_array[i].nb_segs);
@@ -1052,53 +1062,54 @@ test_mbuf_linearize_check(void)
static int
test_mbuf(void)
{
+ int ret = -1;
+ struct rte_mempool *pktmbuf_pool = NULL;
+ struct rte_mempool *pktmbuf_pool2 = NULL;
+
+
RTE_BUILD_BUG_ON(sizeof(struct rte_mbuf) != RTE_CACHE_LINE_MIN_SIZE * 2);
/* create pktmbuf pool if it does not exist */
- if (pktmbuf_pool == NULL) {
- pktmbuf_pool = rte_pktmbuf_pool_create("test_pktmbuf_pool",
+ pktmbuf_pool = rte_pktmbuf_pool_create("test_pktmbuf_pool",
NB_MBUF, 32, 0, MBUF_DATA_SIZE, SOCKET_ID_ANY);
- }
if (pktmbuf_pool == NULL) {
printf("cannot allocate mbuf pool\n");
- return -1;
+ goto err;
}
/* create a specific pktmbuf pool with a priv_size != 0 and no data
* room size */
- if (pktmbuf_pool2 == NULL) {
- pktmbuf_pool2 = rte_pktmbuf_pool_create("test_pktmbuf_pool2",
+ pktmbuf_pool2 = rte_pktmbuf_pool_create("test_pktmbuf_pool2",
NB_MBUF, 32, MBUF2_PRIV_SIZE, 0, SOCKET_ID_ANY);
- }
if (pktmbuf_pool2 == NULL) {
printf("cannot allocate mbuf pool\n");
- return -1;
+ goto err;
}
/* test multiple mbuf alloc */
- if (test_pktmbuf_pool() < 0) {
+ if (test_pktmbuf_pool(pktmbuf_pool) < 0) {
printf("test_mbuf_pool() failed\n");
- return -1;
+ goto err;
}
/* do it another time to check that all mbufs were freed */
- if (test_pktmbuf_pool() < 0) {
+ if (test_pktmbuf_pool(pktmbuf_pool) < 0) {
printf("test_mbuf_pool() failed (2)\n");
- return -1;
+ goto err;
}
/* test that the pointer to the data on a packet mbuf is set properly */
- if (test_pktmbuf_pool_ptr() < 0) {
+ if (test_pktmbuf_pool_ptr(pktmbuf_pool) < 0) {
printf("test_pktmbuf_pool_ptr() failed\n");
- return -1;
+ goto err;
}
/* test data manipulation in mbuf */
- if (test_one_pktmbuf() < 0) {
+ if (test_one_pktmbuf(pktmbuf_pool) < 0) {
printf("test_one_mbuf() failed\n");
- return -1;
+ goto err;
}
@@ -1106,47 +1117,52 @@ test_mbuf(void)
* do it another time, to check that allocation reinitialize
* the mbuf correctly
*/
- if (test_one_pktmbuf() < 0) {
+ if (test_one_pktmbuf(pktmbuf_pool) < 0) {
printf("test_one_mbuf() failed (2)\n");
- return -1;
+ goto err;
}
- if (test_pktmbuf_with_non_ascii_data() < 0) {
+ if (test_pktmbuf_with_non_ascii_data(pktmbuf_pool) < 0) {
printf("test_pktmbuf_with_non_ascii_data() failed\n");
- return -1;
+ goto err;
}
/* test free pktmbuf segment one by one */
- if (test_pktmbuf_free_segment() < 0) {
+ if (test_pktmbuf_free_segment(pktmbuf_pool) < 0) {
printf("test_pktmbuf_free_segment() failed.\n");
- return -1;
+ goto err;
}
- if (testclone_testupdate_testdetach()<0){
+ if (testclone_testupdate_testdetach(pktmbuf_pool) < 0) {
printf("testclone_and_testupdate() failed \n");
- return -1;
+ goto err;
}
- if (test_attach_from_different_pool() < 0) {
+ if (test_attach_from_different_pool(pktmbuf_pool, pktmbuf_pool2) < 0) {
printf("test_attach_from_different_pool() failed\n");
- return -1;
+ goto err;
}
if (test_refcnt_mbuf()<0){
printf("test_refcnt_mbuf() failed \n");
- return -1;
+ goto err;
}
- if (test_failing_mbuf_sanity_check() < 0) {
+ if (test_failing_mbuf_sanity_check(pktmbuf_pool) < 0) {
printf("test_failing_mbuf_sanity_check() failed\n");
- return -1;
+ goto err;
}
- if (test_mbuf_linearize_check() < 0) {
+ if (test_mbuf_linearize_check(pktmbuf_pool) < 0) {
printf("test_mbuf_linearize_check() failed\n");
- return -1;
+ goto err;
}
- return 0;
+ ret = 0;
+
+err:
+ rte_mempool_free(pktmbuf_pool);
+ rte_mempool_free(pktmbuf_pool2);
+ return ret;
}
REGISTER_TEST_COMMAND(mbuf_autotest, test_mbuf);
--
2.11.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH v3] test/test_mbuf: remove mempool global var
2017-06-26 10:04 ` [dpdk-dev] [PATCH v3] " Santosh Shukla
@ 2017-06-27 17:09 ` Thomas Monjalon
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2017-06-27 17:09 UTC (permalink / raw)
To: Santosh Shukla; +Cc: dev, olivier.matz
26/06/2017 12:04, Santosh Shukla:
> Currently, pool resources are allocated statically
> and are not freed. Results of that test can not run more than once.
>
> Fix removes static dependency from test application and
> now allocating and freeing resources dynamically.
> Test runs for more than once.
>
> Fixes: af75078fece3 ("first public release")
>
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
Applied, thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-06-27 17:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-08 14:28 [dpdk-dev] [PATCH v2] test/test_mbuf: remove mempool global var Santosh Shukla
2017-06-16 14:35 ` Olivier Matz
2017-06-19 20:37 ` Thomas Monjalon
2017-06-20 4:14 ` santosh
2017-06-20 7:35 ` Thomas Monjalon
2017-06-20 8:22 ` Yuanhan Liu
2017-06-22 5:31 ` santosh
2017-06-22 5:29 ` santosh
2017-06-26 10:04 ` [dpdk-dev] [PATCH v3] " Santosh Shukla
2017-06-27 17:09 ` Thomas Monjalon
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).