From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EE615A0C41; Wed, 17 Nov 2021 15:02:52 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BC951410E4; Wed, 17 Nov 2021 15:02:52 +0100 (CET) Received: from mail-il1-f176.google.com (mail-il1-f176.google.com [209.85.166.176]) by mails.dpdk.org (Postfix) with ESMTP id A603540687 for ; Wed, 17 Nov 2021 15:02:50 +0100 (CET) Received: by mail-il1-f176.google.com with SMTP id i9so2846532ilu.8 for ; Wed, 17 Nov 2021 06:02:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=QTwM9JtmJ4gPIdRJdjGoZEtOCJL/WGoP3tHvM0BLCfg=; b=p08AXzXhBDQs4/H8nPli2P/qyIvCEr1KHdf93z4BNNjIT8dPnmlceGJDfPrRVui2iz x3879cRU1hKadNZrcv0R48J6lJVjE43zdVuuJT5+0fL/XlxhHCWbjbzUW2hQSSDHnwy/ Nz+ejI9PWV4IWJvyX4Qi7zzNuoqqSCX4Spk76AniBBSI58na3TyF234IJXgEc7j2MNwH E1vw5HmXNDMYyyQyhxvfK3DY6X8xll1oNNGiJkQPiBLdbJx5iXf7PSNQth4K4VNYsm4S fRP3K3B5nz1BrWnfGRFgXjtqrRTJKFUVY2rxEvowkfXbyjjOMbcGGLbgml49ETxwpTWU B1xA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=QTwM9JtmJ4gPIdRJdjGoZEtOCJL/WGoP3tHvM0BLCfg=; b=zTRb974StRZwj/ljCYvRjQRNtH/YZy15e6qXjcQeGrzJg7clr/uC2mEYMIxO4WEyQI QAl0J7b3OqZJCaRMHZMfgh/cqIrFgs2gHMD2KHXc/3du7Az5fc5idH92Rt1VTbjCv4xr 8/S3tVt+ON0lqzycbAIt5Xzxw0j3FHl2mDORbLh8pLAcLbrmxKvHFu4H8ty4W6IXqab1 Pakq8+Ns8HUT1eUjUS6+qUbE3rWyQY4cF1e3Bvwa+Jq3wpPrm4xro6J9DCIV37upOQv3 rNjKaeV+Qc99AgXnak4L7OTr/3sv8lczFscbZdf7yGQJY3B7LlYaXVmhvM2VrpeboC2/ w+Iw== X-Gm-Message-State: AOAM531qgBcw/7yZ0hd0bPbJwNtALu9z5qo2CQjwZaiubDNXyd/xM82/ XajcD3X/Zg59S6P9pZCMLi0p+2IYKXVtMKSzL6I= X-Google-Smtp-Source: ABdhPJyXUKUbGRfujJMSgj0EcBokK87JtCK6ei63LlVJWwAiYyND8iCwBxOQLWHujTl47jmG86CJ+fXLD2ssyqnISTs= X-Received: by 2002:a05:6e02:1bcb:: with SMTP id x11mr9609808ilv.94.1637157769939; Wed, 17 Nov 2021 06:02:49 -0800 (PST) MIME-Version: 1.0 References: <20211029204909.21318-1-eagostini@nvidia.com> <20211117030459.8274-1-eagostini@nvidia.com> <20211117030459.8274-2-eagostini@nvidia.com> <20211116133449.7b7d21d1@hermes.local> <20243569-b7f0-53c5-02c5-ba29734e30c2@intel.com> In-Reply-To: From: Jerin Jacob Date: Wed, 17 Nov 2021 19:32:23 +0530 Message-ID: Subject: Re: [PATCH v3 1/1] app/testpmd: add GPU memory option for mbuf pools To: Elena Agostini Cc: "Ananyev, Konstantin" , "Yigit, Ferruh" , "Richardson, Bruce" , Stephen Hemminger , "dev@dpdk.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org te On Wed, Nov 17, 2021 at 7:20 PM Elena Agostini wrote= : > > > On Wed, Nov 17, 2021 at 6:09 PM Elena Agostini w= rote: > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>> On Wed, 17 Nov 2021 03:04:59 +0000 > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >>>>>> This patch introduces GPU memory in testpmd through the gp= udev library. > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> Testpmd can be used for network benchmarks when using GPU = memory > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> instead of regular CPU memory to send and receive packets. > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> This option is currently limited to iofwd engine to ensure > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> no workload is applied on packets not accessible from the = CPU. > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> The options chose is --mbuf-size so buffer split feature a= cross > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> different mempools can be enabled. > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>>> Signed-off-by: Elena Agostini > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>> > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>> Won't this create a hard dependency of test-pmd on gpudev? > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>>> I thought gpudev was supposed to be optional > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>> > > > > > > > > > > >> > > > > > > > > > > >>>> Sure, let me submit another patch to make it optional > > > > > > > > > > >> > > > > > > > > > > >>> > > > > > > > > > > >> > > > > > > > > > > >>> Why to add yet another compile time macro everywhere in testp= md and > > > > > > > > > > >> > > > > > > > > > > >>> make hard to maintain? > > > > > > > > > > >> > > > > > > > > > > >>> Adding iofwd kind of code is very simple to add test/test-gpu= dev and > > > > > > > > > > >> > > > > > > > > > > >>> all GPU specific options > > > > > > > > > > >> > > > > > > > > > > >>> can be added in test-gpudev. It also helps to review the patc= hes as > > > > > > > > > > >> > > > > > > > > > > >>> test cases focus on > > > > > > > > > > >> > > > > > > > > > > >>> each device class. > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> Test-gpudev is standalone unit test to ensure gpudev functions= work correctly. > > > > > > > > > > >> > > > > > > > > > > >> In testpmd instead, there is a connection between gpudev and t= he network. > > > > > > > > > > > > > > > > > > > > > > I understand that. We had the same case with eventdev, where it= needs to > > > > > > > > > > > work with network. Testpmd is already complicated, IMO, we shou= ld > > > > > > > > > > > focus only ethdev > > > > > > > > > > > test cases on testpmd, test-gpudev can use ethdev API to enable > > > > > > > > > > > networking requirements for gpudev. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +1 > > > > > > > > > > > > > > > > > > +1 > > > > > > > > > > > > > > > > Testpmd already manages different type of memories for mempools. > > > > > > > > gpudev is just another type of memory, there is nothing more than tha= t. > > > > > > Let take this example: > > > 1) New code changes > > > > > > app/test-pmd/cmdline.c | 32 +++++++- > > > app/test-pmd/config.c | 4 +- > > > app/test-pmd/icmpecho.c | 2 +- > > > app/test-pmd/meson.build | 2 +- > > > app/test-pmd/parameters.c | 15 +++- > > > app/test-pmd/testpmd.c | 167 +++++++++++++++++++++++++++++++++++--- > > > app/test-pmd/testpmd.h | 16 +++- > > > 7 files changed, 217 insertions(+), 21 deletions(-) > > > > > > 2) Good amount of code need to go through condition compilation as > > > gpudev is optional that make > > > testpmd further ugly. > > > > > > 3) It introduces new memtype, now > > > > > > +enum mbuf_mem_type { > > > + MBUF_MEM_CPU, > > > + MBUF_MEM_GPU > > > +}; > > > > > > The question largely, why testpmd need to pollute for this, testpmd, > > > we are using for testing ethdev device class. > > > All we are saying is to enable this use case in test-gpudev so that it > > > focuses on GPU specific, Whoever is not > > > interested in specific libraries do not even need to review the testpmd= patches. > > > > I understand your point. I don=E2=80=99t understand why this testpmd patc= h is there since Oct 29 but > > I'm receiving reviews only few days before rc4 when I have a limited amou= nt of time to get new code accepted. I understand that pain. Welcome to DPDK, we have all gone through this review issue one or another way. > > > > I can provide a gpudev + ethdev example by end of today (I'd like to keep= test-gpudev as it is to test gpudev API standalone). > > Is there any chance this new example will be reviewed and eventually acce= pted in DPDK 21.11? Why a new example? I don't have any issues in updating app/test-gpudev/.