[ovs-dev] [PATCH] ovn-controller: use localnet port for directly connected datapath only.

Han Zhou zhouhan at gmail.com
Tue Jun 27 17:40:56 UTC 2017


On Tue, Jun 27, 2017 at 10:12 AM, Mickey Spiegel <mickeys.dev at gmail.com>
wrote:
>
>
> On Tue, Jun 27, 2017 at 1:02 AM, Han Zhou <zhouhan at gmail.com> 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 at HV1 is on LS A, port b at 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.


This patch fixes only the "flaw" scenario, and I don't think it breaks the
distributed gateway scenario. 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*. 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



>
>>
>> Signed-off-by: Han Zhou <zhouhan at gmail.com>
>> Reported-by: Qianyu Wang <wang.qianyu at zte.com.cn>
>> ---
>>  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
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list