Ubuntu
systemd package

Merge ~rafaeldtinoco/ubuntu/+source/systemd:lp1815101-eoan into ubuntu/+source/systemd:ubuntu/eoan-devel

  1. Git
  2. lp:~rafaeldtinoco/ubuntu/+source/systemd
  3. lp1815101-eoan
  4. Merge into ubuntu/eoan-devel
Proposed by Rafael David Tinoco
Status: Merged
Approved by: Christian Ehrhardt 
Approved revision: f7adc9af11f0e34607380f64677cc71b9df7ebfe
Merge reported by: Bryce Harrington
Merged at revision: f7adc9af11f0e34607380f64677cc71b9df7ebfe
Proposed branch: ~rafaeldtinoco/ubuntu/+source/systemd:lp1815101-eoan
Merge into: ubuntu/+source/systemd:ubuntu/eoan-devel
Diff against target: 674 lines (+628/-0)
7 files modified
debian/changelog (+11/-0)
debian/patches/lp1815101-01-networkd-add-support-to-keep-configuration.patch (+209/-0)
debian/patches/lp1815101-02-networkd-stop-clients-when-networkd-shuts-down.patch (+94/-0)
debian/patches/lp1815101-03-network-add-KeepConfiguration-dhcp-on-stop.patch (+155/-0)
debian/patches/lp1815101-04-network-make-KeepConfiguration-static-drop-DHCP-addr.patch (+93/-0)
debian/patches/lp1815101-05-man-add-documentation-about-KeepConfiguration.patch (+61/-0)
debian/patches/series (+5/-0)
Reviewer Review Type Date Requested Status
Dan Streetman (community) Approve
Christian Ehrhardt  (community) Approve
Balint Reczey Pending
Dimitri John Ledkov Pending
Canonical Server Pending
Review via email: mp+374027@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

@xnox,

would you mind telling me if you see this as a SRU for Disco and Bionic as well ? looks like sustaining want a fix for networkd restarts dropping HA aliases from nics. more robust HA software (like pacemaker) have monitors on VIP resources and restart virtual IPs when networkd is restarted and wipes virtual aliases, but, some others, like keepalived or ha-proxy, don't: thats why I want to consider KeepConfiguration= as a "fix" and not as a feature, if you agree.

(This will help us out not pursing all HA software and checking which of them are broken because networkd wipes the aliases, and consider this as 1 fix for all).

comments ?

thx a lot!

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

Overall, the patchset looks mostly good (one comment farther down in the patches), but I'm quite concerned about handling of dhcp addresses on stop, start, and restart. I don't think it's even entirely worked out correctly/fully upstream yet. That's not to say that the dhcp handling behavior in our SRU releases is correct, or that upstream is correct, it's just complex and I feel like the KeepConfiguration param makes it more complex.

Since HA and the other software that need this only uses manually-added static addresses/routes (right?), I'd much prefer something smaller, less complex, that only causes networkd to ignore foreign static addresses/routes.

But I don't currently have any suggestions on how to do that :-)

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Dan,

I agree with you 100% there. I made sure to review the code being added to this SRU to check if, at least, it would keep default behavior without KeepConfiguration= being set, and that is true.

Other behaviors could be considered "broken" for Eoan/Disco/Bionic (possibly upstream ?) so I don't think, like you said, we should care too much (specially when one of them brakes DHCP spec, by not ever releasing the lease, on purpose).

The summary for this change is pretty much:

- [KEEP_CONFIGURATION_NO] = "no",
- [KEEP_CONFIGURATION_DHCP] = "dhcp",
- [KEEP_CONFIGURATION_STATIC] = "static",
- [KEEP_CONFIGURATION_YES] = "yes",

+ [KEEP_CONFIGURATION_NO] = "no",
+ [KEEP_CONFIGURATION_DHCP_ON_STOP] = "dhcp-on-stop",
+ [KEEP_CONFIGURATION_DHCP] = "dhcp",
+ [KEEP_CONFIGURATION_STATIC] = "static",
+ [KEEP_CONFIGURATION_YES] = "yes",

where:

"dhcp-stop": will not drop dhcp addresses and routes on stopping daemon (fixing restart issues)
"dhcp": will NEVER drop dhcp addresses (breaking dhcp spec on purpose, just like upstream)

so, as long as we default to "dhcp-stop" we have the same behavior as the previous CriticalConnection= setting, even if it is still used. So, I'm not removing or breaking existing configs.

Perhaps, a suggestion to your point, would be to have "static" only option, but then I would have to backport the feature not being 1:1 on upstream commits, harder to maintain and a worse delta in the next merge :\.

No free lunch I guess...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I have reviewed the patches and they LGTM as well.
The complexity discussion is lost as soon as you consider what it already does and I appreciate that we fix it with whatever upstream did in later versions instead of coming up with our own version of it - that way at least only one kind of complexity exists :-)

This definitely needs a review of someone skilled in systemd and I appreciate that ddstreet already looked over it and would also wait and see what xnox says. We might subscribe rbalint as well, I saw him do a lot of systemd recently.

Also on upload this maybe deserves a few extra days in proposed and given the time you most likely need to modify this to be an SRU. I guess early 20.04 will get systemd 243 and you then need to modify this to become 242-7ubuntu2.1 for eoan.

review: Approve
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Thank you Christian, subscribed rbalint also.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

> Thank you Christian, subscribed rbalint also.

Err, you already did. Ok.

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

Ok, +1 from me (not that my approval is needed :)

We'll get the complexity this param adds in 20.04 anyway, and I don't see any other way better than this to handle the problem.

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

I'll include this in my upload to Eoan unless I hear any objections.

Thanks!

Revision history for this message
Bryce Harrington (bryce) wrote :

This looks like it has migrated as 242-7ubuntu3.2 for systemd in eoan-updates (main).
https://launchpad.net/ubuntu/+source/systemd/242-7ubuntu3.2

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index e18b0fa..16c9115 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,14 @@
6+systemd (242-7ubuntu3) eoan; urgency=medium
7+
8+ * Add support to KeepConfiguration= fixing behaviour for HA (LP: #1815101)
9+ - d/p/lp1815101-01-networkd-add-support-to-keep-configuration.patch
10+ - d/p/lp1815101-02-networkd-stop-clients-when-networkd-shuts-down.patch
11+ - d/p/lp1815101-03-network-add-KeepConfiguration-dhcp-on-stop.patch
12+ - d/p/lp1815101-04-network-make-KeepConfiguration-static-drop-DHCP-addr.patch
13+ - d/p/lp1815101-05-man-add-documentation-about-KeepConfiguration.patch
14+
15+ -- Rafael David Tinoco <rafaeldtinoco@ubuntu.com> Fri, 11 Oct 2019 01:44:38 +0000
16+
17 systemd (242-7ubuntu2) eoan; urgency=medium
18
19 [ Bryan Quigley ]
20diff --git a/debian/patches/lp1815101-01-networkd-add-support-to-keep-configuration.patch b/debian/patches/lp1815101-01-networkd-add-support-to-keep-configuration.patch
21new file mode 100644
22index 0000000..9674a57
23--- /dev/null
24+++ b/debian/patches/lp1815101-01-networkd-add-support-to-keep-configuration.patch
25@@ -0,0 +1,209 @@
26+Description: networkd: add support to keep configuration
27+Author: Susant Sahani <ssahani@vmware.com>
28+Origin: backport, https://github.com/systemd/systemd/commit/7da377ef16a2112a673247b39041a180b07e973a
29+Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1815101
30+Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
31+Last-Update: 2019-10-10
32+
33+networkd: add support to keep configuration
34+
35+---
36+ src/network/networkd-dhcp4.c | 6 ++--
37+ src/network/networkd-link.c | 3 +-
38+ src/network/networkd-network-gperf.gperf | 3 +-
39+ src/network/networkd-network.c | 29 +++++++++++++++++++
40+ src/network/networkd-network.h | 16 +++++++++-
41+ .../fuzz-network-parser/directives.network | 1 +
42+ 6 files changed, 52 insertions(+), 6 deletions(-)
43+
44+diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c
45+index 85685d6df..956ae6eef 100644
46+--- a/src/network/networkd-dhcp4.c
47++++ b/src/network/networkd-dhcp4.c
48+@@ -378,7 +378,7 @@ static int dhcp_lease_renew(sd_dhcp_client *client, Link *link) {
49+ if (r < 0)
50+ return log_link_warning_errno(link, r, "DHCP error: no netmask: %m");
51+
52+- if (!link->network->dhcp_critical) {
53++ if (!FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_DHCP)) {
54+ r = sd_dhcp_lease_get_lifetime(link->dhcp_lease, &lifetime);
55+ if (r < 0)
56+ return log_link_warning_errno(link, r, "DHCP error: no lifetime: %m");
57+@@ -493,7 +493,7 @@ static int dhcp_lease_acquired(sd_dhcp_client *client, Link *link) {
58+ }
59+ }
60+
61+- if (!link->network->dhcp_critical) {
62++ if (!FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_DHCP)) {
63+ r = sd_dhcp_lease_get_lifetime(link->dhcp_lease, &lifetime);
64+ if (r < 0)
65+ return log_link_warning_errno(link, r, "DHCP error: no lifetime: %m");
66+@@ -523,7 +523,7 @@ static void dhcp4_handler(sd_dhcp_client *client, int event, void *userdata) {
67+ case SD_DHCP_CLIENT_EVENT_EXPIRED:
68+ case SD_DHCP_CLIENT_EVENT_STOP:
69+ case SD_DHCP_CLIENT_EVENT_IP_CHANGE:
70+- if (link->network->dhcp_critical) {
71++ if (FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_DHCP)) {
72+ log_link_error(link, "DHCPv4 connection considered system critical, ignoring request to reconfigure it.");
73+ return;
74+ }
75+diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
76+index 15e5c72ed..35e3806b2 100644
77+--- a/src/network/networkd-link.c
78++++ b/src/network/networkd-link.c
79+@@ -3063,7 +3063,8 @@ static int link_configure(Link *link) {
80+
81+ /* Drop foreign config, but ignore loopback or critical devices.
82+ * We do not want to remove loopback address or addresses used for root NFS. */
83+- if (!(link->flags & IFF_LOOPBACK) && !(link->network->dhcp_critical)) {
84++ if (!(link->flags & IFF_LOOPBACK) &&
85++ !(link->network->keep_configuration & (KEEP_CONFIGURATION_DHCP | KEEP_CONFIGURATION_STATIC))) {
86+ r = link_drop_foreign_config(link);
87+ if (r < 0)
88+ return r;
89+diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf
90+index 47a9f7d80..96e786929 100644
91+--- a/src/network/networkd-network-gperf.gperf
92++++ b/src/network/networkd-network-gperf.gperf
93+@@ -83,6 +83,7 @@ Network.IPv6ProxyNDPAddress, config_parse_ipv6_proxy_ndp_address,
94+ Network.BindCarrier, config_parse_strv, 0, offsetof(Network, bind_carrier)
95+ Network.ConfigureWithoutCarrier, config_parse_bool, 0, offsetof(Network, configure_without_carrier)
96+ Network.IgnoreCarrierLoss, config_parse_bool, 0, offsetof(Network, ignore_carrier_loss)
97++Network.KeepConfiguration, config_parse_keep_configuration, 0, offsetof(Network, keep_configuration)
98+ Address.Address, config_parse_address, 0, 0
99+ Address.Peer, config_parse_address, 0, 0
100+ Address.Broadcast, config_parse_broadcast, 0, 0
101+@@ -137,7 +138,7 @@ DHCP.Anonymize, config_parse_bool,
102+ DHCP.SendHostname, config_parse_bool, 0, offsetof(Network, dhcp_send_hostname)
103+ DHCP.Hostname, config_parse_hostname, 0, offsetof(Network, dhcp_hostname)
104+ DHCP.RequestBroadcast, config_parse_bool, 0, offsetof(Network, dhcp_broadcast)
105+-DHCP.CriticalConnection, config_parse_bool, 0, offsetof(Network, dhcp_critical)
106++DHCP.CriticalConnection, config_parse_tristate, 0, offsetof(Network, dhcp_critical) /* deprecated */
107+ DHCP.VendorClassIdentifier, config_parse_string, 0, offsetof(Network, dhcp_vendor_class_identifier)
108+ DHCP.UserClass, config_parse_dhcp_user_class, 0, offsetof(Network, dhcp_user_class)
109+ DHCP.DUIDType, config_parse_duid_type, 0, offsetof(Network, duid)
110+diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c
111+index 112e9cfbe..5ab3e45cf 100644
112+--- a/src/network/networkd-network.c
113++++ b/src/network/networkd-network.c
114+@@ -252,6 +252,20 @@ int network_verify(Network *network) {
115+ network->dhcp_use_mtu = false;
116+ }
117+
118++ if (network->dhcp_critical >= 0) {
119++ if (network->keep_configuration >= 0)
120++ log_warning("%s: Both KeepConfiguration= and deprecated CriticalConnection= are set. "
121++ "Ignoring CriticalConnection=.", network->filename);
122++ else if (network->dhcp_critical)
123++ /* CriticalConnection=yes also preserve foreign static configurations. */
124++ network->keep_configuration = KEEP_CONFIGURATION_YES;
125++ else
126++ network->keep_configuration = KEEP_CONFIGURATION_NO;
127++ }
128++
129++ if (network->keep_configuration < 0)
130++ network->keep_configuration = KEEP_CONFIGURATION_NO;
131++
132+ LIST_FOREACH_SAFE(addresses, address, address_next, network->static_addresses)
133+ if (address_section_verify(address) < 0)
134+ address_free(address);
135+@@ -334,6 +348,7 @@ int network_load_one(Manager *manager, const char *filename) {
136+ .required_for_online = true,
137+ .required_operstate_for_online = LINK_OPERSTATE_DEGRADED,
138+ .dhcp = ADDRESS_FAMILY_NO,
139++ .dhcp_critical = -1,
140+ .dhcp_use_ntp = true,
141+ .dhcp_use_dns = true,
142+ .dhcp_use_hostname = true,
143+@@ -401,6 +416,8 @@ int network_load_one(Manager *manager, const char *filename) {
144+ .ipv6_accept_ra_route_table = RT_TABLE_MAIN,
145+ .ipv6_accept_ra_route_table_set = false,
146+
147++ .keep_configuration = _KEEP_CONFIGURATION_INVALID,
148++
149+ .can_triple_sampling = -1,
150+ };
151+
152+@@ -1645,3 +1662,15 @@ int config_parse_required_for_online(
153+
154+ return 0;
155+ }
156++
157++DEFINE_CONFIG_PARSE_ENUM(config_parse_keep_configuration, keep_configuration, KeepConfiguration,
158++ "Failed to parse KeepConfiguration= setting");
159++
160++static const char* const keep_configuration_table[_KEEP_CONFIGURATION_MAX] = {
161++ [KEEP_CONFIGURATION_NO] = "no",
162++ [KEEP_CONFIGURATION_DHCP] = "dhcp",
163++ [KEEP_CONFIGURATION_STATIC] = "static",
164++ [KEEP_CONFIGURATION_YES] = "yes",
165++};
166++
167++DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(keep_configuration, KeepConfiguration, KEEP_CONFIGURATION_YES);
168+diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h
169+index 852144da3..c290a63d1 100644
170+--- a/src/network/networkd-network.h
171++++ b/src/network/networkd-network.h
172+@@ -84,6 +84,15 @@ typedef enum RADVPrefixDelegation {
173+ _RADV_PREFIX_DELEGATION_INVALID = -1,
174+ } RADVPrefixDelegation;
175+
176++typedef enum KeepConfiguration {
177++ KEEP_CONFIGURATION_NO = 0,
178++ KEEP_CONFIGURATION_DHCP = 1 << 0,
179++ KEEP_CONFIGURATION_STATIC = 1 << 1,
180++ KEEP_CONFIGURATION_YES = KEEP_CONFIGURATION_DHCP | KEEP_CONFIGURATION_STATIC,
181++ _KEEP_CONFIGURATION_MAX,
182++ _KEEP_CONFIGURATION_INVALID = -1,
183++} KeepConfiguration;
184++
185+ typedef struct Manager Manager;
186+
187+ struct Network {
188+@@ -122,7 +131,7 @@ struct Network {
189+ bool dhcp_anonymize;
190+ bool dhcp_send_hostname;
191+ bool dhcp_broadcast;
192+- bool dhcp_critical;
193++ int dhcp_critical;
194+ bool dhcp_use_dns;
195+ bool dhcp_use_ntp;
196+ bool dhcp_use_mtu;
197+@@ -223,6 +232,7 @@ struct Network {
198+ bool unmanaged;
199+ bool configure_without_carrier;
200+ bool ignore_carrier_loss;
201++ KeepConfiguration keep_configuration;
202+ uint32_t iaid;
203+ DUID duid;
204+
205+@@ -315,6 +325,7 @@ CONFIG_PARSER_PROTOTYPE(config_parse_dhcp_user_class);
206+ CONFIG_PARSER_PROTOTYPE(config_parse_ntp);
207+ CONFIG_PARSER_PROTOTYPE(config_parse_iaid);
208+ CONFIG_PARSER_PROTOTYPE(config_parse_required_for_online);
209++CONFIG_PARSER_PROTOTYPE(config_parse_keep_configuration);
210+ /* Legacy IPv4LL support */
211+ CONFIG_PARSER_PROTOTYPE(config_parse_ipv4ll);
212+
213+@@ -336,3 +347,6 @@ LLDPMode lldp_mode_from_string(const char *s) _pure_;
214+
215+ const char* radv_prefix_delegation_to_string(RADVPrefixDelegation i) _const_;
216+ RADVPrefixDelegation radv_prefix_delegation_from_string(const char *s) _pure_;
217++
218++const char* keep_configuration_to_string(KeepConfiguration i) _const_;
219++KeepConfiguration keep_configuration_from_string(const char *s) _pure_;
220+diff --git a/test/fuzz/fuzz-network-parser/directives.network b/test/fuzz/fuzz-network-parser/directives.network
221+index ddc60a9cb..039c7b82e 100644
222+--- a/test/fuzz/fuzz-network-parser/directives.network
223++++ b/test/fuzz/fuzz-network-parser/directives.network
224+@@ -122,6 +122,7 @@ DHCPServer=
225+ BindCarrier=
226+ VRF=
227+ IgnoreCarrierLoss=
228++KeepConfiguration=
229+ [IPv6Prefix]
230+ Prefix=
231+ OnLink=
232+--
233+2.20.1
234+
235diff --git a/debian/patches/lp1815101-02-networkd-stop-clients-when-networkd-shuts-down.patch b/debian/patches/lp1815101-02-networkd-stop-clients-when-networkd-shuts-down.patch
236new file mode 100644
237index 0000000..041a30e
238--- /dev/null
239+++ b/debian/patches/lp1815101-02-networkd-stop-clients-when-networkd-shuts-down.patch
240@@ -0,0 +1,94 @@
241+Description: networkd: stop clients when networkd shuts down (#12463)
242+Author: Susant Sahani <ssahani@redhat.com>
243+Origin: backport, https://github.com/systemd/systemd/commit/946f8e14d59ec1262f1779bc9a65d1c048d6544b
244+Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1815101
245+Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
246+Last-Update: 2019-10-11
247+
248+networkd: stop clients when networkd shuts down (#12463)
249+
250+We not stopping the clients when networkd stops. They
251+should shut down cleanly and then we need to clean the DS.
252+
253+One of requirements to implement
254+https://github.com/systemd/systemd/issues/10820.
255+
256+```
257+^CBus bus-api-network: changing state RUNNING → CLOSED
258+DHCP SERVER: UNREF
259+DHCP SERVER: STOPPED
260+DHCP CLIENT (0x60943df0): STOPPED
261+veth-test: DHCP lease lost
262+veth-test: Removing address 192.168.5.31
263+NDISC: Stopping IPv6 Router Solicitation client
264+DHCP CLIENT (0x0): FREE
265+==24308==
266+==24308== HEAP SUMMARY:
267+==24308== in use at exit: 8,192 bytes in 2 blocks
268+==24308== total heap usage: 4,230 allocs, 4,228 frees, 1,209,732 bytes allocated
269+==24308==
270+==24308== LEAK SUMMARY:
271+==24308== definitely lost: 0 bytes in 0 blocks
272+==24308== indirectly lost: 0 bytes in 0 blocks
273+==24308== possibly lost: 0 bytes in 0 blocks
274+==24308== still reachable: 8,192 bytes in 2 blocks
275+==24308== suppressed: 0 bytes in 0 blocks
276+==24308== Rerun with --leak-check=full to see details of leaked memory
277+==24308==
278+==24308== For lists of detected and suppressed errors, rerun with: -s
279+==24308== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
280+==24308== could not unlink /tmp/vgdb-pipe-from-vgdb-to-24308-by-sus-on-Zeus
281+==24308== could not unlink /tmp/vgdb-pipe-to-vgdb-from-24308-by-sus-on-Zeus
282+==24308== could not unlink /tmp/vgdb-pipe-shared-mem-vgdb-24308-by-sus-on-Zeus
283+
284+```
285+
286+---
287+ src/network/networkd-link.c | 2 +-
288+ src/network/networkd-link.h | 2 ++
289+ src/network/networkd-manager.c | 3 +++
290+ 3 files changed, 6 insertions(+), 1 deletion(-)
291+
292+diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
293+index 35e3806b2..0ac415a0e 100644
294+--- a/src/network/networkd-link.c
295++++ b/src/network/networkd-link.c
296+@@ -759,7 +759,7 @@ static void link_enter_unmanaged(Link *link) {
297+ link_dirty(link);
298+ }
299+
300+-static int link_stop_clients(Link *link) {
301++int link_stop_clients(Link *link) {
302+ int r = 0, k;
303+
304+ assert(link);
305+diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h
306+index f2e53cc57..031a90eac 100644
307+--- a/src/network/networkd-link.h
308++++ b/src/network/networkd-link.h
309+@@ -169,6 +169,8 @@ int dhcp6_configure(Link *link);
310+ int dhcp6_request_address(Link *link, int ir);
311+ int dhcp6_lease_pd_prefix_lost(sd_dhcp6_client *client, Link* link);
312+
313++int link_stop_clients(Link *link);
314++
315+ const char* link_state_to_string(LinkState s) _const_;
316+ LinkState link_state_from_string(const char *s) _pure_;
317+
318+diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
319+index 05107da83..e1bbde233 100644
320+--- a/src/network/networkd-manager.c
321++++ b/src/network/networkd-manager.c
322+@@ -1446,6 +1446,9 @@ void manager_free(Manager *m) {
323+ while ((link = hashmap_steal_first(m->links))) {
324+ if (link->dhcp6_client)
325+ (void) dhcp6_lease_pd_prefix_lost(link->dhcp6_client, link);
326++
327++ link_stop_clients(link);
328++
329+ link_unref(link);
330+ }
331+
332+--
333+2.20.1
334+
335diff --git a/debian/patches/lp1815101-03-network-add-KeepConfiguration-dhcp-on-stop.patch b/debian/patches/lp1815101-03-network-add-KeepConfiguration-dhcp-on-stop.patch
336new file mode 100644
337index 0000000..887b521
338--- /dev/null
339+++ b/debian/patches/lp1815101-03-network-add-KeepConfiguration-dhcp-on-stop.patch
340@@ -0,0 +1,155 @@
341+Description: network: add KeepConfiguration=dhcp-on-stop
342+Author: Yu Watanabe <watanabe.yu+github@gmail.com>
343+Origin: backport, https://github.com/systemd/systemd/commit/95355a281c06c5970b7355c38b066910c3be4958
344+Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1815101
345+Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
346+Last-Update: 2019-10-11
347+
348+network: add KeepConfiguration=dhcp-on-stop
349+
350+The option prevents to drop lease address on stop.
351+By setting this, we can safely restart networkd.
352+
353+---
354+ src/network/networkd-link.c | 11 ++++++-----
355+ src/network/networkd-link.h | 2 +-
356+ src/network/networkd-manager.c | 2 +-
357+ src/network/networkd-network.c | 15 +++++++++------
358+ src/network/networkd-network.h | 10 ++++++----
359+ 5 files changed, 23 insertions(+), 17 deletions(-)
360+
361+diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
362+index 0ac415a0e..01ef248d6 100644
363+--- a/src/network/networkd-link.c
364++++ b/src/network/networkd-link.c
365+@@ -759,14 +759,15 @@ static void link_enter_unmanaged(Link *link) {
366+ link_dirty(link);
367+ }
368+
369+-int link_stop_clients(Link *link) {
370++int link_stop_clients(Link *link, bool may_keep_dhcp) {
371+ int r = 0, k;
372+
373+ assert(link);
374+ assert(link->manager);
375+ assert(link->manager->event);
376+
377+- if (link->dhcp_client) {
378++ if (link->dhcp_client && (!may_keep_dhcp || !link->network ||
379++ !FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_DHCP_ON_STOP))) {
380+ k = sd_dhcp_client_stop(link->dhcp_client);
381+ if (k < 0)
382+ r = log_link_warning_errno(link, k, "Could not stop DHCPv4 client: %m");
383+@@ -810,7 +811,7 @@ void link_enter_failed(Link *link) {
384+
385+ link_set_state(link, LINK_STATE_FAILED);
386+
387+- link_stop_clients(link);
388++ link_stop_clients(link, false);
389+
390+ link_dirty(link);
391+ }
392+@@ -3064,7 +3065,7 @@ static int link_configure(Link *link) {
393+ /* Drop foreign config, but ignore loopback or critical devices.
394+ * We do not want to remove loopback address or addresses used for root NFS. */
395+ if (!(link->flags & IFF_LOOPBACK) &&
396+- !(link->network->keep_configuration & (KEEP_CONFIGURATION_DHCP | KEEP_CONFIGURATION_STATIC))) {
397++ !(link->network->keep_configuration & (KEEP_CONFIGURATION_DHCP_ON_START | KEEP_CONFIGURATION_STATIC))) {
398+ r = link_drop_foreign_config(link);
399+ if (r < 0)
400+ return r;
401+@@ -3773,7 +3774,7 @@ static int link_carrier_lost(Link *link) {
402+ if (link->setting_mtu)
403+ return 0;
404+
405+- r = link_stop_clients(link);
406++ r = link_stop_clients(link, false);
407+ if (r < 0) {
408+ link_enter_failed(link);
409+ return r;
410+diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h
411+index 031a90eac..60c5a594d 100644
412+--- a/src/network/networkd-link.h
413++++ b/src/network/networkd-link.h
414+@@ -169,7 +169,7 @@ int dhcp6_configure(Link *link);
415+ int dhcp6_request_address(Link *link, int ir);
416+ int dhcp6_lease_pd_prefix_lost(sd_dhcp6_client *client, Link* link);
417+
418+-int link_stop_clients(Link *link);
419++int link_stop_clients(Link *link, bool may_keep_dhcp);
420+
421+ const char* link_state_to_string(LinkState s) _const_;
422+ LinkState link_state_from_string(const char *s) _pure_;
423+diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
424+index e1bbde233..4ce03eeb0 100644
425+--- a/src/network/networkd-manager.c
426++++ b/src/network/networkd-manager.c
427+@@ -1447,7 +1447,7 @@ void manager_free(Manager *m) {
428+ if (link->dhcp6_client)
429+ (void) dhcp6_lease_pd_prefix_lost(link->dhcp6_client, link);
430+
431+- link_stop_clients(link);
432++ (void) link_stop_clients(link, true);
433+
434+ link_unref(link);
435+ }
436+diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c
437+index 5ab3e45cf..513491df1 100644
438+--- a/src/network/networkd-network.c
439++++ b/src/network/networkd-network.c
440+@@ -260,11 +260,13 @@ int network_verify(Network *network) {
441+ /* CriticalConnection=yes also preserve foreign static configurations. */
442+ network->keep_configuration = KEEP_CONFIGURATION_YES;
443+ else
444+- network->keep_configuration = KEEP_CONFIGURATION_NO;
445++ /* For backward compatibility, we do not release DHCP addresses on manager stop. */
446++ network->keep_configuration = KEEP_CONFIGURATION_DHCP_ON_STOP;
447+ }
448+
449+ if (network->keep_configuration < 0)
450+- network->keep_configuration = KEEP_CONFIGURATION_NO;
451++ /* For backward compatibility, we do not release DHCP addresses on manager stop. */
452++ network->keep_configuration = KEEP_CONFIGURATION_DHCP_ON_STOP;
453+
454+ LIST_FOREACH_SAFE(addresses, address, address_next, network->static_addresses)
455+ if (address_section_verify(address) < 0)
456+@@ -1667,10 +1669,11 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_keep_configuration, keep_configuration, Ke
457+ "Failed to parse KeepConfiguration= setting");
458+
459+ static const char* const keep_configuration_table[_KEEP_CONFIGURATION_MAX] = {
460+- [KEEP_CONFIGURATION_NO] = "no",
461+- [KEEP_CONFIGURATION_DHCP] = "dhcp",
462+- [KEEP_CONFIGURATION_STATIC] = "static",
463+- [KEEP_CONFIGURATION_YES] = "yes",
464++ [KEEP_CONFIGURATION_NO] = "no",
465++ [KEEP_CONFIGURATION_DHCP_ON_STOP] = "dhcp-on-stop",
466++ [KEEP_CONFIGURATION_DHCP] = "dhcp",
467++ [KEEP_CONFIGURATION_STATIC] = "static",
468++ [KEEP_CONFIGURATION_YES] = "yes",
469+ };
470+
471+ DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(keep_configuration, KeepConfiguration, KEEP_CONFIGURATION_YES);
472+diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h
473+index c290a63d1..934cb9b2b 100644
474+--- a/src/network/networkd-network.h
475++++ b/src/network/networkd-network.h
476+@@ -85,10 +85,12 @@ typedef enum RADVPrefixDelegation {
477+ } RADVPrefixDelegation;
478+
479+ typedef enum KeepConfiguration {
480+- KEEP_CONFIGURATION_NO = 0,
481+- KEEP_CONFIGURATION_DHCP = 1 << 0,
482+- KEEP_CONFIGURATION_STATIC = 1 << 1,
483+- KEEP_CONFIGURATION_YES = KEEP_CONFIGURATION_DHCP | KEEP_CONFIGURATION_STATIC,
484++ KEEP_CONFIGURATION_NO = 0,
485++ KEEP_CONFIGURATION_DHCP_ON_START = 1 << 0,
486++ KEEP_CONFIGURATION_DHCP_ON_STOP = 1 << 1,
487++ KEEP_CONFIGURATION_DHCP = KEEP_CONFIGURATION_DHCP_ON_START | KEEP_CONFIGURATION_DHCP_ON_STOP,
488++ KEEP_CONFIGURATION_STATIC = 1 << 2,
489++ KEEP_CONFIGURATION_YES = KEEP_CONFIGURATION_DHCP | KEEP_CONFIGURATION_STATIC,
490+ _KEEP_CONFIGURATION_MAX,
491+ _KEEP_CONFIGURATION_INVALID = -1,
492+ } KeepConfiguration;
493+--
494+2.20.1
495+
496diff --git a/debian/patches/lp1815101-04-network-make-KeepConfiguration-static-drop-DHCP-addr.patch b/debian/patches/lp1815101-04-network-make-KeepConfiguration-static-drop-DHCP-addr.patch
497new file mode 100644
498index 0000000..28a9574
499--- /dev/null
500+++ b/debian/patches/lp1815101-04-network-make-KeepConfiguration-static-drop-DHCP-addr.patch
501@@ -0,0 +1,93 @@
502+Description: network: make KeepConfiguration=static drop DHCP addresses and routes
503+Author: Yu Watanabe <watanabe.yu+github@gmail.com>
504+Origin: backport, https://github.com/systemd/systemd/commit/db51778f85cb076e9ed1fe7f7e29cc740365c245
505+Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1815101
506+Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
507+Last-Update: 2019-10-11
508+
509+network: make KeepConfiguration=static drop DHCP addresses and routes
510+
511+Also, KeepConfiguration=dhcp drops static foreign addresses and routes.
512+
513+---
514+ src/network/networkd-link.c | 43 ++++++++++++++++++++++++++++++++++++-
515+ 1 file changed, 42 insertions(+), 1 deletion(-)
516+
517+diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
518+index 01ef248d6..1a9539c5d 100644
519+--- a/src/network/networkd-link.c
520++++ b/src/network/networkd-link.c
521+@@ -2921,6 +2921,33 @@ static bool link_is_static_route_configured(Link *link, Route *route) {
522+ return false;
523+ }
524+
525++static bool link_address_is_dynamic(Link *link, Address *address) {
526++ Route *route;
527++ Iterator i;
528++
529++ assert(link);
530++ assert(address);
531++
532++ if (address->cinfo.ifa_prefered != CACHE_INFO_INFINITY_LIFE_TIME)
533++ return true;
534++
535++ /* Even when the address is leased from a DHCP server, networkd assign the address
536++ * without lifetime when KeepConfiguration=dhcp. So, let's check that we have
537++ * corresponding routes with RTPROT_DHCP. */
538++ SET_FOREACH(route, link->routes_foreign, i) {
539++ if (route->protocol != RTPROT_DHCP)
540++ continue;
541++
542++ if (address->family != route->family)
543++ continue;
544++
545++ if (in_addr_equal(address->family, &address->in_addr, &route->prefsrc))
546++ return true;
547++ }
548++
549++ return false;
550++}
551++
552+ static int link_drop_foreign_config(Link *link) {
553+ Address *address;
554+ Route *route;
555+@@ -2932,6 +2959,12 @@ static int link_drop_foreign_config(Link *link) {
556+ if (address->family == AF_INET6 && in_addr_is_link_local(AF_INET6, &address->in_addr) == 1)
557+ continue;
558+
559++ if (link_address_is_dynamic(link, address)) {
560++ if (FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_DHCP))
561++ continue;
562++ } else if (FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_STATIC))
563++ continue;
564++
565+ if (link_is_static_address_configured(link, address)) {
566+ r = address_add(link, address->family, &address->in_addr, address->prefixlen, NULL);
567+ if (r < 0)
568+@@ -2948,6 +2981,14 @@ static int link_drop_foreign_config(Link *link) {
569+ if (route->protocol == RTPROT_KERNEL)
570+ continue;
571+
572++ if (route->protocol == RTPROT_STATIC &&
573++ FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_STATIC))
574++ continue;
575++
576++ if (route->protocol == RTPROT_DHCP &&
577++ FLAGS_SET(link->network->keep_configuration, KEEP_CONFIGURATION_DHCP))
578++ continue;
579++
580+ if (link_is_static_route_configured(link, route)) {
581+ r = route_add(link, route->family, &route->dst, route->dst_prefixlen, route->tos, route->priority, route->table, NULL);
582+ if (r < 0)
583+@@ -3065,7 +3106,7 @@ static int link_configure(Link *link) {
584+ /* Drop foreign config, but ignore loopback or critical devices.
585+ * We do not want to remove loopback address or addresses used for root NFS. */
586+ if (!(link->flags & IFF_LOOPBACK) &&
587+- !(link->network->keep_configuration & (KEEP_CONFIGURATION_DHCP_ON_START | KEEP_CONFIGURATION_STATIC))) {
588++ link->network->keep_configuration != KEEP_CONFIGURATION_YES) {
589+ r = link_drop_foreign_config(link);
590+ if (r < 0)
591+ return r;
592+--
593+2.20.1
594+
595diff --git a/debian/patches/lp1815101-05-man-add-documentation-about-KeepConfiguration.patch b/debian/patches/lp1815101-05-man-add-documentation-about-KeepConfiguration.patch
596new file mode 100644
597index 0000000..b09f64a
598--- /dev/null
599+++ b/debian/patches/lp1815101-05-man-add-documentation-about-KeepConfiguration.patch
600@@ -0,0 +1,61 @@
601+Description: man: add documentation about KeepConfiguration
602+Author: Yu Watanabe <watanabe.yu+github@gmail.com>
603+Origin: backport, https://github.com/systemd/systemd/commit/c98d78d32abba6aadbe89eece7acf0742f59047c
604+Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1815101
605+Reviewed-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
606+Last-Update: 2019-10-11
607+
608+man: add documentation about KeepConfiguration
609+
610+---
611+ man/systemd.network.xml | 27 ++++++++++++++++-----------
612+ 1 file changed, 16 insertions(+), 11 deletions(-)
613+
614+diff --git a/man/systemd.network.xml b/man/systemd.network.xml
615+index 4759fd6fd..7c7414ec0 100644
616+--- a/man/systemd.network.xml
617++++ b/man/systemd.network.xml
618+@@ -810,6 +810,22 @@
619+ </para>
620+ </listitem>
621+ </varlistentry>
622++ <varlistentry>
623++ <term><varname>KeepConfiguration=</varname></term>
624++ <listitem>
625++ <para>Takes a boolean or one of <literal>static</literal>, <literal>dhcp-on-stop</literal>,
626++ <literal>dhcp</literal>. When <literal>static</literal>, <command>systemd-networkd</command>
627++ will not drop static addresses and routes on starting up process. When set to
628++ <literal>dhcp-on-stop</literal>, <command>systemd-networkd</command> will not drop addresses
629++ and routes on stopping the daemon. When <literal>dhcp</literal>,
630++ the addresses and routes provided by a DHCP server will never be dropped even if the DHCP
631++ lease expires. This is contrary to the DHCP specification, but may be the best choice if,
632++ e.g., the root filesystem relies on this connection. The setting <literal>dhcp</literal>
633++ implies <literal>dhcp-on-stop</literal>, and <literal>yes</literal> implies
634++ <literal>dhcp</literal> and <literal>static</literal>. Defaults to
635++ <literal>dhcp-on-stop</literal>.</para>
636++ </listitem>
637++ </varlistentry>
638+
639+ </variablelist>
640+
641+@@ -1337,17 +1353,6 @@
642+ system. Defaults to <literal>no</literal>.</para></listitem>
643+ </varlistentry>
644+
645+- <varlistentry>
646+- <term><varname>CriticalConnection=</varname></term>
647+- <listitem>
648+- <para>When true, the connection will never be torn down
649+- even if the DHCP lease expires. This is contrary to the
650+- DHCP specification, but may be the best choice if, say,
651+- the root filesystem relies on this connection. Defaults to
652+- false.</para>
653+- </listitem>
654+- </varlistentry>
655+-
656+ <varlistentry>
657+ <term><varname>ClientIdentifier=</varname></term>
658+ <listitem>
659+--
660+2.20.1
661+
662diff --git a/debian/patches/series b/debian/patches/series
663index 308878f..d7d51a2 100644
664--- a/debian/patches/series
665+++ b/debian/patches/series
666@@ -62,3 +62,8 @@ ask-password-prevent-buffer-overrow-when-reading-fro.patch
667 rdrand-workaround-on-amd.patch
668 Skip-falling-back-to-device-name-when-net_get_name-device.patch
669 test-execute-Filter-dev-.lxc-in-exec-dynamicuser-statedir.patch
670+lp1815101-01-networkd-add-support-to-keep-configuration.patch
671+lp1815101-02-networkd-stop-clients-when-networkd-shuts-down.patch
672+lp1815101-03-network-add-KeepConfiguration-dhcp-on-stop.patch
673+lp1815101-04-network-make-KeepConfiguration-static-drop-DHCP-addr.patch
674+lp1815101-05-man-add-documentation-about-KeepConfiguration.patch

Subscribers

People subscribed via source and target branches