Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check non-inclusive DHCP overrides #305

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Check non-inclusive DHCP overrides #305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dadbcf5
fail test on silently disregarded dhcp6-overrides
pajod Dec 27, 2022
445ef16
networkd: debug log DHCP options when DHCPv6 on
pajod Dec 27, 2022
3c423c1
-Werror=comment
pajod Dec 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 44 additions & 11 deletions src/networkd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,12 @@ write_addr_option(NetplanAddressOptions* o, GString* s)
"ERROR: %s: networkd requires that %s has the same value in both " \
"dhcp4_overrides and dhcp6_overrides\n"

static void handle_ignored_dhcp6_override(gboolean dhcp6_overrides_in_combined_dhcp_section, const char* opt_name)
{
if (dhcp6_overrides_in_combined_dhcp_section)
g_debug("networkd: writing option DHCP.%s without effect on DHCPv6.%s", opt_name, opt_name);
}

static gboolean
combine_dhcp_overrides(const NetplanNetDefinition* def, NetplanDHCPOverrides* combined_dhcp_overrides, GError** error)
{
Expand All @@ -658,7 +664,7 @@ combine_dhcp_overrides(const NetplanNetDefinition* def, NetplanDHCPOverrides* co
} else if (!def->dhcp4 && def->dhcp6) {
*combined_dhcp_overrides = def->dhcp6_overrides;
} else {
/* networkd doesn't support separately configuring dhcp4 and dhcp6, so
/* we don't support separately configuring dhcp4 and dhcp6, so
* we enforce that they are the same.
*/
if (def->dhcp4_overrides.use_dns != def->dhcp6_overrides.use_dns) {
Expand Down Expand Up @@ -720,6 +726,7 @@ netplan_netdef_write_network_file(
GString* s = NULL;
mode_t orig_umask;
gboolean is_optional = def->optional;
static gboolean combined_dhcp_section = TRUE;

SET_OPT_OUT_PTR(has_been_written, FALSE);

Expand Down Expand Up @@ -892,29 +899,39 @@ netplan_netdef_write_network_file(
}
}

if (def->dhcp4 || def->dhcp6 || def->critical) {
/*
for (combined_dhcp_section) {

if (!combined_dhcp_section && def->dhcp6) {
g_string_append(network, "\n[DHCPv6]\n");
}
*/
if (combined_dhcp_section && (def->dhcp4 || def->dhcp6 || def->critical)) {
/* NetworkManager compatible route metrics */
g_string_append(network, "\n[DHCP]\n");
}

if (def->critical)
/* see DHCP.KeepConfiguration */
if (combined_dhcp_section && def->critical)
g_string_append_printf(network, "CriticalConnection=true\n");

if (def->dhcp4 || def->dhcp6) {
if ((combined_dhcp_section && def->dhcp4) || def->dhcp6) {
if (def->dhcp_identifier)
g_string_append_printf(network, "ClientIdentifier=%s\n", def->dhcp_identifier);

NetplanDHCPOverrides combined_dhcp_overrides;
if (!combine_dhcp_overrides(def, &combined_dhcp_overrides, error))
return FALSE;

/* backward compatibility via config_parse_dhcp_route_metric - now at DHCPv4.RouteMetric + IPv6AcceptRA.RouteMetric */
if (combined_dhcp_overrides.metric == NETPLAN_METRIC_UNSPEC) {
g_string_append_printf(network, "RouteMetric=%i\n", (def->type == NETPLAN_DEF_TYPE_WIFI ? 600 : 100));
} else {
g_string_append_printf(network, "RouteMetric=%u\n",
combined_dhcp_overrides.metric);
}

/* FIXME: add handle_ignored_dhcp6_override() see DHCPv4.UseMTU + IPv6AcceptRA.UseMTU */
/* Only set MTU from DHCP if use-mtu dhcp-override is not false. */
if (!combined_dhcp_overrides.use_mtu) {
/* isc-dhcp dhclient compatible UseMTU, networkd default is to
Expand All @@ -925,21 +942,37 @@ netplan_netdef_write_network_file(
}

/* Only write DHCP options that differ from the networkd default. */
if (!combined_dhcp_overrides.use_routes)
if (!combined_dhcp_overrides.use_routes) {
handle_ignored_dhcp6_override(def->dhcp6 && combined_dhcp_section, "UseRoutes");
g_string_append_printf(network, "UseRoutes=false\n");
if (!combined_dhcp_overrides.use_dns)
}
if (!combined_dhcp_overrides.use_dns) {
/* backward compatibility via config_parse_dhcp_use_dns see DHCPv4.UseDNS + DHCPv6.UseDNS */
g_string_append_printf(network, "UseDNS=false\n");
if (combined_dhcp_overrides.use_domains)
}
if (combined_dhcp_overrides.use_domains) {
/* backward compatibility via config_parse_dhcp_use_domains see DHCPv4.UseDomains + DHCPv6.UseDomains */
g_string_append_printf(network, "UseDomains=%s\n", combined_dhcp_overrides.use_domains);
if (!combined_dhcp_overrides.use_ntp)
}
if (!combined_dhcp_overrides.use_ntp) {
/* backward compatibility via config_parse_dhcp_use_ntp see DHCPv4.UseNTP + DHCPv6.UseNTP */
g_string_append_printf(network, "UseNTP=false\n");
if (!combined_dhcp_overrides.send_hostname)
}
if (!combined_dhcp_overrides.send_hostname) {
handle_ignored_dhcp6_override(def->dhcp6 && combined_dhcp_section, "SendHostname");
g_string_append_printf(network, "SendHostname=false\n");
if (!combined_dhcp_overrides.use_hostname)
}
if (!combined_dhcp_overrides.use_hostname) {
handle_ignored_dhcp6_override(def->dhcp6 && combined_dhcp_section, "UseHostname");
g_string_append_printf(network, "UseHostname=false\n");
if (combined_dhcp_overrides.hostname)
}
if (combined_dhcp_overrides.hostname) {
handle_ignored_dhcp6_override(def->dhcp6 && combined_dhcp_section, "Hostname");
g_string_append_printf(network, "Hostname=%s\n", combined_dhcp_overrides.hostname);
}
}

/* } * for (combined_dhcp_section) */

/* IP-over-InfiniBand, IPoIB */
if (def->ib_mode != NETPLAN_IB_MODE_KERNEL) {
Expand Down
19 changes: 19 additions & 0 deletions tests/generator/test_dhcp_overrides.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@


class TestNetworkd(TestBase):
# simple test for a specific class of bugs
# FIXME: adapt all tests in assert_dhcp_overrides_bool() for the override=false case after updating dhcp6-override generation
def assert_dhcp6_overrides_bool_false(self, override_name, networkd_name):
# dhcp6 no
self.generate('''network:
version: 2
ethernets:
engreen:
dhcp6: yes
dhcp6-overrides:
%s: no
''' % override_name)
# must not get silently ignored since true is the default
kiss = self.get_network_config_for_link('engreen')
# fuzzy
self.assertIn('\n%s=false\n' % networkd_name, kiss)
self.assertIn('\n[DHCPv6]\n', kiss)

# Common tests for dhcp override booleans
def assert_dhcp_overrides_bool(self, override_name, networkd_name):
Expand Down Expand Up @@ -354,6 +371,8 @@ def test_dhcp_overrides_send_hostname(self):

def test_dhcp_overrides_use_hostname(self):
self.assert_dhcp_overrides_bool('use-hostname', 'UseHostname')
# example, other overrides are affected as well
self.assert_dhcp6_overrides_bool_false('use-hostname', 'UseHostname')

def test_dhcp_overrides_hostname(self):
self.assert_dhcp_overrides_string('hostname', 'Hostname')
Expand Down