2016-07-15 5:44 GMT-07:00 Russell Bryant <russ...@ovn.org>:

>
>
> On Thu, Jul 14, 2016 at 7:26 PM, Daniele Di Proietto <diproiet...@ovn.org>
> wrote:
>
>> Thanks Darrell, this looks good to me, but I'm not familiar with every
>> aspect of ovs-vtep.
>>
>> I'm mostly interested in this patch because it fixes an intermittent
>> failure of the testcase "ovn-controller-vtep - vtep-macs 1".
>>
>> Russell (since you commented on v1), does this look good to you?
>>
>>
> I'm not super familiar with ovs-vtep, either.  This patch looks straight
> forward enough to me, though.
>
> Acked-by: Russell Bryant <russ...@ovn.org>
>

Thanks Russell for the review and Darrell for the patch.

I added my ack and pushed this to master.


>
>
>> Thanks,
>>
>> Daniele
>>
>> 2016-07-14 14:59 GMT-07:00 Darrell Ball <dlu...@gmail.com>:
>>
>>> Presently, ovs-vtep expects the datapath tunnel key to be available
>>> in the VTEP DB at startup. This may not be the case which is also
>>> observed as interrmittent unit test failures. This patch allows
>>> for the tunnel key to later appear in the VTEP database.
>>>
>>> Signed-off-by: Darrell Ball <dlu...@gmail.com>
>>> ---
>>>
>>> v1->v2: Cleanup the existing code with broken error handling.
>>>
>>>  vtep/ovs-vtep | 28 ++++++++++++++--------------
>>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
>>> index e52c66f..b32a82a 100644
>>> --- a/vtep/ovs-vtep
>>> +++ b/vtep/ovs-vtep
>>> @@ -91,7 +91,6 @@ class Logical_Switch(object):
>>>          self.local_macs = set()
>>>          self.remote_macs = {}
>>>          self.unknown_dsts = set()
>>> -        self.tunnel_key = 0
>>>          self.setup_ls()
>>>          self.replication_mode = "service_node"
>>>
>>> @@ -99,16 +98,6 @@ class Logical_Switch(object):
>>>          vlog.info("destroying lswitch %s" % self.name)
>>>
>>>      def setup_ls(self):
>>> -        column = vtep_ctl("--columns=tunnel_key find logical_switch "
>>> -                          "name=%s" % self.name)
>>> -        tunnel_key = column.partition(":")[2].strip()
>>> -        if tunnel_key and isinstance(eval(tunnel_key),
>>> six.integer_types):
>>> -            self.tunnel_key = tunnel_key
>>> -            vlog.info("using tunnel key %s in %s"
>>> -                      % (self.tunnel_key, self.name))
>>> -        else:
>>> -            self.tunnel_key = 0
>>> -            vlog.warn("invalid tunnel key for %s, using 0" % self.name)
>>>
>>>          if ps_type:
>>>              ovs_vsctl("--may-exist add-br %s -- set Bridge %s
>>> datapath_type=%s"
>>> @@ -175,7 +164,7 @@ class Logical_Switch(object):
>>>          del self.ports[lbinding]
>>>          self.update_flood()
>>>
>>> -    def add_tunnel(self, tunnel):
>>> +    def add_tunnel(self, tunnel, tunnel_key):
>>>          global tun_id
>>>          vlog.info("adding tunnel %s" % tunnel)
>>>          encap, ip = tunnel.split("/")
>>> @@ -189,7 +178,7 @@ class Logical_Switch(object):
>>>
>>>          ovs_vsctl("add-port %s %s -- set Interface %s type=vxlan "
>>>                    "options:key=%s options:remote_ip=%s"
>>> -                  % (self.short_name, tun_name, tun_name,
>>> self.tunnel_key, ip))
>>> +                  % (self.short_name, tun_name, tun_name, tunnel_key,
>>> ip))
>>>
>>>          for i in range(10):
>>>              port_no = ovs_vsctl("get Interface %s ofport" % tun_name)
>>> @@ -259,6 +248,17 @@ class Logical_Switch(object):
>>>          tunnels = set()
>>>          parse_ucast = True
>>>
>>> +        column = vtep_ctl("--columns=tunnel_key find logical_switch "
>>> +                          "name=%s" % self.name)
>>> +        tunnel_key = column.partition(":")[2].strip()
>>> +        if tunnel_key and isinstance(eval(tunnel_key),
>>> six.integer_types):
>>> +            vlog.info("update_remote_macs: using tunnel key %s in %s"
>>> +                      % (tunnel_key, self.name))
>>> +        else:
>>> +            vlog.info("Invalid tunnel key %s in %s post VTEP DB
>>> requery"
>>> +                      % (tunnel_key, self.name))
>>> +            return
>>> +
>>>          mac_list = vtep_ctl("list-remote-macs %s" % self.name
>>> ).splitlines()
>>>          for line in mac_list:
>>>              if (line.find("mcast-mac-remote") != -1):
>>> @@ -282,7 +282,7 @@ class Logical_Switch(object):
>>>          old_tunnels = set(self.tunnels.keys())
>>>
>>>          for tunnel in tunnels.difference(old_tunnels):
>>> -            self.add_tunnel(tunnel)
>>> +            self.add_tunnel(tunnel, tunnel_key)
>>>
>>>          for tunnel in old_tunnels.difference(tunnels):
>>>              self.del_tunnel(tunnel)
>>> --
>>> 1.9.1
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
>>>
>>
>>
>
>
> --
> Russell Bryant
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to