dhcp: add support for inet + inet6 dhcp on same interface

Ticket: CM-12370
Reviewed By: Roopa, Kanna, Scott E
Testing Done:

This patch also fixes a problem where dhcp6 used to create lease file with
a trailing whitespace. dhcp6 operation were also sometimes using the wrong
pid file. I added some code in the debian.postinst script to correctly
rename these files if they exists when we install/update ifupdown2.

(cumulus-qa-infra/cl-tests/tests/smoke/testdhcp.py:Testdhcp_relay)
auto swp1
iface swp1 inet dhcp
      link-speed 10000
      link-duplex full
      link-autoneg off

auto swp1
iface swp1 inet6 dhcp

Signed-off-by: Julien Fortin <julien@cumulusnetworks.com>

addons/address.py

@@ -572,7 +572,7 @@ class address(moduleBase):
         isloopback = self.ipcmd.link_isloopback(ifaceobjrunning.name)
         if isloopback:
             default_addrs = ['127.0.0.1/8', '::1/128']
-            ifaceobjrunning.addr_family = 'inet'
+            ifaceobjrunning.addr_family.append('inet')
             ifaceobjrunning.addr_method = 'loopback'
         else:
             default_addrs = []

addons/dhcp.py

@@ -44,7 +44,7 @@ class dhcp(moduleBase):
                 self.ipcmd.link_exists(vrf)):
                 dhclient_cmd_prefix = '%s %s' %(self.vrf_exec_cmd_prefix, vrf)
 
-            if ifaceobj.addr_family == 'inet':
+            if 'inet' in ifaceobj.addr_family:
                 # First release any existing dhclient processes
                 try:
                     if not ifupdownflags.flags.PERFMODE:
@@ -53,7 +53,7 @@ class dhcp(moduleBase):
                     pass
                 self.dhclientcmd.start(ifaceobj.name, wait=wait,
                                        cmd_prefix=dhclient_cmd_prefix)
-            elif ifaceobj.addr_family == 'inet6':
+            if 'inet6' in ifaceobj.addr_family:
                 accept_ra = ifaceobj.get_attr_value_first('accept_ra')
                 if accept_ra:
                     # XXX: Validate value
@@ -82,37 +82,43 @@ class dhcp(moduleBase):
         if (vrf and self.vrf_exec_cmd_prefix and
             self.ipcmd.link_exists(vrf)):
             dhclient_cmd_prefix = '%s %s' %(self.vrf_exec_cmd_prefix, vrf)
-        if ifaceobj.addr_family == 'inet6':
+        if 'inet6' in ifaceobj.addr_family:
             self.dhclientcmd.release6(ifaceobj.name, dhclient_cmd_prefix)
-        else:
+        if 'inet' in ifaceobj.addr_family:
             self.dhclientcmd.release(ifaceobj.name, dhclient_cmd_prefix)
         self.ipcmd.link_down(ifaceobj.name)
 
     def _query_check(self, ifaceobj, ifaceobjcurr):
-        if self.dhclientcmd.is_running(ifaceobjcurr.name):
-            ifaceobjcurr.addr_family = 'inet'
-            if ifaceobj.addr_family != 'inet':
-                ifaceobjcurr.status = ifaceStatus.ERROR
+        status = ifaceStatus.SUCCESS
+        dhcp_running = False
+
+        dhcp_v4 = self.dhclientcmd.is_running(ifaceobjcurr.name)
+        dhcp_v6 = self.dhclientcmd.is_running6(ifaceobjcurr.name)
+
+        if dhcp_v4:
+            dhcp_running = True
+            if 'inet' not in ifaceobj.addr_family and not dhcp_v6:
+                status = ifaceStatus.ERROR
             ifaceobjcurr.addr_method = 'dhcp'
-            ifaceobjcurr.status = ifaceStatus.SUCCESS
-        elif self.dhclientcmd.is_running6(ifaceobjcurr.name):
-            ifaceobjcurr.addr_family = 'inet6'
-            if ifaceobj.addr_family != 'inet6':
-                ifaceobjcurr.status = ifaceStatus.ERROR
+        if dhcp_v6:
+            dhcp_running = True
+            if 'inet6' not in ifaceobj.addr_family and not dhcp_v4:
+                status = ifaceStatus.ERROR
             ifaceobjcurr.addr_method = 'dhcp'
