Merge ~ubuntu-core-dev/ubuntu/+source/systemd:lp1664844-hirsute into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-hirsute
- Git
- lp:~ubuntu-core-dev/ubuntu/+source/systemd
- lp1664844-hirsute
- Merge into ubuntu-hirsute
Proposed by
Łukasz Zemczak
Status: | Merged |
---|---|
Merged at revision: | 85b4a5ba7086f93859e13d7b025212c383043b67 |
Proposed branch: | ~ubuntu-core-dev/ubuntu/+source/systemd:lp1664844-hirsute |
Merge into: | ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-hirsute |
Diff against target: |
622 lines (+588/-0) 5 files modified
debian/changelog (+10/-0) debian/patches/lp1664844/0001-network-add-ActivationPolicy-configuration-parameter.patch (+344/-0) debian/patches/lp1664844/0002-test-add-ActivationPolicy-unit-tests.patch (+121/-0) debian/patches/lp1664844/0003-save-link-activation-policy-to-state-file-and-displa.patch (+110/-0) debian/patches/series (+3/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Dan Streetman | Approve | ||
Balint Reczey | Approve | ||
Dimitri John Ledkov | Pending | ||
Review via email: mp+402646@code.launchpad.net |
Commit message
add support for configuring the activation policy for an interface
Description of the change
add support for configuring the activation policy for an interface
To post a comment you must log in.
Revision history for this message
Balint Reczey (rbalint) wrote : | # |
Also please don't use this central repository for bugfix branches, it is big enough with the release branches already.
Revision history for this message
Łukasz Zemczak (sil2100) wrote : | # |
Did a quick sanity test with a dummy interface on multipass using test packages from a bileto PPA (https:/
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/debian/changelog b/debian/changelog | |||
2 | index 5efa976..161740e 100644 | |||
3 | --- a/debian/changelog | |||
4 | +++ b/debian/changelog | |||
5 | @@ -1,3 +1,13 @@ | |||
6 | 1 | systemd (247.3-3ubuntu4) UNRELEASED; urgency=medium | ||
7 | 2 | |||
8 | 3 | * d/p/lp1664844/0001-network-add-ActivationPolicy-configuration-parameter.patch, | ||
9 | 4 | d/p/lp1664844/0002-test-add-ActivationPolicy-unit-tests.patch, | ||
10 | 5 | d/p/lp1664844/0003-save-link-activation-policy-to-state-file-and-displa.patch: | ||
11 | 6 | - add support for configuring the activation policy for an interface | ||
12 | 7 | (LP: #1664844) | ||
13 | 8 | |||
14 | 9 | -- Łukasz 'sil2100' Zemczak <lukasz.zemczak@ubuntu.com> Wed, 12 May 2021 11:56:50 +0200 | ||
15 | 10 | |||
16 | 1 | systemd (247.3-3ubuntu3) hirsute; urgency=medium | 11 | systemd (247.3-3ubuntu3) hirsute; urgency=medium |
17 | 2 | 12 | ||
18 | 3 | * Make systemd-rfkill work with latest Linux kernels (LP: #1921696) | 13 | * Make systemd-rfkill work with latest Linux kernels (LP: #1921696) |
19 | diff --git a/debian/patches/lp1664844/0001-network-add-ActivationPolicy-configuration-parameter.patch b/debian/patches/lp1664844/0001-network-add-ActivationPolicy-configuration-parameter.patch | |||
20 | 4 | new file mode 100644 | 14 | new file mode 100644 |
21 | index 0000000..08a8fb5 | |||
22 | --- /dev/null | |||
23 | +++ b/debian/patches/lp1664844/0001-network-add-ActivationPolicy-configuration-parameter.patch | |||
24 | @@ -0,0 +1,344 @@ | |||
25 | 1 | From 61135582e0b2e847e49c96af05e4d101323ce00c Mon Sep 17 00:00:00 2001 | ||
26 | 2 | From: Dan Streetman <ddstreet@canonical.com> | ||
27 | 3 | Date: Thu, 18 Jun 2020 16:09:40 -0400 | ||
28 | 4 | Subject: [PATCH 1/3] network: add ActivationPolicy= configuration parameter | ||
29 | 5 | Origin: upstream, https://github.com/systemd/systemd/pull/16228 | ||
30 | 6 | Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1664844 | ||
31 | 7 | |||
32 | 8 | This parameter allows configuring the activation policy for an interface, | ||
33 | 9 | meaning how it manages the interface's administrative state (IFF_UP flag). | ||
34 | 10 | The policy can be configured to bring the interface either up or down when | ||
35 | 11 | the interface is (re)configured, to always force the interface either up or | ||
36 | 12 | down, or to never change the interface administrative state. | ||
37 | 13 | |||
38 | 14 | If the interface is bound with BindCarrier=, its administrative state is | ||
39 | 15 | controlled by the interface(s) it's bound to, and this parameter is forced | ||
40 | 16 | to 'bound'. | ||
41 | 17 | |||
42 | 18 | This changes the default behavior of how systemd-networkd sets the IFF_UP | ||
43 | 19 | flag; previously, it was set up (if not already up) every time the | ||
44 | 20 | link_joined() function was called. Now, with the default ActivationPolicy= | ||
45 | 21 | setting of 'up', it will only set the IFF_UP flag once, the first time | ||
46 | 22 | link_joined() is called, during an interface's configuration; and on | ||
47 | 23 | the first link_joined() call each time the interface is reconfigured. | ||
48 | 24 | |||
49 | 25 | Fixes: #3031 | ||
50 | 26 | Fixes: #17437 | ||
51 | 27 | --- | ||
52 | 28 | man/systemd.network.xml | 39 ++++++++++++- | ||
53 | 29 | src/network/networkd-link.c | 58 ++++++++++++++++++- | ||
54 | 30 | src/network/networkd-link.h | 1 + | ||
55 | 31 | src/network/networkd-network-gperf.gperf | 1 + | ||
56 | 32 | src/network/networkd-network.c | 40 ++++++++++++- | ||
57 | 33 | src/network/networkd-network.h | 16 +++++ | ||
58 | 34 | .../fuzz-network-parser/directives.network | 1 + | ||
59 | 35 | 7 files changed, 148 insertions(+), 8 deletions(-) | ||
60 | 36 | |||
61 | 37 | --- a/man/systemd.network.xml | ||
62 | 38 | +++ b/man/systemd.network.xml | ||
63 | 39 | @@ -228,6 +228,36 @@ | ||
64 | 40 | if <literal>RequiredForOnline=no</literal>.</para> | ||
65 | 41 | </listitem> | ||
66 | 42 | </varlistentry> | ||
67 | 43 | + <varlistentry> | ||
68 | 44 | + <term><varname>ActivationPolicy=</varname></term> | ||
69 | 45 | + <listitem> | ||
70 | 46 | + <para>Specifies the policy for <command>systemd-networkd</command> managing the link | ||
71 | 47 | + administrative state. Specifically, this controls how <command>systemd-networkd</command> | ||
72 | 48 | + changes the network device's <literal>IFF_UP</literal> flag, which is sometimes | ||
73 | 49 | + controlled by system administrators by running e.g., <command>ip set dev eth0 up</command> | ||
74 | 50 | + or <command>ip set dev eth0 down</command>, and can also be changed with | ||
75 | 51 | + <command>networkctl up eth0</command> or <command>networkctl down eth0</command>.</para> | ||
76 | 52 | + | ||
77 | 53 | + <para>Takes one of <literal>up</literal>, <literal>always-up</literal>, | ||
78 | 54 | + <literal>manual</literal>, <literal>always-down</literal>, <literal>down</literal>, | ||
79 | 55 | + or <literal>bound</literal>. When <literal>manual</literal>, <command>systemd-networkd</command> | ||
80 | 56 | + will not change the link's admin state automatically; the system administrator must bring the | ||
81 | 57 | + interface up or down manually, as desired. When <literal>up</literal> (the default) or | ||
82 | 58 | + <literal>always-up</literal>, or <literal>down</literal> or <literal>always-down</literal>, | ||
83 | 59 | + <command>systemd-networkd</command> will set the link up or down, respectively, | ||
84 | 60 | + when the interface is (re)configured. When <literal>always-up</literal> or | ||
85 | 61 | + <literal>always-down</literal>, <command>systemd-networkd</command> will set the link up | ||
86 | 62 | + or down, respectively, any time <command>systemd-networkd</command> detects a change in | ||
87 | 63 | + the administrative state. When <varname>BindCarrier=</varname> is also set, this is | ||
88 | 64 | + automatically set to <literal>bound</literal> and any other value is ignored.</para> | ||
89 | 65 | + | ||
90 | 66 | + <para>The administrative state is not the same as the carrier state, so using | ||
91 | 67 | + <literal>always-up</literal> does not mean the link will never lose carrier. The link | ||
92 | 68 | + carrier depends on both the administrative state as well as the network device's physical | ||
93 | 69 | + connection. However, to avoid reconfiguration failures, when using <literal>always-up</literal>, | ||
94 | 70 | + <varname>IgnoreCarrierLoss=</varname> is forced to true.</para> | ||
95 | 71 | + </listitem> | ||
96 | 72 | + </varlistentry> | ||
97 | 73 | </variablelist> | ||
98 | 74 | </refsect1> | ||
99 | 75 | |||
100 | 76 | @@ -573,8 +603,9 @@ | ||
101 | 77 | <listitem> | ||
102 | 78 | <para>A link name or a list of link names. When set, controls the behavior of the current | ||
103 | 79 | link. When all links in the list are in an operational down state, the current link is brought | ||
104 | 80 | - down. When at least one link has carrier, the current interface is brought up. | ||
105 | 81 | - </para> | ||
106 | 82 | + down. When at least one link has carrier, the current interface is brought up.</para> | ||
107 | 83 | + | ||
108 | 84 | + <para>This forces <varname>ActivationPolicy=</varname> to be set to <literal>bound</literal>.</para> | ||
109 | 85 | </listitem> | ||
110 | 86 | </varlistentry> | ||
111 | 87 | <varlistentry> | ||
112 | 88 | @@ -947,6 +978,10 @@ | ||
113 | 89 | of the interface even if its carrier is lost. When unset, the value specified with | ||
114 | 90 | <option>ConfigureWithoutCarrier=</option> is used. | ||
115 | 91 | </para> | ||
116 | 92 | + | ||
117 | 93 | + <para>When <varname>ActivationPolicy=</varname> is set to <literal>always-up</literal>, this | ||
118 | 94 | + is forced to <literal>true</literal>. | ||
119 | 95 | + </para> | ||
120 | 96 | </listitem> | ||
121 | 97 | </varlistentry> | ||
122 | 98 | <varlistentry> | ||
123 | 99 | --- a/src/network/networkd-link.c | ||
124 | 100 | +++ b/src/network/networkd-link.c | ||
125 | 101 | @@ -1812,17 +1812,38 @@ | ||
126 | 102 | assert(link); | ||
127 | 103 | assert(link->network); | ||
128 | 104 | |||
129 | 105 | - if (!hashmap_isempty(link->bound_to_links)) { | ||
130 | 106 | + switch (link->network->activation_policy) { | ||
131 | 107 | + case ACTIVATION_POLICY_BOUND: | ||
132 | 108 | r = link_handle_bound_to_list(link); | ||
133 | 109 | if (r < 0) | ||
134 | 110 | return r; | ||
135 | 111 | - } else if (!(link->flags & IFF_UP)) { | ||
136 | 112 | + break; | ||
137 | 113 | + case ACTIVATION_POLICY_UP: | ||
138 | 114 | + if (link->activated) | ||
139 | 115 | + break; | ||
140 | 116 | + _fallthrough_; | ||
141 | 117 | + case ACTIVATION_POLICY_ALWAYS_UP: | ||
142 | 118 | r = link_up(link); | ||
143 | 119 | if (r < 0) { | ||
144 | 120 | link_enter_failed(link); | ||
145 | 121 | return r; | ||
146 | 122 | } | ||
147 | 123 | + break; | ||
148 | 124 | + case ACTIVATION_POLICY_DOWN: | ||
149 | 125 | + if (link->activated) | ||
150 | 126 | + break; | ||
151 | 127 | + _fallthrough_; | ||
152 | 128 | + case ACTIVATION_POLICY_ALWAYS_DOWN: | ||
153 | 129 | + r = link_down(link, NULL); | ||
154 | 130 | + if (r < 0) { | ||
155 | 131 | + link_enter_failed(link); | ||
156 | 132 | + return r; | ||
157 | 133 | + } | ||
158 | 134 | + break; | ||
159 | 135 | + default: | ||
160 | 136 | + break; | ||
161 | 137 | } | ||
162 | 138 | + link->activated = true; | ||
163 | 139 | |||
164 | 140 | if (link->network->bridge) { | ||
165 | 141 | r = link_set_bridge(link); | ||
166 | 142 | @@ -2216,6 +2237,7 @@ | ||
167 | 143 | return r; | ||
168 | 144 | |||
169 | 145 | link_set_state(link, LINK_STATE_INITIALIZED); | ||
170 | 146 | + link->activated = false; | ||
171 | 147 | link_dirty(link); | ||
172 | 148 | |||
173 | 149 | /* link_configure_duid() returns 0 if it requests product UUID. In that case, | ||
174 | 150 | @@ -2687,6 +2709,16 @@ | ||
175 | 151 | static int link_admin_state_up(Link *link) { | ||
176 | 152 | int r; | ||
177 | 153 | |||
178 | 154 | + assert(link); | ||
179 | 155 | + | ||
180 | 156 | + if (!link->network) | ||
181 | 157 | + return 0; | ||
182 | 158 | + | ||
183 | 159 | + if (link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_DOWN) { | ||
184 | 160 | + log_link_info(link, "ActivationPolicy is \"always-off\", forcing link down"); | ||
185 | 161 | + return link_down(link, NULL); | ||
186 | 162 | + } | ||
187 | 163 | + | ||
188 | 164 | /* We set the ipv6 mtu after the device mtu, but the kernel resets | ||
189 | 165 | * ipv6 mtu on NETDEV_UP, so we need to reset it. The check for | ||
190 | 166 | * ipv6_mtu_set prevents this from trying to set it too early before | ||
191 | 167 | @@ -2701,6 +2733,21 @@ | ||
192 | 168 | return 0; | ||
193 | 169 | } | ||
194 | 170 | |||
195 | 171 | +static int link_admin_state_down(Link *link) { | ||
196 | 172 | + | ||
197 | 173 | + assert(link); | ||
198 | 174 | + | ||
199 | 175 | + if (!link->network) | ||
200 | 176 | + return 0; | ||
201 | 177 | + | ||
202 | 178 | + if (link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_UP) { | ||
203 | 179 | + log_link_info(link, "ActivationPolicy is \"always-on\", forcing link up"); | ||
204 | 180 | + return link_up(link); | ||
205 | 181 | + } | ||
206 | 182 | + | ||
207 | 183 | + return 0; | ||
208 | 184 | +} | ||
209 | 185 | + | ||
210 | 186 | int link_update(Link *link, sd_netlink_message *m) { | ||
211 | 187 | _cleanup_strv_free_ char **s = NULL; | ||
212 | 188 | hw_addr_data hw_addr; | ||
213 | 189 | @@ -2813,9 +2860,14 @@ | ||
214 | 190 | r = link_admin_state_up(link); | ||
215 | 191 | if (r < 0) | ||
216 | 192 | return r; | ||
217 | 193 | - } else if (link_was_admin_up && !(link->flags & IFF_UP)) | ||
218 | 194 | + } else if (link_was_admin_up && !(link->flags & IFF_UP)) { | ||
219 | 195 | log_link_info(link, "Link DOWN"); | ||
220 | 196 | |||
221 | 197 | + r = link_admin_state_down(link); | ||
222 | 198 | + if (r < 0) | ||
223 | 199 | + return r; | ||
224 | 200 | + } | ||
225 | 201 | + | ||
226 | 202 | r = link_update_lldp(link); | ||
227 | 203 | if (r < 0) | ||
228 | 204 | return r; | ||
229 | 205 | --- a/src/network/networkd-link.h | ||
230 | 206 | +++ b/src/network/networkd-link.h | ||
231 | 207 | @@ -130,6 +130,7 @@ | ||
232 | 208 | bool setting_genmode:1; | ||
233 | 209 | bool ipv6_mtu_set:1; | ||
234 | 210 | bool bridge_mdb_configured:1; | ||
235 | 211 | + bool activated:1; | ||
236 | 212 | |||
237 | 213 | sd_dhcp_server *dhcp_server; | ||
238 | 214 | |||
239 | 215 | --- a/src/network/networkd-network-gperf.gperf | ||
240 | 216 | +++ b/src/network/networkd-network-gperf.gperf | ||
241 | 217 | @@ -63,6 +63,7 @@ | ||
242 | 218 | Link.Multicast, config_parse_tristate, 0, offsetof(Network, multicast) | ||
243 | 219 | Link.AllMulticast, config_parse_tristate, 0, offsetof(Network, allmulticast) | ||
244 | 220 | Link.Unmanaged, config_parse_bool, 0, offsetof(Network, unmanaged) | ||
245 | 221 | +Link.ActivationPolicy, config_parse_activation_policy, 0, offsetof(Network, activation_policy) | ||
246 | 222 | Link.RequiredForOnline, config_parse_required_for_online, 0, 0 | ||
247 | 223 | SR-IOV.VirtualFunction, config_parse_sr_iov_uint32, 0, 0 | ||
248 | 224 | SR-IOV.VLANId, config_parse_sr_iov_uint32, 0, 0 | ||
249 | 225 | --- a/src/network/networkd-network.c | ||
250 | 226 | +++ b/src/network/networkd-network.c | ||
251 | 227 | @@ -238,9 +238,6 @@ | ||
252 | 228 | if (network->dhcp_use_gateway < 0) | ||
253 | 229 | network->dhcp_use_gateway = network->dhcp_use_routes; | ||
254 | 230 | |||
255 | 231 | - if (network->ignore_carrier_loss < 0) | ||
256 | 232 | - network->ignore_carrier_loss = network->configure_without_carrier; | ||
257 | 233 | - | ||
258 | 234 | if (network->dhcp_critical >= 0) { | ||
259 | 235 | if (network->keep_configuration >= 0) | ||
260 | 236 | log_warning("%s: Both KeepConfiguration= and deprecated CriticalConnection= are set. " | ||
261 | 237 | @@ -252,6 +249,30 @@ | ||
262 | 238 | network->keep_configuration = KEEP_CONFIGURATION_NO; | ||
263 | 239 | } | ||
264 | 240 | |||
265 | 241 | + if (!strv_isempty(network->bind_carrier)) { | ||
266 | 242 | + if (!IN_SET(network->activation_policy, _ACTIVATION_POLICY_INVALID, ACTIVATION_POLICY_BOUND)) | ||
267 | 243 | + log_warning("%s: ActivationPolicy=bound is required with BindCarrier=. " | ||
268 | 244 | + "Setting ActivationPolicy=bound.", network->filename); | ||
269 | 245 | + network->activation_policy = ACTIVATION_POLICY_BOUND; | ||
270 | 246 | + } else if (network->activation_policy == ACTIVATION_POLICY_BOUND) { | ||
271 | 247 | + log_warning("%s: ActivationPolicy=bound requires BindCarrier=. " | ||
272 | 248 | + "Ignoring ActivationPolicy=bound.", network->filename); | ||
273 | 249 | + network->activation_policy = ACTIVATION_POLICY_UP; | ||
274 | 250 | + } | ||
275 | 251 | + | ||
276 | 252 | + if (network->activation_policy == _ACTIVATION_POLICY_INVALID) | ||
277 | 253 | + network->activation_policy = ACTIVATION_POLICY_UP; | ||
278 | 254 | + | ||
279 | 255 | + if (network->activation_policy == ACTIVATION_POLICY_ALWAYS_UP) { | ||
280 | 256 | + if (network->ignore_carrier_loss == false) | ||
281 | 257 | + log_warning("%s: IgnoreCarrierLoss=false conflicts with ActivationPolicy=always-up. " | ||
282 | 258 | + "Setting IgnoreCarrierLoss=true.", network->filename); | ||
283 | 259 | + network->ignore_carrier_loss = true; | ||
284 | 260 | + } | ||
285 | 261 | + | ||
286 | 262 | + if (network->ignore_carrier_loss < 0) | ||
287 | 263 | + network->ignore_carrier_loss = network->configure_without_carrier; | ||
288 | 264 | + | ||
289 | 265 | if (network->keep_configuration < 0) | ||
290 | 266 | network->keep_configuration = KEEP_CONFIGURATION_NO; | ||
291 | 267 | |||
292 | 268 | @@ -329,6 +350,7 @@ | ||
293 | 269 | |||
294 | 270 | .required_for_online = true, | ||
295 | 271 | .required_operstate_for_online = LINK_OPERSTATE_RANGE_DEFAULT, | ||
296 | 272 | + .activation_policy = _ACTIVATION_POLICY_INVALID, | ||
297 | 273 | .arp = -1, | ||
298 | 274 | .multicast = -1, | ||
299 | 275 | .allmulticast = -1, | ||
300 | 276 | @@ -1238,3 +1260,15 @@ | ||
301 | 277 | |||
302 | 278 | DEFINE_STRING_TABLE_LOOKUP(ipv6_link_local_address_gen_mode, IPv6LinkLocalAddressGenMode); | ||
303 | 279 | DEFINE_CONFIG_PARSE_ENUM(config_parse_ipv6_link_local_address_gen_mode, ipv6_link_local_address_gen_mode, IPv6LinkLocalAddressGenMode, "Failed to parse IPv6 link local address generation mode"); | ||
304 | 280 | + | ||
305 | 281 | +static const char* const activation_policy_table[_ACTIVATION_POLICY_MAX] = { | ||
306 | 282 | + [ACTIVATION_POLICY_UP] = "up", | ||
307 | 283 | + [ACTIVATION_POLICY_ALWAYS_UP] = "always-up", | ||
308 | 284 | + [ACTIVATION_POLICY_MANUAL] = "manual", | ||
309 | 285 | + [ACTIVATION_POLICY_ALWAYS_DOWN] = "always-down", | ||
310 | 286 | + [ACTIVATION_POLICY_DOWN] = "down", | ||
311 | 287 | + [ACTIVATION_POLICY_BOUND] = "bound", | ||
312 | 288 | +}; | ||
313 | 289 | + | ||
314 | 290 | +DEFINE_STRING_TABLE_LOOKUP(activation_policy, ActivationPolicy); | ||
315 | 291 | +DEFINE_CONFIG_PARSE_ENUM(config_parse_activation_policy, activation_policy, ActivationPolicy, "Failed to parse activation policy"); | ||
316 | 292 | --- a/src/network/networkd-network.h | ||
317 | 293 | +++ b/src/network/networkd-network.h | ||
318 | 294 | @@ -46,6 +46,17 @@ | ||
319 | 295 | _IPV6_LINK_LOCAL_ADDRESS_GEN_MODE_INVALID = -1 | ||
320 | 296 | } IPv6LinkLocalAddressGenMode; | ||
321 | 297 | |||
322 | 298 | +typedef enum ActivationPolicy { | ||
323 | 299 | + ACTIVATION_POLICY_UP, | ||
324 | 300 | + ACTIVATION_POLICY_ALWAYS_UP, | ||
325 | 301 | + ACTIVATION_POLICY_MANUAL, | ||
326 | 302 | + ACTIVATION_POLICY_ALWAYS_DOWN, | ||
327 | 303 | + ACTIVATION_POLICY_DOWN, | ||
328 | 304 | + ACTIVATION_POLICY_BOUND, | ||
329 | 305 | + _ACTIVATION_POLICY_MAX, | ||
330 | 306 | + _ACTIVATION_POLICY_INVALID = -1 | ||
331 | 307 | +} ActivationPolicy; | ||
332 | 308 | + | ||
333 | 309 | typedef struct Manager Manager; | ||
334 | 310 | |||
335 | 311 | typedef struct NetworkDHCPServerEmitAddress { | ||
336 | 312 | @@ -98,6 +109,7 @@ | ||
337 | 313 | bool unmanaged; | ||
338 | 314 | bool required_for_online; /* Is this network required to be considered online? */ | ||
339 | 315 | LinkOperationalStateRange required_operstate_for_online; | ||
340 | 316 | + ActivationPolicy activation_policy; | ||
341 | 317 | |||
342 | 318 | /* misc settings */ | ||
343 | 319 | bool configure_without_carrier; | ||
344 | 320 | @@ -330,6 +342,7 @@ | ||
345 | 321 | CONFIG_PARSER_PROTOTYPE(config_parse_required_for_online); | ||
346 | 322 | CONFIG_PARSER_PROTOTYPE(config_parse_keep_configuration); | ||
347 | 323 | CONFIG_PARSER_PROTOTYPE(config_parse_ipv6_link_local_address_gen_mode); | ||
348 | 324 | +CONFIG_PARSER_PROTOTYPE(config_parse_activation_policy); | ||
349 | 325 | |||
350 | 326 | const struct ConfigPerfItem* network_network_gperf_lookup(const char *key, GPERF_LEN_TYPE length); | ||
351 | 327 | |||
352 | 328 | @@ -338,3 +351,6 @@ | ||
353 | 329 | |||
354 | 330 | const char* ipv6_link_local_address_gen_mode_to_string(IPv6LinkLocalAddressGenMode s) _const_; | ||
355 | 331 | IPv6LinkLocalAddressGenMode ipv6_link_local_address_gen_mode_from_string(const char *s) _pure_; | ||
356 | 332 | + | ||
357 | 333 | +const char* activation_policy_to_string(ActivationPolicy i) _const_; | ||
358 | 334 | +ActivationPolicy activation_policy_from_string(const char *s) _pure_; | ||
359 | 335 | --- a/test/fuzz/fuzz-network-parser/directives.network | ||
360 | 336 | +++ b/test/fuzz/fuzz-network-parser/directives.network | ||
361 | 337 | @@ -30,6 +30,7 @@ | ||
362 | 338 | MACAddress= | ||
363 | 339 | PermanentMACAddress= | ||
364 | 340 | [Link] | ||
365 | 341 | +ActivationPolicy= | ||
366 | 342 | RequiredForOnline= | ||
367 | 343 | ARP= | ||
368 | 344 | AllMulticast= | ||
369 | diff --git a/debian/patches/lp1664844/0002-test-add-ActivationPolicy-unit-tests.patch b/debian/patches/lp1664844/0002-test-add-ActivationPolicy-unit-tests.patch | |||
370 | 0 | new file mode 100644 | 345 | new file mode 100644 |
371 | index 0000000..80a31ef | |||
372 | --- /dev/null | |||
373 | +++ b/debian/patches/lp1664844/0002-test-add-ActivationPolicy-unit-tests.patch | |||
374 | @@ -0,0 +1,121 @@ | |||
375 | 1 | From 2236d75df955118ad5d84c5ab787484c0921dfda Mon Sep 17 00:00:00 2001 | ||
376 | 2 | From: Dan Streetman <ddstreet@canonical.com> | ||
377 | 3 | Date: Thu, 18 Jun 2020 18:31:18 -0400 | ||
378 | 4 | Subject: [PATCH 2/3] test: add ActivationPolicy= unit tests | ||
379 | 5 | Origin: upstream, https://github.com/systemd/systemd/pull/16228 | ||
380 | 6 | Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1664844 | ||
381 | 7 | |||
382 | 8 | --- | ||
383 | 9 | .../conf/25-activation-policy.network | 6 +++ | ||
384 | 10 | .../always-down.conf | 2 + | ||
385 | 11 | .../always-up.conf | 2 + | ||
386 | 12 | .../25-activation-policy.network.d/down.conf | 2 + | ||
387 | 13 | .../manual.conf | 2 + | ||
388 | 14 | .../25-activation-policy.network.d/up.conf | 2 + | ||
389 | 15 | test/test-network/systemd-networkd-tests.py | 48 +++++++++++++++++++ | ||
390 | 16 | 7 files changed, 64 insertions(+) | ||
391 | 17 | create mode 100644 test/test-network/conf/25-activation-policy.network | ||
392 | 18 | create mode 100644 test/test-network/conf/25-activation-policy.network.d/always-down.conf | ||
393 | 19 | create mode 100644 test/test-network/conf/25-activation-policy.network.d/always-up.conf | ||
394 | 20 | create mode 100644 test/test-network/conf/25-activation-policy.network.d/down.conf | ||
395 | 21 | create mode 100644 test/test-network/conf/25-activation-policy.network.d/manual.conf | ||
396 | 22 | create mode 100644 test/test-network/conf/25-activation-policy.network.d/up.conf | ||
397 | 23 | |||
398 | 24 | --- /dev/null | ||
399 | 25 | +++ b/test/test-network/conf/25-activation-policy.network | ||
400 | 26 | @@ -0,0 +1,6 @@ | ||
401 | 27 | +[Match] | ||
402 | 28 | +Name=test1 | ||
403 | 29 | + | ||
404 | 30 | +[Network] | ||
405 | 31 | +Address=192.168.10.30/24 | ||
406 | 32 | +Gateway=192.168.10.1 | ||
407 | 33 | --- /dev/null | ||
408 | 34 | +++ b/test/test-network/conf/25-activation-policy.network.d/always-down.conf | ||
409 | 35 | @@ -0,0 +1,2 @@ | ||
410 | 36 | +[Link] | ||
411 | 37 | +ActivationPolicy=always-down | ||
412 | 38 | --- /dev/null | ||
413 | 39 | +++ b/test/test-network/conf/25-activation-policy.network.d/always-up.conf | ||
414 | 40 | @@ -0,0 +1,2 @@ | ||
415 | 41 | +[Link] | ||
416 | 42 | +ActivationPolicy=always-up | ||
417 | 43 | --- /dev/null | ||
418 | 44 | +++ b/test/test-network/conf/25-activation-policy.network.d/down.conf | ||
419 | 45 | @@ -0,0 +1,2 @@ | ||
420 | 46 | +[Link] | ||
421 | 47 | +ActivationPolicy=down | ||
422 | 48 | --- /dev/null | ||
423 | 49 | +++ b/test/test-network/conf/25-activation-policy.network.d/manual.conf | ||
424 | 50 | @@ -0,0 +1,2 @@ | ||
425 | 51 | +[Link] | ||
426 | 52 | +ActivationPolicy=manual | ||
427 | 53 | --- /dev/null | ||
428 | 54 | +++ b/test/test-network/conf/25-activation-policy.network.d/up.conf | ||
429 | 55 | @@ -0,0 +1,2 @@ | ||
430 | 56 | +[Link] | ||
431 | 57 | +ActivationPolicy=up | ||
432 | 58 | --- a/test/test-network/systemd-networkd-tests.py | ||
433 | 59 | +++ b/test/test-network/systemd-networkd-tests.py | ||
434 | 60 | @@ -1745,6 +1745,7 @@ | ||
435 | 61 | '25-address-peer-ipv4.network', | ||
436 | 62 | '25-address-preferred-lifetime-zero.network', | ||
437 | 63 | '25-address-static.network', | ||
438 | 64 | + '25-activation-policy.network', | ||
439 | 65 | '25-bind-carrier.network', | ||
440 | 66 | '25-bond-active-backup-slave.netdev', | ||
441 | 67 | '25-fibrule-invert.network', | ||
442 | 68 | @@ -2609,6 +2610,53 @@ | ||
443 | 69 | self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1') | ||
444 | 70 | self.wait_operstate('test1', 'routable') | ||
445 | 71 | |||
446 | 72 | + def _test_activation_policy(self, test): | ||
447 | 73 | + self.setUp() | ||
448 | 74 | + conffile = '25-activation-policy.network' | ||
449 | 75 | + if test: | ||
450 | 76 | + conffile = f'{conffile}.d/{test}.conf' | ||
451 | 77 | + copy_unit_to_networkd_unit_path('11-dummy.netdev', conffile, dropins=False) | ||
452 | 78 | + start_networkd() | ||
453 | 79 | + | ||
454 | 80 | + always = test.startswith('always') | ||
455 | 81 | + if test == 'manual': | ||
456 | 82 | + initial_up = 'UP' in check_output('ip link show test1') | ||
457 | 83 | + else: | ||
458 | 84 | + initial_up = not test.endswith('down') # note: default is up | ||
459 | 85 | + expect_up = initial_up | ||
460 | 86 | + next_up = not expect_up | ||
461 | 87 | + | ||
462 | 88 | + # if initial expected state is down, must wait for setup_state to reach configuring | ||
463 | 89 | + # so systemd-networkd considers it 'activated' | ||
464 | 90 | + setup_state = None if initial_up else 'configuring' | ||
465 | 91 | + | ||
466 | 92 | + for iteration in range(4): | ||
467 | 93 | + with self.subTest(iteration=iteration, expect_up=expect_up): | ||
468 | 94 | + operstate = 'routable' if expect_up else 'off' | ||
469 | 95 | + self.wait_operstate('test1', operstate, setup_state=setup_state, setup_timeout=20) | ||
470 | 96 | + setup_state = None | ||
471 | 97 | + | ||
472 | 98 | + if expect_up: | ||
473 | 99 | + self.assertIn('UP', check_output('ip link show test1')) | ||
474 | 100 | + self.assertIn('192.168.10.30/24', check_output('ip address show test1')) | ||
475 | 101 | + self.assertIn('default via 192.168.10.1', check_output('ip route show')) | ||
476 | 102 | + else: | ||
477 | 103 | + self.assertIn('DOWN', check_output('ip link show test1')) | ||
478 | 104 | + | ||
479 | 105 | + if next_up: | ||
480 | 106 | + check_output('ip link set dev test1 up') | ||
481 | 107 | + else: | ||
482 | 108 | + check_output('ip link set dev test1 down') | ||
483 | 109 | + expect_up = initial_up if always else next_up | ||
484 | 110 | + next_up = not next_up | ||
485 | 111 | + | ||
486 | 112 | + self.tearDown() | ||
487 | 113 | + | ||
488 | 114 | + def test_activation_policy(self): | ||
489 | 115 | + for test in ['up', 'always-up', 'manual', 'always-down', 'down', '']: | ||
490 | 116 | + with self.subTest(test=test): | ||
491 | 117 | + self._test_activation_policy(test) | ||
492 | 118 | + | ||
493 | 119 | def test_domain(self): | ||
494 | 120 | copy_unit_to_networkd_unit_path('12-dummy.netdev', '24-search-domain.network') | ||
495 | 121 | start_networkd() | ||
496 | diff --git a/debian/patches/lp1664844/0003-save-link-activation-policy-to-state-file-and-displa.patch b/debian/patches/lp1664844/0003-save-link-activation-policy-to-state-file-and-displa.patch | |||
497 | 0 | new file mode 100644 | 122 | new file mode 100644 |
498 | index 0000000..be89aae | |||
499 | --- /dev/null | |||
500 | +++ b/debian/patches/lp1664844/0003-save-link-activation-policy-to-state-file-and-displa.patch | |||
501 | @@ -0,0 +1,110 @@ | |||
502 | 1 | From a853652ae983699460b160bc2bf72f6fae0bfcd6 Mon Sep 17 00:00:00 2001 | ||
503 | 2 | From: Dan Streetman <ddstreet@canonical.com> | ||
504 | 3 | Date: Thu, 13 Aug 2020 11:52:53 -0400 | ||
505 | 4 | Subject: [PATCH 3/3] save link activation policy to state file and display in | ||
506 | 5 | networkctl | ||
507 | 6 | Origin: upstream, https://github.com/systemd/systemd/pull/16228 | ||
508 | 7 | Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1664844 | ||
509 | 8 | |||
510 | 9 | --- | ||
511 | 10 | src/libsystemd/sd-network/sd-network.c | 21 +++++++++++++++++++++ | ||
512 | 11 | src/network/networkctl.c | 12 +++++++++++- | ||
513 | 12 | src/network/networkd-link.c | 3 +++ | ||
514 | 13 | src/systemd/sd-network.h | 5 +++++ | ||
515 | 14 | test/test-network/systemd-networkd-tests.py | 1 + | ||
516 | 15 | 5 files changed, 41 insertions(+), 1 deletion(-) | ||
517 | 16 | |||
518 | 17 | --- a/src/libsystemd/sd-network/sd-network.c | ||
519 | 18 | +++ b/src/libsystemd/sd-network/sd-network.c | ||
520 | 19 | @@ -212,6 +212,27 @@ | ||
521 | 20 | return 0; | ||
522 | 21 | } | ||
523 | 22 | |||
524 | 23 | +_public_ int sd_network_link_get_activation_policy(int ifindex, char **policy) { | ||
525 | 24 | + _cleanup_free_ char *s = NULL; | ||
526 | 25 | + int r; | ||
527 | 26 | + | ||
528 | 27 | + assert_return(policy, -EINVAL); | ||
529 | 28 | + | ||
530 | 29 | + r = network_link_get_string(ifindex, "ACTIVATION_POLICY", &s); | ||
531 | 30 | + if (r < 0) { | ||
532 | 31 | + if (r != -ENODATA) | ||
533 | 32 | + return r; | ||
534 | 33 | + | ||
535 | 34 | + /* For compatibility, assuming up. */ | ||
536 | 35 | + s = strdup("up"); | ||
537 | 36 | + if (!s) | ||
538 | 37 | + return -ENOMEM; | ||
539 | 38 | + } | ||
540 | 39 | + | ||
541 | 40 | + *policy = TAKE_PTR(s); | ||
542 | 41 | + return 0; | ||
543 | 42 | +} | ||
544 | 43 | + | ||
545 | 44 | _public_ int sd_network_link_get_llmnr(int ifindex, char **llmnr) { | ||
546 | 45 | return network_link_get_string(ifindex, "LLMNR", llmnr); | ||
547 | 46 | } | ||
548 | 47 | --- a/src/network/networkctl.c | ||
549 | 48 | +++ b/src/network/networkctl.c | ||
550 | 49 | @@ -1390,7 +1390,7 @@ | ||
551 | 50 | |||
552 | 51 | _cleanup_strv_free_ char **dns = NULL, **ntp = NULL, **sip = NULL, **search_domains = NULL, **route_domains = NULL; | ||
553 | 52 | _cleanup_free_ char *t = NULL, *network = NULL, *iaid = NULL, *duid = NULL, | ||
554 | 53 | - *setup_state = NULL, *operational_state = NULL, *lease_file = NULL; | ||
555 | 54 | + *setup_state = NULL, *operational_state = NULL, *lease_file = NULL, *activation_policy = NULL; | ||
556 | 55 | const char *driver = NULL, *path = NULL, *vendor = NULL, *model = NULL, *link = NULL, | ||
557 | 56 | *on_color_operational, *off_color_operational, *on_color_setup, *off_color_setup; | ||
558 | 57 | _cleanup_free_ int *carrier_bound_to = NULL, *carrier_bound_by = NULL; | ||
559 | 58 | @@ -2065,6 +2065,16 @@ | ||
560 | 59 | if (r < 0) | ||
561 | 60 | return r; | ||
562 | 61 | |||
563 | 62 | + r = sd_network_link_get_activation_policy(info->ifindex, &activation_policy); | ||
564 | 63 | + if (r >= 0) { | ||
565 | 64 | + r = table_add_many(table, | ||
566 | 65 | + TABLE_EMPTY, | ||
567 | 66 | + TABLE_STRING, "Activation Policy:", | ||
568 | 67 | + TABLE_STRING, activation_policy); | ||
569 | 68 | + if (r < 0) | ||
570 | 69 | + return table_log_add_error(r); | ||
571 | 70 | + } | ||
572 | 71 | + | ||
573 | 72 | if (lease) { | ||
574 | 73 | const void *client_id; | ||
575 | 74 | size_t client_id_len; | ||
576 | 75 | --- a/src/network/networkd-link.c | ||
577 | 76 | +++ b/src/network/networkd-link.c | ||
578 | 77 | @@ -3040,6 +3040,9 @@ | ||
579 | 78 | st.max != LINK_OPERSTATE_RANGE_DEFAULT.max ? ":" : "", | ||
580 | 79 | st.max != LINK_OPERSTATE_RANGE_DEFAULT.max ? strempty(link_operstate_to_string(st.max)) : ""); | ||
581 | 80 | |||
582 | 81 | + fprintf(f, "ACTIVATION_POLICY=%s\n", | ||
583 | 82 | + activation_policy_to_string(link->network->activation_policy)); | ||
584 | 83 | + | ||
585 | 84 | fprintf(f, "NETWORK_FILE=%s\n", link->network->filename); | ||
586 | 85 | |||
587 | 86 | /************************************************************/ | ||
588 | 87 | --- a/src/systemd/sd-network.h | ||
589 | 88 | +++ b/src/systemd/sd-network.h | ||
590 | 89 | @@ -103,6 +103,11 @@ | ||
591 | 90 | */ | ||
592 | 91 | int sd_network_link_get_required_for_online(int ifindex); | ||
593 | 92 | |||
594 | 93 | +/* Get activation policy for ifindex. | ||
595 | 94 | + * Possible values are as specified for ActivationPolicy= | ||
596 | 95 | + */ | ||
597 | 96 | +int sd_network_link_get_activation_policy(int ifindex, char **policy); | ||
598 | 97 | + | ||
599 | 98 | /* Get path to .network file applied to link */ | ||
600 | 99 | int sd_network_link_get_network_file(int ifindex, char **filename); | ||
601 | 100 | |||
602 | 101 | --- a/test/test-network/systemd-networkd-tests.py | ||
603 | 102 | +++ b/test/test-network/systemd-networkd-tests.py | ||
604 | 103 | @@ -2931,6 +2931,7 @@ | ||
605 | 104 | self.assertRegex(data, r'OPER_STATE=routable') | ||
606 | 105 | self.assertRegex(data, r'REQUIRED_FOR_ONLINE=yes') | ||
607 | 106 | self.assertRegex(data, r'REQUIRED_OPER_STATE_FOR_ONLINE=routable') | ||
608 | 107 | + self.assertRegex(data, r'ACTIVATION_POLICY=up') | ||
609 | 108 | self.assertRegex(data, r'NETWORK_FILE=/run/systemd/network/state-file-tests.network') | ||
610 | 109 | self.assertRegex(data, r'DNS=10.10.10.10#aaa.com 10.10.10.11:1111#bbb.com \[1111:2222::3333\]:1234#ccc.com') | ||
611 | 110 | self.assertRegex(data, r'NTP=0.fedora.pool.ntp.org 1.fedora.pool.ntp.org') | ||
612 | diff --git a/debian/patches/series b/debian/patches/series | |||
613 | index afeff62..b220c6e 100644 | |||
614 | --- a/debian/patches/series | |||
615 | +++ b/debian/patches/series | |||
616 | @@ -62,3 +62,6 @@ lp1887744-basic-unit-file-when-loading-linked-unit-files-use-l.patch | |||
617 | 62 | lp1921696/rfkill-improve-error-logging.patch | 62 | lp1921696/rfkill-improve-error-logging.patch |
618 | 63 | lp1921696/rfkill-use-short-writes-and-accept-long-reads.patch | 63 | lp1921696/rfkill-use-short-writes-and-accept-long-reads.patch |
619 | 64 | LoadCredentials-do-not-assert-on-invalid-syntax.patch | 64 | LoadCredentials-do-not-assert-on-invalid-syntax.patch |
620 | 65 | lp1664844/0001-network-add-ActivationPolicy-configuration-parameter.patch | ||
621 | 66 | lp1664844/0002-test-add-ActivationPolicy-unit-tests.patch | ||
622 | 67 | lp1664844/0003-save-link-activation-policy-to-state-file-and-displa.patch |
LGTM, but adding Dan if there is anything I missed.