> -----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. > > -- > Thanks, > Anatoly
