23/05/2023 12:12, Burakov, Anatoly: > 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>
Applied, thanks.
