network: use defined route table name in debug logs

And rename route_table_from_string_full() to
manager_get_route_table_from_string().

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)

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. */

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;

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,16 +2414,17 @@ int config_parse_route_table_names(
                 void *data,
                 void *userdata) {
 
-        Hashmap **s = data;
+        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;
         }
 
@@ -2441,7 +2471,7 @@ int config_parse_route_table_names(
                         continue;
                 }
 
-                r = hashmap_ensure_put(s, &string_hash_ops, name, UINT32_TO_PTR(table));
+                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) {
@@ -2454,8 +2484,27 @@ int config_parse_route_table_names(
                                    "Failed to store route table name and number pair, ignoring assignment: %s:%s", name, num);
                         continue;
                 }
-                if (r > 0)
-                        TAKE_PTR(name);
+                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);
         }
 }
 

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);

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);