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. Mickey > Signed-off-by: Han Zhou <[email protected]> > Reported-by: Qianyu Wang <[email protected]> > --- > ovn/controller/binding.c | 1 + > ovn/controller/ovn-controller.h | 3 +++ > ovn/controller/physical.c | 2 +- > 3 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c > index bb76608..03c310d 100644 > --- a/ovn/controller/binding.c > +++ b/ovn/controller/binding.c > @@ -129,6 +129,7 @@ add_local_datapath__(const struct ldatapath_index > *ldatapaths, > ovs_assert(ld->ldatapath); > ld->localnet_port = NULL; > ld->has_local_l3gateway = has_local_l3gateway; > + ld->hops = depth; > > if (depth >= 100) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn- > controller.h > index 4bc0467..9b85087 100644 > --- a/ovn/controller/ovn-controller.h > +++ b/ovn/controller/ovn-controller.h > @@ -66,6 +66,9 @@ struct local_datapath { > /* True if this datapath contains an l3gateway port located on this > * hypervisor. */ > bool has_local_l3gateway; > + > + /* Number of logical hops the datapath is connected to this > hypervisor. */ > + int hops; > }; > > struct local_datapath *get_local_datapath(const struct hmap *, > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c > index f2d9676..7051906 100644 > --- a/ovn/controller/physical.c > +++ b/ovn/controller/physical.c > @@ -151,7 +151,7 @@ get_localnet_port(struct hmap *local_datapaths, > int64_t tunnel_key) > { > struct local_datapath *ld = get_local_datapath(local_datapaths, > tunnel_key); > - return ld ? ld->localnet_port : NULL; > + return ld && !ld->hops ? ld->localnet_port : NULL; > } > > /* Datapath zone IDs for connection tracking and NAT */ > -- > 2.1.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
