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

Reply via email to