[ovs-dev] [PATCH ovn v6 2/3] Add ipam unit tests
Dumitru Ceara
dceara at redhat.com
Wed Jan 13 10:38:03 UTC 2021
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 <mmichels at redhat.com>
> ---
Hi Mark,
AddressSanitizer is complaining about memleaks of the 'config' smaps.
With those fixed:
Acked-by: Dumitru Ceara <dceara at redhat.com>
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])
>
More information about the dev
mailing list