diff mbox series

[ovs-dev,v3,2/2] ipfix: make template and stats interval configurable

Message ID 20230210160314.131210-2-amorenoz@redhat.com
State Changes Requested
Headers show
Series [ovs-dev,v3,1/2] ofproto-ipfix: use per-domain template timeouts | expand

Checks

Context Check Description
ovsrobot/intel-ovs-compilation success test: success
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Adrián Moreno Feb. 10, 2023, 4:03 p.m. UTC 
From: Adrian Moreno <amorenoz@redhat.com>

Add options to the IPFIX table configure the interval to send statistics
and template information.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>

---
- v3:
   - Removed unit tests which generate errors in Intel CI. Will submit in
     independent series.
- v2:
  - Fixed a potential race condition in unit test.
- v1:
  - Added unit test.
---
 NEWS                         |  2 ++
 ofproto/ofproto-dpif-ipfix.c | 38 ++++++++++++++++++++++++++----------
 ofproto/ofproto.h            |  9 +++++++++
 vswitchd/bridge.c            | 17 ++++++++++++++++
 vswitchd/vswitch.ovsschema   | 12 +++++++++++-
 vswitchd/vswitch.xml         | 20 +++++++++++++++++++
 6 files changed, 87 insertions(+), 11 deletions(-)

Comments

