[ovs-dev] [PATCH] ovn: Discard flows for non-local ports.

Russell Bryant russell at ovn.org
Tue Sep 19 16:22:47 UTC 2017


On Tue, Sep 19, 2017 at 9:38 AM, Russell Bryant <russell at ovn.org> wrote:
> On Mon, Sep 18, 2017 at 7:31 PM, Han Zhou <zhouhan at gmail.com> wrote:
>> Thanks Russell for the quick work!
>>
>> On Mon, Sep 18, 2017 at 8:24 AM, Russell Bryant <russell at ovn.org> wrote:
>>
>>> @@ -301,6 +305,22 @@ consider_logical_flow(struct controller_ctx *ctx,
>>>          if (m->match.wc.masks.conj_id) {
>>>              m->match.flow.conj_id += *conj_id_ofs;
>>>          }
>>> +        if (is_switch(ldp)) {
>>> +            unsigned int reg_index
>>> +                = (ingress ? MFF_LOG_INPORT : MFF_LOG_OUTPORT) -
>>> MFF_REG0;
>>> +            int64_t port_id = m->match.flow.regs[reg_index];
>>> +            if (port_id) {
>>> +                int64_t dp_id = lflow->logical_datapath->tunnel_key;
>>> +                char buf[16];
>>> +                snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id,
>>> port_id);
>>> +                if (!sset_contains(local_lport_ids, buf)) {
>>> +                    //VLOG_INFO("Matching on port id %"PRId64" dp
>>> %"PRId64", is NOT local", port_id, dp_id);
>>> +                    continue;
>>> +                } else {
>>> +                    //VLOG_INFO("Matching on port id %"PRId64" dp
>>> %"PRId64", is local", port_id, dp_id);
>>> +                }
>>> +            }
>>> +        }
>>>          if (!m->n) {
>>>              ofctrl_add_flow(flow_table, ptable, lflow->priority,
>>>                              lflow->header_.uuid.parts[0], &m->match,
>>> &ofpacts);
>>
>> I remember the expr_parse_string() is one of the biggest cost in
>> ovn-controller, so I wonder would it be better to move the check for
>> local_lport_ids before the parse happens, i.e. check against logical flows
>> instead of ovs flows?
>
> Yes, that may be better.  This was just a quick fix for the memory
> consumption issue.  There is also a small improvement to CPU usage in
> my basic testing.  I created 1000 ports with ACLs that used a 1000
> member address set.  Only one port was local.  It finished about 10%
> faster with the patch.
>
> Moving this to before the logical flow to OpenFlow translation would
> obviously be better, but it's not quite straight forward.  It's easy
> if you make assumptions about what the expression looks like, but
> doing it in a general way seems just as complicated as the existing
> expression parser.
>
>> Acked-by: Han Zhou <zhouhan at gmail.com>
>
> Thanks!
>
> I think I'll do a bit more testing before applying this.  I'm also not
> sure how far this should be backported.  The memory usage is bad
> enough with the default OpenStack security groups on large networks
> that I tend to think this should go back to 2.8 and 2.7 as well.

I ran this patch through OpenStack CI and it works there too.  I
applied this to master, branch-2.8, and branch-2.7.

I think the next piece that makes sense is looking at improving use of
conjunctive flows.

-- 
Russell Bryant


More information about the dev mailing list