From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 4DBB142BF1;
	Mon,  5 Jun 2023 14:38:51 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id D890140A87;
	Mon,  5 Jun 2023 14:38:50 +0200 (CEST)
Received: from mail-ua1-f45.google.com (mail-ua1-f45.google.com
 [209.85.222.45]) by mails.dpdk.org (Postfix) with ESMTP id 58D4B4021F
 for <dev@dpdk.org>; Mon,  5 Jun 2023 14:38:49 +0200 (CEST)
Received: by mail-ua1-f45.google.com with SMTP id
 a1e0cc1a2514c-7841f18f9f7so1216593241.0
 for <dev@dpdk.org>; Mon, 05 Jun 2023 05:38:49 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=gmail.com; s=20221208; t=1685968728; x=1688560728;
 h=content-transfer-encoding:cc:to:subject:message-id:date:from
 :in-reply-to:references:mime-version:from:to:cc:subject:date
 :message-id:reply-to;
 bh=I6ZA9KTpOjPn5RL5OiaaFDsGjBIvDGLOFCqqG+XiUwc=;
 b=hI8NqjfGr+Heizk/7MGATbtS/u7z5GXwfPTpjZ25+tUQKHhhzQvEy+90rv9K/GlA0C
 ArRrEFwu/WxhFkQiKCh5uhccyYsLOJ5ZJNATjXa4pkkTCRJERrkvvyrb4o92cmPhWWfh
 N7Nc91Ez8Ypz6DwfyghICSE1PIYk5lFqzqswCYlE/Wb63s3zDTZBRuyFTMQDTYwzjAm8
 PP9LvrvThErKyKO6MJT/qeX4uVtCXAMqCH54gnsOEYDwRlBnAFAiRLm3mKk+U9OFhsLB
 Ev0tDO8PZBxNRLuiFHQxOKYUpvime+pPK+GUitdN5tHVr526cdTNoo8Q45LhUB3Zupge
 1SUg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20221208; t=1685968728; x=1688560728;
 h=content-transfer-encoding:cc:to:subject:message-id:date:from
 :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc
 :subject:date:message-id:reply-to;
 bh=I6ZA9KTpOjPn5RL5OiaaFDsGjBIvDGLOFCqqG+XiUwc=;
 b=QbVZQyHGxp/gZ9x1rtXKYUrYYJxkcUug/0oKAkVnUgOOnOsrgLVq0j01Mo7fPFqS0/
 w9IfZ+c16vKlWtQLp46cDip0mi+DrByPEsCSqGRDBAetAigFrhq/ghRV9QLXv9k4n/yX
 mX8Msu8MhQM1ioY8oAJHr6e0fZh5EEJDQsMjlZG6q9Xl+HQhVGygdGDt+LYYLM6/1oZd
 cST3rfx+FUFqPKR9KRWPoCz2QO+fB40chx7I1W5zZ0Lducd6CidqL/q2kPoMmrljr+ZG
 XORutHMk2umq4L1sM7Z2E8qDfS3PkFlur65MWckJ+5dcdfUFwu/AtF2LY+1+RcbFtcte
 0B2Q==
X-Gm-Message-State: AC+VfDyyQhalb/UXJ92ieDN9S39f/WkXJFIOaHlXxPHBrWolxrDazMHY
 FTNgmLyvIAxUGMJmdewIu5l4VOUFa5k7mFC+630=
X-Google-Smtp-Source: ACHHUZ4ZJLrsAgLEDXDJ+r3WDhX9rh3W/pIQOTD0llJAXXF/dKMbeo2m090YAHeGP8rVWw0fqpB6xcnDMxNBlgCXnx0=
X-Received: by 2002:a67:ec0b:0:b0:43b:1ae4:71a5 with SMTP id
 d11-20020a67ec0b000000b0043b1ae471a5mr2963438vso.34.1685968728518; Mon, 05
 Jun 2023 05:38:48 -0700 (PDT)
