[v3,1/1] app/testpmd: add GPU memory option for mbuf pools
Checks
Commit Message
From: Elena Agostini <eagostini@nvidia.com>
This patch introduces GPU memory in testpmd through the gpudev 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 across
different mempools can be enabled.
Signed-off-by: Elena Agostini <eagostini@nvidia.com>
---
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(-)
Comments
On Wed, 17 Nov 2021 03:04:59 +0000
<eagostini@nvidia.com> wrote:
> From: Elena Agostini <eagostini@nvidia.com>
>
> This patch introduces GPU memory in testpmd through the gpudev 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 across
> different mempools can be enabled.
>
> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
Won't this create a hard dependency of test-pmd on gpudev?
I thought gpudev was supposed to be optional
> External email: Use caution opening links or attachments
>
>
> On Wed, 17 Nov 2021 03:04:59 +0000
> <eagostini@nvidia.com> wrote:
>
> > From: Elena Agostini <eagostini@nvidia.com>
> >
> > This patch introduces GPU memory in testpmd through the gpudev 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 across
> > different mempools can be enabled.
> >
> > Signed-off-by: Elena Agostini <eagostini@nvidia.com>
>
> 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
On Wed, Nov 17, 2021 at 4:38 PM Elena Agostini <eagostini@nvidia.com> wrote:
>
> > External email: Use caution opening links or attachments
>
> >
>
> >
>
> > On Wed, 17 Nov 2021 03:04:59 +0000
>
> > <eagostini@nvidia.com> wrote:
>
> >
>
> > > From: Elena Agostini <eagostini@nvidia.com>
>
> > >
>
> > > This patch introduces GPU memory in testpmd through the gpudev 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 across
>
> > > different mempools can be enabled.
>
> > >
>
> > > Signed-off-by: Elena Agostini <eagostini@nvidia.com>
>
> >
>
> > 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 testpmd and
make hard to maintain?
Adding iofwd kind of code is very simple to add test/test-gpudev and
all GPU specific options
can be added in test-gpudev. It also helps to review the patches as
test cases focus on
each device class.
> On Wed, Nov 17, 2021 at 4:38 PM Elena Agostini <eagostini@nvidia.com> wrote:
> >
> > > External email: Use caution opening links or attachments
> >
> > >
> >
> > >
> >
> > > On Wed, 17 Nov 2021 03:04:59 +0000
> >
> > > <eagostini@nvidia.com> wrote:
> >
> > >
> >
> > > > From: Elena Agostini <eagostini@nvidia.com>
> >
> > > >
> >
> > > > This patch introduces GPU memory in testpmd through the gpudev 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 across
> >
> > > > different mempools can be enabled.
> >
> > > >
> >
> > > > Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> >
> > >
> >
> > > 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 testpmd and
> make hard to maintain?
> Adding iofwd kind of code is very simple to add test/test-gpudev and
> all GPU specific options
> can be added in test-gpudev. It also helps to review the patches 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 the network.
On Wed, Nov 17, 2021 at 4:56 PM Elena Agostini <eagostini@nvidia.com> wrote:
>
> > On Wed, Nov 17, 2021 at 4:38 PM Elena Agostini <eagostini@nvidia.com> wrote:
>
> > >
>
> > > > External email: Use caution opening links or attachments
>
> > >
>
> > > >
>
> > >
>
> > > >
>
> > >
>
> > > > On Wed, 17 Nov 2021 03:04:59 +0000
>
> > >
>
> > > > <eagostini@nvidia.com> wrote:
>
> > >
>
> > > >
>
> > >
>
> > > > > From: Elena Agostini <eagostini@nvidia.com>
>
> > >
>
> > > > >
>
> > >
>
> > > > > This patch introduces GPU memory in testpmd through the gpudev 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 across
>
> > >
>
> > > > > different mempools can be enabled.
>
> > >
>
> > > > >
>
> > >
>
> > > > > Signed-off-by: Elena Agostini <eagostini@nvidia.com>
>
> > >
>
> > > >
>
> > >
>
> > > > 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 testpmd and
>
> > make hard to maintain?
>
> > Adding iofwd kind of code is very simple to add test/test-gpudev and
>
> > all GPU specific options
>
> > can be added in test-gpudev. It also helps to review the patches 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 the network.
I understand that. We had the same case with eventdev, where it needs to
work with network. Testpmd is already complicated, IMO, we should
focus only ethdev
test cases on testpmd, test-gpudev can use ethdev API to enable
networking requirements for gpudev.
On 11/17/2021 11:31 AM, Jerin Jacob wrote:
> On Wed, Nov 17, 2021 at 4:56 PM Elena Agostini <eagostini@nvidia.com> wrote:
>>
>>> On Wed, Nov 17, 2021 at 4:38 PM Elena Agostini <eagostini@nvidia.com> wrote:
>>
>>>>
>>
>>>>> External email: Use caution opening links or attachments
>>
>>>>
>>
>>>>>
>>
>>>>
>>
>>>>>
>>
>>>>
>>
>>>>> On Wed, 17 Nov 2021 03:04:59 +0000
>>
>>>>
>>
>>>>> <eagostini@nvidia.com> wrote:
>>
>>>>
>>
>>>>>
>>
>>>>
>>
>>>>>> From: Elena Agostini <eagostini@nvidia.com>
>>
>>>>
>>
>>>>>>
>>
>>>>
>>
>>>>>> This patch introduces GPU memory in testpmd through the gpudev 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 across
>>
>>>>
>>
>>>>>> different mempools can be enabled.
>>
>>>>
>>
>>>>>>
>>
>>>>
>>
>>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
>>
>>>>
>>
>>>>>
>>
>>>>
>>
>>>>> 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 testpmd and
>>
>>> make hard to maintain?
>>
>>> Adding iofwd kind of code is very simple to add test/test-gpudev and
>>
>>> all GPU specific options
>>
>>> can be added in test-gpudev. It also helps to review the patches 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 the network.
>
> I understand that. We had the same case with eventdev, where it needs to
> work with network. Testpmd is already complicated, IMO, we should
> focus only ethdev
> test cases on testpmd, test-gpudev can use ethdev API to enable
> networking requirements for gpudev.
>
+1
> >>
> >>>>
> >>
> >>>>> On Wed, 17 Nov 2021 03:04:59 +0000
> >>
> >>>>
> >>>>>> This patch introduces GPU memory in testpmd through the gpudev 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 across
> >>
> >>>>
> >>
> >>>>>> different mempools can be enabled.
> >>
> >>>>
> >>
> >>>>>>
> >>
> >>>>
> >>
> >>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> >>
> >>>>
> >>
> >>>>>
> >>
> >>>>
> >>
> >>>>> 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 testpmd and
> >>
> >>> make hard to maintain?
> >>
> >>> Adding iofwd kind of code is very simple to add test/test-gpudev and
> >>
> >>> all GPU specific options
> >>
> >>> can be added in test-gpudev. It also helps to review the patches 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 the network.
> >
> > I understand that. We had the same case with eventdev, where it needs to
> > work with network. Testpmd is already complicated, IMO, we should
> > focus only ethdev
> > test cases on testpmd, test-gpudev can use ethdev API to enable
> > networking requirements for gpudev.
> >
>
> +1
+1
> > >>
> > >>>>
> > >>
> > >>>>> On Wed, 17 Nov 2021 03:04:59 +0000
> > >>
> > >>>>
> > >>>>>> This patch introduces GPU memory in testpmd through the gpudev 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 across
> > >>
> > >>>>
> > >>
> > >>>>>> different mempools can be enabled.
> > >>
> > >>>>
> > >>
> > >>>>>>
> > >>
> > >>>>
> > >>
> > >>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> > >>
> > >>>>
> > >>
> > >>>>>
> > >>
> > >>>>
> > >>
> > >>>>> 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 testpmd and
> > >>
> > >>> make hard to maintain?
> > >>
> > >>> Adding iofwd kind of code is very simple to add test/test-gpudev and
> > >>
> > >>> all GPU specific options
> > >>
> > >>> can be added in test-gpudev. It also helps to review the patches 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 the network.
> > >
> > > I understand that. We had the same case with eventdev, where it needs to
> > > work with network. Testpmd is already complicated, IMO, we should
> > > 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 that.
On Wed, Nov 17, 2021 at 6:09 PM Elena Agostini <eagostini@nvidia.com> wrote:
>
> > > >>
>
> > > >>>>
>
> > > >>
>
> > > >>>>> On Wed, 17 Nov 2021 03:04:59 +0000
>
> > > >>
>
> > > >>>>
>
> > > >>>>>> This patch introduces GPU memory in testpmd through the gpudev 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 across
>
> > > >>
>
> > > >>>>
>
> > > >>
>
> > > >>>>>> different mempools can be enabled.
>
> > > >>
>
> > > >>>>
>
> > > >>
>
> > > >>>>>>
>
> > > >>
>
> > > >>>>
>
> > > >>
>
> > > >>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
>
> > > >>
>
> > > >>>>
>
> > > >>
>
> > > >>>>>
>
> > > >>
>
> > > >>>>
>
> > > >>
>
> > > >>>>> 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 testpmd and
>
> > > >>
>
> > > >>> make hard to maintain?
>
> > > >>
>
> > > >>> Adding iofwd kind of code is very simple to add test/test-gpudev and
>
> > > >>
>
> > > >>> all GPU specific options
>
> > > >>
>
> > > >>> can be added in test-gpudev. It also helps to review the patches 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 the network.
>
> > > >
>
> > > > I understand that. We had the same case with eventdev, where it needs to
>
> > > > work with network. Testpmd is already complicated, IMO, we should
>
> > > > 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 that.
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.
> On Wed, Nov 17, 2021 at 6:09 PM Elena Agostini <eagostini@nvidia.com> wrote:
> >
> > > > >>
> >
> > > > >>>>
> >
> > > > >>
> >
> > > > >>>>> On Wed, 17 Nov 2021 03:04:59 +0000
> >
> > > > >>
> >
> > > > >>>>
> >
> > > > >>>>>> This patch introduces GPU memory in testpmd through the gpudev 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 across
> >
> > > > >>
> >
> > > > >>>>
> >
> > > > >>
> >
> > > > >>>>>> different mempools can be enabled.
> >
> > > > >>
> >
> > > > >>>>
> >
> > > > >>
> >
> > > > >>>>>>
> >
> > > > >>
> >
> > > > >>>>
> >
> > > > >>
> >
> > > > >>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> >
> > > > >>
> >
> > > > >>>>
> >
> > > > >>
> >
> > > > >>>>>
> >
> > > > >>
> >
> > > > >>>>
> >
> > > > >>
> >
> > > > >>>>> 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 testpmd and
> >
> > > > >>
> >
> > > > >>> make hard to maintain?
> >
> > > > >>
> >
> > > > >>> Adding iofwd kind of code is very simple to add test/test-gpudev and
> >
> > > > >>
> >
> > > > >>> all GPU specific options
> >
> > > > >>
> >
> > > > >>> can be added in test-gpudev. It also helps to review the patches 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 the network.
> >
> > > > >
> >
> > > > > I understand that. We had the same case with eventdev, where it needs to
> >
> > > > > work with network. Testpmd is already complicated, IMO, we should
> >
> > > > > 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 that.
>
> 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’t understand why this testpmd patch is there since Oct 29 but
I'm receiving reviews only few days before rc4 when I have a limited amount of time to get new code accepted.
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 accepted in DPDK 21.11?
te
On Wed, Nov 17, 2021 at 7:20 PM Elena Agostini <eagostini@nvidia.com> wrote:
>
> > On Wed, Nov 17, 2021 at 6:09 PM Elena Agostini <eagostini@nvidia.com> wrote:
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>> On Wed, 17 Nov 2021 03:04:59 +0000
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>
>
> > >
>
> > > > > >>>>>> This patch introduces GPU memory in testpmd through the gpudev 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 across
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>>> different mempools can be enabled.
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>>>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>>>> 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 testpmd and
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>> make hard to maintain?
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>> Adding iofwd kind of code is very simple to add test/test-gpudev and
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>> all GPU specific options
>
> > >
>
> > > > > >>
>
> > >
>
> > > > > >>> can be added in test-gpudev. It also helps to review the patches 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 the network.
>
> > >
>
> > > > > >
>
> > >
>
> > > > > > I understand that. We had the same case with eventdev, where it needs to
>
> > >
>
> > > > > > work with network. Testpmd is already complicated, IMO, we should
>
> > >
>
> > > > > > 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 that.
>
> >
>
> > 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’t understand why this testpmd patch is there since Oct 29 but
>
> I'm receiving reviews only few days before rc4 when I have a limited amount 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 accepted in DPDK 21.11?
Why a new example? I don't have any issues in updating app/test-gpudev/.
> On Wed, Nov 17, 2021 at 7:20 PM Elena Agostini <eagostini@nvidia.com> wrote:
> >
> > > On Wed, Nov 17, 2021 at 6:09 PM Elena Agostini <eagostini@nvidia.com> wrote:
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>> On Wed, 17 Nov 2021 03:04:59 +0000
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>
> >
> > > >
> >
> > > > > > >>>>>> This patch introduces GPU memory in testpmd through the gpudev 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 across
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>>> different mempools can be enabled.
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>>>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>>>> 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 testpmd and
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>> make hard to maintain?
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>> Adding iofwd kind of code is very simple to add test/test-gpudev and
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>> all GPU specific options
> >
> > > >
> >
> > > > > > >>
> >
> > > >
> >
> > > > > > >>> can be added in test-gpudev. It also helps to review the patches 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 the network.
> >
> > > >
> >
> > > > > > >
> >
> > > >
> >
> > > > > > > I understand that. We had the same case with eventdev, where it needs to
> >
> > > >
> >
> > > > > > > work with network. Testpmd is already complicated, IMO, we should
> >
> > > >
> >
> > > > > > > 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 that.
> >
> > >
> >
> > > 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’t understand why this testpmd patch is there since Oct 29 but
> >
> > I'm receiving reviews only few days before rc4 when I have a limited amount 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 accepted in DPDK 21.11?>
> Why a new example? I don't have any issues in updating app/test-gpudev/.
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
> > On Wed, Nov 17, 2021 at 7:20 PM Elena Agostini <eagostini@nvidia.com> wrote:
> > >
> > > > On Wed, Nov 17, 2021 at 6:09 PM Elena Agostini <eagostini@nvidia.com> wrote:
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>> On Wed, 17 Nov 2021 03:04:59 +0000
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>
> > >
> > > > >
> > >
> > > > > > > >>>>>> This patch introduces GPU memory in testpmd through the gpudev 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 across
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>>> different mempools can be enabled.
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>>>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>>> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>>>> 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 testpmd and
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>> make hard to maintain?
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>> Adding iofwd kind of code is very simple to add test/test-gpudev and
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>> all GPU specific options
> > >
> > > > >
> > >
> > > > > > > >>
> > >
> > > > >
> > >
> > > > > > > >>> can be added in test-gpudev. It also helps to review the patches 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 the network.
> > >
> > > > >
> > >
> > > > > > > >
> > >
> > > > >
> > >
> > > > > > > > I understand that. We had the same case with eventdev, where it needs to
> > >
> > > > >
> > >
> > > > > > > > work with network. Testpmd is already complicated, IMO, we should
> > >
> > > > >
> > >
> > > > > > > > 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 that.
> > >
> > > >
> > >
> > > > 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’t understand why this testpmd patch is there since Oct 29 but
> > >
> > I'm receiving reviews only few days before rc4 when I have a limited amount 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 accepted in DPDK 21.11?>
>
> > Why a new example? I don't have any issues in updating app/test-gpudev/.
>
> 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
This is the patch to introduce ethdev in app/test-gpudev as alternative to testpmd patch, please give me your feedback.
https://patches.dpdk.org/project/dpdk/patch/20211118015228.30628-2-eagostini@nvidia.com
It would be really helpful for the gpudev library to have a concrete example to send and receive packets using GPU memory
@@ -3614,6 +3614,7 @@ parse_item_list(const char *str, const char *item_name, unsigned int max_items,
unsigned int j;
int value_ok;
char c;
+ int gpu_mbuf = 0;
/*
* First parse all items in the list and store their value.
@@ -3623,11 +3624,25 @@ parse_item_list(const char *str, const char *item_name, unsigned int max_items,
value_ok = 0;
for (i = 0; i < strnlen(str, STR_TOKEN_SIZE); i++) {
c = str[i];
+
if ((c >= '0') && (c <= '9')) {
value = (unsigned int) (value * 10 + (c - '0'));
value_ok = 1;
continue;
}
+ if (c == 'g') {
+ /*
+ * When this flag is set, mbufs for this segment
+ * will be created on GPU memory.
+ */
+ if (i < strnlen(str, STR_TOKEN_SIZE) - 1 && str[i+1] != ',') {
+ fprintf(stderr, "input param list is not well formatted\n");
+ return 0;
+ }
+
+ gpu_mbuf = 1;
+ continue;
+ }
if (c != ',') {
fprintf(stderr, "character %c is not a decimal digit\n", c);
return 0;
@@ -3640,6 +3655,8 @@ parse_item_list(const char *str, const char *item_name, unsigned int max_items,
parsed_items[nb_item] = value;
value_ok = 0;
value = 0;
+ mbuf_mem_types[nb_item] = gpu_mbuf ? MBUF_MEM_GPU : MBUF_MEM_CPU;
+ gpu_mbuf = 0;
}
nb_item++;
}
@@ -3648,12 +3665,15 @@ parse_item_list(const char *str, const char *item_name, unsigned int max_items,
item_name, nb_item + 1, max_items);
return 0;
}
+
+ mbuf_mem_types[nb_item] = gpu_mbuf ? MBUF_MEM_GPU : MBUF_MEM_CPU;
+
parsed_items[nb_item++] = value;
if (! check_unique_values)
return nb_item;
/*
- * Then, check that all values in the list are different.
+ * Then, check that all values in the list are differents.
* No optimization here...
*/
for (i = 0; i < nb_item; i++) {
@@ -6865,6 +6885,16 @@ static void cmd_set_fwd_mode_parsed(void *parsed_result,
__rte_unused void *data)
{
struct cmd_set_fwd_mode_result *res = parsed_result;
+ int idx;
+
+ for (idx = 0; idx < MAX_SEGS_BUFFER_SPLIT; idx++) {
+ if (mbuf_mem_types[idx] == MBUF_MEM_GPU &&
+ strcmp(res->mode, "io") != 0) {
+ TESTPMD_LOG(ERR,
+ "GPU memory mbufs can be used with iofwd engine only\n");
+ return;
+ }
+ }
retry_enabled = 0;
set_pkt_forwarding_mode(res->mode);
@@ -2965,7 +2965,7 @@ port_rss_reta_info(portid_t port_id,
}
/*
- * Displays the RSS hash functions of a port, and, optionally, the RSS hash
+ * Displays the RSS hash functions of a port, and, optionaly, the RSS hash
* key of the port.
*/
void
@@ -5250,7 +5250,7 @@ mcast_addr_pool_remove(struct rte_port *port, uint32_t addr_idx)
{
port->mc_addr_nb--;
if (addr_idx == port->mc_addr_nb) {
- /* No need to recompact the set of multicast addresses. */
+ /* No need to recompact the set of multicast addressses. */
if (port->mc_addr_nb == 0) {
/* free the pool of multicast addresses. */
free(port->mc_addr_pool);
@@ -54,7 +54,7 @@ arp_op_name(uint16_t arp_op)
default:
break;
}
- return "Unknown ARP op";
+ return "Unkwown ARP op";
}
static const char *
@@ -32,7 +32,7 @@ if dpdk_conf.has('RTE_HAS_JANSSON')
ext_deps += jansson_dep
endif
-deps += ['ethdev', 'gro', 'gso', 'cmdline', 'metrics', 'bus_pci']
+deps += ['ethdev', 'gro', 'gso', 'cmdline', 'metrics', 'bus_pci', 'gpudev']
if dpdk_conf.has('RTE_CRYPTO_SCHEDULER')
deps += 'crypto_scheduler'
endif
@@ -87,7 +87,10 @@ usage(char* progname)
"in NUMA mode.\n");
printf(" --mbuf-size=N,[N1[,..Nn]: set the data size of mbuf to "
"N bytes. If multiple numbers are specified the extra pools "
- "will be created to receive with packet split features\n");
+ "will be created to receive with packet split features\n"
+ "Use 'g' suffix for GPU memory.\n"
+ "If no or an unrecognized suffix is provided, CPU is assumed\n");
+
printf(" --total-num-mbufs=N: set the number of mbufs to be allocated "
"in mbuf pools.\n");
printf(" --max-pkt-len=N: set the maximum size of packet to N bytes.\n");
@@ -595,6 +598,7 @@ launch_args_parse(int argc, char** argv)
struct rte_eth_dev_info dev_info;
uint16_t rec_nb_pkts;
int ret;
+ uint32_t idx = 0;
static struct option lgopts[] = {
{ "help", 0, 0, 0 },
@@ -1544,4 +1548,13 @@ launch_args_parse(int argc, char** argv)
"ignored\n");
mempool_flags = 0;
}
+
+ for (idx = 0; idx < mbuf_data_size_n; idx++) {
+ if (mbuf_mem_types[idx] == MBUF_MEM_GPU &&
+ strcmp(cur_fwd_eng->fwd_mode_name, "io") != 0) {
+ TESTPMD_LOG(ERR,
+ "GPU memory mbufs can be used with iofwd engine only\n");
+ rte_exit(EXIT_FAILURE, "Command line is incorrect\n");
+ }
+ }
}
@@ -205,6 +205,13 @@ uint32_t mbuf_data_size_n = 1; /* Number of specified mbuf sizes. */
uint16_t mbuf_data_size[MAX_SEGS_BUFFER_SPLIT] = {
DEFAULT_MBUF_DATA_SIZE
}; /**< Mbuf data space size. */
+
+/* Mbuf memory types. */
+enum mbuf_mem_type mbuf_mem_types[MAX_SEGS_BUFFER_SPLIT];
+/* Pointers to external memory allocated for mempools. */
+uintptr_t mempools_ext_ptr[MAX_SEGS_BUFFER_SPLIT];
+size_t mempools_ext_size[MAX_SEGS_BUFFER_SPLIT];
+
uint32_t param_total_num_mbufs = 0; /**< number of mbufs in all pools - if
* specified on command-line. */
uint16_t stats_period; /**< Period to show statistics (disabled by default) */
@@ -543,6 +550,12 @@ int proc_id;
*/
unsigned int num_procs = 1;
+/*
+ * In case of GPU memory external mbufs use, for simplicity,
+ * the first GPU device in the list.
+ */
+int gpu_id = 0;
+
static void
eth_rx_metadata_negotiate_mp(uint16_t port_id)
{
@@ -1103,6 +1116,81 @@ setup_extbuf(uint32_t nb_mbufs, uint16_t mbuf_sz, unsigned int socket_id,
return ext_num;
}
+static struct rte_mempool *
+gpu_mbuf_pool_create(uint16_t mbuf_seg_size, unsigned int nb_mbuf,
+ unsigned int socket_id, uint16_t port_id,
+ int gpu_id, uintptr_t *mp_addr, size_t *mp_size)
+{
+ int ret = 0;
+ char pool_name[RTE_MEMPOOL_NAMESIZE];
+ struct rte_eth_dev_info dev_info;
+ struct rte_mempool *rte_mp = NULL;
+ struct rte_pktmbuf_extmem gpu_mem;
+ struct rte_gpu_info ginfo;
+ uint8_t gpu_page_shift = 16;
+ uint32_t gpu_page_size = (1UL << gpu_page_shift);
+
+ if (rte_gpu_count_avail() == 0)
+ rte_exit(EXIT_FAILURE, "No GPU device available.\n");
+
+ if (rte_gpu_info_get(gpu_id, &ginfo))
+ rte_exit(EXIT_FAILURE, "Can't retrieve info about GPU %d - bye\n", gpu_id);
+
+ ret = eth_dev_info_get_print_err(port_id, &dev_info);
+ if (ret != 0)
+ rte_exit(EXIT_FAILURE,
+ "Failed to get device info for port %d\n",
+ port_id);
+
+ mbuf_poolname_build(socket_id, pool_name, sizeof(pool_name), port_id, MBUF_MEM_GPU);
+ if (!is_proc_primary()) {
+ rte_mp = rte_mempool_lookup(pool_name);
+ if (rte_mp == NULL)
+ rte_exit(EXIT_FAILURE,
+ "Get mbuf pool for socket %u failed: %s\n",
+ socket_id, rte_strerror(rte_errno));
+ return rte_mp;
+ }
+
+ TESTPMD_LOG(INFO,
+ "create a new mbuf pool <%s>: n=%u, size=%u, socket=%u GPU device=%s\n",
+ pool_name, nb_mbuf, mbuf_seg_size, socket_id, ginfo.name);
+
+ /* Create an external memory mempool using memory allocated on the GPU. */
+
+ gpu_mem.elt_size = RTE_MBUF_DEFAULT_BUF_SIZE;
+ gpu_mem.buf_len = RTE_ALIGN_CEIL(nb_mbuf * gpu_mem.elt_size, gpu_page_size);
+ gpu_mem.buf_iova = RTE_BAD_IOVA;
+
+ gpu_mem.buf_ptr = rte_gpu_mem_alloc(gpu_id, gpu_mem.buf_len);
+ if (gpu_mem.buf_ptr == NULL)
+ rte_exit(EXIT_FAILURE, "Could not allocate GPU device memory\n");
+
+ ret = rte_extmem_register(gpu_mem.buf_ptr, gpu_mem.buf_len, NULL, gpu_mem.buf_iova, gpu_page_size);
+ if (ret)
+ rte_exit(EXIT_FAILURE, "Unable to register addr 0x%p, ret %d\n", gpu_mem.buf_ptr, ret);
+
+ uint16_t pid = 0;
+
+ RTE_ETH_FOREACH_DEV(pid)
+ {
+ ret = rte_dev_dma_map(dev_info.device, gpu_mem.buf_ptr,
+ gpu_mem.buf_iova, gpu_mem.buf_len);
+ if (ret)
+ rte_exit(EXIT_FAILURE, "Unable to DMA map addr 0x%p for device %s\n",
+ gpu_mem.buf_ptr, dev_info.device->name);
+ }
+
+ rte_mp = rte_pktmbuf_pool_create_extbuf(pool_name, nb_mbuf, mb_mempool_cache, 0, mbuf_seg_size, socket_id, &gpu_mem, 1);
+ if (rte_mp == NULL)
+ rte_exit(EXIT_FAILURE, "Creation of GPU mempool <%s> failed\n", pool_name);
+
+ *mp_addr = (uintptr_t) gpu_mem.buf_ptr;
+ *mp_size = (size_t) gpu_mem.buf_len;
+
+ return rte_mp;
+}
+
/*
* Configuration initialisation done once at init time.
*/
@@ -1117,7 +1205,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
mb_size = sizeof(struct rte_mbuf) + mbuf_seg_size;
#endif
- mbuf_poolname_build(socket_id, pool_name, sizeof(pool_name), size_idx);
+ mbuf_poolname_build(socket_id, pool_name, sizeof(pool_name), size_idx, MBUF_MEM_CPU);
if (!is_proc_primary()) {
rte_mp = rte_mempool_lookup(pool_name);
if (rte_mp == NULL)
@@ -1700,19 +1788,42 @@ init_config(void)
for (i = 0; i < num_sockets; i++)
for (j = 0; j < mbuf_data_size_n; j++)
- mempools[i * MAX_SEGS_BUFFER_SPLIT + j] =
- mbuf_pool_create(mbuf_data_size[j],
- nb_mbuf_per_pool,
- socket_ids[i], j);
+ {
+ if (mbuf_mem_types[j] == MBUF_MEM_GPU) {
+ mempools[i * MAX_SEGS_BUFFER_SPLIT + j] =
+ gpu_mbuf_pool_create(mbuf_data_size[j],
+ nb_mbuf_per_pool,
+ socket_ids[i], j, gpu_id,
+ &(mempools_ext_ptr[i]),
+ &(mempools_ext_size[i]));
+ } else {
+ mempools[i * MAX_SEGS_BUFFER_SPLIT + j] =
+ mbuf_pool_create(mbuf_data_size[j],
+ nb_mbuf_per_pool,
+ socket_ids[i], j);
+ }
+ }
} else {
uint8_t i;
for (i = 0; i < mbuf_data_size_n; i++)
- mempools[i] = mbuf_pool_create
- (mbuf_data_size[i],
- nb_mbuf_per_pool,
- socket_num == UMA_NO_CONFIG ?
- 0 : socket_num, i);
+ {
+ if (mbuf_mem_types[i] == MBUF_MEM_GPU) {
+ mempools[i] =
+ gpu_mbuf_pool_create(mbuf_data_size[i],
+ nb_mbuf_per_pool,
+ socket_num == UMA_NO_CONFIG ? 0 : socket_num,
+ i, gpu_id,
+ &(mempools_ext_ptr[i]),
+ &(mempools_ext_size[i]));
+ } else {
+ mempools[i] = mbuf_pool_create(mbuf_data_size[i],
+ nb_mbuf_per_pool,
+ socket_num == UMA_NO_CONFIG ?
+ 0 : socket_num, i);
+ }
+ }
+
}
init_port_config();
@@ -3414,9 +3525,43 @@ pmd_test_exit(void)
return;
}
}
+
for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
- if (mempools[i])
+ if (mempools[i]) {
mempool_free_mp(mempools[i]);
+ if (mbuf_mem_types[i] == MBUF_MEM_GPU) {
+ if (mempools_ext_ptr[i] != 0) {
+ ret = rte_extmem_unregister(
+ (void *)mempools_ext_ptr[i],
+ mempools_ext_size[i]);
+
+ if (ret)
+ RTE_LOG(ERR, EAL,
+ "rte_extmem_unregister 0x%p -> %d (rte_errno = %d)\n",
+ (uint8_t *)mempools_ext_ptr[i], ret, rte_errno);
+
+ RTE_ETH_FOREACH_DEV(pt_id) {
+ struct rte_eth_dev_info dev_info;
+ ret = eth_dev_info_get_print_err(pt_id, &dev_info);
+ if (ret != 0)
+ rte_exit(EXIT_FAILURE,
+ "Failed to get device info for port %d\n",
+ pt_id);
+
+ ret = rte_dev_dma_unmap(dev_info.device,
+ (void *)mempools_ext_ptr[i], RTE_BAD_IOVA,
+ mempools_ext_size[i]);
+
+ if (ret)
+ RTE_LOG(ERR, EAL,
+ "rte_dev_dma_unmap 0x%p -> %d (rte_errno = %d)\n",
+ (uint8_t *)mempools_ext_ptr[i], ret, rte_errno);
+ }
+
+ rte_gpu_mem_free(gpu_id, (void *)mempools_ext_ptr[i]);
+ }
+ }
+ }
}
free(xstats_display);
@@ -12,6 +12,7 @@
#include <rte_gro.h>
#include <rte_gso.h>
#include <rte_os_shim.h>
+#include <rte_gpudev.h>
#include <cmdline.h>
#include <sys/queue.h>
#ifdef RTE_HAS_JANSSON
@@ -474,6 +475,11 @@ extern uint8_t dcb_config;
extern uint32_t mbuf_data_size_n;
extern uint16_t mbuf_data_size[MAX_SEGS_BUFFER_SPLIT];
/**< Mbuf data space size. */
+enum mbuf_mem_type {
+ MBUF_MEM_CPU,
+ MBUF_MEM_GPU
+};
+extern enum mbuf_mem_type mbuf_mem_types[MAX_SEGS_BUFFER_SPLIT];
extern uint32_t param_total_num_mbufs;
extern uint16_t stats_period;
@@ -717,14 +723,16 @@ current_fwd_lcore(void)
/* Mbuf Pools */
static inline void
mbuf_poolname_build(unsigned int sock_id, char *mp_name,
- int name_size, uint16_t idx)
+ int name_size, uint16_t idx, enum mbuf_mem_type mem_type)
{
+ const char *suffix = mem_type == MBUF_MEM_GPU ? "_gpu" : "";
+
if (!idx)
snprintf(mp_name, name_size,
- MBUF_POOL_NAME_PFX "_%u", sock_id);
+ MBUF_POOL_NAME_PFX "_%u%s", sock_id, suffix);
else
snprintf(mp_name, name_size,
- MBUF_POOL_NAME_PFX "_%hu_%hu", (uint16_t)sock_id, idx);
+ MBUF_POOL_NAME_PFX "_%hu_%hu%s", (uint16_t)sock_id, idx, suffix);
}
static inline struct rte_mempool *
@@ -732,7 +740,7 @@ mbuf_pool_find(unsigned int sock_id, uint16_t idx)
{
char pool_name[RTE_MEMPOOL_NAMESIZE];
- mbuf_poolname_build(sock_id, pool_name, sizeof(pool_name), idx);
+ mbuf_poolname_build(sock_id, pool_name, sizeof(pool_name), idx, mbuf_mem_types[idx]);
return rte_mempool_lookup((const char *)pool_name);
}