On 5/23/2023 4:45 AM, Ruifeng Wang wrote:
-----Original Message-----
From: Burakov, Anatoly <anatoly.bura...@intel.com>
Sent: Monday, May 22, 2023 6:19 PM
To: Ruifeng Wang <ruifeng.w...@arm.com>; olivier.m...@6wind.com
Cc: dev@dpdk.org; sta...@dpdk.org; tho...@monjalon.net;
step...@networkplumber.org; Justin
He <justin...@arm.com>; Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; nd
<n...@arm.com>
Subject: Re: [PATCH] test/mbuf: fix the forked process segment fault
On 5/22/2023 10:55 AM, Ruifeng Wang wrote:
-----Original Message-----
From: Burakov, Anatoly <anatoly.bura...@intel.com>
Sent: Monday, May 22, 2023 5:24 PM
To: Ruifeng Wang <ruifeng.w...@arm.com>; olivier.m...@6wind.com
Cc: dev@dpdk.org; sta...@dpdk.org; tho...@monjalon.net;
step...@networkplumber.org; Justin He <justin...@arm.com>; Honnappa
Nagarahalli <honnappa.nagaraha...@arm.com>; nd <n...@arm.com>
Subject: Re: [PATCH] test/mbuf: fix the forked process segment fault
On 5/22/2023 7:01 AM, Ruifeng Wang wrote:
Access of any memory in the hugepage shared file-backed area will
trigger an unexpected forked child process segment fault. The root
cause is DPDK doesn't support fork model [1] (calling rte_eal_init() before
fork()).
Forked child process can't be treated as a secondary process.
Hence fix it by avoiding fork and doing verification in the main process.
[1] https://mails.dpdk.org/archives/dev/2018-July/108106.html
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Jia He <justin...@arm.com>
Signed-off-by: Ruifeng Wang <ruifeng.w...@arm.com>
---
Would this be something that a secondary process-based test could test?
That's how we test rte_panic() and other calls.
This case validates mbuf. IMO there is no need to do validation in a secondary
process.
Unit test for rte_panic() also uses fork() and could have the same issue.
In that case, rte_panic() test should be fixed as well.
My concern is that ideally, we shouldn't intentionally crash the test app if
something
goes wrong, and calling rte_panic() accomplishes just that - which is why I
suggested
running them in secondary processes instead, so that any call into rte_panic
happens
inside a secondary process, and the main test process doesn't crash even if the
test has
failed.
Agree that intentionally crashing the test app is bad.
In this patch, verification of mbuf is changed to use another API without
rte_panic().
Then the verification can be done directly in the primary. And the indirectness
of
using a secondary process is removed. Because verification will not crash the
process.
Oh,
My apologies, I did not notice that. In that case,
Acked-by: Anatoly Burakov <anatoly.bura...@intel.com>
--
Thanks,
Anatoly
