From b9f1beb06e624cc3d6bb0bb8723347392376e200 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 27 Nov 2019 13:18:52 +0100 Subject: [PATCH] all: add support for "scope" attribute for IPv4 routes - systemd-networkd and initscripts both support it. - it seems suggested to configure routes with scope "link" on AWS. - the scope is only supported for IPv4 routes. Kernel ignores the attribute for IPv6 routes. - we don't support the aliases like "link" or "global". Instead only the numeric value is supported. This is different from systemd-networkd, which accepts names like "global" and "link", but no numerical values. I think restricting ourself only to the aliases unnecessarily limits what is possible on netlink. The alternative would be to allow aliases and numbers both, but that causes multiple ways to define something and has thus downsides. So, only numeric values. - when setting rtm_scope to RT_SCOPE_NOWHERE (0, the default), kernel will coerce that to RT_SCOPE_LINK. This ambiguity of nowhere vs. link is a problem, but we don't do anything about it. - The other problem is, that when deleting a route with scope RT_SCOPE_NOWHERE, this acts as a wild care and removes the first route that matches (given the other route attributes). That means, NetworkManager has no meaningful way to delete a route with scope zero, there is always the danger that we might delete the wrong route. But this is nothing new to this patch. The problem existed already previously, except that NetworkManager could only add routes with scope nowhere (i.e. link). --- NEWS | 1 + libnm-core/nm-setting-ip-config.c | 1 + libnm-core/nm-setting-ip-config.h | 1 + src/nm-ip4-config.c | 7 +++- src/platform/nm-platform.h | 2 +- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 39 +++++++++++++++++++ .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 2 + .../route-test-wired-static-routes | 4 +- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 2 + 9 files changed, 55 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index b33d76d8be..daead33e0b 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,7 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! type instances would have been created by NMClient for a while now. * DHCP: switch "internal" DHCPv4 plugin from code based on systemd to use nettools' n-dhcp4 library. +* Add support for "scope" attribute for IPv4 routes. * libnm: heavily internal rework NMClient. This slims down libnm and makes the implementation more efficient. NMClient should work now well with a separate GMainContext. diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index 0d7e9b0530..cd833e09fc 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -1208,6 +1208,7 @@ static const NMVariantAttributeSpec *const ip_route_attribute_spec[] = { NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_LOCK_WINDOW, G_VARIANT_TYPE_BOOLEAN, .v4 = TRUE, .v6 = TRUE, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_MTU, G_VARIANT_TYPE_UINT32, .v4 = TRUE, .v6 = TRUE, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_ONLINK, G_VARIANT_TYPE_BOOLEAN, .v4 = TRUE, .v6 = TRUE, ), + NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_SCOPE, G_VARIANT_TYPE_BYTE, .v4 = TRUE, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_SRC, G_VARIANT_TYPE_STRING, .v4 = TRUE, .v6 = TRUE, .str_type = 'a', ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_TABLE, G_VARIANT_TYPE_UINT32, .v4 = TRUE, .v6 = TRUE, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_TOS, G_VARIANT_TYPE_BYTE, .v4 = TRUE, ), diff --git a/libnm-core/nm-setting-ip-config.h b/libnm-core/nm-setting-ip-config.h index feea89db22..07581976db 100644 --- a/libnm-core/nm-setting-ip-config.h +++ b/libnm-core/nm-setting-ip-config.h @@ -138,6 +138,7 @@ gboolean nm_ip_route_attribute_validate (const char *name, #define NM_IP_ROUTE_ATTRIBUTE_LOCK_WINDOW "lock-window" #define NM_IP_ROUTE_ATTRIBUTE_MTU "mtu" #define NM_IP_ROUTE_ATTRIBUTE_ONLINK "onlink" +#define NM_IP_ROUTE_ATTRIBUTE_SCOPE "scope" #define NM_IP_ROUTE_ATTRIBUTE_SRC "src" #define NM_IP_ROUTE_ATTRIBUTE_TABLE "table" #define NM_IP_ROUTE_ATTRIBUTE_TOS "tos" diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index 30bd5c6790..cd14fb8f94 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -862,8 +862,13 @@ _nm_ip_config_merge_route_attributes (int addr_family, GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_TABLE, table, UINT32, uint32, 0); r->table_coerced = nm_platform_route_table_coerce (table ?: (route_table ?: RT_TABLE_MAIN)); - if (addr_family == AF_INET) + if (addr_family == AF_INET) { + guint8 scope; + GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_TOS, r4->tos, BYTE, byte, 0); + GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_SCOPE, scope, BYTE, byte, RT_SCOPE_NOWHERE); + r4->scope_inv = nm_platform_route_scope_inv (scope); + } GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_ONLINK, onlink, BOOLEAN, boolean, FALSE); diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 0340136c86..4bd8e34d1e 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -496,7 +496,7 @@ struct _NMPlatformIP4Route { /* The bitwise inverse of the route scope rtm_scope. It is inverted so that the * default value (RT_SCOPE_NOWHERE) is zero. Use nm_platform_route_scope_inv() - * to convert back and forth between the inverese representation and the + * to convert back and forth between the inverse representation and the * real value. * * rtm_scope is part of the primary key for IPv4 routes. When deleting a route, diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 66658bde75..5f61d4fb0f 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "nm-glib-aux/nm-secret-utils.h" #include "nm-connection.h" @@ -791,6 +792,7 @@ enum { PARSE_LINE_ATTR_ROUTE_SRC, PARSE_LINE_ATTR_ROUTE_FROM, PARSE_LINE_ATTR_ROUTE_TOS, + PARSE_LINE_ATTR_ROUTE_SCOPE, PARSE_LINE_ATTR_ROUTE_ONLINK, PARSE_LINE_ATTR_ROUTE_WINDOW, PARSE_LINE_ATTR_ROUTE_CWND, @@ -814,6 +816,7 @@ enum { #define PARSE_LINE_TYPE_ADDR_WITH_PREFIX 'p' #define PARSE_LINE_TYPE_IFNAME 'i' #define PARSE_LINE_TYPE_FLAG 'f' +#define PARSE_LINE_TYPE_ROUTE_SCOPE 'S' /** * parse_route_line: @@ -856,6 +859,9 @@ parse_route_line (const char *line, .type = PARSE_LINE_TYPE_UINT8, .int_base_16 = TRUE, .ignore = PARSE_LINE_AF_FLAG_FOR_IPV6, }, + [PARSE_LINE_ATTR_ROUTE_SCOPE] = { .key = NM_IP_ROUTE_ATTRIBUTE_SCOPE, + .type = PARSE_LINE_TYPE_ROUTE_SCOPE, + .ignore = PARSE_LINE_AF_FLAG_FOR_IPV6, }, [PARSE_LINE_ATTR_ROUTE_ONLINK] = { .key = NM_IP_ROUTE_ATTRIBUTE_ONLINK, .type = PARSE_LINE_TYPE_FLAG, }, [PARSE_LINE_ATTR_ROUTE_WINDOW] = { .key = NM_IP_ROUTE_ATTRIBUTE_WINDOW, @@ -969,6 +975,9 @@ parse_route_line (const char *line, case PARSE_LINE_TYPE_FLAG: i_words++; goto next; + case PARSE_LINE_TYPE_ROUTE_SCOPE: + i_words++; + goto parse_line_type_route_scope; default: nm_assert_not_reached (); } @@ -989,6 +998,35 @@ parse_route_line (const char *line, "Unrecognized argument (\"to\" is duplicate or \"%s\" is garbage)", w); return -EINVAL; +parse_line_type_route_scope: + s = words[i_words]; + if (!s) + goto err_word_missing_argument; + if (nm_streq (s, "global")) + p_data->v.uint8 = RT_SCOPE_UNIVERSE; + else if (nm_streq (s, "nowhere")) + p_data->v.uint8 = RT_SCOPE_NOWHERE; + else if (nm_streq (s, "host")) + p_data->v.uint8 = RT_SCOPE_HOST; + else if (nm_streq (s, "link")) + p_data->v.uint8 = RT_SCOPE_LINK; + else if (nm_streq (s, "site")) + p_data->v.uint8 = RT_SCOPE_SITE; + else { + p_data->v.uint8 = _nm_utils_ascii_str_to_int64 (s, + 0, + 0, + G_MAXUINT8, + 0);; + if (errno) { + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, + "Argument for \"%s\" is not a valid number", w); + return -EINVAL; + } + } + i_words++; + goto next; + parse_line_type_uint8: s = words[i_words]; if (!s) @@ -1149,6 +1187,7 @@ next: switch (p_info->type) { case PARSE_LINE_TYPE_UINT8: + case PARSE_LINE_TYPE_ROUTE_SCOPE: nm_ip_route_set_attribute (route, p_info->key, g_variant_new_byte (p_data->v.uint8)); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index c457b810ea..ea2cef803c 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -2084,6 +2084,8 @@ get_route_attributes_string (NMIPRoute *route, int family) /* we also have a corresponding attribute with the numeric value. The * lock setting is handled above. */ } + } else if (nm_streq (names[i], NM_IP_ROUTE_ATTRIBUTE_SCOPE)) { + g_string_append_printf (str, "%s %u", names[i], (unsigned) g_variant_get_byte (attr)); } else if (nm_streq (names[i], NM_IP_ROUTE_ATTRIBUTE_TOS)) { g_string_append_printf (str, "%s 0x%02x", names[i], (unsigned) g_variant_get_byte (attr)); } else if (nm_streq (names[i], NM_IP_ROUTE_ATTRIBUTE_TABLE)) { diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes b/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes index 8d6aaac2f6..5d02c62ef3 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes @@ -6,10 +6,10 @@ ADDRESS1=44.55.66.77 NETMASK1=255.255.255.255 GATEWAY1=192.168.1.7 METRIC1=3 -OPTIONS1="mtu lock 9000 cwnd 12 src 1.1.1.1 tos 0x28 window 30000 initcwnd lock 13 initrwnd 14" +OPTIONS1="mtu lock 9000 cwnd 12 src 1.1.1.1 tos 0x28 window 30000 scope 10 initcwnd lock 13 initrwnd 14" ADDRESS2=44.55.66.78 NETMASK2=255.255.255.255 GATEWAY2=192.168.1.8 METRIC2=3 -OPTIONS2="mtu lock 9000 cwnd 12 src 1.1.1.1 tos 0x28 onlink window 30000 initcwnd lock 13 initrwnd 14" +OPTIONS2="mtu lock 9000 cwnd 12 src 1.1.1.1 tos 0x28 onlink window 30000 initcwnd lock 13 initrwnd 14 scope link" diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 459a78eca0..d3651978f2 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -1334,6 +1334,7 @@ test_read_wired_static_routes (void) nmtst_assert_route_attribute_boolean (ip4_route, NM_IP_ROUTE_ATTRIBUTE_LOCK_MTU, TRUE); nmtst_assert_route_attribute_boolean (ip4_route, NM_IP_ROUTE_ATTRIBUTE_LOCK_INITCWND, TRUE); nmtst_assert_route_attribute_string (ip4_route, NM_IP_ROUTE_ATTRIBUTE_SRC, "1.1.1.1"); + nmtst_assert_route_attribute_byte (ip4_route, NM_IP_ROUTE_ATTRIBUTE_SCOPE, 10); ip4_route = nm_setting_ip_config_get_route (s_ip4, 2); g_assert (ip4_route); @@ -1351,6 +1352,7 @@ test_read_wired_static_routes (void) nmtst_assert_route_attribute_boolean (ip4_route, NM_IP_ROUTE_ATTRIBUTE_LOCK_INITCWND, TRUE); nmtst_assert_route_attribute_string (ip4_route, NM_IP_ROUTE_ATTRIBUTE_SRC, "1.1.1.1"); nmtst_assert_route_attribute_boolean (ip4_route, NM_IP_ROUTE_ATTRIBUTE_ONLINK, TRUE); + nmtst_assert_route_attribute_byte (ip4_route, NM_IP_ROUTE_ATTRIBUTE_SCOPE, 253); g_object_unref (connection); }