diff mbox

[ovs-dev,patch_v1] ovs-vtep: Handle tunnel key configuration in any order.

Message ID 1468526997-7163-1-git-send-email-dlu998@gmail.com
State Superseded
Headers show

Commit Message

Darrell Ball July 14, 2016, 8:09 p.m. UTC
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 <dlu998@gmail.com>
---
 vtep/ovs-vtep | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Russell Bryant July 14, 2016, 8:51 p.m. UTC | #1
On Thu, Jul 14, 2016 at 4:09 PM, Darrell Ball <dlu998@gmail.com> wrote:

> 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 <dlu998@gmail.com>
> ---
>  vtep/ovs-vtep | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
> index e52c66f..871b999 100644
> --- a/vtep/ovs-vtep
> +++ b/vtep/ovs-vtep
> @@ -259,6 +259,21 @@ class Logical_Switch(object):
>          tunnels = set()
>          parse_ucast = True
>


> +        if not self.tunnel_key:
> +            vlog.info("Invalid tunnel key %s in %s; requery VTEP DB"
> +                      % (self.tunnel_key, self.name))
> +            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("update_remote_macs: using tunnel key %s in %s"
> +                          % (self.tunnel_key, self.name))
> +            else:
> +                vlog.info("Invalid tunnel key %s in %s post VTEP DB
> requery"
> +                          % (self.tunnel_key, self.name))
> +                return
> +



It looks like this largely copies code from setup_ls().  How about a new
function?



>          mac_list = vtep_ctl("list-remote-macs %s" % self.name
> ).splitlines()
>          for line in mac_list:
>              if (line.find("mcast-mac-remote") != -1):
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
Darrell Ball July 14, 2016, 9:59 p.m. UTC | #2
On Thu, Jul 14, 2016 at 1:51 PM, Russell Bryant <russell@ovn.org> wrote:

>
> On Thu, Jul 14, 2016 at 4:09 PM, Darrell Ball <dlu998@gmail.com> wrote:
>
>> 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 <dlu998@gmail.com>
>> ---
>>  vtep/ovs-vtep | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
>> index e52c66f..871b999 100644
>> --- a/vtep/ovs-vtep
>> +++ b/vtep/ovs-vtep
>> @@ -259,6 +259,21 @@ class Logical_Switch(object):
>>          tunnels = set()
>>          parse_ucast = True
>>
>
>
>> +        if not self.tunnel_key:
>> +            vlog.info("Invalid tunnel key %s in %s; requery VTEP DB"
>> +                      % (self.tunnel_key, self.name))
>> +            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("update_remote_macs: using tunnel key %s in
>> %s"
>> +                          % (self.tunnel_key, self.name))
>> +            else:
>> +                vlog.info("Invalid tunnel key %s in %s post VTEP DB
>> requery"
>> +                          % (self.tunnel_key, self.name))
>> +                return
>> +
>
>
>
> It looks like this largely copies code from setup_ls().  How about a new
> function?
>


It was a toss up about breaking out the 4 common lines originally.

+            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

However, I was leaning in the opposite direction, towards removing the
existing code since the error handling is wrong anyways and it is now
redundant.
It simplifies the logic as well.



>
>
>>          mac_list = vtep_ctl("list-remote-macs %s" % self.name
>> ).splitlines()
>>          for line in mac_list:
>>              if (line.find("mcast-mac-remote") != -1):
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>
>
>
> --
> Russell Bryant
>
diff mbox

Patch

diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
index e52c66f..871b999 100644
--- a/vtep/ovs-vtep
+++ b/vtep/ovs-vtep
@@ -259,6 +259,21 @@  class Logical_Switch(object):
         tunnels = set()
         parse_ucast = True
 
+        if not self.tunnel_key:
+            vlog.info("Invalid tunnel key %s in %s; requery VTEP DB"
+                      % (self.tunnel_key, self.name))
+            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("update_remote_macs: using tunnel key %s in %s"
+                          % (self.tunnel_key, self.name))
+            else:
+                vlog.info("Invalid tunnel key %s in %s post VTEP DB requery"
+                          % (self.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):