Default route from DHCP no longer set after upgrade 235.38 -> 236.0

[thierer]

Submission type

• Bug report

systemd version the issue has been seen with

236.0

Used distribution

Arch Linux

In case of bug report: Expected behaviour you didn't see

235.38

> ip route
default via 192.168.165.1 dev enp3s0 proto dhcp src 192.168.165.5 metric 1024
192.168.165.0/24 dev enp3s0 proto kernel scope link src 192.168.165.5
192.168.165.1 dev enp3s0 proto dhcp scope link src 192.168.165.5 metric 1024

In case of bug report: Unexpected behaviour you saw

236.0

> ip route
192.168.165.0/24 dev enp3s0 proto kernel scope link src 192.168.165.5

even though the route is apparently correctly received:

> journalctl -b -o cat -u systemd-networkd
Starting Network Service...
Enumeration completed
Started Network Service.
eth0: Interface name change detected, eth0 has been renamed to enp3s0.
enp3s0: IPv6 successfully enabled
enp3s0: Gained carrier
enp3s0: Gained IPv6LL
enp3s0: DHCPv4 address 192.168.165.5/24 via 192.168.165.1
enp3s0: Configured

In case of bug report: Steps to reproduce the problem

> cat /etc/systemd/network/eth0.network
[Match]
Name=enp3s0

[Network]
DHCP=yes

[DHCP]
UseDomains=true

[ssahani]

I think you are missing #7432

[thierer]

Do you mean it should be fixed by 8dc787d? But shouldn't that be in 236.0?

[ssahani]

yes . But I can't reproduce with current git.

[thierer]

Turns out I left out one important piece of information in my bug report, apologies for that:

In my case the DHCP-Server sets an additional route via DHCP-Option 33 ("Static Route"). When I remove that option, the default route is set.

So 5f04a20 does some collateral damage: #5695 is about DHCP-Option 121 ("Classless Static Route"; RFC 3442) but 5f04a20 breaks DHCP-Option 33 ("Static Route" from RFC 2132), as routes from both options end up in the same array (lease->static_route) and the default route is discarded if any of them is set.

[ssahani]

isn't option 33 is deprecated ?

[thierer]

Well, RFC 3442 says so, but then again its point is making a case for option 121... I can't see a reason for not using it and it was working just fine for all my clients until systemd 236.0.

[poettering]

@ssahani if option 33 is used in the world we need to support it, it's that easy, regardless if deprecated or not.

[ssahani]

@poettering yes right but we first loop around and set all the routes .

@thierer Could you give me a dnsmasq conf . From the code we are extracting n routes that is all the routes. I cant see how that is happening really ?

[thierer]

@ssahani, sure. These entries to dnsmasq.conf

dhcp-option=option:router,192.168.0.1
dhcp-option=option:static-route,10.2.0.0,192.168.0.10

should result in these routes being set

default via 192.168.0.1
10.0.0.0/8 via 192.168.0.10

Which stopped working in 236, the default route is no longer set.

Sorry if I'm wrong, but your remarks about "looping around" and "extracting all routes" make me suspect you didn't understand the problem:

