addons: tunnel: fix tunnel creation (#80)- master branch refactoring

During the recent merge between master and master-next the changes introduced
by PR #80 were lost. This commit adds them back with some refactoring to use
the new netlink cache.

Co-authored-by: Maximilian Wilhelm <max@sdn.clinic>
Co-authored-by: Julien Fortin <julien@cumulusnetworks.com>
Signed-off-by: Maximilian Wilhelm <max@sdn.clinic>
Signed-off-by: Julien Fortin <julien@cumulusnetworks.com>

diff --git a/ifupdown2/addons/tunnel.py b/ifupdown2/addons/tunnel.py
index fd05929d2d3982ffffbf9a8104d032de5d3a3849..611a4715802d807d0283f35f621b11bbe7c07f65 100644 (file)
--- a/ifupdown2/addons/tunnel.py
+++ b/ifupdown2/addons/tunnel.py
@@ -72,16 +72,11 @@ class tunnel(moduleBase):
     def _is_my_interface(ifaceobj):
         return ifaceobj.addr_method == "tunnel" and ifaceobj.get_attr_value_first('mode')
 
-    def _check_settings(self, ifaceobj, attrs):
-        linkup = self.ipcmd.is_link_up(ifaceobj.name)
-        try:
-            if attrs:
-                self.ipcmd.tunnel_change(ifaceobj.name, attrs)
-        except:
-            raise
-        finally:
-            if attrs and linkup:
-                netlink.link_set_updown(ifaceobj.name, 'up')
+    def _has_config_changed(self, attrs_present, attrs_configured):
+        for key, value in attrs_configured.iteritems():
+            if attrs_present.get(key) != value:
+                return True
+        return False
 
     def _up(self, ifaceobj):
         attr_map = {
@@ -94,26 +89,31 @@ class tunnel(moduleBase):
 
         mode = ifaceobj.get_attr_value_first('mode')
         attrs = {}
+        attrs_mapped = {}
 
         # Only include attributes which have been set and map ifupdown2 names
         # to attribute names expected by iproute
         for attr, iproute_attr in attr_map.items():
             attr_val = ifaceobj.get_attr_value_first(attr)
             if attr_val != None:
-                attrs[iproute_attr] = attr_val
+                attrs_mapped[iproute_attr] = attr_val
+                attrs[attr] = attr_val
+
+        # Create the tunnel if it doesn't exist yet...
+        if not self.ipcmd.link_exists(ifaceobj.name):
+            self.ipcmd.tunnel_create(ifaceobj.name, mode, attrs_mapped)
+            return
 
+        # If it's present, check if there were changes
+        current_attrs = self.ipcmd.link_get_linkinfo_attrs(ifaceobj.name)
         current_mode = self.ipcmd.link_cache_get([ifaceobj.name, 'kind'])
 
         try:
-            if not self.ipcmd.link_exists(ifaceobj.name):
-                self.ipcmd.tunnel_create(ifaceobj.name, mode, attrs)
-            elif current_mode and current_mode != mode and (('6' in mode and '6' not in current_mode) or ('6' not in mode and '6' in current_mode)):
-                # Mode changes between ipv4 and ipv6 are not possible without recreating the interface
+            if current_attrs and current_mode != mode or self._has_config_changed(current_attrs, attrs):
+                # Mode and some other changes are not possible without recreating the interface,
+                # so just recreate it IFF there have been changes.
                 self.ipcmd.link_delete(ifaceobj.name)
-                self.ipcmd.tunnel_create(ifaceobj.name, mode, attrs)
-            else:
-                attrs['mode'] = mode
-                self._check_settings(ifaceobj, attrs)
+                self.ipcmd.tunnel_create(ifaceobj.name, mode, attrs_mapped)
         except Exception, e:
             self.log_warn(str(e))
 
@@ -170,7 +170,7 @@ class tunnel(moduleBase):
             # when accessing tunattrs parsed from 'ip -d link'.
             self._query_check_n_update(ifaceobj, ifaceobjcurr, attr,
                                        ifaceobj.get_attr_value_first(attr),
-                                       tunattrs.get(attr.replace("tunnel-", "")))
+                                       tunattrs.get(attr))
 
     # Operations supported by this addon (yet).
     _run_ops = {

diff --git a/ifupdown2/ifupdown/netlink.py b/ifupdown2/ifupdown/netlink.py
index 3bc9cc2346dd40a2930859021ae472163889e264..4152c0284ca58b227c961e1c4f9966a25a0141f8 100644 (file)
--- a/ifupdown2/ifupdown/netlink.py
+++ b/ifupdown2/ifupdown/netlink.py
@@ -517,7 +517,7 @@ class Netlink(utilsBase):
             "endpoint": str(info_slave_data.get(Link.IFLA_GRE_REMOTE)),
             "local": str(info_slave_data.get(Link.IFLA_GRE_LOCAL)),
             "ttl": str(info_slave_data.get(Link.IFLA_GRE_TTL)),
-            "physdev": self.get_iface_name(tunnel_link_ifindex) if tunnel_link_ifindex else ""
+            "tunnel-physdev": self.get_iface_name(tunnel_link_ifindex) if tunnel_link_ifindex else ""
         }
 
     def _link_dump_info_data_iptun_tunnel(self, ifname, info_slave_data):
@@ -527,7 +527,7 @@ class Netlink(utilsBase):
             "endpoint": str(info_slave_data.get(Link.IFLA_IPTUN_REMOTE)),
             "local": str(info_slave_data.get(Link.IFLA_IPTUN_LOCAL)),
             "ttl": str(info_slave_data.get(Link.IFLA_IPTUN_TTL)),
-            "physdev": self.get_iface_name(tunnel_link_ifindex) if tunnel_link_ifindex else ""
+            "tunnel-physdev": self.get_iface_name(tunnel_link_ifindex) if tunnel_link_ifindex else ""
         }
 
     def _link_dump_info_data_vti_tunnel(self, ifname, info_slave_data):
