From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 95AAAA0527; Sat, 7 Nov 2020 18:13:07 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1BE503253; Sat, 7 Nov 2020 18:13:06 +0100 (CET) Received: from mail-io1-f67.google.com (mail-io1-f67.google.com [209.85.166.67]) by dpdk.org (Postfix) with ESMTP id 84DF42E8B for ; Sat, 7 Nov 2020 18:13:04 +0100 (CET) Received: by mail-io1-f67.google.com with SMTP id o11so5482258ioo.11 for ; Sat, 07 Nov 2020 09:13:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=whECAbrzT0sKK/FFvxXvsUWJIZoUfdzsKuy5CIaDeAA=; b=qIr58S9byUROV9c0wc1sdv+uu1oLYhhk9nMj2rKdKLp5+xVOLNw5RofqBiTsFJW/et 5mRCuhC2RGGKt1uShdvdGnf2ntpTCg0t1JtsU0ecBtXS8fsn+4gATTSKBBxOsnMBziRo MRswdQ+nwwr8imydm3hkAPQIBlYM3SIlpWqEUgdPL2WF/WnbkR+ytQDoyB7pACzNZWNI +KVoJ+eZ3tAW/mr0c/qv1nwp0sUlx6UEY8enNTD69UsbQlwYHCJwLxTZOm6nRtcQYQPg ktyUcxsruxQE1ojWL28zx+KO3TrM2xgVGnlSSSePH8CGl/sfvQGpW+MwZGqrE5B4fV1H rlOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=whECAbrzT0sKK/FFvxXvsUWJIZoUfdzsKuy5CIaDeAA=; b=INxKFKlnFK1zUyjsAxZnbixYTH9920K3khMPrRGn3RfEcsy3eQIRs0slndHyxYCCV7 3MUx48NRzJ3BRG5Zo2f1RnsZPWmGWMcX+Ux1n1ui9wFNps1SWBiWv2ytX8jf22IWUMNh 9EdD6Q+M3OcAUoV4QLH05lLeVa9DneojWX0OGFLbPc7DAJ3ahTfIZaKhZ73rMrBCNb9s v0sgfU49jJBoQtYZaNTstN8seGS/AaKy3eRziG7Xw2trI0IFR4pxgJJfmpXqnAnzxsKm vxoPJijzGIMNrIWkbKBQSxM0fQfcvgd0arK3hYWmMz7IahhXQn4sqiai/2ziSABenhVI Xc4A== X-Gm-Message-State: AOAM5306KPsNRuxPA3ha2K0VSjST1Z84F8cACEigwwzMmTojSljfl79+ 9GJmyDK59RNieQG9AAhuTOPuqQXXe2ZyWnoUVvk= X-Google-Smtp-Source: ABdhPJyX6ocbSkeewlCEEWbqcSYVEQrZXqpLTrInYmTVnboCcqAcxoN38qvzmoiBRLnM+JSYVWBTR2NWsmW4bdmf/K4= X-Received: by 2002:a02:cc79:: with SMTP id j25mr6044239jaq.104.1604769182701; Sat, 07 Nov 2020 09:13:02 -0800 (PST) MIME-Version: 1.0 References: <20201107155306.463148-1-thomas@monjalon.net> In-Reply-To: <20201107155306.463148-1-thomas@monjalon.net> From: Jerin Jacob Date: Sat, 7 Nov 2020 22:42:46 +0530 Message-ID: To: Thomas Monjalon Cc: dpdk-dev , David Marchand , Ferruh Yigit , Olivier Matz , =?UTF-8?Q?Morten_Br=C3=B8rup?= , "Ananyev, Konstantin" , Andrew Rybchenko , Viacheslav Ovsiienko , Ajit Khaparde , Jerin Jacob , Hemant Agrawal , Ray Kinsella , Neil Horman , Nithin Dabilpuram , Kiran Kumar K Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH 1/1] mbuf: move pool pointer in first half X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Sat, Nov 7, 2020 at 10:04 PM Thomas Monjalon wrote: > > The mempool pointer in the mbuf struct is moved > from the second to the first half. > It should increase performance on most systems having 64-byte cache line, > i.e. mbuf is split in two cache lines. But In any event, Tx needs to touch the pool to freeing back to the pool upon Tx completion. Right? Not able to understand the motivation for moving it to the first 64B cache line? The gain varies from driver to driver. For example, a Typical ARM-based NPU does not need to touch the pool in Rx and its been filled by HW. Whereas it needs to touch in Tx if the reference count is implemented. > > Due to this change, tx_offload is moved, so some vector data paths > may need to be adjusted. Note: OCTEON TX2 check is removed temporarily! It will be breaking the Tx path, Please just don't remove the static assert without adjusting the code. > > Moving this field gives more space to dynfield1. > > This is how the mbuf layout looks like (pahole-style): > > word type name byte size > 0 void * buf_addr; /* 0 + 8 */ > 1 rte_iova_t buf_iova /* 8 + 8 */ > /* --- RTE_MARKER64 rearm_data; */ > 2 uint16_t data_off; /* 16 + 2 */ > uint16_t refcnt; /* 18 + 2 */ > uint16_t nb_segs; /* 20 + 2 */ > uint16_t port; /* 22 + 2 */ > 3 uint64_t ol_flags; /* 24 + 8 */ > /* --- RTE_MARKER rx_descriptor_fields1; */ > 4 uint32_t union packet_type; /* 32 + 4 */ > uint32_t pkt_len; /* 36 + 4 */ > 5 uint16_t data_len; /* 40 + 2 */ > uint16_t vlan_tci; /* 42 + 2 */ > 5.5 uint64_t union hash; /* 44 + 8 */ > 6.5 uint16_t vlan_tci_outer; /* 52 + 2 */ > uint16_t buf_len; /* 54 + 2 */ > 7 struct rte_mempool * pool; /* 56 + 8 */ > /* --- RTE_MARKER cacheline1; */ > 8 struct rte_mbuf * next; /* 64 + 8 */ > 9 uint64_t union tx_offload; /* 72 + 8 */ > 10 struct rte_mbuf_ext_shared_info * shinfo; /* 80 + 8 */ > 11 uint16_t priv_size; /* 88 + 2 */ > uint16_t timesync; /* 90 + 2 */ > 11.5 uint32_t dynfield1[9]; /* 92 + 36 */ > 16 /* --- END 128 */ > > Signed-off-by: Thomas Monjalon > --- > doc/guides/rel_notes/deprecation.rst | 7 ------- > drivers/net/octeontx2/otx2_ethdev.c | 2 -- > lib/librte_kni/rte_kni_common.h | 3 ++- > lib/librte_mbuf/rte_mbuf.h | 1 - > lib/librte_mbuf/rte_mbuf_core.h | 5 ++--- > lib/librte_mbuf/rte_mbuf_dyn.c | 1 - > 6 files changed, 4 insertions(+), 15 deletions(-) > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst > index f3258eb3f7..efb09f0c5e 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -81,13 +81,6 @@ Deprecation Notices > us extending existing enum/define. > One solution can be using a fixed size array instead of ``.*MAX.*`` value. > > -* mbuf: Some fields will be converted to dynamic API in DPDK 20.11 > - in order to reserve more space for the dynamic fields, as explained in > - `this presentation `_. > - As a consequence, the layout of the ``struct rte_mbuf`` will be re-arranged, > - avoiding impact on vectorized implementation of the driver datapaths, > - while evaluating performance gains of a better use of the first cache line. > - > * ethdev: The flow director API, including ``rte_eth_conf.fdir_conf`` field, > and the related structures (``rte_fdir_*`` and ``rte_eth_fdir_*``), > will be removed in DPDK 20.11. > diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c > index 6cebbe677d..d6e0f1dd03 100644 > --- a/drivers/net/octeontx2/otx2_ethdev.c > +++ b/drivers/net/octeontx2/otx2_ethdev.c > @@ -748,8 +748,6 @@ nix_tx_offload_flags(struct rte_eth_dev *eth_dev) > offsetof(struct rte_mbuf, buf_iova) + 16); > RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, pkt_len) != > offsetof(struct rte_mbuf, ol_flags) + 12); > - RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, tx_offload) != > - offsetof(struct rte_mbuf, pool) + 2 * sizeof(void *)); > > if (conf & DEV_TX_OFFLOAD_VLAN_INSERT || > conf & DEV_TX_OFFLOAD_QINQ_INSERT) > diff --git a/lib/librte_kni/rte_kni_common.h b/lib/librte_kni/rte_kni_common.h > index 36d66e2ffa..ffb3182731 100644 > --- a/lib/librte_kni/rte_kni_common.h > +++ b/lib/librte_kni/rte_kni_common.h > @@ -84,10 +84,11 @@ 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. */ > + char pad3[14]; > + void *pool; > > /* fields on second cache line */ > __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE))) > - void *pool; > void *next; /**< Physical address of next mbuf in kernel. */ > }; > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index 3190a29cce..c4c9ebfaa0 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -1107,7 +1107,6 @@ rte_pktmbuf_attach_extbuf(struct rte_mbuf *m, void *buf_addr, > static inline void > rte_mbuf_dynfield_copy(struct rte_mbuf *mdst, const struct rte_mbuf *msrc) > { > - memcpy(&mdst->dynfield0, msrc->dynfield0, sizeof(mdst->dynfield0)); > memcpy(&mdst->dynfield1, msrc->dynfield1, sizeof(mdst->dynfield1)); > } > > diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h > index debaace95a..567551deab 100644 > --- a/lib/librte_mbuf/rte_mbuf_core.h > +++ b/lib/librte_mbuf/rte_mbuf_core.h > @@ -586,12 +586,11 @@ struct rte_mbuf { > > uint16_t buf_len; /**< Length of segment buffer. */ > > - uint64_t dynfield0[1]; /**< Reserved for dynamic fields. */ > + struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ > > /* second cache line - fields only used in slow path or on TX */ > RTE_MARKER cacheline1 __rte_cache_min_aligned; > > - struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ > struct rte_mbuf *next; /**< Next segment of scattered packet. */ > > /* fields to support TX offloads */ > @@ -645,7 +644,7 @@ struct rte_mbuf { > /** Timesync flags for use with IEEE1588. */ > uint16_t timesync; > > - uint32_t dynfield1[7]; /**< Reserved for dynamic fields. */ > + uint32_t dynfield1[9]; /**< Reserved for dynamic fields. */ > } __rte_cache_aligned; > > /** > diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c > index fd3e019a22..7d5e942bf0 100644 > --- a/lib/librte_mbuf/rte_mbuf_dyn.c > +++ b/lib/librte_mbuf/rte_mbuf_dyn.c > @@ -125,7 +125,6 @@ init_shared_mem(void) > * rte_mbuf_dynfield_copy(). > */ > memset(shm, 0, sizeof(*shm)); > - mark_free(dynfield0); > mark_free(dynfield1); > > /* init free_flags */ > -- > 2.28.0 >