[ovs-dev] [IPFIX FIXES 2/2] ipfix: allow empty targets column in table IPFIX

Romain Lenglet rlenglet at vmware.com
Tue Nov 19 19:17:38 UTC 2013


The "targets" column in IPFIX had a min=1 constraints, so OVSDB
implicitly adds an empty string "" into that column if no value is
given.  No connection can be opened to a target with address "", so
the whole IPFIX exporter for that row was disabled until that ""
target was removed by users.  That behavior is correct but proved to
be unintuitive to users.

This patch removes the min=1 constraint, to avoid the trouble for
users who insert IPFIX rows with no targets: it eliminates the log
messages due to failed connections to target "", and eliminates the
need to manually remove the "" target after row insertion.

This doesn't impact the behavior for any existing row, whether it has
a "" target or not.

Signed-off-by: Romain Lenglet <rlenglet at vmware.com>
---
 vswitchd/bridge.c          |   14 +++++++++-----
 vswitchd/vswitch.ovsschema |    4 ++--
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 8e43df3..65bea35 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -988,9 +988,12 @@ bridge_configure_sflow(struct bridge *br, int *sflow_bridge_number)
 static void
 bridge_configure_ipfix(struct bridge *br)
 {
-#define __VALID_FSCS(fscs) (fscs->ipfix && fscs->bridge == br->cfg)
+#define __VALID_IPFIX(ipfix) (ipfix && ipfix->n_targets > 0)
+#define __VALID_FSCS(fscs) (__VALID_IPFIX(fscs->ipfix) \
+                            && fscs->bridge == br->cfg)
 
     const struct ovsrec_ipfix *be_cfg = br->cfg->ipfix;
+    bool valid_be_cfg = __VALID_IPFIX(be_cfg);
     const struct ovsrec_flow_sample_collector_set *fe_cfg;
     struct ofproto_ipfix_bridge_exporter_options be_opts;
     struct ofproto_ipfix_flow_exporter_options *fe_opts = NULL;
@@ -1002,12 +1005,12 @@ bridge_configure_ipfix(struct bridge *br)
         }
     }
 
-    if (!be_cfg && n_fe_opts == 0) {
+    if (!valid_be_cfg && n_fe_opts == 0) {
         ofproto_set_ipfix(br->ofproto, NULL, NULL, 0);
         return;
     }
 
-    if (be_cfg) {
+    if (valid_be_cfg) {
         memset(&be_opts, 0, sizeof be_opts);
 
         sset_init(&be_opts.targets);
@@ -1051,10 +1054,10 @@ bridge_configure_ipfix(struct bridge *br)
         }
     }
 
-    ofproto_set_ipfix(br->ofproto, be_cfg ? &be_opts : NULL, fe_opts,
+    ofproto_set_ipfix(br->ofproto, valid_be_cfg ? &be_opts : NULL, fe_opts,
                       n_fe_opts);
 
-    if (be_cfg) {
+    if (valid_be_cfg) {
         sset_destroy(&be_opts.targets);
     }
 
@@ -1068,6 +1071,7 @@ bridge_configure_ipfix(struct bridge *br)
         free(fe_opts);
     }
 
+#undef __VALID_IPFIX
 #undef __VALID_FSCS
 }
 
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 78ebc89..9392457 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
  "version": "7.3.0",
- "cksum": "2811681289 20311",
+ "cksum": "2495231516 20311",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -412,7 +412,7 @@
    "IPFIX": {
      "columns": {
        "targets": {
-         "type": {"key": "string", "min": 1, "max": "unlimited"}},
+         "type": {"key": "string", "min": 0, "max": "unlimited"}},
        "sampling": {
          "type": {"key": {"type": "integer",
                           "minInteger": 1,
-- 
1.7.9.5



More information about the dev mailing list