diff mbox series

[ovs-dev,v3,3/3] northd: add drop sampling

Message ID 20220927073116.2166024-4-amorenoz@redhat.com
State Superseded
Headers show
Series Add ovn drop debugging | expand

Checks

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

Commit Message

Adrian Moreno Sept. 27, 2022, 7:31 a.m. UTC
Two new options are added to NB_Global table that enable drop
sampling by specifying the collector_set_id and the obs_domain_id of
the sample actions added to all drop flows.

For drops coming from an lflow, the sample has the following fields:
- obs_domain_id (32-bit): obs_domain_id << 8 | tunnel_key
  - 8 most significant bits: the obs_domain_id specified in the
    NB_Global options.
  - 24 least significant bits: the tunnel_key.
- obs_point_id: the cookie (first 32-bits of the lflow's UUID).

For drops that are inserted by ovn-controller without any associated
lflow, the sample will have the follwing fields:
- obs_domain_id (32-bit): obs_domain_id << 8
  - 8 most significant bits: the obs_domain_id specified in the
    NB_Global options.
  - 24 least significant bits: 0.
- obs_point_id: The table number.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 NEWS                        |  2 +
 controller/ovn-controller.c | 31 ++++++++++---
 controller/physical.c       | 34 ++++++++++++--
 controller/physical.h       |  8 +++-
 northd/debug.c              | 92 +++++++++++++++++++++++++++++++++++--
 northd/debug.h              | 10 ++++
 northd/northd.c             | 73 +++++++++++++++--------------
 ovn-nb.xml                  | 24 ++++++++++
 tests/ovn.at                | 65 ++++++++++++++++----------
 9 files changed, 263 insertions(+), 76 deletions(-)
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 224a7b83e..c01774499 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@ 
 Post v22.09.0
 -------------
+  - ovn-northd: Add configuration knobs to make drops explicit and
+    optionally sample them (using OVS's per-flow IPFIX sampling).
 
 OVN v22.09.0 - 16 Sep 2022
 --------------------------
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index cc3bea64b..149199912 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3119,7 +3119,7 @@  struct ed_type_pflow_output {
     /* Desired physical flows. */
     struct ovn_desired_flow_table flow_table;
     /* Drop debugging options. */
-    bool debug_drop;
+    struct physical_debug debug;
 };
 
 static void init_physical_ctx(struct engine_node *node,
@@ -3196,8 +3196,15 @@  static void init_physical_ctx(struct engine_node *node,
     p_ctx->local_bindings = &rt_data->lbinding_data.bindings;
     p_ctx->patch_ofports = &non_vif_data->patch_ofports;
     p_ctx->chassis_tunnels = &non_vif_data->chassis_tunnels;
-    p_ctx->debug_drop = smap_get_bool(&sb_global->options,
+    p_ctx->debug.enabled = smap_get_bool(&sb_global->options,
                                       "debug_drop_mode", false);
+    p_ctx->debug.collector_set_id = smap_get_uint(&sb_global->options,
+                                                  "debug_drop_collector_set",
+                                                  0);
+
+    p_ctx->debug.obs_domain_id = smap_get_uint(&sb_global->options,
+                                               "debug_drop_domain_id",
+                                               0);
 }
 
 static void *
@@ -3411,12 +3418,22 @@  pflow_output_sb_sb_global_handler(struct engine_node *node, void *data)
 
     struct ed_type_pflow_output *pfo = data;
 
-    bool debug_drop = smap_get_bool(&sb_global->options,
-                                    "debug_drop_mode", false);
-
-    if (pfo->debug_drop != debug_drop) {
+    bool debug_enabled = smap_get_bool(&sb_global->options,
+                                       "debug_drop_mode", false);
+    uint32_t collector_set_id = smap_get_uint(&sb_global->options,
+                                              "debug_drop_collector_set",
+                                              0);
+    uint32_t obs_domain_id = smap_get_uint(&sb_global->options,
+                                           "debug_drop_domain_id",
+                                           0);
+
+    if (pfo->debug.enabled != debug_enabled ||
+        pfo->debug.collector_set_id != collector_set_id ||
+        pfo->debug.obs_domain_id != obs_domain_id) {
         engine_set_node_state(node, EN_UPDATED);
-        pfo->debug_drop = debug_drop;
+        pfo->debug.enabled = debug_enabled;
+        pfo->debug.collector_set_id = collector_set_id;
+        pfo->debug.obs_domain_id = obs_domain_id;
     }
     return true;
 }
diff --git a/controller/physical.c b/controller/physical.c
index e86d0297c..ee8fd9200 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -825,25 +825,44 @@  put_zones_ofpacts(const struct zone_ids *zone_ids, struct ofpbuf *ofpacts_p)
     }
 }
 
+static void
+put_drop(const struct physical_debug *debug, uint8_t table_id,
+         struct ofpbuf *ofpacts)
+{
+    if (debug->collector_set_id) {
+        struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts);
+        os->probability = UINT16_MAX;
+        os->collector_set_id = debug->collector_set_id;
+        os->obs_domain_id = (debug->obs_domain_id << 24);
+        os->obs_point_id = table_id;
+    }
+}
+
 static void
 add_default_drop_flow(const struct physical_ctx *p_ctx,
                       uint8_t table_id,
                       struct ovn_desired_flow_table *flow_table)
 {
-    if (p_ctx->debug_drop) {
+    if (p_ctx->debug.enabled) {
         struct match match = MATCH_CATCHALL_INITIALIZER;
         struct ofpbuf ofpacts;
         ofpbuf_init(&ofpacts, 0);
+
+        put_drop(&p_ctx->debug, table_id, &ofpacts);
         ofctrl_add_flow(flow_table, table_id, 0, 0, &match,
                         &ofpacts, hc_uuid);
+
+        ofpbuf_uninit(&ofpacts);
     }
 }
 
+
 static void
 put_local_common_flows(uint32_t dp_key,
                        const struct sbrec_port_binding *pb,
                        const struct sbrec_port_binding *parent_pb,
                        const struct zone_ids *zone_ids,
+                       const struct physical_debug *debug,
                        struct ofpbuf *ofpacts_p,
                        struct ovn_desired_flow_table *flow_table)
 {
@@ -879,6 +898,7 @@  put_local_common_flows(uint32_t dp_key,
      * and the MLF_ALLOW_LOOPBACK flag is not set. */
     match_init_catchall(&match);
     ofpbuf_clear(ofpacts_p);
+    put_drop(debug, OFTABLE_CHECK_LOOPBACK, ofpacts_p);
     match_set_metadata(&match, htonll(dp_key));
     match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
                          0, MLF_ALLOW_LOOPBACK);
@@ -1150,6 +1170,7 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
                       const struct hmap *chassis_tunnels,
                       const struct sbrec_port_binding *binding,
                       const struct sbrec_chassis *chassis,
+                      const struct physical_debug *debug,
                       struct ovn_desired_flow_table *flow_table,
                       struct ofpbuf *ofpacts_p)
 {
@@ -1173,7 +1194,7 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
 
         struct zone_ids binding_zones = get_zone_ids(binding, ct_zones);
         put_local_common_flows(dp_key, binding, NULL, &binding_zones,
-                               ofpacts_p, flow_table);
+                               debug, ofpacts_p, flow_table);
 
         ofpbuf_clear(ofpacts_p);
         match_outport_dp_and_port_keys(&match, dp_key, port_key);
@@ -1349,7 +1370,7 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
         /* Pass the parent port binding if the port is a nested
          * container. */
         put_local_common_flows(dp_key, binding, parent_port, &zone_ids,
-                               ofpacts_p, flow_table);
+                               debug, ofpacts_p, flow_table);
 
         /* Table 0, Priority 150 and 100.
          * ==============================
@@ -1481,6 +1502,7 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
 
             /* Drop LOCAL_ONLY traffic leaking through localnet ports. */
             ofpbuf_clear(ofpacts_p);
+            put_drop(debug, OFTABLE_CHECK_LOOPBACK, ofpacts_p);
             match_outport_dp_and_port_keys(&match, dp_key, port_key);
             match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
                                  MLF_LOCAL_ONLY, MLF_LOCAL_ONLY);
