[ovs-dev] [PATCH] ovn: support requested-chassis option for logical switch ports

Russell Bryant russell at ovn.org
Fri Aug 18 20:20:54 UTC 2017


On Fri, Aug 18, 2017 at 12:13 PM, Lance Richardson <lrichard at redhat.com> wrote:
> This patch adds support for a "requested-chassis" option for logical
> switch ports. If set, the only chassis that will claim this port is the
> chassis identfied by this option; if already bound by another chassis,
> it will be released.
>
> The primary benefit of this enhancement is allowing a CMS to prevent
> "thrashing" in the southbound database during live migration by keeping
> the original chassis from attempting to re-bind a port that is in the
> process of migrating.
>
> This would also allow (with some additional work) RBAC to be applied
> to the Port_Binding table for additional security.
>
> Signed-off-by: Lance Richardson <lrichard at redhat.com>

Acked-by: Russell Bryant <russell at ovn.org>

but since I talked to you about this approach a good bit before you
submitted it, I'd like to leave more time for others to comment on it.

One more comment inline ...

> ---
>  ovn/controller/binding.c | 15 +++++++++++-
>  ovn/ovn-nb.xml           |  5 ++++
>  ovn/ovn-sb.xml           |  5 ++++
>  tests/ovn.at             | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index 32309e9e3..6a56e26ca 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -440,7 +440,12 @@ consider_local_datapath(struct controller_ctx *ctx,
>      }
>
>      if (ctx->ovnsb_idl_txn) {
> -        if (our_chassis) {
> +        const char *vif_chassis = smap_get(&binding_rec->options,
> +                                           "requested-chassis");
> +        bool can_bind = !vif_chassis || !vif_chassis[0] ||
> +                        !strcmp(vif_chassis, chassis_rec->name);
> +
> +        if (can_bind && our_chassis) {
>              if (binding_rec->chassis != chassis_rec) {
>                  if (binding_rec->chassis) {
>                      VLOG_INFO("Changing chassis for lport %s from %s to %s.",
> @@ -461,6 +466,14 @@ consider_local_datapath(struct controller_ctx *ctx,
>              VLOG_INFO("Releasing lport %s from this chassis.",
>                        binding_rec->logical_port);
>              sbrec_port_binding_set_chassis(binding_rec, NULL);
> +        } else if (our_chassis) {
> +            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",
> +                         binding_rec->logical_port,
> +                         chassis_rec->name,
> +                         vif_chassis);
>          }
>      }
>  }
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 31303a846..7db860f57 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -435,6 +435,11 @@
>            (empty string)
>          </p>
>
> +        <column name="options" key="requested-chassis">
> +          If set, identifies a specific chassis (by name) that is allowed to
> +          bind this port.
> +        </column>
> +

Perhaps we could expand on the documentation explain why this is
useful.  I'd say something like ...

"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."

>          <column name="options" key="qos_max_rate">
>            If set, indicates the maximum rate for data sent from this interface,
>            in bit/s. The traffic will be shaped according to this limit.
> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index 08d784ac6..583f8ae39 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -2159,6 +2159,11 @@ tcp.flags = RST;
>          (empty string)
>        </p>
>
> +      <column name="options" key="requested-chassis">
> +        If set, identifies a specific chassis (by name) that is allowed to
> +        bind this port.
> +      </column>
> +
>        <column name="options" key="qos_max_rate">
>          If set, indicates the maximum rate for data sent from this interface,
>          in bit/s. The traffic will be shaped according to this limit.
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 7d816785e..dcd0cf1a1 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -8281,3 +8281,62 @@ AT_CHECK([grep "Releasing lport" gw1/ovn-controller.log], [1], [])
>  OVN_CLEANUP([gw1],[gw2],[hv1])
>
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- options:requested-chassis for logical port])
> +ovn_start
> +
> +net_add n1
> +
> +ovn-nbctl ls-add ls0
> +ovn-nbctl lsp-add ls0 lsp0
> +
> +# create two hypervisors, each with one vif port
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.11
> +ovs-vsctl -- add-port br-int hv1-vif0
> +
> +sim_add hv2
> +as hv2
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.12
> +ovs-vsctl -- add-port br-int hv2-vif0
> +
> +# Allow only chassis hv1 to bind logical port lsp0.
> +ovn-nbctl lsp-set-options lsp0 requested-chassis=hv1
> +
> +# Allow some time for ovn-northd and ovn-controller to catch up.
> +ovn-nbctl --wait=hv --timeout=3 sync
> +
> +# Retrieve hv1 and hv2 chassis UUIDs from southbound database
> +hv1_uuid=$(ovn-sbctl --bare --columns _uuid find chassis name=hv1)
> +hv2_uuid=$(ovn-sbctl --bare --columns _uuid find chassis name=hv2)
> +
> +# (1) Chassis hv2 should not bind lsp0 when requested-chassis is hv1.
> +echo "verifying that hv2 does not bind lsp0 when hv2 physical/logical mapping is added"
> +as hv2
> +ovs-vsctl set interface hv2-vif0 external-ids:iface-id=lsp0
> +
> +OVS_WAIT_UNTIL([test 1 = $(grep -c "Not claiming lport lsp0" hv2/ovn-controller.log)])
> +AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = x], [0], [])
> +
> +# (2) Chassis hv1 should bind lsp0 when physical to logical mapping exists on hv1.
> +echo "verifying that hv1 binds lsp0 when hv1 physical/logical mapping is added"
> +as hv1
> +ovs-vsctl set interface hv1-vif0 external-ids:iface-id=lsp0
> +
> +OVS_WAIT_UNTIL([test 1 = $(grep -c "Claiming lport lsp0" hv1/ovn-controller.log)])
> +AT_CHECK([test $(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = "$hv1_uuid"], [0], [])
> +
> +# (3) Chassis hv1 should release lsp0 binding and chassis hv2 should bind lsp0 when
> +# the requested chassis for lsp0 is changed from hv1 to hv2.
> +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)])
> +OVS_WAIT_UNTIL([test $(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = "$hv2_uuid"])
> +
> +OVN_CLEANUP([hv1],[hv2])
> +
> +AT_CLEANUP
> --
> 2.13.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev



-- 
Russell Bryant


More information about the dev mailing list