<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=Windows-1252"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="en-IT" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <div class="WordSection1"> <div> > On Wed, Nov 17, 2021 at 7:20 PM Elena Agostini <eagostini@nvidia.com> wrote:<o:p></o:p> > ><o:p></o:p> > > > On Wed, Nov 17, 2021 at 6:09 PM Elena Agostini <eagostini@nvidia.com> wrote:<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>> On Wed, 17 Nov 2021 03:04:59 +0000<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>> This patch introduces GPU memory in testpmd through the gpudev library.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>> Testpmd can be used for network benchmarks when using GPU memory<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>> instead of regular CPU memory to send and receive packets.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>> This option is currently limited to iofwd engine to ensure<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>> no workload is applied on packets not accessible from the CPU.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>> The options chose is --mbuf-size so buffer split feature across<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>> different mempools can be enabled.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>> Won't this create a hard dependency of test-pmd on gpudev?<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>>> I thought gpudev was supposed to be optional<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>>> Sure, let me submit another patch to make it optional<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>> Why to add yet another compile time macro everywhere in testpmd and<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>> make hard to maintain?<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>> Adding iofwd kind of code is very simple to add test/test-gpudev and<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>> all GPU specific options<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>> can be added in test-gpudev. It also helps to review the patches as<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>> test cases focus on<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >>> each device class.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >> Test-gpudev is standalone unit test to ensure gpudev functions work correctly.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > >> In testpmd instead, there is a connection between gpudev and the network.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > ><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > > I understand that. We had the same case with eventdev, where it needs to<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > > work with network. Testpmd is already complicated, IMO, we should<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > > focus only ethdev<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > > test cases on testpmd, test-gpudev can use ethdev API to enable<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > > networking requirements for gpudev.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > ><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > ><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > > +1<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > ><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > > +1<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > Testpmd already manages different type of memories for mempools.<o:p></o:p> > ><o:p></o:p> > > > ><o:p></o:p> > ><o:p></o:p> > > > > gpudev is just another type of memory, there is nothing more than that.<o:p></o:p> > ><o:p></o:p> > > ><o:p></o:p> > ><o:p></o:p> > > > Let take this example:<o:p></o:p> > ><o:p></o:p> > > > 1) New code changes<o:p></o:p> > ><o:p></o:p> > > ><o:p></o:p> > ><o:p></o:p> > > > app/test-pmd/cmdline.c | 32 +++++++-<o:p></o:p> > ><o:p></o:p> > > > app/test-pmd/config.c | 4 +-<o:p></o:p> > ><o:p></o:p> > > > app/test-pmd/icmpecho.c | 2 +-<o:p></o:p> > ><o:p></o:p> > > > app/test-pmd/meson.build | 2 +-<o:p></o:p> > ><o:p></o:p> > > > app/test-pmd/parameters.c | 15 +++-<o:p></o:p> > ><o:p></o:p> > > > app/test-pmd/testpmd.c | 167 +++++++++++++++++++++++++++++++++++---<o:p></o:p> > ><o:p></o:p> > > > app/test-pmd/testpmd.h | 16 +++-<o:p></o:p> > ><o:p></o:p> > > > 7 files changed, 217 insertions(+), 21 deletions(-)<o:p></o:p> > ><o:p></o:p> > > ><o:p></o:p> > ><o:p></o:p> > > > 2) Good amount of code need to go through condition compilation as<o:p></o:p> > ><o:p></o:p> > > > gpudev is optional that make<o:p></o:p> > ><o:p></o:p> > > > testpmd further ugly.<o:p></o:p> > ><o:p></o:p> > > ><o:p></o:p> > ><o:p></o:p> > > > 3) It introduces new memtype, now<o:p></o:p> > ><o:p></o:p> > > ><o:p></o:p> > ><o:p></o:p> > > > +enum mbuf_mem_type {<o:p></o:p> > ><o:p></o:p> > > > + MBUF_MEM_CPU,<o:p></o:p> > ><o:p></o:p> > > > + MBUF_MEM_GPU<o:p></o:p> > ><o:p></o:p> > > > +};<o:p></o:p> > ><o:p></o:p> > > ><o:p></o:p> > ><o:p></o:p> > > > The question largely, why testpmd need to pollute for this, testpmd,<o:p></o:p> > ><o:p></o:p> > > > we are using for testing ethdev device class.<o:p></o:p> > ><o:p></o:p> > > > All we are saying is to enable this use case in test-gpudev so that it<o:p></o:p> > ><o:p></o:p> > > > focuses on GPU specific, Whoever is not<o:p></o:p> > ><o:p></o:p> > > > interested in specific libraries do not even need to review the testpmd patches.<o:p></o:p> > ><o:p></o:p> > ><o:p></o:p> > ><o:p></o:p> > > I understand your point. I don�t understand why this testpmd patch is there since Oct 29 but<o:p></o:p> > ><o:p></o:p> > > I'm receiving reviews only few days before rc4 when I have a limited amount of time to get new code accepted.> <o:p></o:p> <o:p> </o:p> > I understand that pain. Welcome to DPDK, we have all gone through this<o:p></o:p> > review issue one or another way.> <o:p></o:p> > <o:p></o:p> <o:p> </o:p> > ><o:p></o:p> > ><o:p></o:p> > ><o:p></o:p> > > 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).<o:p></o:p> > ><o:p></o:p> > > Is there any chance this new example will be reviewed and eventually accepted in DPDK 21.11?> <o:p></o:p> <o:p> </o:p> > Why a new example? I don't have any issues in updating app/test-gpudev/.<o:p></o:p> <o:p> </o:p> Understood. If this can also help to speed up the acceptance process, I�ll provide a new patch for app/test-gpudev to introduce ethdev to send and receive packets<o:p></o:p> </div> </div> </body> </html>