[ovs-dev] [PATCH ovn v8 3/4] controller: Consider plugging VIF on CMS request.
Han Zhou
hzhou at ovn.org
Fri Nov 5 07:42:52 UTC 2021
On Tue, Nov 2, 2021 at 4:29 AM Frode Nordahl <frode.nordahl at canonical.com>
wrote:
>
> When OVN is linked with an appropriate VIF plug provider,
> CMS can request an OVN controller to plug individual lports into
> the Open vSwitch instance it manages.
>
> The port and interface record will be maintained throughout the
> lifetime of the lport and it will be removed on release of lport.
>
> Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
> ---
> controller/ovn-controller.c | 61 +++++++++++++++++-
> ovn-nb.xml | 21 +++++++
> tests/ovn-macros.at | 2 +-
> tests/ovn.at | 121 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 203 insertions(+), 2 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index dd09d57e2..b4d4636d8 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -106,6 +106,7 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
> #define IF_STATUS_MGR_UPDATE_STOPWATCH_NAME "if-status-mgr-update"
> #define OFCTRL_SEQNO_RUN_STOPWATCH_NAME "ofctrl-seqno-run"
> #define BFD_RUN_STOPWATCH_NAME "bfd-run"
> +#define VIF_PLUG_RUN_STOPWATCH_NAME "vif-plug-run"
>
> #define OVS_NB_CFG_NAME "ovn-nb-cfg"
> #define OVS_NB_CFG_TS_NAME "ovn-nb-cfg-ts"
> @@ -937,6 +938,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> binding_register_ovs_idl(ovs_idl);
> bfd_register_ovs_idl(ovs_idl);
> physical_register_ovs_idl(ovs_idl);
> + vif_plug_register_ovs_idl(ovs_idl);
> }
>
> #define SB_NODES \
> @@ -3205,6 +3207,7 @@ main(int argc, char *argv[])
> stopwatch_create(IF_STATUS_MGR_UPDATE_STOPWATCH_NAME, SW_MS);
> stopwatch_create(OFCTRL_SEQNO_RUN_STOPWATCH_NAME, SW_MS);
> stopwatch_create(BFD_RUN_STOPWATCH_NAME, SW_MS);
> + stopwatch_create(VIF_PLUG_RUN_STOPWATCH_NAME, SW_MS);
>
> /* Define inc-proc-engine nodes. */
> ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA(ct_zones, "ct_zones");
> @@ -3440,6 +3443,11 @@ main(int argc, char *argv[])
> };
> struct if_status_mgr *if_mgr = ctrl_engine_ctx.if_mgr;
>
> + struct shash vif_plug_deleted_iface_ids =
> + SHASH_INITIALIZER(&vif_plug_deleted_iface_ids);
> + struct shash vif_plug_changed_iface_ids =
> + SHASH_INITIALIZER(&vif_plug_changed_iface_ids);
> +
> char *ovn_version = ovn_get_internal_version();
> VLOG_INFO("OVN internal version is : [%s]", ovn_version);
>
> @@ -3650,6 +3658,37 @@ main(int argc, char *argv[])
> ovsrec_port_table_get(ovs_idl_loop.idl),
> br_int, chassis,
&runtime_data->local_datapaths);
> stopwatch_stop(PATCH_RUN_STOPWATCH_NAME,
time_msec());
> + if (vif_plug_provider_has_providers() &&
ovs_idl_txn) {
> + struct vif_plug_ctx_in vif_plug_ctx_in = {
> + .ovs_idl_txn = ovs_idl_txn,
> + .sbrec_port_binding_by_name =
> + sbrec_port_binding_by_name,
> + .sbrec_port_binding_by_requested_chassis
=
> +
sbrec_port_binding_by_requested_chassis,
> + .ovsrec_port_by_interfaces =
> + ovsrec_port_by_interfaces,
> + .ovs_table = ovs_table,
> + .br_int = br_int,
> + .iface_table =
> + ovsrec_interface_table_get(
> + ovs_idl_loop.idl),
> + .chassis_rec = chassis,
> + .local_bindings =
> +
&runtime_data->lbinding_data.bindings,
> + };
> + struct vif_plug_ctx_out vif_plug_ctx_out = {
> + .deleted_iface_ids =
> + &vif_plug_deleted_iface_ids,
> + .changed_iface_ids =
> + &vif_plug_changed_iface_ids,
> + };
> + stopwatch_start(VIF_PLUG_RUN_STOPWATCH_NAME,
> + time_msec());
> + vif_plug_run(&vif_plug_ctx_in,
> + &vif_plug_ctx_out);
> + stopwatch_stop(VIF_PLUG_RUN_STOPWATCH_NAME,
> + time_msec());
> + }
> stopwatch_start(PINCTRL_RUN_STOPWATCH_NAME,
> time_msec());
> pinctrl_run(ovnsb_idl_txn,
> @@ -3806,7 +3845,16 @@ main(int argc, char *argv[])
> engine_set_force_recompute(true);
> }
>
> - if (ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop) == 1) {
> + int ovs_txn_status =
ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop);
> + if (!ovs_txn_status) {
> + /* The transaction failed. */
> + vif_plug_clear_deleted(
> + &vif_plug_deleted_iface_ids);
> + vif_plug_clear_changed(
> + &vif_plug_changed_iface_ids);
> + } else if (ovs_txn_status == 1) {
> + /* The transaction committed successfully
> + * (or it did not change anything in the database). */
> ct_zones_data = engine_get_data(&en_ct_zones);
> if (ct_zones_data) {
> struct shash_node *iter, *iter_next;
> @@ -3819,6 +3867,15 @@ main(int argc, char *argv[])
> }
> }
> }
> +
> + vif_plug_finish_deleted(
> + &vif_plug_deleted_iface_ids);
> + vif_plug_finish_changed(
> + &vif_plug_changed_iface_ids);
> + } else if (ovs_txn_status == -1) {
> + /* The commit is still in progress */
> + } else {
> + OVS_NOT_REACHED();
> }
>
> ovsdb_idl_track_clear(ovnsb_idl_loop.idl);
> @@ -3894,6 +3951,8 @@ loop_done:
> pinctrl_destroy();
> patch_destroy();
> if_status_mgr_destroy(if_mgr);
> + shash_destroy(&vif_plug_deleted_iface_ids);
> + shash_destroy(&vif_plug_changed_iface_ids);
> vif_plug_provider_destroy_all();
>
> ovsdb_idl_loop_destroy(&ovs_idl_loop);
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index e31578fb6..036ffa64f 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1031,6 +1031,27 @@
> DHCP reply.
> </p>
> </column>
> +
> + <group title="VIF Plugging Options">
> + <column name="options" key="vif-plug-type">
> + If set, OVN will attempt to perform plugging of this VIF.
In order
> + to get this port plugged by the OVN controller, OVN must be
built
> + with support for VIF plugging. The default behavior is for
the CMS
> + to do the VIF plugging. Each VIF plug provider have their
own
> + options namespaced by name, for example
"vif-plug:representor:key".
> +
> + Please refer to the VIF plug provider documentation located
in
> + Documentation/topics/vif-plug-providers/ for more
information.
> + </column>
> +
> + <column name="options" key="vif-plug-mtu-request">
> + Requested MTU for plugged interfaces. When set the OVN
controller
> + will fill the <ref table="Interface" column="mtu_request"/>
column
> + of the Open vSwitch database's
> + <ref table="Interface" db="vswitch"/> table. This in turn
will
> + make OVS vswitchd update the MTU of the linked interface.
> + </column>
> + </group>
> </group>
>
> <group title="Virtual port Options">
> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
> index f06f2e68e..21b7d985e 100644
> --- a/tests/ovn-macros.at
> +++ b/tests/ovn-macros.at
> @@ -327,7 +327,7 @@ ovn_az_attach() {
> -- --may-exist add-br br-int \
> -- set bridge br-int fail-mode=secure
other-config:disable-in-band=true \
> || return 1
> - start_daemon ovn-controller || return 1
> + start_daemon ovn-controller --enable-dummy-vif-plug || return 1
> }
>
> # ovn_attach NETWORK BRIDGE IP [MASKLEN]
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 7cfdede77..b93dbdcff 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -28789,3 +28789,124 @@ AT_CHECK([grep -q "Not claiming"
hv1/ovn-controller.log], [1], [])
> OVN_CLEANUP([hv1])
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-controller - VIF plugging])
> +AT_KEYWORDS([vif-plug])
> +
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +sim_add hv2
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.2
> +
> +check ovn-nbctl ls-add lsw0
> +
> +check ovn-nbctl lsp-add lsw0 lsp1
> +check ovn-nbctl lsp-set-addresses lsp1 "f0:00:00:00:00:01 172.16.0.101"
> +check ovn-nbctl lsp-set-options lsp1 \
> + requested-chassis=hv1 \
> + vif-plug-type=dummy \
> + vif-plug-mtu-request=42
> +
> +check ovn-nbctl lsp-add lsw0 lsp2
> +check ovn-nbctl lsp-set-addresses lsp2 "f0:00:00:00:00:02 172.16.0.102"
> +check ovn-nbctl lsp-set-options lsp2 \
> + requested-chassis=hv2 \
> + vif-plug-type=dummy \
> + vif-plug-mtu-request=42
> +
> +wait_for_ports_up lsp1 lsp2
> +
> +lsp1_uuid=$(ovn-nbctl --bare --columns _uuid find Logical_Switch_Port
name=lsp1)
> +iface1_uuid=$(as hv1 ovs-vsctl --bare --columns _uuid find Interface
name=lsp1)
> +
> +lsp2_uuid=$(ovn-nbctl --bare --columns _uuid find Logical_Switch_Port
name=lsp2)
> +iface2_uuid=$(as hv2 ovs-vsctl --bare --columns _uuid find Interface
name=lsp2)
> +
> +# Check that the lport was plugged
> +AT_CHECK([test xvalue = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid}
options:vif-plug-dummy-option)], [0], [])
> +AT_CHECK([test x42 = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid}
mtu_request)], [0], [])
> +
> +# Check that updating the lport updates the local iface
> +check ovn-nbctl --wait=hv lsp-set-options lsp1 \
> + requested-chassis=hv1 \
> + vif-plug-type=dummy \
> + vif-plug-mtu-request=43
> +OVS_WAIT_UNTIL([
> + test x43 = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid}
mtu_request)
> +])
> +
> +# Check that local modification of iface will trigger ovn-controller to
update
> +# the iface record
> +check as hv1 ovs-vsctl set interface ${iface1_uuid} mtu_request=44
> +OVS_WAIT_UNTIL([
> + test x43 = x$(as hv1 ovs-vsctl get Interface ${iface1_uuid}
mtu_request)
> +])
> +
> +# Check that pointing requested-chassis somewhere else will unplug the
port
> +check ovn-nbctl --wait=hv set Logical_Switch_Port lsp1 \
> + options:requested-chassis=non-existent-chassis
> +OVS_WAIT_UNTIL([
> + ! as hv1 ovs-vsctl get Interface ${iface1_uuid} _uuid
> +])
> +
> +# Check that removing an lport will unplug it
> +AT_CHECK([test x${iface2_uuid} = x$(as hv2 ovs-vsctl get Interface
${iface2_uuid} _uuid)], [0], [])
> +check ovn-nbctl --wait=hv lsp-del ${lsp2_uuid}
> +OVS_WAIT_UNTIL([
> + ! as hv2 ovs-vsctl get Interface ${iface2_uuid} _uuid
> +])
> +
> +# Check that port is unplugged when we simulate presence of a port
previously
> +# plugged by us in local OVSDB with no record in SB DB.
> +check as hv2 ovs-vsctl \
> + -- add-port br-int vif1
> +
> +# From one moment it's there...
> +vif1_uuid=$(as hv2 ovs-vsctl --bare --columns _uuid find Interface
name=vif1)
> +OVS_WAIT_UNTIL([
> + as hv2 ovs-vsctl get Interface ${vif1_uuid} _uuid
> +])
> +
> +# Add the external-ids we expect
> +check as hv2 ovs-vsctl \
> + -- set Interface ${vif1_uuid} \
> + external-ids:ovn-plugged=dummy \
> + external-ids:iface-id=non-existing-lsp
> +
> +# ...to the next moment it's gone.
> +OVS_WAIT_UNTIL([
> + ! as hv2 ovs-vsctl get Interface ${vif1_uuid} _uuid
> +])
> +
> +# Check that a warning is logged when CMS requests plugging of an
interface
> +# with lbinding already plugged by someone else.
> +check as hv2 ovs-vsctl \
> + -- add-port br-int vif3 \
> + -- set Interface vif3 \
> + external-ids:iface-id=lsp3
> +
> +check ovn-nbctl lsp-add lsw0 lsp3
> +check ovn-nbctl lsp-set-addresses lsp3 "f0:00:00:00:00:03 172.16.0.103"
> +check ovn-nbctl lsp-set-options lsp3 \
> + requested-chassis=hv2
> +
> +wait_for_ports_up lsp3
> +
> +check ovn-nbctl --wait=hv lsp-set-options lsp3 \
> + requested-chassis=hv2 \
> + vif-plug-type=dummy
> +
> +OVS_WAIT_UNTIL([
> + grep -q "CMS requested plugging of lport lsp3" hv2/ovn-controller.log
> +])
> +
> +OVN_CLEANUP([hv1],[hv2])
> +AT_CLEANUP
> +])
> --
> 2.32.0
>
Acked-by: Han Zhou <hzhou at ovn.org>
More information about the dev
mailing list