From: Numan Siddique <num...@ovn.org>
Consider the below logical topology
sw0-p1 -
| (distributed NATs configured)
sw0-p2 - -> sw0 -> lr0 ----
... | |
sw0-pn - |
|
sw1-p1 - |
| |
sw1p-2 - -> sw1 -> lr1 ---- --- public (provider switch)
... | |
sw1-pn- |
|
swn-p1 - |
| |
swn-p2- -> swn -> lrn ----
... |
swn-pn -
All the routers are connected to the provider switch via a ditributed
gateway port.
If sw0-p1 is resident on the chassis C1, then since there is a path
to all the switches and the routers, ovn-controller will add all
these datapaths to its 'local_datapaths' map. This in turn results
in processing all the logical flows and installing all the openflows
and in turn wasting the CPU time. This can be very costly in
a highly scaled deployment.
Note that for the same topology, if there are no distributed NATs
configured on lr0, thanks to the commit [1], ovn-northd will set
the option always-redirect=true in the chassis-redirect port.
If this option is set, then ovn-controller only adds [sw0, lr0]
in its local datapaths.
However for deployments with distributed NATs, ovn-controller adds
all the datapaths to its local_datapaths map.
With this patch, when ovn-controller claims sw0-p1, it only adds
[sw0, lr0, public] to its local_datapaths map. If it later claims
sw1-p1, it will add [sw1, lr1] to the map.
This reduces the recompute time and the number of openflow rules
added to ovs-vswitchd significantly.
If the public switch is connected to a router 'lr4' and and the
router port is not a distributed gateway port (DGP), then [lr4, sw4]
are also added to the map.
This patch handles the scenario when a gateway port is configured as
a normal distributed router port (i.e its gateway_chassis or
ha_chassis_group is cleared). In this case it adds both the switch
and router datapaths to the local_datapaths map incrementally.
However it doesn't handle the other way - i.e a distributed router
port is configured as gateway port. ovn-controller will not delete
the switch and router datapaths from its map when handling these
changes incrementally. A subsequent full recompute will fix this.
Although we can handle this scenario incrementally, it comes up with
a bit of code complexity and we can handle it later if required.
Having the openflows of these datapaths is not going to be of any
harm.
I tested this patch with a deployment of below logical resources:
No of logical switches - 778
No of logical routers - 871
No of logical flows - 85626
No of 'ovn-sbctl dump-flows' - 208631
Without this patch, afte claiming sw0-p1, ovn-controller adds 269098
openflow rules and it takes approx 2500 milli seconds for a
recompute.
With this patch, after claiming sw0-p1, ovn-controller adds 21350
openflow rules and it takes approx 280 milli seconds for a recompute.
There is approx 90% reduction in the openflow rules and 88% reduction
in recompute time when a compute node has VIFs from one logical
switch.
[1] - 22298fd37 ("ovn-controller: Don't flood fill local datapaths beyond DGP
boundary.)
Suggested-by: Han Zhou <hz...@ovn.org>
Co-authored-by: Xavier Simonart <xsimo...@redhat.com>
Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
Signed-off-by: Numan Siddique <num...@ovn.org>
---
controller/binding.c | 35 ++--
controller/local_data.c | 72 ++++----
controller/local_data.h | 6 +-
tests/ovn.at | 375 ++++++++++++++++++++++++++++++++++++++++
tests/system-ovn.at | 28 +++
5 files changed, 468 insertions(+), 48 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index b657de9e44..fdb0ad124b 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -1923,11 +1923,14 @@ consider_nonvif_lport_(const struct sbrec_port_binding
*pb,
/* Add the peer datapath to the local datapaths if it's
* not present yet.
*/
- if (need_add_peer_to_local(
+ const struct sbrec_port_binding *peer =
+ lport_get_peer(pb, b_ctx_in->sbrec_port_binding_by_name);
+
+ if (peer && need_add_peer_to_local(
b_ctx_in->sbrec_port_binding_by_name, pb,
- b_ctx_in->chassis_rec)) {
+ peer, b_ctx_in->chassis_rec)) {
add_local_datapath_peer_port(
- pb, b_ctx_in->chassis_rec,
+ pb, peer, b_ctx_in->chassis_rec,
b_ctx_in->sbrec_datapath_binding_by_key,
b_ctx_in->sbrec_port_binding_by_datapath,
b_ctx_in->sbrec_port_binding_by_name,
@@ -2958,27 +2961,27 @@ consider_patch_port_for_local_datapaths(const struct
sbrec_port_binding *pb,
struct binding_ctx_in *b_ctx_in,
struct binding_ctx_out *b_ctx_out)
{
- struct local_datapath *ld =
- get_local_datapath(b_ctx_out->local_datapaths,
- pb->datapath->tunnel_key);
+ const struct sbrec_port_binding *peer;
+ struct local_datapath *peer_ld = NULL;
+ struct local_datapath *ld = NULL;
+
+ ld = get_local_datapath(b_ctx_out->local_datapaths,
+ pb->datapath->tunnel_key);
+ peer = lport_get_peer(pb, b_ctx_in->sbrec_port_binding_by_name);
if (!ld) {
/* If 'ld' for this lport is not present, then check if
* there is a peer for this lport. If peer is present
* and peer's datapath is already in the local datapaths,
* then add this lport's datapath to the local_datapaths.
* */
- const struct sbrec_port_binding *peer;
- struct local_datapath *peer_ld = NULL;
- peer = lport_get_patch_peer(pb, b_ctx_in->sbrec_port_binding_by_name);
if (peer) {
- peer_ld =
- get_local_datapath(b_ctx_out->local_datapaths,
- peer->datapath->tunnel_key);
+ peer_ld = get_local_datapath(b_ctx_out->local_datapaths,
+ peer->datapath->tunnel_key);
}
if (peer_ld && need_add_peer_to_local(
b_ctx_in->sbrec_port_binding_by_name, peer,
- b_ctx_in->chassis_rec)) {
+ pb, b_ctx_in->chassis_rec)) {
add_local_datapath(
b_ctx_in->sbrec_datapath_binding_by_key,
b_ctx_in->sbrec_port_binding_by_datapath,
@@ -2991,11 +2994,11 @@ consider_patch_port_for_local_datapaths(const struct
sbrec_port_binding *pb,
/* Add the peer datapath to the local datapaths if it's
* not present yet.
*/
- if (need_add_peer_to_local(
+ if (peer && need_add_peer_to_local(
b_ctx_in->sbrec_port_binding_by_name, pb,
- b_ctx_in->chassis_rec)) {
+ peer, b_ctx_in->chassis_rec)) {
add_local_datapath_peer_port(
- pb, b_ctx_in->chassis_rec,
+ pb, peer, b_ctx_in->chassis_rec,
b_ctx_in->sbrec_datapath_binding_by_key,
b_ctx_in->sbrec_port_binding_by_datapath,
b_ctx_in->sbrec_port_binding_by_name,
diff --git a/controller/local_data.c b/controller/local_data.c
index 2df5cd3392..eb76f80626 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -130,44 +130,61 @@ local_datapath_destroy(struct local_datapath *ld)
free(ld);
}
-/* Checks if pb is running on local gw router or pb is a patch port
- * and the peer datapath should be added to local datapaths. */
+/* Checks if pb is running on local gw router or pb and peer
+ * are patch ports and if the peer's datapath should be added to
+ * local datapaths or not. */
bool
need_add_peer_to_local(
struct ovsdb_idl_index *sbrec_port_binding_by_name,
const struct sbrec_port_binding *pb,
+ const struct sbrec_port_binding *peer,
const struct sbrec_chassis *chassis)
{
/* This port is running on local gw router. */
- if (!strcmp(pb->type, "l3gateway") && pb->chassis == chassis) {
+ if (!strcmp(pb->type, "l3gateway") && pb->chassis == chassis &&
+ peer->chassis == chassis) {
return true;
}
- /* If it is not a patch port, no peer to add. */
+ /* If peer1 is not a patch port, no peer to add. */
if (strcmp(pb->type, "patch")) {
return false;
}
- /* If it is a regular patch port, it is fully distributed, add the peer. */
- const char *crp_name = smap_get(&pb->options, "chassis-redirect-port");
- if (!crp_name) {
+ const char *cr_pb_name = smap_get(&pb->options,
+ "chassis-redirect-port");
+ const char *cr_peer_name = smap_get(&peer->options,
+ "chassis-redirect-port");
+ if (!cr_pb_name && !cr_peer_name) {
+ /* pb and peer are regular patch ports (fully distributed),
+ * add the peer to local datapaths. */
return true;
}
- /* A DGP, find its chassis-redirect-port pb. */
- const struct sbrec_port_binding *pb_crp =
- lport_get_cr_port(sbrec_port_binding_by_name, pb, crp_name);
+ const struct sbrec_port_binding *cr_pb =
+ lport_get_cr_port(sbrec_port_binding_by_name, pb, cr_pb_name);
+ const struct sbrec_port_binding *cr_peer =
+ lport_get_cr_port(sbrec_port_binding_by_name, peer, cr_peer_name);
- if (!pb_crp) {
+ if (!cr_pb && !cr_peer) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
VLOG_WARN_RL(&rl, "chassis-redirect-port %s for DGP %s is not found.",
- crp_name, pb->logical_port);
+ cr_pb_name ? cr_pb_name : cr_peer_name,
+ cr_pb_name ? pb->logical_port :
+ peer->logical_port);
return false;
}
- /* Check if it is configured as "always-redirect". If not, then we will
+ if (datapath_is_switch(pb->datapath) && cr_peer) {
+ /* pb belongs to logical switch and peer most likely
+ * belongs to logical router. Add the peer to local datapaths
+ * only if its chassis-redirect-port is local. */
+ return ha_chassis_group_contains(cr_peer->ha_chassis_group, chassis);
+ }
+
+ /* Check if cr-pb is configured as "always-redirect". If not, then we will
* need to add the peer to local for distributed processing. */
- if (!smap_get_bool(&pb_crp->options, "always-redirect", false)) {
+ if (cr_pb && !smap_get_bool(&cr_pb->options, "always-redirect", false)) {
return true;
}
@@ -175,9 +192,10 @@ need_add_peer_to_local(
* the peer to local, which could be the localnet network, which doesn't
* have other chances to be added to local datapaths if there is no VIF
* bindings. */
- if (pb_crp->ha_chassis_group) {
- return ha_chassis_group_contains(pb_crp->ha_chassis_group, chassis);
+ if (cr_pb && cr_pb->ha_chassis_group) {
+ return ha_chassis_group_contains(cr_pb->ha_chassis_group, chassis);
}
+
return false;
}
@@ -200,6 +218,7 @@ add_local_datapath(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
void
add_local_datapath_peer_port(
const struct sbrec_port_binding *pb,
+ const struct sbrec_port_binding *peer,
const struct sbrec_chassis *chassis,
struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
@@ -208,25 +227,18 @@ add_local_datapath_peer_port(
struct hmap *local_datapaths,
struct hmap *tracked_datapaths)
{
- const struct sbrec_port_binding *peer =
- lport_get_peer(pb, sbrec_port_binding_by_name);
-
- if (!peer) {
- return;
- }
-
local_datapath_peer_port_add(ld, pb, peer);
struct local_datapath *peer_ld =
get_local_datapath(local_datapaths,
peer->datapath->tunnel_key);
if (!peer_ld) {
- add_local_datapath__(sbrec_datapath_binding_by_key,
- sbrec_port_binding_by_datapath,
- sbrec_port_binding_by_name, 1,
- peer->datapath, chassis, local_datapaths,
- tracked_datapaths);
- return;
+ peer_ld =
+ add_local_datapath__(sbrec_datapath_binding_by_key,
+ sbrec_port_binding_by_datapath,
+ sbrec_port_binding_by_name, 1,
+ peer->datapath, chassis, local_datapaths,
+ tracked_datapaths);
}
local_datapath_peer_port_add(peer_ld, peer, pb);
@@ -626,7 +638,7 @@ add_local_datapath__(struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
const struct sbrec_port_binding *peer =
lport_get_peer(pb, sbrec_port_binding_by_name);
if (peer && need_add_peer_to_local(sbrec_port_binding_by_name,
- pb, chassis)) {
+ pb, peer, chassis)) {
struct local_datapath *peer_ld =
add_local_datapath__(sbrec_datapath_binding_by_key,
sbrec_port_binding_by_datapath,
diff --git a/controller/local_data.h b/controller/local_data.h
index 1d60dada86..19d5582328 100644
--- a/controller/local_data.h
+++ b/controller/local_data.h
@@ -76,7 +76,8 @@ struct local_datapath *get_local_datapath_no_hash(
bool
need_add_peer_to_local(
struct ovsdb_idl_index *sbrec_port_binding_by_name,
- const struct sbrec_port_binding *,
+ const struct sbrec_port_binding *pb,
+ const struct sbrec_port_binding *peer,
const struct sbrec_chassis *);
void add_local_datapath(
struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
@@ -90,7 +91,8 @@ void add_local_datapath(
void local_datapaths_destroy(struct hmap *local_datapaths);
void local_datapath_destroy(struct local_datapath *ld);
void add_local_datapath_peer_port(
- const struct sbrec_port_binding *,
+ const struct sbrec_port_binding *pb,
+ const struct sbrec_port_binding *peer,
const struct sbrec_chassis *,
struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
diff --git a/tests/ovn.at b/tests/ovn.at
index bbaa653a82..a8b0770a34 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -42499,6 +42499,381 @@ wait_row_count ACL_ID 0
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ovn-controller -- local datapaths check])
+ovn_start
+net_add n1
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+sim_add hv2
+as hv2
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+
+sim_add gw1
+as gw1
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.3
+
+sim_add gw2
+as gw2
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.4
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lsp-add sw0 sw0-port1
+check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3
1000::3"
+check ovn-nbctl lsp-add sw0 sw0-port2
+check ovn-nbctl lsp-set-addresses sw0-port2 "50:54:00:00:00:02 10.0.0.4
1000::4"
+
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::1/64
+check ovn-nbctl lsp-add sw0 sw0-lr0
+check ovn-nbctl lsp-set-type sw0-lr0 router
+check ovn-nbctl lsp-set-addresses sw0-lr0 router
+check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
+
+check ovn-nbctl ls-add public
+check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
2000::1/64
+check ovn-nbctl lsp-add public public-lr0
+check ovn-nbctl lsp-set-type public-lr0 router
+check ovn-nbctl lsp-set-addresses public-lr0 router
+check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
+
+# localnet port
+check ovn-nbctl lsp-add public ln-public
+check ovn-nbctl lsp-set-type ln-public localnet
+check ovn-nbctl lsp-set-addresses ln-public unknown
+check ovn-nbctl lsp-set-options ln-public network_name=phys
+
+check ovn-nbctl lrp-set-gateway-chassis lr0-public gw1 20
+check ovn-nbctl lr-nat-add lr0 snat 172.168.0.100 10.0.0.0/24
+check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.4 sw0-port2
f0:00:00:01:02:04
+check ovn-nbctl lr-nat-add lr0 snat 2000::1 1000::/64
+check ovn-nbctl lr-nat-add lr0 dnat_and_snat 2000::2 1000::4 sw0-port2
f0:00:00:01:02:04
+
+check ovn-nbctl --wait=hv sync
+
+check_local_datapath() {
+ hv=$1
+ expected_dps=$2
+ OVS_WAIT_UNTIL(
+ [check_expected $hv $expected_dps], [echo "Did not find flows on
$expected_dps on $hv"])
+ OVS_WAIT_UNTIL(
+ [check_unexpected $hv $expected_dps], [echo "Found unexpected flows on
$hv"])
+ found=$(as $hv ovn-appctl -t ovn-controller debug/dump-local-datapaths \
+| grep -v "Local" | sed 's/.*Datapath:\s*\([[a-z0-9-]]*\).*/\1/')
+ dps=$(echo "$expected_dps" | tr ',' '\n' | sort | tr '\n' ' ')
+ found=$(echo "$found" | sort | tr '\n' ' ')
+ check test "$dps" = "$found"
+}
+
+check_unexpected() {
+ hv=$1
+ dps=$2
+ flows=$(as $hv ovs-ofctl dump-flows br-int)
+
+ # Check for flows from unexpected datapaths
+ dps=$(echo "$dps" | tr ',' ' ')
+ echo "Looking for unexpected flows on $hv"
+ for dp in $dps
+ do
+ if [[ -n "${dp}" ]]; then
+ key=$(printf "0x%x" $(fetch_column Datapath_Binding tunnel_key
external_ids:name=$dp))
+ if [[ -n "${key}" ]]; then
+ flows=$(echo "$flows" | grep -v "metadata=$key" )
+ fi
+ fi
+ done
+ n_flows=$(echo "$flows" | grep "metadata=" | wc -l)
+ if [[ "${n_flows}" != "0" ]]; then
+ #echo "hv $hv has $n_flows remaining flows"
+ return 1
+ fi
+}
+
+check_expected() {
+ # Check for flows from expected datapaths
+ hv=$1
+ dps=$2
+ dps=$(echo "$dps" | tr ',' ' ')
+ for dp in $dps
+ do
+ if [[ -n "${dp}" ]]; then
+ key=$(printf "0x%x" $(fetch_column Datapath_Binding tunnel_key
external_ids:name=$dp))
+ if [[ -n "${key}" ]]; then
+ echo "Looking for expected flows in $hv for $dp i.e. tunnel_key
+$key"
+ n_flows=$(as $hv ovs-ofctl dump-flows br-int | grep -c
"metadata=$key")
+ if [[ "${n_flows}" -eq "0" ]]; then
+ #echo "hv $hv has no flows on dp $dp"
+ return 1
+ fi
+ fi
+ fi
+ done
+}
+
+trigger_recompute() {
+ check ovn-nbctl --wait=hv sync
+ for hv in hv1 hv2 gw1 gw2
+ do
+ as $hv ovn-appctl inc-engine/recompute
+ done
+ check ovn-nbctl --wait=hv sync
+}
+
+check_local_datapath hv1 ""
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 ""
+
+# Create a VIF on hv1 for sw0-port1
+AS_BOX([create a VIF on hv1 for sw0-port1])
+
+as hv1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+ set interface hv1-vif1 external-ids:iface-id=sw0-port1 \
+ options:tx_pcap=hv1/vif1-tx.pcap \
+ options:rxq_pcap=hv1/vif1-rx.pcap \
+ ofport-request=1
+
+wait_for_ports_up sw0-port1
+
+AS_BOX([Create a VIF on hv1 for sw0-port1 - hv1 should have flows for sw0, lr0
and public])
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 ""
+
+AS_BOX([create a switch sw1 and router lr1, attach both and attach lr1 to
public])
+
+check ovn-nbctl ls-add sw1
+check ovn-nbctl lsp-add sw1 sw1-port1
+check ovn-nbctl lsp-set-addresses sw1-port1 "60:54:00:00:00:01 20.0.0.3"
+
+check ovn-nbctl lr-add lr1
+check ovn-nbctl lrp-add lr1 lr1-sw1 00:00:01:00:ef:01 20.0.0.1/24
+check ovn-nbctl lsp-add sw1 sw1-lr1
+check ovn-nbctl lsp-set-type sw1-lr1 router
+check ovn-nbctl lsp-set-addresses sw1-lr1 router
+check ovn-nbctl lsp-set-options sw1-lr1 router-port=lr1-sw1
+
+check ovn-nbctl lrp-add lr1 lr1-public 00:00:20:30:22:13 172.168.0.101/24
+check ovn-nbctl lsp-add public public-lr1
+check ovn-nbctl lsp-set-type public-lr1 router
+check ovn-nbctl lsp-set-addresses public-lr1 router
+check ovn-nbctl lsp-set-options public-lr1 router-port=lr1-public
+
+check ovn-nbctl lr-nat-add lr1 snat 172.168.0.101 20.0.0.0/24
+check ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.168.0.140 20.0.0.3
+
+AS_BOX([create a switch sw2 and router lr2, attach both and attach lr2 to
public])
+
+check ovn-nbctl ls-add sw2
+check ovn-nbctl lsp-add sw2 sw2-port1
+check ovn-nbctl lsp-set-addresses sw2-port1 "70:54:00:00:00:01 30.0.0.3"
+
+check ovn-nbctl lr-add lr2
+check ovn-nbctl lrp-add lr2 lr2-sw2 00:00:02:00:ef:01 30.0.0.1/24
+check ovn-nbctl lsp-add sw2 sw2-lr2
+check ovn-nbctl lsp-set-type sw2-lr2 router
+check ovn-nbctl lsp-set-addresses sw2-lr2 router
+check ovn-nbctl lsp-set-options sw2-lr2 router-port=lr2-sw2
+
+check ovn-nbctl lrp-add lr2 lr2-public 00:00:20:40:22:53 172.168.0.102/24
+check ovn-nbctl lsp-add public public-lr2
+check ovn-nbctl lsp-set-type public-lr2 router
+check ovn-nbctl lsp-set-addresses public-lr2 router
+check ovn-nbctl lsp-set-options public-lr2 router-port=lr2-public
+
+check ovn-nbctl lr-nat-add lr2 snat 172.168.0.102 30.0.0.0/24
+check ovn-nbctl lr-nat-add lr2 dnat_and_snat 172.168.0.150 30.0.0.3
+
+check ovn-nbctl --wait=hv sync
+
+check_local_datapath hv1 sw0,lr0,public,sw1,lr1,sw2,lr2
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public,sw1,lr1,sw2,lr2
+check_local_datapath gw2 ""
+
+AS_BOX([Set gw2 as gateway chassis for lr1-public and lr2-public])
+check ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw2 20
+check ovn-nbctl --wait=hv lrp-set-gateway-chassis lr2-public gw2 30
+wait_row_count Port_Binding 1 logical_port=cr-lr1-public
+wait_row_count Port_Binding 1 logical_port=cr-lr2-public
+
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Create a VIF on hv2 for sw1-port1])
+
+as hv2
+ovs-vsctl -- add-port br-int hv2-vif1 -- \
+ set interface hv2-vif1 external-ids:iface-id=sw1-port1 \
+ options:tx_pcap=hv2/vif1-tx.pcap \
+ options:rxq_pcap=hv2/vif1-rx.pcap \
+ ofport-request=1
+
+wait_for_ports_up sw1-port1
+
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 sw1,lr1
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+# Add distributed dnat_and_snat in lr1. hv2 should have
+# public in its local datapaths.
+AS_BOX([ Add distributed dnat_and_snat in lr1])
+
+check ovn-nbctl lr-nat-del lr1 dnat_and_snat
+check ovn-nbctl --wait=hv lr-nat-add lr1 dnat_and_snat 172.168.0.140 20.0.0.3
sw1-port1 10:00:00:01:02:14
+
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 sw1,lr1,public
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Create a VIF on hv2 for sw0-port2])
+
+as hv2
+ovs-vsctl -- add-port br-int hv2-vif2 -- \
+ set interface hv2-vif2 external-ids:iface-id=sw0-port2 \
+ options:tx_pcap=hv2/vif2-tx.pcap \
+ options:rxq_pcap=hv2/vif2-rx.pcap \
+ ofport-request=2
+
+wait_for_ports_up sw0-port2
+
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 sw0,lr0,sw1,lr1,public
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Delete the VIF for sw1-port1 in hv2])
+
+as hv2 ovs-vsctl del-port hv2-vif1
+check ovn-nbctl --wait=hv sync
+check_column "false" Port_Binding up logical_port=sw1-port1
+
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 sw0,lr0,public
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Delete the VIF for sw0-port2 in hv2])
+
+# Presently when a port binding is released we are not
+# deleting its datapath from the local_datapaths if it
+# is not relevant anymore.
+
+as hv2 ovs-vsctl del-port hv2-vif2
+check ovn-nbctl --wait=hv sync
+check_column "false" Port_Binding up logical_port=sw0-port2
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 sw0,lr0,public
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Trigger a recompute])
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Disconnect sw2 from lr2])
+
+check ovn-nbctl --wait=hv lsp-set-options sw2-lr2 router-port=lr2-sw2xxx
+
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,lr2,public
+
+AS_BOX([Reconnect sw2 to lr2 again])
+
+check ovn-nbctl --wait=hv lsp-set-options sw2-lr2 router-port=lr2-sw2
+
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Create a VIF on gw2 for sw1-port1])
+
+as gw2
+ovs-vsctl -- add-port br-int gw2-vif2 -- \
+ set interface gw2-vif2 external-ids:iface-id=sw1-port1 \
+ options:tx_pcap=gw2/vif2-tx.pcap \
+ options:rxq_pcap=gw2/vif2-rx.pcap \
+ ofport-request=2
+
+wait_for_ports_up sw1-port1
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Delete the VIF for sw1-port1 in gw2])
+
+as gw2 ovs-vsctl del-port gw2-vif2
+check ovn-nbctl --wait=hv sync
+check_column "false" Port_Binding up logical_port=sw1-port1
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 ""
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+AS_BOX([Create a logical port for public and bind it on hv2])
+# hv2 will only have public in its local datapaths.
+check ovn-nbctl lsp-add public public-p1
+
+as hv2
+ovs-vsctl -- add-port br-int hv2-vif3 -- \
+ set interface hv2-vif3 external-ids:iface-id=public-p1 \
+ options:tx_pcap=hv2/vif3-tx.pcap \
+ options:rxq_pcap=hv2/vif3-rx.pcap \
+ ofport-request=2
+
+wait_for_ports_up public-p1
+trigger_recompute
+
+check_local_datapath hv1 sw0,lr0,public
+check_local_datapath hv2 public
+check_local_datapath gw1 sw0,lr0,public
+check_local_datapath gw2 sw1,lr1,sw2,lr2,public
+
+OVN_CLEANUP([hv1], [hv2], [gw1], [gw2])
+AT_CLEANUP
+])
+
# Test when a load balancer is added to two logical switches
# which are local to different hypervisors.
OVN_FOR_EACH_NORTHD([
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 7097e4155c..5fa740cfb1 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -16672,6 +16672,13 @@ OVN_ROUTE_EQUAL([ovnvrf$vrf], [dnl
blackhole 10.42.10.10 proto ovn metric 1000
blackhole 172.16.1.150 proto ovn metric 1000])
+# Before cleanup of hv1 ovn-controller, trigger a recompute
+# to cleanup the local datapaths. Otherwise, the test will fail.
+# This is because we don't remove a datapath from
+# the local_datapaths hmap while handling the port binding
+# changes incrementally yet.
+check ovn-appctl inc-engine/recompute
+
OVN_CLEANUP_CONTROLLER([hv1])
# Ensure system resources are cleaned up.
@@ -16810,6 +16817,13 @@ OVN_ROUTE_V6_EQUAL([ovnvrf$vrf], [dnl
blackhole 2001:db8:1001::150 dev lo proto ovn metric 1000 pref medium
blackhole 2001:db8:3001::150 dev lo proto ovn metric 1000 pref medium])
+# Before cleanup of hv1 ovn-controller, trigger a recompute
+# to cleanup the local datapaths. Otherwise, the test will fail.
+# This is because we don't remove a datapath from
+# the local_datapaths hmap while handling the port binding
+# changes incrementally yet.
+check ovn-appctl inc-engine/recompute
+
OVN_CLEANUP_CONTROLLER([hv1])
# Ensure system resources are cleaned up.
@@ -17013,6 +17027,13 @@ blackhole 10.42.20.11 proto ovn metric 100
blackhole 172.16.1.10 proto ovn metric 1000
blackhole 172.16.1.11 proto ovn metric 1000])
+# Before cleanup of hv1 ovn-controller, trigger a recompute
+# to cleanup the local datapaths. Otherwise, the test will fail.
+# This is because we don't remove a datapath from
+# the local_datapaths hmap while handling the port binding
+# changes incrementally yet.
+check ovn-appctl inc-engine/recompute
+
# Skip ls-join in flows comparison between I+P and recompute, because R2 has
multiple DGPs.
# This causes the following flows in sb
# table=xx(ls_in_l2_lkup ), priority=80 , match=(flags[1] == 0 && arp.op == 1 &&
arp.tpa == 10.42.10.10), action=(outport = "lsp-join-to-r2"; output;)
@@ -17220,6 +17241,13 @@ blackhole 2001:db8:1004::150 dev lo proto ovn metric
1000 pref medium
blackhole 2001:db8:1004::151 dev lo proto ovn metric 1000 pref medium
blackhole 2001:db8:1005::150 dev lo proto ovn metric 100 pref medium])
+# Before cleanup of hv1 ovn-controller, trigger a recompute
+# to cleanup the local datapaths. Otherwise, the test will fail.
+# This is because we don't remove a datapath from
+# the local_datapaths hmap while handling the port binding
+# changes incrementally yet.
+check ovn-appctl inc-engine/recompute
+
OVN_CLEANUP_CONTROLLER([hv1], [], [], [ls-join])
# Ensure system resources are cleaned up.
AT_CHECK([ip link | grep -q ovnvrf1003:.*UP], [1])
