On Thu, May 26, 2022 at 12:19 PM Ihar Hrachyshka <[email protected]> wrote: > > When the option is set to a comma separated list of chassis names, OVN > will attempt to bind the port at any number of other locations in > addition to the main chassis. > > This is useful in live migration scenarios where it's important to > prepare the environment for workloads to move to, avoiding costly flow > configuration at the moment of the final port binding location change. > > This may also be useful in port mirroring scenarios. > > Corresponding database fields (pb->additional_chassis, > pb->requested_additional_chassis, pb->additional_encap) are introduced > as part of the patch. These are similar to fields designed to track the > main chassis (->chassis, ->requested_chassis, ->encap). But because we > support any number of additional chassis, these fields are lists. > > Signed-off-by: Ihar Hrachyshka <[email protected]>
There's one minor comment below. With that addressed: Acked-by: Numan Siddique <[email protected]> > --- > NEWS | 1 + > controller/binding.c | 294 ++++++++++++++++++++++++++++++++++++------- > controller/lport.c | 46 ++++--- > controller/lport.h | 11 +- > northd/northd.c | 62 ++++++--- > northd/ovn-northd.c | 4 +- > ovn-nb.xml | 20 ++- > ovn-sb.ovsschema | 17 ++- > ovn-sb.xml | 63 ++++++++-- > tests/ovn.at | 218 ++++++++++++++++++++++++++++++++ > 10 files changed, 632 insertions(+), 104 deletions(-) > > diff --git a/NEWS b/NEWS > index 4294f7d6c..0517bd337 100644 > --- a/NEWS > +++ b/NEWS > @@ -26,6 +26,7 @@ OVN v22.06.0 - XX XXX XXXX > - --ovn-northd-n-threads command line argument in ovn-ctl > - Added support for setting the Next server IP in the DHCP header > using the private DHCP option - 253 in native OVN DHCPv4 responder. > + - Support list of chassis for > Logical_Switch_Port:options:requested-chassis. > > OVN v22.03.0 - 11 Mar 2022 > -------------------------- > diff --git a/controller/binding.c b/controller/binding.c > index 4c1fb4e5f..fc553f4f0 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -917,6 +917,130 @@ claimed_lport_set_up(const struct sbrec_port_binding > *pb, > } > } > > +typedef void (*set_func)(const struct sbrec_port_binding *pb, > + const struct sbrec_encap *); > + > +static bool > +update_port_encap_if_needed(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec, > + const struct ovsrec_interface *iface_rec, > + bool sb_readonly) > +{ > + const struct sbrec_encap *encap_rec = > + sbrec_get_port_encap(chassis_rec, iface_rec); > + if ((encap_rec && pb->encap != encap_rec) || > + (!encap_rec && pb->encap)) { > + if (sb_readonly) { > + return false; > + } > + sbrec_port_binding_set_encap(pb, encap_rec); > + } > + return true; > +} > + > +static void > +append_additional_encap(const struct sbrec_port_binding *pb, > + const struct sbrec_encap *encap) > +{ > + struct sbrec_encap **additional_encap = xmalloc( > + (pb->n_additional_encap + 1) * (sizeof *additional_encap)); > + memcpy(additional_encap, pb->additional_encap, > + pb->n_additional_encap * (sizeof *additional_encap)); > + additional_encap[pb->n_additional_encap] = (struct sbrec_encap *) encap; > + sbrec_port_binding_set_additional_encap( > + pb, additional_encap, pb->n_additional_encap + 1); > + free(additional_encap); > +} > + > +static void > +remove_additional_encap_for_chassis(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec) > +{ > + struct sbrec_encap **additional_encap = xmalloc( > + pb->n_additional_encap * (sizeof *additional_encap)); > + > + int idx = 0; > + for (int i = 0; i < pb->n_additional_encap; i++) { Please use 'size_t' instead of 'int' for 'i' and 'idx'. The type of pb->n_additional_encap is size_t. There are many such occurrences in patch 2, 3 and 4. Please change all of them to 'size_t'. Thanks Numan > + if (!strcmp(pb->additional_encap[i]->chassis_name, > + chassis_rec->name)) { > + continue; > + } > + additional_encap[idx++] = pb->additional_encap[i]; > + } > + sbrec_port_binding_set_additional_encap(pb, additional_encap, idx); > + free(additional_encap); > +} > + > +static bool > +update_port_additional_encap_if_needed( > + const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec, > + const struct ovsrec_interface *iface_rec, > + bool sb_readonly) > +{ > + const struct sbrec_encap *encap_rec = > + sbrec_get_port_encap(chassis_rec, iface_rec); > + if (encap_rec) { > + for (int i = 0; i < pb->n_additional_encap; i++) { > + if (pb->additional_encap[i] == encap_rec) { > + return true; > + } > + } > + if (sb_readonly) { > + return false; > + } > + append_additional_encap(pb, encap_rec); > + } > + return true; > +} > + > +static bool > +is_additional_chassis(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec) > +{ > + for (int i = 0; i < pb->n_additional_chassis; i++) { > + if (pb->additional_chassis[i] == chassis_rec) { > + return true; > + } > + } > + return false; > +} > + > +static void > +append_additional_chassis(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec) > +{ > + struct sbrec_chassis **additional_chassis = xmalloc( > + (pb->n_additional_chassis + 1) * (sizeof *additional_chassis)); > + memcpy(additional_chassis, pb->additional_chassis, > + pb->n_additional_chassis * (sizeof *additional_chassis)); > + additional_chassis[pb->n_additional_chassis] = ( > + (struct sbrec_chassis *) chassis_rec); > + sbrec_port_binding_set_additional_chassis( > + pb, additional_chassis, pb->n_additional_chassis + 1); > + free(additional_chassis); > +} > + > +static void > +remove_additional_chassis(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec) > +{ > + struct sbrec_chassis **additional_chassis = xmalloc( > + (pb->n_additional_chassis - 1) * (sizeof *additional_chassis)); > + int idx = 0; > + for (int i = 0; i < pb->n_additional_chassis; i++) { > + if (pb->additional_chassis[i] == chassis_rec) { > + continue; > + } > + additional_chassis[idx++] = pb->additional_chassis[i]; > + } > + sbrec_port_binding_set_additional_chassis( > + pb, additional_chassis, pb->n_additional_chassis - 1); > + free(additional_chassis); > + > + remove_additional_encap_for_chassis(pb, chassis_rec); > +} > + > /* Returns false if lport is not claimed due to 'sb_readonly'. > * Returns true otherwise. > */ > @@ -933,40 +1057,65 @@ claim_lport(const struct sbrec_port_binding *pb, > claimed_lport_set_up(pb, parent_pb, chassis_rec, notify_up, if_mgr); > } > > - if (pb->chassis != chassis_rec) { > - if (sb_readonly) { > - return false; > - } > + enum can_bind can_bind = lport_can_bind_on_this_chassis(chassis_rec, pb); > + bool update_tracked = 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); > - } > - for (int i = 0; i < pb->n_mac; i++) { > - VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); > + if (can_bind == CAN_BIND_AS_MAIN) { > + 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); > + } > + 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 (is_additional_chassis(pb, chassis_rec)) { > + remove_additional_chassis(pb, chassis_rec); > + } > + update_tracked = true; > } > + } else if (can_bind == CAN_BIND_AS_ADDITIONAL) { > + if (!is_additional_chassis(pb, chassis_rec)) { > + if (sb_readonly) { > + return false; > + } > > - sbrec_port_binding_set_chassis(pb, chassis_rec); > + VLOG_INFO("Claiming lport %s for this additional chassis.", > + pb->logical_port); > + for (int i = 0; i < pb->n_mac; i++) { > + VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); > + } > > - if (tracked_datapaths) { > - update_lport_tracking(pb, tracked_datapaths, true); > + append_additional_chassis(pb, chassis_rec); > + if (pb->chassis == chassis_rec) { > + sbrec_port_binding_set_chassis(pb, NULL); > + } > + update_tracked = true; > } > } > > - /* Check if the port encap binding, if any, has changed */ > - struct sbrec_encap *encap_rec = > - sbrec_get_port_encap(chassis_rec, iface_rec); > - if ((encap_rec && pb->encap != encap_rec) || > - (!encap_rec && pb->encap)) { > - if (sb_readonly) { > - return false; > - } > - sbrec_port_binding_set_encap(pb, encap_rec); > + if (update_tracked && tracked_datapaths) { > + update_lport_tracking(pb, tracked_datapaths, true); > } > > + /* Check if the port encap binding, if any, has changed */ > + if (can_bind == CAN_BIND_AS_MAIN) { > + return update_port_encap_if_needed( > + pb, chassis_rec, iface_rec, sb_readonly); > + } else if (can_bind == CAN_BIND_AS_ADDITIONAL) { > + return update_port_additional_encap_if_needed( > + pb, chassis_rec, iface_rec, sb_readonly); > + } > return true; > } > > @@ -978,7 +1127,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_main_chassis(const struct sbrec_port_binding *pb, > + bool sb_readonly) > { > if (pb->encap) { > if (sb_readonly) { > @@ -1006,11 +1156,42 @@ release_lport_(const struct sbrec_port_binding *pb, > bool sb_readonly) > } > > static bool > -release_lport(const struct sbrec_port_binding *pb, bool sb_readonly, > +release_lport_additional_chassis(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec, > + bool sb_readonly) > +{ > + if (pb->additional_encap) { > + if (sb_readonly) { > + return false; > + } > + remove_additional_encap_for_chassis(pb, chassis_rec); > + } > + > + if (is_additional_chassis(pb, chassis_rec)) { > + if (sb_readonly) { > + return false; > + } > + remove_additional_chassis(pb, chassis_rec); > + } > + > + VLOG_INFO("Releasing lport %s from this additional chassis.", > + pb->logical_port); > + return true; > +} > + > +static bool > +release_lport(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec, bool sb_readonly, > struct hmap *tracked_datapaths, struct if_status_mgr *if_mgr) > { > - if (!release_lport_(pb, sb_readonly)) { > - return false; > + if (pb->chassis == chassis_rec) { > + if (!release_lport_main_chassis(pb, sb_readonly)) { > + return false; > + } > + } else if (is_additional_chassis(pb, chassis_rec)) { > + if (!release_lport_additional_chassis(pb, chassis_rec, sb_readonly)) > { > + return false; > + } > } > > update_lport_tracking(pb, tracked_datapaths, false); > @@ -1029,7 +1210,8 @@ is_binding_lport_this_chassis(struct binding_lport > *b_lport, > const struct sbrec_chassis *chassis) > { > return (b_lport && b_lport->pb && chassis && > - b_lport->pb->chassis == chassis); > + (b_lport->pb->chassis == chassis > + || is_additional_chassis(b_lport->pb, chassis))); > } > > /* Returns 'true' if the 'lbinding' has binding lports of type LP_CONTAINER, > @@ -1054,7 +1236,7 @@ release_binding_lport(const struct sbrec_chassis > *chassis_rec, > { > if (is_binding_lport_this_chassis(b_lport, chassis_rec)) { > remove_related_lport(b_lport->pb, b_ctx_out); > - if (!release_lport(b_lport->pb, sb_readonly, > + if (!release_lport(b_lport->pb, chassis_rec, sb_readonly, > b_ctx_out->tracked_dp_bindings, > b_ctx_out->if_mgr)) { > return false; > @@ -1108,22 +1290,21 @@ consider_vif_lport_(const struct sbrec_port_binding > *pb, > } else { > /* We could, but can't claim the lport. */ > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_INFO_RL(&rl, > - "Not claiming lport %s, chassis %s " > - "requested-chassis %s", > - pb->logical_port, > - b_ctx_in->chassis_rec->name, > - pb->requested_chassis ? > - pb->requested_chassis->name : "(option points > at " > - "non-existent " > - "chassis)"); > + const char *requested_chassis_option = smap_get( > + &pb->options, "requested-chassis"); > + VLOG_INFO_RL(&rl, > + "Not claiming lport %s, chassis %s requested-chassis %s", > + pb->logical_port, b_ctx_in->chassis_rec->name, > + requested_chassis_option ? requested_chassis_option : "[]"); > } > } > > - if (pb->chassis == b_ctx_in->chassis_rec) { > + if (pb->chassis == b_ctx_in->chassis_rec > + || is_additional_chassis(pb, b_ctx_in->chassis_rec)) { > /* Release the lport if there is no lbinding. */ > if (!lbinding_set || !can_bind) { > - return release_lport(pb, !b_ctx_in->ovnsb_idl_txn, > + return release_lport(pb, b_ctx_in->chassis_rec, > + !b_ctx_in->ovnsb_idl_txn, > b_ctx_out->tracked_dp_bindings, > b_ctx_out->if_mgr); > } > @@ -1245,7 +1426,8 @@ consider_container_lport(const struct > sbrec_port_binding *pb, > * if it was bound earlier. */ > if (is_binding_lport_this_chassis(container_b_lport, > b_ctx_in->chassis_rec)) { > - return release_lport(pb, !b_ctx_in->ovnsb_idl_txn, > + return release_lport(pb, b_ctx_in->chassis_rec, > + !b_ctx_in->ovnsb_idl_txn, > b_ctx_out->tracked_dp_bindings, > b_ctx_out->if_mgr); > } > @@ -1338,11 +1520,20 @@ 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)) { > + enum can_bind can_bind = lport_can_bind_on_this_chassis( > + b_ctx_in->chassis_rec, pb); > + if (can_bind == CAN_BIND_AS_MAIN) { > + if (!release_lport_main_chassis(pb, !b_ctx_in->ovnsb_idl_txn)) { > + return false; > + } > + } else if (can_bind == CAN_BIND_AS_ADDITIONAL) { > + if (!release_lport_additional_chassis(pb, b_ctx_in->chassis_rec, > + !b_ctx_in->ovnsb_idl_txn)) { > return false; > } > + } > > + if (can_bind) { > remove_related_lport(pb, b_ctx_out); > } > > @@ -1373,8 +1564,12 @@ consider_nonvif_lport_(const struct sbrec_port_binding > *pb, > !b_ctx_in->ovnsb_idl_txn, false, > b_ctx_out->tracked_dp_bindings, > b_ctx_out->if_mgr); > - } else if (pb->chassis == b_ctx_in->chassis_rec) { > - return release_lport(pb, !b_ctx_in->ovnsb_idl_txn, > + } > + > + if (pb->chassis == b_ctx_in->chassis_rec || > + is_additional_chassis(pb, b_ctx_in->chassis_rec)) { > + return release_lport(pb, b_ctx_in->chassis_rec, > + !b_ctx_in->ovnsb_idl_txn, > b_ctx_out->tracked_dp_bindings, > b_ctx_out->if_mgr); > } > @@ -1701,6 +1896,11 @@ binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > sbrec_port_binding_set_chassis(binding_rec, NULL); > any_changes = true; > } > + if (is_additional_chassis(binding_rec, chassis_rec)) { > + remove_additional_encap_for_chassis(binding_rec, chassis_rec); > + remove_additional_chassis(binding_rec, chassis_rec); > + any_changes = true; > + } > } > > if (any_changes) { > diff --git a/controller/lport.c b/controller/lport.c > index 5ad40f6d3..d88215715 100644 > --- a/controller/lport.c > +++ b/controller/lport.c > @@ -108,29 +108,41 @@ lport_get_l3gw_peer(const struct sbrec_port_binding *pb, > return get_peer_lport(pb, sbrec_port_binding_by_name); > } > > -bool > +enum can_bind > lport_can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, > const struct sbrec_port_binding *pb) > { > - /* We need to check for presence of the requested-chassis option in > - * addittion to checking the pb->requested_chassis column because this > - * column will be set to NULL whenever the option points to a > non-existent > - * chassis. As the controller routinely clears its own chassis record > this > - * might occur more often than one might think. */ > + if (pb->requested_chassis == chassis_rec) { > + return CAN_BIND_AS_MAIN; > + } > + > + for (int i = 0; i < pb->n_requested_additional_chassis; i++) { > + if (pb->requested_additional_chassis[i] == chassis_rec) { > + return CAN_BIND_AS_ADDITIONAL; > + } > + } > + > const char *requested_chassis_option = smap_get(&pb->options, > "requested-chassis"); > - if (requested_chassis_option && requested_chassis_option[0] > - && !pb->requested_chassis) { > - /* The requested-chassis option is set, but the requested_chassis > - * column is not filled. This means that the chassis the option > - * points to is currently not running, or is in the process of > starting > - * up. In this case we must fall back to comparing the strings to > - * avoid release/claim thrashing. */ > - return !strcmp(requested_chassis_option, chassis_rec->name) > - || !strcmp(requested_chassis_option, chassis_rec->hostname); > + if (!requested_chassis_option || !strcmp("", requested_chassis_option)) { > + return CAN_BIND_AS_MAIN; > + } > + > + char *tokstr = xstrdup(requested_chassis_option); > + char *save_ptr = NULL; > + char *chassis; > + enum can_bind can_bind = CAN_BIND_AS_MAIN; > + for (chassis = strtok_r(tokstr, ",", &save_ptr); chassis != NULL; > + chassis = strtok_r(NULL, ",", &save_ptr)) { > + if (!strcmp(chassis, chassis_rec->name) > + || !strcmp(chassis, chassis_rec->hostname)) { > + free(tokstr); > + return can_bind; > + } > + can_bind = CAN_BIND_AS_ADDITIONAL; > } > - return !requested_chassis_option || !requested_chassis_option[0] > - || chassis_rec == pb->requested_chassis; > + free(tokstr); > + return CANNOT_BIND; > } > > const struct sbrec_datapath_binding * > diff --git a/controller/lport.h b/controller/lport.h > index 4716c58f9..115881655 100644 > --- a/controller/lport.h > +++ b/controller/lport.h > @@ -43,8 +43,15 @@ const struct sbrec_port_binding *lport_lookup_by_key( > struct ovsdb_idl_index *sbrec_port_binding_by_key, > uint64_t dp_key, uint64_t port_key); > > -bool lport_can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, > - const struct sbrec_port_binding *pb); > +enum can_bind { > + CANNOT_BIND = 0, > + CAN_BIND_AS_MAIN, > + CAN_BIND_AS_ADDITIONAL, > +}; > + > +enum can_bind > +lport_can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, > + const struct sbrec_port_binding *pb); > > const struct sbrec_datapath_binding *datapath_lookup_by_key( > struct ovsdb_idl_index *sbrec_datapath_binding_by_key, uint64_t dp_key); > diff --git a/northd/northd.c b/northd/northd.c > index 51dec36b3..3ebe60a7e 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -3245,31 +3245,53 @@ ovn_port_update_sbrec_chassis( > const struct ovn_port *op) > { > const char *requested_chassis; /* May be NULL. */ > - bool reset_requested_chassis = false; > + > + int n_requested_chassis = 0; > + struct sbrec_chassis **requested_chassis_sb = xcalloc( > + n_requested_chassis, sizeof *requested_chassis_sb); > + > requested_chassis = smap_get(&op->nbsp->options, > "requested-chassis"); > if (requested_chassis) { > - const struct sbrec_chassis *chassis = chassis_lookup( > - sbrec_chassis_by_name, sbrec_chassis_by_hostname, > - requested_chassis); > - if (chassis) { > - sbrec_port_binding_set_requested_chassis(op->sb, chassis); > - } else { > - reset_requested_chassis = true; > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT( > - 1, 1); > - VLOG_WARN_RL( > - &rl, > - "Unknown chassis '%s' set as " > - "options:requested-chassis on LSP '%s'.", > - requested_chassis, op->nbsp->name); > - } > - } else if (op->sb->requested_chassis) { > - reset_requested_chassis = true; > - } > - if (reset_requested_chassis) { > + char *tokstr = xstrdup(requested_chassis); > + char *save_ptr = NULL; > + char *chassis; > + for (chassis = strtok_r(tokstr, ",", &save_ptr); chassis != NULL; > + chassis = strtok_r(NULL, ",", &save_ptr)) { > + const struct sbrec_chassis *chassis_sb = chassis_lookup( > + sbrec_chassis_by_name, sbrec_chassis_by_hostname, chassis); > + if (chassis_sb) { > + requested_chassis_sb = xrealloc( > + requested_chassis_sb, > + ++n_requested_chassis * (sizeof *requested_chassis_sb)); > + requested_chassis_sb[n_requested_chassis - 1] = ( > + (struct sbrec_chassis *) chassis_sb); > + } else { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT( > + 1, 1); > + VLOG_WARN_RL( > + &rl, > + "Unknown chassis '%s' set in " > + "options:requested-chassis on LSP '%s'.", > + chassis, op->nbsp->name); > + } > + } > + free(tokstr); > + } > + > + if (n_requested_chassis > 0) { > + sbrec_port_binding_set_requested_chassis(op->sb, > + *requested_chassis_sb); > + } else { > sbrec_port_binding_set_requested_chassis(op->sb, NULL); > } > + if (n_requested_chassis > 1) { > + sbrec_port_binding_set_requested_additional_chassis( > + op->sb, &requested_chassis_sb[1], n_requested_chassis - 1); > + } else { > + sbrec_port_binding_set_requested_additional_chassis(op->sb, NULL, 0); > + } > + free(requested_chassis_sb); > } > > static void > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index a25ff57a3..e4e980720 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -105,7 +105,9 @@ static const char *rbac_fdb_update[] = > static const char *rbac_port_binding_auth[] = > {""}; > static const char *rbac_port_binding_update[] = > - {"chassis", "encap", "up", "virtual_parent"}; > + {"chassis", "additional_chassis", > + "encap", "additional_encap", > + "up", "virtual_parent"}; > > static const char *rbac_mac_binding_auth[] = > {""}; > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 3e3e142b3..66bedda33 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -1020,12 +1020,20 @@ > </p> > > <column name="options" key="requested-chassis"> > - If set, identifies a specific chassis (by name or hostname) that > - is allowed to bind this port. Using this option will prevent > - thrashing between two chassis trying to bind the same port during > - a live migration. It can also prevent similar thrashing due to a > - mis-configuration, if a port is accidentally created on more than > - one chassis. > + <p> > + If set, identifies a specific chassis (by name or hostname) that > + is allowed to bind this port. Using this option will prevent > + thrashing between two chassis trying to bind the same port during > + a live migration. It can also prevent similar thrashing due to a > + mis-configuration, if a port is accidentally created on more than > + one chassis. > + </p> > + > + <p> > + If set to a comma separated list, the first entry identifies the > + main chassis and the rest are one or more additional chassis that > + are allowed to bind the same port. > + </p> > </column> > > <column name="options" key="iface-id-ver"> > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema > index 860952194..3b78ea6f6 100644 > --- a/ovn-sb.ovsschema > +++ b/ovn-sb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Southbound", > "version": "20.23.0", > - "cksum": "2468078434 27629", > + "cksum": "4045988377 28575", > "tables": { > "SB_Global": { > "columns": { > @@ -218,10 +218,18 @@ > "refTable": "Chassis", > "refType": "weak"}, > "min": 0, "max": 1}}, > + "additional_chassis": {"type": {"key": {"type": "uuid", > + "refTable": > "Chassis", > + "refType": "weak"}, > + "min": 0, "max": > "unlimited"}}, > "encap": {"type": {"key": {"type": "uuid", > "refTable": "Encap", > "refType": "weak"}, > "min": 0, "max": 1}}, > + "additional_encap": {"type": {"key": {"type": "uuid", > + "refTable": "Encap", > + "refType": "weak"}, > + "min": 0, "max": "unlimited"}}, > "mac": {"type": {"key": "string", > "min": 0, > "max": "unlimited"}}, > @@ -239,7 +247,12 @@ > "requested_chassis": {"type": {"key": {"type": "uuid", > "refTable": "Chassis", > "refType": "weak"}, > - "min": 0, "max": 1}}}, > + "min": 0, "max": 1}}, > + "requested_additional_chassis": { > + "type": {"key": {"type": "uuid", > + "refTable": "Chassis", > + "refType": "weak"}, > + "min": 0, "max": > "unlimited"}}}, > "indexes": [["datapath", "tunnel_key"], ["logical_port"]], > "isRoot": true}, > "MAC_Binding": { > diff --git a/ovn-sb.xml b/ovn-sb.xml > index 4c35dda36..e231da302 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -2906,9 +2906,17 @@ tcp.flags = RST; > </column> > > <column name="encap"> > - Points to supported encapsulation configurations to transmit > - logical dataplane packets to this chassis. Each entry is a <ref > - table="Encap"/> record that describes the configuration. > + Points to preferred encapsulation configuration to transmit > + logical dataplane packets to this chassis. The entry is reference to > + a <ref table="Encap"/> record. > + </column> > + > + <column name="additional_encap"> > + Points to preferred encapsulation configuration to transmit > + logical dataplane packets to this additional chassis. The entry is > + reference to a <ref table="Encap"/> record. > + > + See also <ref column="additional_chassis"/>. > </column> > > <column name="chassis"> > @@ -2960,6 +2968,13 @@ tcp.flags = RST; > > </column> > > + <column name="additional_chassis"> > + The meaning of this column is the same as for the > + <ref column="chassis"/>. The column is used to track an additional > + physical location of the logical port. Used with regular (empty > + <ref column="type"/>) port bindings. > + </column> > + > <column name="gateway_chassis"> > <p> > A list of <ref table="Gateway_Chassis"/>. > @@ -3133,6 +3148,28 @@ tcp.flags = RST; > db="OVN_Northbound"/> > is defined and contains a string matching the name or hostname of an > existing chassis. > + > + See also > + <ref table="Port_Binding" column="requested_additional_chassis"/>. > + </column> > + <column name="requested_additional_chassis"> > + This column exists so that the ovn-controller can effectively monitor > + all <ref table="Port_Binding"/> records destined for it, and is a > + supplement to the <ref > + table="Port_Binding" > + column="options" > + key="requested-chassis"/> option when multiple chassis are listed. > + > + This column must be a list of > + <ref table="Chassis"/> records. This is populated by > + <code>ovn-northd</code> when the <ref > + table="Logical_Switch_Port" > + column="options" > + key="requested-chassis" > + db="OVN_Northbound"/> > + is defined as a list of chassis names or hostnames. > + > + See also <ref table="Port_Binding" column="requested_chassis"/>. > </column> > </group> > > @@ -3292,12 +3329,20 @@ tcp.flags = RST; > </p> > > <column name="options" key="requested-chassis"> > - If set, identifies a specific chassis (by name or hostname) that > - is allowed to bind this port. Using this option will prevent > - thrashing between two chassis trying to bind the same port during > - a live migration. It can also prevent similar thrashing due to a > - mis-configuration, if a port is accidentally created on more than > - one chassis. > + <p> > + If set, identifies a specific chassis (by name or hostname) that > + is allowed to bind this port. Using this option will prevent > + thrashing between two chassis trying to bind the same port during > + a live migration. It can also prevent similar thrashing due to a > + mis-configuration, if a port is accidentally created on more than > + one chassis. > + </p> > + > + <p> > + If set to a comma separated list, the first entry identifies the > main > + chassis and the rest are one or more additional chassis that are > + allowed to bind the same port. > + </p> > </column> > > <column name="options" key="iface-id-ver"> > diff --git a/tests/ovn.at b/tests/ovn.at > index 4461b840e..a9b623d1d 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -14000,6 +14000,224 @@ OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([options:multiple requested-chassis for logical port]) > +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.11 > + > +sim_add hv2 > +as hv2 > +check ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.12 > + > +check ovn-nbctl ls-add ls0 > +check ovn-nbctl lsp-add ls0 lsp0 > + > +# Allow only chassis hv1 to bind logical port lsp0. > +check ovn-nbctl lsp-set-options lsp0 requested-chassis=hv1 > + > +as hv1 check ovs-vsctl -- add-port br-int lsp0 -- \ > + set Interface lsp0 external-ids:iface-id=lsp0 > +as hv2 check ovs-vsctl -- add-port br-int lsp0 -- \ > + set Interface lsp0 external-ids:iface-id=lsp0 > + > +wait_row_count Chassis 1 name=hv1 > +wait_row_count Chassis 1 name=hv2 > +hv1_uuid=$(fetch_column Chassis _uuid name=hv1) > +hv2_uuid=$(fetch_column Chassis _uuid name=hv2) > + > +wait_column "$hv1_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "" Port_Binding additional_chassis logical_port=lsp0 > +wait_column "" Port_Binding requested_additional_chassis logical_port=lsp0 > + > +# Request port binding at an additional chassis > +check ovn-nbctl lsp-set-options lsp0 \ > + requested-chassis=hv1,hv2 > + > +wait_column "$hv1_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "$hv2_uuid" Port_Binding additional_chassis logical_port=lsp0 > +wait_column "$hv2_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +# Check that setting iface:encap-ip populates Port_Binding:additional_encap > +wait_row_count Encap 2 chassis_name=hv1 > +wait_row_count Encap 2 chassis_name=hv2 > +encap_hv1_uuid=$(fetch_column Encap _uuid chassis_name=hv1 type=geneve) > +encap_hv2_uuid=$(fetch_column Encap _uuid chassis_name=hv2 type=geneve) > + > +wait_column "" Port_Binding encap logical_port=lsp0 > +wait_column "" Port_Binding additional_encap logical_port=lsp0 > + > +as hv1 check ovs-vsctl -- \ > + set Interface lsp0 external-ids:encap-ip=192.168.0.11 > +as hv2 check ovs-vsctl -- \ > + set Interface lsp0 external-ids:encap-ip=192.168.0.12 > + > +wait_column "$encap_hv1_uuid" Port_Binding encap logical_port=lsp0 > +wait_column "$encap_hv2_uuid" Port_Binding additional_encap logical_port=lsp0 > + > +# Complete moving the binding to the new location > +check ovn-nbctl lsp-set-options lsp0 requested-chassis=hv2 > + > +wait_column "$hv2_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv2_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "" Port_Binding additional_chassis logical_port=lsp0 > +wait_column "" Port_Binding requested_additional_chassis logical_port=lsp0 > + > +# Check that additional_encap is cleared > +wait_column "" Port_Binding additional_encap logical_port=lsp0 > + > +# Check that abrupted port migration clears additional_encap > +check ovn-nbctl lsp-set-options lsp0 \ > + requested-chassis=hv2,hv1 > +wait_column "$hv2_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv2_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding additional_chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > +check ovn-nbctl lsp-set-options lsp0 requested-chassis=hv2 > +wait_column "" Port_Binding additional_encap logical_port=lsp0 > + > +OVN_CLEANUP([hv1],[hv2]) > + > +AT_CLEANUP > +]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([options:multiple requested-chassis for logical port: change > chassis role]) > +ovn_start > + > +net_add n1 > + > +for i in 1 2 3; do > + sim_add hv$i > + as hv$i > + check ovs-vsctl add-br br-phys > + ovn_attach n1 br-phys 192.168.0.1$i > +done > + > +check ovn-nbctl ls-add ls0 > +check ovn-nbctl lsp-add ls0 lsp0 > + > +# Bind the port to all of them > +check ovn-nbctl lsp-set-options lsp0 requested-chassis=hv1,hv2,hv3 > + > +for i in 1 2 3; do > + as hv$i check ovs-vsctl -- add-port br-int lsp0 -- \ > + set Interface lsp0 external-ids:iface-id=lsp0 > +done > + > +for i in 1 2 3; do > + wait_row_count Chassis 1 name=hv$i > +done > +hv1_uuid=$(fetch_column Chassis _uuid name=hv1) > +hv2_uuid=$(fetch_column Chassis _uuid name=hv2) > +hv3_uuid=$(fetch_column Chassis _uuid name=hv3) > + > +wait_column "$hv1_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding additional_chassis > logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +# Now swap hv1 and hv2 chassis roles: main <-> additional > +check ovn-nbctl lsp-set-options lsp0 requested-chassis=hv3,hv2,hv1 > + > +wait_column "$hv3_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv3_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "$hv2_uuid $hv1_uuid" Port_Binding additional_chassis > logical_port=lsp0 > +wait_column "$hv2_uuid $hv1_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +OVN_CLEANUP([hv1],[hv2],[hv3]) > + > +AT_CLEANUP > +]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([options:multiple requested-chassis for logical port: unclaimed > behavior]) > +ovn_start > + > +net_add n1 > + > +for i in 1 2 3; do > + sim_add hv$i > + as hv$i > + check ovs-vsctl add-br br-phys > + ovn_attach n1 br-phys 192.168.0.1$i > +done > + > +check ovn-nbctl ls-add ls0 > +check ovn-nbctl lsp-add ls0 lsp0 > + > +# Bind the port to all of them > +check ovn-nbctl lsp-set-options lsp0 requested-chassis=hv1,hv2,hv3 > + > +for i in 1 2 3; do > + wait_row_count Chassis 1 name=hv$i > +done > +hv1_uuid=$(fetch_column Chassis _uuid name=hv1) > +hv2_uuid=$(fetch_column Chassis _uuid name=hv2) > +hv3_uuid=$(fetch_column Chassis _uuid name=hv3) > + > +# Port not claimed by any of chassis > +wait_column "" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "" Port_Binding additional_chassis logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +# Now claim on main chassis > +as hv1 check ovs-vsctl -- add-port br-int lsp0 -- \ > + set Interface lsp0 external-ids:iface-id=lsp0 > + > +wait_column "$hv1_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "" Port_Binding additional_chassis logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +# Now claim on the second additional chassis > +as hv3 check ovs-vsctl -- add-port br-int lsp0 -- \ > + set Interface lsp0 external-ids:iface-id=lsp0 > + > +wait_column "$hv1_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "$hv3_uuid" Port_Binding additional_chassis logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +# Now claim the other additional chassis > +as hv2 check ovs-vsctl -- add-port br-int lsp0 -- \ > + set Interface lsp0 external-ids:iface-id=lsp0 > + > +wait_column "$hv1_uuid" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding additional_chassis > logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +# Now unclaim from the main chassis > +as hv1 check ovs-vsctl -- del-port br-int lsp0 > + > +wait_column "" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding additional_chassis > logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +# Now unclaim from an additional chassis > +as hv3 check ovs-vsctl -- del-port br-int lsp0 > + > +wait_column "" Port_Binding chassis logical_port=lsp0 > +wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0 > +wait_column "$hv2_uuid" Port_Binding additional_chassis logical_port=lsp0 > +wait_column "$hv2_uuid $hv3_uuid" Port_Binding requested_additional_chassis > logical_port=lsp0 > + > +OVN_CLEANUP([hv1],[hv2],[hv3]) > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([options:requested-chassis for logical port]) > ovn_start > -- > 2.34.1 > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
