On 5/23/2023 4:45 AM, Ruifeng Wang wrote:
-----Original Message-----
From: Burakov, Anatoly <[email protected]>
Sent: Monday, May 22, 2023 6:19 PM
To: Ruifeng Wang <[email protected]>; [email protected]
Cc: [email protected]; [email protected]; [email protected];
[email protected]; Justin
He <[email protected]>; Honnappa Nagarahalli <[email protected]>; nd
<[email protected]>
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 <[email protected]>
Sent: Monday, May 22, 2023 5:24 PM
To: Ruifeng Wang <[email protected]>; [email protected]
Cc: [email protected]; [email protected]; [email protected];
[email protected]; Justin He <[email protected]>; Honnappa
Nagarahalli <[email protected]>; nd <[email protected]>
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: [email protected]
Signed-off-by: Jia He <[email protected]>
Signed-off-by: Ruifeng Wang <[email protected]>
---
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 <[email protected]>
--
Thanks,
Anatoly
