On 7/12/24 19:06, Adrian Moreno wrote: > Add a new resource in ofproto-dpif and the corresponding API in > ofproto_provider.h to represent and local sampling configuration. > > Acked-by: Eelco Chaudron <[email protected]> > Signed-off-by: Adrian Moreno <[email protected]> > --- > ofproto/automake.mk | 2 + > ofproto/ofproto-dpif-lsample.c | 187 +++++++++++++++++++++++++++++++++ > ofproto/ofproto-dpif-lsample.h | 35 ++++++ > ofproto/ofproto-dpif.c | 38 +++++++ > ofproto/ofproto-dpif.h | 1 + > ofproto/ofproto-provider.h | 9 ++ > ofproto/ofproto.c | 12 +++ > ofproto/ofproto.h | 8 ++ > 8 files changed, 292 insertions(+) > create mode 100644 ofproto/ofproto-dpif-lsample.c > create mode 100644 ofproto/ofproto-dpif-lsample.h
A couple nits inline. > > diff --git a/ofproto/automake.mk b/ofproto/automake.mk > index 7c08b563b..cb1361b8a 100644 > --- a/ofproto/automake.mk > +++ b/ofproto/automake.mk > @@ -30,6 +30,8 @@ ofproto_libofproto_la_SOURCES = \ > ofproto/ofproto-dpif.h \ > ofproto/ofproto-dpif-ipfix.c \ > ofproto/ofproto-dpif-ipfix.h \ > + ofproto/ofproto-dpif-lsample.c \ > + ofproto/ofproto-dpif-lsample.h \ > ofproto/ofproto-dpif-mirror.c \ > ofproto/ofproto-dpif-mirror.h \ > ofproto/ofproto-dpif-monitor.c \ > diff --git a/ofproto/ofproto-dpif-lsample.c b/ofproto/ofproto-dpif-lsample.c > new file mode 100644 > index 000000000..a2b2e8059 > --- /dev/null > +++ b/ofproto/ofproto-dpif-lsample.c > @@ -0,0 +1,187 @@ > +/* > + * Copyright (c) 2024 Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > +#include "ofproto-dpif-lsample.h" > + > +#include "cmap.h" > +#include "hash.h" > +#include "ofproto.h" > +#include "openvswitch/thread.h" > + > +/* Dpif local sampling. > + * > + * Thread safety: dpif_lsample allows lockless concurrent reads of local > + * sampling exporters as long as the following restrictions are met: > + * 1) While the last reference is being dropped, i.e: a thread is calling > + * "dpif_lsample_unref" on the last reference, other threads cannot call > + * "dpif_lsample_ref". > + * 2) Threads do not quiese while holding references to internal > + * lsample_exporter objects. > + */ > + > +struct dpif_lsample { > + struct cmap exporters; /* Contains lsample_exporter_node > instances > + * indexed by collector_set_id. */ > + struct ovs_mutex mutex; /* Protects concurrent insertion/deletion > + * of exporters. */ > + This empty line not needed here. > + struct ovs_refcount ref_cnt; /* Controls references to this instance. > */ > +}; > + > +struct lsample_exporter { > + struct ofproto_lsample_options options; > +}; > + > +struct lsample_exporter_node { > + struct cmap_node node; /* In dpif_lsample->exporters. */ > + struct lsample_exporter exporter; > +}; > + > +static void > +dpif_lsample_delete_exporter(struct dpif_lsample *lsample, > + struct lsample_exporter_node *node) > +{ > + ovs_mutex_lock(&lsample->mutex); > + cmap_remove(&lsample->exporters, &node->node, > + hash_int(node->exporter.options.collector_set_id, 0)); > + ovs_mutex_unlock(&lsample->mutex); > + > + ovsrcu_postpone(free, node); > +} > + > +/* Adds an exporter with the provided options which are copied. */ > +static struct lsample_exporter_node * > +dpif_lsample_add_exporter(struct dpif_lsample *lsample, > + const struct ofproto_lsample_options *options) > +{ > + struct lsample_exporter_node *node; > + > + node = xzalloc(sizeof *node); > + node->exporter.options = *options; > + > + ovs_mutex_lock(&lsample->mutex); > + cmap_insert(&lsample->exporters, &node->node, > + hash_int(options->collector_set_id, 0)); > + ovs_mutex_unlock(&lsample->mutex); > + > + return node; > +} > + > +static struct lsample_exporter_node * > +dpif_lsample_find_exporter_node(const struct dpif_lsample *lsample, > + const uint32_t collector_set_id) > +{ > + struct lsample_exporter_node *node; > + > + CMAP_FOR_EACH_WITH_HASH (node, node, > + hash_int(collector_set_id, 0), > + &lsample->exporters) { Indentation is not correct. And can actually be written in just 2 lines. > + if (node->exporter.options.collector_set_id == collector_set_id) { > + return node; > + } > + } > + return NULL; > +} > + > +/* Sets the lsample configuration and returns true if the configuration > + * has changed. */ > +bool > +dpif_lsample_set_options(struct dpif_lsample *lsample, > + const struct ofproto_lsample_options *options, > + size_t n_options) > +{ > + const struct ofproto_lsample_options *opt; > + struct lsample_exporter_node *node; > + bool changed = false; > + int i; > + > + for (i = 0; i < n_options; i++) { > + opt = &options[i]; > + node = dpif_lsample_find_exporter_node(lsample, > + opt->collector_set_id); > + if (!node) { > + dpif_lsample_add_exporter(lsample, opt); > + changed = true; > + } else if (memcmp(&node->exporter.options, opt, sizeof(*opt))) { Don't parenthesize the argument of the sizeof. > + dpif_lsample_delete_exporter(lsample, node); > + dpif_lsample_add_exporter(lsample, opt); > + changed = true; > + } > + } > + > + /* Delete exporters that have been removed. */ > + CMAP_FOR_EACH (node, node, &lsample->exporters) { > + for (i = 0; i < n_options; i++) { > + if (node->exporter.options.collector_set_id > + == options[i].collector_set_id) { > + break; > + } > + } > + if (i == n_options) { > + dpif_lsample_delete_exporter(lsample, node); > + changed = true; > + } > + } > + > + return changed; > +} > + > +struct dpif_lsample * > +dpif_lsample_create(void) > +{ > + struct dpif_lsample *lsample; > + > + lsample = xzalloc(sizeof *lsample); > + cmap_init(&lsample->exporters); > + ovs_mutex_init(&lsample->mutex); > + ovs_refcount_init(&lsample->ref_cnt); > + > + return lsample; > +} > + > +static void > +dpif_lsample_destroy(struct dpif_lsample *lsample) > +{ > + if (lsample) { > + struct lsample_exporter_node *node; > + > + CMAP_FOR_EACH (node, node, &lsample->exporters) { > + dpif_lsample_delete_exporter(lsample, node); > + } > + cmap_destroy(&lsample->exporters); > + free(lsample); > + } > +} > + > +struct dpif_lsample * > +dpif_lsample_ref(const struct dpif_lsample *lsample_) > +{ > + struct dpif_lsample *lsample = CONST_CAST(struct dpif_lsample *, > lsample_); > + > + if (lsample) { > + ovs_refcount_ref(&lsample->ref_cnt); > + } > + return lsample; > +} > + > +void > +dpif_lsample_unref(struct dpif_lsample *lsample) > +{ > + if (lsample && ovs_refcount_unref_relaxed(&lsample->ref_cnt) == 1) { > + dpif_lsample_destroy(lsample); > + } > +} > diff --git a/ofproto/ofproto-dpif-lsample.h b/ofproto/ofproto-dpif-lsample.h > new file mode 100644 > index 000000000..a491c137d > --- /dev/null > +++ b/ofproto/ofproto-dpif-lsample.h > @@ -0,0 +1,35 @@ > +/* > + * Copyright (c) 2024 Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#ifndef OFPROTO_DPIF_LSAMPLE_H > +#define OFPROTO_DPIF_LSAMPLE_H 1 > + > +#include <stdbool.h> > +#include <stdlib.h> > + > +struct dpif_lsample; > +struct ofproto_lsample_options; > + > +struct dpif_lsample *dpif_lsample_create(void); > + > +struct dpif_lsample *dpif_lsample_ref(const struct dpif_lsample *); > +void dpif_lsample_unref(struct dpif_lsample *); > + > +bool dpif_lsample_set_options(struct dpif_lsample *, > + const struct ofproto_lsample_options *, > + size_t n_opts); > + > +#endif /* OFPROTO_DPIF_LSAMPLE_H */ > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index c2d2fab6f..59b9a5252 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -50,6 +50,7 @@ > #include "ofproto-dpif-sflow.h" > #include "ofproto-dpif-trace.h" > #include "ofproto-dpif-upcall.h" > +#include "ofproto-dpif-lsample.h" > #include "ofproto-dpif-xlate.h" > #include "ofproto-dpif-xlate-cache.h" > #include "openvswitch/ofp-actions.h" > @@ -1957,6 +1958,7 @@ destruct(struct ofproto *ofproto_, bool del) > netflow_unref(ofproto->netflow); > dpif_sflow_unref(ofproto->sflow); > dpif_ipfix_unref(ofproto->ipfix); > + dpif_lsample_unref(ofproto->lsample); > hmap_destroy(&ofproto->bundles); > mac_learning_unref(ofproto->ml); > mcast_snooping_unref(ofproto->ms); > @@ -2516,6 +2518,41 @@ get_ipfix_stats(const struct ofproto *ofproto_, > return dpif_ipfix_get_stats(di, bridge_ipfix, replies); > } > > +static int > +set_local_sample(struct ofproto *ofproto_, > + const struct ofproto_lsample_options *options, > + size_t n_opts) > +{ > + struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); > + struct dpif_lsample *lsample = ofproto->lsample; > + bool changed = false; > + > + if (!ofproto->backer->rt_support.psample) { > + return EOPNOTSUPP; > + } > + > + if (n_opts && !lsample) { > + lsample = ofproto->lsample = dpif_lsample_create(); > + changed = true; > + } > + > + if (lsample) { > + if (!n_opts) { > + dpif_lsample_unref(lsample); > + lsample = ofproto->lsample = NULL; > + changed = true; > + } else if (dpif_lsample_set_options(lsample, options, n_opts)) { > + changed = true; > + } > + } > + > + if (changed) { > + ofproto->backer->need_revalidate = REV_RECONFIGURE; > + } > + > + return 0; > +} > + > static int > set_cfm(struct ofport *ofport_, const struct cfm_settings *s) > { > @@ -7201,6 +7238,7 @@ const struct ofproto_class ofproto_dpif_class = { > set_sflow, > set_ipfix, > get_ipfix_stats, > + set_local_sample, > set_cfm, > cfm_status_changed, > get_cfm_status, > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index ea18ccedf..d7b7d5f8c 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -331,6 +331,7 @@ struct ofproto_dpif { > struct netflow *netflow; > struct dpif_sflow *sflow; > struct dpif_ipfix *ipfix; > + struct dpif_lsample *lsample; > struct hmap bundles; /* Contains "struct ofbundle"s. */ > struct mac_learning *ml; > struct mcast_snooping *ms; > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > index 83c509fcf..85991554c 100644 > --- a/ofproto/ofproto-provider.h > +++ b/ofproto/ofproto-provider.h > @@ -1489,6 +1489,15 @@ struct ofproto_class { > bool bridge_ipfix, struct ovs_list *replies > ); > > + /* Configures local sampling on 'ofproto' according to the options array > + * of 'options' which contains 'n_options' elements. > + * > + * EOPNOTSUPP as a return value indicates that 'ofproto' does not support > + * local sampling. */ > + int (*set_local_sample)(struct ofproto *ofproto, > + const struct ofproto_lsample_options *options, > + size_t n_options); > + > /* Configures connectivity fault management on 'ofport'. > * > * If 'cfm_settings' is nonnull, configures CFM according to its members. > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index 21c6a1d82..8c1efe4bf 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -1000,6 +1000,18 @@ ofproto_get_datapath_cap(const char *datapath_type, > struct smap *dp_cap) > } > } > > +int ofproto_set_local_sample(struct ofproto *ofproto, > + const struct ofproto_lsample_options *options, > + size_t n_options) > +{ > + if (ofproto->ofproto_class->set_local_sample) { > + return ofproto->ofproto_class->set_local_sample(ofproto, options, > + n_options); > + } else { > + return EOPNOTSUPP; > + } > +} > + > /* Connection tracking configuration. */ > void > ofproto_ct_set_zone_timeout_policy(const char *datapath_type, uint16_t > zone_id, > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h > index 1c07df275..f1ff80e52 100644 > --- a/ofproto/ofproto.h > +++ b/ofproto/ofproto.h > @@ -103,6 +103,11 @@ struct ofproto_ipfix_flow_exporter_options { > char *virtual_obs_id; > }; > > +struct ofproto_lsample_options { > + uint32_t collector_set_id; > + uint32_t group_id; > +}; > + > struct ofproto_rstp_status { > bool enabled; /* If false, ignore other members. */ > rstp_identifier root_id; > @@ -371,6 +376,9 @@ int ofproto_set_ipfix(struct ofproto *, > const struct ofproto_ipfix_bridge_exporter_options *, > const struct ofproto_ipfix_flow_exporter_options *, > size_t); > +int ofproto_set_local_sample(struct ofproto *ofproto, > + const struct ofproto_lsample_options *, > + size_t n_options); > void ofproto_set_flow_restore_wait(bool flow_restore_wait_db); > bool ofproto_get_flow_restore_wait(void); > int ofproto_set_stp(struct ofproto *, const struct ofproto_stp_settings *); _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
