[dpdk-dev] [PATCH v2 0/3] Increase test compatibility with PA IOVA

[dpdk-dev] [PATCH v2 0/3] Increase test compatibility with PA IOVA

[Stanislaw Kardach]

While working on a RISC-V port, using a HiFive Unmatched (FU740) which
does not have IOMMU (hence only RTE_IOVA_PA is available), I've noticed
that some of the EAL tests are failing because of a totally different
reason than the test itself.
Namely the --no-huge flag and --iova-mode=pa can't be used together and
EAL init fails warning about a lack of access to physical addresses.
This patchset tries to cleanup the --no-huge usage so that it doesn't
hide the real state of tests when RTE_IOVA_PA is used (i.e. on platforms
without IOMMU).

I'm proposing to skip the no-huge test for RTE_IOVA_PA environments as
it is not supported by design as well as removing no-huge usage on Linux
as it seems that it is used (along with --no-shconf) to increase the
compatibility with FreeBSD.

Please let me know if I'm missing a bigger picture with the --no-huge
and --no-shconf usage on non-FreeBSD platforms.

I'm not adding stable@dpdk.org on purpose as this does not affect any
current platform I'm aware of (at least in a production scenario).

---

V2:
- Fix checkpatch errors
- Add affected platform in the cover letter.

Stanislaw Kardach (3):
  test: disable no-huge test with PA IOVA
  test: disable no-huge where it's not necessary
  test: fix the -n unit test description

 app/test/test_eal_flags.c | 63 ++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 21 deletions(-)

-- 
2.27.0

[dpdk-dev] [PATCH v2 1/3] test: disable no-huge test with PA IOVA

[Stanislaw Kardach]

On Linux systems without IOMMU support available (be it lack of
supported IOMMU or lack of IOMMU support in kernel or explicit --no-huge
EAL parameter), the IOVA mapping will default to DMA with physical
addresses. This implicitly requires hugepage support to work (checked in
rte_eal_using_phys_addrs).
Therefore trying to run the eal_flags_no_huge_autotest in such scenario
is not a valid requirement. This issue was discovered on RISC-V arch.

To verify this even on x86 do (output from i5-10210U):

$ ./app/test/dpdk-test -m 18 --iova-mode=pa --no-huge
EAL: Detected 8 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: FATAL: Cannot use IOVA as 'PA' since physical addresses are not ...
EAL: Cannot use IOVA as 'PA' since physical addresses are not available

While doing:

$ sudo ./app/test/dpdk-test --iova-mode=pa
EAL: Detected 8 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: No available 1048576 kB hugepages reported
EAL: Probing VFIO support...
EAL: VFIO support initialized
TELEMETRY: No legacy callbacks, legacy socket not created
APP: HPET is not enabled, using TSC as default timer
RTE>>

This commit finishes the above test early with SKIP status to signify
that no-huge support is simply not available.

Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
---
 app/test/test_eal_flags.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 932fbe3d08..462dc63842 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -756,6 +756,15 @@ test_no_huge_flag(void)
 #else
 	const char * prefix = "--file-prefix=nohuge";
 #endif
+#ifdef RTE_EXEC_ENV_LINUX
+	/* EAL requires hugepages for RTE_IOVA_PA operation on linux.
+	 * The test application is run with RTE_IOVA_DC, so if at this point we
+	 * get RTE_IOVA_PA, it means that newly spawned process will also get
+	 * it.
+	 */
+	if (rte_eal_iova_mode() == RTE_IOVA_PA)
+		return TEST_SKIPPED;
+#endif
 
 	/* With --no-huge */
 	const char *argv1[] = {prgname, prefix, no_huge};
-- 
2.27.0

[dpdk-dev] [PATCH v2 2/3] test: disable no-huge where it's not necessary

[Stanislaw Kardach]

