In the physical processing of ovn-controller, there are two
sets of OF flows that are still fully recalculated every cycle:
Flows that aren't associated with any logical flow, and
Flows calculated based on multicast groups
Because these flows are recalculated fully each cycle, full
duplicates of existing OF flows are created and the OF management
code in ovn-controller pollutes the logs with false positive
warnings about repeated duplicates.
As a short term measure, ignore full duplicates for both of
these types of flows, but still warn if the action changes
(as that is not expected and may be indicative of a problem).
Signed-off-by: Ryan Moats <rmo...@us.ibm.com>
---
ovn/controller/ofctrl.c | 26 +++++++++++++++++++++-----
ovn/controller/ofctrl.h | 3 +++
ovn/controller/physical.c | 28 +++++++++++++++++++---------
3 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index f0451b7..2b26f2d 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -550,10 +550,10 @@ log_ovn_flow_rl(struct vlog_rate_limit *rl, enum
vlog_level level,
*
* This just assembles the desired flow tables in memory. Nothing is actually
* sent to the switch until a later call to ofctrl_run(). */
-void
-ofctrl_add_flow(uint8_t table_id, uint16_t priority,
+static void
+_ofctrl_add_flow(uint8_t table_id, uint16_t priority,
const struct match *match, const struct ofpbuf *actions,
- const struct uuid *uuid)
+ const struct uuid *uuid, bool dupwarn)
{
/* Structure that uses table_id+priority+various things as hashes. */
struct ovn_flow *f = xmalloc(sizeof *f);
@@ -591,8 +591,10 @@ ofctrl_add_flow(uint8_t table_id, uint16_t priority,
*/
if (ofpacts_equal(f->ofpacts, f->ofpacts_len,
d->ofpacts, d->ofpacts_len)) {
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
- log_ovn_flow_rl(&rl, VLL_INFO, f, "duplicate flow");
+ if (dupwarn) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
1);
+ log_ovn_flow_rl(&rl, VLL_INFO, f, "duplicate flow");
+ }
} else {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
log_ovn_flow_rl(&rl, VLL_WARN, f,
@@ -617,6 +619,20 @@ ofctrl_add_flow(uint8_t table_id, uint16_t priority,
f->uuid_hindex_node.hash);
}
+void
+ofctrl_add_flow(uint8_t table_id, uint16_t priority,
+ const struct match *match, const struct ofpbuf *actions,
+ const struct uuid *uuid) {
+ _ofctrl_add_flow(table_id, priority, match, actions, uuid, true);
+}
+
+void
+ofctrl_add_flow_no_warn(uint8_t table_id, uint16_t priority,
+ const struct match *match, const struct ofpbuf
*actions,
+ const struct uuid *uuid) {
+ _ofctrl_add_flow(table_id, priority, match, actions, uuid, false);
+}
+
/* Removes a bundles of flows from the flow table. */
void
ofctrl_remove_flows(const struct uuid *uuid)
diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
index 49b95b0..b591e82 100644
--- a/ovn/controller/ofctrl.h
+++ b/ovn/controller/ofctrl.h
@@ -42,6 +42,9 @@ struct ovn_flow *ofctrl_dup_flow(struct ovn_flow *source);
void ofctrl_add_flow(uint8_t table_id, uint16_t priority,
const struct match *, const struct ofpbuf *ofpacts,
const struct uuid *uuid);
+void ofctrl_add_flow_no_warn(uint8_t table_id, uint16_t priority,
+ const struct match *, const struct ofpbuf
*ofpacts,
+ const struct uuid *uuid);
void ofctrl_remove_flows(const struct uuid *uuid);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index a104e33..9e6dff4 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -549,8 +549,9 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
* group as the logical output port. */
put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, ofpacts_p);
- ofctrl_add_flow(OFTABLE_LOCAL_OUTPUT, 100,
- &match, ofpacts_p, &mc->header_.uuid);
+ /* Add flow, allowing expected full duplication to be ignored. */
+ ofctrl_add_flow_no_warn(OFTABLE_LOCAL_OUTPUT, 100,
+ &match, ofpacts_p, &mc->header_.uuid);
}
/* Table 32, priority 100.
@@ -587,8 +588,10 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
if (local_ports) {
put_resubmit(OFTABLE_LOCAL_OUTPUT, remote_ofpacts_p);
}
- ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 100,
- &match, remote_ofpacts_p, &mc->header_.uuid);
+ /* Add flow, allowing expected full duplication to be ignored. */
+ ofctrl_add_flow_no_warn(OFTABLE_REMOTE_OUTPUT, 100,
+ &match, remote_ofpacts_p,
+ &mc->header_.uuid);
}
}
sset_destroy(&remote_chassis);
@@ -794,8 +797,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
- ofctrl_add_flow(OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts,
- hc_uuid);
+ /* Add flow, allowing expected full duplication to be ignored. */
+ ofctrl_add_flow_no_warn(OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts,
+ hc_uuid);
}
/* Add flows for VXLAN encapsulations. Due to the limited amount of
@@ -828,7 +832,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
put_load(binding->tunnel_key, MFF_LOG_INPORT, 0, 15, &ofpacts);
put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
- ofctrl_add_flow(OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts,
hc_uuid);
+ /* Add flow, allowing expected full duplication to be ignored. */
+ ofctrl_add_flow_no_warn(OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts,
+ hc_uuid);
}
}
@@ -841,7 +847,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
match_init_catchall(&match);
ofpbuf_clear(&ofpacts);
put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
- ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 0, &match, &ofpacts, hc_uuid);
+ /* Add flow, allowing expected full duplication to be ignored. */
+ ofctrl_add_flow_no_warn(OFTABLE_REMOTE_OUTPUT, 0, &match, &ofpacts,
+ hc_uuid);
/* Table 34, Priority 0.
* =======================
@@ -855,7 +863,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
MFF_LOG_REGS;
#undef MFF_LOG_REGS
put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, &ofpacts);
- ofctrl_add_flow(OFTABLE_DROP_LOOPBACK, 0, &match, &ofpacts, hc_uuid);
+ /* Add flow, allowing expected full duplication to be ignored. */
+ ofctrl_add_flow_no_warn(OFTABLE_DROP_LOOPBACK, 0, &match, &ofpacts,
+ hc_uuid);
ofpbuf_uninit(&ofpacts);
HMAP_FOR_EACH_POP (tun, hmap_node, &tunnels) {
--
1.9.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
