Message ID | cover.1663767715.git.sthotton@marvell.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1C230A00C3; Wed, 21 Sep 2022 15:56:51 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B90DF4067C; Wed, 21 Sep 2022 15:56:50 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by mails.dpdk.org (Postfix) with ESMTP id 86F3C4014F for <dev@dpdk.org>; Wed, 21 Sep 2022 15:56:49 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 28LBPhOX032186; Wed, 21 Sep 2022 06:56:44 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=pfpt0220; bh=Ft/KZW6/vVHv3kDMWBmGvYL4jwa/4hHJ4gvL/YvdUw0=; b=IcUMKwvMUShF98bQIAeC2FDOhGNlHBwvabP6+SLLw/EbbXiPODNRqoTo5AaRyG5PBysE 6q6wUJqObJAciDSnoHO6K11nFu9dRx5vyzuw89by8ARceYndst710LlFXGXQZBH40Iu9 0h2h0S3o/E57T5R1fyS4ut8hB3NofToPjtHtI3uBBJk7otx9syBdNzwbaUAJOC7Nmp5l j2WaOr/2SjvfOYcs9AbUSYPFAa2N5/2rkn2O346NXqC6YZ799tB/1K16DOqSuQxzLDua MrwagIB9+En6dbtXG2wsxhd68HtMntxtd6Gb7eXukAb1LUOKnE6EqNAizbiliu4yXmPU rw== Received: from dc5-exch02.marvell.com ([199.233.59.182]) by mx0b-0016f401.pphosted.com (PPS) with ESMTPS id 3jr1qmgggv-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 21 Sep 2022 06:56:44 -0700 Received: from DC5-EXCH02.marvell.com (10.69.176.39) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server (TLS) id 15.0.1497.18; Wed, 21 Sep 2022 06:56:42 -0700 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server id 15.0.1497.18 via Frontend Transport; Wed, 21 Sep 2022 06:56:42 -0700 Received: from localhost.localdomain (unknown [10.28.34.29]) by maili.marvell.com (Postfix) with ESMTP id 559483F7041; Wed, 21 Sep 2022 06:56:39 -0700 (PDT) From: Shijith Thotton <sthotton@marvell.com> To: <dev@dpdk.org> CC: <pbhagavatula@marvell.com>, Shijith Thotton <sthotton@marvell.com>, <Honnappa.Nagarahalli@arm.com>, <bruce.richardson@intel.com>, <jerinj@marvell.com>, <mb@smartsharesystems.com>, <olivier.matz@6wind.com>, <stephen@networkplumber.org>, <thomas@monjalon.net>, <david.marchand@redhat.com> Subject: [PATCH v3 0/5] mbuf dynamic field expansion Date: Wed, 21 Sep 2022 19:26:16 +0530 Message-ID: <cover.1663767715.git.sthotton@marvell.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220907134340.3629224-1-sthotton@marvell.com> References: <20220907134340.3629224-1-sthotton@marvell.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Proofpoint-GUID: 7JpEOZFeuCWsOhaJ0S_clsX4cM9JDs4- X-Proofpoint-ORIG-GUID: 7JpEOZFeuCWsOhaJ0S_clsX4cM9JDs4- X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1 definitions=2022-09-21_08,2022-09-20_02,2022-06-22_01 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org |
Series |
mbuf dynamic field expansion
|
|
Message
Shijith Thotton
Sept. 21, 2022, 1:56 p.m. UTC
This is a continuation of the discussions[1] to add mbuf physical address field to dynamic field. Previous version was to add PA field to dynamic field area based on the EAL IOVA mode option. It was deemed unsafe as some components could still use the PA field without checking IOVA mode and there are drivers which need PA to work. One suggestion was to make the IOVA mode check at compile time so that drivers which need PA can be disabled during build. This series adds this new meson build options. Second patch adds mbuf PA field to dynamic field on such builds. Last two patches enable Marvell cnxk PMDs and software PMDs in IOVA as VA build as they work without PA field. 1. https://inbox.dpdk.org/dev/57d2ab7fff672716d37ba4078e2e3bb2db126607.1656605763.git.sthotton@marvell.com/. v3: * Cleared use of buf_iova from cnxk PMD. v2: * Used RTE_IOVA_AS_VA instread of rte_is_iova_as_va_build(). * Moved mbuf next pointer to first cacheline if RTE_IOVA_AS_VA = 1. Shijith Thotton (5): build: add meson option to configure IOVA mode as VA mbuf: add second dynamic field member for VA only build lib: move mbuf next pointer to first cache line drivers: mark Marvell cnxk PMDs work with IOVA as VA drivers: mark software PMDs work with IOVA as VA app/test-bbdev/test_bbdev_perf.c | 2 +- app/test-crypto-perf/cperf_test_common.c | 5 +-- app/test/test_bpf.c | 2 +- app/test/test_dmadev.c | 33 ++++++-------- app/test/test_mbuf.c | 12 +++--- app/test/test_pcapng.c | 2 +- config/arm/meson.build | 8 +++- config/meson.build | 1 + drivers/common/cnxk/meson.build | 1 + drivers/crypto/armv8/meson.build | 1 + drivers/crypto/cnxk/cn10k_ipsec_la_ops.h | 4 +- drivers/crypto/cnxk/cn9k_ipsec_la_ops.h | 2 +- drivers/crypto/cnxk/meson.build | 2 + drivers/crypto/ipsec_mb/meson.build | 1 + drivers/crypto/null/meson.build | 1 + drivers/crypto/openssl/meson.build | 1 + drivers/dma/cnxk/meson.build | 1 + drivers/dma/skeleton/meson.build | 1 + drivers/event/cnxk/meson.build | 1 + drivers/event/dsw/meson.build | 1 + drivers/event/opdl/meson.build | 1 + drivers/event/skeleton/meson.build | 1 + drivers/event/sw/meson.build | 1 + drivers/mempool/bucket/meson.build | 1 + drivers/mempool/cnxk/meson.build | 1 + drivers/mempool/ring/meson.build | 1 + drivers/mempool/stack/meson.build | 1 + drivers/meson.build | 6 +++ drivers/net/af_packet/meson.build | 1 + drivers/net/af_xdp/meson.build | 2 + drivers/net/bonding/meson.build | 1 + drivers/net/cnxk/cn10k_tx.h | 55 +++++++----------------- drivers/net/cnxk/cn9k_tx.h | 55 +++++++----------------- drivers/net/cnxk/cnxk_ethdev.h | 1 - drivers/net/cnxk/meson.build | 1 + drivers/net/failsafe/meson.build | 1 + drivers/net/memif/meson.build | 1 + drivers/net/null/meson.build | 1 + drivers/net/pcap/meson.build | 1 + drivers/net/ring/meson.build | 1 + drivers/net/tap/meson.build | 1 + drivers/raw/cnxk_bphy/meson.build | 1 + drivers/raw/cnxk_gpio/meson.build | 1 + drivers/raw/skeleton/meson.build | 1 + lib/eal/linux/eal.c | 7 +++ lib/mbuf/rte_mbuf.c | 8 ++-- lib/mbuf/rte_mbuf.h | 17 +++++--- lib/mbuf/rte_mbuf_core.h | 55 ++++++++++++++++++------ lib/mbuf/rte_mbuf_dyn.c | 2 + lib/meson.build | 3 ++ lib/vhost/vhost.h | 2 +- lib/vhost/vhost_crypto.c | 54 +++++++++++++++++------ meson_options.txt | 2 + 53 files changed, 220 insertions(+), 150 deletions(-)
Comments
> >This is a continuation of the discussions[1] to add mbuf physical address field to >dynamic field. >Previous version was to add PA field to dynamic field area based on the EAL IOVA >mode option. It was >deemed unsafe as some components could still use the PA field without checking >IOVA mode and there >are drivers which need PA to work. One suggestion was to make the IOVA mode >check at compile time so >that drivers which need PA can be disabled during build. This series adds this new >meson build >options. Second patch adds mbuf PA field to dynamic field on such builds. Last two >patches enable >Marvell cnxk PMDs and software PMDs in IOVA as VA build as they work without >PA field. > >1. >https://inbox.dpdk.org/dev/57d2ab7fff672716d37ba4078e2e3bb2db126607.16566 >05763.git.sthotton@marvell.com/. > >v3: > * Cleared use of buf_iova from cnxk PMD. > >v2: > * Used RTE_IOVA_AS_VA instread of rte_is_iova_as_va_build(). > * Moved mbuf next pointer to first cacheline if RTE_IOVA_AS_VA = 1. > >Shijith Thotton (5): > build: add meson option to configure IOVA mode as VA > mbuf: add second dynamic field member for VA only build > lib: move mbuf next pointer to first cache line > drivers: mark Marvell cnxk PMDs work with IOVA as VA > drivers: mark software PMDs work with IOVA as VA > Hi All, Please comment if any changes are needed on the series. Right now, there is 1 ack from Morten. Thanks, Shijith
Hi Shijith, On Wed, Sep 21, 2022 at 07:26:16PM +0530, Shijith Thotton wrote: > This is a continuation of the discussions[1] to add mbuf physical address field to dynamic field. > Previous version was to add PA field to dynamic field area based on the EAL IOVA mode option. It was > deemed unsafe as some components could still use the PA field without checking IOVA mode and there > are drivers which need PA to work. One suggestion was to make the IOVA mode check at compile time so > that drivers which need PA can be disabled during build. This series adds this new meson build > options. Second patch adds mbuf PA field to dynamic field on such builds. Last two patches enable > Marvell cnxk PMDs and software PMDs in IOVA as VA build as they work without PA field. Thank you for this patchset. To be honnest, initially I was really reserved to remove the use of buf_iova for some specific platforms. But what made me change my mind is that the removal if buf_iova will likely happen in the long-term future. It looks there is a consensus on this. I think your patchset is a good way to prepare this transition. What is missing, I think, is a good description of the problem you are solving: - more space for dynamic mbuf fields -> why? can you give more detail about this need? - increase performance -> you previously said that it was not your point, but if we move the next field into the first cache line, I think this has to be highlighted. Out of curiosity, did you made measurements? I'm sending separate comments as replies to the patches. Olivier > > 1. https://inbox.dpdk.org/dev/57d2ab7fff672716d37ba4078e2e3bb2db126607.1656605763.git.sthotton@marvell.com/. > > v3: > * Cleared use of buf_iova from cnxk PMD. > > v2: > * Used RTE_IOVA_AS_VA instread of rte_is_iova_as_va_build(). > * Moved mbuf next pointer to first cacheline if RTE_IOVA_AS_VA = 1. > > Shijith Thotton (5): > build: add meson option to configure IOVA mode as VA > mbuf: add second dynamic field member for VA only build > lib: move mbuf next pointer to first cache line > drivers: mark Marvell cnxk PMDs work with IOVA as VA > drivers: mark software PMDs work with IOVA as VA > > app/test-bbdev/test_bbdev_perf.c | 2 +- > app/test-crypto-perf/cperf_test_common.c | 5 +-- > app/test/test_bpf.c | 2 +- > app/test/test_dmadev.c | 33 ++++++-------- > app/test/test_mbuf.c | 12 +++--- > app/test/test_pcapng.c | 2 +- > config/arm/meson.build | 8 +++- > config/meson.build | 1 + > drivers/common/cnxk/meson.build | 1 + > drivers/crypto/armv8/meson.build | 1 + > drivers/crypto/cnxk/cn10k_ipsec_la_ops.h | 4 +- > drivers/crypto/cnxk/cn9k_ipsec_la_ops.h | 2 +- > drivers/crypto/cnxk/meson.build | 2 + > drivers/crypto/ipsec_mb/meson.build | 1 + > drivers/crypto/null/meson.build | 1 + > drivers/crypto/openssl/meson.build | 1 + > drivers/dma/cnxk/meson.build | 1 + > drivers/dma/skeleton/meson.build | 1 + > drivers/event/cnxk/meson.build | 1 + > drivers/event/dsw/meson.build | 1 + > drivers/event/opdl/meson.build | 1 + > drivers/event/skeleton/meson.build | 1 + > drivers/event/sw/meson.build | 1 + > drivers/mempool/bucket/meson.build | 1 + > drivers/mempool/cnxk/meson.build | 1 + > drivers/mempool/ring/meson.build | 1 + > drivers/mempool/stack/meson.build | 1 + > drivers/meson.build | 6 +++ > drivers/net/af_packet/meson.build | 1 + > drivers/net/af_xdp/meson.build | 2 + > drivers/net/bonding/meson.build | 1 + > drivers/net/cnxk/cn10k_tx.h | 55 +++++++----------------- > drivers/net/cnxk/cn9k_tx.h | 55 +++++++----------------- > drivers/net/cnxk/cnxk_ethdev.h | 1 - > drivers/net/cnxk/meson.build | 1 + > drivers/net/failsafe/meson.build | 1 + > drivers/net/memif/meson.build | 1 + > drivers/net/null/meson.build | 1 + > drivers/net/pcap/meson.build | 1 + > drivers/net/ring/meson.build | 1 + > drivers/net/tap/meson.build | 1 + > drivers/raw/cnxk_bphy/meson.build | 1 + > drivers/raw/cnxk_gpio/meson.build | 1 + > drivers/raw/skeleton/meson.build | 1 + > lib/eal/linux/eal.c | 7 +++ > lib/mbuf/rte_mbuf.c | 8 ++-- > lib/mbuf/rte_mbuf.h | 17 +++++--- > lib/mbuf/rte_mbuf_core.h | 55 ++++++++++++++++++------ > lib/mbuf/rte_mbuf_dyn.c | 2 + > lib/meson.build | 3 ++ > lib/vhost/vhost.h | 2 +- > lib/vhost/vhost_crypto.c | 54 +++++++++++++++++------ > meson_options.txt | 2 + > 53 files changed, 220 insertions(+), 150 deletions(-) > > -- > 2.25.1 >
Hi Olivier, Thanks for the review. >On Wed, Sep 21, 2022 at 07:26:16PM +0530, Shijith Thotton wrote: >> This is a continuation of the discussions[1] to add mbuf physical address field to >dynamic field. >> Previous version was to add PA field to dynamic field area based on the EAL >IOVA mode option. It was >> deemed unsafe as some components could still use the PA field without >checking IOVA mode and there >> are drivers which need PA to work. One suggestion was to make the IOVA mode >check at compile time so >> that drivers which need PA can be disabled during build. This series adds this >new meson build >> options. Second patch adds mbuf PA field to dynamic field on such builds. Last >two patches enable >> Marvell cnxk PMDs and software PMDs in IOVA as VA build as they work without >PA field. > >Thank you for this patchset. > >To be honnest, initially I was really reserved to remove the use of >buf_iova for some specific platforms. > >But what made me change my mind is that the removal if buf_iova will >likely happen in the long-term future. It looks there is a consensus on >this. I think your patchset is a good way to prepare this transition. > >What is missing, I think, is a good description of the problem you are >solving: > >- more space for dynamic mbuf fields -> why? can you give more detail about > this need? Idea was to let app/lib use an additional 8-bytes of dynamic area. >- increase performance -> you previously said that it was not your point, > but if we move the next field into the first cache line, I think this > has to be highlighted. Out of curiosity, did you made measurements? > I'm yet to do it. I will update, once I have the numbers. >> >> 1. https://urldefense.proofpoint.com/v2/url?u=https- >3A__inbox.dpdk.org_dev_57d2ab7fff672716d37ba4078e2e3bb2db126607.1656605 >763.git.sthotton- >40marvell.com_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=G9w4KsPaQLACBf >GCL35PtiRH996yqJDxAZwrWegU2qQ&m=O9JeIb0lfExyVnC8dV3WUADowh165KkS >3s9JrmAjLwj8Uw5Iyb0tqSQ9YvQWpbIc&s=DaHEYGwUqUmAFmQ9Jkj8jGnOS4aw8 >iZ8Tcww-jPTdFE&e= . >> >> v3: >> * Cleared use of buf_iova from cnxk PMD. >> >> v2: >> * Used RTE_IOVA_AS_VA instread of rte_is_iova_as_va_build(). >> * Moved mbuf next pointer to first cacheline if RTE_IOVA_AS_VA = 1. >> >> Shijith Thotton (5): >> build: add meson option to configure IOVA mode as VA >> mbuf: add second dynamic field member for VA only build >> lib: move mbuf next pointer to first cache line >> drivers: mark Marvell cnxk PMDs work with IOVA as VA >> drivers: mark software PMDs work with IOVA as VA >> >> app/test-bbdev/test_bbdev_perf.c | 2 +- >> app/test-crypto-perf/cperf_test_common.c | 5 +-- >> app/test/test_bpf.c | 2 +- >> app/test/test_dmadev.c | 33 ++++++-------- >> app/test/test_mbuf.c | 12 +++--- >> app/test/test_pcapng.c | 2 +- >> config/arm/meson.build | 8 +++- >> config/meson.build | 1 + >> drivers/common/cnxk/meson.build | 1 + >> drivers/crypto/armv8/meson.build | 1 + >> drivers/crypto/cnxk/cn10k_ipsec_la_ops.h | 4 +- >> drivers/crypto/cnxk/cn9k_ipsec_la_ops.h | 2 +- >> drivers/crypto/cnxk/meson.build | 2 + >> drivers/crypto/ipsec_mb/meson.build | 1 + >> drivers/crypto/null/meson.build | 1 + >> drivers/crypto/openssl/meson.build | 1 + >> drivers/dma/cnxk/meson.build | 1 + >> drivers/dma/skeleton/meson.build | 1 + >> drivers/event/cnxk/meson.build | 1 + >> drivers/event/dsw/meson.build | 1 + >> drivers/event/opdl/meson.build | 1 + >> drivers/event/skeleton/meson.build | 1 + >> drivers/event/sw/meson.build | 1 + >> drivers/mempool/bucket/meson.build | 1 + >> drivers/mempool/cnxk/meson.build | 1 + >> drivers/mempool/ring/meson.build | 1 + >> drivers/mempool/stack/meson.build | 1 + >> drivers/meson.build | 6 +++ >> drivers/net/af_packet/meson.build | 1 + >> drivers/net/af_xdp/meson.build | 2 + >> drivers/net/bonding/meson.build | 1 + >> drivers/net/cnxk/cn10k_tx.h | 55 +++++++----------------- >> drivers/net/cnxk/cn9k_tx.h | 55 +++++++----------------- >> drivers/net/cnxk/cnxk_ethdev.h | 1 - >> drivers/net/cnxk/meson.build | 1 + >> drivers/net/failsafe/meson.build | 1 + >> drivers/net/memif/meson.build | 1 + >> drivers/net/null/meson.build | 1 + >> drivers/net/pcap/meson.build | 1 + >> drivers/net/ring/meson.build | 1 + >> drivers/net/tap/meson.build | 1 + >> drivers/raw/cnxk_bphy/meson.build | 1 + >> drivers/raw/cnxk_gpio/meson.build | 1 + >> drivers/raw/skeleton/meson.build | 1 + >> lib/eal/linux/eal.c | 7 +++ >> lib/mbuf/rte_mbuf.c | 8 ++-- >> lib/mbuf/rte_mbuf.h | 17 +++++--- >> lib/mbuf/rte_mbuf_core.h | 55 ++++++++++++++++++------ >> lib/mbuf/rte_mbuf_dyn.c | 2 + >> lib/meson.build | 3 ++ >> lib/vhost/vhost.h | 2 +- >> lib/vhost/vhost_crypto.c | 54 +++++++++++++++++------ >> meson_options.txt | 2 + >> 53 files changed, 220 insertions(+), 150 deletions(-) >> >> -- >> 2.25.1 >>
21/09/2022 15:56, Shijith Thotton: > This is a continuation of the discussions[1] to add mbuf physical address field to dynamic field. > Previous version was to add PA field to dynamic field area based on the EAL IOVA mode option. It was > deemed unsafe as some components could still use the PA field without checking IOVA mode and there > are drivers which need PA to work. One suggestion was to make the IOVA mode check at compile time so > that drivers which need PA can be disabled during build. This series adds this new meson build > options. Second patch adds mbuf PA field to dynamic field on such builds. Last two patches enable > Marvell cnxk PMDs and software PMDs in IOVA as VA build as they work without PA field. Shijith, in case it was not clear, we can accept this change only in -rc1 closing today, and we didn't receive the expected v4 yet.
>> This is a continuation of the discussions[1] to add mbuf physical address field to >dynamic field. >> Previous version was to add PA field to dynamic field area based on the EAL >IOVA mode option. It was >> deemed unsafe as some components could still use the PA field without >checking IOVA mode and there >> are drivers which need PA to work. One suggestion was to make the IOVA mode >check at compile time so >> that drivers which need PA can be disabled during build. This series adds this >new meson build >> options. Second patch adds mbuf PA field to dynamic field on such builds. Last >two patches enable >> Marvell cnxk PMDs and software PMDs in IOVA as VA build as they work without >PA field. > >Shijith, in case it was not clear, >we can accept this change only in -rc1 closing today, >and we didn't receive the expected v4 yet. > Hi Thomas, Sorry for the delay, I was not aware of this deadline. I have posted v4 now. https://patchwork.dpdk.org/project/dpdk/list/?series=25039