On Mon, Apr 14, 2025 at 1:00 AM Han Zhou <hz...@ovn.org> wrote:
>
>
>
> 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>
Thank you Han, Mark, Dumitru and Xavier.
I applied this patch series to main.
After a week, I'm planning to backport this series to branch-25.03,
Please let me know if there are any objections.
Numan
>
> > ---
> > 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