Simon Horman Feb. 20, 2023, 3:42 p.m. UTC | #1 
On Fri, Feb 10, 2023 at 05:03:14PM +0100, Adrián Moreno wrote:
> From: Adrian Moreno <amorenoz@redhat.com>
> 
> Add options to the IPFIX table configure the interval to send statistics
> and template information.
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> 
> ---
> - v3:
>    - Removed unit tests which generate errors in Intel CI. Will submit in
>      independent series.
> - v2:
>   - Fixed a potential race condition in unit test.
> - v1:
>   - Added unit test.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Ilya Maximets Feb. 20, 2023, 6 p.m. UTC | #2 
On 2/10/23 17:03, Adrián Moreno wrote:
> From: Adrian Moreno <amorenoz@redhat.com>
> 
> Add options to the IPFIX table configure the interval to send statistics
> and template information.
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> 
> ---
> - v3:
>    - Removed unit tests which generate errors in Intel CI. Will submit in
>      independent series.
> - v2:
>   - Fixed a potential race condition in unit test.
> - v1:
>   - Added unit test.
> ---
>  NEWS                         |  2 ++
>  ofproto/ofproto-dpif-ipfix.c | 38 ++++++++++++++++++++++++++----------
>  ofproto/ofproto.h            |  9 +++++++++
>  vswitchd/bridge.c            | 17 ++++++++++++++++
>  vswitchd/vswitch.ovsschema   | 12 +++++++++++-
>  vswitchd/vswitch.xml         | 20 +++++++++++++++++++
>  6 files changed, 87 insertions(+), 11 deletions(-)
> 
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index 1a49cdffe..8ab609dfb 100644
> --- a/vswitchd/vswitch.ovsschema
> +++ b/vswitchd/vswitch.ovsschema
> @@ -1,6 +1,6 @@
>  {"name": "Open_vSwitch",
>   "version": "8.3.1",

You're adding new columns.  Need to bump the version to 8.4.0.

The numbering scheme is described here:
  https://docs.openvswitch.org/en/latest/ref/ovsdb.7/#schemas

Best regards, Ilya Maximets.
Adrián Moreno Feb. 22, 2023, 1:35 p.m. UTC | #3 
On 2/20/23 19:00, Ilya Maximets wrote:
> On 2/10/23 17:03, Adrián Moreno wrote:
>> From: Adrian Moreno <amorenoz@redhat.com>
>>
>> Add options to the IPFIX table configure the interval to send statistics
>> and template information.
>>
>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>
>> ---
>> - v3:
>>     - Removed unit tests which generate errors in Intel CI. Will submit in
>>       independent series.
>> - v2:
>>    - Fixed a potential race condition in unit test.
>> - v1:
>>    - Added unit test.
>> ---
>>   NEWS                         |  2 ++
>>   ofproto/ofproto-dpif-ipfix.c | 38 ++++++++++++++++++++++++++----------
>>   ofproto/ofproto.h            |  9 +++++++++
>>   vswitchd/bridge.c            | 17 ++++++++++++++++
>>   vswitchd/vswitch.ovsschema   | 12 +++++++++++-
>>   vswitchd/vswitch.xml         | 20 +++++++++++++++++++
>>   6 files changed, 87 insertions(+), 11 deletions(-)
>>
>> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
>> index 1a49cdffe..8ab609dfb 100644
>> --- a/vswitchd/vswitch.ovsschema
>> +++ b/vswitchd/vswitch.ovsschema
>> @@ -1,6 +1,6 @@
>>   {"name": "Open_vSwitch",
>>    "version": "8.3.1",
> 
> You're adding new columns.  Need to bump the version to 8.4.0.
> 
> The numbering scheme is described here:
>    https://docs.openvswitch.org/en/latest/ref/ovsdb.7/#schemas
> 

Thanks for the pointer, I did not know about that policy!

I'll resend the patch.
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index fe6055a27..4857046b8 100644
--- a/NEWS
+++ b/NEWS
@@ -50,6 +50,8 @@  v3.1.0 - xx xxx xxxx
      * Add new experimental PMD load based sleeping feature. PMD threads can
        request to sleep up to a user configured 'pmd-maxsleep' value under
        low load conditions.
+   - IPFIX template and statistics intervals can be configured through two new
+       options in the IPFIX table: 'template_interval' and 'stats_interval'.
 
 
 v3.0.0 - 15 Aug 2022
diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 1701d91e8..bae1eb9a9 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -140,6 +140,8 @@  struct dpif_ipfix_exporter {
     struct ovs_list cache_flow_start_timestamp_list;  /* ipfix_flow_cache_entry. */
     uint32_t cache_active_timeout;  /* In seconds. */
     uint32_t cache_max_flows;
+    uint32_t stats_interval;
+    uint32_t template_interval;
     char *virtual_obs_id;
     uint8_t virtual_obs_len;
 
@@ -174,11 +176,6 @@  struct dpif_ipfix {
 
 #define IPFIX_VERSION 0x000a
 
-/* When using UDP, IPFIX Template Records must be re-sent regularly.
- * The standard default interval is 10 minutes (600 seconds).
- * Cf. IETF RFC 5101 Section 10.3.6. */
-#define IPFIX_TEMPLATE_INTERVAL 600
-
 /* Cf. IETF RFC 5101 Section 3.1. */
 OVS_PACKED(
 struct ipfix_header {
@@ -637,6 +634,8 @@  ofproto_ipfix_bridge_exporter_options_equal(
             && a->sampling_rate == b->sampling_rate
             && a->cache_active_timeout == b->cache_active_timeout
             && a->cache_max_flows == b->cache_max_flows
+            && a->stats_interval == b->stats_interval
+            && a->template_interval == b->template_interval
             && a->enable_tunnel_sampling == b->enable_tunnel_sampling
             && a->enable_input_sampling == b->enable_input_sampling
             && a->enable_output_sampling == b->enable_output_sampling
@@ -674,6 +673,8 @@  ofproto_ipfix_flow_exporter_options_equal(
     return (a->collector_set_id == b->collector_set_id
             && a->cache_active_timeout == b->cache_active_timeout
             && a->cache_max_flows == b->cache_max_flows
+            && a->stats_interval == b->stats_interval
+            && a->template_interval == b->template_interval
             && a->enable_tunnel_sampling == b->enable_tunnel_sampling
             && sset_equals(&a->targets, &b->targets)
             && nullable_string_is_equal(a->virtual_obs_id, b->virtual_obs_id));
@@ -712,6 +713,9 @@  dpif_ipfix_exporter_init(struct dpif_ipfix_exporter *exporter)
     ovs_list_init(&exporter->cache_flow_start_timestamp_list);
     exporter->cache_active_timeout = 0;
     exporter->cache_max_flows = 0;
+    exporter->stats_interval = OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL;
+    exporter->template_interval = OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL;
+    exporter->last_stats_sent_time = 0;
     exporter->virtual_obs_id = NULL;
     exporter->virtual_obs_len = 0;
     hmap_init(&exporter->domains);
@@ -734,6 +738,9 @@  dpif_ipfix_exporter_clear(struct dpif_ipfix_exporter *exporter)
     exporter->last_stats_sent_time = 0;
     exporter->cache_active_timeout = 0;
     exporter->cache_max_flows = 0;
+    exporter->stats_interval = OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL;
+    exporter->template_interval = OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL;
+    exporter->last_stats_sent_time = 0;
     free(exporter->virtual_obs_id);
     exporter->virtual_obs_id = NULL;
     exporter->virtual_obs_len = 0;
@@ -761,6 +768,8 @@  dpif_ipfix_exporter_set_options(struct dpif_ipfix_exporter *exporter,
                                 const struct sset *targets,
                                 const uint32_t cache_active_timeout,
                                 const uint32_t cache_max_flows,
+                                const uint32_t stats_interval,
+                                const uint32_t template_interval,
                                 const char *virtual_obs_id) OVS_REQUIRES(mutex)
 {
     size_t virtual_obs_len;
@@ -775,6 +784,8 @@  dpif_ipfix_exporter_set_options(struct dpif_ipfix_exporter *exporter,
     }
     exporter->cache_active_timeout = cache_active_timeout;
     exporter->cache_max_flows = cache_max_flows;
+    exporter->stats_interval = stats_interval;
+    exporter->template_interval = template_interval;
     virtual_obs_len = virtual_obs_id ? strlen(virtual_obs_id) : 0;
     if (virtual_obs_len > IPFIX_VIRTUAL_OBS_MAX_LEN) {
         VLOG_WARN_RL(&rl, "Virtual obsevation ID too long (%d bytes), "
@@ -1007,6 +1018,7 @@  dpif_ipfix_bridge_exporter_set_options(
         if (!dpif_ipfix_exporter_set_options(
                 &exporter->exporter, &options->targets,
                 options->cache_active_timeout, options->cache_max_flows,
+                options->stats_interval, options->template_interval,
                 options->virtual_obs_id)) {
             return;
         }
@@ -1022,6 +1034,14 @@  dpif_ipfix_bridge_exporter_set_options(
     exporter->probability =
         MAX(1, UINT32_MAX / exporter->options->sampling_rate);
 
+    /* Configure static observation_domain_id. */
+    struct dpif_ipfix_domain *dom;
+    HMAP_FOR_EACH_SAFE (dom, hmap_node, &(exporter->exporter.domains)) {
+        dpif_ipfix_exporter_del_domain(&exporter->exporter, dom);
+    }
+    dpif_ipfix_exporter_insert_domain(&exporter->exporter,
+                                      options->obs_domain_id);
+
     /* Run over the cache as some entries might have expired after
      * changing the timeouts. */
     dpif_ipfix_cache_expire_now(&exporter->exporter, false);
@@ -1102,6 +1122,7 @@  dpif_ipfix_flow_exporter_set_options(
         if (!dpif_ipfix_exporter_set_options(
                 &exporter->exporter, &options->targets,
                 options->cache_active_timeout, options->cache_max_flows,
+                options->stats_interval, options->template_interval,
                 options->virtual_obs_id)) {
             return false;
         }
@@ -2882,7 +2903,7 @@  dpif_ipfix_should_send_template(struct dpif_ipfix_exporter *exporter,
                                                    observation_domain_id);
     }
 
-    if ((domain->last_template_set_time + IPFIX_TEMPLATE_INTERVAL)
+    if ((domain->last_template_set_time + exporter->template_interval)
         <= export_time_sec) {
         domain->last_template_set_time = export_time_sec;
         return true;
@@ -2922,10 +2943,7 @@  dpif_ipfix_cache_expire(struct dpif_ipfix_exporter *exporter,
             break;
         }
 
-        /* XXX: Make frequency of the (Options) Template and Exporter Process
-         * Statistics transmission configurable.
-         * Cf. IETF RFC 5101 Section 4.3. and 10.3.6. */
-        if ((exporter->last_stats_sent_time + IPFIX_TEMPLATE_INTERVAL)
+        if ((exporter->last_stats_sent_time + exporter->stats_interval)
              <= export_time_sec) {
             exporter->last_stats_sent_time = export_time_sec;
             ipfix_send_exporter_data_msg(exporter, export_time_sec);
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 4e15167ab..c79f372bc 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -72,6 +72,11 @@  struct ofproto_sflow_options {
     char *control_ip;
 };
 
+/* When using UDP, IPFIX Template Records must be re-sent regularly.
+ * The standard default interval is 10 minutes (600 seconds).
+ * Cf. IETF RFC 5101 Section 10.3.6. */
+#define OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL 600
+
 struct ofproto_ipfix_bridge_exporter_options {
     struct sset targets;
     uint32_t sampling_rate;
@@ -79,6 +84,8 @@  struct ofproto_ipfix_bridge_exporter_options {
     uint32_t obs_point_id;  /* Bridge-wide Observation Point ID. */
     uint32_t cache_active_timeout;
     uint32_t cache_max_flows;
+    uint32_t template_interval;
+    uint32_t stats_interval;
     bool enable_tunnel_sampling;
     bool enable_input_sampling;
     bool enable_output_sampling;
@@ -90,6 +97,8 @@  struct ofproto_ipfix_flow_exporter_options {
     struct sset targets;
     uint32_t cache_active_timeout;
     uint32_t cache_max_flows;
+    uint32_t template_interval;
+    uint32_t stats_interval;
     bool enable_tunnel_sampling;
     char *virtual_obs_id;
 };
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index abf2afe57..307a51527 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1542,6 +1542,17 @@  bridge_configure_ipfix(struct bridge *br)
         if (be_cfg->cache_max_flows) {
             be_opts.cache_max_flows = *be_cfg->cache_max_flows;
         }
+        if (be_cfg->stats_interval) {
+            be_opts.stats_interval = *be_cfg->stats_interval;
+        } else {
+            be_opts.stats_interval = OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL;
+        }
+        if (be_cfg->template_interval) {
+            be_opts.template_interval = *be_cfg->template_interval;
+        } else {
+            be_opts.template_interval =
+                OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL;
+        }
 
         be_opts.enable_tunnel_sampling = smap_get_bool(&be_cfg->other_config,
                                              "enable-tunnel-sampling", true);
@@ -1570,6 +1581,12 @@  bridge_configure_ipfix(struct bridge *br)
                     ? *fe_cfg->ipfix->cache_active_timeout : 0;
                 opts->cache_max_flows = fe_cfg->ipfix->cache_max_flows
                     ? *fe_cfg->ipfix->cache_max_flows : 0;
+                opts->stats_interval = fe_cfg->ipfix->stats_interval
+                    ? *fe_cfg->ipfix->stats_interval
+                    : OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL;
+                opts->template_interval = fe_cfg->ipfix->template_interval
+                    ? *fe_cfg->ipfix->template_interval
+                    : OFPROTO_IPFIX_DEFAULT_TEMPLATE_INTERVAL;
                 opts->enable_tunnel_sampling = smap_get_bool(
                                                    &fe_cfg->ipfix->other_config,
                                                   "enable-tunnel-sampling", true);
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 1a49cdffe..8ab609dfb 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@ 
 {"name": "Open_vSwitch",
  "version": "8.3.1",
- "cksum": "3012963480 26720",
+ "cksum": "142028610 27127",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -531,6 +531,16 @@ 
                           "minInteger": 0,
                           "maxInteger": 4294967295},
                   "min": 0, "max": 1}},
+       "stats_interval": {
+         "type": {"key": {"type": "integer",
+                          "minInteger": 1,
+                          "maxInteger": 3600},
+                  "min": 0, "max": 1}},
+       "template_interval": {
+         "type": {"key": {"type": "integer",
+                          "minInteger": 1,
+                          "maxInteger": 3600},
+                  "min": 0, "max": 1}},
        "other_config": {
          "type": {"key": "string", "value": "string",
                   "min": 0, "max": "unlimited"}},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 64f23302d..9c7bdb218 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -6592,6 +6592,26 @@  ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
       disabled.
     </column>
 
+    <column name="stats_interval">
+      <p>
+          Interval (in seconds) for sending IPFIX exporting process statistics
+          according to IETF RFC 5101 Section 4.3.
+      </p>
+      <p>
+          Default value is 600
+      </p>
+    </column>
+
+    <column name="template_interval">
+      <p>
+          Interval (in seconds) for sending IPFIX Template information for each
+          Observation Domain ID.
+      </p>
+      <p>
+          Default value is 600
+      </p>
+    </column>
+
     <column name="other_config" key="enable-tunnel-sampling"
             type='{"type": "boolean"}'>
       <p>