Re: [ovs-dev] [PATCH ovn v6 3/3] northd: add drop sampling

2022-11-23 Thread Numan Siddique
On Mon, Nov 21, 2022 at 11:13 AM Adrian Moreno  wrote:
>
> 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 | datapath_key
>   - 8 most significant bits: the obs_domain_id specified in the
> NB_Global options.
>   - 24 least significant bits: the datapath 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 openflow table number.
>
> Adding this configuration is not enough to make OVS sample drops. The
> apropriate configuration IPFIX needs to be added to those chassis that
> you wish to sample from. See man(5) ovs-vswitchd.conf for more details.
>
> Signed-off-by: Adrian Moreno 

Acked-by: Numan Siddique 

Numan

> ---
>  NEWS|  2 +
>  controller/ovn-controller.c | 42 
>  controller/physical.c   | 40 ---
>  controller/physical.h   |  6 +++
>  northd/automake.mk  |  2 +
>  northd/debug.c  | 98 +
>  northd/debug.h  | 30 
>  northd/northd.c | 77 -
>  northd/ovn-northd.8.xml | 26 ++
>  ovn-nb.xml  | 28 +++
>  ovn-sb.xml  | 29 +++
>  tests/ovn.at| 67 -
>  12 files changed, 380 insertions(+), 67 deletions(-)
>  create mode 100644 northd/debug.c
>  create mode 100644 northd/debug.h
>
> diff --git a/NEWS b/NEWS
> index 224a7b83e..6c4573b50 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,7 @@
>  Post v22.09.0
>  -
> +  - ovn-northd: Add configuration knobs to enable drop sampling 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 7dd83e7f4..0752a71ad 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -3172,6 +3172,8 @@ lflow_output_sb_meter_handler(struct engine_node *node, 
> void *data)
>  struct ed_type_pflow_output {
>  /* Desired physical flows. */
>  struct ovn_desired_flow_table flow_table;
> +/* Drop debugging options. */
> +struct physical_debug debug;
>  };
>
>  static void init_physical_ctx(struct engine_node *node,
> @@ -3216,6 +3218,11 @@ static void init_physical_ctx(struct engine_node *node,
>  chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
>  }
>
> +const struct sbrec_sb_global_table *sb_global_table =
> +EN_OVSDB_GET(engine_get_input("SB_sb_global", node));
> +const struct sbrec_sb_global *sb_global =
> +sbrec_sb_global_table_first(sb_global_table);
> +
>  ovs_assert(br_int && chassis);
>
>  struct ed_type_ct_zones *ct_zones_data =
> @@ -3237,6 +3244,13 @@ static void init_physical_ctx(struct engine_node *node,
>  p_ctx->local_bindings = _data->lbinding_data.bindings;
>  p_ctx->patch_ofports = _vif_data->patch_ofports;
>  p_ctx->chassis_tunnels = _vif_data->chassis_tunnels;
> +p_ctx->debug.collector_set_id = smap_get_uint(_global->options,
> +  "debug_drop_collector_set",
> +  0);
> +
> +p_ctx->debug.obs_domain_id = smap_get_uint(_global->options,
> +   "debug_drop_domain_id",
> +   0);
>  }
>
>  static void *
> @@ -3439,6 +3453,32 @@ pflow_output_activated_ports_handler(struct 
> engine_node *node, void *data)
>  return true;
>  }
>
> +static bool
> +pflow_output_sb_sb_global_handler(struct engine_node *node, void *data)
> +{
> +const struct sbrec_sb_global_table *sb_global_table =
> +EN_OVSDB_GET(engine_get_input("SB_sb_global", node));
> +const struct sbrec_sb_global *sb_global =
> +sbrec_sb_global_table_first(sb_global_table);
> +
> +struct ed_type_pflow_output *pfo = data;
> +
> +uint32_t collector_set_id = smap_get_uint(_global->options,
> +  "debug_drop_collector_set",
> +  0);
> +uint32_t obs_domain_id = smap_get_uint(_global->options,
> +   "debug_drop_domain_id",
> +   0);
> +
> +if (pfo->debug.collector_set_id != collector_set_id ||
> +

[ovs-dev] [PATCH ovn v6 3/3] northd: add drop sampling

2022-11-21 Thread Adrian Moreno
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 | datapath_key
  - 8 most significant bits: the obs_domain_id specified in the
NB_Global options.
  - 24 least significant bits: the datapath 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 openflow table number.

Adding this configuration is not enough to make OVS sample drops. The
apropriate configuration IPFIX needs to be added to those chassis that
you wish to sample from. See man(5) ovs-vswitchd.conf for more details.

Signed-off-by: Adrian Moreno 
---
 NEWS|  2 +
 controller/ovn-controller.c | 42 
 controller/physical.c   | 40 ---
 controller/physical.h   |  6 +++
 northd/automake.mk  |  2 +
 northd/debug.c  | 98 +
 northd/debug.h  | 30 
 northd/northd.c | 77 -
 northd/ovn-northd.8.xml | 26 ++
 ovn-nb.xml  | 28 +++
 ovn-sb.xml  | 29 +++
 tests/ovn.at| 67 -
 12 files changed, 380 insertions(+), 67 deletions(-)
 create mode 100644 northd/debug.c
 create mode 100644 northd/debug.h

diff --git a/NEWS b/NEWS
index 224a7b83e..6c4573b50 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Post v22.09.0
 -
+  - ovn-northd: Add configuration knobs to enable drop sampling 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 7dd83e7f4..0752a71ad 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3172,6 +3172,8 @@ lflow_output_sb_meter_handler(struct engine_node *node, 
void *data)
 struct ed_type_pflow_output {
 /* Desired physical flows. */
 struct ovn_desired_flow_table flow_table;
+/* Drop debugging options. */
+struct physical_debug debug;
 };
 
 static void init_physical_ctx(struct engine_node *node,
@@ -3216,6 +3218,11 @@ static void init_physical_ctx(struct engine_node *node,
 chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
 }
 
+const struct sbrec_sb_global_table *sb_global_table =
+EN_OVSDB_GET(engine_get_input("SB_sb_global", node));
+const struct sbrec_sb_global *sb_global =
+sbrec_sb_global_table_first(sb_global_table);
+
 ovs_assert(br_int && chassis);
 
 struct ed_type_ct_zones *ct_zones_data =
@@ -3237,6 +3244,13 @@ static void init_physical_ctx(struct engine_node *node,
 p_ctx->local_bindings = _data->lbinding_data.bindings;
 p_ctx->patch_ofports = _vif_data->patch_ofports;
 p_ctx->chassis_tunnels = _vif_data->chassis_tunnels;
+p_ctx->debug.collector_set_id = smap_get_uint(_global->options,
+  "debug_drop_collector_set",
+  0);
+
+p_ctx->debug.obs_domain_id = smap_get_uint(_global->options,
+   "debug_drop_domain_id",
+   0);
 }
 
 static void *
@@ -3439,6 +3453,32 @@ pflow_output_activated_ports_handler(struct engine_node 
*node, void *data)
 return true;
 }
 
+static bool
+pflow_output_sb_sb_global_handler(struct engine_node *node, void *data)
+{
+const struct sbrec_sb_global_table *sb_global_table =
+EN_OVSDB_GET(engine_get_input("SB_sb_global", node));
+const struct sbrec_sb_global *sb_global =
+sbrec_sb_global_table_first(sb_global_table);
+
+struct ed_type_pflow_output *pfo = data;
+
+uint32_t collector_set_id = smap_get_uint(_global->options,
+  "debug_drop_collector_set",
+  0);
+uint32_t obs_domain_id = smap_get_uint(_global->options,
+   "debug_drop_domain_id",
+   0);
+
+if (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.collector_set_id = collector_set_id;
+pfo->debug.obs_domain_id = obs_domain_id;
+}
+return true;
+}
+
 static void *
 en_flow_output_init(struct engine_node *node OVS_UNUSED,