mbox series

[00/11] Introduce support for RISC-V architecture

Message ID 20220505173003.3242618-1-kda@semihalf.com (mailing list archive)
Headers
Series Introduce support for RISC-V architecture |

Message

Stanislaw Kardach May 5, 2022, 5:29 p.m. UTC
  This patchset adds support for building and running DPDK on 64bit RISC-V
architecture. The initial support targets rv64gc (rv64imafdc) ISA and
was tested on SiFive Unmatched development board with the Freedom U740
SoC running Linux (freedom-u-sdk based kernel).
I have tested this codebase using DPDK unit and perf tests as well as
test-pmd, l2fwd and l3fwd examples.
The NIC attached to the DUT was Intel X520-DA2 which uses ixgbe PMD.
On the UIO side, since U740 does not have an IOMMU, I've used igb_uio,
uio_pci_generic and vfio-pci noiommu drivers.

Commits 1-2 fix small issues which are encountered if a given platform
   does not support any vector operations (which is the case with U740).
Commit 3 introduces EAL and build system support for RISC-V architecture
   as well as documentation updates.
Commits 4-7 add missing defines and stubs to enable RISC-V operation in
   non-EAL parts.
Commit 8 adds RISC-V specific cpuflags test.
Commit 9 works around a bug in the current GCC in test_ring compiled
   with -O0 or -Og.
Commit 10 adds RISC-V testing to test-meson-builds.sh automatically
   iterating over cross-compile config files (currently present for
   generic rv64gc and SiFive U740).
Commit 11 extends hash r/w perf test by displaying both HTM and non-HTM
   measurements. This is an extraneous commit which is not directly
   needed for RISC-V support but was noticed when we have started
   gathering test results. If needed, I can submit it separately.

I appreciate Your comments and feedback.

Best Regards,
Stanislaw Kardach

NOTE: This work was sponsored by StarFive and SiFive which is signified by
   "Sponsored-by:" sign-offs in each commit message. After discussing it
   with Thomas Monjalon it seemed a better choice than "Suggested-by" which
   does not fully convey the nature of involvement. However it makes
   Linux checkpatch unhappy so I'm not sure if I shouldn't change the
   sign-offs.

NOTE2: I have added maintainers for each commit based on MAINTAINERS file.
   However some modules (l3fwd, net/tap and cpuflags unit tests) do not have
   any maintainers assigned, hence I've targeted dev@dpdk.org mailing list as
   if it was a commit adding new files.

Michal Mazurek (3):
  lpm: add a scalar version of lookupx4 function
  eal: add initial support for RISC-V architecture
  test/cpuflags: add test for RISC-V cpu flag

Stanislaw Kardach (8):
  examples/l3fwd: fix scalar LPM compilation
  net/ixgbe: enable vector stubs for RISC-V
  net/memif: set memfd syscall ID on RISC-V
  net/tap: set BPF syscall ID for RISC-V
  examples/l3fwd: enable RISC-V operation
  test/ring: disable problematic tests for RISC-V
  devtools: add RISC-V to test-meson-builds.sh
  test/hash: report non HTM numbers for single r/w

 MAINTAINERS                                   |   6 +
 app/test/test_cpuflags.c                      |  81 ++++++++++
 app/test/test_hash_readwrite.c                |   8 +-
 app/test/test_ring.c                          |   8 +
 app/test/test_xmmt_ops.h                      |  16 ++
 config/meson.build                            |   2 +
 config/riscv/meson.build                      | 148 ++++++++++++++++++
 config/riscv/riscv64_linux_gcc                |  17 ++
 config/riscv/riscv64_sifive_u740_linux_gcc    |  19 +++
 devtools/test-meson-builds.sh                 |   6 +
 doc/guides/contributing/design.rst            |   2 +-
 .../linux_gsg/cross_build_dpdk_for_riscv.rst  | 125 +++++++++++++++
 doc/guides/linux_gsg/index.rst                |   1 +
 doc/guides/nics/features.rst                  |   5 +
 doc/guides/nics/features/default.ini          |   1 +
 doc/guides/nics/features/ixgbe.ini            |   1 +
 doc/guides/rel_notes/release_22_07.rst        |  29 ++++
 drivers/net/i40e/meson.build                  |   6 +
 drivers/net/ixgbe/ixgbe_rxtx.c                |   4 +-
 drivers/net/memif/rte_eth_memif.h             |   2 +
 drivers/net/tap/tap_bpf.h                     |   2 +
 examples/l3fwd/l3fwd_em.c                     |   8 +
 examples/l3fwd/l3fwd_fib.c                    |   2 +
 examples/l3fwd/l3fwd_lpm.c                    |   2 +-
 lib/eal/riscv/include/meson.build             |  23 +++
 lib/eal/riscv/include/rte_atomic.h            |  52 ++++++
 lib/eal/riscv/include/rte_byteorder.h         |  44 ++++++
 lib/eal/riscv/include/rte_cpuflags.h          |  55 +++++++
 lib/eal/riscv/include/rte_cycles.h            | 103 ++++++++++++
 lib/eal/riscv/include/rte_io.h                |  21 +++
 lib/eal/riscv/include/rte_mcslock.h           |  18 +++
 lib/eal/riscv/include/rte_memcpy.h            |  63 ++++++++
 lib/eal/riscv/include/rte_pause.h             |  31 ++++
 lib/eal/riscv/include/rte_pflock.h            |  17 ++
 lib/eal/riscv/include/rte_power_intrinsics.h  |  22 +++
 lib/eal/riscv/include/rte_prefetch.h          |  50 ++++++
 lib/eal/riscv/include/rte_rwlock.h            |  44 ++++++
 lib/eal/riscv/include/rte_spinlock.h          |  67 ++++++++
 lib/eal/riscv/include/rte_ticketlock.h        |  21 +++
 lib/eal/riscv/include/rte_vect.h              |  55 +++++++
 lib/eal/riscv/meson.build                     |  11 ++
 lib/eal/riscv/rte_cpuflags.c                  | 122 +++++++++++++++
 lib/eal/riscv/rte_cycles.c                    |  77 +++++++++
 lib/eal/riscv/rte_hypervisor.c                |  13 ++
 lib/eal/riscv/rte_power_intrinsics.c          |  56 +++++++
 lib/lpm/meson.build                           |   1 +
 lib/lpm/rte_lpm.h                             |   4 +-
 lib/lpm/rte_lpm_scalar.h                      | 122 +++++++++++++++
 meson.build                                   |   2 +
 49 files changed, 1588 insertions(+), 7 deletions(-)
 create mode 100644 config/riscv/meson.build
 create mode 100644 config/riscv/riscv64_linux_gcc
 create mode 100644 config/riscv/riscv64_sifive_u740_linux_gcc
 create mode 100644 doc/guides/linux_gsg/cross_build_dpdk_for_riscv.rst
 create mode 100644 lib/eal/riscv/include/meson.build
 create mode 100644 lib/eal/riscv/include/rte_atomic.h
 create mode 100644 lib/eal/riscv/include/rte_byteorder.h
 create mode 100644 lib/eal/riscv/include/rte_cpuflags.h
 create mode 100644 lib/eal/riscv/include/rte_cycles.h
 create mode 100644 lib/eal/riscv/include/rte_io.h
 create mode 100644 lib/eal/riscv/include/rte_mcslock.h
 create mode 100644 lib/eal/riscv/include/rte_memcpy.h
 create mode 100644 lib/eal/riscv/include/rte_pause.h
 create mode 100644 lib/eal/riscv/include/rte_pflock.h
 create mode 100644 lib/eal/riscv/include/rte_power_intrinsics.h
 create mode 100644 lib/eal/riscv/include/rte_prefetch.h
 create mode 100644 lib/eal/riscv/include/rte_rwlock.h
 create mode 100644 lib/eal/riscv/include/rte_spinlock.h
 create mode 100644 lib/eal/riscv/include/rte_ticketlock.h
 create mode 100644 lib/eal/riscv/include/rte_vect.h
 create mode 100644 lib/eal/riscv/meson.build
 create mode 100644 lib/eal/riscv/rte_cpuflags.c
 create mode 100644 lib/eal/riscv/rte_cycles.c
 create mode 100644 lib/eal/riscv/rte_hypervisor.c
 create mode 100644 lib/eal/riscv/rte_power_intrinsics.c
 create mode 100644 lib/lpm/rte_lpm_scalar.h
  

Comments

David Marchand May 6, 2022, 9:13 a.m. UTC | #1
On Thu, May 5, 2022 at 7:30 PM Stanislaw Kardach <kda@semihalf.com> wrote:
>
> This patchset adds support for building and running DPDK on 64bit RISC-V
> architecture. The initial support targets rv64gc (rv64imafdc) ISA and
> was tested on SiFive Unmatched development board with the Freedom U740
> SoC running Linux (freedom-u-sdk based kernel).
> I have tested this codebase using DPDK unit and perf tests as well as
> test-pmd, l2fwd and l3fwd examples.
> The NIC attached to the DUT was Intel X520-DA2 which uses ixgbe PMD.
> On the UIO side, since U740 does not have an IOMMU, I've used igb_uio,
> uio_pci_generic and vfio-pci noiommu drivers.
>
> Commits 1-2 fix small issues which are encountered if a given platform
>    does not support any vector operations (which is the case with U740).
> Commit 3 introduces EAL and build system support for RISC-V architecture
>    as well as documentation updates.
> Commits 4-7 add missing defines and stubs to enable RISC-V operation in
>    non-EAL parts.
> Commit 8 adds RISC-V specific cpuflags test.
> Commit 9 works around a bug in the current GCC in test_ring compiled
>    with -O0 or -Og.
> Commit 10 adds RISC-V testing to test-meson-builds.sh automatically
>    iterating over cross-compile config files (currently present for
>    generic rv64gc and SiFive U740).
> Commit 11 extends hash r/w perf test by displaying both HTM and non-HTM
>    measurements. This is an extraneous commit which is not directly
>    needed for RISC-V support but was noticed when we have started
>    gathering test results. If needed, I can submit it separately.
>
> I appreciate Your comments and feedback.

