On 1/12/21 7:39 PM, Mark Michelson wrote: > This adds unit tests for IPAM IPv6 initialization, IPv4 initialization, > and IPv4 address retrieval. > > Signed-off-by: Mark Michelson <[email protected]> > ---
Hi Mark, AddressSanitizer is complaining about memleaks of the 'config' smaps. With those fixed: Acked-by: Dumitru Ceara <[email protected]> Thanks, Dumitru > northd/test-ipam.c | 146 ++++++++++++++++++++++++++++ > tests/automake.mk | 8 +- > tests/ovn-ipam.at | 237 +++++++++++++++++++++++++++++++++++++++++++++ > tests/testsuite.at | 1 + > 4 files changed, 390 insertions(+), 2 deletions(-) > create mode 100644 northd/test-ipam.c > create mode 100644 tests/ovn-ipam.at > > diff --git a/northd/test-ipam.c b/northd/test-ipam.c > new file mode 100644 > index 000000000..7872585a3 > --- /dev/null > +++ b/northd/test-ipam.c > @@ -0,0 +1,146 @@ > +/* > + * Copyright (c) 2020 Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > +#include "tests/ovstest.h" > + > +#include "openvswitch/dynamic-string.h" > +#include "smap.h" > +#include "packets.h" > +#include "bitmap.h" > + > +#include "ipam.h" > + > +static void > +test_ipam_get_unused_ip(struct ovs_cmdl_context *ctx) > +{ > + struct ipam_info info; > + > + struct smap config = SMAP_INITIALIZER(&config); > + smap_add(&config, "subnet", ctx->argv[1]); > + int num_ips; > + str_to_int(ctx->argv[2], 0, &num_ips); > + if (ctx->argc > 3) { > + smap_add(&config, "exclude_ips", ctx->argv[3]); > + } > + init_ipam_info(&info, &config, "Unused IP test"); > + > + bool fail = false; > + struct ds output = DS_EMPTY_INITIALIZER; > + struct ds err = DS_EMPTY_INITIALIZER; > + for (size_t i = 0; i < num_ips; i++) { > + uint32_t next_ip = ipam_get_unused_ip(&info); > + ds_put_format(&output, IP_FMT "\n", IP_ARGS(htonl(next_ip))); > + if (next_ip) { > + ovs_assert(ipam_insert_ip(&info, next_ip)); > + } > + } > + > + printf("%s", ds_cstr(&output)); > + if (fail) { > + fprintf(stderr, "%s", ds_cstr(&err)); > + } > + > + smap_destroy(&config); > + destroy_ipam_info(&info); > + ds_destroy(&output); > + ds_destroy(&err); > +} > + > +static void > +test_ipam_init_ipv4(struct ovs_cmdl_context *ctx) > +{ > + const char *subnet = ctx->argv[1]; > + const char *exclude_ips = ctx->argc > 2 ? ctx->argv[2] : NULL; > + struct smap config = SMAP_INITIALIZER(&config); > + smap_add(&config, "subnet", subnet); > + if (exclude_ips) { > + smap_add(&config, "exclude_ips", exclude_ips); > + } > + struct ipam_info ipam; > + init_ipam_info(&ipam, &config, "IPv4 test"); > + > + struct ds output = DS_EMPTY_INITIALIZER; > + ds_put_format(&output, "start_ipv4: " IP_FMT "\n", > + IP_ARGS(htonl(ipam.start_ipv4))); > + ds_put_format(&output, "total_ipv4s: %" PRIuSIZE "\n", ipam.total_ipv4s); > + > + ds_put_cstr(&output, "allocated_ipv4s: "); > + if (ipam.allocated_ipv4s) { > + int start = 0; > + int end = ipam.total_ipv4s; > + for (size_t bit = bitmap_scan(ipam.allocated_ipv4s, true, start, > end); > + bit != end; > + bit = bitmap_scan(ipam.allocated_ipv4s, true, bit + 1, end)) { > + ds_put_format(&output, IP_FMT " ", > + IP_ARGS((htonl(ipam.start_ipv4 + bit)))); > + } > + } > + ds_chomp(&output, ' '); > + ds_put_char(&output, '\n'); > + > + printf("%s", ds_cstr(&output)); > + We're leaking 'config' here: smap_destroy(&config); > + destroy_ipam_info(&ipam); > + ds_destroy(&output); > +} > + > +static void > +test_ipam_init_ipv6_prefix(struct ovs_cmdl_context *ctx) > +{ > + const char *prefix = ctx->argc > 1 ? ctx->argv[1] : NULL; > + struct smap config = SMAP_INITIALIZER(&config); > + if (prefix) { > + smap_add(&config, "ipv6_prefix", prefix); > + }; > + struct ipam_info ipam; > + init_ipam_info(&ipam, &config, "IPv6 test"); > + > + struct ds output = DS_EMPTY_INITIALIZER; > + ds_put_format(&output, "ipv6_prefix_set: %s\n", > + ipam.ipv6_prefix_set ? "true" : "false"); > + if (ipam.ipv6_prefix_set) { > + char ipv6[INET6_ADDRSTRLEN]; > + inet_ntop(AF_INET6, &ipam.ipv6_prefix, > + ipv6, sizeof ipv6); > + ds_put_format(&output, "ipv6_prefix: %s\n", ipv6); > + } > + > + printf("%s", ds_cstr(&output)); > + We're leaking 'config' here: smap_destroy(&config); > + destroy_ipam_info(&ipam); > + ds_destroy(&output); > +} > + > +static void > +test_ipam_main(int argc, char *argv[]) > +{ > + set_program_name(argv[0]); > + static const struct ovs_cmdl_command commands[] = { > + {"ipam_get_unused_ip", NULL, 2, 3, test_ipam_get_unused_ip, OVS_RO}, > + {"ipam_init_ipv6_prefix", NULL, 0, 1, test_ipam_init_ipv6_prefix, > + OVS_RO}, > + {"ipam_init_ipv4", NULL, 1, 2, test_ipam_init_ipv4, > + OVS_RO}, > + {NULL, NULL, 0, 0, NULL, OVS_RO}, > + }; > + struct ovs_cmdl_context ctx; > + ctx.argc = argc - 1; > + ctx.argv = argv + 1; > + ovs_cmdl_run_command(&ctx, commands); > +} > + > +OVSTEST_REGISTER("test-ipam", test_ipam_main); > diff --git a/tests/automake.mk b/tests/automake.mk > index db934cb95..16190cc8e 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -31,7 +31,8 @@ TESTSUITE_AT = \ > tests/ovn-controller-vtep.at \ > tests/ovn-ic.at \ > tests/ovn-macros.at \ > - tests/ovn-performance.at > + tests/ovn-performance.at \ > + tests/ovn-ipam.at > > SYSTEM_KMOD_TESTSUITE_AT = \ > tests/system-common-macros.at \ > @@ -205,7 +206,10 @@ noinst_PROGRAMS += tests/ovstest > tests_ovstest_SOURCES = \ > tests/ovstest.c \ > tests/ovstest.h \ > - tests/test-ovn.c > + tests/test-ovn.c \ > + northd/test-ipam.c \ > + northd/ipam.c \ > + northd/ipam.h > > tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \ > $(OVS_LIBDIR)/libopenvswitch.la lib/libovn.la > diff --git a/tests/ovn-ipam.at b/tests/ovn-ipam.at > new file mode 100644 > index 000000000..61ac8ff7f > --- /dev/null > +++ b/tests/ovn-ipam.at > @@ -0,0 +1,237 @@ > +AT_BANNER([OVN unit tests]) > + > +AT_SETUP([ovn -- unit test -- init_ipam_ipv4]) > +AT_SKIP_IF([test "$ENABLE_UNIT_TESTS" = no]) > +ovn_start > + > +# Valid subnet, no exclude IPs > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.0/29], [0], [dnl > +start_ipv4: 192.168.0.1 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.1 > +]) > + > +# Valid subnet, single exclude IP > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.0/29 192.168.0.3], [0], > [dnl > +start_ipv4: 192.168.0.1 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.1 192.168.0.3 > +]) > + > +# Valid subnet, two exclude IPs > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.0/29 "192.168.0.3 > 192.168.0.5"], [0], [dnl > +start_ipv4: 192.168.0.1 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.1 192.168.0.3 192.168.0.5 > +]) > + > +# Valid subnet, range of exclude IPs > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.0/29 > "192.168.0.3..192.168.0.5"], [0], [dnl > +start_ipv4: 192.168.0.1 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.1 192.168.0.3 192.168.0.4 192.168.0.5 > +]) > + > +# Valid subnet, exclude IP outside of subnet > +# Excluded IP should be ignored. > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.0/29 192.168.0.9], [0], > [dnl > +start_ipv4: 192.168.0.1 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.1 > +],[ignore]) > + > +# Valid subnet, range of exclude IPs starts in subnet but ends outside > +# Excluded IPs inside the subnet should be allocated > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.0/29 > "192.168.0.5..192.168.0.11"], [0], [dnl > +start_ipv4: 192.168.0.1 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.1 192.168.0.5 192.168.0.6 192.168.0.7 > +],[ignore]) > + > +# Valid subnet, range of exclude IPs starts outside subnet but ends inside > +# Excluded IPs inside the subnet should be allocated > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.8/29 > "192.168.0.5..192.168.0.11"], [0], [dnl > +start_ipv4: 192.168.0.9 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.9 192.168.0.10 192.168.0.11 > +],[ignore]) > + > +# Valid subnet, range of exclude IPs starts before and ends after the subnet > +# Entire subnet should be allocated > +# XXX Should excluding every address in a subnet be an invalid configuration? > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.8/29 > "192.168.0.5..192.168.0.18"], [0], [dnl > +start_ipv4: 192.168.0.9 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.9 192.168.0.10 192.168.0.11 192.168.0.12 > 192.168.0.13 192.168.0.14 192.168.0.15 > +],[ignore]) > + > +# Valid subnet, inverted exclude range > +# Exclude range should be ignored > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.0/29 > "192.168.0.5..192.168.0.2"], [0], [dnl > +start_ipv4: 192.168.0.1 > +total_ipv4s: 7 > +allocated_ipv4s: 192.168.0.1 > +],[ignore]) > + > +# XXX At this point, I wanted to insert some tests where I put in invalid > +# IP addresses like 400.500.600.700 to ensure that the start_ipv4 was set > +# to "0.0.0.0". However, ovs_scan_ip_masked() does no validation of the > +# IP address. So long as the given IP address follows the format of > +# xxx.xxx.xxx.xxx, it's seen as valid. In the specific case of > +# "400.500.600.700", it ends up setting the start_ipv4 to > +# "144.244.88.185". This result is probably system-dependent. > + > +# Invalid subnet: Bad mask > +AT_CHECK([ovstest test-ipam ipam_init_ipv4 192.168.0.0/-69], [0], [dnl > +start_ipv4: 0.0.0.0 > +total_ipv4s: 0 > +allocated_ipv4s: > +],[ignore]) > + > +AT_CLEANUP > + > +AT_SETUP([ovn -- unit test -- init_ipam_ipv6_prefix]) > +AT_SKIP_IF([test "$ENABLE_UNIT_TESTS" = no]) > +ovn_start > + > +# No prefix set > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix], [0], [dnl > +ipv6_prefix_set: false > +]) > + > +# Good prefix with no mask > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix aef0::], [0], [dnl > +ipv6_prefix_set: true > +ipv6_prefix: aef0:: > +]) > + > +# Good prefix with good mask > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix aef0::/64], [0], [dnl > +ipv6_prefix_set: true > +ipv6_prefix: aef0:: > +]) > + > +# Bad prefix with no mask > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix aef20::], [0], [dnl > +ipv6_prefix_set: false > +],[ignore]) > + > +# Good prefix with nonsense mask. > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix aef0::/900], [0], [dnl > +ipv6_prefix_set: false > +],[ignore]) > + > +# Good prefix with a non-/64 mask. > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix aef0::/32], [0], [dnl > +ipv6_prefix_set: false > +],[ignore]) > + > +# Bad prefix and a non-/64 mask. > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix aef20::/32], [0], [dnl > +ipv6_prefix_set: false > +],[ignore]) > + > +# Overspecify the IPv6 address. > +# We should "round down" to the /64 network address. > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix aef0::2323], [0], [dnl > +ipv6_prefix_set: true > +ipv6_prefix: aef0:: > +],[ignore]) > + > +# Overspecify the IPv6 address, and specify a mask. > +# We should "round down" to the /64 network address. > +AT_CHECK([ovstest test-ipam ipam_init_ipv6_prefix aef0::2323/64], [0], [dnl > +ipv6_prefix_set: true > +ipv6_prefix: aef0:: > +],[ignore]) > + > +AT_CLEANUP > + > +AT_SETUP([ovn -- unit test -- ipam_get_unused_ip]) > +AT_SKIP_IF([test "$ENABLE_UNIT_TESTS" = no]) > +ovn_start > + > +# Ensure first address returned by IPAM is .2, since .1 is reserved for the > +# connected router > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.0/29 1], [0], [dnl > +192.168.0.2 > +]) > + > +# Ensure that we only grab IPs within the requested subnet > +# Ignore stderr so that the warning about address space being > +# exhausted does not cause the test to fail > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.0/29 6], [0], [dnl > +192.168.0.2 > +192.168.0.3 > +192.168.0.4 > +192.168.0.5 > +192.168.0.6 > +0.0.0.0 > +],[ignore]) > + > +# Set up an exclude IP and ensure it does not get selected > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.0/29 4 > 192.168.0.3], [0], [dnl > +192.168.0.2 > +192.168.0.4 > +192.168.0.5 > +192.168.0.6 > +]) > + > +# Set up an exclude IP range and ensure none gets selected > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.0/29 2 > 192.168.0.3..192.168.0.5], [0], [dnl > +192.168.0.2 > +192.168.0.6 > +]) > + > +# Set up an exclude range from outside the subnet. Ensure it is ignored. > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.0/29 5 > 192.168.1.3..192.168.1.5], [0], [dnl > +192.168.0.2 > +192.168.0.3 > +192.168.0.4 > +192.168.0.5 > +192.168.0.6 > +],[ignore]) > + > +# Set up an exclude range from outside the subnet. Ensure we cannot assign > +# addresses outside the subnet > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.0/29 6 > 192.168.1.3..192.168.1.5], [0], [dnl > +192.168.0.2 > +192.168.0.3 > +192.168.0.4 > +192.168.0.5 > +192.168.0.6 > +0.0.0.0 > +],[ignore]) > + > +# Set up an exclude range that starts before the subnet but ends in the > subnet. > +# The overlapping part should be excluded > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.8/29 2 > 192.168.0.2..192.168.0.12], [0], [dnl > +192.168.0.13 > +192.168.0.14 > +],[ignore]) > + > +# Set up an exclude range that starts in the subnet but ends after the > subnet. > +# The overlapping part should be excluded. > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.0/29 3 > 192.168.0.4..192.168.0.9], [0], [dnl > +192.168.0.2 > +192.168.0.3 > +0.0.0.0 > +],[ignore]) > + > +# Set up an exclude range that starts before the subnet and ends after the > subnet. > +# The entire range should be excluded. > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.8/29 1 > 192.168.0.2..192.168.0.18], [0], [dnl > +0.0.0.0 > +],[ignore]) > + > +# Configure the subnet using a starting IP that is not the network address > of the > +# subnet. Ensure that we "round it down" to the proper subnet starting point. > +AT_CHECK([ovstest test-ipam ipam_get_unused_ip 192.168.0.4/29 5], [0], [dnl > +192.168.0.2 > +192.168.0.3 > +192.168.0.4 > +192.168.0.5 > +192.168.0.6 > +]) > + > +AT_CLEANUP > diff --git a/tests/testsuite.at b/tests/testsuite.at > index 960227dcc..2416d797d 100644 > --- a/tests/testsuite.at > +++ b/tests/testsuite.at > @@ -22,6 +22,7 @@ m4_include([tests/ofproto-macros.at]) > m4_include([tests/ovn-macros.at]) > m4_include([tests/network-functions.at]) > > +m4_include([tests/ovn-ipam.at]) > m4_include([tests/ovn.at]) > m4_include([tests/ovn-performance.at]) > m4_include([tests/ovn-northd.at]) > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
