[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