Limit the number of claim for virtual ports to 1/s with backoff period of 10 seconds. This should help with cluster stabilization during fail-overs. It can happen that during fail-over there will be a lot of attempts to claim the virtual ports. With the backoff we can ensure that the cluster had enough time to try to stabilize the fail-overs.
Reported-at: https://issues.redhat.com/browse/FDP-443 Signed-off-by: Ales Musil <amu...@redhat.com> --- controller/pinctrl.c | 59 ++++++++++++++++++++---- northd/northd.c | 15 ++++++ tests/ovn.at | 106 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 9 deletions(-) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 436e5d487..a269e5375 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -7336,6 +7336,9 @@ pinctrl_find_put_vport_binding(uint32_t dp_key, uint32_t vport_key, return NULL; } +#define VPORT_BACKOFF_INTERVAL 1000ll +#define VPORT_MAX_POSTPONE 10000ll + static void run_put_vport_binding(struct ovsdb_idl_txn *ovnsb_idl_txn OVS_UNUSED, struct ovsdb_idl_index *sbrec_datapath_binding_by_key, @@ -7356,18 +7359,56 @@ run_put_vport_binding(struct ovsdb_idl_txn *ovnsb_idl_txn OVS_UNUSED, } /* pinctrl module updates the port binding only for type 'virtual'. */ - if (!strcmp(pb->type, "virtual")) { - const struct sbrec_port_binding *parent = lport_lookup_by_key( + if (strcmp(pb->type, "virtual")) { + return; + } + + const struct sbrec_port_binding *parent = lport_lookup_by_key( sbrec_datapath_binding_by_key, sbrec_port_binding_by_key, vpb->dp_key, vpb->vport_parent_key); - if (parent) { - VLOG_INFO("Claiming virtual lport %s for this chassis " - "with the virtual parent %s", - pb->logical_port, parent->logical_port); - sbrec_port_binding_set_chassis(pb, chassis); - sbrec_port_binding_set_virtual_parent(pb, parent->logical_port); - } + if (!parent) { + return; } + + if (pb->chassis == chassis && !strcmp(pb->virtual_parent, + parent->logical_port)) { + VLOG_DBG("Virtual port %s is already claimed by this chassis.", + pb->logical_port); + return; + } + + long long timewall_now = time_wall_msec(); + long long last_claim = ovn_smap_get_llong(&pb->options, + "vport-last-claim", 0); + long long backoff = ovn_smap_get_llong(&pb->options, "vport-backoff", 0); + + struct smap options; + smap_clone(&options, &pb->options); + + long long next_backoff; + if (timewall_now >= last_claim + backoff) { + sbrec_port_binding_set_chassis(pb, chassis); + sbrec_port_binding_set_virtual_parent(pb, parent->logical_port); + + next_backoff = VPORT_BACKOFF_INTERVAL; + smap_remove(&options, "vport-last-claim"); + smap_add_format(&options, "vport-last-claim", "%lld", timewall_now); + + VLOG_INFO("Claiming virtual lport %s for this chassis " + "with the virtual parent %s", + pb->logical_port, parent->logical_port); + } else { + next_backoff = MIN(backoff + VPORT_BACKOFF_INTERVAL, + VPORT_MAX_POSTPONE); + VLOG_INFO("Postponing virtual lport %s claim by %lld ms", + pb->logical_port, next_backoff); + } + + smap_remove(&options, "vport-backoff"); + smap_add_format(&options, "vport-backoff", "%lld", next_backoff); + + sbrec_port_binding_set_options(pb, &options); + smap_destroy(&options); } /* Called by pinctrl_run(). Runs with in the main ovn-controller diff --git a/northd/northd.c b/northd/northd.c index 762e4e057..9bca4dcb5 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3332,6 +3332,21 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn, } } + /* Preserve virtual port options. */ + if (!strcmp(op->nbsp->type, "virtual")) { + const char *last_claim = smap_get(&op->sb->options, + "vport-last-claim"); + if (last_claim) { + smap_add(&options, "vport-last-claim", last_claim); + } + + const char *backoff = smap_get(&op->sb->options, + "vport-backoff"); + if (backoff) { + smap_add(&options, "vport-backoff", backoff); + } + } + if (lsp_is_remote(op->nbsp) || op->sb->requested_additional_chassis) { /* ovn-northd is supposed to set port_binding for remote ports diff --git a/tests/ovn.at b/tests/ovn.at index bbaa653a8..6c4a1db7d 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -23023,6 +23023,8 @@ check_virtual_offlows_not_present hv1 # hv2 should not have the flow for ACL. check_virtual_offlows_not_present hv2 +# Remove the backoff period, to not prolong the test. +ovn-sbctl remove Port_Binding sw0-vir options vport-backoff # From sw0-p0 resend GARP for 10.0.0.10. hv1 should reclaim sw0-vir # and sw0-p1 should be its virtual_parent. send_garp hv1 hv1-vif1 1 $(hex_to_mac $eth_src) $(hex_to_mac $eth_dst) $spa $tpa @@ -23067,6 +23069,8 @@ check_virtual_offlows_not_present hv1 # hv2 should not have the above flows. check_virtual_offlows_not_present hv2 +# Wait for the backoff period to pass. +sleep 1.2 # From sw0-p0 send GARP for 10.0.0.10. hv1 should claim sw0-vir # and sw0-p1 should be its virtual_parent. eth_src="50:54:00:00:00:03" @@ -23087,6 +23091,8 @@ check_virtual_offlows_present hv1 # hv2 should not have the above flows. check_virtual_offlows_not_present hv2 +# Remove the backoff period, to not prolong the test. +ovn-sbctl remove Port_Binding sw0-vir options vport-backoff # From sw0-p3 send GARP for 10.0.0.10. hv1 should claim sw0-vir # and sw0-p3 should be its virtual_parent. eth_src="50:54:00:00:00:05" @@ -23115,6 +23121,8 @@ check_virtual_offlows_present hv1 # hv2 should not have the above flows. check_virtual_offlows_not_present hv2 +# Remove the backoff period, to not prolong the test. +ovn-sbctl remove Port_Binding sw0-vir options vport-backoff # send the garp from sw0-p2 (in hv2). hv2 should claim sw0-vir # and sw0-p2 shpuld be its virtual_parent. eth_src="50:54:00:00:00:04" @@ -23142,6 +23150,8 @@ check_virtual_offlows_present hv2 # hv1 should not have the above flows. check_virtual_offlows_not_present hv1 +# Remove the backoff period, to not prolong the test. +ovn-sbctl remove Port_Binding sw0-vir options vport-backoff # Now send arp reply from sw0-p1. hv1 should claim sw0-vir # and sw0-p1 shpuld be its virtual_parent. eth_src=505400000003 @@ -42661,3 +42671,99 @@ echo $expected > expected OVN_CHECK_PACKETS([hv1/server-tx.pcap], [expected]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([virtual port claim postpone]) +AT_KEYWORDS([virtual ports]) +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 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap + +sim_add hv2 +as hv2 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.2 +check ovs-vsctl -- add-port br-int hv2-vif1 -- \ + set interface hv2-vif1 external-ids:iface-id=sw0-p2 \ + options:tx_pcap=hv2/vif1-tx.pcap \ + options:rxq_pcap=hv2/vif1-rx.pcap + +sim_add hv3 +as hv3 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.3 +check ovs-vsctl -- add-port br-int hv3-vif1 -- \ + set interface hv3-vif1 external-ids:iface-id=sw0-p3 \ + options:tx_pcap=hv3/vif1-tx.pcap \ + options:rxq_pcap=hv3/vif1-rx.pcap + +check ovn-nbctl ls-add sw0 + +check ovn-nbctl lsp-add sw0 sw0-p1 +check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:01 10.0.0.1" +check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:01 10.0.0.1 10.0.0.10" + +check ovn-nbctl lsp-add sw0 sw0-p2 +check ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:02 10.0.0.2" +check ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:02 10.0.0.2 10.0.0.10" + +check ovn-nbctl lsp-add sw0 sw0-p3 +check ovn-nbctl lsp-set-addresses sw0-p3 "50:54:00:00:00:03 10.0.0.3" +check ovn-nbctl lsp-set-port-security sw0-p3 "50:54:00:00:00:03 10.0.0.3 10.0.0.10" + +check ovn-nbctl lsp-add sw0 sw0-vir +check ovn-nbctl lsp-set-addresses sw0-vir "50:54:00:00:00:10 10.0.0.10" +check ovn-nbctl lsp-set-type sw0-vir virtual +check ovn-nbctl set logical_switch_port sw0-vir options:virtual-ip=10.0.0.10 +check ovn-nbctl set logical_switch_port sw0-vir options:virtual-parents="sw0-p1,sw0-p2,sw0-p3" + +wait_for_ports_up +check ovn-nbctl --wait=hv sync + +# Claim virtual port on hv1. +send_garp hv1 hv1-vif1 1 "50:54:00:00:00:01" "ff:ff:ff:ff:ff:ff" 10.0.0.10 10.0.0.10 +wait_row_count Port_Binding 1 logical_port=sw0-vir chassis=$(fetch_column Chassis _uuid name=hv1) +wait_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p1 +wait_for_ports_up sw0-vir + +# Check that the backoff interval was set. +AT_CHECK([fetch_column Port_Binding options logical_port=sw0-vir | grep -q "vport-last-claim"]) +check_row_count Port_Binding 1 logical_port=sw0-vir options:vport-backoff=1000 + +# Pass the backoff timer +sleep 1.2 + +# Try to claim on all chassis in quick succession. +for i in $(seq 1 10); do + send_garp hv3 hv3-vif1 1 "50:54:00:00:00:03" "ff:ff:ff:ff:ff:ff" 10.0.0.10 10.0.0.10 + send_garp hv2 hv2-vif1 1 "50:54:00:00:00:02" "ff:ff:ff:ff:ff:ff" 10.0.0.10 10.0.0.10 + send_garp hv1 hv1-vif1 1 "50:54:00:00:00:01" "ff:ff:ff:ff:ff:ff" 10.0.0.10 10.0.0.10 + sleep 0.2 +done + +wait_row_count Port_Binding 1 logical_port=sw0-vir chassis=$(fetch_column Chassis _uuid name=hv3) +wait_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p3 +wait_for_ports_up sw0-vir + +# Check that the backoff interval reached the maximum. +AT_CHECK([fetch_column Port_Binding options logical_port=sw0-vir | grep -q "vport-last-claim"]) +check_row_count Port_Binding 1 logical_port=sw0-vir options:vport-backoff=10000 + +AT_CHECK([grep -q "Postponing virtual lport sw0-vir" hv1/ovn-controller.log ]) +AT_CHECK([grep -q "Postponing virtual lport sw0-vir" hv2/ovn-controller.log ]) + +# hv3 has the claim already, it doesn't bump the backoff interval. +AT_CHECK([grep -q "Postponing virtual lport sw0-vir" hv3/ovn-controller.log], [1]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) -- 2.49.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
