Re: [PATCH v10 4/7] graph: add feature enable/disable APIs

[Nitin Saxena <nsaxena16@gmail.com>]

Hi Kiran,


On Wed, Jun 4, 2025 at 5:06 PM Kiran Kumar Kokkilagadda
<kirankumark@marvell.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Nitin Saxena <nsaxena@marvell.com>
> > Sent: Wednesday, June 4, 2025 3:43 PM
> > To: Jerin Jacob <jerinj@marvell.com>; Kiran Kumar Kokkilagadda
> > <kirankumark@marvell.com>; Nithin Kumar Dabilpuram
> > <ndabilpuram@marvell.com>; Zhirun Yan <yanzhirun_163@163.com>; Robin
> > Jarry <rjarry@redhat.com>; Christophe Fontaine <cfontain@redhat.com>
> > Cc: dev@dpdk.org; Nitin Saxena <nsaxena16@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 <nsaxena@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
>