[ovs-dev] [PATCH ovn 2/4] Provide timestamp information for nb_cfg.
Han Zhou
hzhou at ovn.org
Tue Sep 8 05:54:52 UTC 2020
On Mon, Sep 7, 2020 at 10:55 AM Numan Siddique <numans at ovn.org> wrote:
>
>
> On Wed, Sep 2, 2020 at 4:57 AM Han Zhou <hzhou at ovn.org> wrote:
>
>> This change adds timestamp information for nb_cfg related processing of
>> ovn-northd and ovn-controller. It is useful for measuring end to end
>> control plane latency more accurately. The timestamps tell exactly when
>> ovn-northd starts processing a NB DB change, updates the changes to
>> SB DB, and when the slowest chassis finishes processing the changes.
>>
>> Signed-off-by: Han Zhou <hzhou at ovn.org>
>>
>
> Hi Han,
>
> I have one comment in the documentation. With that comment addressed:
> Acked-by: Numan Siddique <numans at ovn.org>
>
> Please see below
>
> Thanks
> Numan
>
>
>> ---
>> controller/ovn-controller.c | 4 ++++
>> northd/ovn-northd.c | 40 ++++++++++++++++++++++++++++++--------
>> ovn-nb.ovsschema | 7 +++++--
>> ovn-nb.xml | 24 +++++++++++++++++++++--
>> ovn-sb.ovsschema | 5 +++--
>> ovn-sb.xml | 5 +++++
>> tests/ovn.at | 47
>> +++++++++++++++++++++++++++++++++++++++++++++
>> 7 files changed, 118 insertions(+), 14 deletions(-)
>>
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index 874087c..89f10a6 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -2161,6 +2161,8 @@ main(int argc, char *argv[])
>> ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
>> ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
>> &sbrec_chassis_private_col_nb_cfg);
>> + ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
>> + &sbrec_chassis_private_col_nb_cfg_timestamp);
>>
>> /* Omit the external_ids column of all the tables except for -
>> * - DNS. pinctrl.c uses the external_ids column of DNS,
>> @@ -2564,6 +2566,8 @@ main(int argc, char *argv[])
>> int64_t cur_cfg = ofctrl_get_cur_cfg();
>> if (cur_cfg && cur_cfg != chassis_private->nb_cfg) {
>> sbrec_chassis_private_set_nb_cfg(chassis_private,
>> cur_cfg);
>> + sbrec_chassis_private_set_nb_cfg_timestamp(
>> + chassis_private, time_wall_msec());
>> }
>> }
>>
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index 7be0e85..fefbf43 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -44,6 +44,7 @@
>> #include "svec.h"
>> #include "stream.h"
>> #include "stream-ssl.h"
>> +#include "timeval.h"
>> #include "unixctl.h"
>> #include "util.h"
>> #include "uuid.h"
>> @@ -11639,7 +11640,8 @@ ovnnb_db_run(struct northd_context *ctx,
>> struct ovsdb_idl_index *sbrec_chassis_by_name,
>> struct ovsdb_idl_loop *sb_loop,
>> struct hmap *datapaths, struct hmap *ports,
>> - struct ovs_list *lr_list)
>> + struct ovs_list *lr_list,
>> + int64_t loop_start_time)
>> {
>> if (!ctx->ovnsb_txn || !ctx->ovnnb_txn) {
>> return;
>> @@ -11664,7 +11666,10 @@ ovnnb_db_run(struct northd_context *ctx,
>> if (nb->ipsec != sb->ipsec) {
>> sbrec_sb_global_set_ipsec(sb, nb->ipsec);
>> }
>> - sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg);
>> + if (nb->nb_cfg != sb->nb_cfg) {
>> + sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg);
>> + nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time);
>> + }
>> sbrec_sb_global_set_options(sb, &nb->options);
>> sb_loop->next_cfg = nb->nb_cfg;
>>
>> @@ -12038,7 +12043,7 @@ static const char *rbac_chassis_update[] =
>> static const char *rbac_chassis_private_auth[] =
>> {"name"};
>> static const char *rbac_chassis_private_update[] =
>> - {"nb_cfg", "chassis"};
>> + {"nb_cfg", "nb_cfg_timestamp", "chassis"};
>>
>> static const char *rbac_encap_auth[] =
>> {"chassis_name"};
>> @@ -12242,13 +12247,15 @@ check_and_update_rbac(struct northd_context
>> *ctx)
>> /* Updates the sb_cfg and hv_cfg columns in the northbound NB_Global
>> table. */
>> static void
>> update_northbound_cfg(struct northd_context *ctx,
>> - struct ovsdb_idl_loop *sb_loop)
>> + struct ovsdb_idl_loop *sb_loop,
>> + int64_t loop_start_time)
>> {
>> /* Update northbound sb_cfg if appropriate. */
>> const struct nbrec_nb_global *nbg =
>> nbrec_nb_global_first(ctx->ovnnb_idl);
>> int64_t sb_cfg = sb_loop->cur_cfg;
>> if (nbg && sb_cfg && nbg->sb_cfg != sb_cfg) {
>> nbrec_nb_global_set_sb_cfg(nbg, sb_cfg);
>> + nbrec_nb_global_set_sb_cfg_timestamp(nbg, loop_start_time);
>> }
>>
>> /* Update northbound hv_cfg if appropriate. */
>> @@ -12256,6 +12263,7 @@ update_northbound_cfg(struct northd_context *ctx,
>> /* Find minimum nb_cfg among all chassis. */
>> const struct sbrec_chassis_private *chassis_priv;
>> int64_t hv_cfg = nbg->nb_cfg;
>> + int64_t hv_cfg_ts = 0;
>> SBREC_CHASSIS_PRIVATE_FOR_EACH (chassis_priv, ctx->ovnsb_idl) {
>> const struct sbrec_chassis *chassis = chassis_priv->chassis;
>> if (chassis) {
>> @@ -12273,12 +12281,17 @@ update_northbound_cfg(struct northd_context
>> *ctx,
>>
>> if (chassis_priv->nb_cfg < hv_cfg) {
>> hv_cfg = chassis_priv->nb_cfg;
>> + hv_cfg_ts = chassis_priv->nb_cfg_timestamp;
>> + } else if (chassis_priv->nb_cfg == hv_cfg &&
>> + chassis_priv->nb_cfg_timestamp > hv_cfg_ts) {
>> + hv_cfg_ts = chassis_priv->nb_cfg_timestamp;
>> }
>> }
>>
>> /* Update hv_cfg. */
>> if (nbg->hv_cfg != hv_cfg) {
>> nbrec_nb_global_set_hv_cfg(nbg, hv_cfg);
>> + nbrec_nb_global_set_hv_cfg_timestamp(nbg, hv_cfg_ts);
>> }
>> }
>> }
>> @@ -12287,7 +12300,8 @@ update_northbound_cfg(struct northd_context *ctx,
>> static void
>> ovnsb_db_run(struct northd_context *ctx,
>> struct ovsdb_idl_loop *sb_loop,
>> - struct hmap *ports)
>> + struct hmap *ports,
>> + int64_t loop_start_time)
>> {
>> if (!ctx->ovnnb_txn ||
>> !ovsdb_idl_has_ever_connected(ctx->ovnsb_idl)) {
>> return;
>> @@ -12295,7 +12309,7 @@ ovnsb_db_run(struct northd_context *ctx,
>>
>> struct shash ha_ref_chassis_map =
>> SHASH_INITIALIZER(&ha_ref_chassis_map);
>> handle_port_binding_changes(ctx, ports, &ha_ref_chassis_map);
>> - update_northbound_cfg(ctx, sb_loop);
>> + update_northbound_cfg(ctx, sb_loop, loop_start_time);
>> if (ctx->ovnsb_txn) {
>> update_sb_ha_group_ref_chassis(&ha_ref_chassis_map);
>> }
>> @@ -12312,9 +12326,11 @@ ovn_db_run(struct northd_context *ctx,
>> ovs_list_init(&lr_list);
>> hmap_init(&datapaths);
>> hmap_init(&ports);
>> +
>> + int64_t start_time = time_wall_msec();
>> ovnnb_db_run(ctx, sbrec_chassis_by_name, ovnsb_idl_loop,
>> - &datapaths, &ports, &lr_list);
>> - ovnsb_db_run(ctx, ovnsb_idl_loop, &ports);
>> + &datapaths, &ports, &lr_list, start_time);
>> + ovnsb_db_run(ctx, ovnsb_idl_loop, &ports, start_time);
>> destroy_datapaths_and_ports(&datapaths, &ports, &lr_list);
>> }
>>
>> @@ -12447,8 +12463,14 @@ main(int argc, char *argv[])
>> /* We want to detect (almost) all changes to the ovn-nb db. */
>> struct ovsdb_idl_loop ovnnb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
>> ovsdb_idl_create(ovnnb_db, &nbrec_idl_class, true, true));
>> + ovsdb_idl_omit_alert(ovnnb_idl_loop.idl,
>> + &nbrec_nb_global_col_nb_cfg_timestamp);
>> ovsdb_idl_omit_alert(ovnnb_idl_loop.idl,
>> &nbrec_nb_global_col_sb_cfg);
>> + ovsdb_idl_omit_alert(ovnnb_idl_loop.idl,
>> + &nbrec_nb_global_col_sb_cfg_timestamp);
>> ovsdb_idl_omit_alert(ovnnb_idl_loop.idl,
>> &nbrec_nb_global_col_hv_cfg);
>> + ovsdb_idl_omit_alert(ovnnb_idl_loop.idl,
>> + &nbrec_nb_global_col_hv_cfg_timestamp);
>>
>> unixctl_command_register("nb-connection-status", "", 0, 0,
>> ovn_conn_show, ovnnb_idl_loop.idl);
>> @@ -12577,6 +12599,8 @@ main(int argc, char *argv[])
>> &sbrec_chassis_private_col_chassis);
>> ovsdb_idl_add_column(ovnsb_idl_loop.idl,
>> &sbrec_chassis_private_col_nb_cfg);
>> + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
>> + &sbrec_chassis_private_col_nb_cfg_timestamp);
>>
>> ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_ha_chassis);
>> add_column_noalert(ovnsb_idl_loop.idl,
>> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
>> index 0c939b7..c66c9fa 100644
>> --- a/ovn-nb.ovsschema
>> +++ b/ovn-nb.ovsschema
>> @@ -1,14 +1,17 @@
>> {
>> "name": "OVN_Northbound",
>> - "version": "5.25.0",
>> - "cksum": "1354137211 26116",
>> + "version": "5.26.0",
>> + "cksum": "3650391919 26314",
>> "tables": {
>> "NB_Global": {
>> "columns": {
>> "name": {"type": "string"},
>> "nb_cfg": {"type": {"key": "integer"}},
>> + "nb_cfg_timestamp": {"type": {"key": "integer"}},
>> "sb_cfg": {"type": {"key": "integer"}},
>> + "sb_cfg_timestamp": {"type": {"key": "integer"}},
>> "hv_cfg": {"type": {"key": "integer"}},
>> + "hv_cfg_timestamp": {"type": {"key": "integer"}},
>> "external_ids": {
>> "type": {"key": "string", "value": "string",
>> "min": 0, "max": "unlimited"}},
>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>> index 1f2dbb9..cda42ff 100644
>> --- a/ovn-nb.xml
>> +++ b/ovn-nb.xml
>> @@ -54,21 +54,41 @@
>> applying the changes, it may increment this sequence number.
>> </column>
>>
>> + <column name="nb_cfg_timestamp">
>> + The timestamp when <code>ovn-northd</code> sees the latest
>> + <code>nb_cfg</code> and starts processing.
>> + </column>
>> +
>> <column name="sb_cfg">
>> Sequence number that <code>ovn-northd</code> sets to the value
>> of <ref
>> column="nb_cfg"/> after it finishes applying the corresponding
>> configuration changes to the <ref db="OVN_Southbound"/> database.
>> </column>
>>
>> + <column name="sb_cfg_timestamp">
>> + The timestamp when <code>ovn-northd</code> finishes applying the
>> + corresponding configuration changes to the <ref
>> db="OVN_Southbound"/>
>> + database successfully.
>>
>
> Can you please be more specific about the timestamp unit ?
>
> Thanks Numan. I applied this to master, with (ms) added to the document.
>
>
>
>> + </column>
>> +
>> <column name="hv_cfg">
>> Sequence number that <code>ovn-northd</code> sets to the smallest
>> sequence number of all the chassis in the system, as reported in
>> the
>> - <code>Chassis</code> table in the southbound database. Thus,
>> <ref
>> - column="hv_cfg"/> equals <ref column="nb_cfg"/> if all chassis
>> are
>> + <code>Chassis_Private</code> table in the southbound database.
>> Thus,
>> + <ref column="hv_cfg"/> equals <ref column="nb_cfg"/> if all
>> chassis are
>> caught up with the northbound configuration (which may never
>> happen, if
>> any chassis is down). This value can regress, if a chassis was
>> removed
>> from the system and rejoins before catching up.
>> </column>
>> +
>> + <column name="hv_cfg_timestamp">
>> + The largest timestamp of the smallest sequence number of all the
>> + chassis in the system, as reported in the
>> <code>Chassis_Private</code>
>> + table in the southbound database. In other words, this timestamp
>> + reflects the time when the slowest chassis catches up with the
>> + northbound configuration, which is useful for end-to-end control
>> plane
>> + latency measurement.
>> + </column>
>> </group>
>>
>> <group title="Common Columns">
>> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
>> index 3af7654..d1c506a 100644
>> --- a/ovn-sb.ovsschema
>> +++ b/ovn-sb.ovsschema
>> @@ -1,7 +1,7 @@
>> {
>> "name": "OVN_Southbound",
>> - "version": "2.9.0",
>> - "cksum": "223619766 22548",
>> + "version": "2.10.0",
>> + "cksum": "2548342632 22615",
>> "tables": {
>> "SB_Global": {
>> "columns": {
>> @@ -54,6 +54,7 @@
>> "refType": "weak"},
>> "min": 0, "max": 1}},
>> "nb_cfg": {"type": {"key": "integer"}},
>> + "nb_cfg_timestamp": {"type": {"key": "integer"}},
>> "external_ids": {
>> "type": {"key": "string", "value": "string",
>> "min": 0, "max": "unlimited"}}},
>> diff --git a/ovn-sb.xml b/ovn-sb.xml
>> index 59b2171..59888a1 100644
>> --- a/ovn-sb.xml
>> +++ b/ovn-sb.xml
>> @@ -392,6 +392,11 @@
>> from the <ref table="SB_Global"/> table into this column.
>> </column>
>>
>> + <column name="nb_cfg_timestamp">
>> + The timestamp when <code>ovn-controller</code> finishes processing
>> the
>> + change corresponding to <ref column="nb_cfg"/>.
>> + </column>
>> +
>> <group title="Common Columns">
>> The overall purpose of these columns is described under
>> <code>Common
>> Columns</code> at the beginning of this document.
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index 5ad51c0..55257c3 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -21409,3 +21409,50 @@ OVS_WAIT_UNTIL([test x$(as hv1 ovn-appctl -t
>> ovn-controller debug/status) = "xru
>>
>> OVN_CLEANUP([hv1])
>> AT_CLEANUP
>> +
>> +AT_SETUP([ovn -- nb_cfg timestamp])
>> +ovn_start
>> +
>> +ovn-nbctl ls-add s2
>> +ovn-nbctl ls-add join
>> +
>> +net_add n1
>> +
>> +n=10
>> +for i in $(seq 1 $n); do
>> + sim_add hv$i
>> + as hv$i
>> + ovs-vsctl add-br br-phys
>> + ovn_attach n1 br-phys 192.168.0.$i
>> +done
>> +
>> +AT_FAIL_IF([test x$(ovn-nbctl get nb_global . nb_cfg_timestamp) != x0])
>> +ovn-nbctl --wait=hv ls-add s1
>> +AT_CHECK([ovn-nbctl get nb_global . nb_cfg], [0], [1
>> +])
>> +AT_CHECK([test x$(ovn-nbctl get nb_global . nb_cfg_timestamp) != x0])
>> +AT_CHECK([test x$(ovn-nbctl get nb_global . sb_cfg_timestamp) != x0])
>> +AT_CHECK([test x$(ovn-nbctl get nb_global . hv_cfg_timestamp) != x0])
>> +
>> +# kill ovn-controller on chassis hv3, so that it wont update nb_cfg
>> +as hv3 ovn-appctl -t ovn-controller exit --restart
>> +
>> +ovn-nbctl --timeout=1 --wait=hv sync
>> +AT_CHECK([ovn-nbctl get nb_global . nb_cfg], [0], [2
>> +])
>> +AT_CHECK([ovn-sbctl --bare --columns=nb_cfg find chassis_private
>> name=hv3], [0], [1
>> +])
>> +
>> +# start ovn-controller on hv3 again, so that it will update nb_cfg with
>> latest
>> +# timestamp (hv3 will be the slowest one).
>> +as hv3 start_daemon ovn-controller
>> +OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns=nb_cfg find
>> chassis_private name=hv3) == x2])
>> +hv3_ts=$(ovn-sbctl --bare --columns=nb_cfg_timestamp find
>> chassis_private name=hv3)
>> +hv_cfg_ts=$(ovn-nbctl get nb_global . hv_cfg_timestamp)
>> +AT_CHECK([test x$hv3_ts == x$hv_cfg_ts])
>> +
>> +for i in $(seq 2 $n); do
>> + OVN_CLEANUP_SBOX([hv$i])
>> +done
>> +OVN_CLEANUP([hv1])
>> +AT_CLEANUP
>> --
>> 2.1.0
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
More information about the dev
mailing list