RE: [EXTERNAL] Re: [PATCH v3 0/5] add feature arc in rte_graph

[Nitin Saxena <nsaxena@marvell.com>]

Thanks Stephen. Will fix compilation in next version.

Thanks,
Nitin

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, October 9, 2024 11:08 PM
> To: Nitin Saxena <nsaxena@marvell.com>
> Cc: 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>;
> dev@dpdk.org; Nitin Saxena <nsaxena16@gmail.com>
> Subject: [EXTERNAL] Re: [PATCH v3 0/5] add feature arc in rte_graph
> 
> On Wed, 9 Oct 2024 18: 59: 57 +0530 Nitin Saxena <nsaxena@ marvell. com>
> wrote: > Feature arc represents an ordered list of features/protocols at a given
> > networking layer. It is a high level abstraction to connect various > rte_graph
> 
> On Wed, 9 Oct 2024 18:59:57 +0530
> Nitin Saxena <nsaxena@marvell.com> wrote:
> 
> > Feature arc represents an ordered list of features/protocols at a
> > given networking layer. It is a high level abstraction to connect
> > various rte_graph nodes, as feature nodes, and allow packets steering
> > across these nodes in a generic manner.
> >
> > Features (or feature nodes) are nodes which handles partial or
> > complete handling of a protocol in fast path. Like ipv4-rewrite node,
> > which adds rewrite data to an outgoing IPv4 packet.
> >
> > However in above example, outgoing interface(say "eth0") may have
> > outbound IPsec policy enabled, hence packets must be steered from
> > ipv4-rewrite node to ipsec-outbound-policy node for outbound IPsec
> > policy lookup. On the other hand, packets routed to another interface
> > (eth1) will not be sent to ipsec-outbound-policy node as IPsec feature
> > is disabled on eth1. Feature-arc allows rte_graph applications to
> > manage such constraints easily
> >
> > Feature arc abstraction allows rte_graph based application to
> >
> > 1. Seamlessly steer packets across feature nodes based on whether
> > feature is enabled or disabled on an interface. Features enabled on
> > one interface may not be enabled on another interface with in a same
> > feature arc.
> >
> > 2. Allow enabling/disabling of features on an interface at runtime, so
> > that if a feature is disabled, packets associated with that interface
> > won't be steered to corresponding feature node.
> >
> > 3. Provides mechanism to hook custom/user-defined nodes to a feature
> > node and allow packet steering from feature node to custom node
> > without changing former's fast path function
> >
> > 4. Allow expressing features in a particular sequential order so that
> > packets are steered in an ordered way across nodes in fast path. For
> > eg: if IPsec and IPv4 features are enabled on an ingress interface,
> > packets must be sent to IPsec inbound policy node first and then to
> > ipv4 lookup node.
> >
> > This patch series adds feature arc library in rte_graph and also adds
> > "ipv4-output" feature arc handling in "ipv4-rewrite" node.
> >
> > Changes in v3:
> > - rte_graph_feature_arc_t typedef from uint64_t to uintptr_t to fix
> >   compilation on 32-bit machine
> > - Updated images in .png format
> > - Added ABI change section in release notes
> > - Fixed DPDK CI failures
> >
> > Changes in v2:
> > - Added unit tests for feature arc
> > - Fixed issues found in testing
> > - Added new public APIs rte_graph_feature_arc_feature_to_node(),
> >   rte_graph_feature_arc_feature_to_name(),
> >   rte_graph_feature_arc_num_features()
> > - Added programming guide for feature arc
> > - Added release notes for feature arc
> >
> > Nitin Saxena (5):
> >   graph: add feature arc support
> >   graph: add feature arc option in graph create
> >   graph: add IPv4 output feature arc
> >   test/graph_feature_arc: add functional tests
> >   docs: add programming guide for feature arc
> >
> >  app/test/meson.build                        |    1 +
> >  app/test/test_graph_feature_arc.c           | 1410 +++++++++++++++++++
> >  doc/guides/prog_guide/graph_lib.rst         |  288 ++++
> >  doc/guides/prog_guide/img/feature_arc-1.png |  Bin 0 -> 61532 bytes
> > doc/guides/prog_guide/img/feature_arc-2.png |  Bin 0 -> 155806 bytes
> > doc/guides/prog_guide/img/feature_arc-3.png |  Bin 0 -> 143697 bytes
> >  doc/guides/rel_notes/release_24_11.rst      |   13 +
> >  lib/graph/graph.c                           |    1 +
> >  lib/graph/graph_feature_arc.c               | 1223 ++++++++++++++++
> >  lib/graph/graph_populate.c                  |    7 +-
> >  lib/graph/graph_private.h                   |    3 +
> >  lib/graph/meson.build                       |    2 +
> >  lib/graph/node.c                            |    2 +
> >  lib/graph/rte_graph.h                       |    3 +
> >  lib/graph/rte_graph_feature_arc.h           |  431 ++++++
> >  lib/graph/rte_graph_feature_arc_worker.h    |  674 +++++++++
> >  lib/graph/version.map                       |   20 +
> >  lib/node/ip4_rewrite.c                      |  476 +++++--
> >  lib/node/ip4_rewrite_priv.h                 |   15 +-
> >  lib/node/node_private.h                     |   20 +-
> >  lib/node/rte_node_ip4_api.h                 |    3 +
> >  21 files changed, 4494 insertions(+), 98 deletions(-)  create mode
> > 100644 app/test/test_graph_feature_arc.c  create mode 100644
> > doc/guides/prog_guide/img/feature_arc-1.png
> >  create mode 100644 doc/guides/prog_guide/img/feature_arc-2.png
> >  create mode 100644 doc/guides/prog_guide/img/feature_arc-3.png
> >  create mode 100644 lib/graph/graph_feature_arc.c  create mode 100644
> > lib/graph/rte_graph_feature_arc.h  create mode 100644
> > lib/graph/rte_graph_feature_arc_worker.h
> >
> 
> Looks good, but likely missing an RTE_ATOMIC() around the feature enable
> bitmask.
> Build fails:
> 
> ################################################################
> ####################
> #### [Begin job log] "ubuntu-22.04-clang-stdatomic" at step Build and test
> ################################################################
> ####################
>         rte_atomic_store_explicit(&arc->feature_enable_bitmask[passive_list],
> bitmask,
>         ^                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/eal/include/rte_stdatomic.h:76:2: note: expanded from macro
> 'rte_atomic_store_explicit'
>         atomic_store_explicit(ptr, val, memorder)
>         ^                     ~~~
> /usr/lib/llvm-14/lib/clang/14.0.0/include/stdatomic.h:127:31: note: expanded
> from macro 'atomic_store_explicit'
> #define atomic_store_explicit __c11_atomic_store
>                               ^
> ../lib/graph/graph_feature_arc.c:1084:2: error: address argument to atomic
> operation must be a pointer to _Atomic type ('rte_graph_feature_rt_list_t *'
> (aka 'unsigned short *') invalid)
>         rte_atomic_store_explicit(&arc->active_feature_list, passive_list,
>         ^                         ~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/eal/include/rte_stdatomic.h:76:2: note: expanded from macro
> 'rte_atomic_store_explicit'
>         atomic_store_explicit(ptr, val, memorder)
>         ^                     ~~~
> /usr/lib/llvm-14/lib/clang/14.0.0/include/stdatomic.h:127:31: note: expanded
> from macro 'atomic_store_explicit'
> #define atomic_store_explicit __c11_atomic_store
>                               ^
> 10 errors generated.