diff mbox series

[ovs-dev,v6,4/9] northd: Override NB_Global drop sampling id with Sampling_App config.

Message ID 20240806094451.730622-5-dceara@redhat.com
State Superseded
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 Aug. 6, 2024, 9:44 a.m. UTC 
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
V6:
- Removed Ales' ack.
V5:
- Added Ales' ack.
V4:
- Addressed Ales' comments:
  - deprecated old config knob
  - fixed unit test typo
- Added Mark's ack.
---
 NEWS                      |  3 +++
 northd/debug.c            | 12 +++++++-----
 northd/debug.h            |  3 ++-
 northd/en-global-config.c | 31 +++++++++++++++++++++++--------
 northd/inc-proc-northd.c  |  1 +
 ovn-nb.xml                |  5 +++++
 tests/ovn-northd.at       | 27 ++++++++++++++++++++++++++-
 7 files changed, 67 insertions(+), 15 deletions(-)
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 136f890f52..0c91e9608e 100644
--- a/NEWS
+++ b/NEWS
@@ -50,6 +50,9 @@  Post v24.03.0
   - A new LSP option "disable_garp_rarp" has been added to prevent OVN from
     sending GARP or RARP announcements when a VIF is created on a bridged
     logical switch.
+  - The NB_Global.debug_drop_domain_id configured value is now overridden by
+    the ID associated with the Sampling_App record created for drop sampling
+    (Sampling_App.type configured as "drop").
 
 OVN v24.03.0 - 01 Mar 2024
 --------------------------
diff --git a/northd/debug.c b/northd/debug.c
index 59da5d4f66..457993b7cf 100644
--- a/northd/debug.c
+++ b/northd/debug.c
@@ -3,6 +3,7 @@ 
 #include <string.h>
 
 #include "debug.h"
+#include "en-sampling-app.h"
 
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
@@ -26,16 +27,17 @@  debug_enabled(void)
 }
 
 void
