On Fri, Dec 3, 2021 at 12:59 AM Han Zhou <[email protected]> wrote: > > Add the debug command in case lflow conjunction ids mapping needs to be > checked during trouble shooting. Also use the dump function in test > cases. > > An example output: > > $ ovn-appctl -t ovn-controller debug/dump-lflow-conj-ids > Conjunction IDs allocations: > lflow: 4445d4a0-3ca3-4385-a677-c0199d65ea4d, start: 1145427104, n: 1 > lflow: 711b0d24-52bc-41a4-9af2-5cdea2d5167d, start: 1897598244, n: 1 > lflow: 6e712ecb-f863-4a0c-a7c4-367d59914d35, start: 1852911307, n: 1 > ---- > Total 3 IDs used. > > Suggested-by: Numan Siddique <[email protected]> > Signed-off-by: Han Zhou <[email protected]>
I think it's good to have this patch before we branch out. Acked-by: Numan Siddique <[email protected]> Numan > --- > controller/lflow-conj-ids.c | 29 +++++++++++++++++++++++++ > controller/lflow-conj-ids.h | 2 ++ > controller/ovn-controller.c | 15 +++++++++++++ > controller/test-lflow-conj-ids.c | 5 +++++ > tests/ovn-lflow-conj-ids.at | 37 ++++++++++++++++++++++++++++++++ > 5 files changed, 88 insertions(+) > > diff --git a/controller/lflow-conj-ids.c b/controller/lflow-conj-ids.c > index fc33c25e5..bfe63862a 100644 > --- a/controller/lflow-conj-ids.c > +++ b/controller/lflow-conj-ids.c > @@ -195,6 +195,35 @@ void lflow_conj_ids_clear(struct conj_ids *conj_ids) { > lflow_conj_ids_init(conj_ids); > } > > +void > +lflow_conj_ids_dump(struct conj_ids *conj_ids, struct ds *out_data) > +{ > + struct lflow_conj_node *lflow_conj; > + size_t count = 0; > + > + ds_put_cstr(out_data, "Conjunction IDs allocations:\n"); > + HMAP_FOR_EACH (lflow_conj, hmap_node, &conj_ids->lflow_conj_ids) { > + bool has_conflict = > + (lflow_conj->start_conj_id != lflow_conj->lflow_uuid.parts[0]); > + ds_put_format(out_data, "lflow: "UUID_FMT", start: %"PRIu32 > + ", n: %"PRIu32"%s\n", > + UUID_ARGS(&lflow_conj->lflow_uuid), > + lflow_conj->start_conj_id, > + lflow_conj->n_conjs, > + has_conflict ? " (*)" : ""); > + count += lflow_conj->n_conjs; > + } > + > + ds_put_cstr(out_data, "---\n"); > + ds_put_format(out_data, "Total %"PRIuSIZE" IDs used.\n", count); > + > + size_t allocated = hmap_count(&conj_ids->conj_id_allocations); > + if (count != allocated) { > + ds_put_format(out_data, "WARNING: mismatch - %"PRIuSIZE" > allocated\n", > + allocated); > + } > +} > + > /* Insert n_conjs conjuntion ids starting from start_conj_id into the > conj_ids, > * assuming the ids are confirmed to be available. */ > static void > diff --git a/controller/lflow-conj-ids.h b/controller/lflow-conj-ids.h > index d333fa8d5..6da0a612c 100644 > --- a/controller/lflow-conj-ids.h > +++ b/controller/lflow-conj-ids.h > @@ -17,6 +17,7 @@ > #ifndef LFLOW_CONJ_IDS_H > #define LFLOW_CONJ_IDS_H 1 > > +#include "openvswitch/dynamic-string.h" > #include "openvswitch/hmap.h" > #include "uuid.h" > > @@ -37,5 +38,6 @@ void lflow_conj_ids_free(struct conj_ids *, const struct > uuid *lflow_uuid); > void lflow_conj_ids_init(struct conj_ids *); > void lflow_conj_ids_destroy(struct conj_ids *); > void lflow_conj_ids_clear(struct conj_ids *); > +void lflow_conj_ids_dump(struct conj_ids *, struct ds *out_data); > > #endif /* controller/lflow-conj-ids.h */ > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 26593bc0d..5fc90a34a 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -89,6 +89,7 @@ static unixctl_cb_func debug_pause_execution; > static unixctl_cb_func debug_resume_execution; > static unixctl_cb_func debug_status_execution; > static unixctl_cb_func debug_dump_local_bindings; > +static unixctl_cb_func debug_dump_conj_ids; > static unixctl_cb_func lflow_cache_flush_cmd; > static unixctl_cb_func lflow_cache_show_stats_cmd; > static unixctl_cb_func debug_delay_nb_cfg_report; > @@ -3428,6 +3429,10 @@ main(int argc, char *argv[]) > debug_dump_local_bindings, > &runtime_data->lbinding_data); > > + unixctl_command_register("debug/dump-conj-ids", "", 0, 0, > + debug_dump_conj_ids, > + &lflow_output_data->conj_ids); > + > unsigned int ovs_cond_seqno = UINT_MAX; > unsigned int ovnsb_cond_seqno = UINT_MAX; > unsigned int ovnsb_expected_cond_seqno = UINT_MAX; > @@ -4256,3 +4261,13 @@ debug_dump_local_bindings(struct unixctl_conn *conn, > int argc OVS_UNUSED, > unixctl_command_reply(conn, ds_cstr(&binding_data)); > ds_destroy(&binding_data); > } > + > +static void > +debug_dump_conj_ids(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *conj_ids) > +{ > + struct ds conj_ids_dump = DS_EMPTY_INITIALIZER; > + lflow_conj_ids_dump(conj_ids, &conj_ids_dump); > + unixctl_command_reply(conn, ds_cstr(&conj_ids_dump)); > + ds_destroy(&conj_ids_dump); > +} > diff --git a/controller/test-lflow-conj-ids.c > b/controller/test-lflow-conj-ids.c > index 1273f9a4c..55eb3c7b6 100644 > --- a/controller/test-lflow-conj-ids.c > +++ b/controller/test-lflow-conj-ids.c > @@ -106,6 +106,11 @@ test_conj_ids_operations(struct ovs_cmdl_context *ctx) > goto done; > } > } > + struct ds conj_ids_dump = DS_EMPTY_INITIALIZER; > + lflow_conj_ids_dump(&conj_ids, &conj_ids_dump); > + printf("%s", ds_cstr(&conj_ids_dump)); > + ds_destroy(&conj_ids_dump); > + > done: > lflow_conj_ids_destroy(&conj_ids); > } > diff --git a/tests/ovn-lflow-conj-ids.at b/tests/ovn-lflow-conj-ids.at > index 818d67324..b5537c370 100644 > --- a/tests/ovn-lflow-conj-ids.at > +++ b/tests/ovn-lflow-conj-ids.at > @@ -14,6 +14,12 @@ AT_CHECK( > alloc(aaaaaaaa-1111-1111-1111-111111111111, 10): 0xaaaaaaaa > alloc(bbbbbbbb-1111-1111-1111-111111111111, 10): 0xbbbbbbbb > alloc(cccccccc-1111-1111-1111-111111111111, 10): 0xcccccccc > +Conjunction IDs allocations: > +lflow: cccccccc-1111-1111-1111-111111111111, start: 3435973836, n: 10 > +lflow: aaaaaaaa-1111-1111-1111-111111111111, start: 2863311530, n: 10 > +lflow: bbbbbbbb-1111-1111-1111-111111111111, start: 3149642683, n: 10 > +--- > +Total 30 IDs used. > ]) > > AT_CLEANUP > @@ -28,6 +34,11 @@ AT_CHECK( > [0], [dnl > alloc(aaaaaaaa-1111-1111-1111-111111111111, 1): 0xaaaaaaaa > alloc(aaaaaaaa-2222-1111-1111-111111111111, 1): 0xaaaaaaab > +Conjunction IDs allocations: > +lflow: aaaaaaaa-1111-1111-1111-111111111111, start: 2863311530, n: 1 > +lflow: aaaaaaaa-2222-1111-1111-111111111111, start: 2863311531, n: 1 (*) > +--- > +Total 2 IDs used. > ]) > > # Conflict of the different prefix but overlapping range, the second > allocation > @@ -44,6 +55,11 @@ alloc(aaaaaaaa-1111-1111-1111-111111111111, 16): 0xaaaaaaaa > alloc(aaaaaaab-1111-1111-1111-111111111111, 1): 0xaaaaaaba > free(aaaaaaaa-1111-1111-1111-111111111111) > alloc(aaaaaaab-2222-1111-1111-111111111111, 1): 0xaaaaaaab > +Conjunction IDs allocations: > +lflow: aaaaaaab-2222-1111-1111-111111111111, start: 2863311531, n: 1 > +lflow: aaaaaaab-1111-1111-1111-111111111111, start: 2863311546, n: 1 (*) > +--- > +Total 2 IDs used. > ]) > > # Conflict at the tail of the range. > @@ -54,6 +70,11 @@ AT_CHECK( > [0], [dnl > alloc(aaaaaaaa-1111-1111-1111-111111111111, 1): 0xaaaaaaaa > alloc(aaaaaaa0-1111-1111-1111-111111111111, 11): 0xaaaaaaab > +Conjunction IDs allocations: > +lflow: aaaaaaa0-1111-1111-1111-111111111111, start: 2863311531, n: 11 (*) > +lflow: aaaaaaaa-1111-1111-1111-111111111111, start: 2863311530, n: 1 > +--- > +Total 12 IDs used. > ]) > > # Realloc for the same lflow should get the same id, with the old allocations > @@ -67,6 +88,11 @@ AT_CHECK( > alloc(aaaaaaaa-1111-1111-1111-111111111111, 16): 0xaaaaaaaa > alloc(aaaaaaaa-1111-1111-1111-111111111111, 1): 0xaaaaaaaa > alloc(aaaaaaab-1111-1111-1111-111111111111, 1): 0xaaaaaaab > +Conjunction IDs allocations: > +lflow: aaaaaaaa-1111-1111-1111-111111111111, start: 2863311530, n: 1 > +lflow: aaaaaaab-1111-1111-1111-111111111111, start: 2863311531, n: 1 > +--- > +Total 2 IDs used. > ]) > > AT_CLEANUP > @@ -82,6 +108,10 @@ AT_CHECK( > alloc(ffffffff-1111-1111-1111-111111111111, 2): 0x1 > free(ffffffff-1111-1111-1111-111111111111) > alloc(00000000-2222-1111-1111-111111111111, 1): 0x1 > +Conjunction IDs allocations: > +lflow: 00000000-2222-1111-1111-111111111111, start: 1, n: 1 (*) > +--- > +Total 1 IDs used. > ]) > > AT_CLEANUP > @@ -99,6 +129,10 @@ alloc(00000001-1111-1111-1111-111111111111, 16): 0x1 > alloc_specified(0000000a-1111-1111-1111-111111111111, 0xa, 1): false > free(00000001-1111-1111-1111-111111111111) > alloc_specified(0000000a-1111-1111-1111-111111111111, 0xa, 1): true > +Conjunction IDs allocations: > +lflow: 0000000a-1111-1111-1111-111111111111, start: 10, n: 1 > +--- > +Total 1 IDs used. > ]) > > # alloc_specified for a range including 0 should always fail. > @@ -107,6 +141,9 @@ AT_CHECK( > alloc-specified fee1dead-1111-1111-1111-111111111111 0xffffffff 2], > [0], [dnl > alloc_specified(fee1dead-1111-1111-1111-111111111111, 0xffffffff, 2): false > +Conjunction IDs allocations: > +--- > +Total 0 IDs used. > ]) > > AT_CLEANUP > -- > 2.30.2 > > _______________________________________________ > 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
