From 7c644a696668abc4e77ddc6d3dd3036f6c773f9d Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Tue, 8 Jun 2021 12:01:31 -0400 Subject: [PATCH 1/3] network: default RequiredForOnline=false if ActivactionPolicy= not set to up If ActivationPolicy= is set to down, always-down, or manual, then any matching link will delay boot (due to delaying network-online.target). If RequiredForOnline= wasn't explicitly set, then default it to false if ActivationPolicy= is down or manual. If ActivationPolicy=always-down, then force RequiredForOnline=no. --- man/systemd.network.xml | 18 ++++++++++++++---- src/network/networkd-network.c | 19 +++++++++++++++++-- src/network/networkd-network.h | 2 +- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 3e85edec6d..282ce57a00 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -220,11 +220,16 @@ systemd-networkd-wait-online). When no, the network is ignored when determining the online state. When a minimum operational state and an optional maximum operational state are set, yes is implied, and this controls the minimum and maximum - operational state required for the network interface to be considered online. - Defaults to yes. + operational state required for the network interface to be considered online. - The network will be brought up normally in all cases, but in - the event that there is no address being assigned by DHCP or the + Defaults to yes when ActivationPolicy= is not set, + or set to up, always-up, or bound. + Defaults to no when ActivationPolicy= is set to + manual or down. This is forced to no + when ActivationPolicy= is set to always-down. + + The network will be brought up normally (as configured by ActivationPolicy=), + but in the event that there is no address being assigned by DHCP or the cable is not plugged in, the link will simply remain offline and be skipped automatically by systemd-networkd-wait-online if RequiredForOnline=no. @@ -265,6 +270,11 @@ the administrative state. When BindCarrier= is also set, this is automatically set to bound and any other value is ignored. + When the policy is set to down or manual, + the default value of RequiredForOnline= is no. + When the policy is set to always-down, the value of + RequiredForOnline= forced to no. + The administrative state is not the same as the carrier state, so using always-up does not mean the link will never lose carrier. The link carrier depends on both the administrative state as well as the network device's physical diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 011cb94462..850b4f449e 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -229,6 +229,21 @@ int network_verify(Network *network) { if (network->ignore_carrier_loss < 0) network->ignore_carrier_loss = network->configure_without_carrier; + if (IN_SET(network->activation_policy, ACTIVATION_POLICY_DOWN, ACTIVATION_POLICY_ALWAYS_DOWN, ACTIVATION_POLICY_MANUAL)) { + if (network->required_for_online < 0 || + (network->required_for_online == true && network->activation_policy == ACTIVATION_POLICY_ALWAYS_DOWN)) { + log_debug("%s: Setting RequiredForOnline=no because ActivationPolicy=%s.", network->filename, + activation_policy_to_string(network->activation_policy)); + network->required_for_online = false; + } else if (network->required_for_online == true) + log_warning("%s: RequiredForOnline=yes and ActivationPolicy=%s, " + "this may cause a delay at boot.", network->filename, + activation_policy_to_string(network->activation_policy)); + } + + if (network->required_for_online < 0) + network->required_for_online = true; + if (network->keep_configuration < 0) network->keep_configuration = KEEP_CONFIGURATION_NO; @@ -303,7 +318,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi .manager = manager, .n_ref = 1, - .required_for_online = true, + .required_for_online = -1, .required_operstate_for_online = LINK_OPERSTATE_RANGE_DEFAULT, .activation_policy = _ACTIVATION_POLICY_INVALID, .arp = -1, @@ -1108,7 +1123,7 @@ int config_parse_required_for_online( assert(network); if (isempty(rvalue)) { - network->required_for_online = true; + network->required_for_online = -1; network->required_operstate_for_online = LINK_OPERSTATE_RANGE_DEFAULT; return 0; } diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h index 98b8b95295..b39063fe8a 100644 --- a/src/network/networkd-network.h +++ b/src/network/networkd-network.h @@ -102,7 +102,7 @@ struct Network { int allmulticast; int promiscuous; bool unmanaged; - bool required_for_online; /* Is this network required to be considered online? */ + int required_for_online; /* Is this network required to be considered online? */ LinkOperationalStateRange required_operstate_for_online; AddressFamily required_family_for_online; ActivationPolicy activation_policy; From 003015af5ab21b1334e63b1af24a810c8c72ce68 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Wed, 9 Jun 2021 07:57:01 -0400 Subject: [PATCH 2/3] networkctl: add field 'Required For Online' --- src/network/networkctl.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/network/networkctl.c b/src/network/networkctl.c index 1c2cb7691a..6035a7728a 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -2226,6 +2226,16 @@ static int link_status_one( return table_log_add_error(r); } + r = sd_network_link_get_required_for_online(info->ifindex); + if (r >= 0) { + r = table_add_many(table, + TABLE_EMPTY, + TABLE_STRING, "Required For Online:", + TABLE_BOOLEAN, r); + if (r < 0) + return table_log_add_error(r); + } + if (lease) { const void *client_id; size_t client_id_len; From 61764fe4e27c3499eef9e0a300efebcc2f979403 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Tue, 8 Jun 2021 17:13:27 -0400 Subject: [PATCH 3/3] test: add test to verify RequiredForOnline= setting with ActivationPolicy= Add test to verify that the RequiredForOnline= value is correct based on the configuration of ActivationPolicy= --- .../25-activation-policy.network.d/bound.conf | 5 +++ .../required-no.conf | 2 + .../required-yes.conf | 2 + test/test-network/systemd-networkd-tests.py | 45 +++++++++++++++++++ 4 files changed, 54 insertions(+) create mode 100644 test/test-network/conf/25-activation-policy.network.d/bound.conf create mode 100644 test/test-network/conf/25-activation-policy.network.d/required-no.conf create mode 100644 test/test-network/conf/25-activation-policy.network.d/required-yes.conf diff --git a/test/test-network/conf/25-activation-policy.network.d/bound.conf b/test/test-network/conf/25-activation-policy.network.d/bound.conf new file mode 100644 index 0000000000..c6d73f0aeb --- /dev/null +++ b/test/test-network/conf/25-activation-policy.network.d/bound.conf @@ -0,0 +1,5 @@ +[Link] +ActivationPolicy=bound + +[Network] +BindCarrier=dummy98 diff --git a/test/test-network/conf/25-activation-policy.network.d/required-no.conf b/test/test-network/conf/25-activation-policy.network.d/required-no.conf new file mode 100644 index 0000000000..b97b85a1aa --- /dev/null +++ b/test/test-network/conf/25-activation-policy.network.d/required-no.conf @@ -0,0 +1,2 @@ +[Link] +RequiredForOnline=no diff --git a/test/test-network/conf/25-activation-policy.network.d/required-yes.conf b/test/test-network/conf/25-activation-policy.network.d/required-yes.conf new file mode 100644 index 0000000000..84e0f055c9 --- /dev/null +++ b/test/test-network/conf/25-activation-policy.network.d/required-yes.conf @@ -0,0 +1,2 @@ +[Link] +RequiredForOnline=yes diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index cea66a0e8d..4cc0c64f90 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -1793,6 +1793,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): units = [ '11-dummy.netdev', '12-dummy.netdev', + '12-dummy.network', '23-active-slave.network', '24-keep-configuration-static.network', '24-search-domain.network', @@ -2801,6 +2802,50 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): with self.subTest(test=test): self._test_activation_policy(test) + def _test_activation_policy_required_for_online(self, policy, required): + self.setUp() + conffile = '25-activation-policy.network' + units = ['11-dummy.netdev', '12-dummy.netdev', '12-dummy.network', conffile] + if policy: + units += [f'{conffile}.d/{policy}.conf'] + if required: + units += [f'{conffile}.d/required-{required}.conf'] + copy_unit_to_networkd_unit_path(*units, dropins=False) + start_networkd() + + if policy.endswith('down') or policy == 'manual': + self.wait_operstate('test1', 'off', setup_state='configuring') + else: + self.wait_online(['test1']) + + if policy == 'always-down': + # if always-down, required for online is forced to no + expected = False + elif required: + # otherwise if required for online is specified, it should match that + expected = required == 'yes' + elif policy: + # otherwise if only policy specified, required for online defaults to + # true if policy is up, always-up, or bound + expected = policy.endswith('up') or policy == 'bound' + else: + # default is true, if neither are specified + expected = True + + output = check_output(*networkctl_cmd, '-n', '0', 'status', 'test1', env=env) + print(output) + + yesno = 'yes' if expected else 'no' + self.assertRegex(output, f'Required For Online: {yesno}') + + self.tearDown() + + def test_activation_policy_required_for_online(self): + for policy in ['up', 'always-up', 'manual', 'always-down', 'down', 'bound', '']: + for required in ['yes', 'no', '']: + with self.subTest(policy=policy, required=required): + self._test_activation_policy_required_for_online(policy, required) + def test_domain(self): copy_unit_to_networkd_unit_path('12-dummy.netdev', '24-search-domain.network') start_networkd()