On Fri, Jul 12, 2024 at 5:15 PM Dumitru Ceara <dce...@redhat.com> wrote:
> This will represent a unified place to store IPFIX observation domain ID
> configurations for sampling applications (currently only drop sampling
> is supported as application but following commits will add more).
>
> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
> ---
>
Hi Dumitru,
I have a couple of small comments down below.
northd/automake.mk | 2 +
> northd/en-lflow.c | 5 ++
> northd/en-sampling-app.c | 120 +++++++++++++++++++++++++++++++++++++++
> northd/en-sampling-app.h | 51 +++++++++++++++++
> northd/inc-proc-northd.c | 10 +++-
> northd/northd.h | 1 +
> ovn-nb.ovsschema | 21 ++++++-
> ovn-nb.xml | 17 ++++++
> tests/ovn-northd.at | 17 ++++++
> 9 files changed, 241 insertions(+), 3 deletions(-)
> create mode 100644 northd/en-sampling-app.c
> create mode 100644 northd/en-sampling-app.h
>
> diff --git a/northd/automake.mk b/northd/automake.mk
> index d491973a8b..6566ad2999 100644
> --- a/northd/automake.mk
> +++ b/northd/automake.mk
> @@ -32,6 +32,8 @@ northd_ovn_northd_SOURCES = \
> northd/en-lr-stateful.h \
> northd/en-ls-stateful.c \
> northd/en-ls-stateful.h \
> + northd/en-sampling-app.c \
> + northd/en-sampling-app.h \
> northd/inc-proc-northd.c \
> northd/inc-proc-northd.h \
> northd/ipam.c \
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index c4b927fb8c..eb91f2a651 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -25,6 +25,7 @@
> #include "en-ls-stateful.h"
> #include "en-northd.h"
> #include "en-meters.h"
> +#include "en-sampling-app.h"
> #include "lflow-mgr.h"
>
> #include "lib/inc-proc-eng.h"
> @@ -86,6 +87,10 @@ lflow_get_input_data(struct engine_node *node,
> lflow_input->ovn_internal_version_changed =
> global_config->ovn_internal_version_changed;
> lflow_input->svc_monitor_mac = global_config->svc_monitor_mac;
> +
> + struct ed_type_sampling_app_data *sampling_app_data =
> + engine_get_input_data("sampling_app", node);
> + lflow_input->sampling_apps = &sampling_app_data->apps;
> }
>
> void en_lflow_run(struct engine_node *node, void *data)
> diff --git a/northd/en-sampling-app.c b/northd/en-sampling-app.c
> new file mode 100644
> index 0000000000..8d2d45a90f
> --- /dev/null
> +++ b/northd/en-sampling-app.c
> @@ -0,0 +1,120 @@
> +/*
> + * 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 "openvswitch/vlog.h"
> +
> +#include "en-sampling-app.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_sampling_app);
> +
> +/* Static function declarations. */
> +static void sampling_app_table_add(struct sampling_app_table *,
> + const struct nbrec_sampling_app *);
> +static uint8_t sampling_app_table_get_id(const struct sampling_app_table
> *,
> + enum sampling_app_type);
> +static void sampling_app_table_reset(struct sampling_app_table *);
> +static enum sampling_app_type sampling_app_get_by_name(const char
> *app_name);
> +
> +void *
> +en_sampling_app_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *arg OVS_UNUSED)
> +{
> + struct ed_type_sampling_app_data *data = xzalloc(sizeof *data);
> + sampling_app_table_reset(&data->apps);
> + return data;
> +}
> +
> +void
> +en_sampling_app_cleanup(void *data OVS_UNUSED)
> +{
> +}
> +
> +void
> +en_sampling_app_run(struct engine_node *node, void *data_)
> +{
> + const struct nbrec_sampling_app_table *nb_sampling_app_table =
> + EN_OVSDB_GET(engine_get_input("NB_sampling_app", node));
> + struct ed_type_sampling_app_data *data = data_;
> +
> + sampling_app_table_reset(&data->apps);
> +
> + const struct nbrec_sampling_app *sa;
> + NBREC_SAMPLING_APP_TABLE_FOR_EACH (sa, nb_sampling_app_table) {
> + sampling_app_table_add(&data->apps, sa);
> + }
> +
> + engine_set_node_state(node, EN_UPDATED);
> +}
> +
> +uint8_t
> +sampling_app_get_id(const struct sampling_app_table *app_table,
> + enum sampling_app_type app_type)
> +{
> + return sampling_app_table_get_id(app_table, app_type);
> +}
> +
> +/* Static functions. */
> +static
> +void sampling_app_table_add(struct sampling_app_table *table,
> + const struct nbrec_sampling_app *sa)
> +{
> + enum sampling_app_type app_type = sampling_app_get_by_name(sa->name);
> +
> + if (app_type == SAMPLING_APP_MAX) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_WARN_RL(&rl, "Unexpected Sampling_App name: %s", sa->name);
> + return;
> + }
> + table->app_ids[app_type] = sa->id;
> +}
> +
> +static
> +uint8_t sampling_app_table_get_id(const struct sampling_app_table *table,
> + enum sampling_app_type app_type)
>
nit: Wrong format of the function return type, that seems to be the case
for the whole file.
> +{
> + ovs_assert(app_type < SAMPLING_APP_MAX);
> + return table->app_ids[app_type];
> +}
> +
> +static
> +void sampling_app_table_reset(struct sampling_app_table *table)
> +{
> + for (size_t i = 0; i < SAMPLING_APP_MAX; i++) {
> + table->app_ids[i] = SAMPLING_APP_ID_NONE;
> + }
> +}
> +
> +static const char *app_names[] = {
> + [SAMPLING_APP_DROP_DEBUG] =
> + "drop-sampling",
> + [SAMPLING_APP_ACL_NEW_TRAFFIC] =
> + "acl-new-traffic-sampling",
> + [SAMPLING_APP_ACL_EST_TRAFFIC] =
> + "acl-est-traffic-sampling",
> +};
> +
> +static enum sampling_app_type
> +sampling_app_get_by_name(const char *app_name)
> +{
> + for (size_t app_type = 0; app_type < ARRAY_SIZE(app_names);
> app_type++) {
> + if (!strcmp(app_name, app_names[app_type])) {
> + return app_type;
> + }
> + }
> + return SAMPLING_APP_MAX;
> +}
> diff --git a/northd/en-sampling-app.h b/northd/en-sampling-app.h
> new file mode 100644
> index 0000000000..d80bfe2921
> --- /dev/null
> +++ b/northd/en-sampling-app.h
> @@ -0,0 +1,51 @@
> +/*
> + * 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 EN_SAMPLING_APP_H
> +#define EN_SAMPLING_APP_H 1
> +
> +/* OVS includes. */
> +#include "openvswitch/shash.h"
> +
> +/* OVN includes. */
> +#include "lib/inc-proc-eng.h"
> +#include "lib/ovn-nb-idl.h"
> +
> +/* Valid sample IDs are in the 1..255 interval. */
> +#define SAMPLING_APP_ID_NONE 0
> +
> +/* Supported sampling application types. */
> +enum sampling_app_type {
> + SAMPLING_APP_DROP_DEBUG,
> + SAMPLING_APP_ACL_NEW_TRAFFIC,
> + SAMPLING_APP_ACL_EST_TRAFFIC,
> + SAMPLING_APP_MAX,
> +};
> +
> +struct sampling_app_table {
> + uint8_t app_ids[SAMPLING_APP_MAX];
> +};
> +
> +struct ed_type_sampling_app_data {
> + struct sampling_app_table apps;
> +};
> +
> +void *en_sampling_app_init(struct engine_node *, struct engine_arg *);
> +void en_sampling_app_cleanup(void *data);
> +void en_sampling_app_run(struct engine_node *, void *data);
> +uint8_t sampling_app_get_id(const struct sampling_app_table *,
> + enum sampling_app_type);
> +
> +#endif /* EN_SAMPLING_APP_H */
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 522236ad2a..5d89670c29 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -39,6 +39,7 @@
> #include "en-lflow.h"
> #include "en-northd-output.h"
> #include "en-meters.h"
> +#include "en-sampling-app.h"
> #include "en-sync-sb.h"
> #include "en-sync-from-sb.h"
> #include "unixctl.h"
> @@ -61,7 +62,8 @@ static unixctl_cb_func chassis_features_list;
> NB_NODE(meter, "meter") \
> NB_NODE(bfd, "bfd") \
> NB_NODE(static_mac_binding, "static_mac_binding") \
> - NB_NODE(chassis_template_var, "chassis_template_var")
> + NB_NODE(chassis_template_var, "chassis_template_var") \
> + NB_NODE(sampling_app, "sampling_app")
>
> enum nb_engine_node {
> #define NB_NODE(NAME, NAME_STR) NB_##NAME,
> @@ -138,6 +140,7 @@ enum sb_engine_node {
> * avoid sparse errors. */
> static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(northd, "northd");
> static ENGINE_NODE(sync_from_sb, "sync_from_sb");
> +static ENGINE_NODE(sampling_app, "sampling_app");
> static ENGINE_NODE(lflow, "lflow");
> static ENGINE_NODE(mac_binding_aging, "mac_binding_aging");
> static ENGINE_NODE(mac_binding_aging_waker, "mac_binding_aging_waker");
> @@ -170,6 +173,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_lb_data, &en_nb_logical_router,
> lb_data_logical_router_handler);
>
> + engine_add_input(&en_sampling_app, &en_nb_sampling_app, NULL);
> +
> engine_add_input(&en_global_config, &en_nb_nb_global,
> global_config_nb_global_handler);
> engine_add_input(&en_global_config, &en_sb_sb_global,
> @@ -251,6 +256,9 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL);
> engine_add_input(&en_lflow, &en_global_config,
> node_global_config_handler);
> +
> + engine_add_input(&en_lflow, &en_sampling_app, NULL);
> +
> engine_add_input(&en_lflow, &en_northd, lflow_northd_handler);
> engine_add_input(&en_lflow, &en_port_group, lflow_port_group_handler);
> engine_add_input(&en_lflow, &en_lr_stateful,
> lflow_lr_stateful_handler);
> diff --git a/northd/northd.h b/northd/northd.h
> index d4a8d75abc..e50aa6731a 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -190,6 +190,7 @@ struct lflow_input {
> const struct hmap *svc_monitor_map;
> bool ovn_internal_version_changed;
> const char *svc_monitor_mac;
> + const struct sampling_app_table *sampling_apps;
> };
>
> extern int parallelization_state;
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index e3c4aff9df..e131a4c083 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> "version": "7.4.0",
>
Missing version bump.
- "cksum": "1908497390 35615",
> + "cksum": "1498303893 36355",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -691,6 +691,23 @@
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}}},
> "indexes": [["chassis"]],
> - "isRoot": true}
> + "isRoot": true},
> + "Sampling_App": {
> + "columns": {
> + "name": {"type": {"key": {"type": "string",
> + "enum": ["set",
> + ["drop-sampling",
> + "acl-new-traffic-sampling",
> + "acl-est-traffic-sampling"]]}}},
> + "id": {"type": {"key": {"type": "integer",
> + "minInteger": 1,
> + "maxInteger": 255}}},
> + "external_ids": {
> + "type": {"key": "string", "value": "string",
> + "min": 0, "max": "unlimited"}}
> + },
> + "indexes": [["name"]],
> + "isRoot": true
> + }
> }
> }
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 9552534f6d..f2a8b5c076 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -5021,4 +5021,21 @@ or
> </column>
> </group>
> </table>
> + <table name="Sampling_App">
> + <column name="name">
> + The name of the application to be configured for sampling.
> Currently
> + supported options are: "drop-sampling", "acl-new-traffic-sampling",
> + "acl-est-traffic-sampling".
> + </column>
> + <column name="id">
> + The identifier to be encoded in the (IPFIX) samples generated for
> this
> + type of application. This identifier is used as part of the
> sample's
> + observation domain ID.
> + </column>
> + <group title="Common Columns">
> + <column name="external_ids">
> + See <em>External IDs</em> at the beginning of this document.
> + </column>
> + </group>
> + </table>
> </database>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 5acb13c519..f2f11c005c 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -12449,6 +12449,23 @@ check_engine_stats lflow recompute nocompute
>
> AT_CLEANUP
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Sampling_App incremental processing])
> +
> +ovn_start
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +
> +ovn-nbctl create Sampling_App name="acl-new-traffic-sampling" id="42"
> +check_row_count nb:Sampling_App 1
> +check_engine_stats sampling_app recompute nocompute
> +check_engine_stats northd norecompute nocompute
> +check_engine_stats lflow recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +AT_CLEANUP
> +])
> +
> OVN_FOR_EACH_NORTHD_NO_HV([
> AT_SETUP([NAT with match])
> ovn_start
> --
> 2.44.0
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thanks,
Ales
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
amu...@redhat.com
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
