diff options
-rw-r--r-- | man/networkd.conf.xml | 13 | ||||
-rw-r--r-- | src/network/networkd-gperf.gperf | 2 | ||||
-rw-r--r-- | src/network/networkd-manager.c | 3 | ||||
-rw-r--r-- | src/network/networkd-manager.h | 3 | ||||
-rw-r--r-- | src/network/networkd-route.c | 232 | ||||
-rw-r--r-- | src/network/networkd-route.h | 6 | ||||
-rw-r--r-- | src/network/networkd-routing-policy-rule.c | 24 | ||||
-rw-r--r-- | src/network/test-network.c | 50 |
8 files changed, 226 insertions, 107 deletions
diff --git a/man/networkd.conf.xml b/man/networkd.conf.xml index 12ddcf4502..dcce2095ed 100644 --- a/man/networkd.conf.xml +++ b/man/networkd.conf.xml @@ -72,11 +72,14 @@ <varlistentry> <term><varname>RouteTable=</varname></term> - <listitem><para>Specifies the route table name. Takes a route name and table number separated with a - colon. (<literal><replaceable>name</replaceable>:<replaceable>integer</replaceable></literal>. The - route table number must be an integer in the range 1…4294967295. This setting can be specified - multiple times. If an empty string is specified, then all options specified earlier are cleared. - Defaults to unset.</para></listitem> + <listitem><para>Defines the route table name. Takes a whitespace-separated list of the pairs of + route table name and number. The route table name and number in each pair are separated with a + colon, i.e., <literal><replaceable>name</replaceable>:<replaceable>number</replaceable></literal>. + The route table name must not be <literal>default</literal>, <literal>main</literal>, or + <literal>local</literal>, as these route table names are predefined with route table number 253, + 254, and 255, respectively. The route table number must be an integer in the range 1…4294967295. + This setting can be specified multiple times. If an empty string is specified, then the list + specified earlier are cleared. Defaults to unset.</para></listitem> </varlistentry> </variablelist> diff --git a/src/network/networkd-gperf.gperf b/src/network/networkd-gperf.gperf index 4bfb0fe088..b2a2f55790 100644 --- a/src/network/networkd-gperf.gperf +++ b/src/network/networkd-gperf.gperf @@ -23,6 +23,6 @@ struct ConfigPerfItem; Network.SpeedMeter, config_parse_bool, 0, offsetof(Manager, use_speed_meter) Network.SpeedMeterIntervalSec, config_parse_sec, 0, offsetof(Manager, speed_meter_interval_usec) Network.ManageForeignRoutes, config_parse_bool, 0, offsetof(Manager, manage_foreign_routes) -Network.RouteTable, config_parse_route_table_names, 0, offsetof(Manager, route_tables) +Network.RouteTable, config_parse_route_table_names, 0, 0 DHCP.DUIDType, config_parse_duid_type, 0, offsetof(Manager, duid) DHCP.DUIDRawData, config_parse_duid_rawdata, 0, offsetof(Manager, duid) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index befecdc323..0e00cde87a 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -875,7 +875,8 @@ void manager_free(Manager *m) { ordered_set_free_free(m->address_pools); - m->route_tables = hashmap_free_free_key(m->route_tables); + hashmap_free(m->route_table_names_by_number); + hashmap_free(m->route_table_numbers_by_name); /* routing_policy_rule_free() access m->rules and m->rules_foreign. * So, it is necessary to set NULL after the sets are freed. */ diff --git a/src/network/networkd-manager.h b/src/network/networkd-manager.h index 88b9f6bd23..f553abcc54 100644 --- a/src/network/networkd-manager.h +++ b/src/network/networkd-manager.h @@ -66,7 +66,8 @@ struct Manager { Set *routes_foreign; /* Route table name */ - Hashmap *route_tables; + Hashmap *route_table_numbers_by_name; + Hashmap *route_table_names_by_number; /* For link speed meter*/ bool use_speed_meter; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index cfb9befc61..a98394c1ca 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -87,20 +87,68 @@ static const char * const route_table_table[] = { [RT_TABLE_LOCAL] = "local", }; -DEFINE_STRING_TABLE_LOOKUP(route_table, int); +DEFINE_PRIVATE_STRING_TABLE_LOOKUP(route_table, int); -#define ROUTE_TABLE_STR_MAX CONST_MAX(DECIMAL_STR_MAX(int), STRLEN("default") + 1) -static const char *format_route_table(int table, char *buf, size_t size) { +int manager_get_route_table_from_string(const Manager *m, const char *s, uint32_t *ret) { + uint32_t t; + int r; + + assert(m); + assert(s); + assert(ret); + + r = route_table_from_string(s); + if (r >= 0) { + *ret = (uint32_t) r; + return 0; + } + + t = PTR_TO_UINT32(hashmap_get(m->route_table_numbers_by_name, s)); + if (t != 0) { + *ret = t; + return 0; + } + + r = safe_atou32(s, &t); + if (r < 0) + return r; + + if (t == 0) + return -ERANGE; + + *ret = t; + return 0; +} + +int manager_get_route_table_to_string(const Manager *m, uint32_t table, char **ret) { + _cleanup_free_ char *str = NULL; const char *s; - char *p = buf; + + assert(m); + assert(ret); + + if (table == 0) + return -EINVAL; s = route_table_to_string(table); - if (s) - strpcpy(&p, size, s); - else - strpcpyf(&p, size, "%d", table); + if (!s) + s = hashmap_get(m->route_table_names_by_number, UINT32_TO_PTR(table)); - return buf; + if (s) { + /* Currently, this is only used in debugging logs. To not confuse any bug + * reports, let's include the table number. */ + if (asprintf(&str, "%s(%" PRIu32 ")", s, table) < 0) + return -ENOMEM; + + *ret = TAKE_PTR(str); + return 0; + } + + if (asprintf(&str, "%" PRIu32, table) < 0) + return -ENOMEM; + + *ret = TAKE_PTR(str); + return 0; } static const char * const route_protocol_table[] = { @@ -584,15 +632,16 @@ static bool route_type_is_reject(const Route *route) { return IN_SET(route->type, RTN_UNREACHABLE, RTN_PROHIBIT, RTN_BLACKHOLE, RTN_THROW); } -static void log_route_debug(const Route *route, const char *str, const Link *link) { +static void log_route_debug(const Route *route, const char *str, const Link *link, const Manager *m) { assert(route); assert(str); + assert(m); /* link may be NULL. */ if (DEBUG_LOGGING) { - _cleanup_free_ char *dst = NULL, *dst_prefixlen = NULL, *src = NULL, *gw = NULL, *prefsrc = NULL; - char scope[ROUTE_SCOPE_STR_MAX], table[ROUTE_TABLE_STR_MAX], protocol[ROUTE_PROTOCOL_STR_MAX]; + _cleanup_free_ char *dst = NULL, *dst_prefixlen = NULL, *src = NULL, *gw = NULL, *prefsrc = NULL, *table = NULL; + char scope[ROUTE_SCOPE_STR_MAX], protocol[ROUTE_PROTOCOL_STR_MAX]; if (!in_addr_is_null(route->family, &route->dst)) { (void) in_addr_to_string(route->family, &route->dst, &dst); @@ -604,12 +653,13 @@ static void log_route_debug(const Route *route, const char *str, const Link *lin (void) in_addr_to_string(route->gw_family, &route->gw, &gw); if (!in_addr_is_null(route->family, &route->prefsrc)) (void) in_addr_to_string(route->family, &route->prefsrc, &prefsrc); + (void) manager_get_route_table_to_string(m, route->table, &table); log_link_debug(link, "%s route: dst: %s%s, src: %s, gw: %s, prefsrc: %s, scope: %s, table: %s, proto: %s, type: %s", str, strna(dst), strempty(dst_prefixlen), strna(src), strna(gw), strna(prefsrc), format_route_scope(route->scope, scope, sizeof(scope)), - format_route_table(route->table, table, sizeof(table)), + strna(table), format_route_protocol(route->protocol, protocol, sizeof(protocol)), strna(route_type_to_string(route->type))); } @@ -751,7 +801,7 @@ int route_remove( manager = link->manager; /* link may be NULL! */ - log_route_debug(route, "Removing", link); + log_route_debug(route, "Removing", link, manager); r = sd_rtnl_message_new_route(manager->rtnl, &req, RTM_DELROUTE, route->family, @@ -1066,7 +1116,7 @@ int route_configure( return log_link_error_errno(link, SYNTHETIC_ERRNO(E2BIG), "Too many routes are configured, refusing: %m"); - log_route_debug(route, "Configuring", link); + log_route_debug(route, "Configuring", link, link->manager); r = sd_rtnl_message_new_route(link->manager->rtnl, &req, RTM_NEWROUTE, route->family, @@ -1295,10 +1345,10 @@ static int process_route_one(Manager *manager, Link *link, uint16_t type, const case RTM_NEWROUTE: if (!route) { if (!manager->manage_foreign_routes) - log_route_debug(tmp, "Ignoring received foreign", link); + log_route_debug(tmp, "Ignoring received foreign", link, manager); else { /* A route appeared that we did not request */ - log_route_debug(tmp, "Remembering foreign", link); + log_route_debug(tmp, "Remembering foreign", link, manager); r = route_add_foreign(manager, link, tmp, NULL); if (r < 0) { log_link_warning_errno(link, r, "Failed to remember foreign route, ignoring: %m"); @@ -1306,14 +1356,15 @@ static int process_route_one(Manager *manager, Link *link, uint16_t type, const } } } else - log_route_debug(tmp, "Received remembered", link); + log_route_debug(tmp, "Received remembered", link, manager); break; case RTM_DELROUTE: log_route_debug(tmp, route ? "Forgetting" : - manager->manage_foreign_routes ? "Kernel removed unknown" : "Ignoring received foreign", link); + manager->manage_foreign_routes ? "Kernel removed unknown" : "Ignoring received foreign", + link, manager); route_free(route); break; @@ -1868,28 +1919,6 @@ int config_parse_route_scope( return 0; } -int route_table_from_string_full(Manager *m, const char *s, uint32_t *ret) { - int r; - - assert(s); - assert(m); - assert(ret); - - r = route_table_from_string(s); - if (r >= 0) { - *ret = (uint32_t) r; - return 0; - } - - uint32_t t = PTR_TO_UINT32(hashmap_get(m->route_tables, s)); - if (t != 0) { - *ret = t; - return 0; - } - - return safe_atou32(s, ret); -} - int config_parse_route_table( const char *unit, const char *filename, @@ -1921,7 +1950,7 @@ int config_parse_route_table( return 0; } - r = route_table_from_string_full(network->manager, rvalue, &n->table); + r = manager_get_route_table_from_string(network->manager, rvalue, &n->table); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Could not parse route table number \"%s\", ignoring assignment: %m", rvalue); @@ -2385,63 +2414,98 @@ int config_parse_route_table_names( void *data, void *userdata) { - _cleanup_free_ char *name = NULL; - Hashmap **s = data; - uint32_t table; - const char *p; + Manager *m = userdata; int r; assert(filename); assert(lvalue); assert(rvalue); - assert(data); + assert(userdata); if (isempty(rvalue)) { - *s = hashmap_free_free_key(*s); + m->route_table_names_by_number = hashmap_free(m->route_table_names_by_number); + m->route_table_numbers_by_name = hashmap_free(m->route_table_numbers_by_name); return 0; } - p = rvalue; - r = extract_first_word(&p, &name, ":", 0); - if (r == -ENOMEM) - return log_oom(); - if (r <= 0 || isempty(p)) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Invalid RouteTable=, ignoring assignment: %s", rvalue); - return 0; - } + for (const char *p = rvalue;;) { + _cleanup_free_ char *name = NULL; + uint32_t table; + char *num; - if (STR_IN_SET(name, "default", "main","local")) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Route table name %s already preconfigured. Ignoring assignment: %s", name, rvalue); - return 0; - } + r = extract_first_word(&p, &name, NULL, 0); + if (r == -ENOMEM) + return log_oom(); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Invalid RouteTable=, ignoring assignment: %s", rvalue); + return 0; + } + if (r == 0) + return 0; - r = safe_atou32(p, &table); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse RouteTable=, ignoring assignment: %s", p); - return 0; - } + num = strchr(name, ':'); + if (!num) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Invalid route table name and number pair, ignoring assignment: %s", name); + continue; + } - if (table == 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid RouteTable=, ignoring assignment: %s", p); - return 0; - } + *num++ = '\0'; - r = hashmap_ensure_put(s, &string_hash_ops, name, UINT32_TO_PTR(table)); - if (r == -ENOMEM) - return log_oom(); - if (r == -EEXIST) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Specified RouteTable= name and value pair conflicts with others, ignoring assignment: %s", rvalue); - return 0; - } - if (r > 0) - TAKE_PTR(name); + if (STR_IN_SET(name, "default", "main", "local")) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Route table name %s already predefined. Ignoring assignment: %s:%s", name, name, num); + continue; + } - return 0; + r = safe_atou32(num, &table); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse route table number '%s', ignoring assignment: %s:%s", num, name, num); + continue; + } + if (table == 0) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Invalid route table number, ignoring assignment: %s:%s", name, num); + continue; + } + + r = hashmap_ensure_put(&m->route_table_numbers_by_name, &string_hash_ops_free, name, UINT32_TO_PTR(table)); + if (r == -ENOMEM) + return log_oom(); + if (r == -EEXIST) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Specified route table name and number pair conflicts with others, ignoring assignment: %s:%s", name, num); + continue; + } + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to store route table name and number pair, ignoring assignment: %s:%s", name, num); + continue; + } + if (r == 0) + /* The entry is duplicated. It should not be added to route_table_names_by_number hashmap. */ + continue; + + r = hashmap_ensure_put(&m->route_table_names_by_number, NULL, UINT32_TO_PTR(table), name); + if (r < 0) { + hashmap_remove(m->route_table_numbers_by_name, name); + + if (r == -ENOMEM) + return log_oom(); + if (r == -EEXIST) + log_syntax(unit, LOG_WARNING, filename, line, r, + "Specified route table name and number pair conflicts with others, ignoring assignment: %s:%s", name, num); + else + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to store route table name and number pair, ignoring assignment: %s:%s", name, num); + continue; + } + assert(r > 0); + + TAKE_PTR(name); + } } static int route_section_verify(Route *route, Network *network) { diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 28f6f29128..3f93aa6b57 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -86,10 +86,8 @@ int network_add_ipv4ll_route(Network *network); int network_add_default_route_on_device(Network *network); void network_drop_invalid_routes(Network *network); -int route_table_from_string_full(Manager *m, const char *table, uint32_t *ret); - -const char *route_table_to_string(int d) _const_; -int route_table_from_string(const char *d) _pure_; +int manager_get_route_table_from_string(const Manager *m, const char *table, uint32_t *ret); +int manager_get_route_table_to_string(const Manager *m, uint32_t table, char **ret); CONFIG_PARSER_PROTOTYPE(config_parse_gateway); CONFIG_PARSER_PROTOTYPE(config_parse_preferred_src); diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 3534232822..da8778dd1e 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -389,23 +389,25 @@ static int routing_policy_rule_consume_foreign(Manager *m, RoutingPolicyRule *ru return 1; } -static void log_routing_policy_rule_debug(const RoutingPolicyRule *rule, int family, const char *str, const Link *link) { +static void log_routing_policy_rule_debug(const RoutingPolicyRule *rule, int family, const char *str, const Link *link, const Manager *m) { assert(rule); assert(IN_SET(family, AF_INET, AF_INET6)); assert(str); + assert(m); /* link may be NULL. */ if (DEBUG_LOGGING) { - _cleanup_free_ char *from = NULL, *to = NULL; + _cleanup_free_ char *from = NULL, *to = NULL, *table = NULL; (void) in_addr_to_string(family, &rule->from, &from); (void) in_addr_to_string(family, &rule->to, &to); + (void) manager_get_route_table_to_string(m, rule->table, &table); log_link_debug(link, - "%s routing policy rule: priority: %"PRIu32", %s/%u -> %s/%u, iif: %s, oif: %s, table: %"PRIu32, + "%s routing policy rule: priority: %"PRIu32", %s/%u -> %s/%u, iif: %s, oif: %s, table: %s", str, rule->priority, strna(from), rule->from_prefixlen, strna(to), rule->to_prefixlen, - strna(rule->iif), strna(rule->oif), rule->table); + strna(rule->iif), strna(rule->oif), strna(table)); } } @@ -551,7 +553,7 @@ static int routing_policy_rule_remove(const RoutingPolicyRule *rule, Manager *ma assert(manager->rtnl); assert(IN_SET(rule->family, AF_INET, AF_INET6)); - log_routing_policy_rule_debug(rule, rule->family, "Removing", NULL); + log_routing_policy_rule_debug(rule, rule->family, "Removing", NULL, manager); r = sd_rtnl_message_new_routing_policy_rule(manager->rtnl, &m, RTM_DELRULE, rule->family); if (r < 0) @@ -610,7 +612,7 @@ static int routing_policy_rule_configure_internal(const RoutingPolicyRule *rule, assert(link->manager); assert(link->manager->rtnl); - log_routing_policy_rule_debug(rule, family, "Configuring", link); + log_routing_policy_rule_debug(rule, family, "Configuring", link, link->manager); r = sd_rtnl_message_new_routing_policy_rule(link->manager->rtnl, &m, RTM_NEWRULE, family); if (r < 0) @@ -973,9 +975,9 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man switch (type) { case RTM_NEWRULE: if (rule) - log_routing_policy_rule_debug(tmp, tmp->family, "Received remembered", NULL); + log_routing_policy_rule_debug(tmp, tmp->family, "Received remembered", NULL, m); else { - log_routing_policy_rule_debug(tmp, tmp->family, "Remembering foreign", NULL); + log_routing_policy_rule_debug(tmp, tmp->family, "Remembering foreign", NULL, m); r = routing_policy_rule_consume_foreign(m, TAKE_PTR(tmp)); if (r < 0) log_warning_errno(r, "Could not remember foreign rule, ignoring: %m"); @@ -983,10 +985,10 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man break; case RTM_DELRULE: if (rule) { - log_routing_policy_rule_debug(tmp, tmp->family, "Forgetting", NULL); + log_routing_policy_rule_debug(tmp, tmp->family, "Forgetting", NULL, m); routing_policy_rule_free(rule); } else - log_routing_policy_rule_debug(tmp, tmp->family, "Kernel removed unknown", NULL); + log_routing_policy_rule_debug(tmp, tmp->family, "Kernel removed unknown", NULL, m); break; default: @@ -1130,7 +1132,7 @@ int config_parse_routing_policy_rule_table( if (r < 0) return log_oom(); - r = route_table_from_string_full(network->manager, rvalue, &n->table); + r = manager_get_route_table_from_string(network->manager, rvalue, &n->table); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Could not parse RPDB rule route table number \"%s\", ignoring assignment: %m", rvalue); diff --git a/src/network/test-network.c b/src/network/test-network.c index 45cd3fa973..760e33151b 100644 --- a/src/network/test-network.c +++ b/src/network/test-network.c @@ -12,6 +12,7 @@ #include "network-internal.h" #include "networkd-manager.h" #include "string-util.h" +#include "strv.h" #include "tests.h" static void test_deserialize_in_addr(void) { @@ -101,6 +102,53 @@ static void test_deserialize_dhcp_routes(void) { } } +static void test_route_tables_one(Manager *manager, const char *name, uint32_t number) { + _cleanup_free_ char *str = NULL, *expected = NULL, *num_str = NULL; + uint32_t t; + + if (!STR_IN_SET(name, "default", "main", "local")) { + assert_se(streq(hashmap_get(manager->route_table_names_by_number, UINT32_TO_PTR(number)), name)); + assert_se(PTR_TO_UINT32(hashmap_get(manager->route_table_numbers_by_name, name)) == number); + } + + assert_se(asprintf(&expected, "%s(%" PRIu32 ")", name, number) >= 0); + assert_se(manager_get_route_table_to_string(manager, number, &str) >= 0); + assert_se(streq(str, expected)); + + assert_se(manager_get_route_table_from_string(manager, name, &t) >= 0); + assert_se(t == number); + + assert_se(asprintf(&num_str, "%" PRIu32, number) >= 0); + assert_se(manager_get_route_table_from_string(manager, num_str, &t) >= 0); + assert_se(t == number); +} + +static void test_route_tables(Manager *manager) { + assert_se(config_parse_route_table_names("manager", "filename", 1, "section", 1, "RouteTable", 0, "hoge:123 foo:456 aaa:111", manager, manager) >= 0); + assert_se(config_parse_route_table_names("manager", "filename", 1, "section", 1, "RouteTable", 0, "bbb:11111 ccc:22222", manager, manager) >= 0); + assert_se(config_parse_route_table_names("manager", "filename", 1, "section", 1, "RouteTable", 0, "ddd:22222", manager, manager) >= 0); + + test_route_tables_one(manager, "hoge", 123); + test_route_tables_one(manager, "foo", 456); + test_route_tables_one(manager, "aaa", 111); + test_route_tables_one(manager, "bbb", 11111); + test_route_tables_one(manager, "ccc", 22222); + + assert_se(!hashmap_get(manager->route_table_numbers_by_name, "ddd")); + + test_route_tables_one(manager, "default", 253); + test_route_tables_one(manager, "main", 254); + test_route_tables_one(manager, "local", 255); + + assert_se(config_parse_route_table_names("manager", "filename", 1, "section", 1, "RouteTable", 0, "", manager, manager) >= 0); + assert_se(!manager->route_table_names_by_number); + assert_se(!manager->route_table_numbers_by_name); + + test_route_tables_one(manager, "default", 253); + test_route_tables_one(manager, "main", 254); + test_route_tables_one(manager, "local", 255); +} + static int test_load_config(Manager *manager) { int r; /* TODO: should_reload, is false if the config dirs do not exist, so @@ -241,6 +289,8 @@ int main(void) { assert_se(manager_new(&manager) >= 0); + test_route_tables(manager); + r = test_load_config(manager); if (r == -EPERM) return log_tests_skipped("Cannot load configuration"); |