Merge pull request #21667 from yuwata/resolve-do-not-re-read-settings-from-networkd-when-unmodified

resolve: do not re-read settings from networkd when link state file is unmodified

.github/codeql-custom.qls

@@ -31,3 +31,6 @@
       - "security"
       - "correctness"
     severity: "error"
+- exclude:
+    id:
+      - cpp/fixme-comment

man/sd_notify.xml

@@ -218,7 +218,7 @@
 
         <listitem><para>Stores additional file descriptors in the service manager. File descriptors sent this way will
         be maintained per-service by the service manager and will later be handed back using the usual file descriptor
-        passing logic at the next invocation of the service, see
+        passing logic at the next invocation of the service (e.g. when it is restarted), see
         <citerefentry><refentrytitle>sd_listen_fds</refentrytitle><manvolnum>3</manvolnum></citerefentry>.  This is
         useful for implementing services that can restart after an explicit request or a crash without losing
         state. Any open sockets and other file descriptors which should not be closed during the restart may be stored
@@ -232,7 +232,8 @@
         <constant>EPOLLHUP</constant> or <constant>EPOLLERR</constant> event seen on them will result in their
         automatic removal from the store. Multiple arrays of file descriptors may be sent in separate messages, in
         which case the arrays are combined. Note that the service manager removes duplicate (pointing to the same
-        object) file descriptors before passing them to the service. Use <function>sd_pid_notify_with_fds()</function>
+        object) file descriptors before passing them to the service. When a service is stopped, its file descriptor
+        store is discarded and all file descriptors in it are closed. Use <function>sd_pid_notify_with_fds()</function>
         to send messages with <literal>FDSTORE=1</literal>, see below.</para></listitem>
       </varlistentry>
 

src/journal/journald-file.c

