On 5/13/22 15:41, Xavier Simonart wrote: > Hi Dumitru > > Thanks for the review. > I'll go through your suggestions and submit a v2. > > Just commented here on your question - see embedded. > Thanks > Xavier > > On Fri, May 13, 2022 at 2:46 PM Dumitru Ceara <[email protected]> wrote: > >> Hi Xavier, >> >> On 5/12/22 11:04, Xavier Simonart wrote: >>> When VIF ports are claimed on a chassis, SBDB Port_Binding table is >> updated. >>> If the SBDB IDL is still is read-only ("in transaction") when such a >> update >>> is required, the update is not possible and recompute is triggered >> through >>> I+P failure. >>> >>> This situation can happen: >>> - after updating Port_Binding->chassis to SBDB for one port, in a >> following >>> iteration, ovn-controller handles Interface:external_ids:ovn-installed >>> (for the same port) while SBDB is still read-only. >>> - after updating Port_Binding->chassis to SBDB for one port, in a >> following >>> iteration, ovn-controller updates Port_Binding->chassis for another >> port, >>> while SBDB is still read-only. >>> >>> This patch prevent the recompute, by having the if-status module >>> updating the Port_Binding chassis (if needed), when possible. >>> This does not delay Port_Binding chassis update compared to before this >>> patch: Port_Binding chassis will be updated as soon as SBDB is again >>> writable, as it was the case, through a recompute, before this patch. >> >> Just to confirm, with your patch the Port_Binding chassis will be >> updated when the SBDB is again writable and will *not* cause a >> recompute, right? >> >> Yes, confirmed. I can reformulate as: > > This patch prevents the recompute, by having the if-status module > updating the Port_Binding chassis (if needed), when possible. > This does not delay Port_Binding chassis update compared to before this > patch. > - With the patch, Port_Binding chassis will be updated as soon as SBDB is > again > writable, without recompute. > - Without the patch, Port_Binding chassis will be updated as soon as SBDB > is again > writable, through a recompute. >
Thanks for the confirmation! > As a slide note, the patch only prevents the recomputes when up is set > through notification (notify_up=true). > I think we can later fix the recomputes in the (notify_up=false) case if > needed, but I thought it was better to see if > 1) the approach used by the patch is accepted IMO the approach is fine. > 2) the use case notify_up=false is really needed Container and virtual ports are used by OpenStack/Neutron so we might want to try to fix this case too. > In that case, additional states would be needed in if-status state machine. > Why can't we reuse the state machine by adding a flag to the ovs_iface to mark that this is a child interface? In which case the transition from OIF_INSTALL_FLOWS to OIF_MARK_UP would happen only if the parent interface is already in OIF_MARK_UP. OIF_CLAIMED -> OIF_INSTALL_FLOWS -> OIF_MARK_UP -> OIF_INSTALLED I'm OK to address this second case (notify_up=false) as a follow up though. Thanks, Dumitru >> >>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2059253 >>> Signed-off-by: Xavier Simonart <[email protected]> >>> --- >>> controller/binding.c | 124 ++++++++++++++++++++++++++---------- >>> controller/binding.h | 9 ++- >>> controller/if-status.c | 31 +++++++-- >>> controller/if-status.h | 6 +- >>> controller/ovn-controller.c | 6 +- >>> tests/ovn.at | 55 +++++++++++++++- >>> 6 files changed, 184 insertions(+), 47 deletions(-) >>> >>> diff --git a/controller/binding.c b/controller/binding.c >>> index e5ba56b25..d917d8775 100644 >>> --- a/controller/binding.c >>> +++ b/controller/binding.c >>> @@ -657,11 +657,15 @@ local_binding_get_lport_ofport(const struct shash >> *local_bindings, >>> } >>> >>> bool >>> -local_binding_is_up(struct shash *local_bindings, const char *pb_name) >>> +local_binding_is_up(struct shash *local_bindings, const char *pb_name, >>> + const struct sbrec_chassis *chassis_rec) >>> { >>> struct local_binding *lbinding = >>> local_binding_find(local_bindings, pb_name); >>> struct binding_lport *b_lport = >> local_binding_get_primary_lport(lbinding); >>> + if (b_lport && b_lport->pb->chassis != chassis_rec) { >>> + return false; >>> + } >>> if (lbinding && b_lport && lbinding->iface) { >>> if (b_lport->pb->n_up && !b_lport->pb->up[0]) { >>> return false; >>> @@ -673,13 +677,23 @@ local_binding_is_up(struct shash *local_bindings, >> const char *pb_name) >>> } >>> >>> bool >>> -local_binding_is_down(struct shash *local_bindings, const char *pb_name) >>> +local_binding_is_down(struct shash *local_bindings, const char *pb_name, >>> + const struct sbrec_chassis *chassis_rec) >>> { >>> struct local_binding *lbinding = >>> local_binding_find(local_bindings, pb_name); >>> >>> struct binding_lport *b_lport = >> local_binding_get_primary_lport(lbinding); >>> >>> + if (b_lport) { >>> + if (b_lport->pb->chassis == chassis_rec) { >>> + return false; >>> + } else if (b_lport->pb->chassis) { >>> + VLOG_DBG("lport %s already claimed by other chassis", >>> + b_lport->pb->logical_port); >>> + } >>> + } >>> + >>> if (!lbinding) { >>> return true; >>> } >>> @@ -894,6 +908,48 @@ get_lport_type_str(enum en_lport_type lport_type) >>> OVS_NOT_REACHED(); >>> } >>> >>> +static void set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb, >> >> Nit: this should be: >> >> static void >> set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb, >> >>> + const struct sbrec_chassis *chassis_rec) >>> +{ >>> + if (pb->chassis != chassis_rec) { >>> + if (pb->chassis) { >>> + VLOG_INFO("Changing chassis for lport %s from %s to %s.", >>> + pb->logical_port, pb->chassis->name, >>> + chassis_rec->name); >> >> Nit: indentation. >> >>> + } else { >>> + VLOG_INFO("Claiming lport %s for this chassis.", >> pb->logical_port); >>> + } >>> + for (int i = 0; i < pb->n_mac; i++) { >>> + VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); >>> + } >>> + sbrec_port_binding_set_chassis(pb, chassis_rec); >>> + } >>> +} >>> + >>> +static void clear_pb_chassis_in_sbrec(const struct sbrec_port_binding >> *pb) >> >> Here too, the function name should start on a new line. >> >>> +{ >>> + if (pb->chassis) { >>> + sbrec_port_binding_set_chassis(pb, NULL); >>> + } >>> +} >>> + >>> +void >>> +local_binding_set_pb(struct shash *local_bindings, const char *pb_name, >>> + const struct sbrec_chassis *chassis_rec) >> >> Nit: indentation. >> >>> +{ >>> + struct local_binding *lbinding = >>> + local_binding_find(local_bindings, pb_name); >>> + struct binding_lport *b_lport = >> local_binding_get_primary_lport(lbinding); >>> + >>> + if (b_lport) { >>> + if (chassis_rec) { >>> + set_pb_chassis_in_sbrec(b_lport->pb, chassis_rec); >>> + } else { >>> + clear_pb_chassis_in_sbrec(b_lport->pb); >>> + } >>> + } >>> +} >>> + >>> /* For newly claimed ports, if 'notify_up' is 'false': >>> * - set the 'pb.up' field to true if 'pb' has no 'parent_pb'. >>> * - set the 'pb.up' field to true if 'parent_pb.up' is 'true' (e.g., >> for >>> @@ -906,23 +962,37 @@ get_lport_type_str(enum en_lport_type lport_type) >>> * it's explicitly set to 'false'. >>> * This is to ensure compatibility with older versions of ovn-northd. >>> */ >>> -static void >>> +static bool >>> claimed_lport_set_up(const struct sbrec_port_binding *pb, >>> const struct sbrec_port_binding *parent_pb, >>> const struct sbrec_chassis *chassis_rec, >>> - bool notify_up, struct if_status_mgr *if_mgr) >>> + bool notify_up, struct if_status_mgr *if_mgr, >>> + bool sb_readonly) >>> { >>> + >> >> Extra line not needed. >> >>> if (!notify_up) { >>> bool up = true; >>> if (!parent_pb || (parent_pb->n_up && parent_pb->up[0])) { >>> - sbrec_port_binding_set_up(pb, &up, 1); >>> + if (!sb_readonly) { >>> + sbrec_port_binding_set_up(pb, &up, 1); >>> + } else if (pb->n_up && !pb->up[0]) { >>> + return false; >>> + } >>> } >>> - return; >>> + if (sb_readonly && (pb->chassis != chassis_rec)) { >>> + /* Handle the cases where if_status_mgr does not claim the >>> + * interface.In those cases, if we do not set pb->chassis >> in sb >>> + * now (as readonly), we might not do it later ... >>> + */ >>> + return false; >>> + } >>> + return true; >>> } >>> >>> if (pb->chassis != chassis_rec || (pb->n_up && !pb->up[0])) { >>> if_status_mgr_claim_iface(if_mgr, pb->logical_port); >>> } >>> + return true; >>> } >>> >>> /* Returns false if lport is not claimed due to 'sb_readonly'. >>> @@ -937,27 +1007,15 @@ claim_lport(const struct sbrec_port_binding *pb, >>> struct hmap *tracked_datapaths, >>> struct if_status_mgr *if_mgr) >>> { >>> - if (!sb_readonly) { >>> - claimed_lport_set_up(pb, parent_pb, chassis_rec, notify_up, >> if_mgr); >>> - } >>> >>> + if (!claimed_lport_set_up(pb, parent_pb, chassis_rec, notify_up, >>> + if_mgr, sb_readonly)) { >>> + return false; >>> + } >>> if (pb->chassis != chassis_rec) { >>> - if (sb_readonly) { >>> - return false; >>> - } >>> - >>> - if (pb->chassis) { >>> - VLOG_INFO("Changing chassis for lport %s from %s to %s.", >>> - pb->logical_port, pb->chassis->name, >>> - chassis_rec->name); >>> - } else { >>> - VLOG_INFO("Claiming lport %s for this chassis.", >> pb->logical_port); >>> + if (!sb_readonly) { >>> + set_pb_chassis_in_sbrec(pb, chassis_rec); >>> } >>> - for (int i = 0; i < pb->n_mac; i++) { >>> - VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); >>> - } >>> - >>> - sbrec_port_binding_set_chassis(pb, chassis_rec); >>> >>> if (tracked_datapaths) { >>> update_lport_tracking(pb, tracked_datapaths, true); >>> @@ -974,7 +1032,6 @@ claim_lport(const struct sbrec_port_binding *pb, >>> } >>> sbrec_port_binding_set_encap(pb, encap_rec); >>> } >>> - >> >> Unrelated? >> >> > >>> return true; >>> } >>> >>> @@ -986,7 +1043,8 @@ claim_lport(const struct sbrec_port_binding *pb, >>> * Caller should make sure that this is the case. >>> */ >>> static bool >>> -release_lport_(const struct sbrec_port_binding *pb, bool sb_readonly) >>> +release_lport_(const struct sbrec_port_binding *pb, bool sb_readonly, >>> + struct if_status_mgr *if_mgr) >>> { >>> if (pb->encap) { >>> if (sb_readonly) { >>> @@ -995,11 +1053,13 @@ release_lport_(const struct sbrec_port_binding >> *pb, bool sb_readonly) >>> sbrec_port_binding_set_encap(pb, NULL); >>> } >>> >>> + /* if sb readonly, pb->chassis unset through if-status if present */ >>> if (pb->chassis) { >>> - if (sb_readonly) { >>> + if (!sb_readonly) { >>> + sbrec_port_binding_set_chassis(pb, NULL); >>> + } else if (!if_status_mgr_iface_is_present(if_mgr, >> pb->logical_port)) { >>> return false; >>> } >>> - sbrec_port_binding_set_chassis(pb, NULL); >>> } >>> >>> if (pb->virtual_parent) { >>> @@ -1009,7 +1069,8 @@ release_lport_(const struct sbrec_port_binding >> *pb, bool sb_readonly) >>> sbrec_port_binding_set_virtual_parent(pb, NULL); >>> } >>> >>> - VLOG_INFO("Releasing lport %s from this chassis.", >> pb->logical_port); >>> + VLOG_INFO("Releasing lport %s from this chassis (sb_readonly=%d)", >>> + pb->logical_port, sb_readonly); >>> return true; >>> } >>> >>> @@ -1017,7 +1078,7 @@ static bool >>> release_lport(const struct sbrec_port_binding *pb, bool sb_readonly, >>> struct hmap *tracked_datapaths, struct if_status_mgr >> *if_mgr) >>> { >>> - if (!release_lport_(pb, sb_readonly)) { >>> + if (!release_lport_(pb, sb_readonly, if_mgr)) { >>> return false; >>> } >>> >>> @@ -1342,10 +1403,9 @@ consider_localport(const struct >> sbrec_port_binding *pb, >>> /* If the port binding is claimed, then release it as localport is >> claimed >>> * by any ovn-controller. */ >>> if (pb->chassis == b_ctx_in->chassis_rec) { >>> - if (!release_lport_(pb, !b_ctx_in->ovnsb_idl_txn)) { >>> + if (!release_lport_(pb, !b_ctx_in->ovnsb_idl_txn, >> b_ctx_out->if_mgr)) { >>> return false; >>> } >>> - >> >> Unrelated? >> >>> remove_related_lport(pb, b_ctx_out); >>> } >>> >>> diff --git a/controller/binding.h b/controller/binding.h >>> index 430a8d9b1..45202a321 100644 >>> --- a/controller/binding.h >>> +++ b/controller/binding.h >>> @@ -151,14 +151,17 @@ const struct sbrec_port_binding >> *local_binding_get_primary_pb( >>> ofp_port_t local_binding_get_lport_ofport(const struct shash >> *local_bindings, >>> const char *pb_name); >>> >>> -bool local_binding_is_up(struct shash *local_bindings, const char >> *pb_name); >>> -bool local_binding_is_down(struct shash *local_bindings, const char >> *pb_name); >>> +bool local_binding_is_up(struct shash *local_bindings, const char >> *pb_name, >>> + const struct sbrec_chassis *); >>> +bool local_binding_is_down(struct shash *local_bindings, const char >> *pb_name, >>> + const struct sbrec_chassis *); >>> void local_binding_set_up(struct shash *local_bindings, const char >> *pb_name, >>> const char *ts_now_str, bool sb_readonly, >>> bool ovs_readonly); >>> void local_binding_set_down(struct shash *local_bindings, const char >> *pb_name, >>> bool sb_readonly, bool ovs_readonly); >>> - >>> +void local_binding_set_pb(struct shash *local_bindings, const char >> *pb_name, >>> + const struct sbrec_chassis *chassis_rec); >>> void binding_register_ovs_idl(struct ovsdb_idl *); >>> void binding_run(struct binding_ctx_in *, struct binding_ctx_out *); >>> bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, >>> diff --git a/controller/if-status.c b/controller/if-status.c >>> index dbdf8b66f..e36df22a0 100644 >>> --- a/controller/if-status.c >>> +++ b/controller/if-status.c >>> @@ -115,6 +115,7 @@ static void ovs_iface_set_state(struct if_status_mgr >> *, struct ovs_iface *, >>> >>> static void if_status_mgr_update_bindings( >>> struct if_status_mgr *mgr, struct local_binding_data *binding_data, >>> + const struct sbrec_chassis *chassis_rec, >>> bool sb_readonly, bool ovs_readonly); >>> >>> struct if_status_mgr * >>> @@ -181,6 +182,13 @@ if_status_mgr_claim_iface(struct if_status_mgr >> *mgr, const char *iface_id) >>> } >>> } >>> >>> +bool >>> +if_status_mgr_iface_is_present(struct if_status_mgr *mgr, const char >> *iface_id) >>> +{ >>> + struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id); >>> + return (!!iface); >>> +} >>> + >>> void >>> if_status_mgr_release_iface(struct if_status_mgr *mgr, const char >> *iface_id) >>> { >>> @@ -247,7 +255,8 @@ if_status_mgr_delete_iface(struct if_status_mgr >> *mgr, const char *iface_id) >>> >>> void >>> if_status_mgr_update(struct if_status_mgr *mgr, >>> - struct local_binding_data *binding_data) >>> + struct local_binding_data *binding_data, >>> + const struct sbrec_chassis *chassis_rec) >>> { >>> if (!binding_data) { >>> return; >>> @@ -262,7 +271,7 @@ if_status_mgr_update(struct if_status_mgr *mgr, >>> HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_MARK_UP]) { >>> struct ovs_iface *iface = node->data; >>> >>> - if (local_binding_is_up(bindings, iface->id)) { >>> + if (local_binding_is_up(bindings, iface->id, chassis_rec)) { >>> ovs_iface_set_state(mgr, iface, OIF_INSTALLED); >>> } >>> } >>> @@ -273,7 +282,7 @@ if_status_mgr_update(struct if_status_mgr *mgr, >>> HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_MARK_DOWN]) { >>> struct ovs_iface *iface = node->data; >>> >>> - if (local_binding_is_down(bindings, iface->id)) { >>> + if (local_binding_is_down(bindings, iface->id, chassis_rec)) { >>> ovs_iface_destroy(mgr, iface); >>> } >>> } >>> @@ -306,6 +315,7 @@ if_status_mgr_update(struct if_status_mgr *mgr, >>> void >>> if_status_mgr_run(struct if_status_mgr *mgr, >>> struct local_binding_data *binding_data, >>> + const struct sbrec_chassis *chassis_rec, >>> bool sb_readonly, bool ovs_readonly) >>> { >>> struct ofctrl_acked_seqnos *acked_seqnos = >>> @@ -328,8 +338,8 @@ if_status_mgr_run(struct if_status_mgr *mgr, >>> ofctrl_acked_seqnos_destroy(acked_seqnos); >>> >>> /* Update binding states. */ >>> - if_status_mgr_update_bindings(mgr, binding_data, sb_readonly, >>> - ovs_readonly); >>> + if_status_mgr_update_bindings(mgr, binding_data, chassis_rec, >>> + sb_readonly, ovs_readonly); >>> } >>> >>> static void >>> @@ -390,6 +400,7 @@ ovs_iface_set_state(struct if_status_mgr *mgr, >> struct ovs_iface *iface, >>> static void >>> if_status_mgr_update_bindings(struct if_status_mgr *mgr, >>> struct local_binding_data *binding_data, >>> + const struct sbrec_chassis *chassis_rec, >>> bool sb_readonly, bool ovs_readonly) >>> { >>> if (!binding_data) { >>> @@ -404,6 +415,9 @@ if_status_mgr_update_bindings(struct if_status_mgr >> *mgr, >>> */ >>> HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_INSTALL_FLOWS]) { >>> struct ovs_iface *iface = node->data; >>> + if (!sb_readonly) { >>> + local_binding_set_pb(bindings, iface->id, chassis_rec); >>> + } >>> >>> local_binding_set_down(bindings, iface->id, sb_readonly, >> ovs_readonly); >>> } >>> @@ -415,7 +429,9 @@ if_status_mgr_update_bindings(struct if_status_mgr >> *mgr, >>> char *ts_now_str = xasprintf("%lld", time_wall_msec()); >>> HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_MARK_UP]) { >>> struct ovs_iface *iface = node->data; >>> - >>> + if (!sb_readonly) { >>> + local_binding_set_pb(bindings, iface->id, chassis_rec); >>> + } >>> local_binding_set_up(bindings, iface->id, ts_now_str, >>> sb_readonly, ovs_readonly); >>> } >>> @@ -426,6 +442,9 @@ if_status_mgr_update_bindings(struct if_status_mgr >> *mgr, >>> */ >>> HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_MARK_DOWN]) { >>> struct ovs_iface *iface = node->data; >>> + if (!sb_readonly) { >>> + local_binding_set_pb(bindings, iface->id, NULL); >>> + } >>> >>> local_binding_set_down(bindings, iface->id, sb_readonly, >> ovs_readonly); >>> } >>> diff --git a/controller/if-status.h b/controller/if-status.h >>> index ff4aa760e..2c55eb18e 100644 >>> --- a/controller/if-status.h >>> +++ b/controller/if-status.h >>> @@ -31,10 +31,14 @@ void if_status_mgr_claim_iface(struct if_status_mgr >> *, const char *iface_id); >>> void if_status_mgr_release_iface(struct if_status_mgr *, const char >> *iface_id); >>> void if_status_mgr_delete_iface(struct if_status_mgr *, const char >> *iface_id); >>> >>> -void if_status_mgr_update(struct if_status_mgr *, struct >> local_binding_data *); >>> +void if_status_mgr_update(struct if_status_mgr *, struct >> local_binding_data *, >>> + const struct sbrec_chassis *chassis); >>> void if_status_mgr_run(struct if_status_mgr *mgr, struct >> local_binding_data *, >>> + const struct sbrec_chassis *chassis, >>> bool sb_readonly, bool ovs_readonly); >>> void if_status_mgr_get_memory_usage(struct if_status_mgr *mgr, >>> struct simap *usage); >>> +bool if_status_mgr_iface_is_present(struct if_status_mgr *mgr, >>> + const char *iface_id); >>> >>> # endif /* controller/if-status.h */ >>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >>> index 5a6274eb2..994aebdfb 100644 >>> --- a/controller/ovn-controller.c >>> +++ b/controller/ovn-controller.c >>> @@ -3912,7 +3912,7 @@ main(int argc, char *argv[]) >>> runtime_data ? &runtime_data->lbinding_data : >> NULL; >>> stopwatch_start(IF_STATUS_MGR_UPDATE_STOPWATCH_NAME, >>> time_msec()); >>> - if_status_mgr_update(if_mgr, binding_data); >>> + if_status_mgr_update(if_mgr, binding_data, chassis); >>> stopwatch_stop(IF_STATUS_MGR_UPDATE_STOPWATCH_NAME, >>> time_msec()); >>> >>> @@ -3938,8 +3938,8 @@ main(int argc, char *argv[]) >>> time_msec()); >>> stopwatch_start(IF_STATUS_MGR_RUN_STOPWATCH_NAME, >>> time_msec()); >>> - if_status_mgr_run(if_mgr, binding_data, >> !ovnsb_idl_txn, >>> - !ovs_idl_txn); >>> + if_status_mgr_run(if_mgr, binding_data, chassis, >>> + !ovnsb_idl_txn, !ovs_idl_txn); >>> stopwatch_stop(IF_STATUS_MGR_RUN_STOPWATCH_NAME, >>> time_msec()); >>> } >>> diff --git a/tests/ovn.at b/tests/ovn.at >>> index 6a0a169c1..403fbc85f 100644 >>> --- a/tests/ovn.at >>> +++ b/tests/ovn.at >>> @@ -13934,6 +13934,7 @@ ovn_attach n1 br-phys 192.168.0.11 >>> ovs-vsctl -- add-port br-int hv1-vif0 -- \ >>> set Interface hv1-vif0 ofport-request=1 >>> >>> + >> >> Unrelated? >> >>> sim_add hv2 >>> as hv2 >>> ovs-vsctl add-br br-phys >>> @@ -13983,7 +13984,10 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int >> table=65 | grep actions=output:1], >>> echo "verifying that lsp0 binding moves when requested-chassis is >> changed" >>> >>> ovn-nbctl lsp-set-options lsp0 requested-chassis=hv2 >>> -OVS_WAIT_UNTIL([test 1 = $(grep -c "Releasing lport lsp0 from this >> chassis" hv1/ovn-controller.log)]) >>> + >>> +# We might see multiple "Releasing lport ...", when sb is read only >>> +OVS_WAIT_UNTIL([test 1 -le $(grep -c "Releasing lport lsp0 from this >> chassis" hv1/ovn-controller.log)]) >>> + >>> wait_column "$hv2_uuid" Port_Binding chassis logical_port=lsp0 >>> >>> # (6) Chassis hv2 should add flows and hv1 should not. >>> @@ -14029,7 +14033,7 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int >> table=0 | grep in_port=1], [0], [ig >>> AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep >> actions=output:1], [0], [ignore]) >>> >>> check ovn-nbctl --wait=hv lsp-set-options lsp0 >> requested-chassis=non-existant-chassis >>> -OVS_WAIT_UNTIL([test 1 = $(grep -c "Releasing lport lsp0 from this >> chassis" hv1/ovn-controller.log)]) >>> +OVS_WAIT_UNTIL([test 1 -le $(grep -c "Releasing lport lsp0 from this >> chassis" hv1/ovn-controller.log)]) >>> check ovn-nbctl --wait=hv sync >>> wait_column '' Port_Binding chasssi logical_port=lsp0 >>> AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1], >> [1], []) >>> @@ -30595,3 +30599,50 @@ test_mac_binding_flows hv2 >> 00\:00\:00\:00\:10\:10 1 >>> OVN_CLEANUP([hv1], [hv2]) >>> AT_CLEANUP >>> ]) >>> + >>> +OVN_FOR_EACH_NORTHD([ >>> +AT_SETUP([recomputes]) >>> +ovn_start >>> + >>> +# Add chassis >>> +net_add n1 >>> +sim_add hv1 >>> +as hv1 >>> +ovs-vsctl add-br br-phys >>> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys >>> +ovn_attach n1 br-phys 192.168.0.1 >>> + >>> +add_switch_ports() { >>> + for port in $(seq $1 $2); do >>> + logical_switch_port=lsp${port} >>> + check ovn-nbctl lsp-add ls1 $logical_switch_port >>> + check ovn-nbctl lsp-set-addresses $logical_switch_port dynamic >>> + >>> + as hv1 ovs-vsctl \ >>> + -- add-port br-int vif${port} \ >>> + -- set Interface vif${port} >> external_ids:iface-id=$logical_switch_port >>> + done >> >> Don't we need a wait_for_ports_up here to make sure the new ports are >> claimed and processed? >> >>> + >>> + check ovn-nbctl --wait=hv sync >>> + AT_CHECK([test $(ovn-appctl -t ovn-controller coverage/read-counter >> lflow_run) == $lflow_run]) >>> +} >>> + >>> +check ovn-nbctl ls-add ls1 >>> +check ovn-nbctl set Logical_Switch ls1 other_config:subnet=10.1.0.0/16 >>> +check ovn-nbctl set Logical_Switch ls1 >> other_config:exclude_ips=10.1.255.254 >>> + >>> +check ovn-nbctl lr-add lr1 >>> +check ovn-nbctl lsp-add ls1 lsp0 -- set Logical_Switch_Port lsp0 >> type=router options:router-port=lrp0 addresses=dynamic >>> +check ovn-nbctl lrp-add lr1 lrp0 "f0:00:00:01:00:01" 10.1.255.254/16 >>> +check ovn-nbctl lr-nat-add lr1 snat 10.2.0.1 10.1.0.0/16 >>> + >>> +check ovn-nbctl --wait=hv sync >>> +lflow_run=$(ovn-appctl -t ovn-controller coverage/read-counter >> lflow_run) >>> + >>> +add_switch_ports 1 50 >>> +add_switch_ports 51 100 >>> +add_switch_ports 101 150 >>> + >>> +OVN_CLEANUP([hv1]) >>> +AT_CLEANUP >>> +]) >> >> Thanks, >> Dumitru >> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
