initctl: (void)ify epoll_ctl()

CID 996298

src/core/cgroup.c

@@ -2334,29 +2334,25 @@ unsigned manager_dispatch_cgroup_realize_queue(Manager *m) {
 static void unit_add_siblings_to_cgroup_realize_queue(Unit *u) {
         Unit *slice;
 
-        /* This adds the siblings of the specified unit and the
+        /* This adds the siblings of the specified unit and the siblings of all parent units to the cgroup
-         * siblings of all parent units to the cgroup queue. (But
+         * queue. (But neither the specified unit itself nor the parents.) */
-         * neither the specified unit itself nor the parents.) */
 
         while ((slice = UNIT_DEREF(u->slice))) {
                 Iterator i;
                 Unit *m;
                 void *v;
 
-                HASHMAP_FOREACH_KEY(v, m, u->dependencies[UNIT_BEFORE], i) {
+                HASHMAP_FOREACH_KEY(v, m, slice->dependencies[UNIT_BEFORE], i) {
-                        /* Skip units that have a dependency on the slice
+                        /* Skip units that have a dependency on the slice but aren't actually in it. */
-                         * but aren't actually in it. */
                         if (UNIT_DEREF(m->slice) != slice)
                                 continue;
 
-                        /* No point in doing cgroup application for units
+                        /* No point in doing cgroup application for units without active processes. */
-                         * without active processes. */
                         if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(m)))
                                 continue;
 
-                        /* If the unit doesn't need any new controllers
+                        /* If the unit doesn't need any new controllers and has current ones realized, it
-                         * and has current ones realized, it doesn't need
+                         * doesn't need any changes. */
-                         * any changes. */
                         if (unit_has_mask_realized(m,
                                                    unit_get_target_mask(m),
                                                    unit_get_enable_mask(m)))

src/initctl/initctl.c

@@ -221,7 +221,7 @@ static void fifo_free(Fifo *f) {
 
         if (f->fd >= 0) {
                 if (f->server)
-                        epoll_ctl(f->server->epoll_fd, EPOLL_CTL_DEL, f->fd, NULL);
+                        (void) epoll_ctl(f->server->epoll_fd, EPOLL_CTL_DEL, f->fd, NULL);
 
                 safe_close(f->fd);
         }

src/network/networkd-link.c

@@ -1308,7 +1308,7 @@ static int link_set_proxy_arp(Link *link) {
         return 0;
 }
 
-static int link_configure_after_setting_mtu(Link *link);
+static int link_configure_continue(Link *link);
 
 static int set_mtu_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
         int r;
@@ -1329,7 +1329,7 @@ static int set_mtu_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link)
                 log_link_debug(link, "Setting MTU done.");
 
         if (link->state == LINK_STATE_INITIALIZED) {
-                r = link_configure_after_setting_mtu(link);
+                r = link_configure_continue(link);
                 if (r < 0)
                         link_enter_failed(link);
         }
@@ -1598,12 +1598,22 @@ static int link_address_genmode_handler(sd_netlink *rtnl, sd_netlink_message *m,
 
         assert(link);
 
+        link->setting_genmode = false;
+
         if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER))
                 return 1;
 
         r = sd_netlink_message_get_errno(m);
         if (r < 0)
                 log_link_message_warning_errno(link, m, r, "Could not set address genmode for interface, ignoring");
+        else
+                log_link_debug(link, "Setting address genmode done.");
+
+        if (link->state == LINK_STATE_INITIALIZED) {
+                r = link_configure_continue(link);
+                if (r < 0)
+                        link_enter_failed(link);
+        }
 
         return 1;
 }
