From 9cb8c5593443d24c19e40bfd4fc06d672f8c554c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 27 Feb 2019 17:43:08 +0900 Subject: [PATCH 01/10] network: rename GatewayOnlink= to GatewayOnLink= But still GatewayOnlink= is supported for backward compatibility. --- man/systemd.network.xml | 2 +- src/network/networkd-network-gperf.gperf | 1 + src/network/networkd-route.c | 2 +- test/fuzz/fuzz-network-parser/directives.network | 1 + test/fuzz/fuzz-unit-file/directives.service | 2 +- test/test-network/conf/25-route-gateway-on-link.network | 2 +- 6 files changed, 6 insertions(+), 4 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 2058ba74a3..8b6c187f34 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -1081,7 +1081,7 @@ - GatewayOnlink= + GatewayOnLink= Takes a boolean. If set to true, the kernel does not have to check if the gateway is reachable directly by the current machine (i.e., the kernel does diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index 1437d83f73..7888bdf07a 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -117,6 +117,7 @@ Route.Scope, config_parse_route_scope, Route.PreferredSource, config_parse_preferred_src, 0, 0 Route.Table, config_parse_route_table, 0, 0 Route.MTUBytes, config_parse_route_mtu, AF_UNSPEC, 0 +Route.GatewayOnLink, config_parse_gateway_onlink, 0, 0 Route.GatewayOnlink, config_parse_gateway_onlink, 0, 0 Route.IPv6Preference, config_parse_ipv6_route_preference, 0, 0 Route.Protocol, config_parse_route_protocol, 0, 0 diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 6f67f18e64..537f0e48eb 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -946,7 +946,7 @@ int config_parse_gateway_onlink( r = parse_boolean(rvalue); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, - "Could not parse gateway onlink \"%s\", ignoring assignment: %m", rvalue); + "Could not parse %s=\"%s\", ignoring assignment: %m", lvalue, rvalue); return 0; } diff --git a/test/fuzz/fuzz-network-parser/directives.network b/test/fuzz/fuzz-network-parser/directives.network index 587327735f..d77c6989cd 100644 --- a/test/fuzz/fuzz-network-parser/directives.network +++ b/test/fuzz/fuzz-network-parser/directives.network @@ -60,6 +60,7 @@ Protocol= Table= Gateway= InitialAdvertisedReceiveWindow= +GatewayOnLink= GatewayOnlink= Type= InitialCongestionWindow= diff --git a/test/fuzz/fuzz-unit-file/directives.service b/test/fuzz/fuzz-unit-file/directives.service index 4d7526f636..2fdfd51d60 100644 --- a/test/fuzz/fuzz-unit-file/directives.service +++ b/test/fuzz/fuzz-unit-file/directives.service @@ -409,7 +409,7 @@ From= FwMark= GVRP= Gateway= -GatewayOnlink= +GatewayOnLink= GenericReceiveOffload= GenericSegmentationOffload= GratuitousARP= diff --git a/test/test-network/conf/25-route-gateway-on-link.network b/test/test-network/conf/25-route-gateway-on-link.network index aa64c25516..14894c54bf 100644 --- a/test/test-network/conf/25-route-gateway-on-link.network +++ b/test/test-network/conf/25-route-gateway-on-link.network @@ -10,4 +10,4 @@ Scope=link [Route] Gateway=149.10.125.65 -GatewayOnlink=true +GatewayOnLink=true From 4bec2f237bb714b160cd27edf28764fae658b110 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 27 Feb 2019 18:22:40 +0900 Subject: [PATCH 02/10] network: relax the .network file check Previously, if a .networ file contains invalid [Address] or [Route] section, then the file is completely dropped. This makes networkd just drops invalid sections. --- src/network/networkd-network.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index d20f27691d..0eda5b1373 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -196,8 +196,8 @@ static uint32_t network_get_stacked_netdevs_mtu(Network *network) { } static int network_verify(Network *network) { - Address *address; - Route *route; + Address *address, *address_next; + Route *route, *route_next; uint32_t mtu; assert(network); @@ -284,19 +284,24 @@ static int network_verify(Network *network) { network->dhcp_use_mtu = false; } - LIST_FOREACH(routes, route, network->static_routes) - if (!route->family) - return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), - "%s: Route section without Gateway field configured. " - "Ignoring %s.", - network->filename, network->filename); + LIST_FOREACH_SAFE(addresses, address, address_next, network->static_addresses) + if (address->family == AF_UNSPEC) { + log_warning("%s: Address section without Address= field configured. " + "Ignoring [Address] section from line %u.", + network->filename, address->section->line); - LIST_FOREACH(addresses, address, network->static_addresses) - if (!address->family) - return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), - "%s: Address section without Address field configured. " - "Ignoring %s.", - network->filename, network->filename); + address_free(address); + } + + LIST_FOREACH_SAFE(routes, route, route_next, network->static_routes) + if (route->family == AF_UNSPEC) { + log_warning("%s: Route section without Gateway=, Destination=, Source=, " + "or PreferredSource= field configured. " + "Ignoring [Route] section from line %u.", + network->filename, route->section->line); + + route_free(route); + } return 0; } From 54901fd2223bfae6cb68402c71cec8ec6af59c39 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 10:10:38 +0900 Subject: [PATCH 03/10] network: save GatewayOnLink= value as tristate in Route This should not change any behavior. But used in the later commit. --- src/network/networkd-route.c | 7 ++++++- src/network/networkd-route.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 537f0e48eb..232abc843f 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -59,6 +59,7 @@ int route_new(Route **ret) { .table = RT_TABLE_MAIN, .lifetime = USEC_INFINITY, .quickack = -1, + .gateway_onlink = -1, }; *ret = TAKE_PTR(route); @@ -565,6 +566,9 @@ int route_configure( if (r < 0) return log_error_errno(r, "Could not set scope: %m"); + if (route->gateway_onlink >= 0) + SET_FLAG(route->flags, RTNH_F_ONLINK, route->gateway_onlink); + r = sd_rtnl_message_route_set_flags(req, route->flags); if (r < 0) return log_error_errno(r, "Could not set flags: %m"); @@ -950,7 +954,8 @@ int config_parse_gateway_onlink( return 0; } - SET_FLAG(n->flags, RTNH_F_ONLINK, r); + n->gateway_onlink = r; + TAKE_PTR(n); return 0; } diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 4eddf5194f..7a9fc161bb 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -30,6 +30,7 @@ struct Route { uint32_t initrwnd; unsigned char pref; unsigned flags; + int gateway_onlink; union in_addr_union gw; union in_addr_union dst; From 4912ab7774ccc1eed07bfbbc6b87e8a23b1f5495 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 27 Feb 2019 18:41:49 +0900 Subject: [PATCH 04/10] network: enable GatewayOnLink= if Gateway= without static address configured And warn about that. But this only done if GatewayOnLink= is not specified. When it is explicitly disabled, then the flag will not be set. --- src/network/networkd-network.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 0eda5b1373..ab4e90543c 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -293,7 +293,7 @@ static int network_verify(Network *network) { address_free(address); } - LIST_FOREACH_SAFE(routes, route, route_next, network->static_routes) + LIST_FOREACH_SAFE(routes, route, route_next, network->static_routes) { if (route->family == AF_UNSPEC) { log_warning("%s: Route section without Gateway=, Destination=, Source=, " "or PreferredSource= field configured. " @@ -301,8 +301,19 @@ static int network_verify(Network *network) { network->filename, route->section->line); route_free(route); + continue; } + if (network->n_static_addresses == 0 && + in_addr_is_null(route->family, &route->gw) == 0 && + route->gateway_onlink < 0) { + log_warning("%s: Gateway= without static address configured. " + "Enabling GatewayOnLink= option.", + network->filename); + route->gateway_onlink = true; + } + } + return 0; } From af3b1498c6e9e9d9a17d07e92457ac535b1febcc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 27 Feb 2019 19:10:47 +0900 Subject: [PATCH 05/10] test-network: add testcase for #1850 --- ...hcp-client-gateway-onlink-implicit.network | 13 ++++++++++++ test/test-network/systemd-networkd-tests.py | 21 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 test/test-network/conf/dhcp-client-gateway-onlink-implicit.network diff --git a/test/test-network/conf/dhcp-client-gateway-onlink-implicit.network b/test/test-network/conf/dhcp-client-gateway-onlink-implicit.network new file mode 100644 index 0000000000..35aa8e7f50 --- /dev/null +++ b/test/test-network/conf/dhcp-client-gateway-onlink-implicit.network @@ -0,0 +1,13 @@ +[Match] +Name=veth99 + +[Network] +DHCP=ipv4 + +[Route] +Gateway=192.168.0.1 +Destination=10.0.0.0/8 + +[Route] +Gateway=192.168.0.1 +Destination=192.168.100.0/24 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index b9a61a92fe..ced5e048f9 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -1491,6 +1491,7 @@ class NetworkdNetworkDHCPClientTests(unittest.TestCase, Utilities): '25-vrf.network', 'dhcp-client-anonymize.network', 'dhcp-client-critical-connection.network', + 'dhcp-client-gateway-onlink-implicit.network', 'dhcp-client-ipv4-dhcp-settings.network', 'dhcp-client-ipv4-only-ipv6-disabled.network', 'dhcp-client-ipv4-only.network', @@ -1776,6 +1777,26 @@ class NetworkdNetworkDHCPClientTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'State: routable \(configured\)') + def test_dhcp_client_gateway_onlink_implicit(self): + self.copy_unit_to_networkd_unit_path('25-veth.netdev', 'dhcp-server-veth-peer.network', + 'dhcp-client-gateway-onlink-implicit.network') + self.start_networkd() + + self.assertTrue(self.link_exits('veth99')) + + self.start_dnsmasq() + + output = subprocess.check_output(['networkctl', 'status', 'veth99']).rstrip().decode('utf-8') + print(output) + self.assertRegex(output, '192.168.5') + + output = subprocess.check_output(['ip', 'route', 'list', 'dev', 'veth99', '10.0.0.0/8']).rstrip().decode('utf-8') + print(output) + self.assertRegex(output, 'onlink') + output = subprocess.check_output(['ip', 'route', 'list', 'dev', 'veth99', '192.168.100.0/24']).rstrip().decode('utf-8') + print(output) + self.assertRegex(output, 'onlink') + if __name__ == '__main__': unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=3)) From 01d4e7323cda652dffd2783ea65e14c5a535bb96 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 10:37:58 +0900 Subject: [PATCH 06/10] network: do not override previously specified family --- src/network/networkd-route.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 232abc843f..26fcae9cf2 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -704,18 +704,20 @@ int config_parse_gateway( r = route_new_static(network, NULL, 0, &n); } else r = route_new_static(network, filename, section_line, &n); - if (r < 0) return r; - r = in_addr_from_string_auto(rvalue, &n->family, &n->gw); + if (n->family == AF_UNSPEC) + r = in_addr_from_string_auto(rvalue, &n->family, &n->gw); + else + r = in_addr_from_string(n->family, rvalue, &n->gw); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Route is invalid, ignoring assignment: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, + "Invalid %s='%s', ignoring assignment: %m", lvalue, rvalue); return 0; } TAKE_PTR(n); - return 0; } @@ -745,15 +747,17 @@ int config_parse_preferred_src( if (r < 0) return r; - r = in_addr_from_string_auto(rvalue, &n->family, &n->prefsrc); + if (n->family == AF_UNSPEC) + r = in_addr_from_string_auto(rvalue, &n->family, &n->prefsrc); + else + r = in_addr_from_string(n->family, rvalue, &n->prefsrc); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, EINVAL, - "Preferred source is invalid, ignoring assignment: %s", rvalue); + "Invalid %s='%s', ignoring assignment: %m", lvalue, rvalue); return 0; } TAKE_PTR(n); - return 0; } @@ -794,11 +798,13 @@ int config_parse_destination( } else assert_not_reached(lvalue); - r = in_addr_prefix_from_string_auto(rvalue, &n->family, buffer, prefixlen); + if (n->family == AF_UNSPEC) + r = in_addr_prefix_from_string_auto(rvalue, &n->family, buffer, prefixlen); + else + r = in_addr_prefix_from_string(rvalue, n->family, buffer, prefixlen); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, - "Route %s= prefix is invalid, ignoring assignment: %s", - lvalue, rvalue); + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Invalid %s='%s', ignoring assignment: %m", lvalue, rvalue); return 0; } From f205a92a5ce47138e0c1a27a1bb070dda0db86d3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 10:39:14 +0900 Subject: [PATCH 07/10] network: cleanup logging in route related config parsers --- src/network/networkd-route.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 26fcae9cf2..5594ba1224 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1030,7 +1030,8 @@ int config_parse_route_protocol( else { r = safe_atou8(rvalue , &n->protocol); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Could not parse route protocol \"%s\", ignoring assignment: %m", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, + "Could not parse route protocol \"%s\", ignoring assignment: %m", rvalue); return 0; } } @@ -1070,7 +1071,8 @@ int config_parse_route_type( else if (streq(rvalue, "throw")) n->type = RTN_THROW; else { - log_syntax(unit, LOG_ERR, filename, line, r, "Could not parse route type \"%s\", ignoring assignment: %m", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, + "Could not parse route type \"%s\", ignoring assignment: %m", rvalue); return 0; } @@ -1106,9 +1108,14 @@ int config_parse_tcp_window( return r; r = parse_size(rvalue, 1024, &k); - if (r < 0 || k > UINT32_MAX) { + if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, - "Could not parse TCP %s \"%s\" bytes, ignoring assignment: %m", rvalue, lvalue); + "Could not parse TCP %s \"%s\", ignoring assignment: %m", lvalue, rvalue); + return 0; + } + if (k > UINT32_MAX) { + log_syntax(unit, LOG_ERR, filename, line, 0, + "Specified TCP %s \"%s\" is too large, ignoring assignment: %m", lvalue, rvalue); return 0; } @@ -1116,10 +1123,8 @@ int config_parse_tcp_window( n->initcwnd = k; else if (streq(lvalue, "InitialAdvertisedReceiveWindow")) n->initrwnd = k; - else { - log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse TCP %s: %s", lvalue, rvalue); - return 0; - } + else + assert_not_reached("Invalid TCP window type."); TAKE_PTR(n); return 0; @@ -1153,7 +1158,8 @@ int config_parse_quickack( k = parse_boolean(rvalue); if (k < 0) { - log_syntax(unit, LOG_ERR, filename, line, k, "Failed to parse TCP quickack, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, k, + "Failed to parse TCP quickack, ignoring: %s", rvalue); return 0; } From 4aa4c4b0f42cfbaa39b6559090cebcca27ddbc8d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 10:54:17 +0900 Subject: [PATCH 08/10] network: avoid address section freed Otherwise, if HomeAddress= or friends are specified at the first line of a section, then its assignment will be ignored. --- src/network/networkd-address.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index aa827d6ba6..cb23213b5d 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -904,6 +904,7 @@ int config_parse_address_flags(const char *unit, else if (streq(lvalue, "AutoJoin")) n->autojoin = r; + n = NULL; return 0; } From 33680b0a262ef75a1f5cce1e8e24c5b44752c456 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 10:56:33 +0900 Subject: [PATCH 09/10] network: simplify config_parse_lifetime() --- src/network/networkd-address.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index cb23213b5d..d538ee7fde 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -840,26 +840,20 @@ int config_parse_lifetime(const char *unit, if (r < 0) return r; - if (STR_IN_SET(rvalue, "forever", "infinity")) { - n->cinfo.ifa_prefered = CACHE_INFO_INFINITY_LIFE_TIME; - n = NULL; - - return 0; - } - - r = safe_atou(rvalue, &k); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse PreferredLifetime, ignoring: %s", rvalue); - return 0; - } - - if (k != 0) - log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid PreferredLifetime value, ignoring: %d", k); + /* We accept only "forever", "infinity", or "0". */ + if (STR_IN_SET(rvalue, "forever", "infinity")) + k = CACHE_INFO_INFINITY_LIFE_TIME; + else if (streq(rvalue, "0")) + k = 0; else { - n->cinfo.ifa_prefered = k; - n = NULL; + log_syntax(unit, LOG_ERR, filename, line, 0, + "Invalid PreferredLifetime= value, ignoring: %s", rvalue); + return 0; } + n->cinfo.ifa_prefered = k; + n = NULL; + return 0; } From 2850cd40eef12dca993825e303c178bc74ea0cdf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 10:57:20 +0900 Subject: [PATCH 10/10] network: wrap long lines --- src/network/networkd-address.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index d538ee7fde..7c1ee75405 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -690,13 +690,15 @@ int config_parse_broadcast( return r; if (n->family == AF_INET6) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Broadcast is not valid for IPv6 addresses, ignoring assignment: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, + "Broadcast is not valid for IPv6 addresses, ignoring assignment: %s", rvalue); return 0; } r = in_addr_from_string(AF_INET, rvalue, (union in_addr_union*) &n->broadcast); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Broadcast is invalid, ignoring assignment: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, + "Broadcast is invalid, ignoring assignment: %s", rvalue); return 0; } @@ -802,7 +804,8 @@ int config_parse_label( return r; if (!address_label_valid(rvalue)) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Interface label is too long or invalid, ignoring assignment: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, + "Interface label is too long or invalid, ignoring assignment: %s", rvalue); return 0; } @@ -811,7 +814,6 @@ int config_parse_label( return log_oom(); n = NULL; - return 0; } @@ -883,7 +885,8 @@ int config_parse_address_flags(const char *unit, r = parse_boolean(rvalue); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse address flag, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, + "Failed to parse address flag, ignoring: %s", rvalue); return 0; } @@ -897,6 +900,8 @@ int config_parse_address_flags(const char *unit, n->prefix_route = r; else if (streq(lvalue, "AutoJoin")) n->autojoin = r; + else + assert_not_reached("Invalid address flag type."); n = NULL; return 0; @@ -935,13 +940,13 @@ int config_parse_address_scope(const char *unit, else { r = safe_atou8(rvalue , &n->scope); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Could not parse address scope \"%s\", ignoring assignment: %m", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, + "Could not parse address scope \"%s\", ignoring assignment: %m", rvalue); return 0; } } n = NULL; - return 0; }