Change in vdsm[ovirt-3.6]: network: bridge inherits DHCP unique identifier from its DHC... - vdsm-patches - Fedora mailing-lists

27 Aug 2015

Ondřej Svoboda has uploaded a new change for review.
Change subject: network: bridge inherits DHCP unique identifier from its DHCP-enabled port
......................................................................
network: bridge inherits DHCP unique identifier from its DHCP-enabled port
On Fedora 21 or newer, dhclient has started to use DHCP unique identifier
with a link-layer address and time (DUID-LLT) to identify a host.
When a host (e.g. oVirt node) is being added to engine's management network
while its NIC used DHCP, running dhclient on the bridge on top of the NIC
results in connectivity loss because the new dhclient is unaware of the DUID
used previously (dhclient should store it somewhere; bug to be reported soon)
and so the newly-generated DUID contains a different time value (albeit the
bridge's MAC address is the same as the NIC's), thus the host is given
a different IP address as well.
This patch assures that the original DUID is read from the NIC's dhclient
lease file by the dhclient running on the bridge for all network
configurators. A new functional test (a specialized copy of
testSetupNetworksAddDelDhcp) verifies the fix.
Change-Id: If91b46657be8a5a42a118b23155d059df379727e
Bug-Url: https://bugzilla.redhat.com/1219429
Signed-off-by: Ondřej Svoboda osvoboda@redhat.com
Reviewed-on: https://gerrit.ovirt.org/44539
Reviewed-by: Ido Barkan ibarkan@redhat.com
Reviewed-by: Dan Kenigsberg danken@redhat.com
Continuous-Integration: Jenkins CI
(cherry picked from commit 2ccbb949f55a812d97bbc076e7b33ebd11cb602d)
---
M tests/functional/networkTests.py
M vdsm/network/api.py
M vdsm/network/configurators/__init__.py
M vdsm/network/configurators/dhclient.py
M vdsm/network/configurators/ifcfg.py
M vdsm/network/models.py
6 files changed, 83 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/45402/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index e811955..39d8dab 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -2108,6 +2108,58 @@
@cleanupNet
     @RequireVethMod
+    def testDhcpReplaceNicWithBridge(self):
+        with veth.pair() as (left, right):
+            veth.setIP(left, IP_ADDRESS, IP_CIDR)
+            veth.setIP(left, IPv6_ADDRESS, IPv6_CIDR, 6)
+            veth.setLinkUp(left)
+            with dnsmasqDhcp(left):
+
+                # first, a network without a bridge should get a certain
+                # address
+
+                network = {NETWORK_NAME: {'nic': right, 'bridged': False,
+                                          'bootproto': 'dhcp',
+                                          'blockingdhcp': True}}
+                try:
+                    status, msg = self.setupNetworks(network, {}, NOCHK)
+                    self.assertEqual(status, SUCCESS, msg)
+                    self.assertNetworkExists(NETWORK_NAME)
+
+                    test_net = self.vdsm_net.netinfo.networks[NETWORK_NAME]
+                    self.assertEqual(test_net['dhcpv4'], True)
+                    devs = self.vdsm_net.netinfo.nics
+                    device_name = right
+
+                    self.assertIn(device_name, devs)
+                    net_attrs = devs[device_name]
+                    self.assertEqual(net_attrs['dhcpv4'], True)
+
+                    self.assertEqual(test_net['gateway'], IP_GATEWAY)
+                    ip_addr = test_net['addr']
+                    self.assertSourceRoutingConfiguration(device_name, ip_addr)
+
+                    # now, a bridged network should get the same address
+                    # (because dhclient should send the same dhcp-client-
+                    #  identifier)
+
+                    network[NETWORK_NAME]['bridged'] = True
+                    status, msg = self.setupNetworks(network, {}, NOCHK)
+                    self.assertEqual(status, SUCCESS, msg)
+                    test_net = self.vdsm_net.netinfo.networks[NETWORK_NAME]
+                    self.assertEqual(ip_addr, test_net['addr'])
+
+                    network = {NETWORK_NAME: {'remove': True}}
+                    status, msg = self.setupNetworks(network, {}, NOCHK)
+                    self.assertEqual(status, SUCCESS, msg)
+                    self.assertNetworkDoesntExist(NETWORK_NAME)
+
+                finally:
+                    dhcp.delete_dhclient_leases(right, True, False)
+                    dhcp.delete_dhclient_leases(NETWORK_NAME, True, False)
+
+    @cleanupNet
+    @RequireVethMod
     def testSetupNetworksReconfigureBridge(self):
         def setup_test_network(dhcp=True):
             network_params = {'nic': right, 'bridged': True}
diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index 5c8bed5..5b3a905 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -166,6 +166,9 @@
                                      'bridge STP value.' % stp)
         topNetDev = Bridge(bridge, configurator, port=topNetDev, mtu=mtu,
                            stp=stp)
