On Fri, Dec 3, 2021 at 7:15 AM Numan Siddique <[email protected]> wrote:
>
> 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
>
Thanks Numan. I applied it to main with a minor change of just renaming the
unix command from "dump-conj-ids" to "dump-lflow-conj-ids", exactly as what
the commit message has described. (In fact I renamed it before sending it
for review but forgot git add this part).
Han
> > ---
> > 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
