On Sat, Apr 12, 2025 at 8:46 AM <num...@ovn.org> wrote:
>
> 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>
> Acked-by: Mark Michelson <mmich...@redhat.com>
> Co-authored-by: Xavier Simonart <xsimo...@redhat.com>
> Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
> Signed-off-by: Numan Siddique <num...@ovn.org>
Thanks Numan.
Acked-by: Han Zhou <hz...@ovn.org>
> ---
> controller/binding.c | 35 ++--
> controller/local_data.c | 79 +++++----
> controller/local_data.h | 6 +-
> tests/ovn.at | 375 ++++++++++++++++++++++++++++++++++++++++
> tests/system-ovn.at | 28 +++
> 5 files changed, 475 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..e256de5fa4 100644
> --- a/controller/local_data.c
> +++ b/controller/local_data.c
> @@ -130,44 +130,68 @@ 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.
> + *
> + * Note that if 'pb' belongs to a logical switch and 'peer' to a
> + * logical router datapath and if 'peer' has a chassis-redirect port,
> + * then we add the 'peer' to the local datapaths only if the
> + * chassis-redirect port is local.
> + * */
> 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 pb 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 (cr_peer && datapath_is_switch(pb->datapath) &&
> + !datapath_is_switch(peer->datapath)) {
> + /* pb belongs to logical switch and peer 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 +199,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 +225,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 +234,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 +645,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])
> --
> 2.49.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
