On Fri, Jan 20, 2017 at 2:43 PM, Ben Pfaff <[email protected]> wrote:
> On Fri, Jan 20, 2017 at 12:29:49PM -0800, Mickey Spiegel wrote:
> > On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff <[email protected]> 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.
>
I just thought of that and checked how symtab works. I think "flags = 0"
should work.
>
> 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 can do that.
>
> > 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?
>
My thought was that the packet should look like it arrived on the
distributed gateway port.
One question is whether there can be any bad implications if
in_port and logical inport do not match?
It is also possible that the packet will return back to the original
port after SNAT and DNAT on the distributed gateway port, so
if I do not clear in_port then I would have to set flags.loopback.
Mickey
> > For patches 1 through 4, 6, and 8:
> > Acked-by: Mickey Spiegel <[email protected]>
> >
> > 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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
