The IPAM is keeping track of used addresses for given LS to avoid
conflicts and potentially assigning already used address to the
next LSP. However during I-P the IPAM was destroyed and created
again from scratch without adding back the already used IPs. Make
sure we add those IPs back.
Fixes: 31c704a388a7 ("northd: Incremental processing of LSPs IPAM in "northd"
node.")
Reported-at: https://issues.redhat.com/browse/FDP-2982
Signed-off-by: Ales Musil <[email protected]>
---
northd/ipam.c | 79 ++++++++++++++-------------------------------
northd/ipam.h | 4 ++-
northd/northd.c | 12 ++++++-
tests/ovn-macros.at | 11 +++++++
tests/ovn-northd.at | 31 ++++++++++++++++++
tests/ovn.at | 16 ++++-----
6 files changed, 89 insertions(+), 64 deletions(-)
diff --git a/northd/ipam.c b/northd/ipam.c
index a1801e554..5f80649c5 100644
--- a/northd/ipam.c
+++ b/northd/ipam.c
@@ -24,11 +24,6 @@ static void init_ipam_ipv4(const char *subnet_str,
static bool ipam_is_duplicate_mac(struct eth_addr *ea, uint64_t mac64,
bool warn);
-static void ipam_insert_ip_for_datapath(struct ovn_datapath *, uint32_t, bool);
-
-static void ipam_insert_lsp_addresses(struct ovn_datapath *,
- struct lport_addresses *);
-
static enum dynamic_update_type dynamic_mac_changed(const char *,
struct dynamic_address_update *);
@@ -109,17 +104,6 @@ ipam_insert_ip(struct ipam_info *info, uint32_t ip, bool
dynamic)
return true;
}
-static void
-ipam_insert_ip_for_datapath(struct ovn_datapath *od, uint32_t ip,
- bool dynamic)
-{
- if (!od) {
- return;
- }
-
- ipam_insert_ip(&od->ipam_info, ip, dynamic);
-}
-
uint32_t
ipam_get_unused_ip(struct ipam_info *info)
{
@@ -568,7 +552,7 @@ update_unchanged_dynamic_addresses(struct
dynamic_address_update *update)
ipam_insert_mac(&update->current_addresses.ea, false);
}
if (update->ipv4 == NONE && update->current_addresses.n_ipv4_addrs) {
- ipam_insert_ip_for_datapath(update->op->od,
+ ipam_insert_ip(&update->op->od->ipam_info,
ntohl(update->current_addresses.ipv4_addrs[0].addr),
true);
}
@@ -698,7 +682,7 @@ update_dynamic_addresses(struct dynamic_address_update
*update)
ipam_insert_mac(&mac, true);
if (ip4) {
- ipam_insert_ip_for_datapath(update->od, ntohl(ip4), true);
+ ipam_insert_ip(&update->od->ipam_info, ntohl(ip4), true);
ds_put_format(&new_addr, " "IP_FMT, IP_ARGS(ip4));
}
if (!IN6_ARE_ADDR_EQUAL(&ip6, &in6addr_any)) {
@@ -783,54 +767,41 @@ update_ipam_ls(struct ovn_datapath *od, struct vector
*updates,
}
}
-void
-ipam_add_port_addresses(struct ovn_datapath *od, struct ovn_port *op)
+static void
+ipam_add_lport_addresses(struct ipam_info *info,
+ const struct lport_addresses *laddrs)
{
- if (!od || !op) {
- return;
+ for (size_t j = 0; j < laddrs->n_ipv4_addrs; j++) {
+ ipam_insert_ip(info, ntohl(laddrs->ipv4_addrs[j].addr), false);
}
+}
+
+void
+ipam_add_lsp_port_addresses(struct ovn_port *op, bool check_mac_duplicate)
+{
+ ovs_assert(op->nbsp);
+
+ struct ipam_info *info = &op->od->ipam_info;
if (op->n_lsp_non_router_addrs) {
/* Add all the port's addresses to address data structures. */
for (size_t i = 0; i < op->n_lsp_non_router_addrs; i++) {
- ipam_insert_lsp_addresses(od, &op->lsp_addrs[i]);
- }
- } else if (op->lrp_networks.ea_s[0]) {
- ipam_insert_mac(&op->lrp_networks.ea, true);
-
- if (!op->peer || !op->peer->nbsp || !op->peer->od || !op->peer->od->nbs
- || !smap_get(&op->peer->od->nbs->other_config, "subnet")) {
- return;
+ ipam_insert_mac(&op->lsp_addrs[i].ea, check_mac_duplicate);
+ ipam_add_lport_addresses(info, &op->lsp_addrs[i]);
}
+ }
- for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
- uint32_t ip = ntohl(op->lrp_networks.ipv4_addrs[i].addr);
- /* If the router has the first IP address of the subnet, don't add
- * it to IPAM. We already added this when we initialized IPAM for
- * the datapath. This will just result in an erroneous message
- * about a duplicate IP address.
- */
- if (ip != op->peer->od->ipam_info.start_ipv4) {
- ipam_insert_ip_for_datapath(op->peer->od, ip, false);
- }
- }
+ if (op->peer && op->peer->nbrp) {
+ ipam_add_lport_addresses(info, &op->peer->lrp_networks);
}
}
-static void
-ipam_insert_lsp_addresses(struct ovn_datapath *od,
- struct lport_addresses *laddrs)
+void
+ipam_add_lrp_port_addresses(struct ovn_port *op)
{
- ipam_insert_mac(&laddrs->ea, true);
+ ovs_assert(op->nbrp);
- /* IP is only added to IPAM if the switch's subnet option
- * is set, whereas MAC is always added to MACAM. */
- if (!od->ipam_info.allocated_ipv4s) {
- return;
- }
-
- for (size_t j = 0; j < laddrs->n_ipv4_addrs; j++) {
- uint32_t ip = ntohl(laddrs->ipv4_addrs[j].addr);
- ipam_insert_ip_for_datapath(od, ip, false);
+ if (op->lrp_networks.ea_s[0]) {
+ ipam_insert_mac(&op->lrp_networks.ea, true);
}
}
diff --git a/northd/ipam.h b/northd/ipam.h
index c825875a8..24dd23232 100644
--- a/northd/ipam.h
+++ b/northd/ipam.h
@@ -69,6 +69,8 @@ void update_ipam_ls(struct ovn_datapath *, struct vector *,
bool);
void update_dynamic_addresses(struct dynamic_address_update *);
-void ipam_add_port_addresses(struct ovn_datapath *, struct ovn_port *);
+void ipam_add_lrp_port_addresses(struct ovn_port *);
+
+void ipam_add_lsp_port_addresses(struct ovn_port *, bool check_mac_duplicate);
#endif /* NORTHD_IPAM_H */
diff --git a/northd/northd.c b/northd/northd.c
index 983975dac..b1371e4bf 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -2034,7 +2034,11 @@ join_logical_ports(const struct sbrec_port_binding_table
*sbrec_pb_table,
* it relies on proper peers to be set
*/
HMAP_FOR_EACH (op, key_node, ports) {
- ipam_add_port_addresses(op->od, op);
+ if (op->nbsp) {
+ ipam_add_lsp_port_addresses(op, true);
+ } else if (op->nbrp) {
+ ipam_add_lrp_port_addresses(op);
+ }
}
}
@@ -5217,6 +5221,12 @@ bool northd_handle_ipam_changes(struct northd_data *nd)
od->ipam_info_initialized = false;
}
init_ipam_info_for_datapath(od);
+
+ struct ovn_port *op;
+ HMAP_FOR_EACH (op, dp_node, &od->ports) {
+ ipam_add_lsp_port_addresses(op, false);
+ }
+
update_ipam_ls(od, &updates, false);
}
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 8e77be3af..aeb4149d5 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -1442,6 +1442,17 @@ restart_ovsdb_controller_updates() {
AT_CHECK([nft delete table ip ovn-test])
}
+sleep_nb() {
+ echo NB going to sleep
+ AT_CHECK([kill -STOP $(cat ovn-nb/ovsdb-server.pid)])
+ on_exit "test -e ovn-nb/ovsdb-server.pid && kill -CONT $(cat
ovn-nb/ovsdb-server.pid)"
+}
+
+wake_up_nb() {
+ echo NB waking up
+ AT_CHECK([kill -CONT $(cat ovn-nb/ovsdb-server.pid)])
+}
+
trim_zeros() {
sed 's/\(00\)\{1,\}$//'
}
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index e29f6d7b5..a7cfc15ea 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -19902,3 +19902,34 @@ check ovn-nbctl --wait=sb lsp-set-addresses lport1
"f0:00:0f:01:02:04 192.168.0.
check_row_count sb:Service_Monitor 2
AT_CLEANUP
+
+AT_SETUP([ipam router ports - incremental processing])
+ovn_start
+check ovn-nbctl ls-add sw
+check ovn-nbctl set logical_switch sw other-config:subnet=192.168.1.0/24
+for i in 2 3 4; do
+ check ovn-nbctl lr-add ro$i
+ check ovn-nbctl lsp-add sw swp$i
+ check ovn-nbctl --wait=sb sync
+
+ sleep_northd
+ check ovn-nbctl lsp-set-addresses swp$i "02:00:00:00:00:0$i dynamic"
+ sleep_nb
+
+
+ wake_up_northd
+ sleep_northd
+
+ wake_up_nb
+ cidr=$(ovn-nbctl get logical_switch_port swp$i dynamic_addresses |cut -f2
-d' '|cut -f1 -d\")
+ wake_up_northd
+
+ check ovn-nbctl --wait=sb lrp-add ro$i rop$i 02:00:00:00:00:0$i $cidr/24 \
+ -- set logical_switch_port swp$i type=router \
+ options:router-port=rop$i addresses=router
+ AT_CHECK_UNQUOTED([ovn-nbctl get logical_router_port rop$i networks], [0],
[[["192.168.1.$i/24"]]
+])
+done
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
diff --git a/tests/ovn.at b/tests/ovn.at
index 9082bba82..979c6d192 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -9335,7 +9335,7 @@ done
# and addresses set by the user.
check ovn-nbctl lsp-set-addresses p0 "0a:00:00:a8:01:17 192.168.1.2
192.168.1.12 192.168.1.14"
check ovn-nbctl --wait=sb lsp-add sw0 p20 -- lsp-set-addresses p20 dynamic
-check_dynamic_addresses p20 "0a:00:00:a8:01:03 192.168.1.13"
+check_dynamic_addresses p20 "0a:00:00:a8:01:18 192.168.1.13"
# Test for logical router port address management.
check_uuid ovn-nbctl create Logical_Router name=R1
@@ -9344,12 +9344,12 @@ network="192.168.1.1/24" mac=\"0a:00:00:a8:01:19\" \
-- add Logical_Router R1 ports @lrp -- lsp-add sw0 rp-sw0 \
-- set Logical_Switch_Port rp-sw0 type=router options:router-port=sw0
check ovn-nbctl --wait=sb lsp-add sw0 p21 -- lsp-set-addresses p21 dynamic
-check_dynamic_addresses p21 "0a:00:00:a8:01:18 192.168.1.15"
+check_dynamic_addresses p21 "0a:00:00:a8:01:1a 192.168.1.15"
# Test for address reuse after logical port is deleted.
check ovn-nbctl lsp-del p0
check ovn-nbctl --wait=sb lsp-add sw0 p23 -- lsp-set-addresses p23 dynamic
-check_dynamic_addresses p23 "0a:00:00:a8:01:1a 192.168.1.2"
+check_dynamic_addresses p23 "0a:00:00:a8:01:03 192.168.1.2"
# Test for multiple addresses to one logical port.
check ovn-nbctl lsp-add sw0 p25 -- lsp-set-addresses p25 \
@@ -9409,19 +9409,19 @@ check_dynamic_addresses p33 "0a:00:00:a8:01:1f
192.168.1.22"
check 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.
-check_dynamic_addresses p34 "0a:00:00:a8:01:20 192.168.1.51"
+check_dynamic_addresses p34 "0a:00:00:a8:01:34 192.168.1.51"
# Now clear the exclude_ips list. 192.168.1.19 should be assigned.
check ovn-nbctl --wait=sb set Logical-switch sw0
other_config:exclude_ips="invalid"
check ovn-nbctl --wait=sb lsp-add sw0 p35 -- lsp-set-addresses p35 "dynamic"
-check_dynamic_addresses p35 "0a:00:00:a8:01:21 192.168.1.19"
+check_dynamic_addresses p35 "0a:00:00:a8:01:20 192.168.1.19"
# Set invalid data in exclude_ips list. It should be ignored.
check ovn-nbctl --wait=sb set Logical-switch sw0
other_config:exclude_ips="182.168.1.30"
check 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.
-check_dynamic_addresses p36 "0a:00:00:a8:01:22 192.168.1.21"
+check_dynamic_addresses p36 "0a:00:00:a8:01:21 192.168.1.21"
# Clear the dynamic addresses assignment request.
check ovn-nbctl --wait=sb clear logical_switch_port p36 addresses
@@ -9433,7 +9433,7 @@ check ovn-nbctl --wait=sb lsp-add sw0 p37 --
lsp-set-addresses p37 "dynamic"
# With prefix aef0 and mac 0a:00:00:00:00:26, the dynamic IPv6 should be
# - aef0::800:ff:fe00:26 (EUI64)
-check_dynamic_addresses p37 "0a:00:00:a8:01:22 192.168.1.21
aef0::800:ff:fea8:122"
+check_dynamic_addresses p37 "0a:00:00:a8:01:21 192.168.1.21
aef0::800:ff:fea8:121"
check ovn-nbctl --wait=sb ls-add sw4
check ovn-nbctl --wait=sb set Logical-switch sw4
other_config:ipv6_prefix="bef0::" \
@@ -9463,7 +9463,7 @@ check_dynamic_addresses p41 ''
# Set a subnet. Now p41 should have an ipv4 address, too
check ovn-nbctl --wait=sb add Logical-Switch sw5 other_config
subnet=192.168.1.0/24
-check_dynamic_addresses p41 "0a:00:00:a8:01:23 192.168.1.2"
+check_dynamic_addresses p41 "0a:00:00:a8:01:22 192.168.1.2"
# Clear the other_config. The IPv4 address should be gone
check ovn-nbctl --wait=sb clear Logical-Switch sw5 other_config
--
2.53.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
