From ae8e3c2b257c38c5301d20bec39a1ffec2aba6c6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 29 Apr 2021 15:35:47 +0900 Subject: [PATCH 1/4] ether-addr-util: introduce ether_addr_to_string_alloc() --- src/basic/ether-addr-util.c | 16 ++++++++++++++++ src/basic/ether-addr-util.h | 1 + 2 files changed, 17 insertions(+) diff --git a/src/basic/ether-addr-util.c b/src/basic/ether-addr-util.c index c8094b6e45e..4bb65573845 100644 --- a/src/basic/ether-addr-util.c +++ b/src/basic/ether-addr-util.c @@ -43,6 +43,22 @@ char* ether_addr_to_string(const struct ether_addr *addr, char buffer[ETHER_ADDR return buffer; } +int ether_addr_to_string_alloc(const struct ether_addr *addr, char **ret) { + char *buf; + + assert(addr); + assert(ret); + + buf = new(char, ETHER_ADDR_TO_STRING_MAX); + if (!buf) + return -ENOMEM; + + ether_addr_to_string(addr, buf); + + *ret = buf; + return 0; +} + int ether_addr_compare(const struct ether_addr *a, const struct ether_addr *b) { return memcmp(a, b, ETH_ALEN); } diff --git a/src/basic/ether-addr-util.h b/src/basic/ether-addr-util.h index 942ce55621e..712c9277964 100644 --- a/src/basic/ether-addr-util.h +++ b/src/basic/ether-addr-util.h @@ -35,6 +35,7 @@ char* hw_addr_to_string(const hw_addr_data *addr, char buffer[HW_ADDR_TO_STRING_ #define ETHER_ADDR_TO_STRING_MAX (3*6) char* ether_addr_to_string(const struct ether_addr *addr, char buffer[ETHER_ADDR_TO_STRING_MAX]); +int ether_addr_to_string_alloc(const struct ether_addr *addr, char **ret); int ether_addr_compare(const struct ether_addr *a, const struct ether_addr *b); static inline bool ether_addr_equal(const struct ether_addr *a, const struct ether_addr *b) { From 7653a9dcd394231cb0c968bf40d57585855fa23a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 30 Apr 2021 05:43:22 +0900 Subject: [PATCH 2/4] network: reduce indentation in log_address_debug() or friends --- src/network/networkd-address.c | 47 +++++++++++----------- src/network/networkd-nexthop.c | 21 +++++----- src/network/networkd-route.c | 43 ++++++++++---------- src/network/networkd-routing-policy-rule.c | 21 +++++----- 4 files changed, 68 insertions(+), 64 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index e589fff150d..724b0f95d31 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -486,37 +486,38 @@ int link_has_ipv6_address(Link *link, const struct in6_addr *address) { } static void log_address_debug(const Address *address, const char *str, const Link *link) { + _cleanup_free_ char *addr = NULL, *peer = NULL; + char valid_buf[FORMAT_TIMESPAN_MAX], preferred_buf[FORMAT_TIMESPAN_MAX]; + const char *valid_str = NULL, *preferred_str = NULL; + bool has_peer; + assert(address); assert(str); assert(link); - if (DEBUG_LOGGING) { - _cleanup_free_ char *addr = NULL, *peer = NULL; - char valid_buf[FORMAT_TIMESPAN_MAX], preferred_buf[FORMAT_TIMESPAN_MAX]; - const char *valid_str = NULL, *preferred_str = NULL; - bool has_peer; + if (!DEBUG_LOGGING) + return; - (void) in_addr_to_string(address->family, &address->in_addr, &addr); - has_peer = in_addr_is_set(address->family, &address->in_addr_peer); - if (has_peer) - (void) in_addr_to_string(address->family, &address->in_addr_peer, &peer); + (void) in_addr_to_string(address->family, &address->in_addr, &addr); + has_peer = in_addr_is_set(address->family, &address->in_addr_peer); + if (has_peer) + (void) in_addr_to_string(address->family, &address->in_addr_peer, &peer); - if (address->cinfo.ifa_valid != CACHE_INFO_INFINITY_LIFE_TIME) - valid_str = format_timespan(valid_buf, FORMAT_TIMESPAN_MAX, - address->cinfo.ifa_valid * USEC_PER_SEC, - USEC_PER_SEC); + if (address->cinfo.ifa_valid != CACHE_INFO_INFINITY_LIFE_TIME) + valid_str = format_timespan(valid_buf, FORMAT_TIMESPAN_MAX, + address->cinfo.ifa_valid * USEC_PER_SEC, + USEC_PER_SEC); - if (address->cinfo.ifa_prefered != CACHE_INFO_INFINITY_LIFE_TIME) - preferred_str = format_timespan(preferred_buf, FORMAT_TIMESPAN_MAX, - address->cinfo.ifa_prefered * USEC_PER_SEC, - USEC_PER_SEC); + if (address->cinfo.ifa_prefered != CACHE_INFO_INFINITY_LIFE_TIME) + preferred_str = format_timespan(preferred_buf, FORMAT_TIMESPAN_MAX, + address->cinfo.ifa_prefered * USEC_PER_SEC, + USEC_PER_SEC); - log_link_debug(link, "%s address: %s%s%s/%u (valid %s%s, preferred %s%s)", - str, strnull(addr), has_peer ? " peer " : "", - has_peer ? strnull(peer) : "", address->prefixlen, - valid_str ? "for " : "forever", strempty(valid_str), - preferred_str ? "for " : "forever", strempty(preferred_str)); - } + log_link_debug(link, "%s address: %s%s%s/%u (valid %s%s, preferred %s%s)", + str, strnull(addr), has_peer ? " peer " : "", + has_peer ? strnull(peer) : "", address->prefixlen, + valid_str ? "for " : "forever", strempty(valid_str), + preferred_str ? "for " : "forever", strempty(preferred_str)); } static int address_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index c32cc70798b..f3dbd98ed12 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -336,23 +336,24 @@ set_manager: } static void log_nexthop_debug(const NextHop *nexthop, uint32_t id, const char *str, const Link *link) { + _cleanup_free_ char *gw = NULL; + assert(nexthop); assert(str); /* link may be NULL. */ - if (DEBUG_LOGGING) { - _cleanup_free_ char *gw = NULL; + if (!DEBUG_LOGGING) + return; - (void) in_addr_to_string(nexthop->family, &nexthop->gw, &gw); + (void) in_addr_to_string(nexthop->family, &nexthop->gw, &gw); - if (nexthop->id == id) - log_link_debug(link, "%s nexthop: id: %"PRIu32", gw: %s, blackhole: %s", - str, nexthop->id, strna(gw), yes_no(nexthop->blackhole)); - else - log_link_debug(link, "%s nexthop: id: %"PRIu32"→%"PRIu32", gw: %s, blackhole: %s", - str, nexthop->id, id, strna(gw), yes_no(nexthop->blackhole)); - } + if (nexthop->id == id) + log_link_debug(link, "%s nexthop: id: %"PRIu32", gw: %s, blackhole: %s", + str, nexthop->id, strna(gw), yes_no(nexthop->blackhole)); + else + log_link_debug(link, "%s nexthop: id: %"PRIu32"→%"PRIu32", gw: %s, blackhole: %s", + str, nexthop->id, id, strna(gw), yes_no(nexthop->blackhole)); } static int nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 070131f4af7..8f3d84129a8 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -605,35 +605,36 @@ static bool route_type_is_reject(const Route *route) { } static void log_route_debug(const Route *route, const char *str, const Link *link, const Manager *m) { + _cleanup_free_ char *dst = NULL, *src = NULL, *gw = NULL, *prefsrc = NULL, + *table = NULL, *scope = NULL, *proto = NULL; + assert(route); assert(str); assert(m); /* link may be NULL. */ - if (DEBUG_LOGGING) { - _cleanup_free_ char *dst = NULL, *src = NULL, *gw = NULL, *prefsrc = NULL, - *table = NULL, *scope = NULL, *proto = NULL; + if (!DEBUG_LOGGING) + return; - if (in_addr_is_set(route->family, &route->dst)) - (void) in_addr_prefix_to_string(route->family, &route->dst, route->dst_prefixlen, &dst); - if (in_addr_is_set(route->family, &route->src)) - (void) in_addr_to_string(route->family, &route->src, &src); - if (in_addr_is_set(route->gw_family, &route->gw)) - (void) in_addr_to_string(route->gw_family, &route->gw, &gw); - if (in_addr_is_set(route->family, &route->prefsrc)) - (void) in_addr_to_string(route->family, &route->prefsrc, &prefsrc); - (void) route_scope_to_string_alloc(route->scope, &scope); - (void) manager_get_route_table_to_string(m, route->table, &table); - (void) route_protocol_full_to_string_alloc(route->protocol, &proto); + if (in_addr_is_set(route->family, &route->dst)) + (void) in_addr_prefix_to_string(route->family, &route->dst, route->dst_prefixlen, &dst); + if (in_addr_is_set(route->family, &route->src)) + (void) in_addr_to_string(route->family, &route->src, &src); + if (in_addr_is_set(route->gw_family, &route->gw)) + (void) in_addr_to_string(route->gw_family, &route->gw, &gw); + if (in_addr_is_set(route->family, &route->prefsrc)) + (void) in_addr_to_string(route->family, &route->prefsrc, &prefsrc); + (void) route_scope_to_string_alloc(route->scope, &scope); + (void) manager_get_route_table_to_string(m, route->table, &table); + (void) route_protocol_full_to_string_alloc(route->protocol, &proto); - log_link_debug(link, - "%s route: dst: %s, src: %s, gw: %s, prefsrc: %s, scope: %s, table: %s, proto: %s, type: %s, nexthop: %"PRIu32", priority: %"PRIu32, - str, strna(dst), strna(src), strna(gw), strna(prefsrc), - strna(scope), strna(table), strna(proto), - strna(route_type_to_string(route->type)), - route->nexthop_id, route->priority); - } + log_link_debug(link, + "%s route: dst: %s, src: %s, gw: %s, prefsrc: %s, scope: %s, table: %s, proto: %s, type: %s, nexthop: %"PRIu32", priority: %"PRIu32, + str, strna(dst), strna(src), strna(gw), strna(prefsrc), + strna(scope), strna(table), strna(proto), + strna(route_type_to_string(route->type)), + route->nexthop_id, route->priority); } static int route_set_netlink_message(const Route *route, sd_netlink_message *req, Link *link) { diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 03bdd4e640f..14e079eed30 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -391,6 +391,8 @@ static int routing_policy_rule_consume_foreign(Manager *m, RoutingPolicyRule *ru } static void log_routing_policy_rule_debug(const RoutingPolicyRule *rule, int family, const char *str, const Link *link, const Manager *m) { + _cleanup_free_ char *from = NULL, *to = NULL, *table = NULL; + assert(rule); assert(IN_SET(family, AF_INET, AF_INET6)); assert(str); @@ -398,18 +400,17 @@ static void log_routing_policy_rule_debug(const RoutingPolicyRule *rule, int fam /* link may be NULL. */ - if (DEBUG_LOGGING) { - _cleanup_free_ char *from = NULL, *to = NULL, *table = NULL; + if (!DEBUG_LOGGING) + return; - (void) in_addr_prefix_to_string(family, &rule->from, rule->from_prefixlen, &from); - (void) in_addr_prefix_to_string(family, &rule->to, rule->to_prefixlen, &to); - (void) manager_get_route_table_to_string(m, rule->table, &table); + (void) in_addr_prefix_to_string(family, &rule->from, rule->from_prefixlen, &from); + (void) in_addr_prefix_to_string(family, &rule->to, rule->to_prefixlen, &to); + (void) manager_get_route_table_to_string(m, rule->table, &table); - log_link_debug(link, - "%s routing policy rule: priority: %"PRIu32", %s -> %s, iif: %s, oif: %s, table: %s", - str, rule->priority, strna(from), strna(to), - strna(rule->iif), strna(rule->oif), strna(table)); - } + log_link_debug(link, + "%s routing policy rule: priority: %"PRIu32", %s -> %s, iif: %s, oif: %s, table: %s", + str, rule->priority, strna(from), strna(to), + strna(rule->iif), strna(rule->oif), strna(table)); } static int routing_policy_rule_set_netlink_message(const RoutingPolicyRule *rule, sd_netlink_message *m, Link *link) { From 2775e1c57829c4b9e3543551b1535fdd8eaad3b7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 29 Apr 2021 15:49:21 +0900 Subject: [PATCH 3/4] network: introduce log_neighbor_debug() --- src/network/networkd-neighbor.c | 69 +++++++++++++++++---------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index 205351c2a2d..7539d280767 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -213,6 +213,29 @@ static bool neighbor_equal(const Neighbor *n1, const Neighbor *n2) { return neighbor_compare_func(n1, n2) == 0; } +static void log_neighbor_debug(const Neighbor *neighbor, const char *str, const Link *link) { + _cleanup_free_ char *lladdr = NULL, *dst = NULL; + + assert(neighbor); + assert(str); + + if (!DEBUG_LOGGING) + return; + + if (neighbor->lladdr_size == sizeof(struct ether_addr)) + (void) ether_addr_to_string_alloc(&neighbor->lladdr.mac, &lladdr); + else if (neighbor->lladdr_size == sizeof(struct in_addr)) + (void) in_addr_to_string(AF_INET, &neighbor->lladdr.ip, &lladdr); + else if (neighbor->lladdr_size == sizeof(struct in6_addr)) + (void) in_addr_to_string(AF_INET6, &neighbor->lladdr.ip, &lladdr); + + (void) in_addr_to_string(neighbor->family, &neighbor->in_addr, &dst); + + log_link_debug(link, + "%s neighbor: lladdr: %s, dst: %s", + str, strna(lladdr), strna(dst)); +} + static int neighbor_configure_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { int r; @@ -249,6 +272,8 @@ static int neighbor_configure(Neighbor *neighbor, Link *link) { assert(link->manager); assert(link->manager->rtnl); + log_neighbor_debug(neighbor, "Configuring", link); + r = sd_rtnl_message_new_neigh(link->manager->rtnl, &req, RTM_NEWNEIGH, link->ifindex, neighbor->family); if (r < 0) @@ -340,6 +365,8 @@ static int neighbor_remove(Neighbor *neighbor, Link *link) { assert(link->manager); assert(link->manager->rtnl); + log_neighbor_debug(neighbor, "Removing", link); + r = sd_rtnl_message_new_neigh(link->manager->rtnl, &req, RTM_DELNEIGH, link->ifindex, neighbor->family); if (r < 0) @@ -410,41 +437,28 @@ int link_drop_neighbors(Link *link) { return r; } -static int manager_rtnl_process_neighbor_lladdr(sd_netlink_message *message, union lladdr_union *lladdr, size_t *size, char **str) { +static int manager_rtnl_process_neighbor_lladdr(sd_netlink_message *message, union lladdr_union *lladdr, size_t *size) { int r; assert(message); assert(lladdr); assert(size); - assert(str); - - *str = NULL; r = sd_netlink_message_read(message, NDA_LLADDR, sizeof(lladdr->ip.in6), &lladdr->ip.in6); if (r >= 0) { *size = sizeof(lladdr->ip.in6); - if (in_addr_to_string(AF_INET6, &lladdr->ip, str) < 0) - log_warning_errno(r, "Could not print lower address: %m"); return r; } r = sd_netlink_message_read(message, NDA_LLADDR, sizeof(lladdr->mac), &lladdr->mac); if (r >= 0) { *size = sizeof(lladdr->mac); - *str = new(char, ETHER_ADDR_TO_STRING_MAX); - if (!*str) { - log_oom(); - return r; - } - ether_addr_to_string(&lladdr->mac, *str); return r; } r = sd_netlink_message_read(message, NDA_LLADDR, sizeof(lladdr->ip.in), &lladdr->ip.in); if (r >= 0) { *size = sizeof(lladdr->ip.in); - if (in_addr_to_string(AF_INET, &lladdr->ip, str) < 0) - log_warning_errno(r, "Could not print lower address: %m"); return r; } @@ -453,7 +467,6 @@ static int manager_rtnl_process_neighbor_lladdr(sd_netlink_message *message, uni int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { _cleanup_(neighbor_freep) Neighbor *tmp = NULL; - _cleanup_free_ char *addr_str = NULL, *lladdr_str = NULL; Neighbor *neighbor = NULL; uint16_t type, state; int ifindex, r; @@ -523,10 +536,7 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, return 0; } - if (in_addr_to_string(tmp->family, &tmp->in_addr, &addr_str) < 0) - log_link_warning_errno(link, r, "Could not print address: %m"); - - r = manager_rtnl_process_neighbor_lladdr(message, &tmp->lladdr, &tmp->lladdr_size, &lladdr_str); + r = manager_rtnl_process_neighbor_lladdr(message, &tmp->lladdr, &tmp->lladdr_size); if (r < 0) { log_link_warning_errno(link, r, "rtnl: received neighbor message with invalid lladdr, ignoring: %m"); return 0; @@ -537,30 +547,21 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, switch (type) { case RTM_NEWNEIGH: if (neighbor) - log_link_debug(link, "Received remembered neighbor: %s->%s", - strnull(addr_str), strnull(lladdr_str)); + log_neighbor_debug(tmp, "Received remembered", link); else { - /* A neighbor appeared that we did not request */ + log_neighbor_debug(tmp, "Remembering foreign", link); r = neighbor_add_foreign(link, tmp, NULL); if (r < 0) { - log_link_warning_errno(link, r, "Failed to remember foreign neighbor %s->%s, ignoring: %m", - strnull(addr_str), strnull(lladdr_str)); + log_link_warning_errno(link, r, "Failed to remember foreign neighbor, ignoring: %m"); return 0; - } else - log_link_debug(link, "Remembering foreign neighbor: %s->%s", - strnull(addr_str), strnull(lladdr_str)); + } } break; case RTM_DELNEIGH: - if (neighbor) { - log_link_debug(link, "Forgetting neighbor: %s->%s", - strnull(addr_str), strnull(lladdr_str)); - (void) neighbor_free(neighbor); - } else - log_link_debug(link, "Kernel removed a neighbor we don't remember: %s->%s, ignoring.", - strnull(addr_str), strnull(lladdr_str)); + log_neighbor_debug(tmp, neighbor ? "Forgetting" : "Kernel removed unknown", link); + neighbor_free(neighbor); break; From 5aa87ec7ec25eec2e96e8237c3c232e017ca078d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 29 Apr 2021 15:59:41 +0900 Subject: [PATCH 4/4] network: neighbor: use sd_netlink_message_read_data() at one more place --- src/network/networkd-neighbor.c | 37 +++++++-------------------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index 7539d280767..b33f560b798 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -437,36 +437,9 @@ int link_drop_neighbors(Link *link) { return r; } -static int manager_rtnl_process_neighbor_lladdr(sd_netlink_message *message, union lladdr_union *lladdr, size_t *size) { - int r; - - assert(message); - assert(lladdr); - assert(size); - - r = sd_netlink_message_read(message, NDA_LLADDR, sizeof(lladdr->ip.in6), &lladdr->ip.in6); - if (r >= 0) { - *size = sizeof(lladdr->ip.in6); - return r; - } - - r = sd_netlink_message_read(message, NDA_LLADDR, sizeof(lladdr->mac), &lladdr->mac); - if (r >= 0) { - *size = sizeof(lladdr->mac); - return r; - } - - r = sd_netlink_message_read(message, NDA_LLADDR, sizeof(lladdr->ip.in), &lladdr->ip.in); - if (r >= 0) { - *size = sizeof(lladdr->ip.in); - return r; - } - - return r; -} - int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { _cleanup_(neighbor_freep) Neighbor *tmp = NULL; + _cleanup_free_ void *lladdr = NULL; Neighbor *neighbor = NULL; uint16_t type, state; int ifindex, r; @@ -536,11 +509,15 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, return 0; } - r = manager_rtnl_process_neighbor_lladdr(message, &tmp->lladdr, &tmp->lladdr_size); + r = sd_netlink_message_read_data(message, NDA_LLADDR, &tmp->lladdr_size, &lladdr); if (r < 0) { - log_link_warning_errno(link, r, "rtnl: received neighbor message with invalid lladdr, ignoring: %m"); + log_link_warning_errno(link, r, "rtnl: received neighbor message without valid lladdr, ignoring: %m"); + return 0; + } else if (!IN_SET(tmp->lladdr_size, sizeof(struct ether_addr), sizeof(struct in_addr), sizeof(struct in6_addr))) { + log_link_warning(link, "rtnl: received neighbor message with invalid lladdr size (%zu), ignoring: %m", tmp->lladdr_size); return 0; } + memcpy(&tmp->lladdr, lladdr, tmp->lladdr_size); (void) neighbor_get(link, tmp, &neighbor);