[PATCH 1/4] Devices.OvsBridgeDevice: add _dict_to_keyvalues method

[Jan Tluka]

This method transforms dictionary items into key=value pairs
that are used in ovs syntax.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/Devices/OvsBridgeDevice.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lnst/Devices/OvsBridgeDevice.py b/lnst/Devices/OvsBridgeDevice.py
index 7e5bf0c4..044b5411 100644
--- a/lnst/Devices/OvsBridgeDevice.py
+++ b/lnst/Devices/OvsBridgeDevice.py
@@ -35,6 +35,13 @@ class OvsBridgeDevice(SoftDevice):
     def destroy(self):
         exec_cmd("ovs-vsctl del-br %s" % self.name)
 
+    def _dict_to_keyvalues(self, options):
+        opts = ""
+        for opt_name, opt_value in options.items():
+            opts += " %s=%s" % (opt_name, opt_value)
+
+        return opts
+
     def port_add(self, dev, **kwargs):
         options = ""
         for opt_name, opt_value in kwargs.items():
-- 
2.21.1

[PATCH 2/4] Devices.OvsBridgeDevice: add _interface_cmd method

[Jan Tluka]

This method is used to generate the 'set Interface ...' command
to modify Interface table in the OVS database.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/Devices/OvsBridgeDevice.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lnst/Devices/OvsBridgeDevice.py b/lnst/Devices/OvsBridgeDevice.py
index 044b5411..4205470e 100644
--- a/lnst/Devices/OvsBridgeDevice.py
+++ b/lnst/Devices/OvsBridgeDevice.py
@@ -42,6 +42,14 @@ class OvsBridgeDevice(SoftDevice):
 
         return opts
 
+    def _interface_cmd(self, interface, options):
+        keyvalues = self._dict_to_keyvalues(options)
+        cmd = ""
+        if len(keyvalues):
+            cmd = " -- set Interface {} {}".format(interface, keyvalues)
+
+        return cmd
+
     def port_add(self, dev, **kwargs):
         options = ""
         for opt_name, opt_value in kwargs.items():
-- 
2.21.1

[PATCH 3/4] Devices.OvsBridgeDevice: refactor port_add method

[Jan Tluka]

The internal_port_add was fundamentaly wrong as there's no such thing as an
internal port in OpenVSwitch. Instead a user may add a port with internal
device implementation (type internal).

This patch removes the internal_port_add method and refactors port_add method
to fit all of the following cases:
 * ovs-vsctl port-add t_br0 eth0
   that adds an existing device
 * ovs-vsctl port-add t_br0 vnet0 -- set Interface vnet0 ofport_request=5
   same with openflow rules
 * ovs-vsctl port-add t_br0 int0 -- set Interface int0 type=internal
   that adds a port with interface of type internal
 * ovs-vsctl port-add t_br0 vxlan0 -- set Interface vxlan0 type=vxlan ...
   that adds a port with interface of type vxlan

The method now takes three parameters
 * device which must be a Device instance, applicable only for internal and
   tunnel type interfaces
 * port_options, a dictionary of any Port column key-values, e.g. {'tag': 9}
 * interface_options, a dictionary of any Interface column key-values, e.g.
   {'type': 'internal'}, that would generate command like
   'set Interface iface type=internal'

The method tunnel_add was modified to use the refactored port_add method to
remove duplicated code.

All recipes that previously used any of the modified methods were updated.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/Devices/OvsBridgeDevice.py               | 59 ++++++-------------
 lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py     |  7 ++-
 lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py       |  5 +-
 ...rtualOvsBridgeVlanInGuestMirroredRecipe.py |  2 +-
 ...irtualOvsBridgeVlanInHostMirroredRecipe.py |  2 +-
 .../ENRT/VirtualOvsBridgeVlanInHostRecipe.py  |  2 +-
 .../VirtualOvsBridgeVlansOverBondRecipe.py    |  2 +-
 7 files changed, 31 insertions(+), 48 deletions(-)

