diff mbox series

[ovs-dev,v3,6/8] northd: Add Sampling_App table.

Message ID 20240712151416.992033-7-dceara@redhat.com
State Changes Requested
Delegated to: Mark Michelson
Headers show
Series Add ACL Sampling using per-flow IPFIX. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Dumitru Ceara July 12, 2024, 3:14 p.m. UTC
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 <dceara@redhat.com>
---
 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

Comments

Ales Musil July 15, 2024, 9:18 a.m. UTC | #1
On Fri, Jul 12, 2024 at 5:15 PM Dumitru Ceara <dceara@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 <dceara@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
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thanks,
Ales
Mark Michelson July 23, 2024, 7:20 p.m. UTC | #2
I have no additions other than Ales's, so with those fixed,

Acked-by: Mark Michelson <mmichels@redhat.com>

On 7/15/24 05:18, Ales Musil wrote:
> On Fri, Jul 12, 2024 at 5:15 PM Dumitru Ceara <dceara@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 <dceara@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
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> Thanks,
> Ales
Dumitru Ceara July 30, 2024, 10:45 a.m. UTC | #3
On 7/15/24 11:18, Ales Musil wrote:
> On Fri, Jul 12, 2024 at 5:15 PM Dumitru Ceara <dceara@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 <dceara@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.
> 

Oops, I was too much in a hurry, will fix them up.

> 
>> +{
>> +    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.
> 

It was on purpose as I don't expect this patch to ever be committed on
its own but instead together with the one that adds Sample to the
schema.  But I can bump the version twice, no problem.

> -    "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
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> Thanks,
> Ales

Thanks,
Dumitru
diff mbox series

Patch

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)
+{
+    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",
-    "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