On 12/13/16, 8:18 PM, "ovs-dev-boun...@openvswitch.org on behalf of Han Zhou" <ovs-dev-boun...@openvswitch.org on behalf of zhou...@gmail.com> 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 <zhou...@gmail.com> --- 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 d...@openvswitch.org 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev