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

8 files changed, 113 insertions, 84 deletions

diff --git a/src/libsystemd/sd-network/network-util.c b/src/libsystemd/sd-network/network-util.c
index 2059567ef8..25c6e44a77 100644
--- 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 c47e271a76..6fc6015902 100644
--- 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 a39477e78b..49ed8f59bb 100644
--- 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 05b028dff3..b5b1995361 100644
--- 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 2d5c847a6a..08c7da5699 100644
--- 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 9f0e365c22..b79b898832 100644
--- 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 9213795f54..da5eed5d73 100644
--- 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 5328bba2d8..544c360edd 100644
--- 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;