MIME-Version: 1.0
References: <20230509060347.1237884-1-zhirun.yan@intel.com>
 <20230605111923.3772260-1-zhirun.yan@intel.com>
 <20230605111923.3772260-5-zhirun.yan@intel.com>
In-Reply-To: <20230605111923.3772260-5-zhirun.yan@intel.com>
From: Jerin Jacob <jerinjacobk@gmail.com>
Date: Mon, 5 Jun 2023 18:08:22 +0530
Message-ID: <CALBAE1NOPerakzO68PJpeqxAqYteSr-aKer+QJfrntbYFH0LAg@mail.gmail.com>
Subject: Re: [PATCH v7 04/17] graph: add get/set graph worker model APIs
To: Zhirun Yan <zhirun.yan@intel.com>
Cc: dev@dpdk.org, jerinj@marvell.com, kirankumark@marvell.com, 
 ndabilpuram@marvell.com, stephen@networkplumber.org, pbhagavatula@marvell.com, 
 cunming.liang@intel.com, haiyue.wang@intel.com, mattias.ronnblom@ericsson.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Mon, Jun 5, 2023 at 4:56=E2=80=AFPM Zhirun Yan <zhirun.yan@intel.com> wr=
ote:
>
> Add new get/set APIs to configure graph worker model which is used to
> determine which model will be chosen.
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> ---
>  lib/graph/meson.build               |  1 +
>  lib/graph/rte_graph_worker.c        | 70 +++++++++++++++++++++++++++++
>  lib/graph/rte_graph_worker_common.h | 19 ++++++++
>  lib/graph/version.map               |  3 ++
>  4 files changed, 93 insertions(+)
>  create mode 100644 lib/graph/rte_graph_worker.c
>
> diff --git a/lib/graph/meson.build b/lib/graph/meson.build
> index 3526d1b5d4..9fab8243da 100644
> --- a/lib/graph/meson.build
> +++ b/lib/graph/meson.build
> @@ -15,6 +15,7 @@ sources =3D files(
>          'graph_stats.c',
>          'graph_populate.c',
>          'graph_pcap.c',
> +        'rte_graph_worker.c',
>  )
>  headers =3D files('rte_graph.h', 'rte_graph_worker.h')
>
> diff --git a/lib/graph/rte_graph_worker.c b/lib/graph/rte_graph_worker.c
> new file mode 100644
> index 0000000000..fc188e7cfa
> --- /dev/null
> +++ b/lib/graph/rte_graph_worker.c
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2023 Intel Corporation
> + */
> +
> +/**
> + * @file graph_worker.c
> + *
> + * @warning
> + * @b EXPERIMENTAL:
> + * All functions in this file may be changed or removed without prior no=
tice.
> + *
> + * These API enable to set/get graph walking model.
> + *
> + */
> +
> +#include "rte_graph_worker_common.h"
> +#include "graph_private.h"
> +
> +/**
> + * @note This function does not perform any locking, and is only safe to=
 call
> + *    before graph running. It will set all graphs the same model.
> + *
> + * @param name
> + *   Name of the graph worker model.
> + *
> + * @return
> + *   0 on success, -1 otherwise.
< 0 otherwise

Doxygen comment is not required .c file.


> + */
> +int
> +rte_graph_worker_model_set(enum rte_graph_worker_model model)
> +{
> +       struct graph_head *graph_head =3D graph_list_head_get();
> +       struct graph *graph;
> +       int ret =3D 0;
> +
> +       if (model =3D=3D RTE_GRAPH_MODEL_DEFAULT || model =3D=3D RTE_GRAP=
H_MODEL_RTC ||
> +           model =3D=3D RTE_GRAPH_MODEL_MCORE_DISPATCH)
> +               STAILQ_FOREACH(graph, graph_head, next)
> +                       graph->graph->model =3D model;
> +       else {
> +               STAILQ_FOREACH(graph, graph_head, next)
> +                       graph->graph->model =3D RTE_GRAPH_MODEL_DEFAULT;
> +               ret =3D -1;

Why returning -1 here?
Also, why "else" case needed as RTE_GRAPH_MODEL_RTC =3D=3D RTE_GRAPH_MODEL_=
DEFAULT

Why not

struct graph_head *graph_head =3D graph_list_head_get();
struct graph *graph;

[1]
if (model > RTE_GRAPH_MODEL_MCORE_DISPATCH)
       return -EINVAL;

STAILQ_FOREACH(graph, graph_head, next)
              graph->graph->model =3D model;


For [1], Please add internal helper function graph_model_is_valid() to
use everywhere as needed.

> +               }
> +
> +       return ret;
> +}
> +
> +/**
> + * Get the graph worker model
> + *
> + * @note All graph will use the same model and this function will get mo=
del from the first one
> + *
> + * @param name
> + *   Name of the graph worker model.
> + *
> + * @return
> + *   Graph worker model on success.
> + */
> +inline
> +enum rte_graph_worker_model
> +rte_graph_worker_model_get(void)
> +{
> +       struct graph_head *graph_head =3D graph_list_head_get();
> +       struct graph *graph;
> +
> +       graph =3D STAILQ_FIRST(graph_head);

This can be used in fastpath, So lets pass graph object and make
inline function to return graph->model

> +
> +       return graph->graph->model;

> +}
> diff --git a/lib/graph/rte_graph_worker_common.h b/lib/graph/rte_graph_wo=
rker_common.h
> index 41428974db..72d132bae4 100644
> --- a/lib/graph/rte_graph_worker_common.h
> +++ b/lib/graph/rte_graph_worker_common.h
> @@ -29,6 +29,16 @@
>  extern "C" {
>  #endif
>
> +/** Graph worker models */
> +enum rte_graph_worker_model {
> +       RTE_GRAPH_MODEL_DEFAULT,
> +       /**< Default graph model*/
> +       RTE_GRAPH_MODEL_RTC =3D RTE_GRAPH_MODEL_DEFAULT,
> +       /**< Run-To-Completion model. It is the default model. */
> +       RTE_GRAPH_MODEL_MCORE_DISPATCH
> +       /**< Dispatch model to support cross-core dispatching within core=
 affinity. */
> +};
> +
>  /**
>   * @internal
>   *
> @@ -50,6 +60,7 @@ struct rte_graph {
>         /** Number of packets to capture per core. */
>         uint64_t nb_pkt_to_capture;
>         char pcap_filename[RTE_GRAPH_PCAP_FILE_SZ];  /**< Pcap filename. =
*/
> +       enum rte_graph_worker_model model; /**< graph model */

Used in fastpath, use in fastpath area
[main]dell[dpdk.org] $ git diff
diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
index 438595b15c..462bbfa705 100644
--- a/lib/graph/rte_graph_worker.h
+++ b/lib/graph/rte_graph_worker.h
@@ -41,6 +41,7 @@ struct rte_graph {
        rte_node_t nb_nodes;         /**< Number of nodes in the graph. */
        rte_graph_off_t *cir_start;  /**< Pointer to circular buffer. */
        rte_graph_off_t nodes_start; /**< Offset at which node memory start=
s. */
+       enum rte_graph_worker_model  model;
        rte_graph_t id; /**< Graph identifier. */
        int socket;     /**< Socket ID where memory is allocated. */
        char name[RTE_GRAPH_NAMESIZE];  /**< Name of the graph. */

>         uint64_t fence;                 /**< Fence. */
>  } __rte_cache_aligned;
>
> @@ -490,6 +501,14 @@ rte_node_next_stream_move(struct rte_graph *graph, s=
truct rte_node *src,
>         }
>  }
>
> +__rte_experimental
> +enum rte_graph_worker_model
> +rte_graph_worker_model_get(void);
> +
> +__rte_experimental
> +int
> +rte_graph_worker_model_set(enum rte_graph_worker_model model);


Add proper Doxygen comment for both.

Also check the build issue at
http://mails.dpdk.org/archives/test-report/2023-June/404496.html