RE: [PATCH v3 1/1] mbuf: add optional no-copy dynamic field storage

["Morten Brørup" <mb@smartsharesystems.com>]

Yes, this is the solution I was looking for. Thank you.

Nitpicking...

Could you use with a more meaningful name than dynfield3, e.g. dynfield_nc or dynfield_no_copy.

A few minor clarifications suggested inline below.

With or without suggested changes,
Reviewed-by: Morten Brørup <mb@smartsharesystems.com>


> --- a/doc/guides/prog_guide/mbuf_lib.rst
> +++ b/doc/guides/prog_guide/mbuf_lib.rst
> @@ -234,6 +234,15 @@ The dynamic fields and flags are managed with the
> functions ``rte_mbuf_dyn*``.
> 
>  It is not possible to unregister fields or flags.
> 
> +The build option ``mbuf_dynfield3_size`` can add extra cache-line-
> aligned
> +dynamic field storage to ``struct rte_mbuf``.  This increases every
> mbuf by

"increases every mbuf" -> "increases the size of every mbuf"

> +the configured amount and changes the mbuf layout, so applications and
> +secondary processes must be built with the same value as the primary
> process.
> +The option defaults to ``0``.  The extra storage is reserved for
> dynamic
> +fields registered with ``RTE_MBUF_DYNFIELD_F_NO_COPY``.  These fields
> are not
> +copied by mbuf copy and clone operations.  Dynamic fields registered
> without
> +this flag continue to use the existing copied dynamic-field storage.
> +
>  .. _direct_indirect_buffer:
> 
>  Direct and Indirect Buffers
> diff --git a/doc/guides/rel_notes/release_26_11.rst
> b/doc/guides/rel_notes/release_26_11.rst
> index dec96ccbc7..cff68d7034 100644
> --- a/doc/guides/rel_notes/release_26_11.rst
> +++ b/doc/guides/rel_notes/release_26_11.rst
> @@ -60,6 +60,22 @@ New Features
>    Added the experimental ``rte_cpu_socket_id()`` function
>    to map an OS logical CPU ID to the NUMA socket containing that CPU.
> 
> +* **Added optional extra mbuf dynamic field storage.**
> +
> +  Added ``mbuf_dynfield3_size`` build option to enable a
> +  cache-line-aligned ``dynfield3`` area in ``struct rte_mbuf``.
> +  The configured size is defined as ``RTE_MBUF_DYNFIELD3_SIZE``
> +  in ``rte_build_config.h``.
> +  The extra storage is reserved for dynamic fields registered with
> +  ``RTE_MBUF_DYNFIELD_F_NO_COPY``.
> +  These fields are not copied by generic mbuf copy and clone
> operations.
> +  Dynamic fields registered without this flag continue to use the
> existing
> +  copied dynamic-field storage.
> +  Applications and secondary processes must be built with the same
> +  non-zero value as the primary process.

"with the same non-zero value" ->"with the same value"


> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -44,6 +44,8 @@ option('max_numa_nodes', type: 'string', value:
> 'default', description:
>         'Set the highest NUMA node supported by EAL; "default" is
> different per-arch, "detect" detects the highest NUMA node on the build
> machine.')
>  option('enable_iova_as_pa', type: 'boolean', value: true, description:
>         'Support the use of physical addresses for IO addresses, such
> as used by UIO or VFIO in no-IOMMU mode. When disabled, DPDK can only
> run with IOMMU support for address mappings, but will have more space
> available in the mbuf structure.')
> +option('mbuf_dynfield3_size', type: 'integer', min: 0, value: 0,
> description:
> +       'Size of optional extra mbuf dynamic field area, in bytes.')

The description of this option should mention that the area is for mbuf dynamic fields with the no-copy property.

>  option('mbuf_refcnt_atomic', type: 'boolean', value: true,
> description:
>         'Atomically access the mbuf refcnt.')
>  option('platform', type: 'string', value: 'native', description:
> --
> 2.35.6