[1/1,v1] examples: odp_ipsec: runtime select multiple vs single deq

From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>

Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
---
 example/ipsec/odp_ipsec.c        |    3 +++
 example/ipsec/odp_ipsec_stream.c |   27 +++++++++++----------------
 2 files changed, 14 insertions(+), 16 deletions(-)

On 9 April 2015 at 19:39, Robbie King (robking) <robking@cisco.com> wrote:

> One minor nit, see [rk] inline.  If tools won’t allow it, no problem.
>
> Reviewed-by: Robbie King <robking@cisco.com>
>
> -----Original Message-----
> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of
> alexandru.badicioiu@linaro.org
> Sent: Wednesday, April 08, 2015 2:28 AM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH 1/1 v1] examples: odp_ipsec: runtime select
> multiple vs single deq
>
> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>
> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> ---
>  example/ipsec/odp_ipsec.c        |    3 +++
>  example/ipsec/odp_ipsec_stream.c |   27 +++++++++++----------------
>  2 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
> index 9fb048a..cb8f535 100644
> --- a/example/ipsec/odp_ipsec.c
> +++ b/example/ipsec/odp_ipsec.c
> @@ -1498,6 +1498,9 @@ static void usage(char *progname)
>                " can be used to advanced pkt I/O selection for
> linux-generic\n"
>                "                        ODP_IPSEC_USE_POLL_QUEUES\n"
>                " to enable use of poll queues instead of scheduled
> (default)\n"
> +              "                        ODP_IPSEC_STREAM_VERIFY_MDEQ\n"
> +              " to enable use of multiple dequeue for queue draining
> during\n"
> +              " stream verification instead of single dequeue (default)\n"
>                "\n", NO_PATH(progname), NO_PATH(progname)
>                 );
>  }
> diff --git a/example/ipsec/odp_ipsec_stream.c
> b/example/ipsec/odp_ipsec_stream.c
> index ed07355..86cfb7e 100644
> --- a/example/ipsec/odp_ipsec_stream.c
> +++ b/example/ipsec/odp_ipsec_stream.c
> @@ -28,14 +28,6 @@
>
>  #define STREAM_MAGIC 0xBABE01234567CAFE
>
> -/**
> - * Control use of odp_queue_deq versus odp_queue_deq_multi
> - * when draining stream output queues
> - *
> - * @todo Make this command line driven versus compile time
> - *       (see https://bugs.linaro.org/show_bug.cgi?id=626)
> - */
> -#define LOOP_DEQ_MULTIPLE     0     /**< enable multi packet dequeue */
>  #define LOOP_DEQ_COUNT        32    /**< packets to dequeue at once */
>
>  /**
> @@ -517,7 +509,9 @@ odp_bool_t verify_stream_db_outputs(void)
>  {
>         odp_bool_t done = TRUE;
>         stream_db_entry_t *stream = NULL;
> +       const char *env;
>
> +       env = getenv("ODP_IPSEC_STREAM_VERIFY_MDEQ");
>         /* For each stream look for output packets */
>         for (stream = stream_db->list; NULL != stream; stream =
> stream->next) {
>                 int idx;
> @@ -531,14 +525,15 @@ odp_bool_t verify_stream_db_outputs(void)
>                         continue;
>
>                 for (;;) {
> -#if LOOP_DEQ_MULTIPLE
> -                       count = odp_queue_deq_multi(queue,
> -                                                   ev_tbl,
> -                                                   LOOP_DEQ_COUNT);
> -#else
> -                       ev_tbl[0] = odp_queue_deq(queue);
> -                       count = (ev_tbl[0] != ODP_EVENT_INVALID) ? 1 : 0;
> -#endif
> +                       if (env)
> +                               count = odp_queue_deq_multi(queue,
> +                                                           ev_tbl,
> +
>  LOOP_DEQ_COUNT);
>
> [rk] unbalanced "{}", much better in my opinion to have brackets on both
>      "if" and "else" leg, but understand if the check scripts don't let you
>
I agree with Robbie here. Not having the braces on the if-statement doesn't
even save any vertical screen estate. If the tools don't like this, we have
the wrong tools. But I do suspect having the same style on all legs *is*
the recommended style.


>
> +                       else {
> +                               ev_tbl[0] = odp_queue_deq(queue);
> +                               count = (ev_tbl[0] != ODP_EVENT_INVALID) ?
> +                                       1 : 0;
> +                       }
>                         if (!count)
>                                 break;
>                         for (idx = 0; idx < count; idx++) {
> --
> 1.7.3.4
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>

Actually, checkpatch requires balanced braces.  If one arm of an if/else
requires braces, then checkpatch says the other should have braces even if
it's a single statement.

On Mon, Apr 13, 2015 at 4:18 PM, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> On 9 April 2015 at 19:39, Robbie King (robking) <robking@cisco.com> wrote:
>
>> One minor nit, see [rk] inline.  If tools won’t allow it, no problem.
>>
>> Reviewed-by: Robbie King <robking@cisco.com>
>>
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of
>> alexandru.badicioiu@linaro.org
>> Sent: Wednesday, April 08, 2015 2:28 AM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [PATCH 1/1 v1] examples: odp_ipsec: runtime select
>> multiple vs single deq
>>
>> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>>
>> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>> ---
>>  example/ipsec/odp_ipsec.c        |    3 +++
>>  example/ipsec/odp_ipsec_stream.c |   27 +++++++++++----------------
>>  2 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
>> index 9fb048a..cb8f535 100644
>> --- a/example/ipsec/odp_ipsec.c
>> +++ b/example/ipsec/odp_ipsec.c
>> @@ -1498,6 +1498,9 @@ static void usage(char *progname)
>>                " can be used to advanced pkt I/O selection for
>> linux-generic\n"
>>                "                        ODP_IPSEC_USE_POLL_QUEUES\n"
>>                " to enable use of poll queues instead of scheduled
>> (default)\n"
>> +              "                        ODP_IPSEC_STREAM_VERIFY_MDEQ\n"
>> +              " to enable use of multiple dequeue for queue draining
>> during\n"
>> +              " stream verification instead of single dequeue
>> (default)\n"
>>                "\n", NO_PATH(progname), NO_PATH(progname)
>>                 );
>>  }
>> diff --git a/example/ipsec/odp_ipsec_stream.c
>> b/example/ipsec/odp_ipsec_stream.c
>> index ed07355..86cfb7e 100644
>> --- a/example/ipsec/odp_ipsec_stream.c
>> +++ b/example/ipsec/odp_ipsec_stream.c
>> @@ -28,14 +28,6 @@
>>
>>  #define STREAM_MAGIC 0xBABE01234567CAFE
>>
>> -/**
>> - * Control use of odp_queue_deq versus odp_queue_deq_multi
>> - * when draining stream output queues
>> - *
>> - * @todo Make this command line driven versus compile time
>> - *       (see https://bugs.linaro.org/show_bug.cgi?id=626)
>> - */
>> -#define LOOP_DEQ_MULTIPLE     0     /**< enable multi packet dequeue */
>>  #define LOOP_DEQ_COUNT        32    /**< packets to dequeue at once */
>>
>>  /**
>> @@ -517,7 +509,9 @@ odp_bool_t verify_stream_db_outputs(void)
>>  {
>>         odp_bool_t done = TRUE;
>>         stream_db_entry_t *stream = NULL;
>> +       const char *env;
>>
>> +       env = getenv("ODP_IPSEC_STREAM_VERIFY_MDEQ");
>>         /* For each stream look for output packets */
>>         for (stream = stream_db->list; NULL != stream; stream =
>> stream->next) {
>>                 int idx;
>> @@ -531,14 +525,15 @@ odp_bool_t verify_stream_db_outputs(void)
>>                         continue;
>>
>>                 for (;;) {
>> -#if LOOP_DEQ_MULTIPLE
>> -                       count = odp_queue_deq_multi(queue,
>> -                                                   ev_tbl,
>> -                                                   LOOP_DEQ_COUNT);
>> -#else
>> -                       ev_tbl[0] = odp_queue_deq(queue);
>> -                       count = (ev_tbl[0] != ODP_EVENT_INVALID) ? 1 : 0;
>> -#endif
>> +                       if (env)
>> +                               count = odp_queue_deq_multi(queue,
>> +                                                           ev_tbl,
>> +
>>  LOOP_DEQ_COUNT);
>>
>> [rk] unbalanced "{}", much better in my opinion to have brackets on both
>>      "if" and "else" leg, but understand if the check scripts don't let
>> you
>>
> I agree with Robbie here. Not having the braces on the if-statement
> doesn't even save any vertical screen estate. If the tools don't like this,
> we have the wrong tools. But I do suspect having the same style on all legs
> *is* the recommended style.
>
>
>>
>> +                       else {
>> +                               ev_tbl[0] = odp_queue_deq(queue);
>> +                               count = (ev_tbl[0] != ODP_EVENT_INVALID) ?
>> +                                       1 : 0;
>> +                       }
>>                         if (!count)
>>                                 break;
>>                         for (idx = 0; idx < count; idx++) {
>> --
>> 1.7.3.4
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>