On Tue, Sep 19, 2017 at 9:38 AM, Russell Bryant <russ...@ovn.org> wrote: > On Mon, Sep 18, 2017 at 7:31 PM, Han Zhou <zhou...@gmail.com> wrote: >> Thanks Russell for the quick work! >> >> On Mon, Sep 18, 2017 at 8:24 AM, Russell Bryant <russ...@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 <zhou...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev