On Tue, Jul 09, 2024 at 11:45:58AM GMT, Eelco Chaudron wrote:
> On 7 Jul 2024, at 22:09, Adrian Moreno wrote:
>
> > Add a command to dump statistics per exporter.
>
> Small nit below, rest looks good to me. If this is the only change in the
> next series you can add my ACK.
>
> //Eelco
>
> > Signed-off-by: Adrian Moreno <amore...@redhat.com>
> > ---
> > 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.
> >
> >
> > 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;
> > + }
> > +
> > + 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);
>
> I would make this the first definition, above dpif_lsample_create()
Ack.
>
> > 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
> > --
> > 2.45.2
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