@@ -1878,7 +1900,8 @@  physical_eval_port_binding(struct physical_ctx *p_ctx,
                           p_ctx->local_bindings,
                           p_ctx->patch_ofports,
                           p_ctx->chassis_tunnels,
-                          pb, p_ctx->chassis, flow_table, &ofpacts);
+                          pb, p_ctx->chassis, &p_ctx->debug,
+                          flow_table, &ofpacts);
     ofpbuf_uninit(&ofpacts);
 }
 
@@ -2001,7 +2024,8 @@  physical_run(struct physical_ctx *p_ctx,
                               p_ctx->local_bindings,
                               p_ctx->patch_ofports,
                               p_ctx->chassis_tunnels, binding,
-                              p_ctx->chassis, flow_table, &ofpacts);
+                              p_ctx->chassis, &p_ctx->debug,
+                              flow_table, &ofpacts);
     }
 
     /* Handle output to multicast groups, in tables 37 and 38. */
diff --git a/controller/physical.h b/controller/physical.h
index 3947099b4..fc1bab566 100644
--- a/controller/physical.h
+++ b/controller/physical.h
@@ -43,6 +43,12 @@  struct local_nonvif_data;
 #define OVN_GENEVE_TYPE 0x80     /* Critical option. */
 #define OVN_GENEVE_LEN 4
 
