| Message ID | 20221205084122.2554912-2-amorenoz@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v1,1/2] rculist: use rculist_back_protected to access prev | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Mon, Dec 5, 2022 at 3:41 AM Adrian Moreno <amorenoz@redhat.com> wrote: > > Low test coverage on this area caused some errors to remain unnoticed. > Add basic functional test of rculist. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> Looks good! Triggers a warning if the preceding patch hasn't been applied as intended. Acked-by: Mike Pattrick <mkp@redhat.com> > --- > tests/automake.mk | 1 + > tests/library.at | 5 ++ > tests/test-rculist.c | 205 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 211 insertions(+) > create mode 100644 tests/test-rculist.c > > diff --git a/tests/automake.mk b/tests/automake.mk > index d509cf935..88f97b8b7 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -474,6 +474,7 @@ tests_ovstest_SOURCES = \ > tests/test-packets.c \ > tests/test-random.c \ > tests/test-rcu.c \ > + tests/test-rculist.c \ > tests/test-reconnect.c \ > tests/test-rstp.c \ > tests/test-sflow.c \ > diff --git a/tests/library.at b/tests/library.at > index bafb28277..164ae789d 100644 > --- a/tests/library.at > +++ b/tests/library.at > @@ -27,6 +27,11 @@ AT_CHECK([ovstest test-hindex], [0], [..................... > ]) > AT_CLEANUP > > +AT_SETUP([test rcu linked lists]) > +AT_CHECK([ovstest test-rculist], [0], [..... > +]) > +AT_CLEANUP > + > AT_SETUP([cuckoo hash]) > AT_KEYWORDS([cmap]) > AT_CHECK([ovstest test-cmap check 1], [0], [... > diff --git a/tests/test-rculist.c b/tests/test-rculist.c > new file mode 100644 > index 000000000..49fe434ff > --- /dev/null > +++ b/tests/test-rculist.c > @@ -0,0 +1,205 @@ > +#include <config.h> > +#undef NDEBUG > +#include <unistd.h> > + > +#include "ovstest.h" > +#include "rculist.h" > +#include "openvswitch/list.h" > +#include "ovs-thread.h" > +#include "random.h" > +#include "util.h" > + > +enum { MAX_ELEMS = 10, MAX_CHECKS = 200 }; > + > +/* Sample list element. */ > +struct element { > + int value; > + struct rculist node; > +}; > + > +/* Continuously check the integrity of the list until it's empty. */ > +static void * > +checker_main(void *aux) > +{ > + struct element *elem; > + struct rculist *list = (struct rculist *) aux; > + bool checked = false; > + > + for (int i = 0; i < MAX_CHECKS; i++) { > + int value = -1; > + RCULIST_FOR_EACH (elem, node, list) { > + ovs_assert(value <= elem->value); > + ovs_assert(elem->value < MAX_ELEMS); > + value = elem->value; > + if (!checked) { > + checked = true; > + } > + usleep(10); > + } > + > + ovsrcu_quiesce(); > + > + if (checked && rculist_is_empty(list)) { > + break; > + } > + } > + return NULL; > +} > + > +/* Run test while a thread checks the integrity of the list. > + * Tests must end up emptying the list. */ > +static void > +run_test_while_checking(void (*function)(struct rculist *list)) > +{ > + struct rculist list; > + pthread_t checker; > + > + rculist_init(&list); > + > + checker = ovs_thread_create("checker", checker_main, &list); > + function(&list); > + > + ovs_assert(rculist_is_empty(&list)); > + ovsrcu_quiesce(); > + xpthread_join(checker, NULL); > + printf("."); > +} > + > +static void > +test_rculist_insert_delete__(struct rculist *list, bool long_version) > +{ > + struct element *elem; > + int value; > + > + for (int i = 1; i < MAX_ELEMS; i++) { > + elem = xmalloc(sizeof *elem); > + elem->value = i; > + rculist_insert(list, &elem->node); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + > + ovsrcu_quiesce(); > + > + value = MAX_ELEMS; > + RCULIST_FOR_EACH_REVERSE_PROTECTED (elem, node, list) { > + ovs_assert (elem->value <= value); > + value = elem->value; > + } > + > + if (long_version) { > + struct element *next; > + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, next, node, list) { > + rculist_remove(&elem->node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + } else { > + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, node, list) { > + rculist_remove(&elem->node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + } > +} > + > +static void > +test_rculist_insert_delete(struct rculist *list) { > + test_rculist_insert_delete__(list, false); > +} > + > +static void > +test_rculist_insert_delete_long(struct rculist *list) { > + test_rculist_insert_delete__(list, true); > +} > + > +static void > +test_rculist_push_front_pop_back(struct rculist *list) > +{ > + struct element *elem; > + > + for (int i = MAX_ELEMS - 1; i > 0; i--) { > + elem = xmalloc(sizeof *elem); > + elem->value = i; > + rculist_push_front(list, &elem->node); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + > + ovsrcu_quiesce(); > + > + while (!rculist_is_empty(list)) { > + elem = CONTAINER_OF(rculist_pop_back(list), struct element, node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > +} > + > +static void > +test_rculist_push_back_pop_front(struct rculist *list) > +{ > + struct element *elem; > + > + for (int i = 0 ; i < MAX_ELEMS; i++) { > + elem = xmalloc(sizeof *elem); > + elem->value = i; > + rculist_push_back(list, &elem->node); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + > + ovsrcu_quiesce(); > + > + while (!rculist_is_empty(list)) { > + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > +} > + > +static void > +test_rculist_splice(struct rculist *list) > +{ > + struct element *elem; > + struct rculist other; > + rculist_init(&other); > + > + /* Insert elements in list by splicing an intermediate rculist */ > + for (int i = 0; i < MAX_ELEMS; i++) { > + elem = xmalloc(sizeof *elem); > + elem->value = i; > + rculist_insert(&other, &elem->node); > + rculist_splice_hidden(list, rculist_next_protected(&other), &other); > + rculist_init(&other); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + > + ovsrcu_quiesce(); > + > + ovs_assert(rculist_size(list) == MAX_ELEMS); > + ovs_assert(rculist_is_empty(&other)); > + while (!rculist_is_empty(list)) { > + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > +} > + > +static void > +test_rculist_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) > +{ > + run_test_while_checking(test_rculist_insert_delete); > + run_test_while_checking(test_rculist_insert_delete_long); > + run_test_while_checking(test_rculist_push_back_pop_front); > + run_test_while_checking(test_rculist_push_front_pop_back); > + run_test_while_checking(test_rculist_splice); > + printf("\n"); > +} > + > +OVSTEST_REGISTER("test-rculist", test_rculist_main); > -- > 2.38.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 12/5/22 09:41, Adrian Moreno wrote: > Low test coverage on this area caused some errors to remain unnoticed. > Add basic functional test of rculist. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > tests/automake.mk | 1 + > tests/library.at | 5 ++ > tests/test-rculist.c | 205 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 211 insertions(+) > create mode 100644 tests/test-rculist.c > > diff --git a/tests/automake.mk b/tests/automake.mk > index d509cf935..88f97b8b7 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -474,6 +474,7 @@ tests_ovstest_SOURCES = \ > tests/test-packets.c \ > tests/test-random.c \ > tests/test-rcu.c \ > + tests/test-rculist.c \ > tests/test-reconnect.c \ > tests/test-rstp.c \ > tests/test-sflow.c \ > diff --git a/tests/library.at b/tests/library.at > index bafb28277..164ae789d 100644 > --- a/tests/library.at > +++ b/tests/library.at > @@ -27,6 +27,11 @@ AT_CHECK([ovstest test-hindex], [0], [..................... > ]) > AT_CLEANUP > > +AT_SETUP([test rcu linked lists]) > +AT_CHECK([ovstest test-rculist], [0], [..... > +]) > +AT_CLEANUP > + > AT_SETUP([cuckoo hash]) > AT_KEYWORDS([cmap]) > AT_CHECK([ovstest test-cmap check 1], [0], [... > diff --git a/tests/test-rculist.c b/tests/test-rculist.c > new file mode 100644 > index 000000000..49fe434ff > --- /dev/null > +++ b/tests/test-rculist.c > @@ -0,0 +1,205 @@ > +#include <config.h> > +#undef NDEBUG > +#include <unistd.h> > + > +#include "ovstest.h" > +#include "rculist.h" > +#include "openvswitch/list.h" > +#include "ovs-thread.h" > +#include "random.h" > +#include "util.h" > + > +enum { MAX_ELEMS = 10, MAX_CHECKS = 200 }; > + > +/* Sample list element. */ > +struct element { > + int value; > + struct rculist node; > +}; > + > +/* Continuously check the integrity of the list until it's empty. */ > +static void * > +checker_main(void *aux) > +{ > + struct element *elem; > + struct rculist *list = (struct rculist *) aux; > + bool checked = false; > + > + for (int i = 0; i < MAX_CHECKS; i++) { > + int value = -1; > + RCULIST_FOR_EACH (elem, node, list) { > + ovs_assert(value <= elem->value); > + ovs_assert(elem->value < MAX_ELEMS); > + value = elem->value; > + if (!checked) { > + checked = true; > + } > + usleep(10); Hi, Adrian. This change breaks the build on Windows: tests\test-rculist.c(37): error C4013: 'usleep' undefined; assuming extern returning int make[1]: *** [tests/test-rculist.obj] Error 2 You should use xsleep or xnanosleep instead. The problem might be that they are entering quiescent state for the time of the sleep, so the test need to be careful with that. > + } > + > + ovsrcu_quiesce(); > + > + if (checked && rculist_is_empty(list)) { > + break; > + } > + } > + return NULL; > +} > + > +/* Run test while a thread checks the integrity of the list. > + * Tests must end up emptying the list. */ > +static void > +run_test_while_checking(void (*function)(struct rculist *list)) > +{ > + struct rculist list; > + pthread_t checker; > + > + rculist_init(&list); > + > + checker = ovs_thread_create("checker", checker_main, &list); > + function(&list); > + > + ovs_assert(rculist_is_empty(&list)); > + ovsrcu_quiesce(); > + xpthread_join(checker, NULL); > + printf("."); > +} > + > +static void > +test_rculist_insert_delete__(struct rculist *list, bool long_version) > +{ > + struct element *elem; > + int value; > + > + for (int i = 1; i < MAX_ELEMS; i++) { > + elem = xmalloc(sizeof *elem); > + elem->value = i; > + rculist_insert(list, &elem->node); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + > + ovsrcu_quiesce(); > + > + value = MAX_ELEMS; > + RCULIST_FOR_EACH_REVERSE_PROTECTED (elem, node, list) { > + ovs_assert (elem->value <= value); > + value = elem->value; > + } > + > + if (long_version) { > + struct element *next; > + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, next, node, list) { > + rculist_remove(&elem->node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + } else { > + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, node, list) { > + rculist_remove(&elem->node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + } > +} > + > +static void > +test_rculist_insert_delete(struct rculist *list) { '{' should be on a new line. > + test_rculist_insert_delete__(list, false); > +} > + > +static void > +test_rculist_insert_delete_long(struct rculist *list) { Ditto. > + test_rculist_insert_delete__(list, true); > +} > + > +static void > +test_rculist_push_front_pop_back(struct rculist *list) > +{ > + struct element *elem; > + > + for (int i = MAX_ELEMS - 1; i > 0; i--) { > + elem = xmalloc(sizeof *elem); > + elem->value = i; > + rculist_push_front(list, &elem->node); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + > + ovsrcu_quiesce(); > + > + while (!rculist_is_empty(list)) { > + elem = CONTAINER_OF(rculist_pop_back(list), struct element, node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > +} > + > +static void > +test_rculist_push_back_pop_front(struct rculist *list) > +{ > + struct element *elem; > + > + for (int i = 0 ; i < MAX_ELEMS; i++) { > + elem = xmalloc(sizeof *elem); > + elem->value = i; > + rculist_push_back(list, &elem->node); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + > + ovsrcu_quiesce(); > + > + while (!rculist_is_empty(list)) { > + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > +} > + > +static void > +test_rculist_splice(struct rculist *list) > +{ > + struct element *elem; > + struct rculist other; > + rculist_init(&other); > + > + /* Insert elements in list by splicing an intermediate rculist */ > + for (int i = 0; i < MAX_ELEMS; i++) { > + elem = xmalloc(sizeof *elem); > + elem->value = i; > + rculist_insert(&other, &elem->node); > + rculist_splice_hidden(list, rculist_next_protected(&other), &other); > + rculist_init(&other); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > + > + ovsrcu_quiesce(); > + > + ovs_assert(rculist_size(list) == MAX_ELEMS); > + ovs_assert(rculist_is_empty(&other)); > + while (!rculist_is_empty(list)) { > + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); > + ovsrcu_postpone(free, elem); > + /* Leave some time for checkers to iterate through. */ > + usleep(random_range(1000)); > + } > +} > + > +static void > +test_rculist_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) > +{ > + run_test_while_checking(test_rculist_insert_delete); > + run_test_while_checking(test_rculist_insert_delete_long); > + run_test_while_checking(test_rculist_push_back_pop_front); > + run_test_while_checking(test_rculist_push_front_pop_back); > + run_test_while_checking(test_rculist_splice); > + printf("\n"); > +} > + > +OVSTEST_REGISTER("test-rculist", test_rculist_main);
On 12/6/22 17:02, Ilya Maximets wrote: > On 12/5/22 09:41, Adrian Moreno wrote: >> Low test coverage on this area caused some errors to remain unnoticed. >> Add basic functional test of rculist. >> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- >> tests/automake.mk | 1 + >> tests/library.at | 5 ++ >> tests/test-rculist.c | 205 +++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 211 insertions(+) >> create mode 100644 tests/test-rculist.c >> >> diff --git a/tests/automake.mk b/tests/automake.mk >> index d509cf935..88f97b8b7 100644 >> --- a/tests/automake.mk >> +++ b/tests/automake.mk >> @@ -474,6 +474,7 @@ tests_ovstest_SOURCES = \ >> tests/test-packets.c \ >> tests/test-random.c \ >> tests/test-rcu.c \ >> + tests/test-rculist.c \ >> tests/test-reconnect.c \ >> tests/test-rstp.c \ >> tests/test-sflow.c \ >> diff --git a/tests/library.at b/tests/library.at >> index bafb28277..164ae789d 100644 >> --- a/tests/library.at >> +++ b/tests/library.at >> @@ -27,6 +27,11 @@ AT_CHECK([ovstest test-hindex], [0], [..................... >> ]) >> AT_CLEANUP >> >> +AT_SETUP([test rcu linked lists]) >> +AT_CHECK([ovstest test-rculist], [0], [..... >> +]) >> +AT_CLEANUP >> + >> AT_SETUP([cuckoo hash]) >> AT_KEYWORDS([cmap]) >> AT_CHECK([ovstest test-cmap check 1], [0], [... >> diff --git a/tests/test-rculist.c b/tests/test-rculist.c >> new file mode 100644 >> index 000000000..49fe434ff >> --- /dev/null >> +++ b/tests/test-rculist.c >> @@ -0,0 +1,205 @@ >> +#include <config.h> >> +#undef NDEBUG >> +#include <unistd.h> >> + >> +#include "ovstest.h" >> +#include "rculist.h" >> +#include "openvswitch/list.h" >> +#include "ovs-thread.h" >> +#include "random.h" >> +#include "util.h" >> + >> +enum { MAX_ELEMS = 10, MAX_CHECKS = 200 }; >> + >> +/* Sample list element. */ >> +struct element { >> + int value; >> + struct rculist node; >> +}; >> + >> +/* Continuously check the integrity of the list until it's empty. */ >> +static void * >> +checker_main(void *aux) >> +{ >> + struct element *elem; >> + struct rculist *list = (struct rculist *) aux; >> + bool checked = false; >> + >> + for (int i = 0; i < MAX_CHECKS; i++) { >> + int value = -1; >> + RCULIST_FOR_EACH (elem, node, list) { >> + ovs_assert(value <= elem->value); >> + ovs_assert(elem->value < MAX_ELEMS); >> + value = elem->value; >> + if (!checked) { >> + checked = true; >> + } >> + usleep(10); > > Hi, Adrian. > > This change breaks the build on Windows: > > tests\test-rculist.c(37): error C4013: 'usleep' undefined; assuming extern returning int > make[1]: *** [tests/test-rculist.obj] Error 2 > > You should use xsleep or xnanosleep instead. The problem > might be that they are entering quiescent state for the time > of the sleep, so the test need to be careful with that. > We cannot enter quiescent state in the checker thread so I'll see if we can use Windows' "Sleep". Thanks. >> + } >> + >> + ovsrcu_quiesce(); >> + >> + if (checked && rculist_is_empty(list)) { >> + break; >> + } >> + } >> + return NULL; >> +} >> + >> +/* Run test while a thread checks the integrity of the list. >> + * Tests must end up emptying the list. */ >> +static void >> +run_test_while_checking(void (*function)(struct rculist *list)) >> +{ >> + struct rculist list; >> + pthread_t checker; >> + >> + rculist_init(&list); >> + >> + checker = ovs_thread_create("checker", checker_main, &list); >> + function(&list); >> + >> + ovs_assert(rculist_is_empty(&list)); >> + ovsrcu_quiesce(); >> + xpthread_join(checker, NULL); >> + printf("."); >> +} >> + >> +static void >> +test_rculist_insert_delete__(struct rculist *list, bool long_version) >> +{ >> + struct element *elem; >> + int value; >> + >> + for (int i = 1; i < MAX_ELEMS; i++) { >> + elem = xmalloc(sizeof *elem); >> + elem->value = i; >> + rculist_insert(list, &elem->node); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> + >> + ovsrcu_quiesce(); >> + >> + value = MAX_ELEMS; >> + RCULIST_FOR_EACH_REVERSE_PROTECTED (elem, node, list) { >> + ovs_assert (elem->value <= value); >> + value = elem->value; >> + } >> + >> + if (long_version) { >> + struct element *next; >> + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, next, node, list) { >> + rculist_remove(&elem->node); >> + ovsrcu_postpone(free, elem); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> + } else { >> + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, node, list) { >> + rculist_remove(&elem->node); >> + ovsrcu_postpone(free, elem); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> + } >> +} >> + >> +static void >> +test_rculist_insert_delete(struct rculist *list) { > > '{' should be on a new line. > >> + test_rculist_insert_delete__(list, false); >> +} >> + >> +static void >> +test_rculist_insert_delete_long(struct rculist *list) { > > Ditto. > >> + test_rculist_insert_delete__(list, true); >> +} >> + >> +static void >> +test_rculist_push_front_pop_back(struct rculist *list) >> +{ >> + struct element *elem; >> + >> + for (int i = MAX_ELEMS - 1; i > 0; i--) { >> + elem = xmalloc(sizeof *elem); >> + elem->value = i; >> + rculist_push_front(list, &elem->node); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> + >> + ovsrcu_quiesce(); >> + >> + while (!rculist_is_empty(list)) { >> + elem = CONTAINER_OF(rculist_pop_back(list), struct element, node); >> + ovsrcu_postpone(free, elem); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> +} >> + >> +static void >> +test_rculist_push_back_pop_front(struct rculist *list) >> +{ >> + struct element *elem; >> + >> + for (int i = 0 ; i < MAX_ELEMS; i++) { >> + elem = xmalloc(sizeof *elem); >> + elem->value = i; >> + rculist_push_back(list, &elem->node); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> + >> + ovsrcu_quiesce(); >> + >> + while (!rculist_is_empty(list)) { >> + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); >> + ovsrcu_postpone(free, elem); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> +} >> + >> +static void >> +test_rculist_splice(struct rculist *list) >> +{ >> + struct element *elem; >> + struct rculist other; >> + rculist_init(&other); >> + >> + /* Insert elements in list by splicing an intermediate rculist */ >> + for (int i = 0; i < MAX_ELEMS; i++) { >> + elem = xmalloc(sizeof *elem); >> + elem->value = i; >> + rculist_insert(&other, &elem->node); >> + rculist_splice_hidden(list, rculist_next_protected(&other), &other); >> + rculist_init(&other); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> + >> + ovsrcu_quiesce(); >> + >> + ovs_assert(rculist_size(list) == MAX_ELEMS); >> + ovs_assert(rculist_is_empty(&other)); >> + while (!rculist_is_empty(list)) { >> + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); >> + ovsrcu_postpone(free, elem); >> + /* Leave some time for checkers to iterate through. */ >> + usleep(random_range(1000)); >> + } >> +} >> + >> +static void >> +test_rculist_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) >> +{ >> + run_test_while_checking(test_rculist_insert_delete); >> + run_test_while_checking(test_rculist_insert_delete_long); >> + run_test_while_checking(test_rculist_push_back_pop_front); >> + run_test_while_checking(test_rculist_push_front_pop_back); >> + run_test_while_checking(test_rculist_splice); >> + printf("\n"); >> +} >> + >> +OVSTEST_REGISTER("test-rculist", test_rculist_main); >
On 12/7/22 09:08, Adrian Moreno wrote: > > > On 12/6/22 17:02, Ilya Maximets wrote: >> On 12/5/22 09:41, Adrian Moreno wrote: >>> Low test coverage on this area caused some errors to remain unnoticed. >>> Add basic functional test of rculist. >>> >>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >>> --- >>> tests/automake.mk | 1 + >>> tests/library.at | 5 ++ >>> tests/test-rculist.c | 205 +++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 211 insertions(+) >>> create mode 100644 tests/test-rculist.c >>> >>> diff --git a/tests/automake.mk b/tests/automake.mk >>> index d509cf935..88f97b8b7 100644 >>> --- a/tests/automake.mk >>> +++ b/tests/automake.mk >>> @@ -474,6 +474,7 @@ tests_ovstest_SOURCES = \ >>> tests/test-packets.c \ >>> tests/test-random.c \ >>> tests/test-rcu.c \ >>> + tests/test-rculist.c \ >>> tests/test-reconnect.c \ >>> tests/test-rstp.c \ >>> tests/test-sflow.c \ >>> diff --git a/tests/library.at b/tests/library.at >>> index bafb28277..164ae789d 100644 >>> --- a/tests/library.at >>> +++ b/tests/library.at >>> @@ -27,6 +27,11 @@ AT_CHECK([ovstest test-hindex], [0], [..................... >>> ]) >>> AT_CLEANUP >>> +AT_SETUP([test rcu linked lists]) >>> +AT_CHECK([ovstest test-rculist], [0], [..... >>> +]) >>> +AT_CLEANUP >>> + >>> AT_SETUP([cuckoo hash]) >>> AT_KEYWORDS([cmap]) >>> AT_CHECK([ovstest test-cmap check 1], [0], [... >>> diff --git a/tests/test-rculist.c b/tests/test-rculist.c >>> new file mode 100644 >>> index 000000000..49fe434ff >>> --- /dev/null >>> +++ b/tests/test-rculist.c >>> @@ -0,0 +1,205 @@ >>> +#include <config.h> >>> +#undef NDEBUG >>> +#include <unistd.h> >>> + >>> +#include "ovstest.h" >>> +#include "rculist.h" >>> +#include "openvswitch/list.h" >>> +#include "ovs-thread.h" >>> +#include "random.h" >>> +#include "util.h" >>> + >>> +enum { MAX_ELEMS = 10, MAX_CHECKS = 200 }; >>> + >>> +/* Sample list element. */ >>> +struct element { >>> + int value; >>> + struct rculist node; >>> +}; >>> + >>> +/* Continuously check the integrity of the list until it's empty. */ >>> +static void * >>> +checker_main(void *aux) >>> +{ >>> + struct element *elem; >>> + struct rculist *list = (struct rculist *) aux; >>> + bool checked = false; >>> + >>> + for (int i = 0; i < MAX_CHECKS; i++) { >>> + int value = -1; >>> + RCULIST_FOR_EACH (elem, node, list) { >>> + ovs_assert(value <= elem->value); >>> + ovs_assert(elem->value < MAX_ELEMS); >>> + value = elem->value; >>> + if (!checked) { >>> + checked = true; >>> + } >>> + usleep(10); >> >> Hi, Adrian. >> >> This change breaks the build on Windows: >> >> tests\test-rculist.c(37): error C4013: 'usleep' undefined; assuming extern returning int >> make[1]: *** [tests/test-rculist.obj] Error 2 >> >> You should use xsleep or xnanosleep instead. The problem >> might be that they are entering quiescent state for the time >> of the sleep, so the test need to be careful with that. >> > > We cannot enter quiescent state in the checker thread so I'll see if we can use Windows' "Sleep". OK, the Sleep() should have a resolution you need. I guess, you may define a function here in the test that will do usleep() or Sleep() depending on the platform, similarly to xsleep(), but without entering a quiescent state. > Thanks. > >>> + } >>> + >>> + ovsrcu_quiesce(); >>> + >>> + if (checked && rculist_is_empty(list)) { >>> + break; >>> + } >>> + } >>> + return NULL; >>> +} >>> + >>> +/* Run test while a thread checks the integrity of the list. >>> + * Tests must end up emptying the list. */ >>> +static void >>> +run_test_while_checking(void (*function)(struct rculist *list)) >>> +{ >>> + struct rculist list; >>> + pthread_t checker; >>> + >>> + rculist_init(&list); >>> + >>> + checker = ovs_thread_create("checker", checker_main, &list); >>> + function(&list); >>> + >>> + ovs_assert(rculist_is_empty(&list)); >>> + ovsrcu_quiesce(); >>> + xpthread_join(checker, NULL); >>> + printf("."); >>> +} >>> + >>> +static void >>> +test_rculist_insert_delete__(struct rculist *list, bool long_version) >>> +{ >>> + struct element *elem; >>> + int value; >>> + >>> + for (int i = 1; i < MAX_ELEMS; i++) { >>> + elem = xmalloc(sizeof *elem); >>> + elem->value = i; >>> + rculist_insert(list, &elem->node); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> + >>> + ovsrcu_quiesce(); >>> + >>> + value = MAX_ELEMS; >>> + RCULIST_FOR_EACH_REVERSE_PROTECTED (elem, node, list) { >>> + ovs_assert (elem->value <= value); >>> + value = elem->value; >>> + } >>> + >>> + if (long_version) { >>> + struct element *next; >>> + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, next, node, list) { >>> + rculist_remove(&elem->node); >>> + ovsrcu_postpone(free, elem); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> + } else { >>> + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, node, list) { >>> + rculist_remove(&elem->node); >>> + ovsrcu_postpone(free, elem); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> + } >>> +} >>> + >>> +static void >>> +test_rculist_insert_delete(struct rculist *list) { >> >> '{' should be on a new line. >> >>> + test_rculist_insert_delete__(list, false); >>> +} >>> + >>> +static void >>> +test_rculist_insert_delete_long(struct rculist *list) { >> >> Ditto. >> >>> + test_rculist_insert_delete__(list, true); >>> +} >>> + >>> +static void >>> +test_rculist_push_front_pop_back(struct rculist *list) >>> +{ >>> + struct element *elem; >>> + >>> + for (int i = MAX_ELEMS - 1; i > 0; i--) { >>> + elem = xmalloc(sizeof *elem); >>> + elem->value = i; >>> + rculist_push_front(list, &elem->node); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> + >>> + ovsrcu_quiesce(); >>> + >>> + while (!rculist_is_empty(list)) { >>> + elem = CONTAINER_OF(rculist_pop_back(list), struct element, node); >>> + ovsrcu_postpone(free, elem); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> +} >>> + >>> +static void >>> +test_rculist_push_back_pop_front(struct rculist *list) >>> +{ >>> + struct element *elem; >>> + >>> + for (int i = 0 ; i < MAX_ELEMS; i++) { >>> + elem = xmalloc(sizeof *elem); >>> + elem->value = i; >>> + rculist_push_back(list, &elem->node); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> + >>> + ovsrcu_quiesce(); >>> + >>> + while (!rculist_is_empty(list)) { >>> + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); >>> + ovsrcu_postpone(free, elem); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> +} >>> + >>> +static void >>> +test_rculist_splice(struct rculist *list) >>> +{ >>> + struct element *elem; >>> + struct rculist other; >>> + rculist_init(&other); >>> + >>> + /* Insert elements in list by splicing an intermediate rculist */ >>> + for (int i = 0; i < MAX_ELEMS; i++) { >>> + elem = xmalloc(sizeof *elem); >>> + elem->value = i; >>> + rculist_insert(&other, &elem->node); >>> + rculist_splice_hidden(list, rculist_next_protected(&other), &other); >>> + rculist_init(&other); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> + >>> + ovsrcu_quiesce(); >>> + >>> + ovs_assert(rculist_size(list) == MAX_ELEMS); >>> + ovs_assert(rculist_is_empty(&other)); >>> + while (!rculist_is_empty(list)) { >>> + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); >>> + ovsrcu_postpone(free, elem); >>> + /* Leave some time for checkers to iterate through. */ >>> + usleep(random_range(1000)); >>> + } >>> +} >>> + >>> +static void >>> +test_rculist_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) >>> +{ >>> + run_test_while_checking(test_rculist_insert_delete); >>> + run_test_while_checking(test_rculist_insert_delete_long); >>> + run_test_while_checking(test_rculist_push_back_pop_front); >>> + run_test_while_checking(test_rculist_push_front_pop_back); >>> + run_test_while_checking(test_rculist_splice); >>> + printf("\n"); >>> +} >>> + >>> +OVSTEST_REGISTER("test-rculist", test_rculist_main); >> >
diff --git a/tests/automake.mk b/tests/automake.mk index d509cf935..88f97b8b7 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -474,6 +474,7 @@ tests_ovstest_SOURCES = \ tests/test-packets.c \ tests/test-random.c \ tests/test-rcu.c \ + tests/test-rculist.c \ tests/test-reconnect.c \ tests/test-rstp.c \ tests/test-sflow.c \ diff --git a/tests/library.at b/tests/library.at index bafb28277..164ae789d 100644 --- a/tests/library.at +++ b/tests/library.at @@ -27,6 +27,11 @@ AT_CHECK([ovstest test-hindex], [0], [..................... ]) AT_CLEANUP +AT_SETUP([test rcu linked lists]) +AT_CHECK([ovstest test-rculist], [0], [..... +]) +AT_CLEANUP + AT_SETUP([cuckoo hash]) AT_KEYWORDS([cmap]) AT_CHECK([ovstest test-cmap check 1], [0], [... diff --git a/tests/test-rculist.c b/tests/test-rculist.c new file mode 100644 index 000000000..49fe434ff --- /dev/null +++ b/tests/test-rculist.c @@ -0,0 +1,205 @@ +#include <config.h> +#undef NDEBUG +#include <unistd.h> + +#include "ovstest.h" +#include "rculist.h" +#include "openvswitch/list.h" +#include "ovs-thread.h" +#include "random.h" +#include "util.h" + +enum { MAX_ELEMS = 10, MAX_CHECKS = 200 }; + +/* Sample list element. */ +struct element { + int value; + struct rculist node; +}; + +/* Continuously check the integrity of the list until it's empty. */ +static void * +checker_main(void *aux) +{ + struct element *elem; + struct rculist *list = (struct rculist *) aux; + bool checked = false; + + for (int i = 0; i < MAX_CHECKS; i++) { + int value = -1; + RCULIST_FOR_EACH (elem, node, list) { + ovs_assert(value <= elem->value); + ovs_assert(elem->value < MAX_ELEMS); + value = elem->value; + if (!checked) { + checked = true; + } + usleep(10); + } + + ovsrcu_quiesce(); + + if (checked && rculist_is_empty(list)) { + break; + } + } + return NULL; +} + +/* Run test while a thread checks the integrity of the list. + * Tests must end up emptying the list. */ +static void +run_test_while_checking(void (*function)(struct rculist *list)) +{ + struct rculist list; + pthread_t checker; + + rculist_init(&list); + + checker = ovs_thread_create("checker", checker_main, &list); + function(&list); + + ovs_assert(rculist_is_empty(&list)); + ovsrcu_quiesce(); + xpthread_join(checker, NULL); + printf("."); +} + +static void +test_rculist_insert_delete__(struct rculist *list, bool long_version) +{ + struct element *elem; + int value; + + for (int i = 1; i < MAX_ELEMS; i++) { + elem = xmalloc(sizeof *elem); + elem->value = i; + rculist_insert(list, &elem->node); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } + + ovsrcu_quiesce(); + + value = MAX_ELEMS; + RCULIST_FOR_EACH_REVERSE_PROTECTED (elem, node, list) { + ovs_assert (elem->value <= value); + value = elem->value; + } + + if (long_version) { + struct element *next; + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, next, node, list) { + rculist_remove(&elem->node); + ovsrcu_postpone(free, elem); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } + } else { + RCULIST_FOR_EACH_SAFE_PROTECTED (elem, node, list) { + rculist_remove(&elem->node); + ovsrcu_postpone(free, elem); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } + } +} + +static void +test_rculist_insert_delete(struct rculist *list) { + test_rculist_insert_delete__(list, false); +} + +static void +test_rculist_insert_delete_long(struct rculist *list) { + test_rculist_insert_delete__(list, true); +} + +static void +test_rculist_push_front_pop_back(struct rculist *list) +{ + struct element *elem; + + for (int i = MAX_ELEMS - 1; i > 0; i--) { + elem = xmalloc(sizeof *elem); + elem->value = i; + rculist_push_front(list, &elem->node); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } + + ovsrcu_quiesce(); + + while (!rculist_is_empty(list)) { + elem = CONTAINER_OF(rculist_pop_back(list), struct element, node); + ovsrcu_postpone(free, elem); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } +} + +static void +test_rculist_push_back_pop_front(struct rculist *list) +{ + struct element *elem; + + for (int i = 0 ; i < MAX_ELEMS; i++) { + elem = xmalloc(sizeof *elem); + elem->value = i; + rculist_push_back(list, &elem->node); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } + + ovsrcu_quiesce(); + + while (!rculist_is_empty(list)) { + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); + ovsrcu_postpone(free, elem); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } +} + +static void +test_rculist_splice(struct rculist *list) +{ + struct element *elem; + struct rculist other; + rculist_init(&other); + + /* Insert elements in list by splicing an intermediate rculist */ + for (int i = 0; i < MAX_ELEMS; i++) { + elem = xmalloc(sizeof *elem); + elem->value = i; + rculist_insert(&other, &elem->node); + rculist_splice_hidden(list, rculist_next_protected(&other), &other); + rculist_init(&other); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } + + ovsrcu_quiesce(); + + ovs_assert(rculist_size(list) == MAX_ELEMS); + ovs_assert(rculist_is_empty(&other)); + while (!rculist_is_empty(list)) { + elem = CONTAINER_OF(rculist_pop_front(list), struct element, node); + ovsrcu_postpone(free, elem); + /* Leave some time for checkers to iterate through. */ + usleep(random_range(1000)); + } +} + +static void +test_rculist_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) +{ + run_test_while_checking(test_rculist_insert_delete); + run_test_while_checking(test_rculist_insert_delete_long); + run_test_while_checking(test_rculist_push_back_pop_front); + run_test_while_checking(test_rculist_push_front_pop_back); + run_test_while_checking(test_rculist_splice); + printf("\n"); +} + +OVSTEST_REGISTER("test-rculist", test_rculist_main);
Low test coverage on this area caused some errors to remain unnoticed. Add basic functional test of rculist. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- tests/automake.mk | 1 + tests/library.at | 5 ++ tests/test-rculist.c | 205 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 211 insertions(+) create mode 100644 tests/test-rculist.c