Hi,
On Wed, Sep 21, 2022 at 07:26:18PM +0530, Shijith Thotton wrote:
> mbuf physical address field is not used in builds which only uses VA. It
> is used to expand the dynamic field area.
>
> Signed-off-by: Shijith Thotton <sthot...@marvell.com>
> ---
> lib/mbuf/rte_mbuf_core.h | 26 +++++++++++++++++---------
> lib/mbuf/rte_mbuf_dyn.c | 2 ++
> 2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> index c6292e7252..94907f301d 100644
> --- a/lib/mbuf/rte_mbuf_core.h
> +++ b/lib/mbuf/rte_mbuf_core.h
> @@ -579,15 +579,23 @@ struct rte_mbuf {
> RTE_MARKER cacheline0;
>
> void *buf_addr; /**< Virtual address of segment buffer. */
> - /**
> - * Physical address of segment buffer.
> - * This field is invalid if the build is configured to use only
> - * virtual address as IOVA (i.e. RTE_IOVA_AS_VA is 1).
> - * Force alignment to 8-bytes, so as to ensure we have the exact
> - * same mbuf cacheline0 layout for 32-bit and 64-bit. This makes
> - * working on vector drivers easier.
> - */
> - rte_iova_t buf_iova __rte_aligned(sizeof(rte_iova_t));
> + RTE_STD_C11
> + union {
> + /**
> + * Physical address of segment buffer.
> + * This field is invalid if the build is configured to use only
> + * virtual address as IOVA (i.e. RTE_IOVA_AS_VA is 1).
> + * Force alignment to 8-bytes, so as to ensure we have the exact
> + * same mbuf cacheline0 layout for 32-bit and 64-bit. This makes
> + * working on vector drivers easier.
> + */
> + rte_iova_t buf_iova __rte_aligned(sizeof(rte_iova_t));
> + /**
> + * Reserved for dynamic field in builds where physical address
> + * field is invalid.
> + */
> + uint64_t dynfield2;
> + };
Same comment than on previous patch: using a #if instead of the union here looks
better to me to ensure that we never use buf_iova when RTE_IOVA_AS_VA=1.
>
> /* next 8 bytes are initialised on RX descriptor rearm */
> RTE_MARKER64 rearm_data;
> diff --git a/lib/mbuf/rte_mbuf_dyn.c b/lib/mbuf/rte_mbuf_dyn.c
> index 4ae79383b5..6a4cf96897 100644
> --- a/lib/mbuf/rte_mbuf_dyn.c
> +++ b/lib/mbuf/rte_mbuf_dyn.c
> @@ -128,6 +128,8 @@ init_shared_mem(void)
> */
> memset(shm, 0, sizeof(*shm));
> mark_free(dynfield1);
> + if (RTE_IOVA_AS_VA)
> + mark_free(dynfield2);
In this case, it will have to be a #if here too.
>
> /* init free_flags */
> for (mask = RTE_MBUF_F_FIRST_FREE; mask <=
> RTE_MBUF_F_LAST_FREE; mask <<= 1)
Also, I think we can add in the RTE_IOVA_AS_VA documentation that it
replaces the buf_iova by 8 bytes of dynamic field.