-init_debug_config(const struct nbrec_nb_global *nb)
+init_debug_config(const struct nbrec_nb_global *nb,
+                  uint8_t drop_domain_id_override)
 {
-
     const struct smap *options = &nb->options;
     uint32_t collector_set_id = smap_get_uint(options,
                                               "debug_drop_collector_set",
                                               0);
-    uint32_t observation_domain_id = smap_get_uint(options,
-                                                   "debug_drop_domain_id",
-                                                   0);
+    uint32_t observation_domain_id =
+        drop_domain_id_override != SAMPLING_APP_ID_NONE
+        ? drop_domain_id_override
+        : smap_get_uint(options, "debug_drop_domain_id", 0);
 
     if (collector_set_id != config.collector_set_id ||
         observation_domain_id != config.observation_domain_id ||
diff --git a/northd/debug.h b/northd/debug.h
index c1a5e5aadb..a0b535253a 100644
--- a/northd/debug.h
+++ b/northd/debug.h
@@ -21,7 +21,8 @@ 
 #include "lib/ovn-nb-idl.h"
 #include "openvswitch/dynamic-string.h"
 
-void init_debug_config(const struct nbrec_nb_global *nb);
+void init_debug_config(const struct nbrec_nb_global *nb,
+                       uint8_t drop_domain_id_override);
 void destroy_debug_config(void);
 
 const char *debug_drop_action(void);
diff --git a/northd/en-global-config.c b/northd/en-global-config.c
index c5e65966b8..d7607aa074 100644
--- a/northd/en-global-config.c
+++ b/northd/en-global-config.c
@@ -24,6 +24,7 @@ 
 /* OVN includes */
 #include "debug.h"
 #include "en-global-config.h"
+#include "en-sampling-app.h"
 #include "include/ovn/features.h"
 #include "ipam.h"
 #include "lib/ovn-nb-idl.h"
@@ -42,8 +43,10 @@  static bool chassis_features_changed(const struct chassis_features *,
 static bool config_out_of_sync(const struct smap *config,
                                const struct smap *saved_config,
                                const char *key, bool must_be_present);
-static bool check_nb_options_out_of_sync(const struct nbrec_nb_global *,
-                                         struct ed_type_global_config *);
+static bool check_nb_options_out_of_sync(
+    const struct nbrec_nb_global *,
+    struct ed_type_global_config *,
+    const struct sampling_app_table *);
 static void update_sb_config_options_to_sbrec(struct ed_type_global_config *,
                                               const struct sbrec_sb_global *);
 
@@ -72,6 +75,9 @@  en_global_config_run(struct engine_node *node , void *data)
         EN_OVSDB_GET(engine_get_input("SB_sb_global", node));
     const struct sbrec_chassis_table *sbrec_chassis_table =
         EN_OVSDB_GET(engine_get_input("SB_chassis", node));
+    const struct ed_type_sampling_app_data *sampling_app_data =
+        engine_get_input_data("sampling_app", node);
+    const struct sampling_app_table *sampling_apps = &sampling_app_data->apps;
 
     struct ed_type_global_config *config_data = data;
 
@@ -145,7 +151,8 @@  en_global_config_run(struct engine_node *node , void *data)
         build_chassis_features(sbrec_chassis_table, &config_data->features);
     }
 
-    init_debug_config(nb);
+    init_debug_config(nb, sampling_app_get_id(sampling_apps,
+                                              SAMPLING_APP_DROP_DEBUG));
 
     const struct sbrec_sb_global *sb =
         sbrec_sb_global_table_first(sb_global_table);
@@ -186,6 +193,9 @@  global_config_nb_global_handler(struct engine_node *node, void *data)
         EN_OVSDB_GET(engine_get_input("NB_nb_global", node));
     const struct sbrec_sb_global_table *sb_global_table =
         EN_OVSDB_GET(engine_get_input("SB_sb_global", node));
+    const struct ed_type_sampling_app_data *sampling_app_data =
+        engine_get_input_data("sampling_app", node);
+    const struct sampling_app_table *sampling_apps = &sampling_app_data->apps;
 
     const struct nbrec_nb_global *nb =
         nbrec_nb_global_table_first(nb_global_table);
@@ -248,7 +258,7 @@  global_config_nb_global_handler(struct engine_node *node, void *data)
         return false;
     }
 
-    if (check_nb_options_out_of_sync(nb, config_data)) {
+    if (check_nb_options_out_of_sync(nb, config_data, sampling_apps)) {
         config_data->tracked_data.nb_options_changed = true;
     }
 
@@ -461,8 +471,10 @@  config_out_of_sync(const struct smap *config, const struct smap *saved_config,
 }
 
 static bool
-check_nb_options_out_of_sync(const struct nbrec_nb_global *nb,
-                             struct ed_type_global_config *config_data)
+check_nb_options_out_of_sync(
+    const struct nbrec_nb_global *nb,
+    struct ed_type_global_config *config_data,
+    const struct sampling_app_table *sampling_apps)
 {
     if (config_out_of_sync(&nb->options, &config_data->nb_options,
                            "mac_binding_removal_limit", false)) {
@@ -496,13 +508,16 @@  check_nb_options_out_of_sync(const struct nbrec_nb_global *nb,
 
     if (config_out_of_sync(&nb->options, &config_data->nb_options,
                            "debug_drop_domain_id", false)) {
-        init_debug_config(nb);
+        init_debug_config(nb, sampling_app_get_id(sampling_apps,
+                                                  SAMPLING_APP_DROP_DEBUG));
+
         return true;
     }
 
     if (config_out_of_sync(&nb->options, &config_data->nb_options,
                            "debug_drop_collector_set", false)) {
-        init_debug_config(nb);
+        init_debug_config(nb, sampling_app_get_id(sampling_apps,
+                                                  SAMPLING_APP_DROP_DEBUG));
         return true;
     }
 
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 5d89670c29..95bedc5cd0 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -181,6 +181,7 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
                      global_config_sb_global_handler);
     engine_add_input(&en_global_config, &en_sb_chassis,
                      global_config_sb_chassis_handler);
+    engine_add_input(&en_global_config, &en_sampling_app, NULL);
 
     engine_add_input(&en_northd, &en_nb_mirror, NULL);
     engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL);
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 0cf2478cf3..bc44f67642 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -351,6 +351,11 @@ 
           The observation_point_id will be set to the first 32 bits of the
           logical flow's UUID.
         </p>
+        <p>
+          Note: This key is deprecated in favor of the value configured in the
+          <ref table="Sampling_App"/> table for the <code>drop</code>
+          application.
+        </p>
       </column>
 
       <column name="options" key="debug_drop_collector_set">
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 66a651e68e..ebf02ef10a 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -12489,13 +12489,38 @@  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 ovn-nbctl create Sampling_App type="acl-new" 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 northd recompute nocompute
 check_engine_stats lflow recompute nocompute
+check_engine_stats global_config recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Sampling_App override debug_drop_domain_id])
+
+ovn_start
+
+check ovn-nbctl -- set NB_Global . options:debug_drop_collector_set="123" \
+                -- set NB_Global . options:debug_drop_domain_id="1" \
+                -- ls-add ls
+check ovn-nbctl --wait=sb sync
+
+AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_l2_unknown   ), priority=50   , match=(outport == "none"), action=(sample(probability=65535,collector_set=123,obs_domain=1,obs_point=$cookie); /* drop */)
+])
+
+ovn-nbctl --wait=sb create Sampling_App type="drop" id="42"
+check_row_count nb:Sampling_App 1
+
+AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_l2_unknown   ), priority=50   , match=(outport == "none"), action=(sample(probability=65535,collector_set=123,obs_domain=42,obs_point=$cookie); /* drop */)
+])
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD_NO_HV([
 AT_SETUP([NAT with match])
 ovn_start