Thanks for working on this!

Please add a cross compilation job to GHA, something like:
https://github.com/david-marchand/dpdk/commit/4023e28f9050b85fb138eba14068bfe882036f01
Which looks to run fine:
https://github.com/david-marchand/dpdk/runs/6319625002?check_suite_focus=true

Testing all riscv configs in test-meson-buils.sh seems too much to me.
Is there a real value to test both current targets?

About the new "Sponsored-by" tag, it should not raise warnings in the
CI if we agree on its addition.

devtools/check-meson.py caught coding style issues.

In general, please avoid letting arch specific headers leak
internal/non rte_ prefixed helpers out of them.
For example, I noticed a RV64_CSRR macro that can be undefined after usage.

Patch 3 is huge, not sure it is easy to split, did you consider doing so?

The release notes update is verbose and some parts could be dropped,
like the list of verifications that are fine in a series cover letter.

Please resubmit fixes separately from this series so that we can merge
them sooner than this series.
  
Stanislaw Kardach May 9, 2022, 12:24 p.m. UTC | #2
On Fri, May 6, 2022 at 11:13 AM David Marchand <david.marchand@redhat.com>
wrote:

> On Thu, May 5, 2022 at 7:30 PM Stanislaw Kardach <kda@semihalf.com> wrote:
> >
> > This patchset adds support for building and running DPDK on 64bit RISC-V
> > architecture. The initial support targets rv64gc (rv64imafdc) ISA and
> > was tested on SiFive Unmatched development board with the Freedom U740
> > SoC running Linux (freedom-u-sdk based kernel).
> > I have tested this codebase using DPDK unit and perf tests as well as
> > test-pmd, l2fwd and l3fwd examples.
> > The NIC attached to the DUT was Intel X520-DA2 which uses ixgbe PMD.
> > On the UIO side, since U740 does not have an IOMMU, I've used igb_uio,
> > uio_pci_generic and vfio-pci noiommu drivers.
> >
> > Commits 1-2 fix small issues which are encountered if a given platform
> >    does not support any vector operations (which is the case with U740).
> > Commit 3 introduces EAL and build system support for RISC-V architecture
> >    as well as documentation updates.
> > Commits 4-7 add missing defines and stubs to enable RISC-V operation in
> >    non-EAL parts.
> > Commit 8 adds RISC-V specific cpuflags test.
> > Commit 9 works around a bug in the current GCC in test_ring compiled
> >    with -O0 or -Og.
> > Commit 10 adds RISC-V testing to test-meson-builds.sh automatically
> >    iterating over cross-compile config files (currently present for
> >    generic rv64gc and SiFive U740).
> > Commit 11 extends hash r/w perf test by displaying both HTM and non-HTM
> >    measurements. This is an extraneous commit which is not directly
> >    needed for RISC-V support but was noticed when we have started
> >    gathering test results. If needed, I can submit it separately.
> >
> > I appreciate Your comments and feedback.
>
> Thanks for working on this!
>
Thanks for your review!

>
> Please add a cross compilation job to GHA, something like:
>
> https://github.com/david-marchand/dpdk/commit/4023e28f9050b85fb138eba14068bfe882036f01
> Which looks to run fine:
>
> https://github.com/david-marchand/dpdk/runs/6319625002?check_suite_focus=true

Will do in V2.

>
>
> Testing all riscv configs in test-meson-buils.sh seems too much to me.
> Is there a real value to test both current targets?
>
It's for sanity and compilation coverage testing. I.e. SiFive variant has a
specific build config which does not require extra barriers when reading
time and cycle registers for rte_rdtsc_precise(). I want to make sure that
if anyone changes some code based on configuration flags, it gets at least
compile-checked.
I believe similar thing is done for Aarch64 builds.

>
> About the new "Sponsored-by" tag, it should not raise warnings in the
> CI if we agree on its addition.
>
I'll modify it in V2 to be in form of:
  Sponsored by: StarFive Technology
  ...
  Signed-off-by: ...
This was suggested by Stephen Hemminger as having a precedent in Linux
kernel. Interestingly enough first use of this tag in kernel source was
this year in January.

>
> devtools/check-meson.py caught coding style issues.
>
Will fix in V2.

>
> In general, please avoid letting arch specific headers leak
> internal/non rte_ prefixed helpers out of them.
> For example, I noticed a RV64_CSRR macro that can be undefined after usage.
>
Thanks for noticing. I'l fix this one in V2.
There are 2 other symbols that leak but on purpose (out of a better
idea): vect_load_128() and vect_and(). Both are used in l3fwd_em to
simulate vector operations. Other platforms reference their intrinsics
straight in the l3fwd_em.c. As I don't have support for vector ops and I
wanted to indicate that xmm_t should be an isolated API, I've put both in
rte_vect.h. That said I'm not happy with this solution and am open to
suggestions on how to solve it neatly.

>
> Patch 3 is huge, not sure it is easy to split, did you consider doing so?
>
It seems to me the nature of a new EAL implementation, I have to include
all symbols, otherwise DPDK won't compile.
Alternatively I could have a huge initial patch with empty stubs that would
be filled in later commits. Downside of this approach is that it's hard to
verify each commit separately as tests will fail until all implementation
is there, so the division is only visual.

>
> The release notes update is verbose and some parts could be dropped,
> like the list of verifications that are fine in a series cover letter.
>
Will do. I'll move listed items to the cover letter.

>
> Please resubmit fixes separately from this series so that we can merge
> them sooner than this series.
>
Will do. Since at least 2 fixes are required for the RISC-V EAL to work or
compile, I'll put  Depends-on tag in the EAL commit.

>
>
> --
> David Marchand
>
>
  
Thomas Monjalon May 9, 2022, 12:30 p.m. UTC | #3
09/05/2022 14:24, Stanisław Kardach:
> On Fri, May 6, 2022 at 11:13 AM David Marchand <david.marchand@redhat.com>
> wrote:
> > About the new "Sponsored-by" tag, it should not raise warnings in the
> > CI if we agree on its addition.
> >
> I'll modify it in V2 to be in form of:
>   Sponsored by: StarFive Technology

You mean removing the hyphen?
I think it is better to keep it so all tags have the same format.

>   ...
>   Signed-off-by: ...
> 
> This was suggested by Stephen Hemminger as having a precedent in Linux
> kernel. Interestingly enough first use of this tag in kernel source was
> this year in January.

The precedent is not strong enough to be copied in my opinion.
  
David Marchand May 9, 2022, 2:30 p.m. UTC | #4
On Mon, May 9, 2022 at 2:24 PM Stanisław Kardach <kda@semihalf.com> wrote:
>> Testing all riscv configs in test-meson-buils.sh seems too much to me.
>> Is there a real value to test both current targets?
>
> It's for sanity and compilation coverage testing. I.e. SiFive variant has a specific build config which does not require extra barriers when reading time and cycle registers for rte_rdtsc_precise(). I want to make sure that if anyone changes some code based on configuration flags, it gets at least compile-checked.
> I believe similar thing is done for Aarch64 builds.

As far as I experienced, building all those aarch64 combinations never
revealed any specific platform compilation issue.
It only consumes cpu, disk and our (maintainers) time.
I proposed to Thomas to shrink aarch64 builds list not so long ago :-).

The best would be for SiFive to provide a system for the CI to do
those checks on their variant.


>> About the new "Sponsored-by" tag, it should not raise warnings in the
>> CI if we agree on its addition.
>
> I'll modify it in V2 to be in form of:
>   Sponsored by: StarFive Technology
>   ...
>   Signed-off-by: ...
> This was suggested by Stephen Hemminger as having a precedent in Linux kernel. Interestingly enough first use of this tag in kernel source was this year in January.

I don't have an opinion on the spelling.

At the moment, the checks raise a warning:
http://mails.dpdk.org/archives/test-report/2022-May/278580.html