+struct physical_debug {
+    bool enabled;
+    uint32_t collector_set_id;
+    uint32_t obs_domain_id;
+};
+
 struct physical_ctx {
     struct ovsdb_idl_index *sbrec_port_binding_by_name;
     struct ovsdb_idl_index *sbrec_port_binding_by_datapath;
@@ -59,7 +65,7 @@  struct physical_ctx {
     struct shash *local_bindings;
     struct simap *patch_ofports;
     struct hmap *chassis_tunnels;
-    bool debug_drop;
+    struct physical_debug debug;
 };
 
 void physical_register_ovs_idl(struct ovsdb_idl *);
diff --git a/northd/debug.c b/northd/debug.c
index 3db0a3c4f..06709d3f8 100644
--- a/northd/debug.c
+++ b/northd/debug.c
@@ -4,20 +4,104 @@ 
 
 #include "debug.h"
 
+#include "openvswitch/dynamic-string.h"
+#include "openvswitch/vlog.h"
 #include "smap.h"
 
+VLOG_DEFINE_THIS_MODULE(debug)
+
 static struct debug_config config;
 
+bool
+debug_enabled(void)
+{
+    return config.enabled;
+}
+
+bool debug_sampling_enabled(void)
+{
+    return config.collector_set_id != 0;
+}
+
 void
 init_debug_config(const struct nbrec_nb_global *nb)
 {
 
     const struct smap *options = &nb->options;
-    config.enabled = smap_get_bool(options, "debug_drop_mode", false);
+    bool enabled = smap_get_bool(options, "debug_drop_mode", false);
+    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);
+
+    if (enabled != config.enabled ||
+        collector_set_id != config.collector_set_id ||
+        observation_domain_id != config.observation_domain_id ||
+        !config.drop_action.length) {
+
+        if (observation_domain_id >= UINT8_MAX) {
+            VLOG_ERR("Observation domain id must be an 8-bit number");
+            return;
+        }
+
+        if (!enabled && collector_set_id) {
+            VLOG_WARN("Debug collection set configured, "
+                      "assuming debug_drop_mode");
+            enabled = true;
+        }
+
+        config.enabled = enabled;
+        config.collector_set_id = collector_set_id;
+        config.observation_domain_id = observation_domain_id;
+
+        ds_clear(&config.drop_action);
+
+        VLOG_DBG("Debug drop mode: %s", debug_enabled() ? "enabled" :
+                                                          "disabled");
+        if (debug_sampling_enabled()) {
+            ds_put_format(&config.drop_action,
+                          "sample(probability=65535,"
+                          "collector_set=%d,"
+                          "obs_domain=%d,"
+                          "obs_point=$cookie); ",
+                          config.collector_set_id,
+                          config.observation_domain_id);
+
+            ds_put_cstr(&config.drop_action, "/* drop */");
+            VLOG_DBG("Debug drop sampling: enabled");
+        } else {
+            ds_put_cstr(&config.drop_action, "drop;");
+            VLOG_DBG("Debug drop sampling: disabled");
+        }
+    }
 }
 
