On 12/13/16, 8:18 PM, "[email protected] on behalf of Han Zhou"
<[email protected] on behalf of [email protected]> wrote:
In commit 475f0a2c it introduced a priority 150 flow for skipping
VXLAN traffic. However, it added the flow for every remote port
processing, which results in duplicated flows and failures seen
in ovn-controller logs.
Thanks for working on this.
I see the following logs in ovn-controller:
2016-12-14T05:10:31.004Z|01685|ofctrl|INFO|dropping duplicate flow:
table_id=32, priority=150, reg10=0x2/0x2, actions=resubmit(,33)
which implies that the redundant attempts to add the same flow is being
filtered by the below
duplicate check in ofctrl_add_flow(..)
Is the duplicate flow check working in all cases ?
or
Is the check for duplicates failing in some instances and duplicate flows
are really created or leaked somehow ?
void
ofctrl_add_flow(struct hmap *desired_flows,
uint8_t table_id, uint16_t priority,
const struct match *match, const struct ofpbuf *actions)
{
struct ovn_flow *f = xmalloc(sizeof *f);
f->table_id = table_id;
f->priority = priority;
f->match = *match;
f->ofpacts = xmemdup(actions->data, actions->size);
f->ofpacts_len = actions->size;
f->hmap_node.hash = ovn_flow_hash(f);
if (ovn_flow_lookup(desired_flows, f)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
if (!VLOG_DROP_INFO(&rl)) {
char *s = ovn_flow_to_string(f);
VLOG_INFO("dropping duplicate flow: %s", s);
free(s);
}
ovn_flow_destroy(f);
return;
}
This fix moves the logic to global so that
the flow is added only once.
Signed-off-by: Han Zhou <[email protected]>
---
ovn/controller/physical.c | 47
++++++++++++++++++++++++-----------------------
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 48adb78..3b653dd 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -465,33 +465,16 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
} else {
/* Remote port connected by tunnel */
- /* Table 32, priority 150 and 100.
+ /* Table 32, priority 100.
* ===============================
*
- * Priority 150 is for packets received from a VXLAN tunnel
- * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
- * lack of needed metadata in VXLAN, explicitly skip sending
- * back out any tunnels and resubmit to table 33 for local
- * delivery.
- *
- * Priority 100 is for all other traffic which need to be sent
- * to a remote hypervisor. Each flow matches an output port
- * that includes a logical port on a remote hypervisor, and
- * tunnels the packet to that hypervisor.
+ * Priority 100 is for traffic that needs to be sent to a remote
+ * hypervisor. Each flow matches an output port that includes a
+ * logical port on a remote hypervisor, and tunnels the packet to
+ * that hypervisor.
*/
match_init_catchall(&match);
ofpbuf_clear(ofpacts_p);
- match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
- MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
-
- /* Resubmit to table 33. */
- put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
- ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, &match,
- ofpacts_p);
-
-
- match_init_catchall(&match);
- ofpbuf_clear(ofpacts_p);
/* Match MFF_LOG_DATAPATH, MFF_LOG_OUTPORT. */
match_set_metadata(&match, htonll(dp_key));
@@ -870,12 +853,30 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
}
}
+ /* Table 32, priority 150.
+ * ===============================
+ *
+ * Priority 150 is for packets received from a VXLAN tunnel
+ * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
+ * lack of needed metadata in VXLAN, explicitly skip sending
+ * back out any tunnels and resubmit to table 33 for local
+ * delivery.
+ */
+ struct match match;
+ match_init_catchall(&match);
+ ofpbuf_clear(&ofpacts);
+ match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
+ MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
+
+ /* Resubmit to table 33. */
+ put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
+ ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, &match,
&ofpacts);
+
/* Table 32, Priority 0.
* =======================
*
* Resubmit packets that are not directed at tunnels or part of a
* multicast group to the local output table. */
- struct match match;
match_init_catchall(&match);
ofpbuf_clear(&ofpacts);
put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
--
2.1.0
_______________________________________________
dev mailing list
[email protected]
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DgICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=1LugFnsEMayt1rjPUco3DmnurI9ccWoIiw2QMagLp7U&s=u79PaGJbU97fW5exZywHVQiigt2S3O300M2KLqPtArQ&e=
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