My point is that for this new tag, either checkpatch.pl in kernel
handles it (which I don't think it is the case) or we need to disable
the signature check in checkpatch.pl and something is added in dpdk
checkpatches.sh to accept all known tags.


>> In general, please avoid letting arch specific headers leak
>> internal/non rte_ prefixed helpers out of them.
>> For example, I noticed a RV64_CSRR macro that can be undefined after usage.
>
> Thanks for noticing. I'l fix this one in V2.
> There are 2 other symbols that leak but on purpose (out of a better idea): vect_load_128() and vect_and(). Both are used in l3fwd_em to simulate vector operations. Other platforms reference their intrinsics straight in the l3fwd_em.c. As I don't have support for vector ops and I wanted to indicate that xmm_t should be an isolated API, I've put both in rte_vect.h. That said I'm not happy with this solution and am open to suggestions on how to solve it neatly.

I'll try to have a look in the next revision.


>>
>>
>> Patch 3 is huge, not sure it is easy to split, did you consider doing so?
>
> It seems to me the nature of a new EAL implementation, I have to include all symbols, otherwise DPDK won't compile.
> Alternatively I could have a huge initial patch with empty stubs that would be filled in later commits. Downside of this approach is that it's hard to verify each commit separately as tests will fail until all implementation is there, so the division is only visual.

If you are sure there is nothing that can be separated, let's keep it whole.
  
Stanislaw Kardach May 10, 2022, 11:21 a.m. UTC | #5
On Mon, May 9, 2022 at 4:31 PM David Marchand <david.marchand@redhat.com>
wrote:

> On Mon, May 9, 2022 at 2:24 PM Stanisław Kardach <kda@semihalf.com> wrote:
> >> Testing all riscv configs in test-meson-buils.sh seems too much to me.
> >> Is there a real value to test both current targets?
> >
> > It's for sanity and compilation coverage testing. I.e. SiFive variant
> has a specific build config which does not require extra barriers when
> reading time and cycle registers for rte_rdtsc_precise(). I want to make
> sure that if anyone changes some code based on configuration flags, it gets
> at least compile-checked.
> > I believe similar thing is done for Aarch64 builds.
>
> As far as I experienced, building all those aarch64 combinations never
> revealed any specific platform compilation issue.
> It only consumes cpu, disk and our (maintainers) time.
> I proposed to Thomas to shrink aarch64 builds list not so long ago :-).
>
> The best would be for SiFive to provide a system for the CI to do
> those checks on their variant.
>
>
> >> About the new "Sponsored-by" tag, it should not raise warnings in the
> >> CI if we agree on its addition.
> >
> > I'll modify it in V2 to be in form of:
> >   Sponsored by: StarFive Technology
> >   ...
> >   Signed-off-by: ...
> > This was suggested by Stephen Hemminger as having a precedent in Linux
> kernel. Interestingly enough first use of this tag in kernel source was
> this year in January.
>
> I don't have an opinion on the spelling.
>
> At the moment, the checks raise a warning:
> http://mails.dpdk.org/archives/test-report/2022-May/278580.html
>
> My point is that for this new tag, either checkpatch.pl in kernel
> handles it (which I don't think it is the case) or we need to disable
> the signature check in checkpatch.pl and something is added in dpdk
> checkpatches.sh to accept all known tags.
>
BAD_SIGN_OFF handles more than just tag names (in total there's 10 cases
checked). I'm not sure replicating this to checkpatches.sh is worth the
maintenance.
Alternatively I could ignore BAD_SIGN_OFF on initial checkpatch.pl run and
then run it again with just the BAD_SIGN_OFF type and filter out the result.
In that case, what would be the acceptable content of Sponsored-by tag? For
line:
  Sponsored-by: StarFive Technology
Current checkpatch.pl generates (used --terse for brevity):
  0001-eal-add-initial-support-for-RISC-V-architecture.patch:55:
WARNING:BAD_SIGN_OFF: Non-standard signature: Sponsored-by:
  0001-eal-add-initial-support-for-RISC-V-architecture.patch:55:
ERROR:BAD_SIGN_OFF: Unrecognized email address: 'StarFive Technology'

Using "Sponsored by:" does not trigger checks above (still feels like a
hack).

>
> >> In general, please avoid letting arch specific headers leak
> >> internal/non rte_ prefixed helpers out of them.
> >> For example, I noticed a RV64_CSRR macro that can be undefined after
> usage.
> >
> > Thanks for noticing. I'l fix this one in V2.
> > There are 2 other symbols that leak but on purpose (out of a better
> idea): vect_load_128() and vect_and(). Both are used in l3fwd_em to
> simulate vector operations. Other platforms reference their intrinsics
> straight in the l3fwd_em.c. As I don't have support for vector ops and I
> wanted to indicate that xmm_t should be an isolated API, I've put both in
> rte_vect.h. That said I'm not happy with this solution and am open to
> suggestions on how to solve it neatly.
>
> I'll try to have a look in the next revision.
>
>
> >>
> >>
> >> Patch 3 is huge, not sure it is easy to split, did you consider doing
> so?
> >
> > It seems to me the nature of a new EAL implementation, I have to include
> all symbols, otherwise DPDK won't compile.
> > Alternatively I could have a huge initial patch with empty stubs that
> would be filled in later commits. Downside of this approach is that it's
> hard to verify each commit separately as tests will fail until all
> implementation is there, so the division is only visual.
>
> If you are sure there is nothing that can be separated, let's keep it
> whole.
>
>
>
> --
> David Marchand
>
>
  
Thomas Monjalon May 10, 2022, 12:31 p.m. UTC | #6
10/05/2022 13:21, Stanisław Kardach:
> On Mon, May 9, 2022 at 4:31 PM David Marchand <david.marchand@redhat.com>
> wrote:
> > >> About the new "Sponsored-by" tag, it should not raise warnings in the
> > >> CI if we agree on its addition.
> > >
> > > I'll modify it in V2 to be in form of:
> > >   Sponsored by: StarFive Technology
> > >   ...
> > >   Signed-off-by: ...
> > > This was suggested by Stephen Hemminger as having a precedent in Linux
> > kernel. Interestingly enough first use of this tag in kernel source was
> > this year in January.
> >
> > I don't have an opinion on the spelling.
> >
> > At the moment, the checks raise a warning:
> > http://mails.dpdk.org/archives/test-report/2022-May/278580.html
> >
> > My point is that for this new tag, either checkpatch.pl in kernel
> > handles it (which I don't think it is the case) or we need to disable
> > the signature check in checkpatch.pl and something is added in dpdk
> > checkpatches.sh to accept all known tags.
> >
> BAD_SIGN_OFF handles more than just tag names (in total there's 10 cases
> checked). I'm not sure replicating this to checkpatches.sh is worth the
> maintenance.
> Alternatively I could ignore BAD_SIGN_OFF on initial checkpatch.pl run and
> then run it again with just the BAD_SIGN_OFF type and filter out the result.
> In that case, what would be the acceptable content of Sponsored-by tag? For
> line:
>   Sponsored-by: StarFive Technology
> Current checkpatch.pl generates (used --terse for brevity):
>   0001-eal-add-initial-support-for-RISC-V-architecture.patch:55:
> WARNING:BAD_SIGN_OFF: Non-standard signature: Sponsored-by:
>   0001-eal-add-initial-support-for-RISC-V-architecture.patch:55:
> ERROR:BAD_SIGN_OFF: Unrecognized email address: 'StarFive Technology'
> 
> Using "Sponsored by:" does not trigger checks above (still feels like a
> hack).

Agree it is a hack,
and not having the hyphen breaks my Vim colouring :)

We can ignore this checkpatch warning, that's fine.
  
Stanislaw Kardach May 10, 2022, 2 p.m. UTC | #7
On Tue, May 10, 2022 at 2:32 PM Thomas Monjalon <thomas@monjalon.net> wrote:

> 10/05/2022 13:21, Stanisław Kardach:
> >
> > Using "Sponsored by:" does not trigger checks above (still feels like a
> > hack).
>
> Agree it is a hack,
> and not having the hyphen breaks my Vim colouring :)
>
> We can ignore this checkpatch warning, that's fine.
>
> Just to be sure before I post V2 - do you mean to ignore BAD_SIGN_OFF in
checkpatches.sh or rather somewhere in CI configuration?
  
Thomas Monjalon May 10, 2022, 2:23 p.m. UTC | #8
10/05/2022 16:00, Stanisław Kardach:
> On Tue, May 10, 2022 at 2:32 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 10/05/2022 13:21, Stanisław Kardach:
> > >
> > > Using "Sponsored by:" does not trigger checks above (still feels like a
> > > hack).
> >
> > Agree it is a hack,
> > and not having the hyphen breaks my Vim colouring :)
> >
> > We can ignore this checkpatch warning, that's fine.
> >
> > Just to be sure before I post V2 - do you mean to ignore BAD_SIGN_OFF in
> checkpatches.sh or rather somewhere in CI configuration?

Yes I mean CI will return BAD_SIGN_OFF, we'll take note,
but it won't be a blocker to accept the patch.

The CI can return minor warnings, that's OK.
We prefer avoiding the warnings, because it is more work
to check all the warnings manually, but I don't see a better solution here.
  
Morten Brørup May 11, 2022, 8:09 a.m. UTC | #9
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, 9 May 2022 14.31
> 
> 09/05/2022 14:24, Stanisław Kardach:
> > On Fri, May 6, 2022 at 11:13 AM David Marchand
> <david.marchand@redhat.com>
> > wrote:
> > > About the new "Sponsored-by" tag, it should not raise warnings in
> the
> > > CI if we agree on its addition.
> > >
> > I'll modify it in V2 to be in form of:
> >   Sponsored by: StarFive Technology
> 
> You mean removing the hyphen?
> I think it is better to keep it so all tags have the same format.

I agree with Thomas. Please keep the hyphen.

> 
> >   ...
> >   Signed-off-by: ...
> >
> > This was suggested by Stephen Hemminger as having a precedent in
> Linux
> > kernel. Interestingly enough first use of this tag in kernel source
> was
> > this year in January.

I don't get it! Should employees start adding Sponsored-by: <Employer name> to their commits, when doing it as part of their job? And how about contract developers, should they also add a Sponsored-by: <Company name> tag, since they are working under contract and getting paid by that company?

Can someone please provide a reference to the discussion about this on the LKML? I'm curious why they felt the need for such a tag.

> 
> The precedent is not strong enough to be copied in my opinion.

I agree. Also, if it is an acceptable signature tag, the checkpatch.pl script should be updated:

https://elixir.bootlin.com/linux/v5.18-rc6/source/scripts/checkpatch.pl#L607
  
Stanislaw Kardach May 11, 2022, 10:28 a.m. UTC | #10
On Wed, May 11, 2022 at 10:09 AM Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Monday, 9 May 2022 14.31
> >
> > 09/05/2022 14:24, Stanisław Kardach:
> > > On Fri, May 6, 2022 at 11:13 AM David Marchand
> > <david.marchand@redhat.com>
> > > wrote:
> > > > About the new "Sponsored-by" tag, it should not raise warnings in
> > the
> > > > CI if we agree on its addition.
> > > >
> > > I'll modify it in V2 to be in form of:
> > >   Sponsored by: StarFive Technology
> >
> > You mean removing the hyphen?
> > I think it is better to keep it so all tags have the same format.
>
> I agree with Thomas. Please keep the hyphen.
>
> >
> > >   ...
> > >   Signed-off-by: ...
> > >
> > > This was suggested by Stephen Hemminger as having a precedent in
> > Linux
> > > kernel. Interestingly enough first use of this tag in kernel source
> > was
> > > this year in January.
>
> I don't get it! Should employees start adding Sponsored-by: <Employer name> to their commits, when doing it as part of their job? And how about contract developers, should they also add a Sponsored-by: <Company name> tag, since they are working under contract and getting paid by that company?
If I understand correctly, the concern about the commit log staying
technical and not introducing extra elements not beneficial to the
project, correct?
In the scope of this particular patchset, the companies sponsoring the
work are in copyrights for appropriate files, so I can remove the
tags.
For my own curiosity, what would be a proper way for a contract
developer to mark the company sponsoring the work? Some companies may
not care, others will. Maybe it would be beneficial to add a comment
on this into the contributing guide (or it's already there and I've
missed it)?
>
> Can someone please provide a reference to the discussion about this on the LKML? I'm curious why they felt the need for such a tag.
Actually I can't find any discussion on this. Perhaps it was missed
because: "Sponsored by" (without a hyphen) doesn't trigger checkpatch
and only 2 patches (6 and 7) in whole patchset contain it.
Patchset for reference:
https://www.spinics.net/lists/linux-wireless/msg220474.html

>
> >
> > The precedent is not strong enough to be copied in my opinion.
>
> I agree. Also, if it is an acceptable signature tag, the checkpatch.pl script should be updated:
>
> https://elixir.bootlin.com/linux/v5.18-rc6/source/scripts/checkpatch.pl#L607
>
>
  
Thomas Monjalon May 11, 2022, 11:06 a.m. UTC | #11
11/05/2022 12:28, Stanisław Kardach:
> On Wed, May 11, 2022 at 10:09 AM Morten Brørup <mb@smartsharesystems.com> wrote:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 09/05/2022 14:24, Stanisław Kardach:
> > > > On Fri, May 6, 2022 at 11:13 AM David Marchand wrote:
> > > > > About the new "Sponsored-by" tag, it should not raise warnings in
> > > > > the CI if we agree on its addition.
> > > > 
> > > > I'll modify it in V2 to be in form of:
> > > >   Sponsored by: StarFive Technology
> > >
> > > You mean removing the hyphen?
> > > I think it is better to keep it so all tags have the same format.
> >
> > I agree with Thomas. Please keep the hyphen.
> >
> > > >   ...
> > > >   Signed-off-by: ...
> > > >
> > > > This was suggested by Stephen Hemminger as having a precedent in
> > > > Linux kernel. Interestingly enough first use of this tag in kernel
> > > > source was this year in January.
> >
> > I don't get it! Should employees start adding
> > Sponsored-by: <Employer name> to their commits,
> > when doing it as part of their job?
> > And how about contract developers, should they also add a
> > Sponsored-by: <Company name> tag,
> > since they are working under contract and getting paid by that company?

Nothing is mandatory.
It is just a way to give visibility of the sponsorship if requested.

> If I understand correctly, the concern about the commit log staying
> technical and not introducing extra elements not beneficial to the
> project, correct?
> In the scope of this particular patchset, the companies sponsoring the
> work are in copyrights for appropriate files, so I can remove the
> tags.
> For my own curiosity, what would be a proper way for a contract
> developer to mark the company sponsoring the work? Some companies may
> not care, others will. Maybe it would be beneficial to add a comment
> on this into the contributing guide (or it's already there and I've
> missed it)?

I don't know how to do it better.
Any other suggestions?
  
Heinrich Schuchardt May 12, 2022, 8:04 a.m. UTC | #12
On 5/5/22 19:29, Stanislaw Kardach wrote:
> This patchset adds support for building and running DPDK on 64bit RISC-V
> architecture. The initial support targets rv64gc (rv64imafdc) ISA and
> was tested on SiFive Unmatched development board with the Freedom U740
> SoC running Linux (freedom-u-sdk based kernel).
> I have tested this codebase using DPDK unit and perf tests as well as
> test-pmd, l2fwd and l3fwd examples.
> The NIC attached to the DUT was Intel X520-DA2 which uses ixgbe PMD.
> On the UIO side, since U740 does not have an IOMMU, I've used igb_uio,
> uio_pci_generic and vfio-pci noiommu drivers.
> 
> Commits 1-2 fix small issues which are encountered if a given platform
>     does not support any vector operations (which is the case with U740).
> Commit 3 introduces EAL and build system support for RISC-V architecture
>     as well as documentation updates.
> Commits 4-7 add missing defines and stubs to enable RISC-V operation in
>     non-EAL parts.
> Commit 8 adds RISC-V specific cpuflags test.
> Commit 9 works around a bug in the current GCC in test_ring compiled
>     with -O0 or -Og.
> Commit 10 adds RISC-V testing to test-meson-builds.sh automatically
>     iterating over cross-compile config files (currently present for
>     generic rv64gc and SiFive U740).
> Commit 11 extends hash r/w perf test by displaying both HTM and non-HTM
>     measurements. This is an extraneous commit which is not directly
>     needed for RISC-V support but was noticed when we have started
>     gathering test results. If needed, I can submit it separately.
> 
> I appreciate Your comments and feedback.
> 
> Best Regards,
> Stanislaw Kardach

On an SiFive Unmatched board with Ubuntu Jammy I have been compiling 
DPDK origin/main (36db4a1ad464) with this series and the test patches 
that you split off.

The fast tests are either skipped or succeed. But I have seen errors for 
the performance tests:

4 1GiB hugepages defined
no network devices bound
vfio_pci loaded (no IOMMU)

sudo meson -t 30

129/173 DPDK:perf-tests / pmd_perf_autotest FAIL 7.85s (exit status 255 
or signal 127 SIGinvalid)
 >>> DPDK_TEST=pmd_perf_autotest MALLOC_PERTURB_=135 
build/app/test/dpdk-test
136/173 DPDK:perf-tests / ipsec_perf_autotest FAIL 4.47s (exit status 
255 or signal 127 SIGinvalid)
 >>> MALLOC_PERTURB_=181 DPDK_TEST=ipsec_perf_autotest 
build/app/test/dpdk-test
160/173 DPDK:driver-tests / link_bonding_mode4_autotest FAIL 31.80s 
killed by signal 11 SIGSEGV
 >>> MALLOC_PERTURB_=62 DPDK_TEST=link_bonding_mode4_autotest 
build/app/test/dpdk-test

These are the patches used:

# [PATCH 1/1] lpm: add a scalar version of lookupx4 function
   https://inbox.dpdk.org/dev/20220510115824.457885-1-kda@semihalf.com/
# [PATCH 1/1] examples/l3fwd: fix scalar LPM compilation
   https://inbox.dpdk.org/dev/20220510115844.458009-1-kda@semihalf.com/
# [PATCH 1/1] test/hash: report non HTM numbers for single r/w
   https://inbox.dpdk.org/dev/20220510115734.457718-1-kda@semihalf.com/
# [PATCH 1/1] test/ring: remove excessive inlining
   https://inbox.dpdk.org/dev/20220510115758.457794-1-kda@semihalf.com/
# [PATCH v3 0/8] Introduce support for RISC-V architecture
   https://inbox.dpdk.org/dev/20220510154849.530872-1-kda@semihalf.com/
# [PATCH 1/1] drivers: define OPENSSL_API_COMPAT
 
https://inbox.dpdk.org/dev/20220510150635.61975-1-heinrich.schuchardt@canonical.com/

Running sudo app/test/dpdk-test does not show the errors:

RTE>>pmd_perf_autotest
Start PMD RXTX cycles cost test.
At least 1 port(s) used for perf. test
Test Failed
RTE>>ipsec_perf_autotest
USER1: rte_ipsec_pkt_crypto_prepare fail
Test Failed
RTE>>link_bonding_mode4_autotest
...
Test OK
RTE>

Best regards

Heinrich

