Merge pull request #30967 from yuwata/network-can-required-operstate-for-online

network: several cleanups for required operstate for online, and change the default for CAN devices

diff --git a/man/systemd.network.xml b/man/systemd.network.xml
index 76f9f4d042c584f33ad563eb3d9f90ad4fdeff17..35c897af398efdcf94091db15e64bc7a9669fe6a 100644 (file)
--- a/man/systemd.network.xml
+++ b/man/systemd.network.xml
@@ -259,8 +259,9 @@
       <varlistentry>
         <term><varname>RequiredForOnline=</varname></term>
         <listitem>
-          <para>Takes a boolean or a minimum operational state and an optional maximum operational
-          state. Please see
+          <para>Takes a boolean, a minimum operational state (e.g., <literal>carrier</literal>), or a range
+          of operational state separated with a colon (e.g., <literal>degraded:routable</literal>).
+          Please see
           <citerefentry><refentrytitle>networkctl</refentrytitle><manvolnum>1</manvolnum></citerefentry>
           for possible operational states. When <literal>yes</literal>, the network is deemed required
           when determining whether the system is online (including when running
@@ -270,6 +271,12 @@
           minimum and maximum operational state required for the network interface to be considered
           online.</para>
 
+          <para>When <literal>yes</literal> is specified for a CAN device,
+          <command>systemd-networkd-wait-online</command> deems that the interface is online when its
+          operational state becomes <literal>carrier</literal>. For an interface with other type, e.g.
+          <literal>ether</literal>, the interface is deened online when its online state is
+          <literal>degraded</literal> or <literal>routable</literal>.</para>
+
           <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

diff --git a/src/libsystemd/sd-network/network-util.c b/src/libsystemd/sd-network/network-util.c
index 2059567ef894f031c33437ef3f4cd685ed31a37a..25c6e44a7791cdd95eb1a1b5fbba10bb86881470 100644 (file)
--- a/src/libsystemd/sd-network/network-util.c
+++ b/src/libsystemd/sd-network/network-util.c
@@ -90,49 +90,48 @@ static const char *const link_online_state_table[_LINK_ONLINE_STATE_MAX] = {
 
 DEFINE_STRING_TABLE_LOOKUP(link_online_state, LinkOnlineState);
 
-int parse_operational_state_range(const char *str, LinkOperationalStateRange *out) {
-        LinkOperationalStateRange range = { _LINK_OPERSTATE_INVALID, _LINK_OPERSTATE_INVALID };
-        _cleanup_free_ const char *min = NULL;
+int parse_operational_state_range(const char *s, LinkOperationalStateRange *ret) {
+        LinkOperationalStateRange range = LINK_OPERSTATE_RANGE_INVALID;
+        _cleanup_free_ char *buf = NULL;
         const char *p;
 
-        assert(str);
-        assert(out);
-
-        p = strchr(str, ':');
-        if (p) {
-                min = strndup(str, p - str);
+        assert(s);
+        assert(ret);
 
-                if (!isempty(p + 1)) {
-                        range.max = link_operstate_from_string(p + 1);
-                        if (range.max < 0)
-                                return -EINVAL;
-                }
-        } else
-                min = strdup(str);
+        /* allowed formats: "min", "min:", "min:max", ":max" */
 
-        if (!min)
-                return -ENOMEM;
+        if (isempty(s) || streq(s, ":"))
+                return -EINVAL;
 
-        if (!isempty(min)) {
-                range.min = link_operstate_from_string(min);
-                if (range.min < 0)
+        p = strchr(s, ':');
+        if (!p || isempty(p + 1))
+                range.max = LINK_OPERSTATE_ROUTABLE;
+        else {
+                range.max = link_operstate_from_string(p + 1);
+                if (range.max < 0)
                         return -EINVAL;
         }
 
-        /* Fail on empty strings. */
-        if (range.min == _LINK_OPERSTATE_INVALID && range.max == _LINK_OPERSTATE_INVALID)
-                return -EINVAL;
+        if (p) {
+                buf = strndup(s, p - s);
+                if (!buf)
+                        return -ENOMEM;
 
-        if (range.min == _LINK_OPERSTATE_INVALID)
+                s = buf;
+        }
+
+        if (isempty(s))
                 range.min = LINK_OPERSTATE_MISSING;
-        if (range.max == _LINK_OPERSTATE_INVALID)
-                range.max = LINK_OPERSTATE_ROUTABLE;
+        else {
+                range.min = link_operstate_from_string(s);
+                if (range.min < 0)
+                        return -EINVAL;
+        }
 
-        if (range.min > range.max)
+        if (!operational_state_range_is_valid(&range))
                 return -EINVAL;
 
-        *out = range;
-
+        *ret = range;
         return 0;
 }
 

diff --git a/src/libsystemd/sd-network/network-util.h b/src/libsystemd/sd-network/network-util.h
index c47e271a768fd29ab04438b0dc98fc173f150cb1..6fc6015902e878bcaf632298a93b2e16347f7a09 100644 (file)
--- a/src/libsystemd/sd-network/network-util.h
+++ b/src/libsystemd/sd-network/network-util.h
@@ -79,8 +79,30 @@ typedef struct LinkOperationalStateRange {
         LinkOperationalState max;
 } LinkOperationalStateRange;
 
-#define LINK_OPERSTATE_RANGE_DEFAULT (LinkOperationalStateRange) { LINK_OPERSTATE_DEGRADED, \
-                                                                   LINK_OPERSTATE_ROUTABLE }
-
-int parse_operational_state_range(const char *str, LinkOperationalStateRange *out);
+#define LINK_OPERSTATE_RANGE_DEFAULT            \
+        (const LinkOperationalStateRange) {     \
+                .min = LINK_OPERSTATE_DEGRADED, \
+                .max = LINK_OPERSTATE_ROUTABLE, \
+        }
+
+#define LINK_OPERSTATE_RANGE_INVALID            \
+        (const LinkOperationalStateRange) {     \
+                .min = _LINK_OPERSTATE_INVALID, \
+                .max = _LINK_OPERSTATE_INVALID, \
+        }
+
+int parse_operational_state_range(const char *s, LinkOperationalStateRange *ret);
 int network_link_get_operational_state(int ifindex, LinkOperationalState *ret);
+
+static inline bool operational_state_is_valid(LinkOperationalState s) {
+        return s >= 0 && s < _LINK_OPERSTATE_MAX;
+}
+static inline bool operational_state_range_is_valid(const LinkOperationalStateRange *range) {
+        return range &&
+                operational_state_is_valid(range->min) &&
+                operational_state_is_valid(range->max) &&
+                range->min <= range->max;
+}
+static inline bool operational_state_is_in_range(LinkOperationalState s, const LinkOperationalStateRange *range) {
+        return range && range->min <= s && s <= range->max;
+}

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index a39477e78b27a22787847d63602f19281c8e70fe..49ed8f59bb1804c67d1eb46203e3edd8135fe3a7 100644 (file)
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -71,6 +71,21 @@
 #include "udev-util.h"
 #include "vrf.h"
 
+void link_required_operstate_for_online(Link *link, LinkOperationalStateRange *ret) {
+        assert(link);
+        assert(ret);
+
+        if (link->network && operational_state_range_is_valid(&link->network->required_operstate_for_online))
+                *ret = link->network->required_operstate_for_online;
+        else if (link->iftype == ARPHRD_CAN)
+                *ret = (const LinkOperationalStateRange) {
+                        .min = LINK_OPERSTATE_CARRIER,
+                        .max = LINK_OPERSTATE_CARRIER,
+                };
+        else
+                *ret = LINK_OPERSTATE_RANGE_DEFAULT;
+}
+
 bool link_ipv6_enabled(Link *link) {
         assert(link);
 
@@ -1845,12 +1860,16 @@ void link_update_operstate(Link *link, bool also_update_master) {
         else
                 operstate = LINK_OPERSTATE_ENSLAVED;
 
+        LinkOperationalStateRange req;
+        link_required_operstate_for_online(link, &req);
+
         /* Only determine online state for managed links with RequiredForOnline=yes */
         if (!link->network || !link->network->required_for_online)
                 online_state = _LINK_ONLINE_STATE_INVALID;
-        else if (operstate < link->network->required_operstate_for_online.min ||
-                 operstate > link->network->required_operstate_for_online.max)
+
+        else if (!operational_state_is_in_range(operstate, &req))
                 online_state = LINK_ONLINE_STATE_OFFLINE;
+
         else {
                 AddressFamily required_family = link->network->required_family_for_online;
                 bool needs_ipv4 = required_family & ADDRESS_FAMILY_IPV4;
@@ -1861,14 +1880,14 @@ void link_update_operstate(Link *link, bool also_update_master) {
                  * to offline in the blocks below. */
                 online_state = LINK_ONLINE_STATE_ONLINE;
 
-                if (link->network->required_operstate_for_online.min >= LINK_OPERSTATE_DEGRADED) {
+                if (req.min >= LINK_OPERSTATE_DEGRADED) {
                         if (needs_ipv4 && ipv4_address_state < LINK_ADDRESS_STATE_DEGRADED)
                                 online_state = LINK_ONLINE_STATE_OFFLINE;
                         if (needs_ipv6 && ipv6_address_state < LINK_ADDRESS_STATE_DEGRADED)
                                 online_state = LINK_ONLINE_STATE_OFFLINE;
                 }
 
-                if (link->network->required_operstate_for_online.min >= LINK_OPERSTATE_ROUTABLE) {
+                if (req.min >= LINK_OPERSTATE_ROUTABLE) {
                         if (needs_ipv4 && ipv4_address_state < LINK_ADDRESS_STATE_ROUTABLE)
                                 online_state = LINK_ONLINE_STATE_OFFLINE;
                         if (needs_ipv6 && ipv6_address_state < LINK_ADDRESS_STATE_ROUTABLE)

diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h
index 05b028dff31d43f48f5d53a35d2be0950b5ab322..b5b1995361a77ad38d11a6d8d5332d16419f6218 100644 (file)
--- a/src/network/networkd-link.h
+++ b/src/network/networkd-link.h
@@ -259,3 +259,5 @@ int manager_rtnl_process_link(sd_netlink *rtnl, sd_netlink_message *message, Man
 
 int link_flags_to_string_alloc(uint32_t flags, char **ret);
 const char *kernel_operstate_to_string(int t) _const_;
+
+void link_required_operstate_for_online(Link *link, LinkOperationalStateRange *ret);

diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c
index 2d5c847a6a66c5620341d16b5838a4fa2bb25861..08c7da56997422a82a36ed9cc9f7b9f0b3880f21 100644 (file)
--- a/src/network/networkd-network.c
+++ b/src/network/networkd-network.c
@@ -372,7 +372,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi
                 .n_ref = 1,
 
                 .required_for_online = -1,
-                .required_operstate_for_online = LINK_OPERSTATE_RANGE_DEFAULT,
+                .required_operstate_for_online = LINK_OPERSTATE_RANGE_INVALID,
                 .activation_policy = _ACTIVATION_POLICY_INVALID,
                 .group = -1,
                 .arp = -1,
@@ -1213,8 +1213,6 @@ int config_parse_required_for_online(
                 void *userdata) {
 
         Network *network = ASSERT_PTR(userdata);
-        LinkOperationalStateRange range;
-        bool required = true;
         int r;
 
         assert(filename);
@@ -1223,11 +1221,11 @@ int config_parse_required_for_online(
 
         if (isempty(rvalue)) {
                 network->required_for_online = -1;
-                network->required_operstate_for_online = LINK_OPERSTATE_RANGE_DEFAULT;
+                network->required_operstate_for_online = LINK_OPERSTATE_RANGE_INVALID;
                 return 0;
         }
 
-        r = parse_operational_state_range(rvalue, &range);
+        r = parse_operational_state_range(rvalue, &network->required_operstate_for_online);
         if (r < 0) {
                 r = parse_boolean(rvalue);
                 if (r < 0) {
@@ -1237,13 +1235,12 @@ int config_parse_required_for_online(
                         return 0;
                 }
 
-                required = r;
-                range = LINK_OPERSTATE_RANGE_DEFAULT;
+                network->required_for_online = r;
+                network->required_operstate_for_online = LINK_OPERSTATE_RANGE_DEFAULT;
+                return 0;
         }
 
-        network->required_for_online = required;
-        network->required_operstate_for_online = range;
-
+        network->required_for_online = true;
         return 0;
 }
 

diff --git a/src/network/networkd-state-file.c b/src/network/networkd-state-file.c
index 9f0e365c22b6ce1d3dbfa7e52f386b0ce73042e6..b79b898832b3b9dc1edc77163104edb54a4317a2 100644 (file)
--- a/src/network/networkd-state-file.c
+++ b/src/network/networkd-state-file.c
@@ -630,11 +630,11 @@ static int link_save(Link *link) {
                 fprintf(f, "REQUIRED_FOR_ONLINE=%s\n",
                         yes_no(link->network->required_for_online));
 
-                LinkOperationalStateRange st = link->network->required_operstate_for_online;
-                fprintf(f, "REQUIRED_OPER_STATE_FOR_ONLINE=%s%s%s\n",
-                        strempty(link_operstate_to_string(st.min)),
-                        st.max != LINK_OPERSTATE_RANGE_DEFAULT.max ? ":" : "",
-                        st.max != LINK_OPERSTATE_RANGE_DEFAULT.max ? strempty(link_operstate_to_string(st.max)) : "");
+                LinkOperationalStateRange st;
+                link_required_operstate_for_online(link, &st);
+
+                fprintf(f, "REQUIRED_OPER_STATE_FOR_ONLINE=%s:%s\n",
+                        link_operstate_to_string(st.min), link_operstate_to_string(st.max));
 
                 fprintf(f, "REQUIRED_FAMILY_FOR_ONLINE=%s\n",
                         link_required_address_family_to_string(link->network->required_family_for_online));

diff --git a/src/network/wait-online/manager.c b/src/network/wait-online/manager.c
index 9213795f54a5a7d58b3d29e6b808c0cb2e2f7904..da5eed5d73b9e2299559771b4be1c24e1abdef24 100644 (file)
--- a/src/network/wait-online/manager.c
+++ b/src/network/wait-online/manager.c
@@ -52,7 +52,7 @@ static bool manager_ignore_link(Manager *m, Link *link) {
         return false;
 }
 
-static int manager_link_is_online(Manager *m, Link *l, LinkOperationalStateRange s) {
+static int manager_link_is_online(Manager *m, Link *l, const LinkOperationalStateRange *state_range) {
         AddressFamily required_family;
         bool needs_ipv4;
         bool needs_ipv6;
@@ -91,25 +91,23 @@ static int manager_link_is_online(Manager *m, Link *l, LinkOperationalStateRange
                                             "link is being processed by networkd: setup state is %s.",
                                             l->state);
 
-        if (s.min < 0)
-                s.min = m->required_operstate.min >= 0 ? m->required_operstate.min
-                                                       : l->required_operstate.min;
+        const LinkOperationalStateRange *range;
+        FOREACH_POINTER(range, state_range, &m->required_operstate, &l->required_operstate)
+                if (operational_state_range_is_valid(range))
+                        break;
+        assert(range != POINTER_MAX);
 
-        if (s.max < 0)
-                s.max = m->required_operstate.max >= 0 ? m->required_operstate.max
-                                                       : l->required_operstate.max;
-
-        if (l->operational_state < s.min || l->operational_state > s.max)
+        if (!operational_state_is_in_range(l->operational_state, range))
                 return log_link_debug_errno(l, SYNTHETIC_ERRNO(EADDRNOTAVAIL),
                                             "Operational state '%s' is not in range ['%s':'%s']",
                                             link_operstate_to_string(l->operational_state),
-                                            link_operstate_to_string(s.min), link_operstate_to_string(s.max));
+                                            link_operstate_to_string(range->min), link_operstate_to_string(range->max));
 
         required_family = m->required_family > 0 ? m->required_family : l->required_family;
         needs_ipv4 = required_family & ADDRESS_FAMILY_IPV4;
         needs_ipv6 = required_family & ADDRESS_FAMILY_IPV6;
 
-        if (s.min < LINK_OPERSTATE_ROUTABLE) {
+        if (range->min < LINK_OPERSTATE_ROUTABLE) {
                 if (needs_ipv4 && l->ipv4_address_state < LINK_ADDRESS_STATE_DEGRADED)
                         return log_link_debug_errno(l, SYNTHETIC_ERRNO(EADDRNOTAVAIL),
                                                     "No routable or link-local IPv4 address is configured.");
@@ -155,7 +153,7 @@ bool manager_configured(Manager *m) {
                                 continue;
                         }
 
-                        r = manager_link_is_online(m, l, *range);
+                        r = manager_link_is_online(m, l, range);
                         if (r <= 0 && !m->any)
                                 return false;
                         if (r > 0 && m->any)
@@ -175,9 +173,7 @@ bool manager_configured(Manager *m) {
                         continue;
                 }
 
-                r = manager_link_is_online(m, l,
-                                           (LinkOperationalStateRange) { _LINK_OPERSTATE_INVALID,
-                                                                         _LINK_OPERSTATE_INVALID });
+                r = manager_link_is_online(m, l, /* state_range = */ NULL);
                 if (r < 0 && !m->any) /* Unlike the above loop, unmanaged interfaces are ignored here. */
                         return false;
                 if (r > 0) {

diff --git a/src/network/wait-online/wait-online.c b/src/network/wait-online/wait-online.c
index 5328bba2d8c0a53f18354153454d53cb33a1bab7..544c360eddcf980355c45c3e0d392cf21a08bb4d 100644 (file)
--- a/src/network/wait-online/wait-online.c
+++ b/src/network/wait-online/wait-online.c
@@ -19,7 +19,7 @@ static bool arg_quiet = false;
 static usec_t arg_timeout = 120 * USEC_PER_SEC;
 static Hashmap *arg_interfaces = NULL;
 static char **arg_ignore = NULL;
-static LinkOperationalStateRange arg_required_operstate = { _LINK_OPERSTATE_INVALID, _LINK_OPERSTATE_INVALID };
+static LinkOperationalStateRange arg_required_operstate = LINK_OPERSTATE_RANGE_INVALID;
 static AddressFamily arg_required_family = ADDRESS_FAMILY_NO;
 static bool arg_any = false;
 
@@ -71,12 +71,11 @@ static int parse_interface_with_operstate_range(const char *str) {
         if (p) {
                 r = parse_operational_state_range(p + 1, range);
                 if (r < 0)
-                         log_error_errno(r, "Invalid operational state range '%s'", p + 1);
+                        return log_error_errno(r, "Invalid operational state range: %s", p + 1);
 
                 ifname = strndup(optarg, p - optarg);
         } else {
-                range->min = _LINK_OPERSTATE_INVALID;
-                range->max = _LINK_OPERSTATE_INVALID;
+                *range = LINK_OPERSTATE_RANGE_INVALID;
                 ifname = strdup(str);
         }
         if (!ifname)
@@ -84,18 +83,19 @@ static int parse_interface_with_operstate_range(const char *str) {
 
         if (!ifname_valid(ifname))
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Invalid interface name '%s'", ifname);
+                                       "Invalid interface name: %s", ifname);
 
-        r = hashmap_ensure_put(&arg_interfaces, &string_hash_ops, ifname, TAKE_PTR(range));
+        r = hashmap_ensure_put(&arg_interfaces, &string_hash_ops, ifname, range);
         if (r == -ENOMEM)
                 return log_oom();
         if (r < 0)
                 return log_error_errno(r, "Failed to store interface name: %m");
         if (r == 0)
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Interface name %s is already specified", ifname);
+                return log_error_errno(SYNTHETIC_ERRNO(EEXIST),
+                                       "Interface name %s is already specified.", ifname);
 
         TAKE_PTR(ifname);
+        TAKE_PTR(range);
         return 0;
 }
 
@@ -154,17 +154,11 @@ static int parse_argv(int argc, char *argv[]) {
 
                         break;
 
-                case 'o': {
-                        LinkOperationalStateRange range;
-
-                        r = parse_operational_state_range(optarg, &range);
+                case 'o':
+                        r = parse_operational_state_range(optarg, &arg_required_operstate);
                         if (r < 0)
                                 return log_error_errno(r, "Invalid operational state range '%s'", optarg);
-
-                        arg_required_operstate = range;
-
                         break;
-                }
 
                 case '4':
                         arg_required_family |= ADDRESS_FAMILY_IPV4;

diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py
index 6d074e67ff53af986c0d032c375837303985381c..7bf2f5014530413d9e01c03dfd2b00046694b6c2 100755 (executable)
--- a/test/test-network/systemd-networkd-tests.py
+++ b/test/test-network/systemd-networkd-tests.py
@@ -1772,6 +1772,8 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities):
         start_networkd()
 
         self.wait_online(['vcan99:carrier', 'vcan98:carrier'])
+        # For can devices, 'carrier' is the default required operational state.
+        self.wait_online(['vcan99', 'vcan98'])
 
         # https://github.com/systemd/systemd/issues/30140
         output = check_output('ip -d link show vcan99')
@@ -1788,6 +1790,8 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities):
         start_networkd()
 
         self.wait_online(['vxcan99:carrier', 'vxcan-peer:carrier'])
+        # For can devices, 'carrier' is the default required operational state.
+        self.wait_online(['vxcan99', 'vxcan-peer'])
 
     @expectedFailureIfModuleIsNotAvailable('wireguard')
     def test_wireguard(self):