Skip to content

Commit

Permalink
controller: Consider plugging VIF on CMS request.
Browse files Browse the repository at this point in the history
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.

Add port by interfaces index - To be able to effectively remove
ports previously plugged by us we need to look up ports by
interface records.

Add Port_Binding by requested_chassis index - To be able to
effectively iterate over ports destined to our chassis we need to
look up Port_Binding records by requested_chassis.

Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Han Zhou <hzhou@ovn.org>
  • Loading branch information
fnordahl authored and hzhou8 committed Nov 5, 2021
1 parent f685146 commit 42ce561
Show file tree
Hide file tree
Showing 4 changed files with 211 additions and 2 deletions.
69 changes: 68 additions & 1 deletion controller/ovn-controller.c
Expand Up @@ -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"
Expand Down Expand Up @@ -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 \
Expand Down Expand Up @@ -3091,6 +3093,10 @@ main(int argc, char *argv[])
ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false, true));
ctrl_register_ovs_idl(ovs_idl_loop.idl);

struct ovsdb_idl_index *ovsrec_port_by_interfaces
= ovsdb_idl_index_create1(ovs_idl_loop.idl,
&ovsrec_port_col_interfaces);

ovsdb_idl_get_initial_snapshot(ovs_idl_loop.idl);

/* Configure OVN SB database. */
Expand Down Expand Up @@ -3126,6 +3132,9 @@ main(int argc, char *argv[])
struct ovsdb_idl_index *sbrec_port_binding_by_type
= ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_type);
struct ovsdb_idl_index *sbrec_port_binding_by_requested_chassis
= ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_requested_chassis);
struct ovsdb_idl_index *sbrec_datapath_binding_by_key
= ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
&sbrec_datapath_binding_col_tunnel_key);
Expand Down Expand Up @@ -3198,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_WITH_CLEAR_TRACK_DATA_IS_VALID(ct_zones, "ct_zones");
Expand Down Expand Up @@ -3429,6 +3439,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);

Expand Down Expand Up @@ -3485,6 +3500,7 @@ main(int argc, char *argv[])
if (!new_ovnsb_cond_seqno) {
VLOG_INFO("OVNSB IDL reconnected, force recompute.");
engine_set_force_recompute(true);
vif_plug_reset_idl_prime_counter();
}
ovnsb_cond_seqno = new_ovnsb_cond_seqno;
}
Expand Down Expand Up @@ -3639,6 +3655,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,
Expand Down Expand Up @@ -3795,7 +3842,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;
Expand All @@ -3808,6 +3864,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);
Expand Down Expand Up @@ -3883,6 +3948,8 @@ main(int argc, char *argv[])
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);
Expand Down
21 changes: 21 additions & 0 deletions ovn-nb.xml
Expand Up @@ -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">
Expand Down
2 changes: 1 addition & 1 deletion tests/ovn-macros.at
Expand Up @@ -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]
Expand Down
121 changes: 121 additions & 0 deletions tests/ovn.at
Expand Up @@ -28818,3 +28818,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
])

0 comments on commit 42ce561

Please sign in to comment.