-            ifaceobjcurr.status = ifaceStatus.SUCCESS
-        else:
-            ifaceobjcurr.addr_family = None
-            ifaceobjcurr.status = ifaceStatus.ERROR
+        ifaceobjcurr.addr_family = ifaceobj.addr_family
+        if not dhcp_running:
+            ifaceobjcurr.addr_family = []
+            status = ifaceStatus.ERROR
+        ifaceobjcurr.status = status
 
     def _query_running(self, ifaceobjrunning):
         if not self.ipcmd.link_exists(ifaceobjrunning.name):
             return
         if self.dhclientcmd.is_running(ifaceobjrunning.name):
-            ifaceobjrunning.addr_family = 'inet'
+            ifaceobjrunning.addr_family.append('inet')
             ifaceobjrunning.addr_method = 'dhcp'
-        elif self.dhclientcmd.is_running6(ifaceobjrunning.name):
-            ifaceobjrunning.addr_family = 'inet6'
+        if self.dhclientcmd.is_running6(ifaceobjrunning.name):
+            ifaceobjrunning.addr_family.append('inet6')
             ifaceobjrunning.addr_method = 'dhcp6'
 
     _run_ops = {'up' : _up,

addons/usercmds.py

@@ -40,7 +40,7 @@ class usercmds(ifupdownaddons.modulebase.moduleBase):
             os.environ['IFACE'] = ifaceobj.name if ifaceobj.name else ''
             os.environ['LOGICAL'] = ifaceobj.name if ifaceobj.name else ''
             os.environ['METHOD'] = ifaceobj.addr_method if ifaceobj.addr_method else ''
-            os.environ['ADDRFAM'] = ifaceobj.addr_family if ifaceobj.addr_family else ''
+            os.environ['ADDRFAM'] = ','.join(ifaceobj.addr_family) if ifaceobj.addr_family else ''
             for cmd in cmd_list:
                 try:
                     utils.exec_user_command(cmd)

debian/ifupdown2.postinst

@@ -44,6 +44,16 @@ report_err() { report "Error: $*" >&2 ; }
 case "$1" in
     configure)
 
+        # work around to rename the existing dhclient6 lease file containing a space
+        for filename in `find /var/lib/dhcp/ -name "dhclient.*.leases "`
+        do
+            if [ -f "$filename " ];
+            then
+                interface_name=`echo $filename | cut -d'.' -f2,3,4,5`
+                mv "$filename " /var/lib/dhcp/dhclient6.$interface_name
+            fi
+        done
+
         # Generic stuff done on all configurations
         if [ -f /etc/network/interfaces ] ; then
             if ! grep -q "^[[:space:]]*iface[[:space:]]\+lo0\?[[:space:]]\+inet[[:space:]]\+loopback\>" /etc/network/interfaces ; then

ifupdown/iface.py