In tests where no-shconf flag is used, no-huge is also passed due to
compatibility with FreeBSD system, as described in: b5d878e6d.
However on Linux systems with RTE_IOVA_PA (lack of or an incompatible
IOMMU) this causes issues since hugepages are required by EAL.
Therefore replace all occurrences of no_huge which don't actually test
the no-huge logic with a execution environment conditional
no_huge_compat to indicate that it is passed as a compatibility flag,
not as a requirement for a test itself.

Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
Fixes: b5d878e6db56 ("test: fix EAL flags autotest on FreeBSD")
Cc: anatoly.burakov@intel.com
---
 app/test/test_eal_flags.c | 50 ++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 462dc63842..e2248a5d9a 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -29,6 +29,17 @@
 #define mp_flag "--proc-type=secondary"
 #define no_hpet "--no-hpet"
 #define no_huge "--no-huge"
+/* FreeBSD does not support running multiple primary processes, hence for tests
+ * requiring no-shconf, no-huge is also required.
+ * On Linux on the other hand no-huge is not needed so don't pass it as it
+ * would break cases when IOMMU is not able to provide IOVA translation
+ * (rte_eal_iova_mode() == RTE_IOVA_PA).
+ */
+#ifdef RTE_EXEC_ENV_LINUX
+#define no_huge_compat ""
+#else
+#define no_huge_compat no_huge
+#endif
 #define no_shconf "--no-shconf"
 #define allow "--allow"
 #define vdev "--vdev"
@@ -354,18 +365,18 @@ test_invalid_vdev_flag(void)
 #endif
 
 	/* Test with invalid vdev option */
-	const char *vdevinval[] = {prgname, prefix, no_huge,
-				vdev, "eth_dummy"};
+	const char * const vdevinval[] = {prgname, prefix, no_huge_compat,
+					  vdev, "eth_dummy"};
 
 	/* Test with valid vdev option */
-	const char *vdevval1[] = {prgname, prefix, no_huge,
-	vdev, "net_ring0"};
+	const char * const vdevval1[] = {prgname, prefix, no_huge_compat,
+					 vdev, "net_ring0"};
 
-	const char *vdevval2[] = {prgname, prefix, no_huge,
-	vdev, "net_ring0,args=test"};
+	const char * const vdevval2[] = {prgname, prefix, no_huge_compat,
+					 vdev, "net_ring0,args=test"};
 
