On Fri, Jan 20, 2017 at 12:29:49PM -0800, Mickey Spiegel wrote:
> On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff <b...@ovn.org> wrote:
>
> > I believe that, with these patches, egress loopback as proposed by Mickey's
> > patches can be implemented with:
> > clone { inport = outport; outport = ""; flags.loopback = 0;
> > reg0 = 0; reg1 = 0; ... regN = 0;
> > next(pipeline=ingress, table=0); }
> >
>
> My main concern is maintainability as new flags or registers are added.
> Having one line of code buried deep inside ovn/northd/ovn-northd.c that
> needs to be updated whenever a flag or register is added worries me.
> Does it make sense to add "clear_regs" and "clear_flags" actions in
> order to address that concern?
I don't think that flags are a problem. We can just write "flags = 0;"
instead of specific flags; I didn't think of that before.
To ensure that all the registers are cleared, we can just use a loop in
ovn-northd:
for (int i = 0; i < MFF_N_LOG_REGS; i++) {
ds_put_format(&actions, "reg%d = 0; ", i);
}
> I would also need to add in_port to symtab in ovn/lib/logical-fields.c so
> that I can clear it.
Can you explain why in_port needs to be cleared?
> For patches 1 through 4, 6, and 8:
> Acked-by: Mickey Spiegel <mickeys....@gmail.com>
>
> I commented separately on patches 5 and 7.
>
> I could not apply patches 9 and 10 since I manually fixed patch 7
> and the indexes did not match.
Thanks a lot for the reviews.
I'll repost.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