@@ -160,7 +160,7 @@ static void journald_file_set_offline_internal(JournaldFile *f) {
                         (void) fsync(f->file->fd);
 
                         /* If we've archived the journal file, first try to re-enable COW on the file. If the
-                         * FS_NOCOW_FL flag was never set or we succesfully removed it, continue. If we fail
+                         * FS_NOCOW_FL flag was never set or we successfully removed it, continue. If we fail
                          * to remove the flag on the archived file, rewrite the file without the NOCOW flag.
                          * We need this fallback because on some filesystems (BTRFS), the NOCOW flag cannot
                          * be removed after data has been written to a file. The only way to remove it is to

src/libsystemd-network/test-dhcp-option.c

@@ -146,7 +146,6 @@ static int test_options_cb(uint8_t code, uint8_t len, const void *option, void *
         int *desclen = NULL, *descpos = NULL;
         uint8_t optcode = 0;
         uint8_t optlen = 0;
-        uint8_t i;
 
         assert_se((!desc && !code && !len) || desc);
 
@@ -199,10 +198,10 @@ static int test_options_cb(uint8_t code, uint8_t len, const void *option, void *
         assert_se(code == optcode);
         assert_se(len == optlen);
 
-        for (i = 0; i < len; i++) {
+        for (unsigned i = 0; i < len; i++) {
-
                 if (verbose)
-                        printf("0x%02x(0x%02x) ", ((uint8_t*) option)[i],
+                        printf("0x%02x(0x%02x) ",
+                               ((uint8_t*) option)[i],
                                descoption[*descpos + 2 + i]);
 
                 assert_se(((uint8_t*) option)[i] == descoption[*descpos + 2 + i]);
@@ -278,7 +277,7 @@ static void test_option_removal(struct option_desc *desc) {
         assert_se(dhcp_option_parse(message, sizeof(DHCPMessage) + desc->len, NULL, NULL, NULL) < 0);
 }
 
-static uint8_t options[64] = {
+static uint8_t the_options[64] = {
         'A', 'B', 'C', 'D',
         160, 2, 0x11, 0x12,
         0,
@@ -292,7 +291,6 @@ static uint8_t options[64] = {
 static void test_option_set(void) {
         _cleanup_free_ DHCPMessage *result = NULL;
         size_t offset = 0, len, pos;
-        unsigned i;
 
         result = malloc0(sizeof(DHCPMessage) + 11);
         assert_se(result);
@@ -316,26 +314,26 @@ static void test_option_set(void) {
 
         offset = pos = 4;
         len = 11;
-        while (pos < len && options[pos] != SD_DHCP_OPTION_END) {
+        while (pos < len && the_options[pos] != SD_DHCP_OPTION_END) {
                 assert_se(dhcp_option_append(result, len, &offset, DHCP_OVERLOAD_SNAME,
-                                             options[pos],
+                                             the_options[pos],
-                                             options[pos + 1],
+                                             the_options[pos + 1],
-                                             &options[pos + 2]) >= 0);
+                                             &the_options[pos + 2]) >= 0);
 
-                if (options[pos] == SD_DHCP_OPTION_PAD)
+                if (the_options[pos] == SD_DHCP_OPTION_PAD)
                         pos++;
                 else
-                        pos += 2 + options[pos + 1];
+                        pos += 2 + the_options[pos + 1];
 
                 if (pos < len)
                         assert_se(offset == pos);
         }
 
-        for (i = 0; i < 9; i++) {
+        for (unsigned i = 0; i < 9; i++) {
                 if (verbose)
                         printf("%2u: 0x%02x(0x%02x) (options)\n", i, result->options[i],
-                               options[i]);
+                               the_options[i]);
-                assert_se(result->options[i] == options[i]);
+                assert_se(result->options[i] == the_options[i]);
         }
 
         if (verbose)
@@ -350,11 +348,11 @@ static void test_option_set(void) {
 
         assert_se(result->options[10] == SD_DHCP_OPTION_PAD);
 
-        for (i = 0; i < pos - 8; i++) {
+        for (unsigned i = 0; i < pos - 8; i++) {
                 if (verbose)
                         printf("%2u: 0x%02x(0x%02x) (sname)\n", i, result->sname[i],
-                               options[i + 9]);
+                               the_options[i + 9]);
-                assert_se(result->sname[i] == options[i + 9]);
+                assert_se(result->sname[i] == the_options[i + 9]);
         }
 
         if (verbose)
@@ -362,19 +360,17 @@ static void test_option_set(void) {
 }
 
 int main(int argc, char *argv[]) {
-        unsigned i;
-
         test_invalid_buffer_length();
         test_message_init();
 
         test_options(NULL);
 
-        for (i = 0; i < ELEMENTSOF(option_tests); i++)
+        for (unsigned i = 0; i < ELEMENTSOF(option_tests); i++)
                 test_options(&option_tests[i]);
 
         test_option_set();
 
-        for (i = 0; i < ELEMENTSOF(option_tests); i++) {
+        for (unsigned i = 0; i < ELEMENTSOF(option_tests); i++) {
                 struct option_desc *desc = &option_tests[i];
                 if (!desc->success || desc->snamelen > 0 || desc->filelen > 0)
                         continue;

src/libsystemd/sd-network/sd-network.c

@@ -37,27 +37,27 @@ static int network_get_string(const char *field, char **ret) {
         return 0;
 }
 
-_public_ int sd_network_get_operational_state(char **state) {
+int sd_network_get_operational_state(char **state) {
         return network_get_string("OPER_STATE", state);
 }
 
-_public_ int sd_network_get_carrier_state(char **state) {
+int sd_network_get_carrier_state(char **state) {
         return network_get_string("CARRIER_STATE", state);
 }
 
-_public_ int sd_network_get_address_state(char **state) {
+int sd_network_get_address_state(char **state) {
         return network_get_string("ADDRESS_STATE", state);
 }
 
-_public_ int sd_network_get_ipv4_address_state(char **state) {
+int sd_network_get_ipv4_address_state(char **state) {
         return network_get_string("IPV4_ADDRESS_STATE", state);
 }
 
-_public_ int sd_network_get_ipv6_address_state(char **state) {
+int sd_network_get_ipv6_address_state(char **state) {
         return network_get_string("IPV6_ADDRESS_STATE", state);
 }
 
-_public_ int sd_network_get_online_state(char **state) {
+int sd_network_get_online_state(char **state) {
         return network_get_string("ONLINE_STATE", state);
 }
 
@@ -90,24 +90,24 @@ static int network_get_strv(const char *key, char ***ret) {
         return r;
 }
 
-_public_ int sd_network_get_dns(char ***ret) {
+int sd_network_get_dns(char ***ret) {
         return network_get_strv("DNS", ret);
 }
 
-_public_ int sd_network_get_ntp(char ***ret) {
+int sd_network_get_ntp(char ***ret) {
         return network_get_strv("NTP", ret);
 }
 
-_public_ int sd_network_get_search_domains(char ***ret) {
+int sd_network_get_search_domains(char ***ret) {
         return network_get_strv("DOMAINS", ret);
 }
 
-_public_ int sd_network_get_route_domains(char ***ret) {
+int sd_network_get_route_domains(char ***ret) {
         return network_get_strv("ROUTE_DOMAINS", ret);
 }
 
 static int network_link_get_string(int ifindex, const char *field, char **ret) {
-        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex) + 1];
+        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex)];
         _cleanup_free_ char *s = NULL;
         int r;
 
@@ -130,7 +130,7 @@ static int network_link_get_string(int ifindex, const char *field, char **ret) {
 }
 
 static int network_link_get_strv(int ifindex, const char *key, char ***ret) {
-        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex) + 1];
+        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex)];
         _cleanup_strv_free_ char **a = NULL;
         _cleanup_free_ char *s = NULL;
         int r;
@@ -161,19 +161,19 @@ static int network_link_get_strv(int ifindex, const char *key, char ***ret) {
         return r;
 }
 
-_public_ int sd_network_link_get_setup_state(int ifindex, char **state) {
+int sd_network_link_get_setup_state(int ifindex, char **state) {
         return network_link_get_string(ifindex, "ADMIN_STATE", state);
 }
 
-_public_ int sd_network_link_get_network_file(int ifindex, char **filename) {
+int sd_network_link_get_network_file(int ifindex, char **filename) {
         return network_link_get_string(ifindex, "NETWORK_FILE", filename);
 }
 
-_public_ int sd_network_link_get_operational_state(int ifindex, char **state) {
+int sd_network_link_get_operational_state(int ifindex, char **state) {
         return network_link_get_string(ifindex, "OPER_STATE", state);
 }
 
-_public_ int sd_network_link_get_required_family_for_online(int ifindex, char **state) {
+int sd_network_link_get_required_family_for_online(int ifindex, char **state) {
         _cleanup_free_ char *s = NULL;
         int r;
 
@@ -193,35 +193,35 @@ _public_ int sd_network_link_get_required_family_for_online(int ifindex, char **
         return 0;
 }
 
-_public_ int sd_network_link_get_carrier_state(int ifindex, char **state) {
+int sd_network_link_get_carrier_state(int ifindex, char **state) {
         return network_link_get_string(ifindex, "CARRIER_STATE", state);
 }
 
-_public_ int sd_network_link_get_address_state(int ifindex, char **state) {
+int sd_network_link_get_address_state(int ifindex, char **state) {
         return network_link_get_string(ifindex, "ADDRESS_STATE", state);
 }
 
-_public_ int sd_network_link_get_ipv4_address_state(int ifindex, char **state) {
+int sd_network_link_get_ipv4_address_state(int ifindex, char **state) {
         return network_link_get_string(ifindex, "IPV4_ADDRESS_STATE", state);
 }
 
-_public_ int sd_network_link_get_ipv6_address_state(int ifindex, char **state) {
+int sd_network_link_get_ipv6_address_state(int ifindex, char **state) {
         return network_link_get_string(ifindex, "IPV6_ADDRESS_STATE", state);
 }
 
-_public_ int sd_network_link_get_online_state(int ifindex, char **state) {
+int sd_network_link_get_online_state(int ifindex, char **state) {
         return network_link_get_string(ifindex, "ONLINE_STATE", state);
 }
 
-_public_ int sd_network_link_get_dhcp6_client_iaid_string(int ifindex, char **iaid) {
+int sd_network_link_get_dhcp6_client_iaid_string(int ifindex, char **iaid) {
         return network_link_get_string(ifindex, "DHCP6_CLIENT_IAID", iaid);
 }
 
-_public_ int sd_network_link_get_dhcp6_client_duid_string(int ifindex, char **duid) {
+int sd_network_link_get_dhcp6_client_duid_string(int ifindex, char **duid) {
         return network_link_get_string(ifindex, "DHCP6_CLIENT_DUID", duid);
 }
 
-_public_ int sd_network_link_get_required_for_online(int ifindex) {
+int sd_network_link_get_required_for_online(int ifindex) {
         _cleanup_free_ char *s = NULL;
         int r;
 
@@ -236,7 +236,7 @@ _public_ int sd_network_link_get_required_for_online(int ifindex) {
         return parse_boolean(s);
 }
 
-_public_ int sd_network_link_get_required_operstate_for_online(int ifindex, char **state) {
+int sd_network_link_get_required_operstate_for_online(int ifindex, char **state) {
         _cleanup_free_ char *s = NULL;
         int r;
 
@@ -257,7 +257,7 @@ _public_ int sd_network_link_get_required_operstate_for_online(int ifindex, char
         return 0;
 }
 
-_public_ int sd_network_link_get_activation_policy(int ifindex, char **policy) {
+int sd_network_link_get_activation_policy(int ifindex, char **policy) {
         _cleanup_free_ char *s = NULL;
         int r;
 
@@ -278,48 +278,48 @@ _public_ int sd_network_link_get_activation_policy(int ifindex, char **policy) {
         return 0;
 }
 
-_public_ int sd_network_link_get_llmnr(int ifindex, char **llmnr) {
+int sd_network_link_get_llmnr(int ifindex, char **llmnr) {
         return network_link_get_string(ifindex, "LLMNR", llmnr);
 }
 
-_public_ int sd_network_link_get_mdns(int ifindex, char **mdns) {
+int sd_network_link_get_mdns(int ifindex, char **mdns) {
         return network_link_get_string(ifindex, "MDNS", mdns);
 }
 
-_public_ int sd_network_link_get_dns_over_tls(int ifindex, char **dns_over_tls) {
+int sd_network_link_get_dns_over_tls(int ifindex, char **dns_over_tls) {
         return network_link_get_string(ifindex, "DNS_OVER_TLS", dns_over_tls);
 }
 
-_public_ int sd_network_link_get_dnssec(int ifindex, char **dnssec) {
+int sd_network_link_get_dnssec(int ifindex, char **dnssec) {
         return network_link_get_string(ifindex, "DNSSEC", dnssec);
 }
 
-_public_ int sd_network_link_get_dnssec_negative_trust_anchors(int ifindex, char ***nta) {
+int sd_network_link_get_dnssec_negative_trust_anchors(int ifindex, char ***nta) {
         return network_link_get_strv(ifindex, "DNSSEC_NTA", nta);
 }
 
-_public_ int sd_network_link_get_dns(int ifindex, char ***ret) {
+int sd_network_link_get_dns(int ifindex, char ***ret) {
         return network_link_get_strv(ifindex, "DNS", ret);
 }
 
-_public_ int sd_network_link_get_ntp(int ifindex, char ***ret) {
+int sd_network_link_get_ntp(int ifindex, char ***ret) {
         return network_link_get_strv(ifindex, "NTP", ret);
 }
 
-_public_ int sd_network_link_get_sip(int ifindex, char ***ret) {
+int sd_network_link_get_sip(int ifindex, char ***ret) {
         return network_link_get_strv(ifindex, "SIP", ret);
 }
 
-_public_ int sd_network_link_get_search_domains(int ifindex, char ***ret) {
+int sd_network_link_get_search_domains(int ifindex, char ***ret) {
         return network_link_get_strv(ifindex, "DOMAINS", ret);
 }
 
-_public_ int sd_network_link_get_route_domains(int ifindex, char ***ret) {
+int sd_network_link_get_route_domains(int ifindex, char ***ret) {
         return network_link_get_strv(ifindex, "ROUTE_DOMAINS", ret);
 }
 
-_public_ int sd_network_link_get_dns_default_route(int ifindex) {
+int sd_network_link_get_dns_default_route(int ifindex) {
-        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex) + 1];
+        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex)];
         _cleanup_free_ char *s = NULL;
         int r;
 
@@ -338,7 +338,7 @@ _public_ int sd_network_link_get_dns_default_route(int ifindex) {
 }
 
 static int network_link_get_ifindexes(int ifindex, const char *key, int **ret) {
-        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex) + 1];
+        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex)];
         _cleanup_free_ int *ifis = NULL;
         _cleanup_free_ char *s = NULL;
         size_t c = 0;
@@ -379,14 +379,31 @@ static int network_link_get_ifindexes(int ifindex, const char *key, int **ret) {
         return c;
 }
 
-_public_ int sd_network_link_get_carrier_bound_to(int ifindex, int **ret) {
+int sd_network_link_get_carrier_bound_to(int ifindex, int **ret) {
         return network_link_get_ifindexes(ifindex, "CARRIER_BOUND_TO", ret);
 }
 
-_public_ int sd_network_link_get_carrier_bound_by(int ifindex, int **ret) {
+int sd_network_link_get_carrier_bound_by(int ifindex, int **ret) {
         return network_link_get_ifindexes(ifindex, "CARRIER_BOUND_BY", ret);
 }
 
+int sd_network_link_get_stat(int ifindex, struct stat *ret) {
+        char path[STRLEN("/run/systemd/netif/links/") + DECIMAL_STR_MAX(ifindex)];
+        struct stat st;
+
+        assert_return(ifindex > 0, -EINVAL);
+
+        xsprintf(path, "/run/systemd/netif/links/%i", ifindex);
+
+        if (stat(path, &st) < 0)
+                return -errno;
+
+        if (ret)
+                *ret = st;
+
+        return 0;
+}
+
 static int MONITOR_TO_FD(sd_network_monitor *m) {
         return (int) (unsigned long) m - 1;
 }
@@ -417,7 +434,7 @@ static int monitor_add_inotify_watch(int fd) {
         return 0;
 }
 
-_public_ int sd_network_monitor_new(sd_network_monitor **m, const char *category) {
+int sd_network_monitor_new(sd_network_monitor **m, const char *category) {
         _cleanup_close_ int fd = -1;
         int k;
         bool good = false;
@@ -443,14 +460,14 @@ _public_ int sd_network_monitor_new(sd_network_monitor **m, const char *category
         return 0;
 }
 
-_public_ sd_network_monitor* sd_network_monitor_unref(sd_network_monitor *m) {
+sd_network_monitor* sd_network_monitor_unref(sd_network_monitor *m) {
         if (m)
                 (void) close_nointr(MONITOR_TO_FD(m));
 
         return NULL;
 }
 
-_public_ int sd_network_monitor_flush(sd_network_monitor *m) {
+int sd_network_monitor_flush(sd_network_monitor *m) {
         union inotify_event_buffer buffer;
         struct inotify_event *e;
         ssize_t l;
@@ -483,14 +500,14 @@ _public_ int sd_network_monitor_flush(sd_network_monitor *m) {
         return 0;
 }
 
-_public_ int sd_network_monitor_get_fd(sd_network_monitor *m) {
+int sd_network_monitor_get_fd(sd_network_monitor *m) {
 
         assert_return(m, -EINVAL);
 
         return MONITOR_TO_FD(m);
 }
 
-_public_ int sd_network_monitor_get_events(sd_network_monitor *m) {
+int sd_network_monitor_get_events(sd_network_monitor *m) {
 
         assert_return(m, -EINVAL);
 
@@ -501,7 +518,7 @@ _public_ int sd_network_monitor_get_events(sd_network_monitor *m) {
         return POLLIN;
 }
 
-_public_ int sd_network_monitor_get_timeout(sd_network_monitor *m, uint64_t *timeout_usec) {
+int sd_network_monitor_get_timeout(sd_network_monitor *m, uint64_t *timeout_usec) {
 
         assert_return(m, -EINVAL);
         assert_return(timeout_usec, -EINVAL);

src/resolve/resolved-link.c

@@ -16,6 +16,7 @@
 #include "resolved-llmnr.h"
 #include "resolved-mdns.h"
 #include "socket-netlink.h"
+#include "stat-util.h"
 #include "string-util.h"
 #include "strv.h"
 #include "tmpfile-util.h"
@@ -568,27 +569,42 @@ static int link_is_managed(Link *l) {
         return !STR_IN_SET(state, "pending", "initialized", "unmanaged");
 }
 
-static void link_read_settings(Link *l) {
+static void link_enter_unmanaged(Link *l) {
-        int r;
-
         assert(l);
 
-        /* Read settings from networkd, except when networkd is not managing this interface. */
-
-        r = link_is_managed(l);
-        if (r < 0) {
-                log_link_warning_errno(l, r, "Failed to determine whether the interface is managed: %m");
-                return;
-        }
-        if (r == 0) {
-
         /* If this link used to be managed, but is now unmanaged, flush all our settings — but only once. */
         if (l->is_managed)
                 link_flush_settings(l);
 
         l->is_managed = false;
+}
+
+static void link_read_settings(Link *l) {
+        struct stat st;
+        int r;
+
+        assert(l);
+
+        /* Read settings from networkd, except when networkd is not managing this interface. */
+
+        r = sd_network_link_get_stat(l->ifindex, &st);
+        if (r == -ENOENT)
+                return link_enter_unmanaged(l);
+        if (r < 0)
+                return (void) log_link_warning_errno(l, r, "Failed to stat() networkd's link state file, ignoring: %m");
+
+        if (stat_inode_unmodified(&l->networkd_state_file_stat, &st))
+                /* The state file is unmodified. Not necessary to re-read settings. */
                 return;
-        }
+
+        /* Save the new stat for the next event. */
+        l->networkd_state_file_stat = st;
+
+        r = link_is_managed(l);
+        if (r < 0)
+                return (void) log_link_warning_errno(l, r, "Failed to determine whether the interface is managed, ignoring: %m");
+        if (r == 0)
+                return link_enter_unmanaged(l);
 
         l->is_managed = true;
 

src/resolve/resolved-link.h

@@ -1,6 +1,8 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
+#include <sys/stat.h>
+
 #include "sd-netlink.h"
 
 #include "in-addr-util.h"
@@ -65,6 +67,7 @@ struct Link {
         DnsScope *mdns_ipv4_scope;
         DnsScope *mdns_ipv6_scope;
 
+        struct stat networkd_state_file_stat;
         bool is_managed;
 
         char *ifname;

src/systemd/sd-network.h

@@ -18,6 +18,7 @@
 ***/
 
 #include <inttypes.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 
 #include "_sd-common.h"
@@ -187,6 +188,8 @@ int sd_network_link_get_dhcp6_client_iaid_string(int ifindex, char **iaid);
 /* Get DHCPv6 client DUID for a given link. */
 int sd_network_link_get_dhcp6_client_duid_string(int ifindex, char **duid);
 
+int sd_network_link_get_stat(int ifindex, struct stat *ret);
+
 /* Monitor object */
 typedef struct sd_network_monitor sd_network_monitor;
 

src/test/test-exec-util.c

@@ -236,7 +236,7 @@ static int gather_stdout_three(int fd, void *arg) {
         return 0;
 }
 
-const gather_stdout_callback_t gather_stdout[] = {
+const gather_stdout_callback_t gather_stdouts[] = {
         gather_stdout_one,
         gather_stdout_two,
         gather_stdout_three,
@@ -277,7 +277,8 @@ TEST(stdout_gathering) {
         if (access(name, X_OK) < 0 && ERRNO_IS_PRIVILEGE(errno))
                 return;
 
-        r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_stdout, args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS);
+        r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_stdouts, args, NULL, NULL,
+                                EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS);
         assert_se(r >= 0);
 
         log_info("got: %s", output);