> 
> NOTE: This work was sponsored by StarFive and SiFive which is signified by
>     "Sponsored-by:" sign-offs in each commit message. After discussing it
>     with Thomas Monjalon it seemed a better choice than "Suggested-by" which
>     does not fully convey the nature of involvement. However it makes
>     Linux checkpatch unhappy so I'm not sure if I shouldn't change the
>     sign-offs.
> 
> NOTE2: I have added maintainers for each commit based on MAINTAINERS file.
>     However some modules (l3fwd, net/tap and cpuflags unit tests) do not have
>     any maintainers assigned, hence I've targeted dev@dpdk.org mailing list as
>     if it was a commit adding new files.
> 
> Michal Mazurek (3):
>    lpm: add a scalar version of lookupx4 function
>    eal: add initial support for RISC-V architecture
>    test/cpuflags: add test for RISC-V cpu flag
> 
> Stanislaw Kardach (8):
>    examples/l3fwd: fix scalar LPM compilation
>    net/ixgbe: enable vector stubs for RISC-V
>    net/memif: set memfd syscall ID on RISC-V
>    net/tap: set BPF syscall ID for RISC-V
>    examples/l3fwd: enable RISC-V operation
>    test/ring: disable problematic tests for RISC-V
>    devtools: add RISC-V to test-meson-builds.sh
>    test/hash: report non HTM numbers for single r/w
> 
>   MAINTAINERS                                   |   6 +
>   app/test/test_cpuflags.c                      |  81 ++++++++++
>   app/test/test_hash_readwrite.c                |   8 +-
>   app/test/test_ring.c                          |   8 +
>   app/test/test_xmmt_ops.h                      |  16 ++
>   config/meson.build                            |   2 +
>   config/riscv/meson.build                      | 148 ++++++++++++++++++
>   config/riscv/riscv64_linux_gcc                |  17 ++
>   config/riscv/riscv64_sifive_u740_linux_gcc    |  19 +++
>   devtools/test-meson-builds.sh                 |   6 +
>   doc/guides/contributing/design.rst            |   2 +-
>   .../linux_gsg/cross_build_dpdk_for_riscv.rst  | 125 +++++++++++++++
>   doc/guides/linux_gsg/index.rst                |   1 +
>   doc/guides/nics/features.rst                  |   5 +
>   doc/guides/nics/features/default.ini          |   1 +
>   doc/guides/nics/features/ixgbe.ini            |   1 +
>   doc/guides/rel_notes/release_22_07.rst        |  29 ++++
>   drivers/net/i40e/meson.build                  |   6 +
>   drivers/net/ixgbe/ixgbe_rxtx.c                |   4 +-
>   drivers/net/memif/rte_eth_memif.h             |   2 +
>   drivers/net/tap/tap_bpf.h                     |   2 +
>   examples/l3fwd/l3fwd_em.c                     |   8 +
>   examples/l3fwd/l3fwd_fib.c                    |   2 +
>   examples/l3fwd/l3fwd_lpm.c                    |   2 +-
>   lib/eal/riscv/include/meson.build             |  23 +++
>   lib/eal/riscv/include/rte_atomic.h            |  52 ++++++
>   lib/eal/riscv/include/rte_byteorder.h         |  44 ++++++
>   lib/eal/riscv/include/rte_cpuflags.h          |  55 +++++++
>   lib/eal/riscv/include/rte_cycles.h            | 103 ++++++++++++
>   lib/eal/riscv/include/rte_io.h                |  21 +++
>   lib/eal/riscv/include/rte_mcslock.h           |  18 +++
>   lib/eal/riscv/include/rte_memcpy.h            |  63 ++++++++
>   lib/eal/riscv/include/rte_pause.h             |  31 ++++
>   lib/eal/riscv/include/rte_pflock.h            |  17 ++
>   lib/eal/riscv/include/rte_power_intrinsics.h  |  22 +++
>   lib/eal/riscv/include/rte_prefetch.h          |  50 ++++++
>   lib/eal/riscv/include/rte_rwlock.h            |  44 ++++++
>   lib/eal/riscv/include/rte_spinlock.h          |  67 ++++++++
>   lib/eal/riscv/include/rte_ticketlock.h        |  21 +++
>   lib/eal/riscv/include/rte_vect.h              |  55 +++++++
>   lib/eal/riscv/meson.build                     |  11 ++
>   lib/eal/riscv/rte_cpuflags.c                  | 122 +++++++++++++++
>   lib/eal/riscv/rte_cycles.c                    |  77 +++++++++
>   lib/eal/riscv/rte_hypervisor.c                |  13 ++
>   lib/eal/riscv/rte_power_intrinsics.c          |  56 +++++++
>   lib/lpm/meson.build                           |   1 +
>   lib/lpm/rte_lpm.h                             |   4 +-
>   lib/lpm/rte_lpm_scalar.h                      | 122 +++++++++++++++
>   meson.build                                   |   2 +
>   49 files changed, 1588 insertions(+), 7 deletions(-)
>   create mode 100644 config/riscv/meson.build
>   create mode 100644 config/riscv/riscv64_linux_gcc
>   create mode 100644 config/riscv/riscv64_sifive_u740_linux_gcc
>   create mode 100644 doc/guides/linux_gsg/cross_build_dpdk_for_riscv.rst
>   create mode 100644 lib/eal/riscv/include/meson.build
>   create mode 100644 lib/eal/riscv/include/rte_atomic.h
>   create mode 100644 lib/eal/riscv/include/rte_byteorder.h
>   create mode 100644 lib/eal/riscv/include/rte_cpuflags.h
>   create mode 100644 lib/eal/riscv/include/rte_cycles.h
>   create mode 100644 lib/eal/riscv/include/rte_io.h
>   create mode 100644 lib/eal/riscv/include/rte_mcslock.h
>   create mode 100644 lib/eal/riscv/include/rte_memcpy.h
>   create mode 100644 lib/eal/riscv/include/rte_pause.h
>   create mode 100644 lib/eal/riscv/include/rte_pflock.h
>   create mode 100644 lib/eal/riscv/include/rte_power_intrinsics.h
>   create mode 100644 lib/eal/riscv/include/rte_prefetch.h
>   create mode 100644 lib/eal/riscv/include/rte_rwlock.h
>   create mode 100644 lib/eal/riscv/include/rte_spinlock.h
>   create mode 100644 lib/eal/riscv/include/rte_ticketlock.h
>   create mode 100644 lib/eal/riscv/include/rte_vect.h
>   create mode 100644 lib/eal/riscv/meson.build
>   create mode 100644 lib/eal/riscv/rte_cpuflags.c
>   create mode 100644 lib/eal/riscv/rte_cycles.c
>   create mode 100644 lib/eal/riscv/rte_hypervisor.c
>   create mode 100644 lib/eal/riscv/rte_power_intrinsics.c
>   create mode 100644 lib/lpm/rte_lpm_scalar.h
>
  
Stanislaw Kardach May 12, 2022, 8:35 a.m. UTC | #13
On Thu, May 12, 2022 at 10:04 AM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 5/5/22 19:29, Stanislaw Kardach wrote:
> > This patchset adds support for building and running DPDK on 64bit RISC-V
> > architecture. The initial support targets rv64gc (rv64imafdc) ISA and
> > was tested on SiFive Unmatched development board with the Freedom U740
> > SoC running Linux (freedom-u-sdk based kernel).
> > I have tested this codebase using DPDK unit and perf tests as well as
> > test-pmd, l2fwd and l3fwd examples.
> > The NIC attached to the DUT was Intel X520-DA2 which uses ixgbe PMD.
> > On the UIO side, since U740 does not have an IOMMU, I've used igb_uio,
> > uio_pci_generic and vfio-pci noiommu drivers.
> >
> > Commits 1-2 fix small issues which are encountered if a given platform
> >     does not support any vector operations (which is the case with U740).
> > Commit 3 introduces EAL and build system support for RISC-V architecture
> >     as well as documentation updates.
> > Commits 4-7 add missing defines and stubs to enable RISC-V operation in
> >     non-EAL parts.
> > Commit 8 adds RISC-V specific cpuflags test.
> > Commit 9 works around a bug in the current GCC in test_ring compiled
> >     with -O0 or -Og.
> > Commit 10 adds RISC-V testing to test-meson-builds.sh automatically
> >     iterating over cross-compile config files (currently present for
> >     generic rv64gc and SiFive U740).
> > Commit 11 extends hash r/w perf test by displaying both HTM and non-HTM
> >     measurements. This is an extraneous commit which is not directly
> >     needed for RISC-V support but was noticed when we have started
> >     gathering test results. If needed, I can submit it separately.
> >
> > I appreciate Your comments and feedback.
> >
> > Best Regards,
> > Stanislaw Kardach
>
> On an SiFive Unmatched board with Ubuntu Jammy I have been compiling
> DPDK origin/main (36db4a1ad464) with this series and the test patches
> that you split off.
Thank you for testing!
>
> The fast tests are either skipped or succeed. But I have seen errors for
> the performance tests:
>
> 4 1GiB hugepages defined
> no network devices bound
> vfio_pci loaded (no IOMMU)
>
> sudo meson -t 30
>
> 129/173 DPDK:perf-tests / pmd_perf_autotest FAIL 7.85s (exit status 255
> or signal 127 SIGinvalid)
>  >>> DPDK_TEST=pmd_perf_autotest MALLOC_PERTURB_=135
> build/app/test/dpdk-test
This test I believe requires at least a single NIC port. Conversely
fast-tests seem to fail if a device is bound. DPDK CI doesn't bind any
on fast-tests, I'm not sure if perf-tests suite is run there.
On my local x86 machine perf-tests suite fails when no device is bound too.

> 136/173 DPDK:perf-tests / ipsec_perf_autotest FAIL 4.47s (exit status
> 255 or signal 127 SIGinvalid)
>  >>> MALLOC_PERTURB_=181 DPDK_TEST=ipsec_perf_autotest
> build/app/test/dpdk-test
On my unmatched I don't have openssl compiled so most crypto tests are
skipped (since no openssl is installed), aside of this one and it
indeed fails. I'm not sure whether it's a matter of a lack of a guard
to skip this test if there's no crypto backend built or if it's a real
issue.
I admit I haven't tried crypto libraries with RISC-V openssl as
there's no real HW offloading on Unmatched.

> 160/173 DPDK:driver-tests / link_bonding_mode4_autotest FAIL 31.80s
> killed by signal 11 SIGSEGV
>  >>> MALLOC_PERTURB_=62 DPDK_TEST=link_bonding_mode4_autotest
> build/app/test/dpdk-test
I use -t40 when testing on Unmatched. Unfortunately lots of the tests
hit timeouts.

