Re: [dpdk-dev] [PATCH] mbuf: replace c memcpy() code semantics with optimized rte_memcpy()

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 23 Jul 2020 12:02:40 +0500
Sarosh Arif <sarosh.arif@emumba.com> wrote:

> Since rte_memcpy is more optimized it should be used instead of memcpy
> 
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>

The part in pkmbuf_pool_init is not performance critical.

The layout of rte_mbuf_dynfield is sub optimal.

struct rte_mbuf_dynfield {
	char                       name[64];             /*     0    64 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	size_t                     size;                 /*    64     8 */
	size_t                     align;                /*    72     8 */
	unsigned int               flags;                /*    80     4 */

	/* size: 88, cachelines: 2, members: 4 */
	/* padding: 4 */
	/* last cacheline: 24 bytes */
};

1. It should have been sized so that overall it was 64 bytes.

2. Use 8 bytes for size and align is wasteful.

3. Hold 4 bytes for future flags is also wasteful. YAGNI

If you look at assembly output on x86 the copy of params becomes a sequence
of vmovups instructions with Gcc.

For 20.11 maybe:

diff --git a/lib/librte_mbuf/rte_mbuf_dyn.h b/lib/librte_mbuf/rte_mbuf_dyn.h
index 8407230ecfdc..eb1d01f97f40 100644
--- a/lib/librte_mbuf/rte_mbuf_dyn.h
+++ b/lib/librte_mbuf/rte_mbuf_dyn.h
@@ -70,16 +70,16 @@
 /**
  * Maximum length of the dynamic field or flag string.
  */
-#define RTE_MBUF_DYN_NAMESIZE 64
+#define RTE_MBUF_DYN_NAMESIZE 60
 
 /**
  * Structure describing the parameters of a mbuf dynamic field.
  */
 struct rte_mbuf_dynfield {
        char name[RTE_MBUF_DYN_NAMESIZE]; /**< Name of the field. */
-       size_t size;        /**< The number of bytes to reserve. */
-       size_t align;       /**< The alignment constraint (power of 2). */
-       unsigned int flags; /**< Reserved for future use, must be 0. */
+       uint8_t size;        /**< The number of bytes to reserve. */
+       uint8_t align;       /**< The alignment constraint (power of 2). */
+       uint16_t flags; /**< Reserved for future use, must be 0. */
 };
 
 /**

Or make the dynamic field dynamic size to avoid wasting space?