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 aa752caff3b4b4ec668bf6e271f41a8305883284..e9ed30d55e59592acc5a7d74b3c4c86560e91ac9 100644 (file)
--- 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 758475a8ff6617fd4d2faf3fb4bf1e329197ef05..683a310b3f34e09d6cdb32a54d6f2a6c5ff0e9c9 100644 (file)
--- 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 b353ea32febfc19c62ae75fe5cd33168b014f472..dbe8c59a625e55159b87ef6f20523e6134c1946d 100644 (file)
--- 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 51caf4b9446b26e9e61ebfafe4714e242a984a83..031ba398e7ec2274d5a12c163bd2d2ead0d0b03f 100644 (file)
--- 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;