From 8a58c0ae6d603160e8770d3b4192b7a545515b63 Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Sun, 4 Oct 2020 02:49:47 +0200 Subject: [PATCH 1/8] lifecycle: Expose all adresses and gateways via the environment. Gather all IP addresses and gateways and expose them as IF_ADDRESSES and IF_GATEWAYS environment variables to executors. This eliminates the need to run ifquery from execuctors which would have to parse the config file again on every run. Signed-off-by: Maximilian Wilhelm --- libifupdown/lifecycle.c | 59 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 5910804..6cc749e 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -28,6 +28,8 @@ #include "libifupdown/tokenize.h" #include "libifupdown/config-file.h" +#define BUFFER_LEN 4096 + static bool handle_commands_for_phase(const struct lif_execute_opts *opts, char *const envp[], const struct lif_interface *iface, const char *phase) { @@ -112,6 +114,35 @@ query_dependents_from_executors(const struct lif_execute_opts *opts, char *const return true; } +bool +append_to_buffer(char **buffer, size_t *buffer_len, char **end, const char *value) +{ + size_t value_len = strlen (value); + + /* Make sure there is enough room to add the value to the buffer */ + if (*buffer_len < strlen (*buffer) + value_len + 2) + { + printf ("Doubling buffer...\n"); + *buffer = realloc (*buffer, *buffer_len * 2); + if (*buffer == NULL) + /* XXX Here be dragons */ + return false; + + *buffer_len = *buffer_len * 2; + } + + /* Append value to buffer */ + size_t printed = sprintf (*end, "%s ", value); + if (printed < value_len + 1) + /* Here be dragons */ + return false; + + /* Move end pointer to last printed byte */ + *end += printed; + + return true; +} + static void build_environment(char **envp[], const struct lif_execute_opts *opts, const struct lif_interface *iface, const char *lifname, const char *phase, const char *mode) { @@ -132,21 +163,35 @@ build_environment(char **envp[], const struct lif_execute_opts *opts, const stru const struct lif_node *iter; bool did_address = false, did_gateway = false; + /* Allocate a buffer for all possible addresses, if any */ + char *addresses = calloc (BUFFER_LEN, 1); + size_t addresses_size = BUFFER_LEN; + char *addresses_end = addresses; + + /* Allocate a buffer for all possible gateways, if any */ + char *gateways = calloc (BUFFER_LEN, 1); + size_t gateways_size = BUFFER_LEN; + char *gateways_end = gateways; + LIF_DICT_FOREACH(iter, &iface->vars) { const struct lif_dict_entry *entry = iter->data; if (!strcmp(entry->key, "address")) { - if (did_address) - continue; - struct lif_address *addr = entry->data; char addrbuf[4096]; if (!lif_address_unparse(addr, addrbuf, sizeof addrbuf, true)) continue; + /* Append address to buffer */ + append_to_buffer(&addresses, &addresses_size, &addresses_end, addrbuf); + + /* Only print IF_ADDRESS once */ + if (did_address) + continue; + lif_environment_push(envp, "IF_ADDRESS", addrbuf); did_address = true; @@ -154,6 +199,9 @@ build_environment(char **envp[], const struct lif_execute_opts *opts, const stru } else if (!strcmp(entry->key, "gateway")) { + /* Append address to buffer */ + append_to_buffer(&gateways, &gateways_size, &gateways_end, entry->data); + if (did_gateway) continue; @@ -179,6 +227,11 @@ build_environment(char **envp[], const struct lif_execute_opts *opts, const stru lif_environment_push(envp, envkey, (const char *) entry->data); } + + if (addresses != NULL) + lif_environment_push(envp, "IF_ADDRESSES", addresses); + if (gateways != NULL) + lif_environment_push(envp, "IF_GATEWAYS", gateways); } bool From 44be0c072143aac70e79a22bddfe840a5380c99c Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Sun, 4 Oct 2020 02:52:32 +0200 Subject: [PATCH 2/8] Add myself to copyright notice in version information. Signed-off-by: Maximilian Wilhelm --- libifupdown/version.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libifupdown/version.c b/libifupdown/version.c index 1d8ea08..d540bc9 100644 --- a/libifupdown/version.c +++ b/libifupdown/version.c @@ -23,7 +23,8 @@ lif_common_version(void) { printf("%s %s\n", PACKAGE_NAME, PACKAGE_VERSION); - printf("\nCopyright (c) 2020 Ariadne Conill \n\n"); + printf("\nCopyright (c) 2020 Ariadne Conill \n"); + printf("\nCopyright (c) 2020 Maximilian Wilhelm \n\n"); printf("Permission to use, copy, modify, and/or distribute this software for any\n"); printf("purpose with or without fee is hereby granted, provided that the above\n"); From 5557804af9a8d15238f426d520703983fd44004a Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Tue, 6 Oct 2020 04:29:54 +0200 Subject: [PATCH 3/8] static executor: Update executor to use env vars. Signed-off-by: Maximilian Wilhelm --- executor-scripts/linux/static | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/executor-scripts/linux/static b/executor-scripts/linux/static index cb9af18..2790b64 100755 --- a/executor-scripts/linux/static +++ b/executor-scripts/linux/static @@ -2,12 +2,13 @@ set -e -[ -z "$VERBOSE" ] || set -x +[ -z "${VERBOSE}" ] || set -x + +[ -z "${IF_METRIC}" ] && IF_METRIC="1" +[ -n "${IF_VRF_TABLE}" ] && VRF_TABLE="table ${IF_VRF_TABLE}" +[ -n "${IF_VRF_MEMBER}" ] && VRF_TABLE="vrf ${IF_VRF_MEMBER}" +[ -n "${IF_METRIC}" ] && METRIC="metric ${IF_METRIC}" -[ -z "$IF_METRIC" ] && IF_METRIC="1" -[ -n "$IF_VRF_TABLE" ] && VRF_TABLE="table $IF_VRF_TABLE" -[ -n "$IF_VRF_MEMBER" ] && VRF_TABLE="vrf $IF_VRF_MEMBER" -[ -n "$IF_METRIC" ] && METRIC="metric $IF_METRIC" addr_family() { if [ "$1" != "${1#*[0-9].[0-9]}" ]; then @@ -20,22 +21,22 @@ addr_family() { } configure_addresses() { - for i in $(ifquery -p address -i $INTERFACES_FILE $IFACE); do - addrfam=$(addr_family $i) + for addr in ${IF_ADDRESSES}; do + addrfam=$(addr_family ${addr}) if [ "${IF_POINT_TO_POINT}" -a "${addrfam}" = "-4" ]; then PEER="peer ${IF_POINT_TO_POINT}" else PEER="" fi - ${MOCK} ip $addrfam addr $1 $i $PEER dev $IFACE + ${MOCK} ip "${addrfam}" addr "${1}" "${addr}" ${PEER} dev "${IFACE}" done } configure_gateways() { - for i in $(ifquery -p gateway -i $INTERFACES_FILE $IFACE); do - addrfam=$(addr_family $i) - ${MOCK} ip $addrfam route $1 default via $i $VRF_TABLE $METRIC dev $IFACE + for gw in ${IF_GATEWAYS}; do + addrfam=$(addr_family ${gw}) + ${MOCK} ip "${addrfam}" route "${1}" default via "${gw}" ${VRF_TABLE} ${METRIC} dev "${IFACE}" done } From 4a8230f9163ce8d0c4d69df28917b33f6e09ec3d Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Tue, 6 Oct 2020 04:43:32 +0200 Subject: [PATCH 4/8] tests: Update static executor test to reflect latest change. Signed-off-by: Maximilian Wilhelm --- tests/linux/static_test | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/linux/static_test b/tests/linux/static_test index 04c1a85..80afba0 100755 --- a/tests/linux/static_test +++ b/tests/linux/static_test @@ -2,7 +2,6 @@ . $(atf_get_srcdir)/../test_env.sh EXECUTOR="$(atf_get_srcdir)/../../executor-scripts/linux/static" -FIXTURES="$(atf_get_srcdir)/../fixtures" tests_init \ up \ @@ -14,7 +13,8 @@ tests_init \ metric_down up_body() { - export IFACE=eth0 PHASE=up MOCK=echo INTERFACES_FILE="$FIXTURES/static-eth0.interfaces" + export IFACE=eth0 PHASE=up MOCK=echo IF_ADDRESSES="203.0.113.2/24 2001:db8:1000:2::2/64" \ + IF_GATEWAYS="203.0.113.1 2001:db8:1000:2::1" atf_check -s exit:0 \ -o match:'addr add 203.0.113.2/24 dev eth0' \ -o match:'addr add 2001:db8:1000:2::2/64 dev eth0' \ @@ -24,8 +24,8 @@ up_body() { } up_ptp_body() { - export IFACE=eth0 PHASE=up MOCK=echo INTERFACES_FILE="$FIXTURES/static-eth0-ptp.interfaces" \ - IF_POINT_TO_POINT="192.0.2.1" + export IFACE=eth0 PHASE=up MOCK=echo IF_ADDRESSES="203.0.113.2/32 2001:db8:1000:2::2/64" \ + IF_GATEWAYS="192.0.2.1 2001:db8:1000:2::1" IF_POINT_TO_POINT="192.0.2.1" atf_check -s exit:0 \ -o match:'addr add 203.0.113.2/32 peer 192.0.2.1 dev eth0' \ -o match:'route add default via 192.0.2.1 metric 1 dev eth0' \ @@ -35,7 +35,8 @@ up_ptp_body() { } down_body() { - export IFACE=eth0 PHASE=down MOCK=echo INTERFACES_FILE="$FIXTURES/static-eth0.interfaces" + export IFACE=eth0 PHASE=down MOCK=echo IF_ADDRESSES="203.0.113.2/24 2001:db8:1000:2::2/64" \ + IF_GATEWAYS="203.0.113.1 2001:db8:1000:2::1" atf_check -s exit:0 \ -o match:'addr del 203.0.113.2/24 dev eth0' \ -o match:'addr del 2001:db8:1000:2::2/64 dev eth0' \ @@ -45,32 +46,28 @@ down_body() { } vrf_up_body() { - export IFACE=vrf-red PHASE=up MOCK=echo INTERFACES_FILE="$FIXTURES/vrf.interfaces" \ - IF_VRF_TABLE=1 + export IFACE=vrf-red PHASE=up MOCK=echo IF_GATEWAYS=203.0.113.2 IF_VRF_TABLE=1 atf_check -s exit:0 \ -o match:'route add default via 203.0.113.2 table 1' \ ${EXECUTOR} } vrf_down_body() { - export IFACE=vrf-red PHASE=down MOCK=echo INTERFACES_FILE="$FIXTURES/vrf.interfaces" \ - IF_VRF_TABLE=1 + export IFACE=vrf-red PHASE=down MOCK=echo IF_GATEWAYS=203.0.113.2 IF_VRF_TABLE=1 atf_check -s exit:0 \ -o match:'route del default via 203.0.113.2 table 1' \ ${EXECUTOR} } metric_up_body() { - export IFACE=vrf-red PHASE=up MOCK=echo INTERFACES_FILE="$FIXTURES/vrf.interfaces" \ - IF_VRF_TABLE=1 IF_METRIC=20 + export IFACE=vrf-red PHASE=up MOCK=echo IF_GATEWAYS=203.0.113.2 IF_VRF_TABLE=1 IF_METRIC=20 atf_check -s exit:0 \ -o match:'route add default via 203.0.113.2 table 1 metric 20' \ ${EXECUTOR} } metric_down_body() { - export IFACE=vrf-red PHASE=down MOCK=echo INTERFACES_FILE="$FIXTURES/vrf.interfaces" \ - IF_VRF_TABLE=1 IF_METRIC=20 + export IFACE=vrf-red PHASE=down MOCK=echo IF_GATEWAYS=203.0.113.2 IF_VRF_TABLE=1 IF_METRIC=20 atf_check -s exit:0 \ -o match:'route del default via 203.0.113.2 table 1 metric 20' \ ${EXECUTOR} From 99b0d67b8e70c5c923dc0c4fafa553aedc5c4f1c Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Tue, 6 Oct 2020 03:24:14 +0200 Subject: [PATCH 5/8] lifecycle: Remove debugging statement. Signed-off-by: Maximilian Wilhelm --- libifupdown/lifecycle.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 6cc749e..85226d0 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -122,7 +122,6 @@ append_to_buffer(char **buffer, size_t *buffer_len, char **end, const char *valu /* Make sure there is enough room to add the value to the buffer */ if (*buffer_len < strlen (*buffer) + value_len + 2) { - printf ("Doubling buffer...\n"); *buffer = realloc (*buffer, *buffer_len * 2); if (*buffer == NULL) /* XXX Here be dragons */ From 1a2298a759b163ab8c5fe1f8a98b77f1ce68fb1c Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Tue, 6 Oct 2020 03:25:23 +0200 Subject: [PATCH 6/8] lifecycle: Use snprintf() in favor of sprintf() Signed-off-by: Maximilian Wilhelm --- libifupdown/lifecycle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index 85226d0..d39afab 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -131,7 +131,7 @@ append_to_buffer(char **buffer, size_t *buffer_len, char **end, const char *valu } /* Append value to buffer */ - size_t printed = sprintf (*end, "%s ", value); + size_t printed = snprintf (*end, value_len + 2, "%s ", value); if (printed < value_len + 1) /* Here be dragons */ return false; From 5302bee850baf0e6908c90ea2b5de6143f86ac69 Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Tue, 6 Oct 2020 03:26:14 +0200 Subject: [PATCH 7/8] lifecycle: Don't leak allocated memory. Signed-off-by: Maximilian Wilhelm --- libifupdown/lifecycle.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c index d39afab..3da5dae 100644 --- a/libifupdown/lifecycle.c +++ b/libifupdown/lifecycle.c @@ -231,6 +231,10 @@ build_environment(char **envp[], const struct lif_execute_opts *opts, const stru lif_environment_push(envp, "IF_ADDRESSES", addresses); if (gateways != NULL) lif_environment_push(envp, "IF_GATEWAYS", gateways); + + /* Clean up */ + free (addresses); + free (gateways); } bool From 5d6c7732ed9ea7ca90e1239dc01a5d49df426ecc Mon Sep 17 00:00:00 2001 From: Maximilian Wilhelm Date: Tue, 6 Oct 2020 03:26:36 +0200 Subject: [PATCH 8/8] static executor: Ignore errors while removing addresses When having multiple addresses set from the same prefix they might/will(?) be configured as 'secondary' and implicitly removed when removing the non-secondary address. This leads ip complaining about not being able to remove the secondaries as they are already gone. So we ignore errors while deconfiguring addresses as they liked occur when removing a vanish address anyway. Signed-off-by: Maximilian Wilhelm --- executor-scripts/linux/static | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/executor-scripts/linux/static b/executor-scripts/linux/static index 2790b64..4cc6a7b 100755 --- a/executor-scripts/linux/static +++ b/executor-scripts/linux/static @@ -29,7 +29,16 @@ configure_addresses() { PEER="" fi - ${MOCK} ip "${addrfam}" addr "${1}" "${addr}" ${PEER} dev "${IFACE}" + if [ -z "${MOCK}" -a "${1}" = "del" ]; then + # When having multiple addresses set from the same prefix they might/will(?) be configured + # as 'secondary' and implicitly removed when removing the non-secondary address. This + # leads ip complaining about not being able to remove the secondaries as they are already + # gone. So we ignore errors while deconfiguring addresses as they liked occur when removing + # a vanish address anyway. + ${MOCK} ip "${addrfam}" addr "${1}" "${addr}" ${PEER} dev "${IFACE}" 2>/dev/null + else + ${MOCK} ip "${addrfam}" addr "${1}" "${addr}" ${PEER} dev "${IFACE}" + fi done }