From: Yu Watanabe Date: Mon, 10 Jul 2023 02:30:11 +0000 (+0900) Subject: network/neighbor: follow the way how kernel distinguish neighbor settings X-Git-Tag: v255-rc1~882^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=aa9626ee3b96e0d2a9a816b5efd38fd7dc829def;p=thirdparty%2Fsystemd.git network/neighbor: follow the way how kernel distinguish neighbor settings The kernel manages neighbors by the destination address, and the LinkLayerAddress is mutable. Let's manage neighbors in the same way, and dedup settings. --- diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index aa752caff3b..e9ed30d55e5 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -16,7 +16,7 @@ Neighbor *neighbor_free(Neighbor *neighbor) { if (neighbor->network) { assert(neighbor->section); - hashmap_remove(neighbor->network->neighbors_by_section, neighbor->section); + ordered_hashmap_remove(neighbor->network->neighbors_by_section, neighbor->section); } config_section_free(neighbor->section); @@ -43,7 +43,7 @@ static int neighbor_new_static(Network *network, const char *filename, unsigned if (r < 0) return r; - neighbor = hashmap_get(network->neighbors_by_section, n); + neighbor = ordered_hashmap_get(network->neighbors_by_section, n); if (neighbor) { *ret = TAKE_PTR(neighbor); return 0; @@ -60,7 +60,7 @@ static int neighbor_new_static(Network *network, const char *filename, unsigned .source = NETWORK_CONFIG_SOURCE_STATIC, }; - r = hashmap_ensure_put(&network->neighbors_by_section, &config_section_hash_ops, neighbor->section, neighbor); + r = ordered_hashmap_ensure_put(&network->neighbors_by_section, &config_section_hash_ops, neighbor->section, neighbor); if (r < 0) return r; @@ -92,18 +92,13 @@ static void neighbor_hash_func(const Neighbor *neighbor, struct siphash *state) siphash24_compress(&neighbor->family, sizeof(neighbor->family), state); - switch (neighbor->family) { - case AF_INET: - case AF_INET6: - /* Equality of neighbors are given by the pair (addr,lladdr) */ - siphash24_compress(&neighbor->in_addr, FAMILY_ADDRESS_SIZE(neighbor->family), state); - break; - default: + if (!IN_SET(neighbor->family, AF_INET, AF_INET6)) /* treat any other address family as AF_UNSPEC */ - break; - } + return; - hw_addr_hash_func(&neighbor->ll_addr, state); + /* Equality of neighbors are given by the destination address. + * See neigh_lookup() in the kernel. */ + siphash24_compress(&neighbor->in_addr, FAMILY_ADDRESS_SIZE(neighbor->family), state); } static int neighbor_compare_func(const Neighbor *a, const Neighbor *b) { @@ -113,18 +108,25 @@ static int neighbor_compare_func(const Neighbor *a, const Neighbor *b) { if (r != 0) return r; - switch (a->family) { - case AF_INET: - case AF_INET6: - r = memcmp(&a->in_addr, &b->in_addr, FAMILY_ADDRESS_SIZE(a->family)); - if (r != 0) - return r; - } + if (!IN_SET(a->family, AF_INET, AF_INET6)) + /* treat any other address family as AF_UNSPEC */ + return 0; - return hw_addr_compare(&a->ll_addr, &b->ll_addr); + return memcmp(&a->in_addr, &b->in_addr, FAMILY_ADDRESS_SIZE(a->family)); } -DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(neighbor_hash_ops, Neighbor, neighbor_hash_func, neighbor_compare_func, neighbor_free); +DEFINE_PRIVATE_HASH_OPS( + neighbor_hash_ops, + Neighbor, + neighbor_hash_func, + neighbor_compare_func); + +DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( + neighbor_hash_ops_free, + Neighbor, + neighbor_hash_func, + neighbor_compare_func, + neighbor_free); static int neighbor_get(Link *link, const Neighbor *in, Neighbor **ret) { Neighbor *existing; @@ -147,7 +149,7 @@ static int neighbor_add(Link *link, Neighbor *neighbor) { assert(link); assert(neighbor); - r = set_ensure_put(&link->neighbors, &neighbor_hash_ops, neighbor); + r = set_ensure_put(&link->neighbors, &neighbor_hash_ops_free, neighbor); if (r < 0) return r; if (r == 0) @@ -307,7 +309,7 @@ int link_request_static_neighbors(Link *link) { link->static_neighbors_configured = false; - HASHMAP_FOREACH(neighbor, link->network->neighbors_by_section) { + ORDERED_HASHMAP_FOREACH(neighbor, link->network->neighbors_by_section) { r = link_request_neighbor(link, neighbor); if (r < 0) return log_link_warning_errno(link, r, "Could not request neighbor: %m"); @@ -396,7 +398,7 @@ int link_drop_foreign_neighbors(Link *link) { } /* Next, unmark requested neighbors. They will be configured later. */ - HASHMAP_FOREACH(neighbor, link->network->neighbors_by_section) { + ORDERED_HASHMAP_FOREACH(neighbor, link->network->neighbors_by_section) { Neighbor *existing; if (neighbor_get(link, neighbor, &existing) >= 0) @@ -586,14 +588,45 @@ static int neighbor_section_verify(Neighbor *neighbor) { return 0; } -void network_drop_invalid_neighbors(Network *network) { +int network_drop_invalid_neighbors(Network *network) { + _cleanup_set_free_ Set *neighbors = NULL; Neighbor *neighbor; + int r; assert(network); - HASHMAP_FOREACH(neighbor, network->neighbors_by_section) - if (neighbor_section_verify(neighbor) < 0) + ORDERED_HASHMAP_FOREACH(neighbor, network->neighbors_by_section) { + Neighbor *dup; + + if (neighbor_section_verify(neighbor) < 0) { + /* Drop invalid [Neighbor] sections. Note that neighbor_free() will drop the + * neighbor from neighbors_by_section. */ neighbor_free(neighbor); + continue; + } + + /* Always use the setting specified later. So, remove the previously assigned setting. */ + dup = set_remove(neighbors, neighbor); + if (dup) { + log_warning("%s: Duplicated neighbor settings for %s is specified at line %u and %u, " + "dropping the address setting specified at line %u.", + dup->section->filename, + IN_ADDR_TO_STRING(neighbor->family, &neighbor->in_addr), + neighbor->section->line, + dup->section->line, dup->section->line); + /* neighbor_free() will drop the address from neighbors_by_section. */ + neighbor_free(dup); + } + + /* Use neighbor_hash_ops, instead of neighbor_hash_ops_free. Otherwise, the Neighbor objects + * will be freed. */ + r = set_ensure_put(&neighbors, &neighbor_hash_ops, neighbor); + if (r < 0) + return log_oom(); + assert(r > 0); + } + + return 0; } diff --git a/src/network/networkd-neighbor.h b/src/network/networkd-neighbor.h index 758475a8ff6..683a310b3f3 100644 --- a/src/network/networkd-neighbor.h +++ b/src/network/networkd-neighbor.h @@ -28,7 +28,7 @@ typedef struct Neighbor { Neighbor *neighbor_free(Neighbor *neighbor); -void network_drop_invalid_neighbors(Network *network); +int network_drop_invalid_neighbors(Network *network); int link_drop_managed_neighbors(Link *link); int link_drop_foreign_neighbors(Link *link); diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index b353ea32feb..dbe8c59a625 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -317,7 +317,9 @@ int network_verify(Network *network) { network_drop_invalid_nexthops(network); network_drop_invalid_bridge_fdb_entries(network); network_drop_invalid_bridge_mdb_entries(network); - network_drop_invalid_neighbors(network); + r = network_drop_invalid_neighbors(network); + if (r < 0) + return r; network_drop_invalid_address_labels(network); network_drop_invalid_prefixes(network); network_drop_invalid_route_prefixes(network); @@ -772,7 +774,7 @@ static Network *network_free(Network *network) { hashmap_free_with_destructor(network->nexthops_by_section, nexthop_free); hashmap_free_with_destructor(network->bridge_fdb_entries_by_section, bridge_fdb_free); hashmap_free_with_destructor(network->bridge_mdb_entries_by_section, bridge_mdb_free); - hashmap_free_with_destructor(network->neighbors_by_section, neighbor_free); + ordered_hashmap_free_with_destructor(network->neighbors_by_section, neighbor_free); hashmap_free_with_destructor(network->address_labels_by_section, address_label_free); hashmap_free_with_destructor(network->prefixes_by_section, prefix_free); hashmap_free_with_destructor(network->route_prefixes_by_section, route_prefix_free); @@ -828,7 +830,7 @@ bool network_has_static_ipv6_configurations(Network *network) { if (mdb->family == AF_INET6) return true; - HASHMAP_FOREACH(neighbor, network->neighbors_by_section) + ORDERED_HASHMAP_FOREACH(neighbor, network->neighbors_by_section) if (neighbor->family == AF_INET6) return true; diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h index 51caf4b9446..031ba398e7e 100644 --- a/src/network/networkd-network.h +++ b/src/network/networkd-network.h @@ -351,7 +351,7 @@ struct Network { Hashmap *nexthops_by_section; Hashmap *bridge_fdb_entries_by_section; Hashmap *bridge_mdb_entries_by_section; - Hashmap *neighbors_by_section; + OrderedHashmap *neighbors_by_section; Hashmap *address_labels_by_section; Hashmap *prefixes_by_section; Hashmap *route_prefixes_by_section;