Merge pull request #15619 from ddstreet/ignore_carrier_loss_default

Set IgnoreCarrierLoss= default to value of ConfigureWithoutCarrier=

man/systemd.network.xml

@@ -838,15 +838,17 @@
         <term><varname>ConfigureWithoutCarrier=</varname></term>
         <listitem>
           <para>Takes a boolean. Allows networkd to configure a specific link even if it has no carrier.
-          Defaults to false.
+          Defaults to false. If <option>IgnoreCarrierLoss=</option> is not explicitly set, it will
+          default to this value.
           </para>
         </listitem>
       </varlistentry>
       <varlistentry>
         <term><varname>IgnoreCarrierLoss=</varname></term>
         <listitem>
-          <para>A boolean. Allows networkd to retain both the static and dynamic configuration of the
-          interface even if its carrier is lost. Defaults to false.
+          <para>Takes a boolean. Allows networkd to retain both the static and dynamic configuration
+          of the interface even if its carrier is lost. When unset, the value specified with
+          <option>ConfigureWithoutCarrier=</option> is used.
           </para>
         </listitem>
       </varlistentry>

src/libsystemd-network/sd-ipv4ll.c

@@ -252,12 +252,14 @@ static int ipv4ll_start_internal(sd_ipv4ll *ll, bool reset_generation) {
                 return r;
         }
 
-        return 0;
+        return 1;
 }
 
 int sd_ipv4ll_start(sd_ipv4ll *ll) {
         assert_return(ll, -EINVAL);
-        assert_return(sd_ipv4ll_is_running(ll) == 0, -EBUSY);
+
+        if (sd_ipv4ll_is_running(ll))
+                return 0;
 
         return ipv4ll_start_internal(ll, true);
 }

src/libsystemd-network/test-ipv4ll.c

