| Message ID | 20240424195337.3596657-11-amorenoz@redhat.com |
|---|---|
| State | RFC |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | Add psample support to NXAST_SAMPLE action. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/intel-ovs-compilation | success | test: success |
| ovsrobot/apply-robot | success | apply and check: success |
On 24 Apr 2024, at 21:53, Adrian Moreno wrote: Maybe add a small description to this patch. > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > ofproto/ofproto-dpif-psample.c | 59 ++++++++++++++++++++++++++++++++++ > ofproto/ofproto-dpif-psample.h | 1 + > ofproto/ofproto-dpif.c | 1 + > 3 files changed, 61 insertions(+) > > diff --git a/ofproto/ofproto-dpif-psample.c b/ofproto/ofproto-dpif-psample.c > index ea4926eb2..2d73a4ded 100644 > --- a/ofproto/ofproto-dpif-psample.c > +++ b/ofproto/ofproto-dpif-psample.c > @@ -20,9 +20,12 @@ > #include "dpif.h" > #include "hash.h" > #include "ofproto.h" > +#include "ofproto-dpif.h" > +#include "openvswitch/dynamic-string.h" > #include "openvswitch/hmap.h" > #include "openvswitch/thread.h" > #include "openvswitch/vlog.h" > +#include "unixctl.h" > > VLOG_DEFINE_THIS_MODULE(psample); > > @@ -205,3 +208,59 @@ dpif_psample_unref(struct dpif_psample *ps) OVS_EXCLUDED(mutex) > dpif_psample_destroy(ps); > } > } > + > +/* Unix commands. */ > +static void > +psample_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) > +{ > + struct psample_exporter_map_node *node; > + struct ds ds = DS_EMPTY_INITIALIZER; > + const struct ofproto_dpif *ofproto; > + bool first = true; > + > + ofproto = ofproto_dpif_lookup_by_name(argv[1]); > + if (!ofproto) { > + unixctl_command_reply_error(conn, "no such bridge"); > + return; > + } > + > + if (!ofproto->psample) { > + unixctl_command_reply_error(conn, "no psample exporters configured"); > + return; > + } > + > + ds_put_format(&ds, "Psample statistics for bridge \"%s\":\n", argv[1]); > + > + ovs_mutex_lock(&mutex); We hold the global mutex for quite a long time, so we need some other solution. Also, do we need some form of sorted output? > + HMAP_FOR_EACH (node, node, &ofproto->psample->exporters_map) { > + if (!first) { > + ds_put_cstr(&ds, "\n"); > + } else { > + first = false; > + } > + > + ds_put_format(&ds, "- Collector Set ID: %"PRIu32"\n", > + node->exporter.collector_set_id); > + ds_put_format(&ds, " Psample Group ID: %"PRIu32"\n", > + node->exporter.group_id); > + ds_put_format(&ds, " Total number of bytes: %"PRIu64"\n", > + node->exporter.n_bytes); > + ds_put_format(&ds, " Total number of packets: %"PRIu64"\n", > + node->exporter.n_packets); > + } > + ovs_mutex_unlock(&mutex); > + > + unixctl_command_reply(conn, ds_cstr(&ds)); > + ds_destroy(&ds); > +} > + > +void dpif_psample_init(void) > +{ > + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; New line > + if (ovsthread_once_start(&once)) { > + unixctl_command_register("psample/show", "bridge", 1, 1, > + psample_unixctl_show, NULL); > + ovsthread_once_done(&once); > + } > +} > diff --git a/ofproto/ofproto-dpif-psample.h b/ofproto/ofproto-dpif-psample.h > index 763fbd30b..f264ad4c2 100644 > --- a/ofproto/ofproto-dpif-psample.h > +++ b/ofproto/ofproto-dpif-psample.h > @@ -34,5 +34,6 @@ bool dpif_psample_get_group_id(struct dpif_psample *, uint32_t, uint32_t *); > > void dpif_psample_credit_stats(struct dpif_psample *, uint32_t, > const struct dpif_flow_stats *); > +void dpif_psample_init(void); > > #endif // OFPROTO_DPIF_PSAMPLE_H > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index f1efdd482..ebb399307 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -287,6 +287,7 @@ init(const struct shash *iface_hints) > ofproto_unixctl_init(); > ofproto_dpif_trace_init(); > udpif_init(); > + dpif_psample_init(); > } > > static void > -- > 2.44.0
On 5/10/24 12:06 PM, Eelco Chaudron wrote: > On 24 Apr 2024, at 21:53, Adrian Moreno wrote: > > Maybe add a small description to this patch. > >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- >> ofproto/ofproto-dpif-psample.c | 59 ++++++++++++++++++++++++++++++++++ >> ofproto/ofproto-dpif-psample.h | 1 + >> ofproto/ofproto-dpif.c | 1 + >> 3 files changed, 61 insertions(+) >> >> diff --git a/ofproto/ofproto-dpif-psample.c b/ofproto/ofproto-dpif-psample.c >> index ea4926eb2..2d73a4ded 100644 >> --- a/ofproto/ofproto-dpif-psample.c >> +++ b/ofproto/ofproto-dpif-psample.c >> @@ -20,9 +20,12 @@ >> #include "dpif.h" >> #include "hash.h" >> #include "ofproto.h" >> +#include "ofproto-dpif.h" >> +#include "openvswitch/dynamic-string.h" >> #include "openvswitch/hmap.h" >> #include "openvswitch/thread.h" >> #include "openvswitch/vlog.h" >> +#include "unixctl.h" >> >> VLOG_DEFINE_THIS_MODULE(psample); >> >> @@ -205,3 +208,59 @@ dpif_psample_unref(struct dpif_psample *ps) OVS_EXCLUDED(mutex) >> dpif_psample_destroy(ps); >> } >> } >> + >> +/* Unix commands. */ >> +static void >> +psample_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED, >> + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) >> +{ >> + struct psample_exporter_map_node *node; >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + const struct ofproto_dpif *ofproto; >> + bool first = true; >> + >> + ofproto = ofproto_dpif_lookup_by_name(argv[1]); >> + if (!ofproto) { >> + unixctl_command_reply_error(conn, "no such bridge"); >> + return; >> + } >> + >> + if (!ofproto->psample) { >> + unixctl_command_reply_error(conn, "no psample exporters configured"); >> + return; >> + } >> + >> + ds_put_format(&ds, "Psample statistics for bridge \"%s\":\n", argv[1]); >> + >> + ovs_mutex_lock(&mutex); > > We hold the global mutex for quite a long time, so we need some other solution. I agree. I'll see how I can reduce the locking. > Also, do we need some form of sorted output? > Good point, I'll sort the outputs. >> + HMAP_FOR_EACH (node, node, &ofproto->psample->exporters_map) { >> + if (!first) { >> + ds_put_cstr(&ds, "\n"); >> + } else { >> + first = false; >> + } >> + >> + ds_put_format(&ds, "- Collector Set ID: %"PRIu32"\n", >> + node->exporter.collector_set_id); >> + ds_put_format(&ds, " Psample Group ID: %"PRIu32"\n", >> + node->exporter.group_id); >> + ds_put_format(&ds, " Total number of bytes: %"PRIu64"\n", >> + node->exporter.n_bytes); >> + ds_put_format(&ds, " Total number of packets: %"PRIu64"\n", >> + node->exporter.n_packets); >> + } >> + ovs_mutex_unlock(&mutex); >> + >> + unixctl_command_reply(conn, ds_cstr(&ds)); >> + ds_destroy(&ds); >> +} >> + >> +void dpif_psample_init(void) >> +{ >> + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; > > New line > >> + if (ovsthread_once_start(&once)) { >> + unixctl_command_register("psample/show", "bridge", 1, 1, >> + psample_unixctl_show, NULL); >> + ovsthread_once_done(&once); >> + } >> +} >> diff --git a/ofproto/ofproto-dpif-psample.h b/ofproto/ofproto-dpif-psample.h >> index 763fbd30b..f264ad4c2 100644 >> --- a/ofproto/ofproto-dpif-psample.h >> +++ b/ofproto/ofproto-dpif-psample.h >> @@ -34,5 +34,6 @@ bool dpif_psample_get_group_id(struct dpif_psample *, uint32_t, uint32_t *); >> >> void dpif_psample_credit_stats(struct dpif_psample *, uint32_t, >> const struct dpif_flow_stats *); >> +void dpif_psample_init(void); >> >> #endif // OFPROTO_DPIF_PSAMPLE_H >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >> index f1efdd482..ebb399307 100644 >> --- a/ofproto/ofproto-dpif.c >> +++ b/ofproto/ofproto-dpif.c >> @@ -287,6 +287,7 @@ init(const struct shash *iface_hints) >> ofproto_unixctl_init(); >> ofproto_dpif_trace_init(); >> udpif_init(); >> + dpif_psample_init(); >> } >> >> static void >> -- >> 2.44.0 >
diff --git a/ofproto/ofproto-dpif-psample.c b/ofproto/ofproto-dpif-psample.c index ea4926eb2..2d73a4ded 100644 --- a/ofproto/ofproto-dpif-psample.c +++ b/ofproto/ofproto-dpif-psample.c @@ -20,9 +20,12 @@ #include "dpif.h" #include "hash.h" #include "ofproto.h" +#include "ofproto-dpif.h" +#include "openvswitch/dynamic-string.h" #include "openvswitch/hmap.h" #include "openvswitch/thread.h" #include "openvswitch/vlog.h" +#include "unixctl.h" VLOG_DEFINE_THIS_MODULE(psample); @@ -205,3 +208,59 @@ dpif_psample_unref(struct dpif_psample *ps) OVS_EXCLUDED(mutex) dpif_psample_destroy(ps); } } + +/* Unix commands. */ +static void +psample_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED, + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED) +{ + struct psample_exporter_map_node *node; + struct ds ds = DS_EMPTY_INITIALIZER; + const struct ofproto_dpif *ofproto; + bool first = true; + + ofproto = ofproto_dpif_lookup_by_name(argv[1]); + if (!ofproto) { + unixctl_command_reply_error(conn, "no such bridge"); + return; + } + + if (!ofproto->psample) { + unixctl_command_reply_error(conn, "no psample exporters configured"); + return; + } + + ds_put_format(&ds, "Psample statistics for bridge \"%s\":\n", argv[1]); + + ovs_mutex_lock(&mutex); + HMAP_FOR_EACH (node, node, &ofproto->psample->exporters_map) { + if (!first) { + ds_put_cstr(&ds, "\n"); + } else { + first = false; + } + + ds_put_format(&ds, "- Collector Set ID: %"PRIu32"\n", + node->exporter.collector_set_id); + ds_put_format(&ds, " Psample Group ID: %"PRIu32"\n", + node->exporter.group_id); + ds_put_format(&ds, " Total number of bytes: %"PRIu64"\n", + node->exporter.n_bytes); + ds_put_format(&ds, " Total number of packets: %"PRIu64"\n", + node->exporter.n_packets); + } + ovs_mutex_unlock(&mutex); + + unixctl_command_reply(conn, ds_cstr(&ds)); + ds_destroy(&ds); +} + +void dpif_psample_init(void) +{ + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; + if (ovsthread_once_start(&once)) { + unixctl_command_register("psample/show", "bridge", 1, 1, + psample_unixctl_show, NULL); + ovsthread_once_done(&once); + } +} diff --git a/ofproto/ofproto-dpif-psample.h b/ofproto/ofproto-dpif-psample.h index 763fbd30b..f264ad4c2 100644 --- a/ofproto/ofproto-dpif-psample.h +++ b/ofproto/ofproto-dpif-psample.h @@ -34,5 +34,6 @@ bool dpif_psample_get_group_id(struct dpif_psample *, uint32_t, uint32_t *); void dpif_psample_credit_stats(struct dpif_psample *, uint32_t, const struct dpif_flow_stats *); +void dpif_psample_init(void); #endif // OFPROTO_DPIF_PSAMPLE_H diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index f1efdd482..ebb399307 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -287,6 +287,7 @@ init(const struct shash *iface_hints) ofproto_unixctl_init(); ofproto_dpif_trace_init(); udpif_init(); + dpif_psample_init(); } static void
Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- ofproto/ofproto-dpif-psample.c | 59 ++++++++++++++++++++++++++++++++++ ofproto/ofproto-dpif-psample.h | 1 + ofproto/ofproto-dpif.c | 1 + 3 files changed, 61 insertions(+)