Merge ~rbalint/ubuntu/+source/systemd:ubuntu-focal-lp1870410 into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-focal

Proposed by Balint Reczey
Status: Merged
Merged at revision: 4c48893eb04b01a2ec62d2d2823a79a9f5cb2b80
Proposed branch: ~rbalint/ubuntu/+source/systemd:ubuntu-focal-lp1870410
Merge into: ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-focal
Diff against target: 102 lines (+88/-0)
2 files modified
debian/patches/dhcp-Allow-setting-request-options-again.patch (+87/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Dimitri John Ledkov Pending
Review via email: mp+382713@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Balint Reczey (rbalint) wrote :

It fixed wifi on my rpi2 with external wifi dongle and is unlikely to cause regressions.

I'm looking into not calling link_configure multiple times or at least skipping bigger parts of the configuration before forwarding this upstream.

Revision history for this message
Dan Streetman (ddstreet) wrote :

It's possible it's being initialized multiple times because the Ubuntu systemd reverts this (and some related) patch:
https://github.com/systemd/systemd/commit/bd08ce56156751d58584a44e766ef61340cdae2d

The revert commit:
https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?h=ubuntu-focal&id=70e93c2a05f0f7435614f8d52944d726601e319c

I haven't looked at netplan's autopkgtest, but it seems to me like that should be fixed, instead of carrying a problematic revert from upstream.

Revision history for this message
Balint Reczey (rbalint) wrote :

@ddstreet yes, I dropped the reverts but systemd autopkgtest networkd-test.py fails without them now, too. I'm looking into the problem, but this very small change seems better for focal because it comes with lower risk.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/patches/dhcp-Allow-setting-request-options-again.patch b/debian/patches/dhcp-Allow-setting-request-options-again.patch
2new file mode 100644
3index 0000000..683cef6
4--- /dev/null
5+++ b/debian/patches/dhcp-Allow-setting-request-options-again.patch
6@@ -0,0 +1,87 @@
7+From: Balint Reczey <balint.reczey@canonical.com>
8+Date: Tue, 21 Apr 2020 23:45:54 +0200
9+Subject: dhcp: Allow setting request options again
10+
11+link_configure() may be called multiple times on a link causing request options
12+set multiple times.
13+
14+LP: #1870410
15+---
16+ src/libsystemd-network/sd-dhcp-client.c | 6 ++++--
17+ src/libsystemd-network/test-dhcp-client.c | 18 +++++++++---------
18+ 2 files changed, 13 insertions(+), 11 deletions(-)
19+
20+diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
21+index 4122d08..54a5dcc 100644
22+--- a/src/libsystemd-network/sd-dhcp-client.c
23++++ b/src/libsystemd-network/sd-dhcp-client.c
24+@@ -188,8 +188,10 @@ int sd_dhcp_client_set_request_option(sd_dhcp_client *client, uint8_t option) {
25+ }
26+
27+ for (i = 0; i < client->req_opts_size; i++)
28+- if (client->req_opts[i] == option)
29+- return -EEXIST;
30++ if (client->req_opts[i] == option) {
31++ log_dhcp_client(client, "Setting already set request option %d", option);
32++ return 0;
33++ }
34+
35+ if (!GREEDY_REALLOC(client->req_opts, client->req_opts_allocated,
36+ client->req_opts_size + 1))
37+diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c
38+index 4e9b388..071a2c3 100644
39+--- a/src/libsystemd-network/test-dhcp-client.c
40++++ b/src/libsystemd-network/test-dhcp-client.c
41+@@ -71,17 +71,17 @@ static void test_request_basic(sd_event *e) {
42+ assert_se(sd_dhcp_client_set_hostname(client, "~host.domain") == -EINVAL);
43+
44+ assert_se(sd_dhcp_client_set_request_option(client,
45+- SD_DHCP_OPTION_SUBNET_MASK) == -EEXIST);
46++ SD_DHCP_OPTION_SUBNET_MASK) == 0);
47+ assert_se(sd_dhcp_client_set_request_option(client,
48+- SD_DHCP_OPTION_ROUTER) == -EEXIST);
49++ SD_DHCP_OPTION_ROUTER) == 0);
50+ /* This PRL option is not set when using Anonymize, but in this test
51+ * Anonymize settings are not being used. */
52+ assert_se(sd_dhcp_client_set_request_option(client,
53+- SD_DHCP_OPTION_HOST_NAME) == -EEXIST);
54++ SD_DHCP_OPTION_HOST_NAME) == 0);
55+ assert_se(sd_dhcp_client_set_request_option(client,
56+- SD_DHCP_OPTION_DOMAIN_NAME) == -EEXIST);
57++ SD_DHCP_OPTION_DOMAIN_NAME) == 0);
58+ assert_se(sd_dhcp_client_set_request_option(client,
59+- SD_DHCP_OPTION_DOMAIN_NAME_SERVER) == -EEXIST);
60++ SD_DHCP_OPTION_DOMAIN_NAME_SERVER) == 0);
61+
62+ assert_se(sd_dhcp_client_set_request_option(client,
63+ SD_DHCP_OPTION_PAD) == -EINVAL);
64+@@ -101,9 +101,9 @@ static void test_request_basic(sd_event *e) {
65+ * Options not set by default (using or not anonymize) are option 17
66+ * (SD_DHCP_OPTION_ROOT_PATH) and 42 (SD_DHCP_OPTION_NTP_SERVER) */
67+ assert_se(sd_dhcp_client_set_request_option(client, 17) == 0);
68+- assert_se(sd_dhcp_client_set_request_option(client, 17) == -EEXIST);
69++ assert_se(sd_dhcp_client_set_request_option(client, 17) == 0);
70+ assert_se(sd_dhcp_client_set_request_option(client, 42) == 0);
71+- assert_se(sd_dhcp_client_set_request_option(client, 17) == -EEXIST);
72++ assert_se(sd_dhcp_client_set_request_option(client, 17) == 0);
73+
74+ sd_dhcp_client_unref(client);
75+ }
76+@@ -126,7 +126,7 @@ static void test_request_anonymize(sd_event *e) {
77+ assert_se(r >= 0);
78+
79+ assert_se(sd_dhcp_client_set_request_option(client,
80+- SD_DHCP_OPTION_NETBIOS_NAMESERVER) == -EEXIST);
81++ SD_DHCP_OPTION_NETBIOS_NAMESERVER) == 0);
82+ /* This PRL option is not set when using Anonymize */
83+ assert_se(sd_dhcp_client_set_request_option(client,
84+ SD_DHCP_OPTION_HOST_NAME) == 0);
85+@@ -137,7 +137,7 @@ static void test_request_anonymize(sd_event *e) {
86+ /* RFC7844: option 101 (SD_DHCP_OPTION_NEW_TZDB_TIMEZONE) is not set in the
87+ * default PRL when using Anonymize, */
88+ assert_se(sd_dhcp_client_set_request_option(client, 101) == 0);
89+- assert_se(sd_dhcp_client_set_request_option(client, 101) == -EEXIST);
90++ assert_se(sd_dhcp_client_set_request_option(client, 101) == 0);
91+
92+ sd_dhcp_client_unref(client);
93+ }
94diff --git a/debian/patches/series b/debian/patches/series
95index 1eea8ff..48b492b 100644
96--- a/debian/patches/series
97+++ b/debian/patches/series
98@@ -57,3 +57,4 @@ lp1845909/0002-network-add-link-setting_genmode-flag.patch
99 Revert-network-if-sys-is-rw-then-udev-should-be-around.patch
100 network-Set-link-state-to-LINK_STATE_PENDING-instead-of-t.patch
101 riscv64-seccomp.patch
102+dhcp-Allow-setting-request-options-again.patch

Subscribers

People subscribed via source and target branches