@@ -536,8 +536,7 @@ class Netlink(utilsBase):
         return {
             "endpoint": str(info_slave_data.get(Link.IFLA_VTI_REMOTE)),
             "local": str(info_slave_data.get(Link.IFLA_VTI_LOCAL)),
-            "ttl": "None",
-            "physdev": self.get_iface_name(tunnel_link_ifindex) if tunnel_link_ifindex else ""
+            "tunnel-physdev": self.get_iface_name(tunnel_link_ifindex) if tunnel_link_ifindex else ""
         }
 
     def _link_dump_linkinfo(self, link, dump):

diff --git a/ifupdown2/ifupdownaddons/LinkUtils.py b/ifupdown2/ifupdownaddons/LinkUtils.py
index 4b8bbc0e7d9740e7494067b9b03c0fd654e3ea8e..9f3bb294a86e9f012076eb73446b8753689ddaec 100644 (file)
--- a/ifupdown2/ifupdownaddons/LinkUtils.py
+++ b/ifupdown2/ifupdownaddons/LinkUtils.py
@@ -545,6 +545,8 @@ class LinkUtils(utilsBase):
                                 tunattrs['ttl'] = citems[j + 1]
                             elif citems[j] == 'dev':
                                 tunattrs['physdev'] = citems[j + 1]
+                            elif citems[j] in ['vti', 'vti6', 'ip6gre', 'ipip6', 'ip6ip6']:
+                                tunattrs['mode'] = citems[j]
                         linkattrs['linkinfo'] = tunattrs
                         break
                     elif citems[i] == 'link/ppp':
@@ -1270,18 +1272,16 @@ class LinkUtils(utilsBase):
         if '6' in mode:
             cmd = ' -6 '
 
-        if mode == 'gretap':
-            cmd += 'link add'
-            cmd += ' %s type %s' %(tunnelname, mode)
+        if mode in ['gretap']:
+            cmd += 'link add %s type %s' % (tunnelname, mode)
         else:
-            cmd += 'tunnel add'
-            cmd += ' %s mode %s' %(tunnelname, mode)
+            cmd += 'tunnel add %s mode %s' % (tunnelname, mode)
 
         if attrs:
             for k, v in attrs.iteritems():
-                cmd += ' %s' %k
+                cmd += ' %s' % k
                 if v:
-                    cmd += ' %s' %v
+                    cmd += ' %s' % v
         if self.ipbatch and not self.ipbatch_pause:
             self.add_to_batch(cmd)
         else: