On 5 Jun 2024, at 22:23, 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.
Hi Adrian, Some small nits below. Cheers, Eelco > Signed-off-by: Adrian Moreno <[email protected]> > --- > 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 | 15 +++++++++++++++ > ofproto/ofproto-dpif.c | 1 + > 6 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c > index 2c0b5da89..0c71e354d 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 f13425575..2ce096161 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); > void dpif_lsample_unref(struct dpif_lsample *); > @@ -36,5 +37,8 @@ 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..508e5fcb2 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-mirror.h" > +#include "ofproto/ofproto-dpif-lsample.h" If you re-order you might as well put lsample before mirror ;) > #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 5bd215d31..d7648e02c 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -5986,6 +5986,21 @@ xlate_sample_action(struct xlate_ctx *ctx, > sizeof(os->obs_domain_id)); > ofpbuf_put(&emit_sample_args.cookie, &os->obs_point_id, > sizeof(os->obs_point_id)); > + One new line might be enough. > + > + if (ctx->xin->resubmit_stats) { > + dpif_lsample_credit_stats(lsample, > + os->collector_set_id, > + ctx->xin->resubmit_stats); > + } I would add a new line here. > + 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; > + } > + Remove new line. > } > } > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 48167751f..e5a7f0fa5 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -5157,6 +5157,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.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
