From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Wiles, Keith" <keith.wiles@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly
Date: Mon, 27 Jun 2016 08:21:03 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB97725836B7715C@irsmsx105.ger.corp.intel.com> (raw)
In-Reply-To: <1466870154-67659-1-git-send-email-keith.wiles@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Keith Wiles
> Sent: Saturday, June 25, 2016 4:56 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly
>
> Move the next pointer to the first cacheline of the rte_mbuf structure
> and move the offload values to the second cacheline to give better
> performance to applications using chained mbufs.
>
> Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
> is set to No.
First, it would make ixgbe and i40e vector RX functions to work incorrectly.
Second, I don't think we can afford to allow people swap mbuf fields in the way they like.
Otherwise we'll end-up with totally unmaintainable code pretty soon.
So NACK.
Konstantin
>
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
> config/common_base | 2 +
> .../linuxapp/eal/include/exec-env/rte_kni_common.h | 8 +++
> lib/librte_mbuf/rte_mbuf.h | 67 +++++++++++++++-------
> 3 files changed, 56 insertions(+), 21 deletions(-)
>
> diff --git a/config/common_base b/config/common_base
> index 379a791..f7c624e 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -405,6 +405,8 @@ CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
> CONFIG_RTE_MBUF_DEFAULT_MEMPOOL_OPS="ring_mp_mc"
> CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
> CONFIG_RTE_PKTMBUF_HEADROOM=128
> +# Set to y if needing to be mbuf chain friendly.
> +CONFIG_RTE_MBUF_CHAIN_FRIENDLY=n
>
> #
> # Compile librte_timer
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-
> env/rte_kni_common.h
> index 2acdfd9..44d65cd 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> @@ -120,11 +120,19 @@ struct rte_kni_mbuf {
> char pad2[4];
> uint32_t pkt_len; /**< Total pkt len: sum of all segment data_len. */
> uint16_t data_len; /**< Amount of data in segment buffer. */
> +#ifdef RTE_MBUF_CHAIN_FRIENDLY
> + char pad3[8];
> + void *next;
>
> /* fields on second cache line */
> + char pad4[16] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
> + void *pool;
> +#else
> + /* fields on second cache line */
> char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
> void *pool;
> void *next;
> +#endif
> };
>
> /*
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 15e3a10..6e6ba0e 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -765,6 +765,28 @@ typedef uint8_t MARKER8[0]; /**< generic marker with 1B alignment */
> typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8 bytes
> * with a single assignment */
>
> +typedef union {
> + uint32_t rss; /**< RSS hash result if RSS enabled */
> + struct {
> + union {
> + struct {
> + uint16_t hash;
> + uint16_t id;
> + };
> + uint32_t lo;
> + /**< Second 4 flexible bytes */
> + };
> + uint32_t hi;
> + /**< First 4 flexible bytes or FD ID, dependent on
> + PKT_RX_FDIR_* flag in ol_flags. */
> + } fdir; /**< Filter identifier if FDIR enabled */
> + struct {
> + uint32_t lo;
> + uint32_t hi;
> + } sched; /**< Hierarchical scheduler */
> + uint32_t usr; /**< User defined tags. See rte_distributor_process() */
> +} rss_hash_t;
> +
> /**
> * The generic rte_mbuf, containing a packet mbuf.
> */
> @@ -824,28 +846,31 @@ struct rte_mbuf {
> uint16_t data_len; /**< Amount of data in segment buffer. */
> /** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */
> uint16_t vlan_tci;
> +#ifdef RTE_MBUF_CHAIN_FRIENDLY
> + /*
> + * Move offload into the second cache line and next in the first.
> + * Better performance for applications using chained mbufs to have
> + * the next pointer in the first cache line.
> + * If you change this structure, you must change the user-mode
> + * version in rte_kni_common.h to match the new layout.
> + */
> + uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
> + uint16_t vlan_tci_outer; /**< Outer VLAN Tag Control Identifier (CPU order) */
> + struct rte_mbuf *next; /**< Next segment of scattered packet. */
> +
> + /* second cache line - fields only used in slow path or on TX */
> + MARKER cacheline1 __rte_cache_min_aligned;
> +
> + rss_hash_t hash; /**< hash information */
>
> union {
> - uint32_t rss; /**< RSS hash result if RSS enabled */
> - struct {
> - union {
> - struct {
> - uint16_t hash;
> - uint16_t id;
> - };
> - uint32_t lo;
> - /**< Second 4 flexible bytes */
> - };
> - uint32_t hi;
> - /**< First 4 flexible bytes or FD ID, dependent on
> - PKT_RX_FDIR_* flag in ol_flags. */
> - } fdir; /**< Filter identifier if FDIR enabled */
> - struct {
> - uint32_t lo;
> - uint32_t hi;
> - } sched; /**< Hierarchical scheduler */
> - uint32_t usr; /**< User defined tags. See rte_distributor_process() */
> - } hash; /**< hash information */
> + void *userdata; /**< Can be used for external metadata */
> + uint64_t udata64; /**< Allow 8-byte userdata on 32-bit */
> + };
> +
> + struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
> +#else
> + rss_hash_t hash; /**< hash information */
>
> uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
>
> @@ -862,7 +887,7 @@ struct rte_mbuf {
>
> struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
> struct rte_mbuf *next; /**< Next segment of scattered packet. */
> -
> +#endif
> /* fields to support TX offloads */
> union {
> uint64_t tx_offload; /**< combined for easy fetch */
> --
> 2.8.0.GIT
next prev parent reply other threads:[~2016-06-27 8:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-25 15:29 [dpdk-dev] [PATCH] " Keith Wiles
2016-06-25 15:48 ` Wiles, Keith
2016-06-25 15:55 ` [dpdk-dev] [PATCH v2] " Keith Wiles
2016-06-27 8:21 ` Ananyev, Konstantin [this message]
2016-06-27 8:27 ` Olivier Matz
2016-06-27 9:05 ` Thomas Monjalon
2016-06-27 13:06 ` Wiles, Keith
2016-06-27 14:30 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2601191342CEEE43887BDE71AB97725836B7715C@irsmsx105.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=dev@dpdk.org \
--cc=keith.wiles@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).