ofproto-ipfix: Use per-domain template timeouts.

IPFIX templates have to be sent for each Observation Domain ID.
Currently, a timer is kept at each dpif_ipfix_exporter to send them.
This works fine for per-bridge sampling where there is only one
Observation Domain ID per exporter. However, this is does not work for
per-flow sampling where more than one Observation Domain IDs can be
specified by the controller. In this case, ovs-vswitchd will only send
template information for one (arbitrary) DomainID.

Fix per-flow sampling by using an hmap to keep a timer for each
Observation Domain ID.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

1 files changed, 105 insertions, 24 deletions

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 742eed399..f13478a88 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -124,11 +124,18 @@ struct dpif_ipfix_port {
     uint32_t ifindex;
 };
 
+struct dpif_ipfix_domain {
+    struct hmap_node hmap_node; /* In struct dpif_ipfix_exporter's domains. */
+    time_t last_template_set_time;
+};
+
 struct dpif_ipfix_exporter {
     uint32_t exporter_id; /* Exporting Process identifier */
-    struct collectors *collectors;
     uint32_t seq_number;
-    time_t last_template_set_time;
+    struct collectors *collectors;
+    struct hmap domains; /* Contains struct dpif_ipfix_domain indexed by
+                            observation domain id. */
+    time_t last_stats_sent_time;
     struct hmap cache_flow_key_map;  /* ipfix_flow_cache_entry. */
     struct ovs_list cache_flow_start_timestamp_list;  /* ipfix_flow_cache_entry. */
     uint32_t cache_active_timeout;  /* In seconds. */
@@ -617,6 +624,9 @@ static void get_export_time_now(uint64_t *, uint32_t *);
 
 static void dpif_ipfix_cache_expire_now(struct dpif_ipfix_exporter *, bool);
 
+static void dpif_ipfix_exporter_del_domain(struct dpif_ipfix_exporter *,
+                                           struct dpif_ipfix_domain *);
+
 static bool
 ofproto_ipfix_bridge_exporter_options_equal(
     const struct ofproto_ipfix_bridge_exporter_options *a,
@@ -697,13 +707,14 @@ dpif_ipfix_exporter_init(struct dpif_ipfix_exporter *exporter)
     exporter->exporter_id = ++exporter_total_count;
     exporter->collectors = NULL;
     exporter->seq_number = 1;
-    exporter->last_template_set_time = 0;
+    exporter->last_stats_sent_time = 0;
     hmap_init(&exporter->cache_flow_key_map);
     ovs_list_init(&exporter->cache_flow_start_timestamp_list);
     exporter->cache_active_timeout = 0;
     exporter->cache_max_flows = 0;
     exporter->virtual_obs_id = NULL;
     exporter->virtual_obs_len = 0;
+    hmap_init(&exporter->domains);
 
     memset(&exporter->ipfix_global_stats, 0,
            sizeof(struct dpif_ipfix_global_stats));
@@ -711,6 +722,7 @@ dpif_ipfix_exporter_init(struct dpif_ipfix_exporter *exporter)
 
 static void
 dpif_ipfix_exporter_clear(struct dpif_ipfix_exporter *exporter)
+    OVS_REQUIRES(mutex)
 {
     /* Flush the cache with flow end reason "forced end." */
     dpif_ipfix_cache_expire_now(exporter, true);
@@ -719,22 +731,29 @@ dpif_ipfix_exporter_clear(struct dpif_ipfix_exporter *exporter)
     exporter->exporter_id = 0;
     exporter->collectors = NULL;
     exporter->seq_number = 1;
-    exporter->last_template_set_time = 0;
+    exporter->last_stats_sent_time = 0;
     exporter->cache_active_timeout = 0;
     exporter->cache_max_flows = 0;
     free(exporter->virtual_obs_id);
     exporter->virtual_obs_id = NULL;
     exporter->virtual_obs_len = 0;
 
+    struct dpif_ipfix_domain *dom;
+    HMAP_FOR_EACH_SAFE (dom, hmap_node, &exporter->domains) {
+        dpif_ipfix_exporter_del_domain(exporter, dom);
+    }
+
     memset(&exporter->ipfix_global_stats, 0,
            sizeof(struct dpif_ipfix_global_stats));
 }
 
 static void
 dpif_ipfix_exporter_destroy(struct dpif_ipfix_exporter *exporter)
+    OVS_REQUIRES(mutex)
 {
     dpif_ipfix_exporter_clear(exporter);
     hmap_destroy(&exporter->cache_flow_key_map);
+    hmap_destroy(&exporter->domains);
 }
 
 static bool
@@ -742,7 +761,7 @@ 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 char *virtual_obs_id)
+                                const char *virtual_obs_id) OVS_REQUIRES(mutex)
 {
     size_t virtual_obs_len;
     collectors_destroy(exporter->collectors);
@@ -769,6 +788,37 @@ dpif_ipfix_exporter_set_options(struct dpif_ipfix_exporter *exporter,
     return true;
 }
 
+static struct dpif_ipfix_domain *
+dpif_ipfix_exporter_find_domain(const struct dpif_ipfix_exporter *exporter,
+                                uint32_t domain_id) OVS_REQUIRES(mutex)
+{
+    struct dpif_ipfix_domain *dom;
+    HMAP_FOR_EACH_WITH_HASH (dom, hmap_node, hash_int(domain_id, 0),
+                             &exporter->domains) {
+        return dom;
+    }
+    return NULL;
+}
+
+static struct dpif_ipfix_domain *
+dpif_ipfix_exporter_insert_domain(struct dpif_ipfix_exporter *exporter,
+                                  const uint32_t domain_id) OVS_REQUIRES(mutex)
+{
+    struct dpif_ipfix_domain *dom = xmalloc(sizeof *dom);
+    dom->last_template_set_time = 0;
+    hmap_insert(&exporter->domains, &dom->hmap_node, hash_int(domain_id, 0));
+    return dom;
+}
+
+static void
+dpif_ipfix_exporter_del_domain(struct dpif_ipfix_exporter *exporter,
+                               struct dpif_ipfix_domain *dom)
+    OVS_REQUIRES(mutex)
+{
+    hmap_remove(&exporter->domains, &dom->hmap_node);
+    free(dom);
+}
+
 static struct dpif_ipfix_port *
 dpif_ipfix_find_port(const struct dpif_ipfix *di,
                      odp_port_t odp_port) OVS_REQUIRES(mutex)
@@ -909,6 +959,7 @@ dpif_ipfix_bridge_exporter_init(struct dpif_ipfix_bridge_exporter *exporter)
 
 static void
 dpif_ipfix_bridge_exporter_clear(struct dpif_ipfix_bridge_exporter *exporter)
+    OVS_REQUIRES(mutex)
 {
     dpif_ipfix_exporter_clear(&exporter->exporter);
     ofproto_ipfix_bridge_exporter_options_destroy(exporter->options);
@@ -918,6 +969,7 @@ dpif_ipfix_bridge_exporter_clear(struct dpif_ipfix_bridge_exporter *exporter)
 
 static void
 dpif_ipfix_bridge_exporter_destroy(struct dpif_ipfix_bridge_exporter *exporter)
+    OVS_REQUIRES(mutex)
 {
     dpif_ipfix_bridge_exporter_clear(exporter);
     dpif_ipfix_exporter_destroy(&exporter->exporter);
@@ -927,7 +979,7 @@ static void
 dpif_ipfix_bridge_exporter_set_options(
     struct dpif_ipfix_bridge_exporter *exporter,
     const struct ofproto_ipfix_bridge_exporter_options *options,
-    bool *options_changed)
+    bool *options_changed) OVS_REQUIRES(mutex)
 {
     if (!options || sset_is_empty(&options->targets)) {
         /* No point in doing any work if there are no targets. */
@@ -1003,6 +1055,7 @@ dpif_ipfix_flow_exporter_init(struct dpif_ipfix_flow_exporter *exporter)
 
 static void
 dpif_ipfix_flow_exporter_clear(struct dpif_ipfix_flow_exporter *exporter)
+    OVS_REQUIRES(mutex)
 {
     dpif_ipfix_exporter_clear(&exporter->exporter);
     ofproto_ipfix_flow_exporter_options_destroy(exporter->options);
@@ -1011,6 +1064,7 @@ dpif_ipfix_flow_exporter_clear(struct dpif_ipfix_flow_exporter *exporter)
 
 static void
 dpif_ipfix_flow_exporter_destroy(struct dpif_ipfix_flow_exporter *exporter)
+    OVS_REQUIRES(mutex)
 {
     dpif_ipfix_flow_exporter_clear(exporter);
     dpif_ipfix_exporter_destroy(&exporter->exporter);
@@ -1020,7 +1074,7 @@ static bool
 dpif_ipfix_flow_exporter_set_options(
     struct dpif_ipfix_flow_exporter *exporter,
     const struct ofproto_ipfix_flow_exporter_options *options,
-    bool *options_changed)
+    bool *options_changed) OVS_REQUIRES(mutex)
 {
     if (sset_is_empty(&options->targets)) {
         /* No point in doing any work if there are no targets. */
@@ -1071,6 +1125,7 @@ dpif_ipfix_flow_exporter_set_options(
 static void
 remove_flow_exporter(struct dpif_ipfix *di,
                      struct dpif_ipfix_flow_exporter_map_node *node)
+                     OVS_REQUIRES(mutex)
 {
     hmap_remove(&di->flow_exporter_map, &node->node);
     dpif_ipfix_flow_exporter_destroy(&node->exporter);
@@ -2000,6 +2055,7 @@ static void
 ipfix_cache_update(struct dpif_ipfix_exporter *exporter,
                    struct ipfix_flow_cache_entry *entry,
                    enum ipfix_sampled_packet_type sampled_pkt_type)
+                   OVS_REQUIRES(mutex)
 {
     struct ipfix_flow_cache_entry *old_entry;
     size_t current_flows = 0;
@@ -2811,14 +2867,36 @@ dpif_ipfix_flow_sample(struct dpif_ipfix *di, const struct dp_packet *packet,
     ovs_mutex_unlock(&mutex);
 }
 
+static bool
+dpif_ipfix_should_send_template(struct dpif_ipfix_exporter *exporter,
+                                const uint32_t observation_domain_id,
+                                const uint32_t export_time_sec)
+    OVS_REQUIRES(mutex)
+{
+    struct dpif_ipfix_domain *domain;
+    domain = dpif_ipfix_exporter_find_domain(exporter,
+                                             observation_domain_id);
+    if (!domain) {
+        /* First time we see this obs_domain_id. */
+        domain = dpif_ipfix_exporter_insert_domain(exporter,
+                                                   observation_domain_id);
+    }
+
+    if ((domain->last_template_set_time + IPFIX_TEMPLATE_INTERVAL)
+        <= export_time_sec) {
+        domain->last_template_set_time = export_time_sec;
+        return true;
+    }
+    return false;
+}
+
 static void
 dpif_ipfix_cache_expire(struct dpif_ipfix_exporter *exporter,
                         bool forced_end, const uint64_t export_time_usec,
-                        const uint32_t export_time_sec)
+                        const uint32_t export_time_sec) OVS_REQUIRES(mutex)
 {
     struct ipfix_flow_cache_entry *entry;
     uint64_t max_flow_start_timestamp_usec;
-    bool template_msg_sent = false;
     enum ipfix_flow_end_reason flow_end_reason;
 
     if (ovs_list_is_empty(&exporter->cache_flow_start_timestamp_list)) {
@@ -2844,25 +2922,28 @@ dpif_ipfix_cache_expire(struct dpif_ipfix_exporter *exporter,
             break;
         }
 
-        ovs_list_remove(&entry->cache_flow_start_timestamp_list_node);
-        hmap_remove(&exporter->cache_flow_key_map,
-                    &entry->flow_key_map_node);
+        /* 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)
+             <= export_time_sec) {
+            exporter->last_stats_sent_time = export_time_sec;
+            ipfix_send_exporter_data_msg(exporter, export_time_sec);
+        }
 
-         /* 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 (!template_msg_sent
-            && (exporter->last_template_set_time + IPFIX_TEMPLATE_INTERVAL)
-                <= export_time_sec) {
+        if (dpif_ipfix_should_send_template(exporter,
+                                            entry->flow_key.obs_domain_id,
+                                            export_time_sec)) {
+            VLOG_DBG("Sending templates for ObservationDomainID %"PRIu32,
+                     entry->flow_key.obs_domain_id);
             ipfix_send_template_msgs(exporter, export_time_sec,
                                      entry->flow_key.obs_domain_id);
-            exporter->last_template_set_time = export_time_sec;
-            template_msg_sent = true;
-
-            /* Send Exporter Process Statistics. */
-            ipfix_send_exporter_data_msg(exporter, export_time_sec);
         }
 
+        ovs_list_remove(&entry->cache_flow_start_timestamp_list_node);
+        hmap_remove(&exporter->cache_flow_key_map,
+                    &entry->flow_key_map_node);
+
         /* XXX: Group multiple data records for the same obs domain id
          * into the same message. */
         ipfix_send_data_msg(exporter, export_time_sec, entry, flow_end_reason);
@@ -2883,7 +2964,7 @@ get_export_time_now(uint64_t *export_time_usec, uint32_t *export_time_sec)
 
 static void
 dpif_ipfix_cache_expire_now(struct dpif_ipfix_exporter *exporter,
-                            bool forced_end)
+                            bool forced_end) OVS_REQUIRES(mutex)
 {
     uint64_t export_time_usec;
     uint32_t export_time_sec;