From: Han Zhou <[email protected]>
Date: Tuesday, December 13, 2016 at 10:28 PM
To: Darrell Ball <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: Re: [ovs-dev] [PATCH] ovn-controller: Fix duplicated flows in table 32.
Hi Darrell,
The duplicated flows are detected and filtered by the logic you mentioned, and
continuous INFO logs printed (with ratelimiting). I think I should update the
commit message a little bit. It is not "failures", but just the unexpected INFO
logs. Sorry for the confusion.
Could you also comment if this change make sense?
Thanks,
Han
Hi Han
There are two levels of filtering duplicates flows in the ovn specific code.
Another part of the commit message seems to need updating as well.
“In commit 475f0a2c it introduced a priority 150 flow for skipping
VXLAN traffic.”
should be changed to
“In commit 475f0a2c it introduced a priority 150 flow for filtering the sending
of
traffic received from vxlan tunnels back out tunnels.”
We only need to attempt to install this flow once per physical_run() loop
iteration, which is what it looks like your patch is doing.
I’ll test it in the morning to be sure.
On Tue, Dec 13, 2016 at 9:24 PM, Darrell Ball
<[email protected]<mailto:[email protected]>> wrote:
On 12/13/16, 8:18 PM,
"[email protected]<mailto:[email protected]> on
behalf of Han Zhou"
<[email protected]<mailto:[email protected]> on
behalf of [email protected]<mailto:[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]<mailto:[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]<mailto:[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