@@ -159,10 +159,10 @@ static void test_basic_request(sd_event *e) {
         assert_se(sd_ipv4ll_start(ll) == -EINVAL);
 
         assert_se(sd_ipv4ll_set_ifindex(ll, 1) == 0);
-        assert_se(sd_ipv4ll_start(ll) == 0);
+        assert_se(sd_ipv4ll_start(ll) == 1);
 
         sd_event_run(e, (uint64_t) -1);
-        assert_se(sd_ipv4ll_start(ll) == -EBUSY);
+        assert_se(sd_ipv4ll_start(ll) == 0);
 
         assert_se(sd_ipv4ll_is_running(ll));
 

src/libsystemd-network/test-ndisc-rs.c

@@ -297,6 +297,7 @@ static void test_rs(void) {
 
         assert_se(sd_ndisc_stop(nd) >= 0);
         assert_se(sd_ndisc_start(nd) >= 0);
+        assert_se(sd_ndisc_start(nd) >= 0);
         assert_se(sd_ndisc_stop(nd) >= 0);
 
         assert_se(sd_ndisc_start(nd) >= 0);

src/network/networkd-network-gperf.gperf

@@ -102,7 +102,7 @@ Network.ProxyARP,                            config_parse_tristate,
 Network.IPv6ProxyNDPAddress,                 config_parse_ipv6_proxy_ndp_address,                      0,                             0
 Network.BindCarrier,                         config_parse_strv,                                        0,                             offsetof(Network, bind_carrier)
 Network.ConfigureWithoutCarrier,             config_parse_bool,                                        0,                             offsetof(Network, configure_without_carrier)
-Network.IgnoreCarrierLoss,                   config_parse_bool,                                        0,                             offsetof(Network, ignore_carrier_loss)
+Network.IgnoreCarrierLoss,                   config_parse_tristate,                                    0,                             offsetof(Network, ignore_carrier_loss)
 Network.KeepConfiguration,                   config_parse_keep_configuration,                          0,                             offsetof(Network, keep_configuration)
 Address.Address,                             config_parse_address,                                     0,                             0
 Address.Peer,                                config_parse_address,                                     0,                             0

src/network/networkd-network.c

@@ -271,6 +271,9 @@ int network_verify(Network *network) {
         if (network->dhcp_use_gateway < 0)
                 network->dhcp_use_gateway = network->dhcp_use_routes;
 
+        if (network->ignore_carrier_loss < 0)
+                network->ignore_carrier_loss = network->configure_without_carrier;
+
         if (network->dhcp_critical >= 0) {
                 if (network->keep_configuration >= 0)
                         log_warning("%s: Both KeepConfiguration= and deprecated CriticalConnection= are set. "
@@ -461,6 +464,8 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi
                 .ipv6_accept_ra_route_table_set = false,
                 .ipv6_accept_ra_start_dhcp6_client = true,
 
+                .configure_without_carrier = false,
+                .ignore_carrier_loss = -1,
                 .keep_configuration = _KEEP_CONFIGURATION_INVALID,
                 .ipv6_address_gen_mode = _LINK_IPV6_ADDRESS_GEN_MODE_INVALID,
                 .can_triple_sampling = -1,

src/network/networkd-network.h

@@ -258,7 +258,7 @@ struct Network {
         int allmulticast;
         bool unmanaged;
         bool configure_without_carrier;
-        bool ignore_carrier_loss;
+        int ignore_carrier_loss;
         KeepConfiguration keep_configuration;
         LinkIPv6AddressGenMode ipv6_address_gen_mode;
         uint32_t iaid;

test/test-network/conf/25-test1.network

@@ -4,4 +4,3 @@ Name=test1
 [Network]
 Address=192.168.0.15/24
 Gateway=192.168.0.1
-ConfigureWithoutCarrier=true

test/test-network/conf/25-test1.network.d/configure-without-carrier.conf

@@ -0,0 +1,2 @@
+[Network]
+ConfigureWithoutCarrier=true

test/test-network/conf/25-test1.network.d/ignore-carrier-loss-no.conf

@@ -0,0 +1,2 @@
+[Network]
+IgnoreCarrierLoss=false

test/test-network/conf/26-bridge-configure-without-carrier.network

@@ -0,0 +1,9 @@
+[Match]
+Name=bridge99
+
+[Network]
+LinkLocalAddressing=yes
+IPv6AcceptRA=no
+ConfigureWithoutCarrier=yes
+Address=10.1.2.3/24
+Gateway=10.1.2.1

test/test-network/systemd-networkd-tests.py

@@ -445,6 +445,9 @@ class Utilities():
     def check_link_exists(self, link):
         self.assertTrue(link_exists(link))
 
+    def check_link_attr(self, *args):
+        self.assertEqual(read_link_attr(*args[:-1]), args[-1]);
+
     def wait_operstate(self, link, operstate='degraded', setup_state='configured', setup_timeout=5, fail_assert=True):
         """Wait for the link to reach the specified operstate and/or setup state.
 
@@ -1671,11 +1674,11 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         '25-gateway-next-static.network',
         '25-sysctl-disable-ipv6.network',
         '25-sysctl.network',
+        '25-test1.network',
         '25-veth-peer.network',
         '25-veth.netdev',
         '25-vrf.netdev',
         '26-link-local-addressing-ipv6.network',
-        'configure-without-carrier.network',
         'routing-policy-rule-dummy98.network',
         'routing-policy-rule-test1.network']
 
@@ -1762,15 +1765,56 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         self.assertNotRegex(output, '192.168.100.10/24')
 
     def test_configure_without_carrier(self):
-        copy_unit_to_networkd_unit_path('configure-without-carrier.network', '11-dummy.netdev')
+        copy_unit_to_networkd_unit_path('11-dummy.netdev')
         start_networkd()
-        self.wait_online(['test1:routable'])
+        self.wait_operstate('test1', 'off', '')
+        check_output('ip link set dev test1 up carrier off')
 
-        output = check_output(*networkctl_cmd, '-n', '0', 'status', 'test1', env=env)
-        print(output)
-        self.assertRegex(output, '192.168.0.15')
-        self.assertRegex(output, '192.168.0.1')
-        self.assertRegex(output, 'routable')
+        copy_unit_to_networkd_unit_path('25-test1.network.d/configure-without-carrier.conf', dropins=False)
+        restart_networkd()
+        self.wait_online(['test1:no-carrier'])
+
+        carrier_map = {'on': '1', 'off': '0'}
+        routable_map = {'on': 'routable', 'off': 'no-carrier'}
+        for carrier in ['off', 'on', 'off']:
+            with self.subTest(carrier=carrier):
+                if carrier_map[carrier] != read_link_attr('test1', 'carrier'):
+                    check_output(f'ip link set dev test1 carrier {carrier}')
+                self.wait_online([f'test1:{routable_map[carrier]}'])
+
+                output = check_output(*networkctl_cmd, '-n', '0', 'status', 'test1', env=env)
+                print(output)
+                self.assertRegex(output, '192.168.0.15')
+                self.assertRegex(output, '192.168.0.1')
+                self.assertRegex(output, routable_map[carrier])
+
+    def test_configure_without_carrier_yes_ignore_carrier_loss_no(self):
+        copy_unit_to_networkd_unit_path('11-dummy.netdev')
+        start_networkd()
+        self.wait_operstate('test1', 'off', '')
+        check_output('ip link set dev test1 up carrier off')
+
+        copy_unit_to_networkd_unit_path('25-test1.network')
+        restart_networkd()
+        self.wait_online(['test1:no-carrier'])
+
+        carrier_map = {'on': '1', 'off': '0'}
+        routable_map = {'on': 'routable', 'off': 'no-carrier'}
+        for (carrier, have_config) in [('off', True), ('on', True), ('off', False)]:
+            with self.subTest(carrier=carrier, have_config=have_config):
+                if carrier_map[carrier] != read_link_attr('test1', 'carrier'):
+                    check_output(f'ip link set dev test1 carrier {carrier}')
+                self.wait_online([f'test1:{routable_map[carrier]}'])
+
+                output = check_output(*networkctl_cmd, '-n', '0', 'status', 'test1', env=env)
+                print(output)
+                if have_config:
+                    self.assertRegex(output, '192.168.0.15')
+                    self.assertRegex(output, '192.168.0.1')
+                else:
+                    self.assertNotRegex(output, '192.168.0.15')
+                    self.assertNotRegex(output, '192.168.0.1')
+                self.assertRegex(output, routable_map[carrier])
 
     def test_routing_policy_rule(self):
         copy_unit_to_networkd_unit_path('routing-policy-rule-test1.network', '11-dummy.netdev')
@@ -2608,6 +2652,7 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities):
         '11-dummy.netdev',
         '12-dummy.netdev',
         '26-bridge.netdev',
+        '26-bridge-configure-without-carrier.network',
         '26-bridge-slave-interface-1.network',
         '26-bridge-slave-interface-2.network',
         '26-bridge-vlan-master.network',
@@ -2722,6 +2767,55 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, 'ff00::/8 table local metric 256 (linkdown )?pref medium')
 
+    def test_bridge_configure_without_carrier(self):
+        copy_unit_to_networkd_unit_path('26-bridge.netdev', '26-bridge-configure-without-carrier.network',
+                                        '11-dummy.netdev')
+        start_networkd()
+
+        # With ConfigureWithoutCarrier=yes, the bridge should remain configured for all these situations
+        for test in ['no-slave', 'add-slave', 'slave-up', 'slave-no-carrier', 'slave-carrier', 'slave-down']:
+            with self.subTest(test=test):
+                if test == 'no-slave':
+                    # bridge has no slaves; it's up but *might* not have carrier
+                    self.wait_online(['bridge99:no-carrier'])
+                    # due to a bug in the kernel, newly-created bridges are brought up
+                    # *with* carrier, unless they have had any setting changed; e.g.
+                    # their mac set, priority set, etc.  Then, they will lose carrier
+                    # as soon as a (down) slave interface is added, and regain carrier
+                    # again once the slave interface is brought up.
+                    #self.check_link_attr('bridge99', 'carrier', '0')
+                elif test == 'add-slave':
+                    # add slave to bridge, but leave it down; bridge is definitely no-carrier
+                    self.check_link_attr('test1', 'operstate', 'down')
+                    check_output('ip link set dev test1 master bridge99')
+                    self.wait_online(['bridge99:no-carrier:no-carrier'])
+                    self.check_link_attr('bridge99', 'carrier', '0')
+                elif test == 'slave-up':
+                    # bring up slave, which will have carrier; bridge gains carrier
+                    check_output('ip link set dev test1 up')
+                    self.wait_online(['bridge99:routable'])
+                    self.check_link_attr('bridge99', 'carrier', '1')
+                elif test == 'slave-no-carrier':
+                    # drop slave carrier; bridge loses carrier
+                    check_output('ip link set dev test1 carrier off')
+                    self.wait_online(['bridge99:no-carrier:no-carrier'])
+                    self.check_link_attr('bridge99', 'carrier', '0')
+                elif test == 'slave-carrier':
+                    # restore slave carrier; bridge gains carrier
+                    check_output('ip link set dev test1 carrier on')
+                    self.wait_online(['bridge99:routable'])
+                    self.check_link_attr('bridge99', 'carrier', '1')
+                elif test == 'slave-down':
+                    # bring down slave; bridge loses carrier
+                    check_output('ip link set dev test1 down')
+                    self.wait_online(['bridge99:no-carrier:no-carrier'])
+                    self.check_link_attr('bridge99', 'carrier', '0')
+
+                output = check_output(*networkctl_cmd, '-n', '0', 'status', 'bridge99', env=env)
+                print(output)
+                self.assertRegex(output, '10.1.2.3')
+                self.assertRegex(output, '10.1.2.1')
+
     def test_bridge_ignore_carrier_loss(self):
         copy_unit_to_networkd_unit_path('11-dummy.netdev', '12-dummy.netdev', '26-bridge.netdev',
                                         '26-bridge-slave-interface-1.network', '26-bridge-slave-interface-2.network',