diff --git a/lnst/Devices/OvsBridgeDevice.py b/lnst/Devices/OvsBridgeDevice.py
index 4205470e..28d786bb 100644
--- a/lnst/Devices/OvsBridgeDevice.py
+++ b/lnst/Devices/OvsBridgeDevice.py
@@ -50,15 +50,23 @@ class OvsBridgeDevice(SoftDevice):
 
         return cmd
 
-    def port_add(self, dev, **kwargs):
-        options = ""
-        for opt_name, opt_value in kwargs.items():
-            if opt_name == "set_iface" and opt_value:
-                options = (" -- set Interface %s" + options) % dev.name
-            else:
-                options += " %s=%s" % (opt_name, opt_value)
+    def port_add(self, device=None, port_options={}, interface_options={}):
+        if device is None:
+            dev_name = interface_options.get('name',
+                self._if_manager.assign_name(interface_options['type']))
+        else:
+            dev_name = device.name
+
+        exec_cmd("ovs-vsctl add-port {} {}{}{}".format(self.name, dev_name,
+            self._dict_to_keyvalues(port_options),
+            self._interface_cmd(dev_name, interface_options)))
+
+        iface = None
+        if 'type' in interface_options and interface_options['type'] ==
'internal':
+            iface = self._if_manager.get_device_by_name(dev_name)
+            iface._enable()
 
-        exec_cmd("ovs-vsctl add-port %s %s%s" % (self.name, dev.name,
options))
+        return iface
 
     def port_del(self, dev):
         if isinstance(dev, Device):
@@ -83,39 +91,10 @@ class OvsBridgeDevice(SoftDevice):
     def bond_del(self, dev):
         self.port_del(dev)
 
-    def internal_port_add(self, **kwargs):
-        name = self._if_manager.assign_name("int")
-
-        options = ""
-        for opt_name, opt_value in kwargs.items():
-            if opt_name == "name":
-                name = opt_value
-                continue
-
-            options += " %s=%s" % (opt_name, opt_value)
-
-        exec_cmd("ovs-vsctl add-port %s %s -- set Interface %s "\
-                 "type=internal %s" % (self.name, name,
-                                       name, options))
-
-        dev = self._if_manager.get_device_by_name(name)
-        dev._enable()
-        return dev
-
     def tunnel_add(self, tunnel_type, options):
-        name = self._if_manager.assign_name(tunnel_type)
-
-        opts = ""
-        for opt_name, opt_value in options.items():
-            if opt_name == "name":
-                name = opt_value
-                continue
-
-            opts += " %s=%s" % (opt_name, opt_value)
-
-        exec_cmd("ovs-vsctl add-port %s %s -- set Interface %s "\
-                 "type=%s %s" % (self.name, name, name,
-                                 tunnel_type, opts))
+        options_copy = options.copy()
+        options_copy['type'] = tunnel_type
+        self.port_add(device=None, interface_options=options_copy)
 
     def tunnel_del(self, name):
         self.port_del(name)
diff --git a/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
b/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
index 44cd4b6f..0c7b7520 100644
--- a/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
+++ b/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
@@ -31,8 +31,11 @@ class NoVirtOvsVxlanRecipe(CommonHWSubConfigMixin, BaseEnrtRecipe):
             host.eth0.down()
             host.eth0.ip_add(ipaddress(net_addr + "." + str(i+1) +
"/24"))
             host.br0 = OvsBridgeDevice()