The problem is not, that any of the routes from options "static-route" (33) or "classless-static-route" (121) are not set - they are. (Which is a separate problem, I'll get to that).

The problem is that if a route is set by option "static-route" (33) the default route set by option "router" (3) is discarded.

IMHO that's wrong, because RFC 3442 only says this should happen when "Classless Static Route" (121) is used:

If the DHCP server returns both a Classless Static Routes option and
a Router option, the DHCP client MUST ignore the Router option.

Which brings me to my second point, because the RFC also states

Similarly, if the DHCP server returns both a Classless Static Routes
option and a Static Routes option, the DHCP client MUST ignore the
Static Routes option.

That's not happening, which you can test by using

dhcp-option=option:router,192.168.0.1
dhcp-option=option:static-route,10.2.0.0,192.168.0.10
dhcp-option=option:classless-static-route,0.0.0.0/0,192.168.0.1,192.168.1.0/24,192.168.1.1

Following the rules of the RFC, this should only set the routes from the "classless-static-route" entry, but in fact the route from "static-route" (to 10.0.0.0/8) is also set.

To summarize, I see 3 problems with the current code:

1. When only the DHCP-option "static-route" (3) is provided (but no "classless-static-route" option is present), the route given by DHCP-option "router" (3) should still be used. Current behavior: The route from option 3 is discarded.
2. When the DHCP-option "classless-static-route" (121) is provided, both routes given by options 3 and 33 should be discarded. Current behavior: Only the route from option 3 is discarded, the route from option 33 is still used.
3. When any of the routes are discarded, there should be a warning (something like "Classless static route(s) set - ignoring router option as per RFC 3442") to make it easier to understand what's going on. Current behavior: The default route from option 3 is silently ignored.

[ssahani]

Sorry if I'm wrong, but your remarks about "looping around" and "extracting all routes" make me suspect you didn't understand the problem:

The option code 121 and 33 both kept in a array . So we apply them while looping around it .

When only the DHCP-option "static-route" (3) is provided (but no "classless-static-route" option is present), the route given by DHCP-option "router" (3) should still be used. Current behavior: The route from option 3 is discarded.

No we apply both of them I just added debug logs to test them .

When the DHCP-option "classless-static-route" (121) is provided, both routes given by options 3 and 33 should be discarded. Current behavior: Only the route from option 3 is discarded, the route from option 33 is still used.

Yes we need to fix it

When any of the routes are discarded, there should be a warning (something like "Classless static route(s) set - ignoring router option as per RFC 3442") to make it easier to understand what's going on. Current behavior: The default route from option 3 is silently ignored.

Yes we will add.

[thierer]

The option code 121 and 33 both kept in a array . So we apply them while looping around it .

Yes, but as I wrote, that's not the point...

When only the DHCP-option "static-route" (3) is provided (but no "classless-static-route" option is present), the route given by DHCP-option "router" (3) should still be used. Current behavior: The route from option 3 is discarded.

No we apply both of them I just added debug logs to test them.

Really? Did you also check that the default route is actually set?

Because that's not what I'm experiencing, and looking at the code, I can't see how it would work:

This loop iterates through the routes picked up from both "static-route" (option 33) and "classless-static-route" (option 121):

systemd/src/network/networkd-dhcp4.c

Lines 103 to 124 in 5f04a20

	for (i = 0; i < n; i++) {
	_cleanup_route_free_ Route *route = NULL;
	r = route_new(&route);
	if (r < 0)
	return log_link_error_errno(link, r, "Could not allocate route: %m");
	route->family = AF_INET;
	route->protocol = RTPROT_DHCP;
	assert_se(sd_dhcp_route_get_gateway(static_routes[i], &route->gw.in) >= 0);
	assert_se(sd_dhcp_route_get_destination(static_routes[i], &route->dst.in) >= 0);
	assert_se(sd_dhcp_route_get_destination_prefix_length(static_routes[i], &route->dst_prefixlen) >= 0);
	route->priority = link->network->dhcp_route_metric;
	route->table = table;
	route->scope = route_scope_from_address(route, &address);
	r = route_configure(route, link, dhcp4_route_handler);
	if (r < 0)
	return log_link_warning_errno(link, r, "Could not set host route: %m");
	link->dhcp4_messages++;
	}

If at least one route got set by any of these two options, link->dhcp4_messages will be > 0, so the code block which sets the default route from option 3 is skipped:

systemd/src/network/networkd-dhcp4.c

Lines 135 to 181 in 2de2aba

	if (r >= 0 && link->dhcp4_messages <= 0) {
	_cleanup_route_free_ Route *route = NULL;
	_cleanup_route_free_ Route *route_gw = NULL;
	r = route_new(&route);
	if (r < 0)
	return log_link_error_errno(link, r, "Could not allocate route: %m");
	route->protocol = RTPROT_DHCP;
	r = route_new(&route_gw);
	if (r < 0)
	return log_link_error_errno(link, r, "Could not allocate route: %m");
	/* The dhcp netmask may mask out the gateway. Add an explicit
	* route for the gw host so that we can route no matter the
	* netmask or existing kernel route tables. */
	route_gw->family = AF_INET;
	route_gw->dst.in = gateway;
	route_gw->dst_prefixlen = 32;
	route_gw->prefsrc.in = address;
	route_gw->scope = RT_SCOPE_LINK;
	route_gw->protocol = RTPROT_DHCP;
	route_gw->priority = link->network->dhcp_route_metric;
	route_gw->table = table;
	r = route_configure(route_gw, link, dhcp4_route_handler);
	if (r < 0)
	return log_link_warning_errno(link, r, "Could not set host route: %m");
	link->dhcp4_messages++;
	route->family = AF_INET;
	route->gw.in = gateway;
	route->prefsrc.in = address;
	route->priority = link->network->dhcp_route_metric;
	route->table = table;
	r = route_configure(route, link, dhcp4_route_handler);
	if (r < 0) {
	log_link_warning_errno(link, r, "Could not set routes: %m");
	link_enter_failed(link);
	return r;
	}
	link->dhcp4_messages++;
	}

.

[ssahani]

When only the DHCP-option "static-route" (3) is provided (but no "classless-static-route" option is present), the route given by DHCP-option "router" (3) should still be used. Current behavior: The route from option 3 is discarded.

No we apply both of them I just added debug logs to test them.
Really? Did you also check that the default route is actually set?

I looked at option 3 not option 33 Sorry I misread. Of Course if any of them is there default route won't be applied. That was the commit we did for.

[thierer]

That was the commit we did for.

I don't think so. #5695 ist about ignoring the default route when option 121 is set (which makes sense, because option 121 can also be used to set the default route, so the two could be in conflict). But 5f04a20 also causes the default route via option 3 to be ignored if (only) option 33 is set. Which is neither required by RFC 3442 nor does it make sense, because option 33 can't be used to set the default route.

[ssahani]

hmm ssahani@765b7bc Does this fixes your issue ?

[thierer]

I wasn't successful when trying to build a custom package of systemd for Arch, so I can't test it, but from looking at the patch I'd say that should be correct.

I still think there should be warnings when routes are discarded, but I'd consider my original issue as fixed. Thanks!