>
> These are the patches used:
>
> # [PATCH 1/1] lpm: add a scalar version of lookupx4 function
>    https://inbox.dpdk.org/dev/20220510115824.457885-1-kda@semihalf.com/
> # [PATCH 1/1] examples/l3fwd: fix scalar LPM compilation
>    https://inbox.dpdk.org/dev/20220510115844.458009-1-kda@semihalf.com/
> # [PATCH 1/1] test/hash: report non HTM numbers for single r/w
>    https://inbox.dpdk.org/dev/20220510115734.457718-1-kda@semihalf.com/
> # [PATCH 1/1] test/ring: remove excessive inlining
>    https://inbox.dpdk.org/dev/20220510115758.457794-1-kda@semihalf.com/
> # [PATCH v3 0/8] Introduce support for RISC-V architecture
>    https://inbox.dpdk.org/dev/20220510154849.530872-1-kda@semihalf.com/
> # [PATCH 1/1] drivers: define OPENSSL_API_COMPAT
>
> https://inbox.dpdk.org/dev/20220510150635.61975-1-heinrich.schuchardt@canonical.com/
>
> Running sudo app/test/dpdk-test does not show the errors:
>
> RTE>>pmd_perf_autotest
> Start PMD RXTX cycles cost test.
> At least 1 port(s) used for perf. test
> Test Failed
> RTE>>ipsec_perf_autotest
> USER1: rte_ipsec_pkt_crypto_prepare fail
> Test Failed
> RTE>>link_bonding_mode4_autotest
> ...
> Test OK
> RTE>
>
> Best regards
>
> Heinrich
>
> >
> > NOTE: This work was sponsored by StarFive and SiFive which is signified by
> >     "Sponsored-by:" sign-offs in each commit message. After discussing it
> >     with Thomas Monjalon it seemed a better choice than "Suggested-by" which
> >     does not fully convey the nature of involvement. However it makes
> >     Linux checkpatch unhappy so I'm not sure if I shouldn't change the
> >     sign-offs.
> >
> > NOTE2: I have added maintainers for each commit based on MAINTAINERS file.
> >     However some modules (l3fwd, net/tap and cpuflags unit tests) do not have
> >     any maintainers assigned, hence I've targeted dev@dpdk.org mailing list as
> >     if it was a commit adding new files.
> >
> > Michal Mazurek (3):
> >    lpm: add a scalar version of lookupx4 function
> >    eal: add initial support for RISC-V architecture
> >    test/cpuflags: add test for RISC-V cpu flag
> >
> > Stanislaw Kardach (8):
> >    examples/l3fwd: fix scalar LPM compilation
> >    net/ixgbe: enable vector stubs for RISC-V
> >    net/memif: set memfd syscall ID on RISC-V
> >    net/tap: set BPF syscall ID for RISC-V
> >    examples/l3fwd: enable RISC-V operation
> >    test/ring: disable problematic tests for RISC-V
> >    devtools: add RISC-V to test-meson-builds.sh
> >    test/hash: report non HTM numbers for single r/w
> >
> >   MAINTAINERS                                   |   6 +
> >   app/test/test_cpuflags.c                      |  81 ++++++++++
> >   app/test/test_hash_readwrite.c                |   8 +-
> >   app/test/test_ring.c                          |   8 +
> >   app/test/test_xmmt_ops.h                      |  16 ++
> >   config/meson.build                            |   2 +
> >   config/riscv/meson.build                      | 148 ++++++++++++++++++
> >   config/riscv/riscv64_linux_gcc                |  17 ++
> >   config/riscv/riscv64_sifive_u740_linux_gcc    |  19 +++
> >   devtools/test-meson-builds.sh                 |   6 +
> >   doc/guides/contributing/design.rst            |   2 +-
> >   .../linux_gsg/cross_build_dpdk_for_riscv.rst  | 125 +++++++++++++++
> >   doc/guides/linux_gsg/index.rst                |   1 +
> >   doc/guides/nics/features.rst                  |   5 +
> >   doc/guides/nics/features/default.ini          |   1 +
> >   doc/guides/nics/features/ixgbe.ini            |   1 +
> >   doc/guides/rel_notes/release_22_07.rst        |  29 ++++
> >   drivers/net/i40e/meson.build                  |   6 +
> >   drivers/net/ixgbe/ixgbe_rxtx.c                |   4 +-
> >   drivers/net/memif/rte_eth_memif.h             |   2 +
> >   drivers/net/tap/tap_bpf.h                     |   2 +
> >   examples/l3fwd/l3fwd_em.c                     |   8 +
> >   examples/l3fwd/l3fwd_fib.c                    |   2 +
> >   examples/l3fwd/l3fwd_lpm.c                    |   2 +-
> >   lib/eal/riscv/include/meson.build             |  23 +++
> >   lib/eal/riscv/include/rte_atomic.h            |  52 ++++++
> >   lib/eal/riscv/include/rte_byteorder.h         |  44 ++++++
> >   lib/eal/riscv/include/rte_cpuflags.h          |  55 +++++++
> >   lib/eal/riscv/include/rte_cycles.h            | 103 ++++++++++++
> >   lib/eal/riscv/include/rte_io.h                |  21 +++
> >   lib/eal/riscv/include/rte_mcslock.h           |  18 +++
> >   lib/eal/riscv/include/rte_memcpy.h            |  63 ++++++++
> >   lib/eal/riscv/include/rte_pause.h             |  31 ++++
> >   lib/eal/riscv/include/rte_pflock.h            |  17 ++
> >   lib/eal/riscv/include/rte_power_intrinsics.h  |  22 +++
> >   lib/eal/riscv/include/rte_prefetch.h          |  50 ++++++
> >   lib/eal/riscv/include/rte_rwlock.h            |  44 ++++++
> >   lib/eal/riscv/include/rte_spinlock.h          |  67 ++++++++
> >   lib/eal/riscv/include/rte_ticketlock.h        |  21 +++
> >   lib/eal/riscv/include/rte_vect.h              |  55 +++++++
> >   lib/eal/riscv/meson.build                     |  11 ++
> >   lib/eal/riscv/rte_cpuflags.c                  | 122 +++++++++++++++
> >   lib/eal/riscv/rte_cycles.c                    |  77 +++++++++
> >   lib/eal/riscv/rte_hypervisor.c                |  13 ++
> >   lib/eal/riscv/rte_power_intrinsics.c          |  56 +++++++
> >   lib/lpm/meson.build                           |   1 +
> >   lib/lpm/rte_lpm.h                             |   4 +-
> >   lib/lpm/rte_lpm_scalar.h                      | 122 +++++++++++++++
> >   meson.build                                   |   2 +
> >   49 files changed, 1588 insertions(+), 7 deletions(-)
> >   create mode 100644 config/riscv/meson.build
> >   create mode 100644 config/riscv/riscv64_linux_gcc
> >   create mode 100644 config/riscv/riscv64_sifive_u740_linux_gcc
> >   create mode 100644 doc/guides/linux_gsg/cross_build_dpdk_for_riscv.rst
> >   create mode 100644 lib/eal/riscv/include/meson.build
> >   create mode 100644 lib/eal/riscv/include/rte_atomic.h
> >   create mode 100644 lib/eal/riscv/include/rte_byteorder.h
> >   create mode 100644 lib/eal/riscv/include/rte_cpuflags.h
> >   create mode 100644 lib/eal/riscv/include/rte_cycles.h
> >   create mode 100644 lib/eal/riscv/include/rte_io.h
> >   create mode 100644 lib/eal/riscv/include/rte_mcslock.h
> >   create mode 100644 lib/eal/riscv/include/rte_memcpy.h
> >   create mode 100644 lib/eal/riscv/include/rte_pause.h
> >   create mode 100644 lib/eal/riscv/include/rte_pflock.h
> >   create mode 100644 lib/eal/riscv/include/rte_power_intrinsics.h
> >   create mode 100644 lib/eal/riscv/include/rte_prefetch.h
> >   create mode 100644 lib/eal/riscv/include/rte_rwlock.h
> >   create mode 100644 lib/eal/riscv/include/rte_spinlock.h
> >   create mode 100644 lib/eal/riscv/include/rte_ticketlock.h
> >   create mode 100644 lib/eal/riscv/include/rte_vect.h
> >   create mode 100644 lib/eal/riscv/meson.build
> >   create mode 100644 lib/eal/riscv/rte_cpuflags.c
> >   create mode 100644 lib/eal/riscv/rte_cycles.c
> >   create mode 100644 lib/eal/riscv/rte_hypervisor.c
> >   create mode 100644 lib/eal/riscv/rte_power_intrinsics.c
> >   create mode 100644 lib/lpm/rte_lpm_scalar.h
> >
>
  
