From ce59e63249dfcd86e782d056407e38e845cbe19c Mon Sep 17 00:00:00 2001 From: Christoph Arnold Date: Thu, 29 May 2014 16:17:30 +0200 Subject: [PATCH] Neutron does not follow the RFC 3442 spec for DHCP When setting a gateway and additional host routes in neutron subnet, the gateway is only sent to clients via the router dhcp option, dhcp clients conforming to rfc3442 will ignore router option if classless-static-routes are available. This patch ensures setting both the router option and the classless-static-routes including the gateway Change-Id: Ia00b9385025020f848872309ae42ddac08528f53 Closes-Bug: #1317935 --- neutron/agent/linux/dhcp.py | 5 +- neutron/tests/unit/test_linux_dhcp.py | 210 +++++++++++++++----------- 2 files changed, 129 insertions(+), 86 deletions(-) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 1dc8ad900..f83d5fff1 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -537,7 +537,8 @@ class Dnsmasq(DhcpLocalProcess): host_routes = [] for hr in subnet.host_routes: if hr.destination == "0.0.0.0/0": - gateway = hr.nexthop + if not gateway: + gateway = hr.nexthop else: host_routes.append("%s,%s" % (hr.destination, hr.nexthop)) @@ -550,6 +551,8 @@ class Dnsmasq(DhcpLocalProcess): ) if host_routes: + if gateway and subnet.ip_version == 4: + host_routes.append("%s,%s" % ("0.0.0.0/0", gateway)) options.append( self._format_option(i, 'classless-static-route', ','.join(host_routes))) diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py index 76bce4042..f11f3b68b 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -752,16 +752,17 @@ class TestDnsmasq(TestBase): def test_output_opts_file(self): fake_v6 = 'gdca:3ba5:a17a:4ba3::1' fake_v6_cidr = 'gdca:3ba5:a17a:4ba3::/64' - expected = """ -tag:tag0,option:dns-server,8.8.8.8 -tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1 -tag:tag0,249,20.0.0.1/24,20.0.0.1 -tag:tag0,option:router,192.168.0.1 -tag:tag1,option:dns-server,%s -tag:tag1,option:classless-static-route,%s,%s -tag:tag1,249,%s,%s""".lstrip() % (fake_v6, - fake_v6_cidr, fake_v6, - fake_v6_cidr, fake_v6) + expected = ( + 'tag:tag0,option:dns-server,8.8.8.8\n' + 'tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1,' + '0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,20.0.0.1/24,20.0.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,option:router,192.168.0.1\n' + 'tag:tag1,option:dns-server,%s\n' + 'tag:tag1,option:classless-static-route,%s,%s\n' + 'tag:tag1,249,%s,%s').lstrip() % (fake_v6, + fake_v6_cidr, fake_v6, + fake_v6_cidr, fake_v6) with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: conf_fn.return_value = '/foo/opts' @@ -776,7 +777,7 @@ tag:tag1,249,%s,%s""".lstrip() % (fake_v6, fake_v6_cidr = 'gdca:3ba5:a17a:4ba3::/64' expected = """ tag:tag0,option:dns-server,8.8.8.8 -tag:tag0,option:router,10.0.0.1 +tag:tag0,option:router,192.168.0.1 tag:tag1,option:dns-server,%s tag:tag1,option:classless-static-route,%s,%s tag:tag1,249,%s,%s""".lstrip() % (fake_v6, @@ -816,11 +817,12 @@ tag:tag0,option:router,192.168.0.1""".lstrip() self.safe.assert_called_once_with('/foo/opts', expected) def test_output_opts_file_single_dhcp(self): - expected = """ -tag:tag0,option:dns-server,8.8.8.8 -tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1 -tag:tag0,249,20.0.0.1/24,20.0.0.1 -tag:tag0,option:router,192.168.0.1""".lstrip() + expected = ( + 'tag:tag0,option:dns-server,8.8.8.8\n' + 'tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1,' + '0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,20.0.0.1/24,20.0.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,option:router,192.168.0.1').lstrip() with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: conf_fn.return_value = '/foo/opts' dm = dhcp.Dnsmasq(self.conf, FakeDualNetworkSingleDHCP(), @@ -830,11 +832,12 @@ tag:tag0,option:router,192.168.0.1""".lstrip() self.safe.assert_called_once_with('/foo/opts', expected) def test_output_opts_file_single_dhcp_ver2_48(self): - expected = """ -tag0,option:dns-server,8.8.8.8 -tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1 -tag0,249,20.0.0.1/24,20.0.0.1 -tag0,option:router,192.168.0.1""".lstrip() + expected = ( + 'tag0,option:dns-server,8.8.8.8\n' + 'tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1,' + '0.0.0.0/0,192.168.0.1\n' + 'tag0,249,20.0.0.1/24,20.0.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag0,option:router,192.168.0.1').lstrip() with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: conf_fn.return_value = '/foo/opts' dm = dhcp.Dnsmasq(self.conf, FakeDualNetworkSingleDHCP(), @@ -862,10 +865,12 @@ tag:tag0,option:router""".lstrip() self.safe.assert_called_once_with('/foo/opts', expected) def test_output_opts_file_no_neutron_router_on_subnet(self): - expected = """ -tag:tag0,option:classless-static-route,169.254.169.254/32,192.168.1.2 -tag:tag0,249,169.254.169.254/32,192.168.1.2 -tag:tag0,option:router,192.168.1.1""".lstrip() + expected = ( + 'tag:tag0,option:classless-static-route,' + '169.254.169.254/32,192.168.1.2,0.0.0.0/0,192.168.1.1\n' + 'tag:tag0,249,169.254.169.254/32,192.168.1.2,' + '0.0.0.0/0,192.168.1.1\n' + 'tag:tag0,option:router,192.168.1.1').lstrip() with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: conf_fn.return_value = '/foo/opts' @@ -880,17 +885,24 @@ tag:tag0,option:router,192.168.1.1""".lstrip() self.safe.assert_called_once_with('/foo/opts', expected) def test_output_opts_file_pxe_2port_1net(self): - expected = """ -tag:tag0,option:dns-server,8.8.8.8 -tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1 -tag:tag0,249,20.0.0.1/24,20.0.0.1 -tag:tag0,option:router,192.168.0.1 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:tftp-server,192.168.0.3 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:server-ip-address,192.168.0.2 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:bootfile-name,pxelinux.0 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:tftp-server,192.168.0.3 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:server-ip-address,192.168.0.2 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:bootfile-name,pxelinux.0""" + expected = ( + 'tag:tag0,option:dns-server,8.8.8.8\n' + 'tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1,' + '0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,20.0.0.1/24,20.0.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,option:router,192.168.0.1\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:tftp-server,192.168.0.3\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:server-ip-address,192.168.0.2\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:bootfile-name,pxelinux.0\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:tftp-server,192.168.0.3\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:server-ip-address,192.168.0.2\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:bootfile-name,pxelinux.0') expected = expected.lstrip() with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: @@ -902,17 +914,24 @@ tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:bootfile-name,pxelinux.0""" self.safe.assert_called_once_with('/foo/opts', expected) def test_output_opts_file_pxe_2port_1net_diff_details(self): - expected = """ -tag:tag0,option:dns-server,8.8.8.8 -tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1 -tag:tag0,249,20.0.0.1/24,20.0.0.1 -tag:tag0,option:router,192.168.0.1 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:tftp-server,192.168.0.3 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:server-ip-address,192.168.0.2 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:bootfile-name,pxelinux.0 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:tftp-server,192.168.0.5 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:server-ip-address,192.168.0.5 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:bootfile-name,pxelinux.0""" + expected = ( + 'tag:tag0,option:dns-server,8.8.8.8\n' + 'tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1,' + '0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,20.0.0.1/24,20.0.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,option:router,192.168.0.1\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:tftp-server,192.168.0.3\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:server-ip-address,192.168.0.2\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:bootfile-name,pxelinux.0\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:tftp-server,192.168.0.5\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:server-ip-address,192.168.0.5\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:bootfile-name,pxelinux.0') expected = expected.lstrip() with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: @@ -924,20 +943,30 @@ tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:bootfile-name,pxelinux.0""" self.safe.assert_called_once_with('/foo/opts', expected) def test_output_opts_file_pxe_3port_1net_diff_details(self): - expected = """ -tag:tag0,option:dns-server,8.8.8.8 -tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1 -tag:tag0,249,20.0.0.1/24,20.0.0.1 -tag:tag0,option:router,192.168.0.1 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:tftp-server,192.168.0.3 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:server-ip-address,192.168.0.2 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:bootfile-name,pxelinux.0 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:tftp-server,192.168.0.5 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:server-ip-address,192.168.0.5 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:bootfile-name,pxelinux2.0 -tag:44444444-4444-4444-4444-444444444444,option:tftp-server,192.168.0.7 -tag:44444444-4444-4444-4444-444444444444,option:server-ip-address,192.168.0.7 -tag:44444444-4444-4444-4444-444444444444,option:bootfile-name,pxelinux3.0""" + expected = ( + 'tag:tag0,option:dns-server,8.8.8.8\n' + 'tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1,' + '0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,20.0.0.1/24,20.0.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,option:router,192.168.0.1\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:tftp-server,192.168.0.3\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:server-ip-address,192.168.0.2\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:bootfile-name,pxelinux.0\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:tftp-server,192.168.0.5\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:server-ip-address,192.168.0.5\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:bootfile-name,pxelinux2.0\n' + 'tag:44444444-4444-4444-4444-444444444444,' + 'option:tftp-server,192.168.0.7\n' + 'tag:44444444-4444-4444-4444-444444444444,' + 'option:server-ip-address,192.168.0.7\n' + 'tag:44444444-4444-4444-4444-444444444444,' + 'option:bootfile-name,pxelinux3.0') expected = expected.lstrip() with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: @@ -950,20 +979,30 @@ tag:44444444-4444-4444-4444-444444444444,option:bootfile-name,pxelinux3.0""" self.safe.assert_called_once_with('/foo/opts', expected) def test_output_opts_file_pxe_3port_2net(self): - expected = """ -tag:tag0,option:dns-server,8.8.8.8 -tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1 -tag:tag0,249,20.0.0.1/24,20.0.0.1 -tag:tag0,option:router,192.168.0.1 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:tftp-server,192.168.0.3 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:server-ip-address,192.168.0.2 -tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,option:bootfile-name,pxelinux.0 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:tftp-server,192.168.1.3 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:server-ip-address,192.168.1.2 -tag:ffffffff-ffff-ffff-ffff-ffffffffffff,option:bootfile-name,pxelinux2.0 -tag:44444444-4444-4444-4444-444444444444,option:tftp-server,192.168.1.3 -tag:44444444-4444-4444-4444-444444444444,option:server-ip-address,192.168.1.2 -tag:44444444-4444-4444-4444-444444444444,option:bootfile-name,pxelinux3.0""" + expected = ( + 'tag:tag0,option:dns-server,8.8.8.8\n' + 'tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1,' + '0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,20.0.0.1/24,20.0.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,option:router,192.168.0.1\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:tftp-server,192.168.0.3\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:server-ip-address,192.168.0.2\n' + 'tag:eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee,' + 'option:bootfile-name,pxelinux.0\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:tftp-server,192.168.1.3\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:server-ip-address,192.168.1.2\n' + 'tag:ffffffff-ffff-ffff-ffff-ffffffffffff,' + 'option:bootfile-name,pxelinux2.0\n' + 'tag:44444444-4444-4444-4444-444444444444,' + 'option:tftp-server,192.168.1.3\n' + 'tag:44444444-4444-4444-4444-444444444444,' + 'option:server-ip-address,192.168.1.2\n' + 'tag:44444444-4444-4444-4444-444444444444,' + 'option:bootfile-name,pxelinux3.0') expected = expected.lstrip() with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: @@ -1006,16 +1045,17 @@ tag:44444444-4444-4444-4444-444444444444,option:bootfile-name,pxelinux3.0""" exp_opt_name = '/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/opts' fake_v6 = 'gdca:3ba5:a17a:4ba3::1' fake_v6_cidr = 'gdca:3ba5:a17a:4ba3::/64' - exp_opt_data = """ -tag:tag0,option:dns-server,8.8.8.8 -tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1 -tag:tag0,249,20.0.0.1/24,20.0.0.1 -tag:tag0,option:router,192.168.0.1 -tag:tag1,option:dns-server,%s -tag:tag1,option:classless-static-route,%s,%s -tag:tag1,249,%s,%s""".lstrip() % (fake_v6, - fake_v6_cidr, fake_v6, - fake_v6_cidr, fake_v6) + exp_opt_data = ( + 'tag:tag0,option:dns-server,8.8.8.8\n' + 'tag:tag0,option:classless-static-route,20.0.0.1/24,20.0.0.1,' + '0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,20.0.0.1/24,20.0.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,option:router,192.168.0.1\n' + 'tag:tag1,option:dns-server,%s\n' + 'tag:tag1,option:classless-static-route,%s,%s\n' + 'tag:tag1,249,%s,%s').lstrip() % (fake_v6, + fake_v6_cidr, fake_v6, + fake_v6_cidr, fake_v6) return (exp_host_name, exp_host_data, exp_addn_name, exp_addn_data, exp_opt_name, exp_opt_data,) -- 2.45.2