@@ -1618,7 +1628,7 @@ static int link_configure_addrgen_mode(Link *link) {
         assert(link->manager);
         assert(link->manager->rtnl);
 
-        if (!socket_ipv6_is_supported())
+        if (!socket_ipv6_is_supported() || link->setting_genmode)
                 return 0;
 
         log_link_debug(link, "Setting address genmode for link");
@@ -1662,6 +1672,7 @@ static int link_configure_addrgen_mode(Link *link) {
                 return log_link_error_errno(link, r, "Could not send rtnetlink message: %m");
 
         link_ref(link);
+        link->setting_genmode = true;
 
         return 0;
 }
@@ -2495,6 +2506,48 @@ static bool link_address_is_dynamic(Link *link, Address *address) {
         return false;
 }
 
+static int link_enumerate_ipv6_tentative_addresses(Link *link) {
+        _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL, *reply = NULL;
+        sd_netlink_message *addr;
+        int r;
+
+        assert(link);
+        assert(link->manager);
+        assert(link->manager->rtnl);
+
+        r = sd_rtnl_message_new_addr(link->manager->rtnl, &req, RTM_GETADDR, 0, AF_INET6);
+        if (r < 0)
+                return r;
+
+        r = sd_netlink_call(link->manager->rtnl, req, 0, &reply);
+        if (r < 0)
+                return r;
+
+        for (addr = reply; addr; addr = sd_netlink_message_next(addr)) {
+                unsigned char flags;
+                int ifindex;
+
+                r = sd_rtnl_message_addr_get_ifindex(addr, &ifindex);
+                if (r < 0) {
+                        log_link_warning_errno(link, r, "rtnl: invalid ifindex, ignoring: %m");
+                        continue;
+                } else if (link->ifindex != ifindex)
+                        continue;
+
+                r = sd_rtnl_message_addr_get_flags(addr, &flags);
+                if (r < 0) {
+                        log_link_warning_errno(link, r, "rtnl: received address message with invalid flags, ignoring: %m");
+                        continue;
+                } else if (!(flags & IFA_F_TENTATIVE))
+                        continue;
+
+                log_link_debug(link, "Found tentative ipv6 link-local address");
+                (void) manager_rtnl_process_address(link->manager->rtnl, addr, link->manager);
+        }
+
+        return 0;
+}
+
 static int link_drop_foreign_config(Link *link) {
         Address *address;
         Neighbor *neighbor;
@@ -2502,6 +2555,14 @@ static int link_drop_foreign_config(Link *link) {
         Iterator i;
         int r;
 
+        /* The kernel doesn't notify us about tentative addresses;
+         * so if ipv6ll is disabled, we need to enumerate them now so we can drop them below */
+        if (!link_ipv6ll_enabled(link)) {
+                r = link_enumerate_ipv6_tentative_addresses(link);
+                if (r < 0)
+                        return r;
+        }
+
         SET_FOREACH(address, link->addresses_foreign, i) {
                 /* we consider IPv6LL addresses to be managed by the kernel */
                 if (address->family == AF_INET6 && in_addr_is_link_local(AF_INET6, &address->in_addr) == 1 && link_ipv6ll_enabled(link))
@@ -2673,15 +2734,6 @@ static int link_configure(Link *link) {
         if (link->iftype == ARPHRD_CAN)
                 return link_configure_can(link);
 
-        /* Drop foreign config, but ignore loopback or critical devices.
-         * We do not want to remove loopback address or addresses used for root NFS. */
-        if (!(link->flags & IFF_LOOPBACK) &&
-            link->network->keep_configuration != KEEP_CONFIGURATION_YES) {
-                r = link_drop_foreign_config(link);
-                if (r < 0)
-                        return r;
-        }
-
         /* If IPv6 configured that is static IPv6 address and IPv6LL autoconfiguration is enabled
          * for this interface, then enable IPv6 */
         (void) link_update_ipv6_sysctl(link);
@@ -2785,19 +2837,40 @@ static int link_configure(Link *link) {
         if (r < 0)
                 return r;
 
-        return link_configure_after_setting_mtu(link);
+        return link_configure_continue(link);
 }
 
-static int link_configure_after_setting_mtu(Link *link) {
+/* The configuration continues in this separate function, instead of
+ * including this in the above link_configure() function, for two
+ * reasons:
+ * 1) some devices reset the link when the mtu is set, which caused
+ *    an infinite loop here in networkd; see:
+ *    https://github.com/systemd/systemd/issues/6593
+ *    https://github.com/systemd/systemd/issues/9831
+ * 2) if ipv6ll is disabled, then bringing the interface up must be
+ *    delayed until after we get confirmation from the kernel that
+ *    the addr_gen_mode parameter has been set (via netlink), see:
+ *    https://github.com/systemd/systemd/issues/13882
+ */
+static int link_configure_continue(Link *link) {
         int r;
 
         assert(link);
         assert(link->network);
         assert(link->state == LINK_STATE_INITIALIZED);
 
-        if (link->setting_mtu)
+        if (link->setting_mtu || link->setting_genmode)
                 return 0;
 
+        /* Drop foreign config, but ignore loopback or critical devices.
+         * We do not want to remove loopback address or addresses used for root NFS. */
+        if (!(link->flags & IFF_LOOPBACK) &&
+            link->network->keep_configuration != KEEP_CONFIGURATION_YES) {
+                r = link_drop_foreign_config(link);
+                if (r < 0)
+                        return r;
+        }
+
         /* The kernel resets ipv6 mtu after changing device mtu;
          * we must set this here, after we've set device mtu */
         r = link_set_ipv6_mtu(link);

src/network/networkd-link.h

@@ -118,6 +118,7 @@ typedef struct Link {
         bool routing_policy_rules_configured:1;
         bool qdiscs_configured:1;
         bool setting_mtu:1;
+        bool setting_genmode:1;
         bool ipv6_mtu_set:1;
 
         LIST_HEAD(Address, pool_addresses);

src/shared/format-table.c

@@ -117,7 +117,8 @@ struct Table {
         size_t n_cells;
 
         bool header;   /* Whether to show the header row? */
-        size_t width;  /* If != (size_t) -1 the width to format this table in */
+        size_t width;  /* If == 0 format this as wide as necessary. If (size_t) -1 format this to console
+                        * width or less wide, but not wider. Otherwise the width to format this table in. */
 
         TableData **data;
         size_t n_allocated;
@@ -1619,9 +1620,9 @@ int table_print(Table *t, FILE *f) {
         }
 
         /* Calculate effective table width */
-        if (t->width != (size_t) -1)
+        if (t->width != 0 && t->width != (size_t) -1)
                 table_effective_width = t->width;
-        else if (pager_have() || !isatty(STDOUT_FILENO))
+        else if (t->width == 0 || pager_have() || !isatty(STDOUT_FILENO))
                 table_effective_width = table_requested_width;
         else
                 table_effective_width = MIN(table_requested_width, columns());

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

@@ -368,23 +368,62 @@ def restart_networkd(sleep_sec=0, show_logs=True, remove_state_files=True):
     stop_networkd(show_logs, remove_state_files)
     start_networkd(sleep_sec)
 
-def get_operstate(link, show_status=True, setup_state='configured'):
-    output = check_output(*networkctl_cmd, '-n', '0', 'status', link, env=env)
-    if show_status:
-        print(output)
-    for line in output.splitlines():
-        if 'State:' in line and (not setup_state or setup_state in line):
-            return line.split()[1]
-    return None
 
 class Utilities():
     def check_link_exists(self, link):
         self.assertTrue(link_exists(link))
 
-    def check_operstate(self, link, expected, show_status=True, setup_state='configured'):
+    def wait_operstate(self, link, operstate='degraded', setup_state='configured', setup_timeout=5, fail_assert=True):
-        self.assertRegex(get_operstate(link, show_status, setup_state), expected)
+        """Wait for the link to reach the specified operstate and/or setup state.
+
+        Specify None or '' for either operstate or setup_state to ignore that state.
+        This will recheck until the state conditions are met or the timeout expires.
+
+        If the link successfully matches the requested state, this returns True.
+        If this times out waiting for the link to match, the behavior depends on the
+        'fail_assert' parameter; if True, this causes a test assertion failure,
+        otherwise this returns False.  The default is to cause assertion failure.
+
+        Note that this function matches on *exactly* the given operstate and setup_state.
+        To wait for a link to reach *or exceed* a given operstate, use wait_online().
+        """
+        if not operstate:
+            operstate = r'\S+'
+        if not setup_state:
+            setup_state = r'\S+'
+
+        for secs in range(setup_timeout + 1):
+            output = check_output(*networkctl_cmd, '-n', '0', 'status', link, env=env)
+            print(output)
+            if re.search(rf'(?m)^\s*State:\s+{operstate}\s+\({setup_state}\)\s*$', output):
+                return True
+            # don't bother sleeping if time is up
+            if secs < setup_timeout:
+                time.sleep(1)
+        if fail_assert:
+            self.fail(f'Timed out waiting for {link} to reach state {operstate}/{setup_state}')
+        return False
 
     def wait_online(self, links_with_operstate, timeout='20s', bool_any=False, setup_state='configured', setup_timeout=5):
+        """Wait for the link(s) to reach the specified operstate and/or setup state.
+
+        This is similar to wait_operstate() but can be used for multiple links,
+        and it also calls systemd-networkd-wait-online to wait for the given operstate.
+        The operstate should be specified in the link name, like 'eth0:degraded'.
+        If just a link name is provided, wait-online's default operstate to wait for is degraded.
+
+        The 'timeout' parameter controls the systemd-networkd-wait-online timeout, and the
+        'setup_timeout' controls the per-link timeout waiting for the setup_state.
+
+        Set 'bool_any' to True to wait for any (instead of all) of the given links.
+        If this is set, no setup_state checks are done.
+
+        Note that this function waits for the link(s) to reach *or exceed* the given operstate.
+        However, the setup_state, if specified, must be matched *exactly*.
+
+        This returns if the link(s) reached the requested operstate/setup_state; otherwise it
+        raises CalledProcessError or fails test assertion.
+        """
         args = wait_online_cmd + [f'--timeout={timeout}'] + [f'--interface={link}' for link in links_with_operstate]
         if bool_any:
             args += ['--any']
@@ -396,22 +435,8 @@ class Utilities():
                 print(output)
             raise
         if not bool_any and setup_state:
-            # check at least once now, then once per sec for setup_timeout secs
-            for secs in range(setup_timeout + 1):
             for link in links_with_operstate:
-                    output = check_output(*networkctl_cmd, '-n', '0', 'status', link.split(':')[0])
+                self.wait_operstate(link.split(':')[0], None, setup_state, setup_timeout)
-                    print(output)
-                    if not re.search(rf'(?m)^\s*State:.*({setup_state}).*$', output):
-                        # this link isn't in the right state; break into the sleep below
-                        break
-                else:
-                    # all the links were in the right state; break to exit the timer loop
-                    break
-                # don't bother sleeping if time is up
-                if secs < setup_timeout:
-                    time.sleep(1)
-            else:
-                self.fail(f'link {link} state does not match {setup_state}')
 
     def wait_address(self, link, address_regex, scope='global', ipv='', timeout_sec=100):
         for i in range(timeout_sec):
@@ -490,9 +515,7 @@ class NetworkctlTests(unittest.TestCase, Utilities):
 
         copy_unit_to_networkd_unit_path('11-dummy.netdev')
         check_output(*networkctl_cmd, 'reload', env=env)
-        time.sleep(3)
+        self.wait_operstate('test1', 'off', setup_state='unmanaged')
-        self.check_link_exists('test1')
-        self.check_operstate('test1', 'off', setup_state='unmanaged')
 
         copy_unit_to_networkd_unit_path('11-dummy.network')
         check_output(*networkctl_cmd, 'reload', env=env)
@@ -500,16 +523,15 @@ class NetworkctlTests(unittest.TestCase, Utilities):
 
         remove_unit_from_networkd_path(['11-dummy.network'])
         check_output(*networkctl_cmd, 'reload', env=env)
-        time.sleep(1)
+        self.wait_operstate('test1', 'degraded', setup_state='unmanaged')
-        self.check_operstate('test1', 'degraded', setup_state='unmanaged')
 
         remove_unit_from_networkd_path(['11-dummy.netdev'])
         check_output(*networkctl_cmd, 'reload', env=env)
-        self.check_operstate('test1', 'degraded', setup_state='unmanaged')
+        self.wait_operstate('test1', 'degraded', setup_state='unmanaged')
 
         copy_unit_to_networkd_unit_path('11-dummy.netdev', '11-dummy.network')
         check_output(*networkctl_cmd, 'reload', env=env)
-        self.check_operstate('test1', 'degraded')
+        self.wait_operstate('test1', 'degraded')
 
     def test_glob(self):
         copy_unit_to_networkd_unit_path('11-dummy.netdev', '11-dummy.network')
@@ -807,8 +829,8 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities):
 
         self.wait_online(['bridge99', 'test1:degraded'], bool_any=True)
 
-        self.check_operstate('bridge99', '(off|no-carrier)', setup_state='configuring')
+        self.wait_operstate('bridge99', '(off|no-carrier)', setup_state='configuring')
-        self.check_operstate('test1', 'degraded')
+        self.wait_operstate('test1', 'degraded')
 
     def test_bridge(self):
         copy_unit_to_networkd_unit_path('25-bridge.netdev', '25-bridge-configure-without-carrier.network')
@@ -1885,7 +1907,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
 
         self.check_link_exists('dummy98')
 
-        self.check_operstate('dummy98', 'off', setup_state='unmanaged')
+        self.wait_operstate('dummy98', 'off', setup_state='unmanaged')
 
     def test_ipv6_address_label(self):
         copy_unit_to_networkd_unit_path('25-ipv6-address-label-section.network', '12-dummy.netdev')
@@ -2081,7 +2103,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, 'UP,LOWER_UP')
         self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1')
-        self.check_operstate('test1', 'routable')
+        self.wait_operstate('test1', 'routable')
 
         check_output('ip link add dummy99 type dummy')
         check_output('ip link set dummy99 up')
@@ -2090,7 +2112,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, 'UP,LOWER_UP')
         self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1')
-        self.check_operstate('test1', 'routable')
+        self.wait_operstate('test1', 'routable')
 
         check_output('ip link del dummy98')
         time.sleep(2)
@@ -2098,7 +2120,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, 'UP,LOWER_UP')
         self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1')
-        self.check_operstate('test1', 'routable')
+        self.wait_operstate('test1', 'routable')
 
         check_output('ip link set dummy99 down')
         time.sleep(2)
@@ -2107,7 +2129,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         self.assertNotRegex(output, 'UP,LOWER_UP')
         self.assertRegex(output, 'DOWN')
         self.assertNotRegex(output, '192.168.10')
-        self.check_operstate('test1', 'off')
+        self.wait_operstate('test1', 'off')
 
         check_output('ip link set dummy99 up')
         time.sleep(2)
@@ -2115,7 +2137,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, 'UP,LOWER_UP')
         self.assertRegex(output, 'inet 192.168.10.30/24 brd 192.168.10.255 scope global test1')
-        self.check_operstate('test1', 'routable')
+        self.wait_operstate('test1', 'routable')
 
     def test_domain(self):
         copy_unit_to_networkd_unit_path('12-dummy.netdev', '24-search-domain.network')
@@ -2348,39 +2370,29 @@ class NetworkdBondTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, 'MASTER,UP,LOWER_UP')
 
-        self.check_operstate('dummy98', 'enslaved')
+        self.wait_operstate('dummy98', 'enslaved')
-        self.check_operstate('test1', 'enslaved')
+        self.wait_operstate('test1', 'enslaved')
-        self.check_operstate('bond99', 'routable')
+        self.wait_operstate('bond99', 'routable')
 
         check_output('ip link set dummy98 down')
-        time.sleep(2)
 
-        self.check_operstate('dummy98', 'off')
+        self.wait_operstate('dummy98', 'off')
-        self.check_operstate('test1', 'enslaved')
+        self.wait_operstate('test1', 'enslaved')
-        self.check_operstate('bond99', 'degraded-carrier')
+        self.wait_operstate('bond99', 'degraded-carrier')
 
         check_output('ip link set dummy98 up')
-        time.sleep(2)
 
-        self.check_operstate('dummy98', 'enslaved')
+        self.wait_operstate('dummy98', 'enslaved')
-        self.check_operstate('test1', 'enslaved')
+        self.wait_operstate('test1', 'enslaved')
-        self.check_operstate('bond99', 'routable')
+        self.wait_operstate('bond99', 'routable')
 
         check_output('ip link set dummy98 down')
         check_output('ip link set test1 down')
-        time.sleep(2)
 
-        self.check_operstate('dummy98', 'off')
+        self.wait_operstate('dummy98', 'off')
-        self.check_operstate('test1', 'off')
+        self.wait_operstate('test1', 'off')
 
-        for trial in range(30):
+        if not self.wait_operstate('bond99', 'no-carrier', setup_timeout=30, fail_assert=False):
-            if trial > 0:
-                time.sleep(1)
-            output = check_output('ip address show bond99')
-            print(output)
-            if get_operstate('bond99') == 'no-carrier':
-                break
-        else:
             # Huh? Kernel does not recognize that all slave interfaces are down?
             # Let's confirm that networkd's operstate is consistent with ip's result.
             self.assertNotRegex(output, 'NO-CARRIER')
@@ -2491,14 +2503,12 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities):
         self.assertRegex(output, 'ff00::/8 table local metric 256 pref medium')
 
         self.assertEqual(call('ip link del test1'), 0)
-        time.sleep(3)
 
-        self.check_operstate('bridge99', 'degraded-carrier')
+        self.wait_operstate('bridge99', 'degraded-carrier')
 
         check_output('ip link del dummy98')
-        time.sleep(3)
 
-        self.check_operstate('bridge99', 'no-carrier')
+        self.wait_operstate('bridge99', 'no-carrier')
 
         output = check_output('ip address show bridge99')
         print(output)