| Message ID | 20220314082556.4035411-1-xsimonar@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] Added test cases with ovn-northd parallelization enabled | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Mar 14, 2022 at 4:26 AM Xavier Simonart <xsimonar@redhat.com> wrote: > > This will more or less double the number of test cases. > It is possible to select a reduce set of test cases using -k "keywords". > Keyword such as > dp-groups=yes > dp-groups=no > parallelization=yes > parallelization=no > ovn-northd > ovn-northd-ddlog > can be used to select a range of tests, as title is searched as well. > > For instance, to run ovn-parallelization tests, with dp-groups enabled and ddlog disabled: > make check TESTSUITEFLAGS="-k dp-groups=yes,parallelization=yes,\!ovn-northd-ddlog" > > A few additional notes > - Do not use "ovn-northd" as a keyword, or the result might be different than expected. > Even tough ovn-northd is added in all titles for the non ddlog tests, ovn-northd is also > added in title of some other tests, for both ddlog and non ddlog (e.g. "check ovn-northd and ovn-controller version pinning"). > - Running tests for both parallelization=yes and parallelization=no will not run all tests as > some tests are not run through OVN_FOR_EACH_NORTHD. Instead use for instance > make check TESTSUITEFLAGS="-k parallelization=yes" and > make check TESTSUITEFLAGS="-k \!parallelization=yes" to run all tests > > If less than four physical cores are available, then 1 socket/four cores dummy-numa > is used to excercice parallel northd > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> Acked-by: Numan Siddique <numans@ovn.org> Numan > --- > tests/ovn-macros.at | 17 ++++++++++++++--- > tests/ovs-macros.at | 6 +++++- > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index 91d2dafc8..d78315c75 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -171,7 +171,7 @@ ovn_start_northd() { > esac > > if test X$NORTHD_DUMMY_NUMA = Xyes; then > - northd_args="$northd_args --dummy-numa=\"0,0,1,1\"" > + northd_args="$northd_args --dummy-numa=\"0,0,0,0\"" > fi > > local name=${d_prefix}northd${suffix} > @@ -749,12 +749,23 @@ OVS_END_SHELL_HELPERS > > m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) > > +# Use --dummy-numa if system has low cores and we want to force parallelization > +m4_define([NORTHD_DUMMY_NUMA], > + [$(if test $(nproc) -lt 4 && test NORTHD_USE_PARALLELIZATION = yes > + then > + echo "yes" > + else > + echo "no" > + fi) > +]) > + > # Defines a versions of a test with all combinations of northd and > # datapath groups. > m4_define([OVN_FOR_EACH_NORTHD], > [m4_foreach([NORTHD_TYPE], [ovn-northd, ovn-northd-ddlog], > - [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], [$1 > -])])]) > + [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], > + [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 > +])])])]) > > # Some tests aren't prepared for dp groups to be enabled. > m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DP_GROUPS], > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at > index 634c205a8..0482b7f5b 100644 > --- a/tests/ovs-macros.at > +++ b/tests/ovs-macros.at > @@ -10,11 +10,15 @@ dnl set it as a shell variable as well. > dnl - Skip the test if it's for ovn-northd-ddlog but it didn't get built. > m4_rename([AT_SETUP], [OVS_AT_SETUP]) > m4_define([AT_SETUP], > - [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])) > + [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ -- parallelization=NORTHD_USE_PARALLELIZATION])) > m4_ifdef([NORTHD_TYPE], [[NORTHD_TYPE]=NORTHD_TYPE > ])dnl > m4_ifdef([NORTHD_USE_DP_GROUPS], [[NORTHD_USE_DP_GROUPS]=NORTHD_USE_DP_GROUPS > ])dnl > +m4_ifdef([NORTHD_USE_PARALLELIZATION], [[NORTHD_USE_PARALLELIZATION]=NORTHD_USE_PARALLELIZATION > +])dnl > +m4_ifdef([NORTHD_DUMMY_NUMA], [[NORTHD_DUMMY_NUMA]=NORTHD_DUMMY_NUMA > +])dnl > m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl > AT_SKIP_IF([test $TEST_DDLOG = no]) > ])dnl > -- > 2.31.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 3/22/22 11:06, Numan Siddique wrote: > On Mon, Mar 14, 2022 at 4:26 AM Xavier Simonart <xsimonar@redhat.com> wrote: >> >> This will more or less double the number of test cases. >> It is possible to select a reduce set of test cases using -k "keywords". >> Keyword such as >> dp-groups=yes >> dp-groups=no >> parallelization=yes >> parallelization=no >> ovn-northd >> ovn-northd-ddlog >> can be used to select a range of tests, as title is searched as well. >> >> For instance, to run ovn-parallelization tests, with dp-groups enabled and ddlog disabled: >> make check TESTSUITEFLAGS="-k dp-groups=yes,parallelization=yes,\!ovn-northd-ddlog" >> >> A few additional notes >> - Do not use "ovn-northd" as a keyword, or the result might be different than expected. >> Even tough ovn-northd is added in all titles for the non ddlog tests, ovn-northd is also >> added in title of some other tests, for both ddlog and non ddlog (e.g. "check ovn-northd and ovn-controller version pinning"). >> - Running tests for both parallelization=yes and parallelization=no will not run all tests as >> some tests are not run through OVN_FOR_EACH_NORTHD. Instead use for instance >> make check TESTSUITEFLAGS="-k parallelization=yes" and >> make check TESTSUITEFLAGS="-k \!parallelization=yes" to run all tests >> >> If less than four physical cores are available, then 1 socket/four cores dummy-numa >> is used to excercice parallel northd >> >> Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > > Acked-by: Numan Siddique <numans@ovn.org> > > Numan I pushed this to the main branch. > >> --- >> tests/ovn-macros.at | 17 ++++++++++++++--- >> tests/ovs-macros.at | 6 +++++- >> 2 files changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at >> index 91d2dafc8..d78315c75 100644 >> --- a/tests/ovn-macros.at >> +++ b/tests/ovn-macros.at >> @@ -171,7 +171,7 @@ ovn_start_northd() { >> esac >> >> if test X$NORTHD_DUMMY_NUMA = Xyes; then >> - northd_args="$northd_args --dummy-numa=\"0,0,1,1\"" >> + northd_args="$northd_args --dummy-numa=\"0,0,0,0\"" >> fi >> >> local name=${d_prefix}northd${suffix} >> @@ -749,12 +749,23 @@ OVS_END_SHELL_HELPERS >> >> m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) >> >> +# Use --dummy-numa if system has low cores and we want to force parallelization >> +m4_define([NORTHD_DUMMY_NUMA], >> + [$(if test $(nproc) -lt 4 && test NORTHD_USE_PARALLELIZATION = yes >> + then >> + echo "yes" >> + else >> + echo "no" >> + fi) >> +]) >> + >> # Defines a versions of a test with all combinations of northd and >> # datapath groups. >> m4_define([OVN_FOR_EACH_NORTHD], >> [m4_foreach([NORTHD_TYPE], [ovn-northd, ovn-northd-ddlog], >> - [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], [$1 >> -])])]) >> + [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], >> + [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 >> +])])])]) >> >> # Some tests aren't prepared for dp groups to be enabled. >> m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DP_GROUPS], >> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at >> index 634c205a8..0482b7f5b 100644 >> --- a/tests/ovs-macros.at >> +++ b/tests/ovs-macros.at >> @@ -10,11 +10,15 @@ dnl set it as a shell variable as well. >> dnl - Skip the test if it's for ovn-northd-ddlog but it didn't get built. >> m4_rename([AT_SETUP], [OVS_AT_SETUP]) >> m4_define([AT_SETUP], >> - [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])) >> + [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ -- parallelization=NORTHD_USE_PARALLELIZATION])) >> m4_ifdef([NORTHD_TYPE], [[NORTHD_TYPE]=NORTHD_TYPE >> ])dnl >> m4_ifdef([NORTHD_USE_DP_GROUPS], [[NORTHD_USE_DP_GROUPS]=NORTHD_USE_DP_GROUPS >> ])dnl >> +m4_ifdef([NORTHD_USE_PARALLELIZATION], [[NORTHD_USE_PARALLELIZATION]=NORTHD_USE_PARALLELIZATION >> +])dnl >> +m4_ifdef([NORTHD_DUMMY_NUMA], [[NORTHD_DUMMY_NUMA]=NORTHD_DUMMY_NUMA >> +])dnl >> m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl >> AT_SKIP_IF([test $TEST_DDLOG = no]) >> ])dnl >> -- >> 2.31.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 3/22/22 19:05, Mark Michelson wrote: > On 3/22/22 11:06, Numan Siddique wrote: >> On Mon, Mar 14, 2022 at 4:26 AM Xavier Simonart <xsimonar@redhat.com> >> wrote: >>> >>> This will more or less double the number of test cases. >>> It is possible to select a reduce set of test cases using -k "keywords". >>> Keyword such as >>> dp-groups=yes >>> dp-groups=no >>> parallelization=yes >>> parallelization=no >>> ovn-northd >>> ovn-northd-ddlog >>> can be used to select a range of tests, as title is searched as well. >>> >>> For instance, to run ovn-parallelization tests, with dp-groups >>> enabled and ddlog disabled: >>> make check TESTSUITEFLAGS="-k >>> dp-groups=yes,parallelization=yes,\!ovn-northd-ddlog" >>> >>> A few additional notes >>> - Do not use "ovn-northd" as a keyword, or the result might be >>> different than expected. >>> Even tough ovn-northd is added in all titles for the non ddlog >>> tests, ovn-northd is also >>> added in title of some other tests, for both ddlog and non ddlog >>> (e.g. "check ovn-northd and ovn-controller version pinning"). >>> - Running tests for both parallelization=yes and parallelization=no >>> will not run all tests as >>> some tests are not run through OVN_FOR_EACH_NORTHD. Instead use >>> for instance >>> make check TESTSUITEFLAGS="-k parallelization=yes" and >>> make check TESTSUITEFLAGS="-k \!parallelization=yes" to run all >>> tests >>> >>> If less than four physical cores are available, then 1 socket/four >>> cores dummy-numa >>> is used to excercice parallel northd >>> >>> Signed-off-by: Xavier Simonart <xsimonar@redhat.com> >> >> Acked-by: Numan Siddique <numans@ovn.org> >> >> Numan > > I pushed this to the main branch. > This actually uncovered the fact that we're incorrectly writing to the IDL from multiple threads when northd parallelization is enabled and causing issues, like: ================================================================= ==3096883==ERROR: LeakSanitizer: detected memory leaks Direct leak of 32 byte(s) in 1 object(s) allocated from: #0 0x7f9f7c7ef93f in __interceptor_malloc (/lib64/libasan.so.6+0xae93f) #1 0x65eec0 in xmalloc__ lib/util.c:137 #2 0x65eee3 in xmalloc lib/util.c:172 #3 0x59d783 in resize lib/hmap.c:100 #4 0x59db7b in hmap_expand_at lib/hmap.c:175 #5 0x62c02e in hmap_insert_at include/openvswitch/hmap.h:283 #6 0x62c02e in ovsdb_idl_txn_write__ lib/ovsdb-idl.c:3538 #7 0x63063a in ovsdb_idl_txn_write lib/ovsdb-idl.c:3596 #8 0x562ca1 in sbrec_port_binding_set_options lib/ovn-sb-idl.c:28511 #9 0x44eb3c in build_ND_RA_flows_for_lrouter_port northd/northd.c:10920 #10 0x46014e in build_lswitch_and_lrouter_iterate_by_op northd/northd.c:13530 #11 0x460686 in build_lflows_thread northd/northd.c:13592 #12 0x61097c in ovsthread_wrapper lib/ovs-thread.c:422 #13 0x7f9f7c39e298 in start_thread /usr/src/debug/glibc-2.33-20.fc34.x86_64/nptl/pthread_create.c:481 It looks like we need some additional fixes to the northd parallelization code. Regards, Dumitru
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at index 91d2dafc8..d78315c75 100644 --- a/tests/ovn-macros.at +++ b/tests/ovn-macros.at @@ -171,7 +171,7 @@ ovn_start_northd() { esac if test X$NORTHD_DUMMY_NUMA = Xyes; then - northd_args="$northd_args --dummy-numa=\"0,0,1,1\"" + northd_args="$northd_args --dummy-numa=\"0,0,0,0\"" fi local name=${d_prefix}northd${suffix} @@ -749,12 +749,23 @@ OVS_END_SHELL_HELPERS m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) +# Use --dummy-numa if system has low cores and we want to force parallelization +m4_define([NORTHD_DUMMY_NUMA], + [$(if test $(nproc) -lt 4 && test NORTHD_USE_PARALLELIZATION = yes + then + echo "yes" + else + echo "no" + fi) +]) + # Defines a versions of a test with all combinations of northd and # datapath groups. m4_define([OVN_FOR_EACH_NORTHD], [m4_foreach([NORTHD_TYPE], [ovn-northd, ovn-northd-ddlog], - [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], [$1 -])])]) + [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no], + [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1 +])])])]) # Some tests aren't prepared for dp groups to be enabled. m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DP_GROUPS], diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index 634c205a8..0482b7f5b 100644 --- a/tests/ovs-macros.at +++ b/tests/ovs-macros.at @@ -10,11 +10,15 @@ dnl set it as a shell variable as well. dnl - Skip the test if it's for ovn-northd-ddlog but it didn't get built. m4_rename([AT_SETUP], [OVS_AT_SETUP]) m4_define([AT_SETUP], - [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])) + [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ -- NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ -- dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ -- parallelization=NORTHD_USE_PARALLELIZATION])) m4_ifdef([NORTHD_TYPE], [[NORTHD_TYPE]=NORTHD_TYPE ])dnl m4_ifdef([NORTHD_USE_DP_GROUPS], [[NORTHD_USE_DP_GROUPS]=NORTHD_USE_DP_GROUPS ])dnl +m4_ifdef([NORTHD_USE_PARALLELIZATION], [[NORTHD_USE_PARALLELIZATION]=NORTHD_USE_PARALLELIZATION +])dnl +m4_ifdef([NORTHD_DUMMY_NUMA], [[NORTHD_DUMMY_NUMA]=NORTHD_DUMMY_NUMA +])dnl m4_if(NORTHD_TYPE, [ovn-northd-ddlog], [dnl AT_SKIP_IF([test $TEST_DDLOG = no]) ])dnl
This will more or less double the number of test cases. It is possible to select a reduce set of test cases using -k "keywords". Keyword such as dp-groups=yes dp-groups=no parallelization=yes parallelization=no ovn-northd ovn-northd-ddlog can be used to select a range of tests, as title is searched as well. For instance, to run ovn-parallelization tests, with dp-groups enabled and ddlog disabled: make check TESTSUITEFLAGS="-k dp-groups=yes,parallelization=yes,\!ovn-northd-ddlog" A few additional notes - Do not use "ovn-northd" as a keyword, or the result might be different than expected. Even tough ovn-northd is added in all titles for the non ddlog tests, ovn-northd is also added in title of some other tests, for both ddlog and non ddlog (e.g. "check ovn-northd and ovn-controller version pinning"). - Running tests for both parallelization=yes and parallelization=no will not run all tests as some tests are not run through OVN_FOR_EACH_NORTHD. Instead use for instance make check TESTSUITEFLAGS="-k parallelization=yes" and make check TESTSUITEFLAGS="-k \!parallelization=yes" to run all tests If less than four physical cores are available, then 1 socket/four cores dummy-numa is used to excercice parallel northd Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- tests/ovn-macros.at | 17 ++++++++++++++--- tests/ovs-macros.at | 6 +++++- 2 files changed, 19 insertions(+), 4 deletions(-)