On Thu, May 5, 2022 at 6:38 AM 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]>
Thanks Ihar. Please check my comments below.
(Please allow me some more time to review the rest of the patches in this
series.)
> ---
> NEWS | 1 +
> controller/binding.c | 276 ++++++++++++++++++++++++++++++++++++-------
> controller/lport.c | 42 ++++---
> northd/northd.c | 62 ++++++----
> northd/ovn-northd.c | 4 +-
> ovn-nb.xml | 20 +++-
> ovn-sb.ovsschema | 17 ++-
> ovn-sb.xml | 63 ++++++++--
> tests/ovn.at | 89 ++++++++++++++
> 9 files changed, 476 insertions(+), 98 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index dbe89e9cf..3fedcfeed 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -7,6 +7,7 @@ Post v22.03.0
> - Support NAT for logical routers with multiple distributed gateway
ports.
> - Add global option (NB_Global.options:default_acl_drop) to enable
> implicit drop behavior on logical switches with ACLs applied.
> + - 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 e8c96a64a..20f0fd11b 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -922,6 +922,142 @@ 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++) {
> + 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 bool
> +is_requested_additional_chassis(const struct sbrec_port_binding *pb,
> + const struct sbrec_chassis *chassis_rec)
> +{
> + for (int i = 0; i < pb->n_requested_additional_chassis; i++) {
> + if (pb->requested_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.
> */
> @@ -938,38 +1074,56 @@ 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;
> - }
> + if (!pb->requested_chassis || pb->requested_chassis == chassis_rec) {
According to the logic in lport_can_bind_on_this_chassis(), it is possible
that the requested_chassis is NULL but "requested-chassis" option has this
chassis as the main chassis, we still need to claim the port.
> + 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]);
> + 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);
> + }
> }
> + } else if (is_requested_additional_chassis(pb, chassis_rec)) {
Similar to the above, need to check the "requested-chassis" option as well.
It would better that "lport_can_bind_on_this_chassis" returns the
information (CANNOT_BIND, CAN_BIND_AS_MAIN, CAN_BIND_AS_ADDITIONAL)
directly and pass the information, so that we don't have to repeat the
checking here.
> + 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]);
> + }
In the previous branch it checked if it is in additional chassis then
remove it from there. Similarly, shall we checked here in this branch if it
is the current chassis then unset it? Or are there any considerations?
It would be better to add test cases to cover the scenario that a LSP's
requested-chassis and additional-chassis are updated back and force,
including below combinations:
- request-chassis is set but it not claimed, additional chassis is set and
claimed
- a chassis is moved from requested-chassis to requested-additional-chassis
for the LSP
- a chassis is moved from requested-additional-chassis to requested-chassis
for the LSP
etc.
>
> - if (tracked_datapaths) {
> - update_lport_tracking(pb, tracked_datapaths, true);
> + append_additional_chassis(pb, chassis_rec);
> }
> }
>
> + if (tracked_datapaths) {
> + update_lport_tracking(pb, tracked_datapaths, true);
> + }
In the original logic, it is added to the tracked_datapaths here only if
the port is newly claimed, but now it is blindly added, which looks
incorrect and could lead to incremental processing problems.
> +
> /* 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 (pb->chassis == chassis_rec) {
> + return update_port_encap_if_needed(
> + pb, chassis_rec, iface_rec, sb_readonly);
> + } else if (is_additional_chassis(pb, chassis_rec)) {
> + return update_port_additional_encap_if_needed(
> + pb, chassis_rec, iface_rec, sb_readonly);
> }
>
> return true;
> @@ -983,7 +1137,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) {
> @@ -1011,11 +1166,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);
> @@ -1034,7 +1220,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,
> @@ -1059,7 +1246,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 +1295,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 +1431,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);
> }
> @@ -1339,7 +1526,7 @@ 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_main_chassis(pb, !b_ctx_in->ovnsb_idl_txn)) {
> return false;
> }
Shouldn't we also check here if the localport is claimed by this chassis as
an additional-chassis, release it?
>
> @@ -1374,7 +1561,8 @@ consider_nonvif_lport_(const struct
sbrec_port_binding *pb,
> 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,
> + 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);
Similar to the above scenario, should we check is_additional_chassis() here?
Thanks,
Han
> }
> diff --git a/controller/lport.c b/controller/lport.c
> index 5ad40f6d3..ed63608a2 100644
> --- a/controller/lport.c
> +++ b/controller/lport.c
> @@ -112,25 +112,35 @@ bool
> 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 true;
> + }
> +
> + for (int i = 0; i < pb->n_requested_additional_chassis; i++) {
> + if (pb->requested_additional_chassis[i] == chassis_rec) {
> + return true;
> + }
> + }
> +
> 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 true;
> + }
> +
> + char *tokstr = xstrdup(requested_chassis_option);
> + char *save_ptr = NULL;
> + char *chassis;
> + 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 true;
> + }
> }
> - return !requested_chassis_option || !requested_chassis_option[0]
> - || chassis_rec == pb->requested_chassis;
> + free(tokstr);
> + return false;
> }
>
> const struct sbrec_datapath_binding *
> diff --git a/northd/northd.c b/northd/northd.c
> index a56666297..55919188e 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -3177,31 +3177,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 0a0f85010..1cfc2024d 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -100,7 +100,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 9010240a8..3a5c7d3ec 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 66664c840..fcd74b918 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Southbound",
> "version": "20.22.0",
> - "cksum": "1686121686 27471",
> + "cksum": "2805878321 28417",
> "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"}},
> @@ -236,7 +244,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 1ffb24e7a..ff9cb8c83 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2867,9 +2867,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">
> @@ -2921,6 +2929,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"/>.
> @@ -3079,6 +3094,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>
>
> @@ -3238,12 +3275,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 799a6aabd..9210b256b 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -13917,6 +13917,95 @@ 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: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