-bool
-debug_enabled(void)
+void
+destroy_debug_config(void)
 {
-    return config.enabled;
+    if (config.drop_action.string) {
+        ds_destroy(&config.drop_action);
+        ds_init(&config.drop_action);
+    }
+}
+
+const char *
+debug_drop_action(void) {
+    if (OVS_UNLIKELY(debug_sampling_enabled())) {
+        return ds_cstr_ro(&config.drop_action);
+    } else {
+        return "drop;";
+    }
+}
+
+const char *
+debug_implicit_drop_action(void)
+{
+    if (OVS_UNLIKELY(debug_sampling_enabled())) {
+        return ds_cstr_ro(&config.drop_action);
+    } else {
+        return "/* drop */";
+    }
 }
diff --git a/northd/debug.h b/northd/debug.h
index 69d4da171..9a1c02986 100644
--- a/northd/debug.h
+++ b/northd/debug.h
@@ -19,13 +19,23 @@ 
 #include <stdbool.h>
 
 #include "lib/ovn-nb-idl.h"
+#include "openvswitch/dynamic-string.h"
 
 struct debug_config {
     bool enabled;
+    uint32_t collector_set_id;
+    uint32_t observation_domain_id;
+    struct ds drop_action;
 };
 
 void init_debug_config(const struct nbrec_nb_global *nb);
+void destroy_debug_config(void);
 
 bool debug_enabled(void);
+bool debug_sampling_enabled(void);
+
+const char *debug_drop_action(void);
+const char *debug_implicit_drop_action(void);
+const char *debug_reject_action(void);
 
 #endif /* NORTHD_DEBUG_H */
