---
v3->v4:
Print warning when multiple dynamic addresses are configured on a
switch port. Ensure that dynamic addresses beyond the first on a switch
port are ignored. Found by Ben Pfaff.
v2->v3:
Fixed a checkpatch problem (line too long)
v1->v2:
Rebased
---
ovn/northd/ovn-northd.c | 385 ++++++++++++++++++++++++++++++++++--------------
tests/ovn.at | 97 +++++++++---
2 files changed, 350 insertions(+), 132 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 04a072ba8..b75febb44 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -985,9 +985,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct
ovn_port *op)
for (size_t i = 0; i < op->nbsp->n_addresses; i++) {
ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]);
}
- if (op->nbsp->dynamic_addresses) {
- ipam_insert_lsp_addresses(od, op, op->nbsp->dynamic_addresses);
- }
} else if (op->nbrp) {
struct lport_addresses lrp_networks;
if (!extract_lrp_networks(op->nbrp, &lrp_networks)) {
@@ -1060,64 +1057,255 @@ ipam_get_unused_ip(struct ovn_datapath *od)
return od->ipam_info.start_ipv4 + new_ip_index;
}
+enum dynamic_update_type {
+ NONE, /* No change to the address */
+ REMOVE, /* Address is no longer dynamic */
+ STATIC, /* Use static address (MAC only) */
+ DYNAMIC, /* Assign a new dynamic address */
+};
+
+struct dynamic_address_update {
+ struct ovs_list node; /* In build_ipam()'s list of updates. */
+
+ struct ovn_port *op;
+
+ struct lport_addresses current_addresses;
+ struct eth_addr static_mac;
+ enum dynamic_update_type mac;
+ enum dynamic_update_type ipv4;
+ enum dynamic_update_type ipv6;
+};
+
+static enum dynamic_update_type
+dynamic_mac_changed(const char *lsp_addresses,
+ struct dynamic_address_update *update)
+{
+ struct eth_addr ea;
+
+ if (ovs_scan(lsp_addresses, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
+ if (eth_addr_equals(ea, update->current_addresses.ea)) {
+ return NONE;
+ } else {
+ /* MAC is still static, but it has changed */
+ update->static_mac = ea;
+ return STATIC;
+ }
+ }
+
+ uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
+ if ((mac64 ^ MAC_ADDR_PREFIX) >> 24) {
+ return DYNAMIC;
+ } else {
+ return NONE;
+ }
+}
+
+static enum dynamic_update_type
+dynamic_ip4_changed(struct dynamic_address_update *update)
+{
+ bool dynamic_ip4 = update->op->od->ipam_info.allocated_ipv4s != NULL;
+
+ if (!dynamic_ip4) {
+ if (update->current_addresses.n_ipv4_addrs) {
+ return REMOVE;
+ } else {
+ return NONE;
+ }
+ }
+
+ if (!update->current_addresses.n_ipv4_addrs) {
+ /* IPv4 was previously static but now is dynamic */
+ return DYNAMIC;
+ }
+
+ uint32_t ip4 = ntohl(update->current_addresses.ipv4_addrs[0].addr);
+ if (ip4 < update->op->od->ipam_info.start_ipv4) {
+ return DYNAMIC;
+ }
+
+ uint32_t index = ip4 - update->op->od->ipam_info.start_ipv4;
+ if (index > update->op->od->ipam_info.total_ipv4s ||
+ bitmap_is_set(update->op->od->ipam_info.allocated_ipv4s, index)) {
+ /* Previously assigned dynamic IPv4 address can no longer be used.
+ * It's either outside the subnet, conflicts with an excluded IP,
+ * or conflicts with a statically-assigned address on the switch
+ */
+ return DYNAMIC;
+ } else {
+ return NONE;
+ }
+}
+ case STATIC:
+ mac = update->static_mac;
+ break;
+ case DYNAMIC:
+ eth_addr_from_uint64(ipam_get_unused_mac(), &mac);
+ break;
+ }
+
+ ovs_be32 ip4 = 0;
+ switch (update->ipv4) {
+ case NONE:
+ if (update->current_addresses.n_ipv4_addrs) {
+ ip4 = update->current_addresses.ipv4_addrs[0].addr;
+ }
+ break;
+ case REMOVE:
+ break;
+ case STATIC:
+ ovs_assert(0);
+ case DYNAMIC:
+ ip4 = htonl(ipam_get_unused_ip(od));
+ }
+
+ struct in6_addr ip6 = in6addr_any;
+ switch (update->ipv6) {
+ case NONE:
+ if (update->current_addresses.n_ipv6_addrs) {
+ ip6 = update->current_addresses.ipv6_addrs[0].addr;
+ }
+ break;
+ case REMOVE:
+ break;
+ case STATIC:
+ ovs_assert(0);
+ case DYNAMIC:
+ in6_generate_eui64(mac, &od->ipam_info.ipv6_prefix, &ip6);
+ break;
}
- /* Save the dynamic addresses. */
struct ds new_addr = DS_EMPTY_INITIALIZER;
ds_put_format(&new_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
- if (dynamic_ip4 && ip4) {
- ipam_insert_ip(od, ip4);
- ds_put_format(&new_addr, " "IP_FMT, IP_ARGS(htonl(ip4)));
+ if (ip4) {
+ ipam_insert_ip(od, ntohl(ip4));
+ ds_put_format(&new_addr, " "IP_FMT, IP_ARGS(ip4));
}
- if (dynamic_ip6) {
+ if (!IN6_ARE_ADDR_EQUAL(&ip6, &in6addr_any)) {
char ip6_s[INET6_ADDRSTRLEN + 1];
ipv6_string_mapped(ip6_s, &ip6);
ds_put_format(&new_addr, " %s", ip6_s);
}
- ipam_insert_mac(&mac, !dynamic_mac);
- nbrec_logical_switch_port_set_dynamic_addresses(op->nbsp,
+ nbrec_logical_switch_port_set_dynamic_addresses(update->op->nbsp,
ds_cstr(&new_addr));
+ set_lsp_dynamic_addresses(ds_cstr(&new_addr), update->op);
ds_destroy(&new_addr);
- return true;
}
static void
@@ -1133,48 +1321,80 @@ build_ipam(struct hmap *datapaths, struct hmap *ports)
* ports that have the "dynamic" keyword in their addresses column. */
struct ovn_datapath *od;
HMAP_FOR_EACH (od, key_node, datapaths) {
- if (!od->nbs || (!od->ipam_info.allocated_ipv4s &&
- !od->ipam_info.ipv6_prefix_set)) {
+ if (!od->nbs) {
continue;
}
- struct ovn_port *op;
+ struct ovs_list updates;
+ ovs_list_init(&updates);
for (size_t i = 0; i < od->nbs->n_ports; i++) {
- const struct nbrec_logical_switch_port *nbsp =
- od->nbs->ports[i];
+ const struct nbrec_logical_switch_port *nbsp = od->nbs->ports[i];
- if (!nbsp) {
+ if (!od->ipam_info.allocated_ipv4s &&
+ !od->ipam_info.ipv6_prefix_set) {
+ if (nbsp->dynamic_addresses) {
+ nbrec_logical_switch_port_set_dynamic_addresses(nbsp,
+ NULL);
+ }
continue;
}
- op = ovn_port_find(ports, nbsp->name);
- if (!op || (op->nbsp && op->peer)) {
+ struct ovn_port *op = ovn_port_find(ports, nbsp->name);
+ if (!op || op->nbsp != nbsp || op->peer) {
/* Do not allocate addresses for logical switch ports that
* have a peer. */
continue;
}
+ int num_dynamic_addresses = 0;
for (size_t j = 0; j < nbsp->n_addresses; j++) {
- if (is_dynamic_lsp_address(nbsp->addresses[j])
- && !nbsp->dynamic_addresses) {
- if (!ipam_allocate_addresses(od, op, nbsp->addresses[j])
- || !extract_lsp_addresses(nbsp->dynamic_addresses,
- &op->lsp_addrs[op->n_lsp_addrs])) {
- static struct vlog_rate_limit rl
- = VLOG_RATE_LIMIT_INIT(1, 1);
- VLOG_INFO_RL(&rl, "Failed to allocate address.");
+ if (!is_dynamic_lsp_address(nbsp->addresses[j])) {
+ continue;
+ }
+ if (num_dynamic_addresses) {
+ static struct vlog_rate_limit rl
+ = VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_WARN_RL(&rl, "More than one dynamic address "
+ "configured for logical switch port '%s'",
+ nbsp->name);
+ continue;
+ }
+ num_dynamic_addresses++;
+ struct dynamic_address_update *update
+ = xzalloc(sizeof *update);
+ update->op = op;
+ if (nbsp->dynamic_addresses) {
+ extract_lsp_addresses(nbsp->dynamic_addresses,
+ &update->current_addresses);
+ if (dynamic_addresses_check_for_updates(nbsp->addresses[j],
+ update)) {
+ ovs_list_push_back(&updates, &update->node);
} else {
- op->n_lsp_addrs++;
+ /* No changes to dynamic addresses */
+ set_lsp_dynamic_addresses(nbsp->dynamic_addresses, op);
+ destroy_lport_addresses(&update->current_addresses);
+ free(update);
}
- break;
+ } else {
+ set_dynamic_updates(nbsp->addresses[j], update);
+ ovs_list_push_back(&updates, &update->node);
}
}
if (!nbsp->n_addresses && nbsp->dynamic_addresses) {
- nbrec_logical_switch_port_set_dynamic_addresses(op->nbsp,
- NULL);
+ nbrec_logical_switch_port_set_dynamic_addresses(nbsp, NULL);
}
}
+
+ /* After retaining all unchanged dynamic addresses, now assign
+ * new ones.
+ */
+ struct dynamic_address_update *update;
+ LIST_FOR_EACH_POP (update, node, &updates) {
+ update_dynamic_addresses(od, update);
+ destroy_lport_addresses(&update->current_addresses);
+ free(update);
+ }
}
}
@@ -1277,41 +1497,6 @@ tag_alloc_create_new_tag(struct hmap *tag_alloc_table,
}
-/*
- * This function checks if the MAC in "address" parameter (if present) is
- * different from the one stored in Logical_Switch_Port.dynamic_addresses
- * and updates it.
- */
-static void
-check_and_update_mac_in_dynamic_addresses(
- const char *address,
- const struct nbrec_logical_switch_port *nbsp)
-{
- if (!nbsp->dynamic_addresses) {
- return;
- }
- int buf_index = 0;
- struct eth_addr ea;
- if (!ovs_scan_len(address, &buf_index,
- ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
- return;
- }
-
- struct eth_addr present_ea;
- buf_index = 0;
- if (ovs_scan_len(nbsp->dynamic_addresses, &buf_index,
- ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(present_ea))
- && !eth_addr_equals(ea, present_ea)) {
- /* MAC address has changed. Update it */
- char *new_addr = xasprintf(
- ETH_ADDR_FMT"%s", ETH_ADDR_ARGS(ea),
-  ->dynamic_addresses[buf_index]);
- nbrec_logical_switch_port_set_dynamic_addresses(
- nbsp, new_addr);
- free(new_addr);
- }
-}
-
static void
join_logical_ports(struct northd_context *ctx,
struct hmap *datapaths, struct hmap *ports,
@@ -1379,23 +1564,7 @@ join_logical_ports(struct northd_context *ctx,
continue;
}
if (is_dynamic_lsp_address(nbsp->addresses[j])) {
- if (nbsp->dynamic_addresses) {
- check_and_update_mac_in_dynamic_addresses(
- nbsp->addresses[j], nbsp);
- if (!extract_lsp_addresses(nbsp->dynamic_addresses,
- &op->lsp_addrs[op->n_lsp_addrs])) {
- static struct vlog_rate_limit rl
- = VLOG_RATE_LIMIT_INIT(1, 1);
- VLOG_INFO_RL(&rl, "invalid syntax '%s' in "
- "logical switch port "
- "dynamic_addresses. No "
- "MAC address found",
- op->nbsp->dynamic_addresses);
- continue;
- }
- } else {
- continue;
- }
+ continue;
} else if (!extract_lsp_addresses(nbsp->addresses[j],
&op->lsp_addrs[op->n_lsp_addrs])) {
static struct vlog_rate_limit rl
diff --git a/tests/ovn.at b/tests/ovn.at
index d1a8967dd..e80c965a3 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5140,7 +5140,7 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p19
dynamic_addresses], [0],
# Change a port's address to test for multiple ip's for a single address entry
# and addresses set by the user.
-ovn-nbctl lsp-set-addresses p0 "0a:00:00:00:00:15 192.168.1.12 192.168.1.14"
+ovn-nbctl lsp-set-addresses p0 "0a:00:00:00:00:15 192.168.1.2 192.168.1.12
192.168.1.14"
ovn-nbctl --wait=sb lsp-add sw0 p20 -- lsp-set-addresses p20 dynamic
AT_CHECK([ovn-nbctl get Logical-Switch-Port p20 dynamic_addresses], [0],
["0a:00:00:00:00:16 192.168.1.13"
@@ -5208,7 +5208,6 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p31
dynamic_addresses], [0],
# Update the static MAC address with dynamically allocated IP and check
# if the MAC address is updated in 'Logical_Switch_Port.dynamic_adddresses'
ovn-nbctl --wait=sb lsp-set-addresses p31 "fe:dc:ba:98:76:55 dynamic"
-ovn-nbctl get Logical-Switch-Port p31 dynamic_addresses
AT_CHECK([ovn-nbctl get Logical-Switch-Port p31 dynamic_addresses], [0],
["fe:dc:ba:98:76:55 192.168.1.18"
@@ -5216,7 +5215,7 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p31
dynamic_addresses], [0],
ovn-nbctl --wait=sb lsp-set-addresses p31 "dynamic"
AT_CHECK([ovn-nbctl get Logical-Switch-Port p31 dynamic_addresses], [0],
- ["fe:dc:ba:98:76:55 192.168.1.18"
+ ["0a:00:00:00:00:21 192.168.1.18"
])
ovn-nbctl --wait=sb lsp-set-addresses p31 "fe:dc:ba:98:76:56 dynamic"
@@ -5233,21 +5232,21 @@ ovn-nbctl --wait=sb lsp-add sw0 p32 --
lsp-set-addresses p32 \
"dynamic"
# 192.168.1.20 should be assigned as 192.168.1.19 is excluded.
AT_CHECK([ovn-nbctl get Logical-Switch-Port p32 dynamic_addresses], [0],
- ["0a:00:00:00:00:21 192.168.1.20"
+ ["0a:00:00:00:00:22 192.168.1.20"
])
ovn-nbctl --wait=sb lsp-add sw0 p33 -- lsp-set-addresses p33 \
"dynamic"
# 192.168.1.22 should be assigned as 192.168.1.21 is excluded.
AT_CHECK([ovn-nbctl get Logical-Switch-Port p33 dynamic_addresses], [0],
- ["0a:00:00:00:00:22 192.168.1.22"
+ ["0a:00:00:00:00:23 192.168.1.22"
])
ovn-nbctl --wait=sb lsp-add sw0 p34 -- lsp-set-addresses p34 \
"dynamic"
# 192.168.1.51 should be assigned as 192.168.1.23-192.168.1.50 is excluded.
AT_CHECK([ovn-nbctl get Logical-Switch-Port p34 dynamic_addresses], [0],
- ["0a:00:00:00:00:23 192.168.1.51"
+ ["0a:00:00:00:00:24 192.168.1.51"
])
# Now clear the exclude_ips list. 192.168.1.19 should be assigned.
@@ -5255,7 +5254,7 @@ ovn-nbctl --wait=sb set Logical-switch sw0
other_config:exclude_ips="invalid"
ovn-nbctl --wait=sb lsp-add sw0 p35 -- lsp-set-addresses p35 \
"dynamic"
AT_CHECK([ovn-nbctl get Logical-Switch-Port p35 dynamic_addresses], [0],
- ["0a:00:00:00:00:24 192.168.1.19"
+ ["0a:00:00:00:00:25 192.168.1.19"
])
# Set invalid data in exclude_ips list. It should be ignored.
@@ -5264,7 +5263,7 @@ ovn-nbctl --wait=sb lsp-add sw0 p36 -- lsp-set-addresses
p36 \
"dynamic"
# 192.168.1.21 should be assigned as that's the next free one.
AT_CHECK([ovn-nbctl get Logical-Switch-Port p36 dynamic_addresses], [0],
- ["0a:00:00:00:00:25 192.168.1.21"
+ ["0a:00:00:00:00:26 192.168.1.21"
])
# Clear the dynamic addresses assignment request.
@@ -5281,16 +5280,17 @@ ovn-nbctl --wait=sb lsp-add sw0 p37 --
lsp-set-addresses p37 \
# With prefix aef0 and mac 0a:00:00:00:00:26, the dynamic IPv6 should be
# - aef0::800:ff:fe00:26 (EUI64)
AT_CHECK([ovn-nbctl get Logical-Switch-Port p37 dynamic_addresses], [0],
- ["0a:00:00:00:00:26 192.168.1.21 aef0::800:ff:fe00:26"
+ ["0a:00:00:00:00:27 192.168.1.21 aef0::800:ff:fe00:27"
])
ovn-nbctl --wait=sb ls-add sw4
-ovn-nbctl --wait=sb set Logical-switch sw4 other_config:ipv6_prefix="bef0::"
+ovn-nbctl --wait=sb set Logical-switch sw4 other_config:ipv6_prefix="bef0::" \
+-- set Logical-switch sw4 other_config:subnet=192.168.2.0/30
ovn-nbctl --wait=sb lsp-add sw4 p38 -- lsp-set-addresses p38 \
"dynamic"
AT_CHECK([ovn-nbctl get Logical-Switch-Port p38 dynamic_addresses], [0],
- ["0a:00:00:00:00:27 bef0::800:ff:fe00:27"
+ ["0a:00:00:00:00:28 192.168.2.2 bef0::800:ff:fe00:28"
])
ovn-nbctl --wait=sb lsp-add sw4 p39 -- lsp-set-addresses p39 \
@@ -5300,29 +5300,78 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p39
dynamic_addresses], [0],
["f0:00:00:00:10:12 bef0::f200:ff:fe00:1012"
])
-# Clear the other_config for sw4. No dynamic ip should be assigned.
-ovn-nbctl --wait=sb clear Logical-switch sw4 other_config
+# Test the case where IPv4 addresses are exhausted and IPv6 prefix is set
+# p40 should not have an IPv4 address since the pool is exhausted
ovn-nbctl --wait=sb lsp-add sw4 p40 -- lsp-set-addresses p40 \
"dynamic"
-
AT_CHECK([ovn-nbctl get Logical-Switch-Port p40 dynamic_addresses], [0],
+ ["0a:00:00:00:00:29 bef0::800:ff:fe00:29"
+])
+
+# Test dynamic changes on switch ports.
+#
+ovn-nbctl --wait=sb ls-add sw5
+ovn-nbctl --wait=sb lsp-add sw5 p41 -- lsp-set-addresses p41 \
+"dynamic"
+# p41 will start with nothing
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
[[[]]
])
-# Test the case where IPv4 addresses are exhausted and IPv6 prefix is set
-ovn-nbctl --wait=sb set Logical-switch sw4 other_config:subnet=192.168.2.0/30 \
--- set Logical-switch sw4 other_config:ipv6_prefix="bef0::"
+# Set a subnet. Now p41 should have an ipv4 address, too
+ovn-nbctl --wait=sb add Logical-Switch sw5 other_config subnet=192.168.1.0/24
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+ ["0a:00:00:00:00:2a 192.168.1.2"
+])
-# Now p40 should be assigned with dynamic addresses.
-AT_CHECK([ovn-nbctl get Logical-Switch-Port p40 dynamic_addresses], [0],
- ["0a:00:00:00:00:28 192.168.2.2 bef0::800:ff:fe00:28"
+# Clear the other_config. The IPv4 address should be gone
+ovn-nbctl --wait=sb clear Logical-Switch sw5 other_config
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+ [[[]]
])
-ovn-nbctl --wait=sb lsp-add sw4 p41 -- lsp-set-addresses p41 \
-"dynamic"
-# p41 should not have IPv4 address (as the pool is exhausted).
+# Set an IPv6 prefix. Now p41 should have an IPv6 address.
+ovn-nbctl --wait=sb set Logical-Switch sw5 other_config:ipv6_prefix="aef0::"
AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
- ["0a:00:00:00:00:29 bef0::800:ff:fe00:29"
+ ["0a:00:00:00:00:2b aef0::800:ff:fe00:2b"
+])
+
+# Change the MAC address to a static one. The IPv6 address should update.
+ovn-nbctl --wait=sb lsp-set-addresses p41 "f0:00:00:00:10:2b dynamic"
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+ ["f0:00:00:00:10:2b aef0::f200:ff:fe00:102b"
+])
+
+# Change the IPv6 prefix. The IPv6 address should update.
+ovn-nbctl --wait=sb set Logical-Switch sw5 other_config:ipv6_prefix="bef0::"
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+ ["f0:00:00:00:10:2b bef0::f200:ff:fe00:102b"
+])
+
+# Clear the other_config. The IPv6 address should be gone
+ovn-nbctl --wait=sb clear Logical-Switch sw5 other_config
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+ [[[]]
+])
+
+# Set the subnet again. Now p41 should get the IPv4 address again.
+ovn-nbctl --wait=sb add Logical-Switch sw5 other_config subnet=192.168.1.0/24
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+ ["f0:00:00:00:10:2b 192.168.1.2"
+])
+
+# Add another port with a conflicting static IPv4 address. p41 should update.
+ovn-nbctl --wait=sb lsp-add sw5 p42 -- lsp-set-addresses p42 \
+"f0:00:00:00:10:2c 192.168.1.2"
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+ ["f0:00:00:00:10:2b 192.168.1.3"
+])
+
+# Add an excluded IP address that conflicts with p41. p41 should update.
+ovn-nbctl --wait=sb add Logical-Switch sw5 other_config \
+exclude_ips="192.168.1.3"
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
+ ["f0:00:00:00:10:2b 192.168.1.4"
])
as ovn-sb
