On 9 April 2015 at 19:39, Robbie King (robking) <[email protected]> wrote:

> One minor nit, see [rk] inline.  If tools won’t allow it, no problem.
>
> Reviewed-by: Robbie King <[email protected]>
>
> -----Original Message-----
> From: lng-odp [mailto:[email protected]] On Behalf Of
> [email protected]
> Sent: Wednesday, April 08, 2015 2:28 AM
> To: [email protected]
> Subject: [lng-odp] [PATCH 1/1 v1] examples: odp_ipsec: runtime select
> multiple vs single deq
>
> From: Alexandru Badicioiu <[email protected]>
>
> Signed-off-by: Alexandru Badicioiu <[email protected]>
> ---
>  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
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to