Hi Kiran,
On Wed, Jun 4, 2025 at 5:06 PM Kiran Kumar Kokkilagadda
<kirankum...@marvell.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Nitin Saxena <nsax...@marvell.com>
> > Sent: Wednesday, June 4, 2025 3:43 PM
> > To: Jerin Jacob <jer...@marvell.com>; Kiran Kumar Kokkilagadda
> > <kirankum...@marvell.com>; Nithin Kumar Dabilpuram
> > <ndabilpu...@marvell.com>; Zhirun Yan <yanzhirun_...@163.com>; Robin
> > Jarry <rja...@redhat.com>; Christophe Fontaine <cfont...@redhat.com>
> > Cc: dev@dpdk.org; Nitin Saxena <nsaxen...@gmail.com>
> > Subject: [PATCH v10 4/7] graph: add feature enable/disable APIs
> >
> > This patch also adds feature arc fast path APIs as well along with
> > documentation
> >
> > Signed-off-by: Nitin Saxena <nsax...@marvell.com>
> > ---
> > doc/guides/prog_guide/graph_lib.rst | 180 ++++++
> > lib/graph/graph_feature_arc.c | 701 ++++++++++++++++++++++-
> > lib/graph/meson.build | 2 +-
> > lib/graph/rte_graph_feature_arc.h | 134 ++++-
> > lib/graph/rte_graph_feature_arc_worker.h | 305 +++++++++-
> > 5 files changed, 1314 insertions(+), 8 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/graph_lib.rst
> > b/doc/guides/prog_guide/graph_lib.rst
> > index c9ac9e7ae0..fef384d836 100644
> > --- a/doc/guides/prog_guide/graph_lib.rst
> > +++ b/doc/guides/prog_guide/graph_lib.rst
> > @@ -609,6 +609,8 @@ provides application to overload default node path by
> > providing hook
> > points(like netfilter) to insert out-of-tree or another protocol nodes in
> > packet path.
> >
> > +.. _Control_Data_Plane_Synchronization:
> > +
> > Control/Data plane synchronization
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > Feature arc does not stop worker cores for any runtime control plane
> > updates.
> > @@ -839,6 +841,11 @@ which might have allocated during feature enable.
> > notifier_cb() is called, at runtime, for every enable/disable of
> > ``[feature,
> > index]`` from control thread.
> >
> > +If RCU is provided to enable/disable APIs, notifier_cb() is called after
> > +``rte_rcu_qsbr_synchronize()``. Application also needs to call
> > +``rte_rcu_qsbr_quiescent()`` in worker thread (preferably after every
> > +``rte_graph_walk()`` iteration)
> > +
> > override_index_cb()
> > ....................
> > A feature arc is :ref:`registered<Feature_Arc_Registration>` to operate on
> > @@ -869,3 +876,176 @@ sub-system. If not called, feature arc has no impact
> > on application.
> > ``rte_graph_feature_arc_init()`` API should be called before
> > ``rte_graph_create()``. If not called, feature arc is a ``NOP`` to
> > application.
> > +
> > +Runtime feature enable/disable
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +A feature can be enabled or disabled at runtime from control thread using
> > +``rte_graph_feature_enable()`` and ``rte_graph_feature_disable()`` APIs
> > +respectively.
> > +
> > +.. code-block:: c
> > +
> > + struct rte_rcu_qsbr *rcu_qsbr = app_get_rcu_qsbr();
> > + rte_graph_feature_arc_t _arc;
> > + uint16_t app_cookie;
> > +
> > + if (rte_graph_feature_arc_lookup_by_name("Arc1", &_arc) < 0) {
> > + RTE_LOG(ERR, GRAPH, "Arc1 not found\n");
> > + return -ENOENT;
> > + }
> > + app_cookie = 100; /* Specific to ['Feature-1`, `port-0`]*/
> > +
> > + /* Enable feature */
> > + rte_graph_feature_enable(_arc, 0 /* port-0 */,
> > + "Feature-1" /* Name of the node feature */,
> > + app_cookie, rcu_qsbr);
> > +
> > + /* Disable feature */
> > + rte_graph_feature_disable(_arc, 0 /* port-0 */,
> > + "Feature-1" /* Name of the node feature*/,
> > + rcu_qsbr);
> > +
> > +.. note::
> > +
> > + RCU argument is optional argument to enable/disable APIs. See
> > + :ref:`control/data plane
> > + synchronization<Control_Data_Plane_Synchronization>` and
> > + :ref:`notifier_cb<Feature_Notifier_Cb>` for more details on when RCU is
> > + needed.
> > +
> > +Fast path traversal rules
> > +^^^^^^^^^^^^^^^^^^^^^^^^^
> > +``Start node``
> > +**************
> > +If feature arc is :ref:`initialized<Feature_Arc_Initialization>`,
> > +``start_node_feature_process_fn()`` will be called by ``rte_graph_walk()``
> > +instead of node's original ``process()``. This function should allow
> > packets to
> > +enter arc path whenever any feature is enabled at runtime
> > +
> > +.. code-block:: c
> > +
> > + static int nodeA_init(const struct rte_graph *graph, struct rte_node
> > *node)
> > + {
> > + rte_graph_feature_arc_t _arc;
> > +
> > + if (rte_graph_feature_arc_lookup_by_name("Arc1", &_arc) < 0) {
> > + RTE_LOG(ERR, GRAPH, "Arc1 not found\n");
> > + return -ENOENT;
> > + }
> > +
> > + /* Save arc in node context */
> > + node->ctx = _arc;
> > + return 0;
> > + }
> > +
> > + int nodeA_process_inline(struct rte_graph *graph, struct rte_node
> > *node,
> > + void **objs, uint16_t nb_objs,
> > + struct rte_graph_feature_arc *arc,
> > + const int do_arc_processing)
> > + {
> > + for(uint16_t i = 0; i < nb_objs; i++) {
> > + struct rte_mbuf *mbuf = objs[i];
> > + rte_edge_t edge_to_child = 0; /* By default to Node-B */
> > +
> > + if (do_arc_processing) {
> > + struct rte_graph_feature_arc_mbuf_dynfields *dyn =
> > + rte_graph_feature_arc_mbuf_dynfields_get(mbuf, arc-
> > >mbuf_dyn_offset);
> > +
> > + if (rte_graph_feature_data_first_feature_get(mbuf,
> > mbuf->port,
> > +
> > &dyn->feature_data,
> > +
> > &edge_to_child) < 0) {
> > +
> > + /* Some feature is enabled, edge_to_child is
> > overloaded*/
> > + }
> > + }
> > + /* enqueue as usual */
> > + rte_node_enqueue_x1(graph, node, mbuf, edge_to_child);
> > + }
> > + }
> > +
> > + int nodeA_feature_process_fn(struct rte_graph *graph, struct rte_node
> > *node,
> > + void **objs, uint16_t nb_objs)
> > + {
> > + struct rte_graph_feature_arc *arc = rte_graph_feature_arc_get(node-
> > >ctx);
> > +
> > + if (unlikely(rte_graph_feature_arc_has_any_feature(arc)))
> > + return nodeA_process_inline(graph, node, objs, nb_objs, arc, 1
> > /* do
> > arc processing */);
> > + else
> > + return nodeA_process_inline(graph, node, objs, nb_objs, NULL,
> > 0 /*
> > skip arc processing */);
> > + }
> > +
> > +``Feature nodes``
> > +*****************
> > +Following code-snippet explains fast path traversal rule for ``Feature-1``
> > +:ref:`feature node<Feature_Nodes>` shown in :ref:`figure<Figure_Arc_2>`.
> > +
> > +.. code-block:: c
> > +
> > + static int Feature1_node_init(const struct rte_graph *graph, struct
> > rte_node *node)
> > + {
> > + rte_graph_feature_arc_t _arc;
> > +
> > + if (rte_graph_feature_arc_lookup_by_name("Arc1", &_arc) < 0) {
> > + RTE_LOG(ERR, GRAPH, "Arc1 not found\n");
> > + return -ENOENT;
> > + }
> > +
> > + /* Save arc in node context */
> > + node->ctx = _arc;
> > + return 0;
> > + }
> > +
> > + int feature1_process_inline(struct rte_graph *graph, struct rte_node
> > *node,
> > + void **objs, uint16_t nb_objs,
> > + struct rte_graph_feature_arc *arc)
> > + {
> > + for(uint16_t i = 0; i < nb_objs; i++) {
> > + struct rte_mbuf *mbuf = objs[i];
> > + rte_edge_t edge_to_child = 0; /* By default to Node-B */
> > +
> > + struct rte_graph_feature_arc_mbuf_dynfields *dyn =
> > + rte_graph_feature_arc_mbuf_dynfields_get(mbuf, arc-
> > >mbuf_dyn_offset);
> > +
> > + /* Get feature app cookie for mbuf */
> > + uint16_t app_cookie =
> > rte_graph_feature_data_app_cookie_get(mbuf,
> > &dyn->feature_data);
> > +
> > + if (feature_local_lookup(app_cookie) {
> > +
> > + /* Packets is relevant to this feature. Move packet from
> > arc path */
> > + edge_to_child = X;
> > +
> > + } else {
> > +
> > + /* Packet not relevant to this feature. Send this packet to
> > + * next enabled feature
> > + */
> > + rte_graph_feature_data_next_feature_get(mbuf, &dyn-
> > >feature_data,
> > + &edge_to_child);
> > + }
> > +
> > + /* enqueue as usual */
> > + rte_node_enqueue_x1(graph, node, mbuf, edge_to_child);
> > + }
> > + }
> > +
> > + int feature1_process_fn(struct rte_graph *graph, struct rte_node *node,
> > + void **objs, uint16_t nb_objs)
> > + {
> > + struct rte_graph_feature_arc *arc = rte_graph_feature_arc_get(node-
> > >ctx);
> > +
> > + return feature1_process_inline(graph, node, objs, nb_objs, arc);
> > + }
> > +
> > +``End feature node``
> > +********************
> > +An end feature node is a feature node through which packets exits feature
> > arc
> > +path. It should not use any feature arc fast path APIs.
> > +
> > +Feature arc destroy
> > +^^^^^^^^^^^^^^^^^^^
> > +``rte_graph_feature_arc_destroy()`` can be used to free a arc object.
> > +
> > +Feature arc cleanup
> > +^^^^^^^^^^^^^^^^^^^
> > +``rte_graph_feature_arc_cleanup()`` can be used to free all resources
> > +associated with feature arc module.
> > diff --git a/lib/graph/graph_feature_arc.c b/lib/graph/graph_feature_arc.c
> > index b28f0ec321..9cad82947a 100644
> > --- a/lib/graph/graph_feature_arc.c
> > +++ b/lib/graph/graph_feature_arc.c
> > @@ -19,6 +19,9 @@
> >
> > #define graph_uint_cast(f) ((unsigned int)f)
> >
> > +#define fdata_fix_get(arc, feat, index) \
> > + RTE_GRAPH_FEATURE_TO_FEATURE_DATA(arc, feat,
> > index)
> > +
> > #define feat_dbg graph_dbg
> >
> > #define FEAT_COND_ERR(cond, ...)
> > \
> > @@ -61,6 +64,139 @@ static STAILQ_HEAD(, rte_graph_feature_arc_register)
> > feature_arc_list =
> > static STAILQ_HEAD(, rte_graph_feature_register) feature_list =
> >
> > STAILQ_HEAD_INITIALIZER(feature_list);
> >
> > + /*
> > + * feature data index is not fixed for given [feature, index], although
> > it can
> > + * be, which is calculated as follows (fdata_fix_get())
> > + *
> > + * fdata = (arc->max_features * feature ) + index;
> > + *
> > + * But feature data index should not be fixed for any index. i.e
> > + * on any index, feature data can be placed. A slow path array is
> > + * maintained and within a feature range [start, end] it is checked where
> > + * feature_data_index is already placed.
> > + *
> > + * If is_release == false. feature_data_index is searched in a feature
> > range.
> > + * If found, index is returned. If not found, then reserve and return.
> > + *
> > + * If is_release == true, then feature_data_index is released for further
> > + * usage
> > + */
> > +static rte_graph_feature_data_t
> > +fdata_dyn_reserve_or_rel(struct rte_graph_feature_arc *arc,
> > rte_graph_feature_t f,
> > + uint32_t index, bool is_release,
> > + bool fdata_provided, rte_graph_feature_data_t fd)
> > +{
> > + rte_graph_feature_data_t start, end, fdata;
> > + rte_graph_feature_t next_feat;
> > +
> > + if (fdata_provided)
> > + fdata = fd;
> > + else
> > + fdata = fdata_fix_get(arc, f, index);
> > +
> > + next_feat = f + 1;
> > + /* Find in a given feature range, feature data is stored or not */
> > + for (start = fdata_fix_get(arc, f, 0),
> > + end = fdata_fix_get(arc, next_feat, 0);
> > + start < end;
> > + start++) {
> > + if (arc->feature_data_by_index[start] == fdata) {
> > + if (is_release)
> > + arc->feature_data_by_index[start] =
> > RTE_GRAPH_FEATURE_DATA_INVALID;
> > +
> > + return start;
> > + }
> > + }
> > +
> > + if (is_release)
> > + return RTE_GRAPH_FEATURE_DATA_INVALID;
> > +
> > + /* If not found, then reserve valid one */
> > + for (start = fdata_fix_get(arc, f, 0),
> > + end = fdata_fix_get(arc, next_feat, 0);
> > + start < end;
> > + start++) {
> > + if (arc->feature_data_by_index[start] ==
> > RTE_GRAPH_FEATURE_DATA_INVALID) {
> > + arc->feature_data_by_index[start] = fdata;
> > + return start;
> > + }
> > + }
> > +
> > + /* This should not happen */
> > + if (!fdata_provided)
> > + RTE_VERIFY(0);
> > +
>
> Why panic? Return error.
Removed in v11. Have removed other referenced to RTE_VERIFY as well
<snip>
> > +/**
> > + * Prefetch feature data related fast path cache line
> > + *
> > + * @param arc
> > + * RTE_GRAPH feature arc object
> > + * @param fdata
> > + * Pointer to feature data object
> > + */
> > +__rte_experimental
> > +static __rte_always_inline void
> > +rte_graph_feature_arc_feature_data_prefetch(struct rte_graph_feature_arc
> > *arc,
> > + rte_graph_feature_data_t fdata)
> > +{
> > + if (unlikely(fdata == RTE_GRAPH_FEATURE_DATA_INVALID))
> > + return;
> > +
> Do we need above condition? Do we ever run into this? Avoid un necessary
> checks in fast path.
>
Removed check in v11
>
> > + rte_prefetch0((void *)__rte_graph_feature_data_get(arc, fdata));
> > +}
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > --
> > 2.43.0
>