Heinrich Schuchardt May 12, 2022, 9:46 a.m. UTC | #14
On 5/12/22 10:35, Stanisław Kardach wrote:
> On Thu, May 12, 2022 at 10:04 AM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> On 5/5/22 19:29, Stanislaw Kardach wrote:
>>> This patchset adds support for building and running DPDK on 64bit RISC-V
>>> architecture. The initial support targets rv64gc (rv64imafdc) ISA and
>>> was tested on SiFive Unmatched development board with the Freedom U740
>>> SoC running Linux (freedom-u-sdk based kernel).
>>> I have tested this codebase using DPDK unit and perf tests as well as
>>> test-pmd, l2fwd and l3fwd examples.
>>> The NIC attached to the DUT was Intel X520-DA2 which uses ixgbe PMD.
>>> On the UIO side, since U740 does not have an IOMMU, I've used igb_uio,
>>> uio_pci_generic and vfio-pci noiommu drivers.
>>>
>>> Commits 1-2 fix small issues which are encountered if a given platform
>>>      does not support any vector operations (which is the case with U740).
>>> Commit 3 introduces EAL and build system support for RISC-V architecture
>>>      as well as documentation updates.
>>> Commits 4-7 add missing defines and stubs to enable RISC-V operation in
>>>      non-EAL parts.
>>> Commit 8 adds RISC-V specific cpuflags test.
>>> Commit 9 works around a bug in the current GCC in test_ring compiled
>>>      with -O0 or -Og.
>>> Commit 10 adds RISC-V testing to test-meson-builds.sh automatically
>>>      iterating over cross-compile config files (currently present for
>>>      generic rv64gc and SiFive U740).
>>> Commit 11 extends hash r/w perf test by displaying both HTM and non-HTM
>>>      measurements. This is an extraneous commit which is not directly
>>>      needed for RISC-V support but was noticed when we have started
>>>      gathering test results. If needed, I can submit it separately.
>>>
>>> I appreciate Your comments and feedback.
>>>
>>> Best Regards,
>>> Stanislaw Kardach
>>
>> On an SiFive Unmatched board with Ubuntu Jammy I have been compiling
>> DPDK origin/main (36db4a1ad464) with this series and the test patches
>> that you split off.
> Thank you for testing!
>>
>> The fast tests are either skipped or succeed. But I have seen errors for
>> the performance tests:
>>
>> 4 1GiB hugepages defined
>> no network devices bound
>> vfio_pci loaded (no IOMMU)
>>
>> sudo meson -t 30
>>
>> 129/173 DPDK:perf-tests / pmd_perf_autotest FAIL 7.85s (exit status 255
>> or signal 127 SIGinvalid)
>>   >>> DPDK_TEST=pmd_perf_autotest MALLOC_PERTURB_=135
>> build/app/test/dpdk-test
> This test I believe requires at least a single NIC port. Conversely
> fast-tests seem to fail if a device is bound. DPDK CI doesn't bind any
> on fast-tests, I'm not sure if perf-tests suite is run there.
> On my local x86 machine perf-tests suite fails when no device is bound too.
> 
>> 136/173 DPDK:perf-tests / ipsec_perf_autotest FAIL 4.47s (exit status
>> 255 or signal 127 SIGinvalid)
>>   >>> MALLOC_PERTURB_=181 DPDK_TEST=ipsec_perf_autotest
>> build/app/test/dpdk-test
> On my unmatched I don't have openssl compiled so most crypto tests are
> skipped (since no openssl is installed), aside of this one and it
> indeed fails. I'm not sure whether it's a matter of a lack of a guard
> to skip this test if there's no crypto backend built or if it's a real
> issue.
> I admit I haven't tried crypto libraries with RISC-V openssl as
> there's no real HW offloading on Unmatched.
> 
>> 160/173 DPDK:driver-tests / link_bonding_mode4_autotest FAIL 31.80s
>> killed by signal 11 SIGSEGV
>>   >>> MALLOC_PERTURB_=62 DPDK_TEST=link_bonding_mode4_autotest
>> build/app/test/dpdk-test
> I use -t40 when testing on Unmatched. Unfortunately lots of the tests
> hit timeouts.
> 
>>
>> These are the patches used:
>>
>> # [PATCH 1/1] lpm: add a scalar version of lookupx4 function
>>     https://inbox.dpdk.org/dev/20220510115824.457885-1-kda@semihalf.com/
>> # [PATCH 1/1] examples/l3fwd: fix scalar LPM compilation
>>     https://inbox.dpdk.org/dev/20220510115844.458009-1-kda@semihalf.com/
>> # [PATCH 1/1] test/hash: report non HTM numbers for single r/w
>>     https://inbox.dpdk.org/dev/20220510115734.457718-1-kda@semihalf.com/
>> # [PATCH 1/1] test/ring: remove excessive inlining
>>     https://inbox.dpdk.org/dev/20220510115758.457794-1-kda@semihalf.com/
>> # [PATCH v3 0/8] Introduce support for RISC-V architecture
>>     https://inbox.dpdk.org/dev/20220510154849.530872-1-kda@semihalf.com/
>> # [PATCH 1/1] drivers: define OPENSSL_API_COMPAT
>>
>> https://inbox.dpdk.org/dev/20220510150635.61975-1-heinrich.schuchardt@canonical.com/
>>
>> Running sudo app/test/dpdk-test does not show the errors:
>>
>> RTE>>pmd_perf_autotest
>> Start PMD RXTX cycles cost test.
>> At least 1 port(s) used for perf. test
>> Test Failed
>> RTE>>ipsec_perf_autotest
>> USER1: rte_ipsec_pkt_crypto_prepare fail
>> Test Failed
>> RTE>>link_bonding_mode4_autotest
>> ...
>> Test OK
>> RTE>
>>
>> Best regards
>>
>> Heinrich+

The above failures for performance tests without a bound NIC can be 
reproduced on x86_64. So this is not RISC-V specific.

With both ports of an Intel Corporation Ethernet Controller 10-Gigabit 
X540-AT2 bound to the sfio-pci driver some fast tests fail on the 
Unmatched board:

  16/173 DPDK:fast-tests / eal_flags_n_opt_autotest FAIL 5.54s (exit 
status 255 or signal 127 SIGinvalid)
 >>> DPDK_TEST=eal_flags_n_opt_autotest MALLOC_PERTURB_=31 
build/app/test/dpdk-test --file-prefix=eal_flags_n_opt_autotest
  18/173 DPDK:fast-tests / eal_flags_no_huge_autotest FAIL 5.31s (exit 
status 255 or signal 127 SIGinvalid)
 >>> DPDK_TEST=eal_flags_no_huge_autotest MALLOC_PERTURB_=150 
build/app/test/dpdk-test --file-prefix=eal_flags_no_huge_autotest
  21/173 DPDK:fast-tests / eal_flags_vdev_opt_autotest FAIL 5.38s (exit 
status 255 or signal 127 SIGinvalid)
 >>> MALLOC_PERTURB_=71 DPDK_TEST=eal_flags_vdev_opt_autotest 
build/app/test/dpdk-test --file-prefix=eal_flags_vdev_opt_autotest
  25/173 DPDK:fast-tests / eal_flags_misc_autotest FAIL 5.58s (exit 
status 255 or signal 127 SIGinvalid)
 >>> DPDK_TEST=eal_flags_misc_autotest MALLOC_PERTURB_=130 
build/app/test/dpdk-test --file-prefix=eal_flags_misc_autotest

The eal* tests succeed on x86_64 with a bound Intel I211 NIC.

If the eal* tests are run depends on the installed Linux packages. I 
have used 'apt build-dep dpdk' on Ubuntu Jammy to install prerequisite 
packages before building DPDK.

Best regards

Heinrich

>>
>>>
>>> NOTE: This work was sponsored by StarFive and SiFive which is signified by
>>>      "Sponsored-by:" sign-offs in each commit message. After discussing it
>>>      with Thomas Monjalon it seemed a better choice than "Suggested-by" which
>>>      does not fully convey the nature of involvement. However it makes
>>>      Linux checkpatch unhappy so I'm not sure if I shouldn't change the
>>>      sign-offs.
>>>
>>> NOTE2: I have added maintainers for each commit based on MAINTAINERS file.
>>>      However some modules (l3fwd, net/tap and cpuflags unit tests) do not have
>>>      any maintainers assigned, hence I've targeted dev@dpdk.org mailing list as
>>>      if it was a commit adding new files.
>>>
>>> Michal Mazurek (3):
>>>     lpm: add a scalar version of lookupx4 function
>>>     eal: add initial support for RISC-V architecture
>>>     test/cpuflags: add test for RISC-V cpu flag
>>>
>>> Stanislaw Kardach (8):
>>>     examples/l3fwd: fix scalar LPM compilation
>>>     net/ixgbe: enable vector stubs for RISC-V
>>>     net/memif: set memfd syscall ID on RISC-V
>>>     net/tap: set BPF syscall ID for RISC-V
>>>     examples/l3fwd: enable RISC-V operation
>>>     test/ring: disable problematic tests for RISC-V
>>>     devtools: add RISC-V to test-meson-builds.sh
>>>     test/hash: report non HTM numbers for single r/w
>>>
>>>    MAINTAINERS                                   |   6 +
>>>    app/test/test_cpuflags.c                      |  81 ++++++++++
>>>    app/test/test_hash_readwrite.c                |   8 +-
>>>    app/test/test_ring.c                          |   8 +
>>>    app/test/test_xmmt_ops.h                      |  16 ++
>>>    config/meson.build                            |   2 +
>>>    config/riscv/meson.build                      | 148 ++++++++++++++++++
>>>    config/riscv/riscv64_linux_gcc                |  17 ++
>>>    config/riscv/riscv64_sifive_u740_linux_gcc    |  19 +++
>>>    devtools/test-meson-builds.sh                 |   6 +
>>>    doc/guides/contributing/design.rst            |   2 +-
>>>    .../linux_gsg/cross_build_dpdk_for_riscv.rst  | 125 +++++++++++++++
>>>    doc/guides/linux_gsg/index.rst                |   1 +
>>>    doc/guides/nics/features.rst                  |   5 +
>>>    doc/guides/nics/features/default.ini          |   1 +
>>>    doc/guides/nics/features/ixgbe.ini            |   1 +
>>>    doc/guides/rel_notes/release_22_07.rst        |  29 ++++
>>>    drivers/net/i40e/meson.build                  |   6 +
>>>    drivers/net/ixgbe/ixgbe_rxtx.c                |   4 +-
>>>    drivers/net/memif/rte_eth_memif.h             |   2 +
>>>    drivers/net/tap/tap_bpf.h                     |   2 +
>>>    examples/l3fwd/l3fwd_em.c                     |   8 +
>>>    examples/l3fwd/l3fwd_fib.c                    |   2 +
>>>    examples/l3fwd/l3fwd_lpm.c                    |   2 +-
>>>    lib/eal/riscv/include/meson.build             |  23 +++
>>>    lib/eal/riscv/include/rte_atomic.h            |  52 ++++++
>>>    lib/eal/riscv/include/rte_byteorder.h         |  44 ++++++
>>>    lib/eal/riscv/include/rte_cpuflags.h          |  55 +++++++
>>>    lib/eal/riscv/include/rte_cycles.h            | 103 ++++++++++++
>>>    lib/eal/riscv/include/rte_io.h                |  21 +++
>>>    lib/eal/riscv/include/rte_mcslock.h           |  18 +++
>>>    lib/eal/riscv/include/rte_memcpy.h            |  63 ++++++++
>>>    lib/eal/riscv/include/rte_pause.h             |  31 ++++
>>>    lib/eal/riscv/include/rte_pflock.h            |  17 ++
>>>    lib/eal/riscv/include/rte_power_intrinsics.h  |  22 +++
>>>    lib/eal/riscv/include/rte_prefetch.h          |  50 ++++++
>>>    lib/eal/riscv/include/rte_rwlock.h            |  44 ++++++
>>>    lib/eal/riscv/include/rte_spinlock.h          |  67 ++++++++
>>>    lib/eal/riscv/include/rte_ticketlock.h        |  21 +++
>>>    lib/eal/riscv/include/rte_vect.h              |  55 +++++++
>>>    lib/eal/riscv/meson.build                     |  11 ++
>>>    lib/eal/riscv/rte_cpuflags.c                  | 122 +++++++++++++++
>>>    lib/eal/riscv/rte_cycles.c                    |  77 +++++++++
>>>    lib/eal/riscv/rte_hypervisor.c                |  13 ++
>>>    lib/eal/riscv/rte_power_intrinsics.c          |  56 +++++++
>>>    lib/lpm/meson.build                           |   1 +
>>>    lib/lpm/rte_lpm.h                             |   4 +-
>>>    lib/lpm/rte_lpm_scalar.h                      | 122 +++++++++++++++
>>>    meson.build                                   |   2 +
>>>    49 files changed, 1588 insertions(+), 7 deletions(-)
>>>    create mode 100644 config/riscv/meson.build
>>>    create mode 100644 config/riscv/riscv64_linux_gcc
>>>    create mode 100644 config/riscv/riscv64_sifive_u740_linux_gcc
>>>    create mode 100644 doc/guides/linux_gsg/cross_build_dpdk_for_riscv.rst
>>>    create mode 100644 lib/eal/riscv/include/meson.build
>>>    create mode 100644 lib/eal/riscv/include/rte_atomic.h
>>>    create mode 100644 lib/eal/riscv/include/rte_byteorder.h
>>>    create mode 100644 lib/eal/riscv/include/rte_cpuflags.h
>>>    create mode 100644 lib/eal/riscv/include/rte_cycles.h
>>>    create mode 100644 lib/eal/riscv/include/rte_io.h
>>>    create mode 100644 lib/eal/riscv/include/rte_mcslock.h
>>>    create mode 100644 lib/eal/riscv/include/rte_memcpy.h
>>>    create mode 100644 lib/eal/riscv/include/rte_pause.h
>>>    create mode 100644 lib/eal/riscv/include/rte_pflock.h
>>>    create mode 100644 lib/eal/riscv/include/rte_power_intrinsics.h
>>>    create mode 100644 lib/eal/riscv/include/rte_prefetch.h
>>>    create mode 100644 lib/eal/riscv/include/rte_rwlock.h
>>>    create mode 100644 lib/eal/riscv/include/rte_spinlock.h
>>>    create mode 100644 lib/eal/riscv/include/rte_ticketlock.h
>>>    create mode 100644 lib/eal/riscv/include/rte_vect.h
>>>    create mode 100644 lib/eal/riscv/meson.build
>>>    create mode 100644 lib/eal/riscv/rte_cpuflags.c
>>>    create mode 100644 lib/eal/riscv/rte_cycles.c
>>>    create mode 100644 lib/eal/riscv/rte_hypervisor.c
>>>    create mode 100644 lib/eal/riscv/rte_power_intrinsics.c
>>>    create mode 100644 lib/lpm/rte_lpm_scalar.h
>>>
>>
  
