On Sat, Jan 21, 2017 at 11:23 AM, Ben Pfaff <[email protected]> wrote: > On Fri, Jan 20, 2017 at 04:00:34PM -0800, Mickey Spiegel wrote: > > On Fri, Jan 20, 2017 at 3:33 PM, Ben Pfaff <[email protected]> wrote: > > > > > On Fri, Jan 20, 2017 at 03:17:19PM -0800, Mickey Spiegel wrote: > > > > 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: > > > > > > 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. > > > > > > Once a packet has been translated from physical to logical, the only > > > real use of in_port is to discard packets that would loop back on the > > > physical input port. If we want to disable that, one way to do it is > to > > > set flags.loopback to 1, although that also disables discarding packets > > > that would loop back to the logical input port. > > > > > > > I was thinking that egress loopback should be similar to traversing a > > logical patch port. When traversing a logical patch port, in_port is > > cleared. > > > > If you have a problem with adding in_port to symtab, then I can just > > set flags.loopback to 1. I would rather have the loopback check, but > > at the moment add_route in ovn-northd.c sets flags.loopback to 1 in > > all cases, so there is no difference on a logical router. Otherwise, I > > will probably throw in a small patch to add in_port to symtab. > > I don't want to add in_port to symtab because it breaks layering. That > is, in_port is a physical port, not a logical port, and the logical > flows don't have a way to understand its value. I guess that they can > reasonably set it to "0", but that's a pretty limited use. > > I wouldn't want to name it in_port in any case because that would just > cause confusion. > > If setting flags.loopback to 1 is a reasonable solution, let's use that. >
I will go with flags.loopback = 1. As I mentioned earlier, at the moment flags.loopback is always set to 1 on logical router datapaths, in the IP routing pipeline stage. I don't think that is the right thing to do, but I am not sure what will break if that is removed. In any case, IP routing is always protected by ttl, so even if there is a loop the packet will eventually be dropped. > > Stepping back a bit, I am not certain that it ever makes sense to > discard packets in OVN because the physical ingress and egress ports > match. It might be reasonable to always set the physical ingress port > to 0 as part of physical to logical translation at the beginning of the > pipeline. > The check does seem to be somewhat redundant with the logical port loopback check in table 34. The only way to pass the check in table 34 is if flags.loopback is set, in which case in_port will be cleared in table 64. Mickey _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
