On 7/7/24 22:08, Adrian Moreno wrote:
> Add a new resource in ofproto-dpif and the corresponding API in
> ofproto_provider.h to represent and local sampling configuration.
>
> Signed-off-by: Adrian Moreno <amore...@redhat.com>
> ---
> ofproto/automake.mk | 2 +
> ofproto/ofproto-dpif-lsample.c | 187 +++++++++++++++++++++++++++++++++
> ofproto/ofproto-dpif-lsample.h | 36 +++++++
> 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, 293 insertions(+)
> create mode 100644 ofproto/ofproto-dpif-lsample.c
> create mode 100644 ofproto/ofproto-dpif-lsample.h
>
> 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..d675a116f
> --- /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. */
line should start with a '*'.
> + struct ovs_mutex mutex; /* Protects concurrent insertion/deletion
> + * of exporters. */
> +
> + 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) {
> + 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))) {
> + 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..bee36c9c5
> --- /dev/null
> +++ b/ofproto/ofproto-dpif-lsample.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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);
> +
> +void dpif_lsample_unref(struct dpif_lsample *);
> +
> +struct dpif_lsample *dpif_lsample_ref(const 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 94c84d697..56830c630 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -49,6 +49,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"
> @@ -1954,6 +1955,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);
> @@ -2513,6 +2515,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 ENOTSUP;
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 {
> + changed |= dpif_lsample_set_options(lsample, options, n_opts);
Don't use bit operations on booleans.
> + }
> + }
> +
> + if (changed) {
> + ofproto->backer->need_revalidate = REV_RECONFIGURE;
> + }
> +
> + return 0;
> +}
> +
> static int
> set_cfm(struct ofport *ofport_, const struct cfm_settings *s)
> {
> @@ -7100,6 +7137,7 @@ const struct ofproto_class ofproto_dpif_class = {
> get_netflow_ids,
> set_sflow,
> set_ipfix,
> + set_local_sample,
> get_ipfix_stats,
I'd suggest to keep ipfix-related callbacks together. We do so for
all other features.
> set_cfm,
> cfm_status_changed,
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index bc7a19dab..420c350fd 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..02e6710d9 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1479,6 +1479,15 @@ struct ofproto_class {
> const struct ofproto_ipfix_flow_exporter_options
> *flow_exporters_options, size_t n_flow_exporters_options);
>
> + /* 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);
> +
> /* Gets IPFIX stats on 'ofproto' according to the exporter of birdge
> * IPFIX or flow-based IPFIX.
> *
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 21c6a1d82..2a1db8a0a 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 ENOTSUP;
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..fded3a3db 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;
> @@ -390,6 +395,9 @@ void ofproto_ct_zone_limit_protection_update(const char
> *datapath_type,
> bool protected);
> void 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 *,
> + size_t n_options);
>
> /* Configuration of ports. */
> void ofproto_port_unregister(struct ofproto *, ofp_port_t ofp_port);
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
