On Wed, Jul 5, 2017 at 12:36 AM, Han Zhou <[email protected]> wrote: > > > On Tue, Jul 4, 2017 at 10:56 PM, Mickey Spiegel <[email protected]> > wrote: > > > > > > On Tue, Jun 27, 2017 at 10:42 AM, Han Zhou <[email protected]> wrote: > >> > >> > >> > >> On Tue, Jun 27, 2017 at 10:40 AM, Han Zhou <[email protected]> wrote: > >> > > >> > > >> > > >> > On Tue, Jun 27, 2017 at 10:12 AM, Mickey Spiegel < > [email protected]> wrote: > >> > > > >> > > > >> > > On Tue, Jun 27, 2017 at 1:02 AM, Han Zhou <[email protected]> > wrote: > >> > >> > >> > >> Localnet port was supposed to work for directly connected datapath > >> > >> only. However, the recursive local_datapath filling introduced a > >> > >> problem in below scenario: > >> > >> > >> > >> LS A <-> LR <-> LS B, port a@HV1 is on LS A, port b@HV2 is on LS > B. > >> > >> If B has localnet port, then ovn-controller on HV1 would think port > >> > >> b can be reached from HV1 by localnet port on LS B, which is wrong. > >> > > > >> > > > >> > > This scenario is flawed. Logical Routers should only be connected > to Logical Switches with localnet ports through gateway routers or > distributed gateway ports. Distributed gateway port logic will cause > traffic to be emitted from an appropriate hypervisor. It is designed to > work with logical switches with localnet ports, whereas normal router ports > on distributed logical routers were not. > >> > > > >> > >> > >> > >> This patch fixes it by adding hops information in local datapath > >> > >> which can tell if a local-datapath is directly connected to the > >> > >> hypervisor. > >> > > > >> > > > >> > > I have not run any tests or tried it, but I think this patch breaks > distributed gateway ports. We need to use the localnet port to get to the > outside world from the distributed gateway port. There is no problem with > the existing localnet port logic, when it is used as designed. > >> > > > >> > > >> > Hi Mickey, thanks for your comments! I agree the scenario is not real > use case, but I think it is still a bug which is revealed in such "flawed" > scenario, and the result is misleading, regarding the discussion [1]. So I > think it is still worth to be fixed. > >> > > >> It went out too fast: [1] https://mail.openvswitch.org/ > pipermail/ovs-dev/2017-June/334634.html > >> > > >> > This patch fixes only the "flaw" scenario, and I don't think it > breaks the distributed gateway scenario. > > > > > > I still believe that it breaks distributed gateway port functionality. > See more explanation below. > > > >> > >> The patch is just to make sure a remote port can be reached by > "localnet" port that is directly connected to the current HV. The logic is > in the context of how to reach a *remote port*. > > > > > > I still feel that you are trying to fix an unsupported scenario. > Moreover, I don't follow what you are trying to do. > > Yes, I am fixing an unsupported scenario, because when user configure it > this way, the localnet will be abused, and the behavior could be confusing. > This fix is just to make sure the localnet port works in the old and safe > way. Maybe this is not the good way to fix, and a better way might be > detecting such configuration (localnet port and non-gateway LR port on the > same LS) in northd and write a warning log. >
That would be my vote. > I was also thinking about abandoning this patch because I think connecting > logical switches with localnet ports by distributed LR could be useful in > some special case, but since that case would require some more changes and > there are open issues, I'd rather discuss it separately, independent of > this patch. > If you want to continue to try to support this type of connectivity, I agree that it should involve a more comprehensive discussion. You would still have to make changes such as ARP responses. I wonder how far such a solution would go down the path already travelled during development of the distributed gateway port. > > > It seems like you are making it like OVN does not have any localnets at > all, sending a Geneve tunneled packet from one side to the other (since the > sender HV1 does not see the localnet for LS B). It is not using the non-OVN > VXLAN, unless the underlying transport between HV1 and HV2 uses non-OVN > VXLAN, in which case it would be Geneve over VXLAN. > > > >> And the distributed gateway test cases are passed: > >> > > >> > OVN end-to-end tests > >> > > >> > 2336: ovn -- 1 LR with distributed router gateway port ok > >> > 2337: ovn -- send gratuitous arp for NAT rules on distributed router > ok > >> > > >> > >> Please let me know if I misunderstood anything, or the tests are not > complete. > > > > > > The tests are not complete. While there are some multi-node tests in > tests/ovn.at in the "1 LR with distributed router gateway port" test > case, they do not test the NAT functionality. In that existing test case, > without NAT, all north/south traffic goes through the redirect-chassis > which will trigger localnet even with your patch. > > > > Where I think your patch will break things is for distributed NAT rules, > where the traffic can go out of the local instance of the distributed > gateway port even on a chassis that is not the redirect-chassis. As in the > "1 LR with distributed router gateway port" test, the logical topology is: > > foo -- R1 -- distributed gateway port -- alice > > The physical topology is: > > hv1 hosts vif foo1 on LS foo. > > hv2 is the redirect-chassis for the distributed gateway port. > > There is a distributed NAT rule for foo1. > > When foo1 sends traffic to alice1 on hv3, on hv1 it goes > > foo -- R1 -- distributed gateway port -- alice -- localnet > > The localnet will then get the traffic to hv3 on LS alice, so that it > can reach the destination alice1. > > Sounds right. But I wonder if it is a realistic scenario or just another > scenario that we should not support. I think here alice is to model the > external physical network which is on the other side of the gateway, so it > seems not quite reasonable that a logical port alice1 on alice would be the > destination from foo. In the gateway scenario, wouldn't the alice LS be > just for connecting the outside world? > There is nothing restricting the use of VMs on the logical switch connecting to the outside world. Think of provider networks. This does work. I believe OpenStack allows this as well. > > > Similarly, traffic to the outside world will use > > foo -- R1 -- distributed gateway port -- alice -- localnet > > all on hv1. > > Sorry I have different understanding here. For connecting the outside > world, it is the port-binding of the localnet@alice that matters. When > localnet@alice is processed, the physical flows for that localnet port > are added. This patch doesn't change that behavior, because alice still > belongs to local-datapath on HV1, and the localnet@alice port-binding > will still be processed on HV1. > You are right. This should still work even with your patch. Mickey > > In both cases the localnet is needed on hv1, but the depth of alice on > hv1 is 2. > > I was testing these scenarios manually while developing the feature. > > > > When the distributed gateway port functionality was developed, NAT could > only be tested using the kernel module, i.e. "make check-kernel" or "make > check-kmod". However, this restricts the tests to be single node tests > which will not catch the issue introduced by your patch. Userspace NAT was > committed on June 2, but I do not know whether that feature allows for > multi-node NAT tests to be developed. If it does, then an automated version > of the scenario described above should be developed. It is pretty much the > topology of the "1 LR with distributed router gateway port" test case, > mixed with the "DNAT and SNAT on distributed router" test cases from tests/ > system-ovn.at. > > > > Mickey > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