diff --git a/northd/northd.c b/northd/northd.c
index 2cd0d0b11..9baadebef 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3800,7 +3800,7 @@  build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
         if (!n_active_backends) {
             if (!lb_vip->empty_backend_rej) {
                 ds_clear(action);
-                ds_put_cstr(action, "drop;");
+                ds_put_cstr(action, debug_drop_action());
                 skip_hash_fields = true;
             } else {
                 reject = true;
@@ -5156,7 +5156,7 @@  __ovn_lflow_add_default_drop(struct hmap *lflow_map,
                              const char *where)
 {
     if (OVS_UNLIKELY(debug_enabled())) {
-        ovn_lflow_add_at(lflow_map, od, stage, 0, "1", "drop;",
+        ovn_lflow_add_at(lflow_map, od, stage, 0, "1", debug_drop_action(),
                          NULL, NULL, NULL, where );
     }
 }
@@ -6388,7 +6388,7 @@  consider_acl(struct hmap *lflows, struct ovn_datapath *od,
             } else {
                 ds_put_format(match, " && (%s)", acl->match);
                 build_acl_log(actions, acl, meter_groups);
-                ds_put_cstr(actions, "/* drop */");
+                ds_put_cstr(actions, debug_implicit_drop_action());
                 ovn_lflow_add_with_hint(lflows, od, stage,
                                         acl->priority + OVN_ACL_PRI_OFFSET,
                                         ds_cstr(match), ds_cstr(actions),
@@ -6416,7 +6416,7 @@  consider_acl(struct hmap *lflows, struct ovn_datapath *od,
             } else {
                 ds_put_format(match, " && (%s)", acl->match);
                 build_acl_log(actions, acl, meter_groups);
-                ds_put_cstr(actions, "/* drop */");
+                ds_put_cstr(actions, debug_implicit_drop_action());
                 ovn_lflow_add_with_hint(lflows, od, stage,
                                         acl->priority + OVN_ACL_PRI_OFFSET,
                                         ds_cstr(match), ds_cstr(actions),
@@ -6433,7 +6433,7 @@  consider_acl(struct hmap *lflows, struct ovn_datapath *od,
                                        actions, &acl->header_, meter_groups);
             } else {
                 build_acl_log(actions, acl, meter_groups);
-                ds_put_cstr(actions, "/* drop */");
+                ds_put_cstr(actions, debug_implicit_drop_action());
                 ovn_lflow_add_with_hint(lflows, od, stage,
                                         acl->priority + OVN_ACL_PRI_OFFSET,
                                         acl->match, ds_cstr(actions),
@@ -6669,9 +6669,9 @@  build_acls(struct ovn_datapath *od, const struct chassis_features *features,
                       use_ct_inv_match ? "ct.inv || " : "",
                       ct_blocked_match);
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3,
-                      ds_cstr(&match), "drop;");
+                      ds_cstr(&match), debug_drop_action());
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3,
-                      ds_cstr(&match), "drop;");
+                      ds_cstr(&match),  debug_drop_action());
 
         /* Ingress and Egress ACL Table (Priority 65535 - 3).
          *
@@ -7770,7 +7770,7 @@  build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op,
                         rp->lsp_addrs[k].ipv4_addrs[l].addr_s);
                     ovn_lflow_add_with_lport_and_hint(
                         lflows, op->od, S_SWITCH_IN_EXTERNAL_PORT, 100,
-                        ds_cstr(&match), "drop;", port->key,
+                        ds_cstr(&match),  debug_drop_action(), port->key,
                         &op->nbsp->header_);
                 }
                 for (size_t l = 0; l < rp->lsp_addrs[k].n_ipv6_addrs; l++) {
@@ -7786,7 +7786,7 @@  build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op,
                         rp->lsp_addrs[k].ipv6_addrs[l].addr_s);
                     ovn_lflow_add_with_lport_and_hint(
                         lflows, op->od, S_SWITCH_IN_EXTERNAL_PORT, 100,
-                        ds_cstr(&match), "drop;", port->key,
+                        ds_cstr(&match), debug_drop_action(), port->key,
                         &op->nbsp->header_);
                 }
 
@@ -7801,7 +7801,8 @@  build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op,
                 ovn_lflow_add_with_lport_and_hint(lflows, op->od,
                                                   S_SWITCH_IN_EXTERNAL_PORT,
                                                   100, ds_cstr(&match),
-                                                  "drop;", port->key,
+                                                  debug_drop_action(),
+                                                  port->key,
                                                   &op->nbsp->header_);
             }
         }
@@ -7839,7 +7840,7 @@  build_lswitch_flows(const struct hmap *datapaths,
                           "outport = \""MC_UNKNOWN "\"; output;");
         } else {
             ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
-                          "outport == \"none\"", "drop;");
+                          "outport == \"none\"",  debug_drop_action());
         }
         ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 0, "1",
                       "output;");
@@ -7882,18 +7883,18 @@  build_lswitch_lflows_admission_control(struct ovn_datapath *od,
         if (!is_vlan_transparent(od)) {
             /* Block logical VLANs. */
             ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 100,
-                          "vlan.present", "drop;");
+                          "vlan.present", debug_drop_action());
         }
 
         /* Broadcast/multicast source address is invalid. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 100,
-                      "eth.src[40]", "drop;");
+                      "eth.src[40]", debug_drop_action());
 
         ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 50, "1",
                       REGBIT_PORT_SEC_DROP" = check_in_port_sec(); next;");
 
         ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 50,
-                      REGBIT_PORT_SEC_DROP" == 1", "drop;");
+                      REGBIT_PORT_SEC_DROP" == 1", debug_drop_action());
 
         ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 0, "1", "next;");
         /* Port security flows have priority 50
@@ -8425,7 +8426,7 @@  build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od,
                  */
                 if (!mcast_sw_info->flood_relay &&
                         !mcast_sw_info->flood_static) {
-                    ds_put_cstr(actions, "drop;");
+                    ds_put_cstr(actions, debug_drop_action());
                 }
 
                 ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 80,
@@ -8946,7 +8947,7 @@  build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od,
                       out_port->json_key);
 
     } else if (!strcmp(rule->action, "drop")) {
-        ds_put_cstr(&actions, "drop;");
+        ds_put_cstr(&actions, debug_drop_action());
     } else if (!strcmp(rule->action, "allow")) {
         uint32_t pkt_mark = ovn_smap_get_uint(&rule->options, "pkt_mark", 0);
         if (pkt_mark) {
@@ -9796,7 +9797,7 @@  add_route(struct hmap *lflows, struct ovn_datapath *od,
     struct ds common_actions = DS_EMPTY_INITIALIZER;
     struct ds actions = DS_EMPTY_INITIALIZER;
     if (is_discard_route) {
-        ds_put_format(&actions, "drop;");
+        ds_put_cstr(&actions, debug_drop_action());
     } else {
         ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ",
                       is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6);
@@ -10580,7 +10581,7 @@  build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
         ds_put_format(&match, " && %s", ds_cstr(extra_match));
     }
     if (drop) {
-        ds_put_format(&actions, "drop;");
+        ds_put_cstr(&actions, debug_drop_action());
     } else {
         ds_put_format(&actions,
                       "eth.dst = eth.src; "
@@ -10636,7 +10637,7 @@  build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op,
     }
 
     if (drop) {
-        ds_put_format(&actions, "drop;");
+        ds_put_cstr(&actions, debug_drop_action());
         ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
                                 ds_cstr(&match), ds_cstr(&actions), hint);
     } else {
@@ -10786,7 +10787,7 @@  build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage,
 
             char *match = xasprintf("ip4.dst == {%s}", ds_cstr(&match_ips));
             ovn_lflow_add_with_hint(lflows, op->od, stage, priority,
-                                    match, "drop;",
+                                    match, debug_drop_action(),
                                     &op->nbrp->header_);
             free(match);
         }
@@ -10815,7 +10816,7 @@  build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage,
 
             char *match = xasprintf("ip6.dst == {%s}", ds_cstr(&match_ips));
             ovn_lflow_add_with_hint(lflows, op->od, stage, priority,
-                                    match, "drop;",
+                                    match, debug_drop_action(),
                                     &op->nbrp->header_);
             free(match);
         }
@@ -10983,7 +10984,7 @@  build_adm_ctrl_flows_for_lrouter(
         /* Logical VLANs not supported.
          * Broadcast/multicast source address is invalid. */
         ovn_lflow_add(lflows, od, S_ROUTER_IN_ADMISSION, 100,
-                      "vlan.present || eth.src[40]", "drop;");
+                      "vlan.present || eth.src[40]", debug_drop_action());
 
         /* Default action for L2 security is to drop. */
         ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_ADMISSION);
