ipfix cfg creation/deleting triggers revalidation. But this does
not cover the case where ipfix options changes, which also suppose
to trigger revalidation.
Fixes: a9f5ee1199e1 ("ofproto-dpif: Trigger revalidation when ipfix config
set.")
Signed-off-by: lic121 <[email protected]>
---
ofproto/ofproto-dpif-ipfix.c | 46 ++++++++++++++++++++++++++++----------------
ofproto/ofproto-dpif-ipfix.h | 2 +-
ofproto/ofproto-dpif.c | 7 +++----
3 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index fc927fe..7d316ca 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -926,17 +926,19 @@ dpif_ipfix_bridge_exporter_destroy(struct
dpif_ipfix_bridge_exporter *exporter)
static void
dpif_ipfix_bridge_exporter_set_options(
struct dpif_ipfix_bridge_exporter *exporter,
- const struct ofproto_ipfix_bridge_exporter_options *options)
+ const struct ofproto_ipfix_bridge_exporter_options *options,
+ bool *options_changed)
{
- bool options_changed;
-
if (!options || sset_is_empty(&options->targets)) {
/* No point in doing any work if there are no targets. */
- dpif_ipfix_bridge_exporter_clear(exporter);
+ if (exporter->options) {
+ dpif_ipfix_bridge_exporter_clear(exporter);
+ *options_changed = true;
+ }
return;
}
- options_changed = (
+ *options_changed = (
!exporter->options
|| !ofproto_ipfix_bridge_exporter_options_equal(
options, exporter->options));
@@ -945,7 +947,7 @@ dpif_ipfix_bridge_exporter_set_options(
* shortchanged in collectors (which indicates that opening one or
* more of the configured collectors failed, so that we should
* retry). */
- if (options_changed
+ if (*options_changed
|| collectors_count(exporter->exporter.collectors)
< sset_count(&options->targets)) {
if (!dpif_ipfix_exporter_set_options(
@@ -957,7 +959,7 @@ dpif_ipfix_bridge_exporter_set_options(
}
/* Avoid reconfiguring if options didn't change. */
- if (!options_changed) {
+ if (!*options_changed) {
return;
}
@@ -1015,17 +1017,19 @@ dpif_ipfix_flow_exporter_destroy(struct
dpif_ipfix_flow_exporter *exporter)
static bool
dpif_ipfix_flow_exporter_set_options(
struct dpif_ipfix_flow_exporter *exporter,
- const struct ofproto_ipfix_flow_exporter_options *options)
+ const struct ofproto_ipfix_flow_exporter_options *options,
+ bool *options_changed)
{
- bool options_changed;
-
if (sset_is_empty(&options->targets)) {
/* No point in doing any work if there are no targets. */
- dpif_ipfix_flow_exporter_clear(exporter);
+ if (exporter->options) {
+ dpif_ipfix_flow_exporter_clear(exporter);
+ *options_changed = true;
+ }
return true;
}
- options_changed = (
+ *options_changed = (
!exporter->options
|| !ofproto_ipfix_flow_exporter_options_equal(
options, exporter->options));
@@ -1034,7 +1038,7 @@ dpif_ipfix_flow_exporter_set_options(
* shortchanged in collectors (which indicates that opening one or
* more of the configured collectors failed, so that we should
* retry). */
- if (options_changed
+ if (*options_changed
|| collectors_count(exporter->exporter.collectors)
< sset_count(&options->targets)) {
if (!dpif_ipfix_exporter_set_options(
@@ -1046,7 +1050,7 @@ dpif_ipfix_flow_exporter_set_options(
}
/* Avoid reconfiguring if options didn't change. */
- if (!options_changed) {
+ if (!*options_changed) {
return true;
}
@@ -1069,7 +1073,7 @@ remove_flow_exporter(struct dpif_ipfix *di,
free(node);
}
-void
+bool
dpif_ipfix_set_options(
struct dpif_ipfix *di,
const struct ofproto_ipfix_bridge_exporter_options
*bridge_exporter_options,
@@ -1077,12 +1081,14 @@ dpif_ipfix_set_options(
size_t n_flow_exporters_options) OVS_EXCLUDED(mutex)
{
int i;
+ bool beo_changed, feo_changed, entry_changed;
struct ofproto_ipfix_flow_exporter_options *options;
struct dpif_ipfix_flow_exporter_map_node *node;
ovs_mutex_lock(&mutex);
dpif_ipfix_bridge_exporter_set_options(&di->bridge_exporter,
- bridge_exporter_options);
+ bridge_exporter_options,
+ &beo_changed);
/* Add new flow exporters and update current flow exporters. */
options = (struct ofproto_ipfix_flow_exporter_options *)
@@ -1095,10 +1101,14 @@ dpif_ipfix_set_options(
dpif_ipfix_flow_exporter_init(&node->exporter);
hmap_insert(&di->flow_exporter_map, &node->node,
hash_int(options->collector_set_id, 0));
+ feo_changed = true;
}
- if (!dpif_ipfix_flow_exporter_set_options(&node->exporter, options)) {
+ if (!dpif_ipfix_flow_exporter_set_options(&node->exporter,
+ options,
+ &entry_changed)) {
remove_flow_exporter(di, node);
}
+ feo_changed = entry_changed ? true : feo_changed;
options++;
}
@@ -1117,10 +1127,12 @@ dpif_ipfix_set_options(
}
if (i == n_flow_exporters_options) { /* Not found. */
remove_flow_exporter(di, node);
+ feo_changed = true;
}
}
ovs_mutex_unlock(&mutex);
+ return beo_changed || feo_changed;
}
struct dpif_ipfix *
diff --git a/ofproto/ofproto-dpif-ipfix.h b/ofproto/ofproto-dpif-ipfix.h
index 1f42cd5..75c0ab8 100644
--- a/ofproto/ofproto-dpif-ipfix.h
+++ b/ofproto/ofproto-dpif-ipfix.h
@@ -48,7 +48,7 @@ bool dpif_ipfix_get_bridge_exporter_output_sampling(const
struct dpif_ipfix *);
bool dpif_ipfix_get_flow_exporter_tunnel_sampling(const struct dpif_ipfix *,
const uint32_t);
bool dpif_ipfix_is_tunnel_port(const struct dpif_ipfix *, odp_port_t);
-void dpif_ipfix_set_options(
+bool dpif_ipfix_set_options(
struct dpif_ipfix *,
const struct ofproto_ipfix_bridge_exporter_options *,
const struct ofproto_ipfix_flow_exporter_options *, size_t);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6601f23..b42a30c 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2337,6 +2337,7 @@ set_ipfix(
struct dpif_ipfix *di = ofproto->ipfix;
bool has_options = bridge_exporter_options || flow_exporters_options;
bool new_di = false;
+ bool option_changed = false;
if (has_options && !di) {
di = ofproto->ipfix = dpif_ipfix_create();
@@ -2346,7 +2347,7 @@ set_ipfix(
if (di) {
/* Call set_options in any case to cleanly flush the flow
* caches in the last exporters that are to be destroyed. */
- dpif_ipfix_set_options(
+ option_changed = dpif_ipfix_set_options(
di, bridge_exporter_options, flow_exporters_options,
n_flow_exporters_options);
@@ -2363,9 +2364,7 @@ set_ipfix(
ofproto->ipfix = NULL;
}
- /* TODO: need to consider ipfix option changes more than
- * enable/disable */
- if (new_di || !ofproto->ipfix) {
+ if (new_di || option_changed) {
ofproto->backer->need_revalidate = REV_RECONFIGURE;
}
}
--
1.8.3.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
