On Tue, Jul 09, 2024 at 11:45:54AM GMT, Eelco Chaudron wrote:
> On 7 Jul 2024, at 22:08, Adrian Moreno wrote:
>
> > Add a cache entry type for local sample objects.
> > Store both the dpif_lsample reference and the collector_set_id so we can
> > quickly find the particular exporter.
> >
> > Using this mechanism, account for packet and byte statistics.
>
> 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>
> > ---
> > ofproto/ofproto-dpif-lsample.c | 18 ++++++++++++++++++
> > ofproto/ofproto-dpif-lsample.h | 4 ++++
> > ofproto/ofproto-dpif-xlate-cache.c | 11 ++++++++++-
> > ofproto/ofproto-dpif-xlate-cache.h | 6 ++++++
> > ofproto/ofproto-dpif-xlate.c | 14 ++++++++++++++
> > ofproto/ofproto-dpif.c | 1 +
> > 6 files changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c
> > index 534ad96f0..171129d5b 100644
> > --- a/ofproto/ofproto-dpif-lsample.c
> > +++ b/ofproto/ofproto-dpif-lsample.c
> > @@ -18,6 +18,7 @@
> > #include "ofproto-dpif-lsample.h"
> >
> > #include "cmap.h"
> > +#include "dpif.h"
> > #include "hash.h"
> > #include "ofproto.h"
> > #include "openvswitch/thread.h"
> > @@ -44,6 +45,8 @@ struct dpif_lsample {
> >
> > struct lsample_exporter {
> > struct ofproto_lsample_options options;
> > + atomic_uint64_t n_packets;
> > + atomic_uint64_t n_bytes;
> > };
> >
> > struct lsample_exporter_node {
> > @@ -156,6 +159,21 @@ dpif_lsample_get_group_id(struct dpif_lsample *ps,
> > uint32_t collector_set_id,
> > return found;
> > }
> >
> > +void
> > +dpif_lsample_credit_stats(struct dpif_lsample *lsample,
> > + uint32_t collector_set_id,
> > + const struct dpif_flow_stats *stats)
> > +{
> > + struct lsample_exporter_node *node;
> > + uint64_t orig;
> > +
> > + node = dpif_lsample_find_exporter_node(lsample, collector_set_id);
> > + if (node) {
> > + atomic_add_relaxed(&node->exporter.n_packets, stats->n_packets,
> > &orig);
> > + atomic_add_relaxed(&node->exporter.n_bytes, stats->n_bytes, &orig);
> > + }
> > +}
> > +
> > struct dpif_lsample *
> > dpif_lsample_create(void)
> > {
> > diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h
> > index 9c1026551..2666e5478 100644
> > --- a/ofproto/ofproto-dpif-lsample.h
> > +++ b/ofproto/ofproto-dpif-lsample.h
> > @@ -23,6 +23,7 @@
> >
> > struct dpif_lsample;
> > struct ofproto_lsample_options;
> > +struct dpif_flow_stats;
> >
> > struct dpif_lsample *dpif_lsample_create(void);
> >
> > @@ -38,4 +39,7 @@ bool dpif_lsample_get_group_id(struct dpif_lsample *,
> > uint32_t collector_set_id,
> > uint32_t *group_id);
> >
> > +void dpif_lsample_credit_stats(struct dpif_lsample *,
> > + uint32_t collector_set_id,
> > + const struct dpif_flow_stats *);
> > #endif /* OFPROTO_DPIF_LSAMPLE_H */
> > diff --git a/ofproto/ofproto-dpif-xlate-cache.c
> > b/ofproto/ofproto-dpif-xlate-cache.c
> > index 2e1fcb3a6..73d79bfc7 100644
> > --- a/ofproto/ofproto-dpif-xlate-cache.c
> > +++ b/ofproto/ofproto-dpif-xlate-cache.c
> > @@ -35,9 +35,10 @@
> > #include "learn.h"
> > #include "mac-learning.h"
> > #include "netdev-vport.h"
> > +#include "ofproto/ofproto-dpif.h"
> > +#include "ofproto/ofproto-dpif-lsample.h"
> > #include "ofproto/ofproto-dpif-mirror.h"
> > #include "ofproto/ofproto-dpif-xlate.h"
> > -#include "ofproto/ofproto-dpif.h"
> > #include "ofproto/ofproto-provider.h"
> > #include "openvswitch/dynamic-string.h"
> > #include "openvswitch/vlog.h"
> > @@ -162,6 +163,11 @@ xlate_push_stats_entry(struct xc_entry *entry,
> > }
> >
> > break;
> > + case XC_LSAMPLE:
> > + dpif_lsample_credit_stats(entry->lsample.lsample,
> > + entry->lsample.collector_set_id,
> > + stats);
> > + break;
> > default:
> > OVS_NOT_REACHED();
> > }
> > @@ -245,6 +251,9 @@ xlate_cache_clear_entry(struct xc_entry *entry)
> > break;
> > case XC_TUNNEL_HEADER:
> > break;
> > + case XC_LSAMPLE:
> > + dpif_lsample_unref(entry->lsample.lsample);
> > + break;
> > default:
> > OVS_NOT_REACHED();
> > }
> > diff --git a/ofproto/ofproto-dpif-xlate-cache.h
> > b/ofproto/ofproto-dpif-xlate-cache.h
> > index 0fc6d2ea6..df8115419 100644
> > --- a/ofproto/ofproto-dpif-xlate-cache.h
> > +++ b/ofproto/ofproto-dpif-xlate-cache.h
> > @@ -29,6 +29,7 @@
> > struct bfd;
> > struct bond;
> > struct dpif_flow_stats;
> > +struct dpif_lsample;
> > struct flow;
> > struct group_dpif;
> > struct mbridge;
> > @@ -53,6 +54,7 @@ enum xc_type {
> > XC_GROUP,
> > XC_TNL_NEIGH,
> > XC_TUNNEL_HEADER,
> > + XC_LSAMPLE,
> > };
> >
> > /* xlate_cache entries hold enough information to perform the side effects
> > of
> > @@ -126,6 +128,10 @@ struct xc_entry {
> > } operation;
> > uint16_t hdr_size;
> > } tunnel_hdr;
> > + struct {
> > + struct dpif_lsample *lsample;
> > + uint32_t collector_set_id;
> > + } lsample;
> > };
> > };
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 5e8113d5e..b9546dc5b 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -2361,6 +2361,7 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle
> > *xbundle,
> > ctx->xin->resubmit_stats->n_packets,
> > ctx->xin->resubmit_stats->n_bytes);
> > }
> > +
>
> Either remove this new new-line. Or insert in the same code below.
>
Ack.
> > if (ctx->xin->xcache) {
> > struct xc_entry *entry;
> >
> > @@ -6013,6 +6014,19 @@ xlate_sample_action(struct xlate_ctx *ctx,
> > *data = htonl(os->obs_point_id);
> >
> > compose_args.psample = &psample;
> > +
> > + if (ctx->xin->resubmit_stats) {
> > + dpif_lsample_credit_stats(lsample,
> > + os->collector_set_id,
> > + ctx->xin->resubmit_stats);
> > + }
> > + if (ctx->xin->xcache) {
> > + struct xc_entry *entry;
> > +
> > + entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LSAMPLE);
> > + entry->lsample.lsample = dpif_lsample_ref(lsample);
> > + entry->lsample.collector_set_id = os->collector_set_id;
> > + }
> > }
> >
> > if (!compose_args.userspace && !compose_args.psample) {
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 006f67b01..28c564846 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -5156,6 +5156,7 @@ ofproto_dpif_xcache_execute(struct ofproto_dpif
> > *ofproto,
> > case XC_GROUP:
> > case XC_TNL_NEIGH:
> > case XC_TUNNEL_HEADER:
> > + case XC_LSAMPLE:
> > xlate_push_stats_entry(entry, stats, false);
> > break;
> > default:
> > --
> > 2.45.2
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