-            host.int0 = host.br0.internal_port_add(ofport_request='5',
-                name="int0")
+            host.int0 = host.br0.port_add(
+                    interface_options={
+                        'type': 'internal',
+                        'ofport_request': 5,
+                        'name': 'int0'})
             host.int0.ip_add(ipaddress(vxlan_net_addr + "." + str(i+1) +
                 "/24"))
             host.int0.ip_add(ipaddress(vxlan_net_addr6 + "::" + str(i+1) +
diff --git a/lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
b/lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
index e7fcee1d..3d47e407 100644
--- a/lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
+++ b/lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
@@ -67,8 +67,9 @@ class VirtOvsVxlanRecipe(VlanPingEvaluatorMixin,
             host.eth0.ip_add(ipaddress(net_addr + "." + str(i+1) +
"/24"))
             host.br0 = OvsBridgeDevice()
             for dev, ofport_r in [(host.tap0, '5'), (host.tap1, '6')]:
-                host.br0.port_add(dev, set_iface=True,
-                    ofport_request=ofport_r)
+                host.br0.port_add(
+                        device=dev,
+                        interface_options={'ofport_request': ofport_r})
             tunnel_opts = {"option:remote_ip" : net_addr + "." +
str(2-i),
                 "option:key" : "flow", "ofport_request" :
'10'}
             host.br0.tunnel_add("vxlan", tunnel_opts)
diff --git a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInGuestMirroredRecipe.py
b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInGuestMirroredRecipe.py
index 2bdc0aaf..05245c03 100644
--- a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInGuestMirroredRecipe.py
+++ b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInGuestMirroredRecipe.py
@@ -43,7 +43,7 @@ class VirtualOvsBridgeVlanInGuestMirroredRecipe(CommonHWSubConfigMixin,
             host.tap0.down()
             host.br0 = OvsBridgeDevice()
             for dev in [host.eth0, host.tap0]:
-                host.br0.port_add(dev)
+                host.br0.port_add(device=dev)
 
         guest1.eth0.down()
         guest2.eth0.down()
diff --git a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostMirroredRecipe.py
b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostMirroredRecipe.py
index 00720968..b85350ab 100644
--- a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostMirroredRecipe.py
+++ b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostMirroredRecipe.py
@@ -42,7 +42,7 @@ class VirtualOvsBridgeVlanInHostMirroredRecipe(CommonHWSubConfigMixin,
             host.eth0.down()
             host.tap0.down()
             host.br0.port_add(host.eth0)
-            host.br0.port_add(host.tap0, tag="10")
+            host.br0.port_add(host.tap0, port_options={'tag': 10})
 
         guest1.eth0.down()
         guest2.eth0.down()
diff --git a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostRecipe.py
b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostRecipe.py
index ee97fa74..93f2c281 100644
--- a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostRecipe.py
+++ b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostRecipe.py
@@ -38,7 +38,7 @@ class VirtualOvsBridgeVlanInHostRecipe(CommonHWSubConfigMixin,
         host1.tap0.down()
         host1.br0 = OvsBridgeDevice()
         host1.br0.port_add(host1.eth0)
-        host1.br0.port_add(host1.tap0, tag="10")
+        host1.br0.port_add(host1.tap0, port_options={'tag': 10})
 
         host2.eth0.down()
         guest1.eth0.down()
diff --git a/lnst/Recipes/ENRT/VirtualOvsBridgeVlansOverBondRecipe.py
b/lnst/Recipes/ENRT/VirtualOvsBridgeVlansOverBondRecipe.py
index 45f59d07..9d75423d 100644
--- a/lnst/Recipes/ENRT/VirtualOvsBridgeVlansOverBondRecipe.py
+++ b/lnst/Recipes/ENRT/VirtualOvsBridgeVlansOverBondRecipe.py
@@ -58,7 +58,7 @@ class VirtualOvsBridgeVlansOverBondRecipe(VlanPingEvaluatorMixin,
                 dev.down()
             host.br0 = OvsBridgeDevice()
             for dev, tag in [(host.tap0, "10"), (host.tap1, "20")]:
-                host.br0.port_add(dev, tag=tag)
+                host.br0.port_add(device=dev, port_options={'tag': tag})
             #miimon cannot be set due to colon in argument name -->
             #other_config:bond-miimon-interval
             host.br0.bond_add(port_name, (host.eth0, host.eth1),
-- 
2.21.1

[PATCH 3/4] Devices.OvsBridgeDevice: refactor port_add method

[Jan Tluka]

Wed, Apr 01, 2020 at 12:07:31PM CEST, jtluka(a)redhat.com wrote:
> The internal_port_add was fundamentaly wrong as there's no such
> thing as an
> internal port in OpenVSwitch. Instead a user may add a port with internal
> device implementation (type internal).
>
> This patch removes the internal_port_add method and refactors port_add method
> to fit all of the following cases:
>  * ovs-vsctl port-add t_br0 eth0
>    that adds an existing device
>  * ovs-vsctl port-add t_br0 vnet0 -- set Interface vnet0 ofport_request=5
>    same with openflow rules
>  * ovs-vsctl port-add t_br0 int0 -- set Interface int0 type=internal
>    that adds a port with interface of type internal
>  * ovs-vsctl port-add t_br0 vxlan0 -- set Interface vxlan0 type=vxlan ...
>    that adds a port with interface of type vxlan
>
> The method now takes three parameters
>  * device which must be a Device instance, applicable only for internal and
>    tunnel type interfaces 
I will have to send v2 for this. It should say 'not applicable'.
I will wait for other comments.

>  * port_options, a dictionary of any Port column key-values, e.g.
> {'tag': 9}
>  * interface_options, a dictionary of any Interface column key-values, e.g.
>    {'type': 'internal'}, that would generate command like
>    'set Interface iface type=internal'
>
> The method tunnel_add was modified to use the refactored port_add method to
> remove duplicated code.
>
> All recipes that previously used any of the modified methods were updated.
>
> Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
> ---
>  lnst/Devices/OvsBridgeDevice.py               | 59 ++++++-------------
>  lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py     |  7 ++-
>  lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py       |  5 +-
>  ...rtualOvsBridgeVlanInGuestMirroredRecipe.py |  2 +-
>  ...irtualOvsBridgeVlanInHostMirroredRecipe.py |  2 +-
>  .../ENRT/VirtualOvsBridgeVlanInHostRecipe.py  |  2 +-
>  .../VirtualOvsBridgeVlansOverBondRecipe.py    |  2 +-
>  7 files changed, 31 insertions(+), 48 deletions(-)
>
> diff --git a/lnst/Devices/OvsBridgeDevice.py b/lnst/Devices/OvsBridgeDevice.py
> index 4205470e..28d786bb 100644
> --- a/lnst/Devices/OvsBridgeDevice.py
> +++ b/lnst/Devices/OvsBridgeDevice.py
> @@ -50,15 +50,23 @@ class OvsBridgeDevice(SoftDevice):
>  
>          return cmd
>  
> -    def port_add(self, dev, **kwargs):
> -        options = ""
> -        for opt_name, opt_value in kwargs.items():
> -            if opt_name == "set_iface" and opt_value:
> -                options = (" -- set Interface %s" + options) % dev.name
> -            else:
> -                options += " %s=%s" % (opt_name, opt_value)
> +    def port_add(self, device=None, port_options={}, interface_options={}):
> +        if device is None:
> +            dev_name = interface_options.get('name',
> +                self._if_manager.assign_name(interface_options['type']))
> +        else:
> +            dev_name = device.name
> +
> +        exec_cmd("ovs-vsctl add-port {} {}{}{}".format(self.name, dev_name,
> +            self._dict_to_keyvalues(port_options),
> +            self._interface_cmd(dev_name, interface_options)))
> +
> +        iface = None
> +        if 'type' in interface_options and interface_options['type'] ==
> 'internal':
> +            iface = self._if_manager.get_device_by_name(dev_name)
> +            iface._enable()
>  
> -        exec_cmd("ovs-vsctl add-port %s %s%s" % (self.name, dev.name,
> options))
> +        return iface
>  
>      def port_del(self, dev):
>          if isinstance(dev, Device):
> @@ -83,39 +91,10 @@ class OvsBridgeDevice(SoftDevice):
>      def bond_del(self, dev):
>          self.port_del(dev)
>  
> -    def internal_port_add(self, **kwargs):
> -        name = self._if_manager.assign_name("int")
> -
> -        options = ""
> -        for opt_name, opt_value in kwargs.items():
> -            if opt_name == "name":
> -                name = opt_value
> -                continue
> -
> -            options += " %s=%s" % (opt_name, opt_value)
> -
> -        exec_cmd("ovs-vsctl add-port %s %s -- set Interface %s "\
> -                 "type=internal %s" % (self.name, name,
> -                                       name, options))
> -
> -        dev = self._if_manager.get_device_by_name(name)
> -        dev._enable()
> -        return dev
> -
>      def tunnel_add(self, tunnel_type, options):
> -        name = self._if_manager.assign_name(tunnel_type)
> -
> -        opts = ""
> -        for opt_name, opt_value in options.items():
> -            if opt_name == "name":
> -                name = opt_value
> -                continue
> -
> -            opts += " %s=%s" % (opt_name, opt_value)
> -
> -        exec_cmd("ovs-vsctl add-port %s %s -- set Interface %s "\
> -                 "type=%s %s" % (self.name, name, name,
> -                                 tunnel_type, opts))
> +        options_copy = options.copy()
> +        options_copy['type'] = tunnel_type
> +        self.port_add(device=None, interface_options=options_copy)
>  
>      def tunnel_del(self, name):
>          self.port_del(name)
> diff --git a/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
> b/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
> index 44cd4b6f..0c7b7520 100644
> --- a/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
> +++ b/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
> @@ -31,8 +31,11 @@ class NoVirtOvsVxlanRecipe(CommonHWSubConfigMixin, BaseEnrtRecipe):
>              host.eth0.down()
>              host.eth0.ip_add(ipaddress(net_addr + "." + str(i+1) +
> "/24"))
>              host.br0 = OvsBridgeDevice()
> -            host.int0 = host.br0.internal_port_add(ofport_request='5',
> -                name="int0")
> +            host.int0 = host.br0.port_add(
> +                    interface_options={
> +                        'type': 'internal',
> +                        'ofport_request': 5,
> +                        'name': 'int0'})
>              host.int0.ip_add(ipaddress(vxlan_net_addr + "." + str(i+1) +
>                  "/24"))
>              host.int0.ip_add(ipaddress(vxlan_net_addr6 + "::" + str(i+1) +
> diff --git a/lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
> b/lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
> index e7fcee1d..3d47e407 100644
> --- a/lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
> +++ b/lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
> @@ -67,8 +67,9 @@ class VirtOvsVxlanRecipe(VlanPingEvaluatorMixin,
>              host.eth0.ip_add(ipaddress(net_addr + "." + str(i+1) +
> "/24"))
>              host.br0 = OvsBridgeDevice()
>              for dev, ofport_r in [(host.tap0, '5'), (host.tap1, '6')]:
> -                host.br0.port_add(dev, set_iface=True,
> -                    ofport_request=ofport_r)
> +                host.br0.port_add(
> +                        device=dev,
> +                        interface_options={'ofport_request': ofport_r})
>              tunnel_opts = {"option:remote_ip" : net_addr + "." +
> str(2-i),
>                  "option:key" : "flow", "ofport_request" :
> '10'}
>              host.br0.tunnel_add("vxlan", tunnel_opts)
> diff --git a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInGuestMirroredRecipe.py
> b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInGuestMirroredRecipe.py
> index 2bdc0aaf..05245c03 100644
> --- a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInGuestMirroredRecipe.py
> +++ b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInGuestMirroredRecipe.py
> @@ -43,7 +43,7 @@ class VirtualOvsBridgeVlanInGuestMirroredRecipe(CommonHWSubConfigMixin,
>              host.tap0.down()
>              host.br0 = OvsBridgeDevice()
>              for dev in [host.eth0, host.tap0]:
> -                host.br0.port_add(dev)
> +                host.br0.port_add(device=dev)
>  
>          guest1.eth0.down()
>          guest2.eth0.down()
> diff --git a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostMirroredRecipe.py
> b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostMirroredRecipe.py
> index 00720968..b85350ab 100644
> --- a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostMirroredRecipe.py
> +++ b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostMirroredRecipe.py
> @@ -42,7 +42,7 @@ class VirtualOvsBridgeVlanInHostMirroredRecipe(CommonHWSubConfigMixin,
>              host.eth0.down()
>              host.tap0.down()
>              host.br0.port_add(host.eth0)
> -            host.br0.port_add(host.tap0, tag="10")
> +            host.br0.port_add(host.tap0, port_options={'tag': 10})
>  
>          guest1.eth0.down()
>          guest2.eth0.down()
> diff --git a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostRecipe.py
> b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostRecipe.py
> index ee97fa74..93f2c281 100644
> --- a/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostRecipe.py
> +++ b/lnst/Recipes/ENRT/VirtualOvsBridgeVlanInHostRecipe.py
> @@ -38,7 +38,7 @@ class VirtualOvsBridgeVlanInHostRecipe(CommonHWSubConfigMixin,
>          host1.tap0.down()
>          host1.br0 = OvsBridgeDevice()
>          host1.br0.port_add(host1.eth0)
> -        host1.br0.port_add(host1.tap0, tag="10")
> +        host1.br0.port_add(host1.tap0, port_options={'tag': 10})
>  
>          host2.eth0.down()
>          guest1.eth0.down()
> diff --git a/lnst/Recipes/ENRT/VirtualOvsBridgeVlansOverBondRecipe.py
> b/lnst/Recipes/ENRT/VirtualOvsBridgeVlansOverBondRecipe.py
> index 45f59d07..9d75423d 100644
> --- a/lnst/Recipes/ENRT/VirtualOvsBridgeVlansOverBondRecipe.py
> +++ b/lnst/Recipes/ENRT/VirtualOvsBridgeVlansOverBondRecipe.py
> @@ -58,7 +58,7 @@ class VirtualOvsBridgeVlansOverBondRecipe(VlanPingEvaluatorMixin,
>                  dev.down()
>              host.br0 = OvsBridgeDevice()
>              for dev, tag in [(host.tap0, "10"), (host.tap1, "20")]:
> -                host.br0.port_add(dev, tag=tag)
> +                host.br0.port_add(device=dev, port_options={'tag': tag})
>              #miimon cannot be set due to colon in argument name -->
>              #other_config:bond-miimon-interval
>              host.br0.bond_add(port_name, (host.eth0, host.eth1),
> -- 
> 2.21.1
> _______________________________________________
> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
> https://lists.fedorahosted.org/archives/list/[e-mail]...

[PATCH 4/4] Devices.OvsBridge: use ovs-vsctl list to get ovs bridge ports

[Jan Tluka]

This is an additional patch that removes incorrect reference to
internal ports. The property internal_ports is removed and
ports with internal interfaces are now returned in property ports.

The patch also fixes an issue with OvS 2.13 that changed
the output format of 'ovs-vsctl show' that was used to get
information about the ovs bridge ports. To be more future proof
the json output of 'ovs-vsctl list ports' is used instead.

Since tunnels are also ports, the property tunnels reuses the ports
property and filters out the relevant ports.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/Devices/OvsBridgeDevice.py           | 125 +++++++++++-----------
 lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py |   4 +-
 2 files changed, 66 insertions(+), 63 deletions(-)

diff --git a/lnst/Devices/OvsBridgeDevice.py b/lnst/Devices/OvsBridgeDevice.py
index 28d786bb..fbe31b2c 100644
--- a/lnst/Devices/OvsBridgeDevice.py
+++ b/lnst/Devices/OvsBridgeDevice.py
@@ -12,6 +12,7 @@ olichtne(a)redhat.com (Ondrej Lichtner)
 
 import re
 import pprint
+import json
 from lnst.Common.Utils import check_process_running
 from lnst.Common.ExecCmd import exec_cmd
 from lnst.Common.DeviceError import DeviceError
@@ -50,6 +51,51 @@ class OvsBridgeDevice(SoftDevice):
 
         return cmd
 
+    def _format_ovs_json_value(self, value):
+        formatted_value = None
+        if type(value) == list:
+            value_type = value[0]
+
+            if value_type == 'map':
+                formatted_value = value[1]
+            elif value_type == 'set':
+                formatted_value = value[1]
+            elif value_type == 'uuid':
+                formatted_value = value[1]
+            else:
+                raise Exception("Unknown type in ovs json output: {}".format(
+                    value_type))
+        else:
+            formatted_value = value
+
+        return formatted_value
+
+    def _format_ovs_json(self, ovs_json):
+        headings = ovs_json['headings']
+        data = ovs_json['data']
+
+        formatted_data = []
+
+        for data_entry in data:
+            formatted_fields = {}
+            for i, entry_value in enumerate(data_entry):
+                formatted_fields[headings[i]] = self._format_ovs_json_value(entry_value)
+            formatted_data.append(formatted_fields)
+
+        return formatted_data
+
+    def _list_ports(self):
+        out_json = exec_cmd("ovs-vsctl --format json list port",
+                log_outputs=False, json=True)[0]
+
+        return self._format_ovs_json(out_json)
+
+    def _list_interfaces(self):
+        out_json = exec_cmd("ovs-vsctl --format json list interface",
+                log_outputs=False, json=True)[0]
+
+        return self._format_ovs_json(out_json)
+
     def port_add(self, device=None, port_options={}, interface_options={}):
         if device is None:
             dev_name = interface_options.get('name',
@@ -111,35 +157,31 @@ class OvsBridgeDevice(SoftDevice):
 
     @property
     def ports(self):
-        numbered_ports, port_lines = self._get_port_info()
-        ports = {}
-
-        for line in port_lines:
-            if not re.search('type=', line):
-                self._line_to_port_number(line, numbered_ports, ports)
+        ports = self._list_ports()
+        interfaces = self._list_interfaces()
 
-        return ports
+        filtered_ports = {}
 
-    @property
-    def internal_ports(self):
-        numbered_ports, port_lines = self._get_port_info()
-        int_ports = {}
+        for port in ports:
+            port_iface_uuid = port['interfaces']
+            port_ifaces = [ iface for iface in interfaces if iface['_uuid'] ==
port_iface_uuid ]
+            if len(port_ifaces):
+                port_iface = port_ifaces[0]
+                filtered_ports[port['name']] = {
+                        'interface': port_iface['name'],
+                        'type': port_iface['type'],
+                        'options': port_iface['options'],
+                        }
 
-        for line in port_lines:
-            if re.search('type=internal', line):
-                line = re.sub(r",?\stype=internal", "", line)
-                self._line_to_port_number(line, numbered_ports, int_ports)
-
-        return int_ports
+        return filtered_ports
 
     @property
     def tunnels(self):
-        numbered_ports, port_lines = self._get_port_info()
-        tunnels = {}
+        tunnels = self.ports.copy()
 
-        for line in port_lines:
-            if re.search('type=(?!internal)', line):
-                self._line_to_port_number(line, numbered_ports, tunnels)
+        for port in self.ports.keys():
+            if tunnels[port]['type'] in ['', 'internal']:
+                del tunnels[port]
 
         return tunnels
 
@@ -178,42 +220,3 @@ class OvsBridgeDevice(SoftDevice):
                         break
 
         return pprint.pformat(flows[1:])
-
-    def _get_port_info(self):
-        numbered_ports = {}
-        port_lines = []
-
-        dumped_ports = exec_cmd("ovs-ofctl dump-ports-desc %s" %
-            self.name, log_outputs=False)[0]
-
-        for match in re.finditer(r'(\w+)\((\w*)\)',
-            dumped_ports):
-            numbered_ports[match.groups()[1]] = match.groups()[0]
-
-        ovs_show = exec_cmd("ovs-vsctl show",
-            log_outputs=False)[0]
-        regex = r'(Port[\w\W]*?)(?=Port|ovs_version)'
-
-        for match in re.finditer(regex, ovs_show):
-            line = match.groups()[0].replace('\n', ' ')
-            line = self._port_format(line)
-            port_lines.append(line)
-
-        return numbered_ports, port_lines
-
-    def _port_format(self, line):
-        res = re.sub(r":", "", line)
-        res = re.sub(r"(\S[^,])\s(\S)", "\\1=\\2", res)
-        res = re.sub(r"\s{2,}(?=\S)", ", ", res)
-        res = re.sub(r"\s*$", "", res)
-
-        return res
-
-    def _line_to_port_number(self, line, ref, result):
-        name = re.match(r"Port=\"(\w+)\"", line).groups()[0]
-
-        try:
-            number = ref[name]
-            result[number] = line
-        except KeyError:
-            pass
diff --git a/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
b/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
index 0c7b7520..3fbb3845 100644
--- a/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
+++ b/lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
@@ -67,8 +67,8 @@ class NoVirtOvsVxlanRecipe(CommonHWSubConfigMixin, BaseEnrtRecipe):
                 for dev in config.test_wide_devices
             ]),
             "\n".join([
-                "Configured {}.{}.internal_ports = {}".format(
-                    dev.host.hostid, dev.name, dev.internal_ports
+                "Configured {}.{}.ports = {}".format(
+                    dev.host.hostid, dev.name, dev.ports
                 )
                 for dev in [host1.br0, host2.br0]
             ]),
-- 
2.21.1

[PATCH 4/4] Devices.OvsBridge: use ovs-vsctl list to get ovs bridge ports

[Ondrej Lichtner]

On Wed, Apr 01, 2020 at 12:07:32PM +0200, Jan Tluka wrote:
>  This is an additional patch that removes incorrect reference to
>  internal ports. The property internal_ports is removed and
>  ports with internal interfaces are now returned in property ports.
>  
>  The patch also fixes an issue with OvS 2.13 that changed
>  the output format of 'ovs-vsctl show' that was used to get
>  information about the ovs bridge ports. To be more future proof
>  the json output of 'ovs-vsctl list ports' is used instead.
>  
>  Since tunnels are also ports, the property tunnels reuses the ports
>  property and filters out the relevant ports.
>  
>  Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
>  ---
>   lnst/Devices/OvsBridgeDevice.py           | 125 +++++++++++-----------
>   lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py |   4 +-
>   2 files changed, 66 insertions(+), 63 deletions(-)
>  
>  diff --git a/lnst/Devices/OvsBridgeDevice.py b/lnst/Devices/OvsBridgeDevice.py
>  index 28d786bb..fbe31b2c 100644
>  --- a/lnst/Devices/OvsBridgeDevice.py
>  +++ b/lnst/Devices/OvsBridgeDevice.py
>  @@ -12,6 +12,7 @@ olichtne(a)redhat.com (Ondrej Lichtner)
>   
>   import re
>   import pprint
>  +import json 
I think this import is unused.

Other than that I have no other comments to the patchset.

-Ondrej

[PATCH 4/4] Devices.OvsBridge: use ovs-vsctl list to get ovs bridge ports

[Jan Tluka]

Thu, Apr 02, 2020 at 02:18:09PM CEST, olichtne(a)redhat.com wrote:
> On Wed, Apr 01, 2020 at 12:07:32PM +0200, Jan Tluka wrote:
> > This is an additional patch that removes incorrect reference to
> > internal ports. The property internal_ports is removed and
> > ports with internal interfaces are now returned in property ports.
> > 
> > The patch also fixes an issue with OvS 2.13 that changed
> > the output format of 'ovs-vsctl show' that was used to get
> > information about the ovs bridge ports. To be more future proof
> > the json output of 'ovs-vsctl list ports' is used instead.
> > 
> > Since tunnels are also ports, the property tunnels reuses the ports
> > property and filters out the relevant ports.
> > 
> > Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
> > ---
> >  lnst/Devices/OvsBridgeDevice.py           | 125 +++++++++++-----------
> >  lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py |   4 +-
> >  2 files changed, 66 insertions(+), 63 deletions(-)
> > 
> > diff --git a/lnst/Devices/OvsBridgeDevice.py b/lnst/Devices/OvsBridgeDevice.py
> > index 28d786bb..fbe31b2c 100644
> > --- a/lnst/Devices/OvsBridgeDevice.py
> > +++ b/lnst/Devices/OvsBridgeDevice.py
> > @@ -12,6 +12,7 @@ olichtne(a)redhat.com (Ondrej Lichtner)
> >  
> >  import re
> >  import pprint
> > +import json
>
> I think this import is unused.
>
> Other than that I have no other comments to the patchset.
>
> -Ondrej 
Ah, you're right. The json is used in exec_cmd, so this can be removed.
I'll send a v2 version with these minor fixes.

-Jan