On Mon, Jun 5, 2023 at 4:56 PM Zhirun Yan <zhirun....@intel.com> wrote:
>
> 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.w...@intel.com>
> Signed-off-by: Cunming Liang <cunming.li...@intel.com>
> Signed-off-by: Zhirun Yan <zhirun....@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 = files(
> 'graph_stats.c',
> 'graph_populate.c',
> 'graph_pcap.c',
> + 'rte_graph_worker.c',
> )
> headers = 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 notice.
> + *
> + * 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 = graph_list_head_get();
> + struct graph *graph;
> + int ret = 0;
> +
> + if (model == RTE_GRAPH_MODEL_DEFAULT || model == RTE_GRAPH_MODEL_RTC
> ||
> + model == RTE_GRAPH_MODEL_MCORE_DISPATCH)
> + STAILQ_FOREACH(graph, graph_head, next)
> + graph->graph->model = model;
> + else {
> + STAILQ_FOREACH(graph, graph_head, next)
> + graph->graph->model = RTE_GRAPH_MODEL_DEFAULT;
> + ret = -1;
Why returning -1 here?
Also, why "else" case needed as RTE_GRAPH_MODEL_RTC == RTE_GRAPH_MODEL_DEFAULT
Why not
struct graph_head *graph_head = 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 = 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 model
> 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 = graph_list_head_get();
> + struct graph *graph;
> +
> + graph = 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_worker_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 = 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 starts. */
+ 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,
> struct 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
