Re: [PATCH v7 1/2] node: add global node mbuf dynfield

[Thomas Monjalon <thomas@monjalon.net>]

26/06/2025 16:14, Nitin Saxena:
> On Thu, Jun 26, 2025 at 7:29 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > On Thu, 19 Jun 2025 21:14:49 +0530
> > Nitin Saxena <nsaxena@marvell.com> wrote:
> >
> > > +#ifndef RTE_NODE_MBUF_PERSISTENT_FIELDS_SIZE
> > > +/** Size of persistent mbuf fields */
> > > +#define RTE_NODE_MBUF_PERSISTENT_FIELDS_SIZE          (0)
> > > +#endif /* RTE_NODE_MBUF_PERSISTENT_FIELDS_SIZE */
> > > +
> > > +#ifndef RTE_NODE_MBUF_OVERLOADABLE_FIELDS_SIZE
> > > +/** Size of overloadable mbuf fields */
> > > +#define RTE_NODE_MBUF_OVERLOADABLE_FIELDS_SIZE        (8)
> > > +#endif /* RTE_NODE_MBUF_OVERLOADABLE_FIELDS_SIZE */
> > > +
> > > +/** Size of node mbuf dynamic field */
> > > +#define RTE_NODE_MBUF_DYNFIELD_SIZE     \
> > > +     (RTE_NODE_MBUF_PERSISTENT_FIELDS_SIZE + RTE_NODE_MBUF_OVERLOADABLE_FIELDS_SIZE)
> > > +
> > > +/**
> > > + * Node mbuf overloadable data.
> > > + *
> > > + * Out-of-tree nodes can repurpose overloadable fields via
> > > + * rte_node_mbuf_overload_fields_get(mbuf). Overloadable fields are not
> > > + * preserved and typically can be used with-in two adjacent nodes in the graph.
> > > + */
> > > +typedef struct rte_node_mbuf_overload_fields {
> > > +     union {
> > > +             uint8_t data[RTE_NODE_MBUF_OVERLOADABLE_FIELDS_SIZE];
> > > +     };
> > > +} rte_node_mbuf_overload_fields_t;
> >
> > Having a blank scratchpad rather than a real defined structure means you
> > lose any type checking or potential overflow checking.
> 
> Patchset-1 creates a blank scratchpad but patchset-2 adds following
> fields when added field is used in ip4/ip6 nodes
> So idea is to add relevant structures as and when they are used
> 
>  diff --git a/lib/node/rte_node_mbuf_dynfield.h
> b/lib/node/rte_node_mbuf_dynfield.h
>  index 1069c07d44..045c13e352 100644
>  --- a/lib/node/rte_node_mbuf_dynfield.h
>  +++ b/lib/node/rte_node_mbuf_dynfield.h
>  @@ -46,6 +46,15 @@ extern "C" {
>    */
>   typedef struct rte_node_mbuf_overload_fields {
>   >------union {
> +>------>-------/* Following fields used by ip[4|6]-lookup ->
> ip[4|6]-rewrite nodes */
> +>------>-------union {
> +>------>------->-------struct {
> +>------>------->------->-------uint16_t nh;
> +>------>------->------->-------uint16_t ttl;
> +>------>------->------->-------uint32_t cksum;
> +>------>------->-------};
> +>------>------->-------uint64_t u;
> +>------>-------};
> >------>-------uint8_t data[RTE_NODE_MBUF_OVERLOADABLE_FIELDS_SIZE];
> >------};
> } rte_node_mbuf_overload_fields_t;
> 

I assume it is OK to merge.
It was supposed to be merged in -rc1,
so it is applied now for -rc2, thanks.