-	const char *vdevval3[] = {prgname, prefix, no_huge,
-	vdev, "net_ring0,nodeaction=r1:0:CREATE"};
+	const char * const vdevval3[] = {prgname, prefix, no_huge_compat,
+		vdev, "net_ring0,nodeaction=r1:0:CREATE"};
 
 	if (launch_proc(vdevinval) == 0) {
 		printf("Error - process did run ok with invalid "
@@ -674,19 +685,20 @@ test_invalid_n_flag(void)
 #endif
 
 	/* -n flag but no value */
-	const char *argv1[] = { prgname, prefix, no_huge, no_shconf,
-				"-n"};
+	const char * const argv1[] = { prgname, prefix, no_huge_compat,
+				       no_shconf, "-n"};
 	/* bad numeric value */
-	const char *argv2[] = { prgname, prefix, no_huge, no_shconf,
-				"-n", "e" };
+	const char * const argv2[] = { prgname, prefix, no_huge_compat,
+				       no_shconf, "-n", "e" };
 	/* zero is invalid */
-	const char *argv3[] = { prgname, prefix, no_huge, no_shconf,
-				"-n", "0" };
+	const char * const argv3[] = { prgname, prefix, no_huge_compat,
+				       no_shconf, "-n", "0" };
 	/* sanity test - check with good value */
-	const char *argv4[] = { prgname, prefix, no_huge, no_shconf,
-				"-n", "2" };
+	const char * const argv4[] = { prgname, prefix, no_huge_compat,
+				       no_shconf, "-n", "2" };
 	/* sanity test - check with no -n flag */
-	const char *argv5[] = { prgname, prefix, no_huge, no_shconf};
+	const char * const argv5[] = { prgname, prefix, no_huge_compat,
+				       no_shconf};
 
 	if (launch_proc(argv1) == 0
 			|| launch_proc(argv2) == 0
@@ -878,7 +890,7 @@ test_misc_flags(void)
 	const char *argv5[] = {prgname, prefix, mp_flag, "--syslog", "error"};
 	/* With no-sh-conf, also use no-huge to ensure this test runs on BSD */
 	const char *argv6[] = {prgname, "-m", DEFAULT_MEM_SIZE,
-			no_shconf, nosh_prefix, no_huge};
+			no_shconf, nosh_prefix, no_huge_compat};
 
 	/* With --huge-dir */
 	const char *argv7[] = {prgname, "-m", DEFAULT_MEM_SIZE,
@@ -920,7 +932,7 @@ test_misc_flags(void)
 
 	/* With process type as auto-detect with no-shconf */
 	const char * const argv17[] = {prgname, "--proc-type=auto",
-			no_shconf, nosh_prefix, no_huge};
+			no_shconf, nosh_prefix, no_huge_compat};
 
 	/* With process type as --create-uio-dev flag */
 	const char * const argv18[] = {prgname, "--file-prefix=uiodev",
-- 
2.27.0

[dpdk-dev] [PATCH v2 3/3] test: fix the -n unit test description

[Stanislaw Kardach]

When -n argument became optional, the test logic was fixed (by
1e0b51fd4) but the comment indicating why --no-huge and --no-shconf are
used was not changed.
Today those flags are used for compatibility with FreeBSD (see
b5d878e6d), so change the comment to reflect that.

Signed-off-by: Stanislaw Kardach <kda@semihalf.com>

Fixes: b5d878e6db56 ("test: fix EAL flags autotest on FreeBSD")
Cc: anatoly.burakov@intel.com
Fixes: 1e0b51fd4b75 ("app/test: fix unit test for option -n")
Cc: pablo.de.lara.guarch@intel.com
---
 app/test/test_eal_flags.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index e2248a5d9a..b1ab87cf8d 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -666,8 +666,8 @@ test_main_lcore_flag(void)
 /*
  * Test that the app doesn't run with invalid -n flag option.
  * Final test ensures it does run with valid options as sanity check
- * Since -n is not compulsory for MP, we instead use --no-huge and --no-shconf
- * flags.
+ * For compatibility with BSD use --no-huge and --no-shconf flags as we need to
+ * run a primary process.
  */
 static int
 test_invalid_n_flag(void)
-- 
2.27.0

Re: [dpdk-dev] [PATCH v2 0/3] Increase test compatibility with PA IOVA

[Stanislaw Kardach]

On Fri, Jun 04, 2021 at 04:15:58PM +0200, Stanislaw Kardach wrote:
> Please let me know if I'm missing a bigger picture with the --no-huge
> and --no-shconf usage on non-FreeBSD platforms.
> 
After looking at this issue again I'm abandoning this patchset because
it seems wrong for 2 reasons:

1. --iova-mode=pa and --no-huge combination is still unsupported but
   detecting such situation is not straightforward in the
   eal_flags_no_huge_autotest. That is because it is not only a product
   of user input but IOMMU requirements (if any), bus requirements and
   PA map availability. If a PCI device is bound to UIO and no IOMMU is
   avilable, then bus requests PA mapping. However if there's no UIO
   device, PCI reports DC and therefore decision relies on PA map
   availability. If it's not available, VA will be chosen. This changes
   between the parent dpdk-test binary being run (available->PA) and the
   child processes being spawned by the test (not-available->VA).
   However rte_eal_iova_mode() only reports the final IOVA mode. Even if
   in the parent process it is PA, it doesn't mean it will be so in
   the child process. So test will be skipped when it shouldn't.
2. Because of the above, the same test can pass on a non-IOMMU platform
   if no PCI device is bound to UIO while it would fail if there is a
   PCI device bound to UIO. It seems that the silent assumption is that
   fast-tests should be run without any UIO devices (or basically with
   all buses requestin DC IOVA mode).

Sorry for any confusion with this patchset.

-- 
Best Regards,
Stanislaw Kardach