Merge pull request #19883 from ddstreet/activation-policy-down-required-for-online-no

Activation policy down required for online no

man/systemd.network.xml

@@ -220,11 +220,16 @@
           <command>systemd-networkd-wait-online</command>). When <literal>no</literal>, the network is ignored
           when determining the online state. When a minimum operational state and an optional maximum operational
           state are set, <literal>yes</literal> is implied, and this controls the minimum and maximum
-          operational state required for the network interface to be considered online.
-          Defaults to <literal>yes</literal>.</para>
+          operational state required for the network interface to be considered online.</para>
 
-          <para>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
+          <para>Defaults to <literal>yes</literal> when <varname>ActivationPolicy=</varname> is not set,
+          or set to <literal>up</literal>, <literal>always-up</literal>, or <literal>bound</literal>.
+          Defaults to <literal>no</literal> when <varname>ActivationPolicy=</varname> is set to
+          <literal>manual</literal> or <literal>down</literal>. This is forced to <literal>no</literal>
+          when <varname>ActivationPolicy=</varname> is set to <literal>always-down</literal>.</para>
+
+          <para>The network will be brought up normally (as configured by <varname>ActivationPolicy=</varname>),
+          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 <command>systemd-networkd-wait-online</command>
           if <literal>RequiredForOnline=no</literal>.</para>
@@ -265,6 +270,11 @@
           the administrative state. When <varname>BindCarrier=</varname> is also set, this is
           automatically set to <literal>bound</literal> and any other value is ignored.</para>
 
+          <para>When the policy is set to <literal>down</literal> or <literal>manual</literal>,
+          the default value of <varname>RequiredForOnline=</varname> is <literal>no</literal>.
+          When the policy is set to <literal>always-down</literal>, the value of
+          <varname>RequiredForOnline=</varname> forced to <literal>no</literal>.</para>
+
           <para>The administrative state is not the same as the carrier state, so using
           <literal>always-up</literal> 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

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;

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;
         }

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;

test/test-network/conf/25-activation-policy.network.d/bound.conf

@@ -0,0 +1,5 @@
+[Link]
+ActivationPolicy=bound
+
+[Network]
+BindCarrier=dummy98

test/test-network/conf/25-activation-policy.network.d/required-no.conf

@@ -0,0 +1,2 @@
+[Link]
+RequiredForOnline=no

test/test-network/conf/25-activation-policy.network.d/required-yes.conf

@@ -0,0 +1,2 @@
+[Link]
+RequiredForOnline=yes

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()