Stanislaw Kardach May 12, 2022, 1:56 p.m. UTC | #15
On Thu, May 12, 2022 at 11:46 AM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
<snip>
> The above failures for performance tests without a bound NIC can be
> reproduced on x86_64. So this is not RISC-V specific.
>
> With both ports of an Intel Corporation Ethernet Controller 10-Gigabit
> X540-AT2 bound to the sfio-pci driver some fast tests fail on the
> Unmatched board:
>
>   16/173 DPDK:fast-tests / eal_flags_n_opt_autotest FAIL 5.54s (exit
> status 255 or signal 127 SIGinvalid)
>  >>> DPDK_TEST=eal_flags_n_opt_autotest MALLOC_PERTURB_=31
> build/app/test/dpdk-test --file-prefix=eal_flags_n_opt_autotest
>   18/173 DPDK:fast-tests / eal_flags_no_huge_autotest FAIL 5.31s (exit
> status 255 or signal 127 SIGinvalid)
>  >>> DPDK_TEST=eal_flags_no_huge_autotest MALLOC_PERTURB_=150
> build/app/test/dpdk-test --file-prefix=eal_flags_no_huge_autotest
>   21/173 DPDK:fast-tests / eal_flags_vdev_opt_autotest FAIL 5.38s (exit
> status 255 or signal 127 SIGinvalid)
>  >>> MALLOC_PERTURB_=71 DPDK_TEST=eal_flags_vdev_opt_autotest
> build/app/test/dpdk-test --file-prefix=eal_flags_vdev_opt_autotest
>   25/173 DPDK:fast-tests / eal_flags_misc_autotest FAIL 5.58s (exit
> status 255 or signal 127 SIGinvalid)
>  >>> DPDK_TEST=eal_flags_misc_autotest MALLOC_PERTURB_=130
> build/app/test/dpdk-test --file-prefix=eal_flags_misc_autotest
>
> The eal* tests succeed on x86_64 with a bound Intel I211 NIC.
The common element of those tests is --no-huge flag. The reason why
they are failing is a combination of --no-huge and a lack of IOMMU
(VT-d on Intel). Lack of IOMMU means that DMA will be done using
physical addresses (RTE_IOVA_PA), however that implicitly requires
hugepages to function. More details are in [1]. That mail also shows
how to replicate the same issue on x86 even without a device bound to
UIO.
The following fails:
  $ ./app/test/dpdk-test --iova-mode=pa --no-huge
But this works:
  sudo ./app/test/dpdk-test --iova-mode=pa
However fixing it is not as straightforward as I thought (some tests
are not run where they should - [3]). As I explain in [2], the PCI bus
probing may force RTE_IOVA_PA on no-IOMMU platforms. That gives the
same effect as passing `--iova-mode=pa`. If no PCI device is bound,
then DPDK will be in RTE_IOVA_DC mode and that works just fine with
--no-huge.

As DPDK CI does not bind any device for fast-tests I've concluded that
it is the way those tests should be run. If not then I'm not sure how
should I handle --iova-mode=pa + --no-huge in general. Any
suggestions?

[1] http://mails.dpdk.org/archives/dev/2021-June/210773.html
[2] http://mails.dpdk.org/archives/dev/2021-June/211146.html
[3] https://patches.dpdk.org/project/dpdk/patch/20210604141601.275430-4-kda@semihalf.com/

>
> If the eal* tests are run depends on the installed Linux packages. I
> have used 'apt build-dep dpdk' on Ubuntu Jammy to install prerequisite
> packages before building DPDK.
>
> Best regards
>
> Heinrich
>
  
Heinrich Schuchardt May 12, 2022, 9:06 p.m. UTC | #16
On 5/12/22 15:56, Stanisław Kardach wrote:
> On Thu, May 12, 2022 at 11:46 AM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> <snip>
>> The above failures for performance tests without a bound NIC can be
>> reproduced on x86_64. So this is not RISC-V specific.
>>
>> With both ports of an Intel Corporation Ethernet Controller 10-Gigabit
>> X540-AT2 bound to the sfio-pci driver some fast tests fail on the
>> Unmatched board:
>>
>>    16/173 DPDK:fast-tests / eal_flags_n_opt_autotest FAIL 5.54s (exit
>> status 255 or signal 127 SIGinvalid)
>>   >>> DPDK_TEST=eal_flags_n_opt_autotest MALLOC_PERTURB_=31
>> build/app/test/dpdk-test --file-prefix=eal_flags_n_opt_autotest
>>    18/173 DPDK:fast-tests / eal_flags_no_huge_autotest FAIL 5.31s (exit
>> status 255 or signal 127 SIGinvalid)
>>   >>> DPDK_TEST=eal_flags_no_huge_autotest MALLOC_PERTURB_=150
>> build/app/test/dpdk-test --file-prefix=eal_flags_no_huge_autotest
>>    21/173 DPDK:fast-tests / eal_flags_vdev_opt_autotest FAIL 5.38s (exit
>> status 255 or signal 127 SIGinvalid)
>>   >>> MALLOC_PERTURB_=71 DPDK_TEST=eal_flags_vdev_opt_autotest
>> build/app/test/dpdk-test --file-prefix=eal_flags_vdev_opt_autotest
>>    25/173 DPDK:fast-tests / eal_flags_misc_autotest FAIL 5.58s (exit
>> status 255 or signal 127 SIGinvalid)
>>   >>> DPDK_TEST=eal_flags_misc_autotest MALLOC_PERTURB_=130
>> build/app/test/dpdk-test --file-prefix=eal_flags_misc_autotest
>>
>> The eal* tests succeed on x86_64 with a bound Intel I211 NIC.
> The common element of those tests is --no-huge flag. The reason why
> they are failing is a combination of --no-huge and a lack of IOMMU
> (VT-d on Intel). Lack of IOMMU means that DMA will be done using
> physical addresses (RTE_IOVA_PA), however that implicitly requires
> hugepages to function. More details are in [1]. That mail also shows
> how to replicate the same issue on x86 even without a device bound to
> UIO.
> The following fails:
>    $ ./app/test/dpdk-test --iova-mode=pa --no-huge
> But this works:
>    sudo ./app/test/dpdk-test --iova-mode=pa
> However fixing it is not as straightforward as I thought (some tests
> are not run where they should - [3]). As I explain in [2], the PCI bus
> probing may force RTE_IOVA_PA on no-IOMMU platforms. That gives the
> same effect as passing `--iova-mode=pa`. If no PCI device is bound,
> then DPDK will be in RTE_IOVA_DC mode and that works just fine with
> --no-huge.
> 
> As DPDK CI does not bind any device for fast-tests I've concluded that
> it is the way those tests should be run. If not then I'm not sure how
> should I handle --iova-mode=pa + --no-huge in general. Any
> suggestions?

Thanks for your analysis and explanation. The respective tests should 
return TEST_SKIPPED if called with an unsupported parameter combination.

According to your analysis this is not a problem specific to the RISC-V 
series but can be resolved separately.

Maybe you could add a note in doc/guides/rel_notes/known_issues.rst.

Best regards

Heinrich

> 
> [1] http://mails.dpdk.org/archives/dev/2021-June/210773.html
> [2] http://mails.dpdk.org/archives/dev/2021-June/211146.html
> [3] https://patches.dpdk.org/project/dpdk/patch/20210604141601.275430-4-kda@semihalf.com/
> 
>>
>> If the eal* tests are run depends on the installed Linux packages. I
>> have used 'apt build-dep dpdk' on Ubuntu Jammy to install prerequisite
>> packages before building DPDK.
>>
>> Best regards
>>
>> Heinrich
>>