+        # inherit DUID from the port's existing DHCP lease (BZ#1219429)
+        if topNetDev.port and bootproto == 'dhcp':
+            _inherit_dhcp_unique_identifier(topNetDev, _netinfo)
     if topNetDev is None:
         raise ConfigNetworkError(ne.ERR_BAD_PARAMS, 'Network defined without '
                                  'devices.')
@@ -600,6 +603,18 @@
                                      "Unknown nics in: %r" % list(nics))
+def _inherit_dhcp_unique_identifier(bridge, _netinfo):
+    """
+    If there is dhclient already running on a bridge's port we have to use the
+    same DHCP unique identifier (DUID) in order to get the same IP address.
+    """
+    for devices in (_netinfo.nics, _netinfo.bondings, _netinfo.vlans):
+        port = devices.get(bridge.port.name)
+        if port and port['dhcpv4']:
+            bridge.duid_source = bridge.port.name
+            break
+
+
 def _handleBondings(bondings, configurator, in_rollback):
     """ Add/Edit/Remove bond interface """
     logger = logging.getLogger("_handleBondings")
diff --git a/vdsm/network/configurators/__init__.py b/vdsm/network/configurators/__init__.py
index 8cadf1d..bca3eba 100644
--- a/vdsm/network/configurators/__init__.py
+++ b/vdsm/network/configurators/__init__.py
@@ -175,7 +175,7 @@
def runDhclient(iface, family=4, default_route=False):
-    dhclient = DhcpClient(iface.name, family, default_route)
+    dhclient = DhcpClient(iface.name, family, default_route, iface.duid_source)
     rc = dhclient.start(iface.blockingdhcp)
     if iface.blockingdhcp and rc:
         raise ConfigNetworkError(ERR_FAILED_IFUP, 'dhclient%s failed' % family)
diff --git a/vdsm/network/configurators/dhclient.py b/vdsm/network/configurators/dhclient.py
index 88edd1b..181c302 100644
--- a/vdsm/network/configurators/dhclient.py
+++ b/vdsm/network/configurators/dhclient.py
@@ -34,23 +34,25 @@
 from vdsm.utils import rmFile
DHCLIENT_CGROUP = 'vdsm-dhclient'
+LEASE_DIR = '/var/lib/dhclient'
+LEASE_FILE = os.path.join(LEASE_DIR, 'dhclient{0}--{1}.lease')
class DhcpClient(object):
     PID_FILE = '/var/run/dhclient%s-%s.pid'
-    LEASE_DIR = '/var/lib/dhclient'
-    LEASE_FILE = os.path.join(LEASE_DIR, 'dhclient{0}--{1}.lease')
     DHCLIENT = CommandPath('dhclient', '/sbin/dhclient')
-    def __init__(self, iface, family=4, default_route=False,
+    def __init__(self, iface, family=4, default_route=False, duid_source=None,
                  cgroup=DHCLIENT_CGROUP):
         self.iface = iface
         self.family = family
         self.default_route = default_route
+        self.duid_source_file = None if duid_source is None else (
+            LEASE_FILE.format('' if family == 4 else '6', duid_source))
         self.pidFile = self.PID_FILE % (family, self.iface)
-        if not os.path.exists(self.LEASE_DIR):
-            os.mkdir(self.LEASE_DIR)
-        self.leaseFile = self.LEASE_FILE.format(
+        if not os.path.exists(LEASE_DIR):
+            os.mkdir(LEASE_DIR)
+        self.leaseFile = LEASE_FILE.format(
             '' if family == 4 else '6', self.iface)
         self._cgroup = cgroup
@@ -63,6 +65,8 @@
         if not self.default_route:
             # Instruct Fedora/EL's dhclient-script not to set gateway on iface
             cmd += ['-e', 'DEFROUTE=no']
+        if self.duid_source_file:
+            cmd += ['-df', self.duid_source_file]
         cmd = cmdutils.systemd_run(cmd, scope=True, slice=self._cgroup)
         rc, out, err = execCmd(cmd)
         return rc, out, err
diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py
index 94878d0..e1d3e94 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -552,6 +552,10 @@
         if bridge.stp is not None:
             conf += 'STP=%s\n' % ('on' if bridge.stp else 'off')
         conf += 'ONBOOT=yes\n'
+        if bridge.duid_source:
+            duid_source_file = dhclient.LEASE_FILE.format(
+                '', bridge.duid_source)
+            conf += 'DHCLIENTARGS="-df %s"\n' % duid_source_file
if 'custom' in opts and 'bridge_opts' in opts['custom']:
             opts['bridging_opts'] = opts['custom']['bridge_opts']
diff --git a/vdsm/network/models.py b/vdsm/network/models.py
index 64cae6e..a0012d2 100644
--- a/vdsm/network/models.py
+++ b/vdsm/network/models.py
@@ -39,6 +39,7 @@
         self.mtu = mtu
         self.configurator = configurator
         self.master = None
+        self.duid_source = None
def __iter__(self):
         raise NotImplementedError
-- 
To view, visit https://gerrit.ovirt.org/45402
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If91b46657be8a5a42a118b23155d059df379727e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Ondřej Svoboda osvoboda@redhat.com

    