Re: [dpdk-dev] [PATCH v3 12/15] app/eventdev: switch flow ID to dynamic mbuf field

[Jerin Jacob <jerinjacobk@gmail.com>]

On Wed, Oct 28, 2020 at 2:35 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> The order test stored the flow ID in the deprecated mbuf field udata64.
> It is moved to a dynamic field in order to allow removal of udata64.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  app/test-eventdev/test_order_atq.c    |  4 ++--
>  app/test-eventdev/test_order_common.c | 21 ++++++++++++++++-----
>  app/test-eventdev/test_order_common.h | 19 +++++++++++++++++++
>  app/test-eventdev/test_order_queue.c  |  4 ++--
>  4 files changed, 39 insertions(+), 9 deletions(-)

+ timothy.mcdaniel@intel.com

>
> diff --git a/app/test-eventdev/test_order_atq.c b/app/test-eventdev/test_order_atq.c
> index cfcb1dc4e9..b135ac4e55 100644
> --- a/app/test-eventdev/test_order_atq.c
> +++ b/app/test-eventdev/test_order_atq.c
> @@ -35,7 +35,7 @@ order_atq_worker(void *arg, const bool flow_id_cap)
>                 }
>
>                 if (!flow_id_cap)
> -                       ev.flow_id = ev.mbuf->udata64;
> +                       flow_id_copy_from_mbuf(&ev);
>
>                 if (ev.sub_event_type == 0) { /* stage 0 from producer */
>                         order_atq_process_stage_0(&ev);
> @@ -72,7 +72,7 @@ order_atq_worker_burst(void *arg, const bool flow_id_cap)
>
>                 for (i = 0; i < nb_rx; i++) {
>                         if (!flow_id_cap)
> -                               ev[i].flow_id = ev[i].mbuf->udata64;
> +                               flow_id_copy_from_mbuf(&ev[i]);
>
>                         if (ev[i].sub_event_type == 0) { /*stage 0 */
>                                 order_atq_process_stage_0(&ev[i]);
> diff --git a/app/test-eventdev/test_order_common.c b/app/test-eventdev/test_order_common.c
> index dc55d93921..c5f7317440 100644
> --- a/app/test-eventdev/test_order_common.c
> +++ b/app/test-eventdev/test_order_common.c
> @@ -4,6 +4,8 @@
>
>  #include "test_order_common.h"
>
> +int flow_id_dynfield_offset = -1;


See below,


> +
>  int
>  order_test_result(struct evt_test *test, struct evt_options *opt)
>  {
> @@ -46,13 +48,10 @@ order_producer(void *arg)
>                 if (m == NULL)
>                         continue;
>
> -               const uint32_t flow = (uintptr_t)m % nb_flows;
> +               const flow_id_t flow = (uintptr_t)m % nb_flows;
>                 /* Maintain seq number per flow */
>                 m->seqn = producer_flow_seq[flow]++;
> -               m->udata64 = flow;
> -
> -               ev.flow_id = flow;
> -               ev.mbuf = m;
> +               flow_id_save(flow, m, &ev);
>
>                 while (rte_event_enqueue_burst(dev_id, port, &ev, 1) != 1) {
>                         if (t->err)
> @@ -139,6 +138,18 @@ order_test_setup(struct evt_test *test, struct evt_options *opt)
>  {
>         void *test_order;
>
> +       static const struct rte_mbuf_dynfield flow_id_dynfield_desc = {
> +               .name = "test_event_dynfield_flow_id",
> +               .size = sizeof(flow_id_t),
> +               .align = __alignof__(flow_id_t),
> +       };
> +       flow_id_dynfield_offset =
> +               rte_mbuf_dynfield_register(&flow_id_dynfield_desc);


Since this path used in fastpath, could you move flow_id_dynfield_offset to
test_order's initial entry as that cache will be warm always.

> +       if (flow_id_dynfield_offset < 0) {
> +               evt_err("failed to register mbuf field");
> +               return -rte_errno;
> +       }
> +
>         test_order = rte_zmalloc_socket(test->name, sizeof(struct test_order),
>                                 RTE_CACHE_LINE_SIZE, opt->socket_id);

See above


The rest of the eventdev and Graph changes are fine.