@@ -11576,7 +11577,7 @@  build_mcast_lookup_flows_for_lrouter(
          * i.e., router solicitation and router advertisement.
          */
         ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10550,
-                      "nd_rs || nd_ra", "drop;");
+                      "nd_rs || nd_ra", debug_drop_action());
         if (!od->mcast_info.rtr.relay) {
             return;
         }
@@ -11623,13 +11624,13 @@  build_mcast_lookup_flows_for_lrouter(
                 ds_put_format(match, "eth.src == %s && igmp",
                               op->lrp_networks.ea_s);
                 ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10550,
-                              ds_cstr(match), "drop;");
+                              ds_cstr(match), debug_drop_action());
 
                 ds_clear(match);
                 ds_put_format(match, "eth.src == %s && (mldv1 || mldv2)",
                               op->lrp_networks.ea_s);
                 ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10550,
-                              ds_cstr(match), "drop;");
+                              ds_cstr(match), debug_drop_action());
             }
 
             ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10460,
@@ -11653,7 +11654,7 @@  build_mcast_lookup_flows_for_lrouter(
                           "};");
         } else {
             ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10450,
-                          "ip4.mcast || ip6.mcast", "drop;");
+                          "ip4.mcast || ip6.mcast", debug_drop_action());
         }
     }
 }
@@ -12479,7 +12480,7 @@  build_misc_local_traffic_drop_flows_for_lrouter(
                       "ip4.dst == 127.0.0.0/8 || "
                       "ip4.src == 0.0.0.0/8 || "
                       "ip4.dst == 0.0.0.0/8",
-                      "drop;");
+                      debug_drop_action());
 
         /* Drop ARP packets (priority 85). ARP request packets for router's own
          * IPs are handled with priority-90 flows.
@@ -12487,7 +12488,7 @@  build_misc_local_traffic_drop_flows_for_lrouter(
          * IPs are handled with priority-90 flows.
          */
         ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 85,