@@ -289,7 +289,6 @@ class ifaceJsonDecoder():
 
 class iface():
     """ ifupdown2 iface object class
-    
     Attributes:
         **name**      Name of the interface 
 
@@ -344,6 +343,8 @@ class iface():
     version = '0.1'
 
     def __init__(self, attrsdict={}):
+        self.addr_family = []
+
         self._set_attrs_from_dict(attrsdict)
         self._config_status = {}
         """dict with config status of iface attributes"""
@@ -390,10 +391,13 @@ class iface():
     def _set_attrs_from_dict(self, attrdict):
         self.auto = attrdict.get('auto', False)
         self.name = attrdict.get('name')
-        self.addr_family = attrdict.get('addr_family')
         self.addr_method = attrdict.get('addr_method')
         self.config = attrdict.get('config', OrderedDict())
 
+        addr_family = attrdict.get('addr_family')
+        if addr_family:
+            self.addr_family.append(addr_family)
+
     def inc_refcnt(self):
         """ increment refcnt of the interface. Usually used to indicate that
         it has dependents """
@@ -579,6 +583,8 @@ class iface():
                 self.config[attrname].extend(attrlist)
             else:
                 self.config.update([(attrname, attrlist)])
+        # we now support inet and inet6 together
+        self.addr_family.extend(newifaceobj.addr_family)
 
     def __getstate__(self):
         odict = self.__dict__.copy()
@@ -635,7 +641,7 @@ class iface():
     def dump(self, logger):
         indent = '\t'
         logger.info(self.name + ' : {')
-        logger.info(indent + 'family: %s' %self.addr_family)
+        logger.info(indent + 'family: %s' % ' '.join(self.addr_family))
         logger.info(indent + 'method: %s' %self.addr_method)
         logger.info(indent + 'flags: %x' %self.flags)
         logger.info(indent + 'state: %s'
@@ -661,7 +667,7 @@ class iface():
             logger.info(indent + indent + str(config))
         logger.info('}')
 
-    def dump_pretty(self, with_status=False, use_realname=False):
+    def _dump_pretty(self, family, first, addr_method, with_status=False, use_realname=False):
         indent = '\t'
         outbuf = ''
         if use_realname and self.realname:
@@ -675,10 +681,10 @@ class iface():
             ifaceline += 'vlan %s' %name
         else:
             ifaceline += 'iface %s' %name
-        if self.addr_family:
-            ifaceline += ' %s' %self.addr_family
-        if self.addr_method:
-            ifaceline += ' %s' %self.addr_method
+        if family:
+            ifaceline += ' %s' % family
+        if addr_method:
+            ifaceline += ' %s' % addr_method
         if with_status:
             status_str = None
             if (self.status == ifaceStatus.ERROR or
@@ -700,7 +706,7 @@ class iface():
         else:
             outbuf += ifaceline + '\n'
         config = self.config
-        if config:
+        if config and first:
             for cname, cvaluelist in config.items():
                 idx = 0
                 for cv in cvaluelist:
@@ -723,3 +729,23 @@ class iface():
             outbuf = (outbuf.encode('utf8')
                         if isinstance(outbuf, unicode) else outbuf)
         print outbuf
+
+    def dump_pretty(self, with_status=False, use_realname=False):
+        if not self.addr_family:
+            self._dump_pretty(None, True,
+                              self.addr_method,
+                              with_status=with_status,
+                              use_realname=use_realname)
+        else:
+            first = True
+            for family in self.addr_family:
+                addr_method = None
+                if self.addr_method:
+                    if family == 'inet' and 'dhcp' in self.addr_method:
+                        addr_method = 'dhcp'
+                    elif family == 'inet6' and 'dhcp' in self.addr_method:
+                        addr_method = 'dhcp6'
+                self._dump_pretty(family, first, addr_method=addr_method,
+                                  with_status=with_status,
+                                  use_realname=use_realname)
+                first = False

ifupdown/networkinterfaces.py

@@ -94,19 +94,19 @@ class networkInterfaces():
         self.warns += 1
 
     def _validate_addr_family(self, ifaceobj, lineno=-1):
-        if ifaceobj.addr_family:
-            if not self._addrfams.get(ifaceobj.addr_family):
+        for family in ifaceobj.addr_family:
+            if not self._addrfams.get(family):
                 self._parse_error(self._currentfile, lineno,
                                   'iface %s: unsupported address family \'%s\''
-                    %(ifaceobj.name, ifaceobj.addr_family))
-                ifaceobj.addr_family = None
+                                  % (ifaceobj.name, family))
+                ifaceobj.addr_family = []
                 ifaceobj.addr_method = None
                 return
             if ifaceobj.addr_method:
-                if (ifaceobj.addr_method not in
-                        self._addrfams.get(ifaceobj.addr_family)):
+                if ifaceobj.addr_method not in self._addrfams.get(family):
                     self._parse_error(self._currentfile, lineno,
-                        'iface %s: unsupported address method \'%s\''
+                                      'iface %s: unsupported '
+                                      'address method \'%s\''
                                       % (ifaceobj.name, ifaceobj.addr_method))
             else:
                 ifaceobj.addr_method = 'static'
@@ -285,7 +285,8 @@ class networkInterfaces():
         ifaceobj.generate_env()
 
         try:
-            ifaceobj.addr_family = iface_attrs[2]
+            if iface_attrs[2]:
+                ifaceobj.addr_family.append(iface_attrs[2])
             ifaceobj.addr_method = iface_attrs[3]
         except IndexError:
             # ignore

ifupdownaddons/dhclient.py

@@ -83,7 +83,7 @@ class dhclient(utilsBase):
     def start6(self, ifacename, wait=True, cmd_prefix=None):
         cmd = ['/sbin/dhclient', '-6', '-pf',
                 '/run/dhclient6.%s.pid' %ifacename, '-lf',
-                '/var/lib/dhcp/dhclient.%s.leases ' %ifacename,
+                '/var/lib/dhcp/dhclient6.%s.leases' % ifacename,
                 '%s' %ifacename]
         if not wait:
             cmd.append('-nw')
@@ -91,8 +91,8 @@ class dhclient(utilsBase):
 
     def stop6(self, ifacename, cmd_prefix=None):
         cmd = ['/sbin/dhclient', '-6', '-x', '-pf',
-               '/run/dhclient.%s.pid' %ifacename, '-lf',
-               '/var/lib/dhcp/dhclient.%s.leases ' %ifacename,
+               '/run/dhclient6.%s.pid' % ifacename, '-lf',
+               '/var/lib/dhcp/dhclient6.%s.leases' % ifacename,
                '%s' %ifacename]
         self._run_dhclient_cmd(cmd, cmd_prefix)