Skip to content

Commit

Permalink
controller: Fix an issue wrt cleanup of stale patch port.
Browse files Browse the repository at this point in the history
Issue:
Upon updating the network_name option of localnet port from one physical
bridge to another, ovn-controller fails to cleanup the peer localnet
patch port from the old bridge and ends up creating a duplicate peer
localnet patch port which fails in the following ovsdb transaction:

```
"Transaction causes multiple rows in \"Interface\" table to have
identical values
(patch-localnet_a7d47490-8a90-4c4d-8266-2915ad07c185-to-br-int)
```

Current workflow:
1. Keep a set of all existing localnet ports on br-int. Let us call this
   set: existing_ports.
2. For each localnet port in SB:
   2.1 Create a patch port on br-int. (if it already exists on br-int,
       do not create but remove the entry from exisitng_ports set).
   2.2 Create a peer patch port on br-phys-x. (if it already exists on the
       bridge, do not create but remove the entry from exisitng_ports set).
3. Finally, cleanup all the ports and its peers from exisiting_ports.

With the above workflow, upon network_name change of localnet LSP, since
ports on br-int does not change, only peer port needs to be move from
br-phys-x to br-phys-y, the localnet port is removed from
exisiting_ports in #2.1 and its peer never becomes eligible for cleanup.

Fix:
Include both patch port on br-int and its peer port in the
exisiting_ports set as part of #1.
This make sures that the peer port is only removed from existing_ports
only if it is already present on the correct bridge. (#2.1/#2.2)
Otherwise, during the cleanup in ovn-org#3 it will be considered.

Fixes: b600316 ("Don't delete patch ports that don't belong to br-int")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 2609cd9)
  • Loading branch information
priyankar-jain authored and numansiddique committed May 9, 2024
1 parent a2e2279 commit ed56dfb
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 0 deletions.
15 changes: 15 additions & 0 deletions controller/patch.c
Expand Up @@ -295,6 +295,21 @@ patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
|| smap_get(&port->external_ids, "ovn-l3gateway-port")
|| smap_get(&port->external_ids, "ovn-logical-patch-port")) {
shash_add(&existing_ports, port->name, port);
/* Also add peer ports to the list. */
for (size_t j = 0; j < port->n_interfaces; j++) {
struct ovsrec_interface *p_iface = port->interfaces[j];
if (strcmp(p_iface->type, "patch")) {
continue;
}
const char *peer_name = smap_get(&p_iface->options, "peer");
if (peer_name) {
const struct ovsrec_port *peer_port =
get_port(ovsrec_port_by_name, peer_name);
if (peer_port) {
shash_add(&existing_ports, peer_port->name, peer_port);
}
}
}
}
}

Expand Down
109 changes: 109 additions & 0 deletions tests/ovn.at
Expand Up @@ -34870,6 +34870,115 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
])

OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-controller update network_name option for localnet port])
ovn_start
net_add n1

sim_add hv1
as hv1

# Bridge Topology
# Initial: br-int and br-phys-1 connected through ovn-localnet patch port.
#
# br-phys-1 -- br-int
#
# Final: br-int and br-phys-3 connected through ovn-localnet patch port.
# Similarly br-int-2 and br-hys-2 connected through localnet patch port
# but not owned by this controller.
#
# br-phys-1 br-int -- br-phys-3
# br-int-2 -- br-phys-2

# Create bridges
check ovs-vsctl add-br br-int
check ovs-vsctl add-br br-int-2
check ovs-vsctl add-br br-phys-1
check ovs-vsctl add-br br-phys-2
check ovs-vsctl add-br br-phys-3
check ovs-vsctl set open . external-ids:ovn-bridge-mappings-hv1=phys-1:br-phys-1,phy-2:br-phys-2,phys-3:br-phys-3

ovn_attach n1 br-phys-1 192.168.1.1 24

check ovs-vsctl -- add-port br-int vif1 -- \
set interface vif1 external-ids:iface-id=lp1

check ovn-nbctl ls-add ls

check ovn-nbctl lsp-add ls lp1
check ovn-nbctl lsp-set-addresses lp1 "00:00:00:00:00:01 10.0.0.1"

check ovn-nbctl lsp-add ls ln_port
check ovn-nbctl lsp-set-addresses ln_port unknown
check ovn-nbctl lsp-set-type ln_port localnet
check ovn-nbctl lsp-set-options ln_port network_name=phys-1
wait_for_ports_up
check ovn-nbctl --wait=hv sync

# Allow controller to immediately clean patch ports up if any.
check ovn-appctl -t ovn-controller debug/ignore-startup-delay

# check that patch port created on br-int and br-phys-1.
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl --columns _uuid --bare find Port name=patch-br-int-to-ln_port | wc -l)
])
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl list-ports br-int | grep patch-br-int-to-ln_port | wc -l)
])
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl --columns _uuid --bare find Port name=patch-ln_port-to-br-int | wc -l)
])
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl list-ports br-phys-1 | grep patch-ln_port-to-br-int | wc -l)
])

check ovn-appctl debug/pause

# Now move the localnet port from phys-1 to phys-3.
check ovn-nbctl lsp-set-options ln_port network_name=phys-3

# Also create fake localnet ports on br-int-2
check ovs-vsctl -- add-port br-int-2 fake-int-2-to-phys-2 -- \
set port fake-int-2-to-phys-2 external-ids:ovn-localnet-port=fake-port -- \
set interface fake-int-2-to-phys-2 options:peer=fake-phys-2-to-int-2 type=patch
check ovs-vsctl -- add-port br-phys-2 fake-phys-2-to-int-2 -- \
set port fake-phys-2-to-int-2 external-ids:ovn-localnet-port=fake-port -- \
set interface fake-phys-2-to-int-2 options:peer=fake-int-2-to-phys-2 type=patch

check ovn-appctl debug/resume
check ovn-nbctl --wait=hv sync

# patch for port on br-phys-1 is cleanedup.
OVS_WAIT_UNTIL([
test 0 = $(ovs-vsctl list-ports br-phys-1 | grep patch-ln_port-to-br-int | wc -l)
])

# Patch port created on br-int and br-phy-3
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl --columns _uuid --bare find Port name=patch-br-int-to-ln_port | wc -l)
])
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl list-ports br-int | grep patch-br-int-to-ln_port | wc -l)
])
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl --columns _uuid --bare find Port name=patch-ln_port-to-br-int | wc -l)
])
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl list-ports br-phys-3 | grep patch-ln_port-to-br-int | wc -l)
])

# check that patch port on a different bridge is not touched
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl --columns _uuid --bare find Port name=fake-int-2-to-phys-2 | wc -l)
])
OVS_WAIT_UNTIL([
test 1 = $(ovs-vsctl --columns _uuid --bare find Port name=fake-phys-2-to-int-2 | wc -l)
])

OVN_CLEANUP([hv1])
AT_CLEANUP
])

OVN_FOR_EACH_NORTHD([
AT_SETUP([Re-create encap tunnels during integration bridge migration])
ovn_start
Expand Down

0 comments on commit ed56dfb

Please sign in to comment.