Skip to content

Commit

Permalink
binding.c: Make sure that localport is removed from local datapath
Browse files Browse the repository at this point in the history
When localport is removed from NB, and it is the last port
remaining on the host, it is not part of local datapath
anymore. Which can cause troubles when there is recompute
happening in between the removal from NB and the removal
of interface from host. The localport would stay in lport_ids
set, so any new localport that happens to have the same unique
lport key wouldn't be initiliazed properly thorugh I-P.

Remove the localport port binding from local datapath
if it was part of the that said datapath before.

Reported-at: https://bugzilla.redhat.com/2076604
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 321f13e)
  • Loading branch information
almusil authored and numansiddique committed May 27, 2022
1 parent 4e9ecbc commit cb2e570
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 0 deletions.
9 changes: 9 additions & 0 deletions controller/binding.c
Expand Up @@ -2132,6 +2132,15 @@ handle_deleted_lport(const struct sbrec_port_binding *pb,
return;
}

/*
* Remove localport that was part of local datapath that is not
* considered to be local anymore.
*/
if (!ld && !strcmp(pb->type, "localport") &&
sset_find(&b_ctx_out->related_lports->lport_names, pb->logical_port)) {
remove_related_lport(pb, b_ctx_out);
}

/* If the binding is not local, if 'pb' is a L3 gateway port, we should
* remove its peer, if that one is local.
*/
Expand Down
56 changes: 56 additions & 0 deletions tests/ovn-controller.at
Expand Up @@ -2130,3 +2130,59 @@ AT_CHECK([grep "Parsing of ovn-chassis-mac-mappings failed" hv1/ovn-controller.l

OVN_CLEANUP([hv1])
AT_CLEANUP

AT_SETUP([ovn-controller - localport can be recreated])

ovn_start

net_add n1
sim_add hv1
as hv1
ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.1

port_binding_cookie() {
name=$1
ovn-sbctl --bare --columns _uuid find port_binding logical_port=$name |\
cut -d '-' -f 1 | tr -d '\n' | sed 's/^0\{0,8\}//'
}

create_localport() {
AT_CHECK([ovn-nbctl lsp-add ls0 metadata])
AT_CHECK([ovn-nbctl lsp-set-type metadata localport])
AT_CHECK([ovn-nbctl lsp-set-addresses metadata "00:00:00:00:10:25 192.168.10.25"])
}

bind_ports() {
AT_CHECK([ovs-vsctl add-port br-int vm0 -- set interface vm0 type=internal external_ids:iface-id=vm0])
AT_CHECK([ovs-vsctl add-port br-int metadata -- set interface metadata type=internal external_ids:iface-id=metadata])
}

# Create one VIF and localport and bind it to chassis
AT_CHECK([ovn-nbctl ls-add ls0])
AT_CHECK([ovn-nbctl lsp-add ls0 vm0])
AT_CHECK([ovn-nbctl lsp-set-addresses vm0 "00:00:00:00:10:10 192.168.10.10"])
create_localport
bind_ports

# Check that localport has all physical flows defined
OVS_WAIT_UNTIL([test 6 = $(as hv1 ovs-ofctl dump-flows br-int | grep -c $(port_binding_cookie metadata))])

# Remove ls0 from local datapaths
AT_CHECK([ovs-vsctl del-port br-int vm0])
AT_CHECK([ovn-appctl inc-engine/recompute])

# Check that localports physical flows are removed
OVS_WAIT_UNTIL([test 0 = $(as hv1 ovs-ofctl dump-flows br-int | grep -c $(port_binding_cookie metadata))])

# The order is impotant, if the port is removed first, the bug wouldn't be triggered
AT_CHECK([ovn-nbctl lsp-del metadata])
AT_CHECK([ovs-vsctl del-port br-int metadata])
create_localport
bind_ports

# Check that localport has all physical flows re-defined
OVS_WAIT_UNTIL([test 6 = $(as hv1 ovs-ofctl dump-flows br-int | grep -c $(port_binding_cookie metadata))])

OVN_CLEANUP([hv1])
AT_CLEANUP

0 comments on commit cb2e570

Please sign in to comment.