On 10 Jul 2024, at 15:06, Ilya Maximets wrote:
> On 7/7/24 22:09, Adrian Moreno wrote: >> Add a command to dump statistics per exporter. >> >> Signed-off-by: Adrian Moreno <[email protected]> >> --- >> NEWS | 2 + >> ofproto/ofproto-dpif-lsample.c | 111 +++++++++++++++++++++++++++++++++ >> ofproto/ofproto-dpif-lsample.h | 1 + >> ofproto/ofproto-dpif.c | 1 + >> 4 files changed, 115 insertions(+) >> >> diff --git a/NEWS b/NEWS >> index 15faf9fc3..1c53badea 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -20,6 +20,8 @@ Post-v3.3.0 >> allows samples to be emitted locally (instead of via IPFIX) in a >> datapath-specific manner. The Linux kernel datapath is the first to >> support this feature by using the new datapath "psample" action. >> + A new unixctl command 'lsample/show' shows packet and bytes statistics >> + per local sample exporter. > > Maybe add this as a separate bullet point. > >> >> >> v3.3.0 - 16 Feb 2024 >> diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c >> index 171129d5b..82a87c27d 100644 >> --- a/ofproto/ofproto-dpif-lsample.c >> +++ b/ofproto/ofproto-dpif-lsample.c >> @@ -21,7 +21,10 @@ >> #include "dpif.h" >> #include "hash.h" >> #include "ofproto.h" >> +#include "ofproto-dpif.h" >> +#include "openvswitch/dynamic-string.h" >> #include "openvswitch/thread.h" >> +#include "unixctl.h" >> >> /* Dpif local sampling. >> * >> @@ -219,3 +222,111 @@ dpif_lsample_unref(struct dpif_lsample *lsample) >> dpif_lsample_destroy(lsample); >> } >> } >> + >> +static int >> +comp_exporter_collector_id(const void *a_, const void *b_) >> +{ >> + const struct lsample_exporter_node *a, *b; >> + >> + a = *(struct lsample_exporter_node **) a_; >> + b = *(struct lsample_exporter_node **) b_; >> + >> + if (a->exporter.options.collector_set_id > >> + b->exporter.options.collector_set_id) { >> + return 1; >> + } >> + if (a->exporter.options.collector_set_id < >> + b->exporter.options.collector_set_id) { >> + return -1; >> + } >> + return 0; >> +} >> + >> +static void >> +lsample_exporter_list(struct dpif_lsample *lsample, >> + struct lsample_exporter_node ***list, >> + size_t *num_exporters) >> +{ >> + struct lsample_exporter_node **exporter_list; >> + struct lsample_exporter_node *node; >> + size_t k = 0, n; >> + >> + n = cmap_count(&lsample->exporters); >> + >> + exporter_list = xcalloc(n, sizeof *exporter_list); >> + >> + CMAP_FOR_EACH (node, node, &lsample->exporters) { >> + if (k >= n) { >> + break; >> + } >> + exporter_list[k++] = node; > > There is no guarantee that cmap didn't change between getting the > number of exporters and iterating over them. Need to either take > a lock or grow the array on demand. I initially thought the same, but it’s covered by the break above. Thinking about it again, we might actually not show entries that were displayed before, due to a new one being earlier in the map. So I guess we should probably grow the exporter_list (rather than hold the lock). >> + } >> + >> + qsort(exporter_list, k, sizeof *exporter_list, >> comp_exporter_collector_id); >> + >> + *list = exporter_list; >> + *num_exporters = k; >> +} >> + >> +static void >> +lsample_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED, >> + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) >> +{ >> + struct lsample_exporter_node **node_list = NULL; >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + const struct ofproto_dpif *ofproto; >> + size_t i, num; >> + >> + ofproto = ofproto_dpif_lookup_by_name(argv[1]); >> + if (!ofproto) { >> + unixctl_command_reply_error(conn, "no such bridge"); >> + return; >> + } >> + >> + if (!ofproto->lsample) { >> + unixctl_command_reply_error(conn, >> + "no local sampling exporters >> configured"); >> + return; >> + } >> + >> + ds_put_format(&ds, "Local sample statistics for bridge \"%s\":\n", >> + argv[1]); >> + >> + lsample_exporter_list(ofproto->lsample, &node_list, &num); >> + >> + for (i = 0; i < num; i++) { >> + uint64_t n_bytes; >> + uint64_t n_packets; >> + >> + struct lsample_exporter_node *node = node_list[i]; >> + >> + atomic_read_relaxed(&node->exporter.n_packets, &n_packets); >> + atomic_read_relaxed(&node->exporter.n_bytes, &n_bytes); >> + >> + if (i) { >> + ds_put_cstr(&ds, "\n"); >> + } >> + >> + ds_put_format(&ds, "Collector Set ID: %"PRIu32":\n", >> + node->exporter.options.collector_set_id); >> + ds_put_format(&ds, " Group ID : %"PRIu32"\n", >> + node->exporter.options.group_id); >> + ds_put_format(&ds, " Total packets: %"PRIu64"\n", n_packets); >> + ds_put_format(&ds, " Total bytes : %"PRIu64"\n", n_bytes); >> + } >> + >> + free(node_list); >> + unixctl_command_reply(conn, ds_cstr(&ds)); >> + ds_destroy(&ds); >> +} >> + >> +void dpif_lsample_init(void) >> +{ >> + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; >> + >> + if (ovsthread_once_start(&once)) { >> + unixctl_command_register("lsample/show", "bridge", 1, 1, >> + lsample_unixctl_show, NULL); >> + ovsthread_once_done(&once); >> + } >> +} >> diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h >> index 2666e5478..692ac26e6 100644 >> --- a/ofproto/ofproto-dpif-lsample.h >> +++ b/ofproto/ofproto-dpif-lsample.h >> @@ -38,6 +38,7 @@ bool dpif_lsample_set_options(struct dpif_lsample *, >> bool dpif_lsample_get_group_id(struct dpif_lsample *, >> uint32_t collector_set_id, >> uint32_t *group_id); >> +void dpif_lsample_init(void); >> >> void dpif_lsample_credit_stats(struct dpif_lsample *, >> uint32_t collector_set_id, >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >> index 28c564846..ee8fbaa9a 100644 >> --- a/ofproto/ofproto-dpif.c >> +++ b/ofproto/ofproto-dpif.c >> @@ -286,6 +286,7 @@ init(const struct shash *iface_hints) >> ofproto_unixctl_init(); >> ofproto_dpif_trace_init(); >> udpif_init(); >> + dpif_lsample_init(); >> } >> >> static void _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
