Skip to content

Commit

Permalink
ipfix: allow empty targets column in table IPFIX
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
rlenglet authored and blp committed Nov 21, 2013
1 parent 3388e51 commit 88afd5f
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
18 changes: 13 additions & 5 deletions vswitchd/bridge.c
Original file line number Diff line number Diff line change
Expand Up @@ -984,19 +984,27 @@ bridge_configure_sflow(struct bridge *br, int *sflow_bridge_number)
sset_destroy(&oso.targets);
}

/* Returns whether a IPFIX row is valid. */
static bool
ovsrec_ipfix_is_valid(const struct ovsrec_ipfix *ipfix)
{
return ipfix && ipfix->n_targets > 0;
}

/* Returns whether a Flow_Sample_Collector_Set row is valid. */
static bool
ovsrec_fscs_is_valid(const struct ovsrec_flow_sample_collector_set *fscs,
const struct bridge *br)
{
return fscs->ipfix && fscs->bridge == br->cfg;
return ovsrec_ipfix_is_valid(fscs->ipfix) && fscs->bridge == br->cfg;
}

/* Set IPFIX configuration on 'br'. */
static void
bridge_configure_ipfix(struct bridge *br)
{
const struct ovsrec_ipfix *be_cfg = br->cfg->ipfix;
bool valid_be_cfg = ovsrec_ipfix_is_valid(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;
Expand All @@ -1008,12 +1016,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);
Expand Down Expand Up @@ -1057,10 +1065,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);
}

Expand Down
4 changes: 2 additions & 2 deletions vswitchd/vswitch.ovsschema
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{"name": "Open_vSwitch",
"version": "7.3.0",
"cksum": "2811681289 20311",
"cksum": "2495231516 20311",
"tables": {
"Open_vSwitch": {
"columns": {
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 88afd5f

Please sign in to comment.