On Mon, Sep 18, 2017 at 7:31 PM, Han Zhou <[email protected]> wrote: > Thanks Russell for the quick work! > > On Mon, Sep 18, 2017 at 8:24 AM, Russell Bryant <[email protected]> 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 <[email protected]> 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. -- Russell Bryant _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