-                      "arp || nd", "drop;");
+                      "arp || nd", debug_drop_action());
 
         /* Allow IPv6 multicast traffic that's supposed to reach the
          * router pipeline (e.g., router solicitations).
@@ -12497,21 +12498,22 @@  build_misc_local_traffic_drop_flows_for_lrouter(
 
         /* Drop other reserved multicast. */
         ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 83,
-                      "ip6.mcast_rsvd", "drop;");
+                      "ip6.mcast_rsvd", debug_drop_action());
 
         /* Allow other multicast if relay enabled (priority 82). */
         ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 82,
                       "ip4.mcast || ip6.mcast",
-                      od->mcast_info.rtr.relay ? "next;" : "drop;");
+                      (od->mcast_info.rtr.relay ? "next;" :
+                                                  debug_drop_action()));
 
         /* Drop Ethernet local broadcast.  By definition this traffic should
          * not be forwarded.*/
         ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 50,
-                      "eth.bcast", "drop;");
+                      "eth.bcast", debug_drop_action());
 
         /* TTL discard */
         ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30,
-                      "ip4 && ip.ttl == {0, 1}", "drop;");
+                      "ip4 && ip.ttl == {0, 1}", debug_drop_action());
 
         /* Pass other traffic not already handled to the next table for
          * routing. */
@@ -12773,7 +12775,7 @@  build_lrouter_ipv4_ip_input(struct ovn_port *op,
             op_put_v4_networks(match, op, true);
             ds_put_cstr(match, " && "REGBIT_EGRESS_LOOPBACK" == 0");
             ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 100,
-                                    ds_cstr(match), "drop;",
+                                    ds_cstr(match), debug_drop_action(),
                                     &op->nbrp->header_);
 
             /* ICMP echo reply.  These flows reply to ICMP echo requests
@@ -13807,8 +13809,8 @@  build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows,
             struct ovn_port *op = ovn_port_find(ports, nat->logical_port);
             if (op && op->nbsp && !strcmp(op->nbsp->type, "virtual")) {
                 ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_GW_REDIRECT,
-                                        80, ds_cstr(match), "drop;",
-                                        &nat->header_);
+                                        80, ds_cstr(match),
+                                        debug_drop_action(), &nat->header_);
             }
             ds_put_format(match, " && is_chassis_resident(\"%s\")",
                           nat->logical_port);
@@ -15511,6 +15513,7 @@  northd_destroy(struct northd_data *data)
 
     destroy_datapaths_and_ports(&data->datapaths, &data->ports,
                                 &data->lr_list);
+    destroy_debug_config();
 }
 
 static void
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 813ccf6e6..423c8ca72 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -272,6 +272,30 @@ 
         </p>
       </column>
 
+      <column name="options" key="debug_drop_domain_id">
+        <p>
+          If set to a 8-bit number and if
+          <code>debug_drop_collector_set</code> is also configured,
+          <code>ovn-northd</code> will add a <code>sample</code> action to
+          every logical flow that contains a 'drop' action.
+          The 8 most significant bits of the observation_domain_id field will
+          be those specified in the
+          <code> debug_drop_domain_id</code>.
+          The 24 least significant bits of the observation_domain_id field will
+          be the datapath's tunnel key.
+        </p>
+      </column>
+
+      <column name="options" key="debug_drop_collector_set">
+        <p>
+          If set to a 32-bit number <code>ovn-northd</code> will add a
+          <code>sample</code> action to every logical flow that contains a
+          'drop' action. The sample action will have the specified
+          collector_set_id. The value must match that of the local OVS
+          configuration as described in ovs-actions(7).
+        </p>
+      </column>
+
       <group title="Options for configuring interconnection route advertisement">
         <p>
           These options control how routes are advertised between OVN
diff --git a/tests/ovn.at b/tests/ovn.at
index 1ab7a49d4..193180be8 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32928,6 +32928,41 @@  check_default_flows() {
     done
 }
 
+check_sample_drops() {
+    check ovn-nbctl -- remove NB_Global . options debug_drop_collector_set \
+                    -- remove NB_Global . options debug_drop_domain_id
+    check ovn-nbctl --wait=hv sync
+
+    ovs-ofctl dump-flows --no-stats br-int > oflows_nosample
+    AT_CAPTURE_FILE([oflows_nosample])
+    # Take match part of flows that contain "drop".
+    drop_matches="$(grep 'drop' oflows_nosample | grep -oP 'table=\d*, priority=.* ')"
+
+    check ovn-nbctl -- set NB_Global . options:debug_drop_collector_set="123" \
+                    -- set NB_Global . options:debug_drop_domain_id="1"
+    check ovn-nbctl --wait=hv sync
+
+    ovs-ofctl dump-flows --no-stats br-int > oflows_sample
+    AT_CAPTURE_FILE([oflows_sample])
+
+    # Check that every drop has now contains a "sample" action.
+    for flow in "$drop_matches"; do
+        AT_CHECK([grep -q "$flow actions=.*sample.*" oflows_sample], [0], [ignore], [ignore], [echo "Flow $flow has a drop and did not get sampled"])
+    done
+}
+
+check_debug() {
+    as hv1
+    check_default_flows
+    as hv2
+    check_default_flows
+
+    as hv1
+    check_sample_drops
+    as hv2
+    check_sample_drops
+}
+
 check ovn-nbctl -- set NB_Global . options:debug_drop_mode="true"
 
 # Logical network:
@@ -32995,10 +33030,7 @@  ovs-vsctl -- add-port br-int hv2-vif1 -- \
 wait_for_ports_up
 check ovn-nbctl --wait=hv sync
 
-as hv1
-check_default_flows
-as hv2
-check_default_flows
+check_debug
 
 # Add stateless ACL
 check ovn-nbctl --wait=sb \
@@ -33006,10 +33038,7 @@  check ovn-nbctl --wait=sb \
 check ovn-nbctl --wait=sb \
                 -- acl-add ls2 from-lport 100 'ip4' allow-stateless
 
-as hv1
-check_default_flows
-as hv2
-check_default_flows
+check_debug
 
 check ovn-nbctl --wait=sb acl-del ls1
 check ovn-nbctl --wait=sb acl-del ls2
@@ -33020,10 +33049,7 @@  check ovn-nbctl --wait=sb \
 check ovn-nbctl --wait=sb \
                 -- acl-add ls2 from-lport 100 "udp" allow-related
 
-as hv1
-check_default_flows
-as hv2
-check_default_flows
+check_debug
 
 check ovn-nbctl --wait=sb acl-del ls1
 check ovn-nbctl --wait=sb acl-del ls2
@@ -33037,10 +33063,7 @@  check ovn-nbctl --wait=sb \
     -- lb-add lb2 "10.0.1.1" "10.0.1.2" \
     -- ls-lb-add ls2 lb2
 
-as hv1
-check_default_flows
-as hv2
-check_default_flows
+check_debug
 
 # LB + stateless ACL
 check ovn-nbctl --wait=sb \
@@ -33048,10 +33071,7 @@  check ovn-nbctl --wait=sb \
 check ovn-nbctl --wait=sb \
                 -- acl-add ls2 from-lport 100 'ip4' allow-stateless
 
-as hv1
-check_default_flows
-as hv2
-check_default_flows
+check_debug
 
 check ovn-nbctl --wait=sb acl-del ls1
 check ovn-nbctl --wait=sb acl-del ls2
@@ -33062,10 +33082,7 @@  check ovn-nbctl --wait=sb \
 check ovn-nbctl --wait=sb \
                 -- acl-add ls2 from-lport 100 "udp" allow-related
 
-as hv1
-check_default_flows
-as hv2
-check_default_flows
+check_debug
 
 OVN_CLEANUP([hv1],[hv2])
 AT_CLEANUP