[dpdk-dev] Issue observed with execution of Reorder test app

[dpdk-dev] Issue observed with execution of Reorder test app

[Mukesh Dua]

I see issue with reorder test app failing on x86 environment due to changes
made between release 2.0.0 and 2.1.0:

App reorder_test (app/test/test_reorder.c)
============
Function failing: test_reorder_insert

There had been some changes with respect to addition of parameter
is_initialized to the structure rte_reorder_buffer. In parallel the changes
were made to initialize some of the parameters in function
rte_reorder_insert

rte_reorder_insert(struct rte_reorder_buffer *b, struct rte_mbuf *mbuf)
{
    uint32_t offset, position;
    struct cir_buffer *order_buf = &b->order_buf;

*    if (!b->is_initialized) {*
*        b->min_seqn = mbuf->seqn;*
        *b->is_initialized = 1;*
*    }*

=> I don't see any reason to set b->min_seqn to mbuf->seqn and if that has
to be done, the conditional checks should have been modified in function
test_reorder_insert soon after a call to rte_reorder_insert. Additionally,
the next seqn number being populated should have been changed in function
test_reorder_insert:

    ret = rte_reorder_insert(b, bufs[0]);
*    if (!((ret == -1) && (rte_errno == ERANGE))) {*
*        printf("%s:%d: No error inserting late packet with seqn:"*
*                " 3 * size\n", __func__, __LINE__);*
*        ret = -1;*
*        goto exit;*
*    }*

    for (i = 0; i < num_bufs; i++)
        bufs[i]->seqn = i;

On the other hand, changing the code in function rte_reorder_insert:
rte_reorder_insert(struct rte_reorder_buffer *b, struct rte_mbuf *mbuf)
{
    uint32_t offset, position;
    struct cir_buffer *order_buf = &b->order_buf;

    if (!b->is_initialized) {
*        b->min_seqn = 0;  //Removed initialization from mbuf->seqn*
        b->is_initialized = 1;
    }
fixes the issues and the test case passes.

Regards,
Mukesh

Re: [dpdk-dev] Issue observed with execution of Reorder test app

[Gonzalez Monroy, Sergio]

On 20/08/2015 12:38, Mukesh Dua wrote:
> I see issue with reorder test app failing on x86 environment due to changes
> made between release 2.0.0 and 2.1.0:
>
> App reorder_test (app/test/test_reorder.c)
> ============
> Function failing: test_reorder_insert
>
> There had been some changes with respect to addition of parameter
> is_initialized to the structure rte_reorder_buffer. In parallel the changes
> were made to initialize some of the parameters in function
> rte_reorder_insert
>
> rte_reorder_insert(struct rte_reorder_buffer *b, struct rte_mbuf *mbuf)
> {
>      uint32_t offset, position;
>      struct cir_buffer *order_buf = &b->order_buf;
>
> *    if (!b->is_initialized) {*
> *        b->min_seqn = mbuf->seqn;*
>          *b->is_initialized = 1;*
> *    }*
>
> => I don't see any reason to set b->min_seqn to mbuf->seqn and if that has
> to be done, the conditional checks should have been modified in function
> test_reorder_insert soon after a call to rte_reorder_insert. Additionally,
> the next seqn number being populated should have been changed in function
> test_reorder_insert:
>
>      ret = rte_reorder_insert(b, bufs[0]);
> *    if (!((ret == -1) && (rte_errno == ERANGE))) {*
> *        printf("%s:%d: No error inserting late packet with seqn:"*
> *                " 3 * size\n", __func__, __LINE__);*
> *        ret = -1;*
> *        goto exit;*
> *    }*
>
>      for (i = 0; i < num_bufs; i++)
>          bufs[i]->seqn = i;
>
> On the other hand, changing the code in function rte_reorder_insert:
> rte_reorder_insert(struct rte_reorder_buffer *b, struct rte_mbuf *mbuf)
> {
>      uint32_t offset, position;
>      struct cir_buffer *order_buf = &b->order_buf;
>
>      if (!b->is_initialized) {
> *        b->min_seqn = 0;  //Removed initialization from mbuf->seqn*
>          b->is_initialized = 1;
>      }
> fixes the issues and the test case passes.
>
> Regards,
> Mukesh
Hi Mukesh,

The reason for that change is explained in its commit message and also 
in this thread:
http://dpdk.org/ml/archives/dev/2015-May/017930.html

Hope this info helps to clarify your concern.

Sergio

Re: [dpdk-dev] Issue observed with execution of Reorder test app

[Mukesh Dua]

Hi, Thanks for sharing the details.
On the basis of my understanding, have made the following changes. The test
is now passing with the changes.

diff -rupN a/app/test/test_reorder.c b/app/test/test_reorder.c
--- a/app/test/test_reorder.c   2015-08-20 13:59:55.000000000 -0400
+++ b/app/test/test_reorder.c   2015-08-21 00:19:10.305447948 -0400
@@ -181,10 +181,10 @@ test_reorder_insert(void)
    ret = rte_mempool_get_bulk(p, (void *)bufs, num_bufs);
    TEST_ASSERT_SUCCESS(ret, "Error getting mbuf from pool");

-   /* late packet */
+   /* late packet - registers the min_seqn */
    bufs[0]->seqn = 3 * size;
    ret = rte_reorder_insert(b, bufs[0]);
-   if (!((ret == -1) && (rte_errno == ERANGE))) {
+   if (ret != 0) {
        printf("%s:%d: No error inserting late packet with seqn:"
                " 3 * size\n", __func__, __LINE__);
        ret = -1;
@@ -192,7 +192,7 @@ test_reorder_insert(void)
    }

    for (i = 0; i < num_bufs; i++)
-       bufs[i]->seqn = i;
+       bufs[i]->seqn = bufs[0]->seqn + i;

    /* This should fill up order buffer:
     * reorder_seq = 0
@@ -223,7 +223,7 @@ test_reorder_insert(void)
    }

    /* early packet from current sequence window - full ready buffer */
-   bufs[5]->seqn = 2 * size;
+   bufs[5]->seqn = 5 * size;
    ret = rte_reorder_insert(b, bufs[5]);
    if (!((ret == -1) && (rte_errno == ENOSPC))) {
        printf("%s:%d: No error inserting early packet with full ready
buffer\n",
@@ -276,29 +276,30 @@ test_reorder_drain(void)
    /* Insert packet with seqn 1:
     * reorder_seq = 0
     * RB[] = {NULL, NULL, NULL, NULL}
-    * OB[] = {NULL, 1, NULL, NULL}
+    * OB[] = {1, NULL, NULL, NULL}


Regards,
Mukesh

On Thu, Aug 20, 2015 at 5:35 PM, Gonzalez Monroy, Sergio <
sergio.gonzalez.monroy@intel.com> wrote:

> On 20/08/2015 12:38, Mukesh Dua wrote:
>
>> I see issue with reorder test app failing on x86 environment due to
>> changes
>> made between release 2.0.0 and 2.1.0:
>>
>> App reorder_test (app/test/test_reorder.c)
>> ============
>> Function failing: test_reorder_insert
>>
>> There had been some changes with respect to addition of parameter
>> is_initialized to the structure rte_reorder_buffer. In parallel the
>> changes
>> were made to initialize some of the parameters in function
>> rte_reorder_insert
>>
>> rte_reorder_insert(struct rte_reorder_buffer *b, struct rte_mbuf *mbuf)
>> {
>>      uint32_t offset, position;
>>      struct cir_buffer *order_buf = &b->order_buf;
>>
>> *    if (!b->is_initialized) {*
>> *        b->min_seqn = mbuf->seqn;*
>>          *b->is_initialized = 1;*
>> *    }*
>>
>> => I don't see any reason to set b->min_seqn to mbuf->seqn and if that has
>> to be done, the conditional checks should have been modified in function
>> test_reorder_insert soon after a call to rte_reorder_insert. Additionally,
>> the next seqn number being populated should have been changed in function
>> test_reorder_insert:
>>
>>      ret = rte_reorder_insert(b, bufs[0]);
>> *    if (!((ret == -1) && (rte_errno == ERANGE))) {*
>> *        printf("%s:%d: No error inserting late packet with seqn:"*
>> *                " 3 * size\n", __func__, __LINE__);*
>> *        ret = -1;*
>> *        goto exit;*
>> *    }*
>>
>>      for (i = 0; i < num_bufs; i++)
>>          bufs[i]->seqn = i;
>>
>> On the other hand, changing the code in function rte_reorder_insert:
>> rte_reorder_insert(struct rte_reorder_buffer *b, struct rte_mbuf *mbuf)
>> {
>>      uint32_t offset, position;
>>      struct cir_buffer *order_buf = &b->order_buf;
>>
>>      if (!b->is_initialized) {
>> *        b->min_seqn = 0;  //Removed initialization from mbuf->seqn*
>>          b->is_initialized = 1;
>>      }
>> fixes the issues and the test case passes.
>>
>> Regards,
>> Mukesh
>>
> Hi Mukesh,
>
> The reason for that change is explained in its commit message and also in
> this thread:
> http://dpdk.org/ml/archives/dev/2015-May/017930.html
>
> Hope this info helps to clarify your concern.
>
> Sergio
>