From 7558f9e717381eef0ddc8ddfb5a754ea4b0f3e6c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 14:14:44 +0900 Subject: [PATCH 01/12] network: use request queue to configure CAN interfaces This also makes SR-IOV configurations are ignored for CAN interfaces, as CAN interfaces seem not to support SR-IOV features. --- src/network/networkd-can.c | 125 +++++++-------------------------- src/network/networkd-can.h | 4 +- src/network/networkd-link.c | 36 +++++----- src/network/networkd-link.h | 1 - src/network/networkd-setlink.c | 37 +++++++++- src/network/networkd-setlink.h | 4 +- 6 files changed, 85 insertions(+), 122 deletions(-) diff --git a/src/network/networkd-can.c b/src/network/networkd-can.c index c2644a1b8e..1fa37a0584 100644 --- a/src/network/networkd-can.c +++ b/src/network/networkd-can.c @@ -3,10 +3,9 @@ #include #include -#include "netlink-util.h" #include "networkd-can.h" #include "networkd-link.h" -#include "networkd-manager.h" +#include "networkd-network.h" #include "networkd-setlink.h" #include "parse-util.h" #include "string-util.h" @@ -53,62 +52,25 @@ int config_parse_can_bitrate( return 0; } -static int link_set_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - int r; - - assert(link); - - if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) - return 1; - - r = sd_netlink_message_get_errno(m); - if (r < 0 && r != -EEXIST) { - log_link_message_warning_errno(link, m, r, "Failed to configure CAN link"); - link_enter_failed(link); - return 1; - } - - log_link_debug(link, "Link set"); - - r = link_request_to_activate(link); - if (r < 0) { - link_enter_failed(link); - return 1; - } - - link->can_configured = true; - link_check_ready(link); - - return 1; -} - -static int link_set_can(Link *link) { - _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; +int can_set_netlink_message(Link *link, sd_netlink_message *m) { struct can_ctrlmode cm = {}; int r; assert(link); assert(link->network); - assert(link->manager); - assert(link->manager->rtnl); - - log_link_debug(link, "Configuring CAN link."); - - r = sd_rtnl_message_new_link(link->manager->rtnl, &m, RTM_NEWLINK, link->ifindex); - if (r < 0) - return log_link_error_errno(link, r, "Failed to allocate netlink message: %m"); + assert(m); r = sd_netlink_message_set_flags(m, NLM_F_REQUEST | NLM_F_ACK); if (r < 0) - return log_link_error_errno(link, r, "Could not set netlink flags: %m"); + return log_link_debug_errno(link, r, "Could not set netlink flags: %m"); r = sd_netlink_message_open_container(m, IFLA_LINKINFO); if (r < 0) - return log_link_error_errno(link, r, "Failed to open netlink container: %m"); + return log_link_debug_errno(link, r, "Failed to open IFLA_LINKINFO container: %m"); r = sd_netlink_message_open_container_union(m, IFLA_INFO_DATA, link->kind); if (r < 0) - return log_link_error_errno(link, r, "Could not append IFLA_INFO_DATA attribute: %m"); + return log_link_debug_errno(link, r, "Could not open IFLA_INFO_DATA container: %m"); if (link->network->can_bitrate > 0 || link->network->can_sample_point > 0) { struct can_bittiming bt = { @@ -124,7 +86,7 @@ static int link_set_can(Link *link) { r = sd_netlink_message_append_data(m, IFLA_CAN_BITTIMING, &bt, sizeof(bt)); if (r < 0) - return log_link_error_errno(link, r, "Could not append IFLA_CAN_BITTIMING attribute: %m"); + return log_link_debug_errno(link, r, "Could not append IFLA_CAN_BITTIMING attribute: %m"); } if (link->network->can_data_bitrate > 0 || link->network->can_data_sample_point > 0) { @@ -141,19 +103,7 @@ static int link_set_can(Link *link) { r = sd_netlink_message_append_data(m, IFLA_CAN_DATA_BITTIMING, &bt, sizeof(bt)); if (r < 0) - return log_link_error_errno(link, r, "Could not append IFLA_CAN_DATA_BITTIMING attribute: %m"); - } - - if (link->network->can_fd_mode >= 0) { - cm.mask |= CAN_CTRLMODE_FD; - SET_FLAG(cm.flags, CAN_CTRLMODE_FD, link->network->can_fd_mode); - log_link_debug(link, "Setting FD mode to '%s'.", yes_no(link->network->can_fd_mode)); - } - - if (link->network->can_non_iso >= 0) { - cm.mask |= CAN_CTRLMODE_FD_NON_ISO; - SET_FLAG(cm.flags, CAN_CTRLMODE_FD_NON_ISO, link->network->can_non_iso); - log_link_debug(link, "Setting FD non-ISO mode to '%s'.", yes_no(link->network->can_non_iso)); + return log_link_debug_errno(link, r, "Could not append IFLA_CAN_DATA_BITTIMING attribute: %m"); } if (link->network->can_restart_us > 0) { @@ -168,13 +118,25 @@ static int link_set_can(Link *link) { format_timespan(time_string, FORMAT_TIMESPAN_MAX, restart_ms * 1000, MSEC_PER_SEC); if (restart_ms > UINT32_MAX) - return log_link_error_errno(link, SYNTHETIC_ERRNO(ERANGE), "restart timeout (%s) too big.", time_string); + return log_link_debug_errno(link, SYNTHETIC_ERRNO(ERANGE), "restart timeout (%s) too big.", time_string); log_link_debug(link, "Setting restart = %s", time_string); r = sd_netlink_message_append_u32(m, IFLA_CAN_RESTART_MS, restart_ms); if (r < 0) - return log_link_error_errno(link, r, "Could not append IFLA_CAN_RESTART_MS attribute: %m"); + return log_link_debug_errno(link, r, "Could not append IFLA_CAN_RESTART_MS attribute: %m"); + } + + if (link->network->can_fd_mode >= 0) { + cm.mask |= CAN_CTRLMODE_FD; + SET_FLAG(cm.flags, CAN_CTRLMODE_FD, link->network->can_fd_mode); + log_link_debug(link, "Setting FD mode to '%s'.", yes_no(link->network->can_fd_mode)); + } + + if (link->network->can_non_iso >= 0) { + cm.mask |= CAN_CTRLMODE_FD_NON_ISO; + SET_FLAG(cm.flags, CAN_CTRLMODE_FD_NON_ISO, link->network->can_non_iso); + log_link_debug(link, "Setting FD non-ISO mode to '%s'.", yes_no(link->network->can_non_iso)); } if (link->network->can_triple_sampling >= 0) { @@ -198,60 +160,25 @@ static int link_set_can(Link *link) { if (cm.mask != 0) { r = sd_netlink_message_append_data(m, IFLA_CAN_CTRLMODE, &cm, sizeof(cm)); if (r < 0) - return log_link_error_errno(link, r, "Could not append IFLA_CAN_CTRLMODE attribute: %m"); + return log_link_debug_errno(link, r, "Could not append IFLA_CAN_CTRLMODE attribute: %m"); } if (link->network->can_termination >= 0) { - log_link_debug(link, "Setting can-termination to '%s'.", yes_no(link->network->can_termination)); r = sd_netlink_message_append_u16(m, IFLA_CAN_TERMINATION, link->network->can_termination ? CAN_TERMINATION_OHM_VALUE : 0); if (r < 0) - return log_link_error_errno(link, r, "Could not append IFLA_CAN_TERMINATION attribute: %m"); - + return log_link_debug_errno(link, r, "Could not append IFLA_CAN_TERMINATION attribute: %m"); } r = sd_netlink_message_close_container(m); if (r < 0) - return log_link_error_errno(link, r, "Failed to close netlink container: %m"); + return log_link_debug_errno(link, r, "Failed to close IFLA_INFO_DATA container: %m"); r = sd_netlink_message_close_container(m); if (r < 0) - return log_link_error_errno(link, r, "Failed to close netlink container: %m"); - - r = netlink_call_async(link->manager->rtnl, NULL, m, link_set_handler, - link_netlink_destroy_callback, link); - if (r < 0) - return log_link_error_errno(link, r, "Could not send rtnetlink message: %m"); - - link_ref(link); - - return 0; -} - -int link_configure_can(Link *link) { - int r; - - link_set_state(link, LINK_STATE_CONFIGURING); - - if (streq_ptr(link->kind, "can")) { - /* The CAN interface must be down to configure bitrate, etc... */ - if (link->flags & IFF_UP) { - r = link_down(link); - if (r < 0) - return r; - } - - return link_set_can(link); - } - - r = link_request_to_activate(link); - if (r < 0) - return r; - - link->can_configured = true; - link_check_ready(link); + return log_link_debug_errno(link, r, "Failed to close IFLA_LINKINFO container: %m"); return 0; } diff --git a/src/network/networkd-can.h b/src/network/networkd-can.h index 7a2705bf9a..781494ed3c 100644 --- a/src/network/networkd-can.h +++ b/src/network/networkd-can.h @@ -1,10 +1,12 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "sd-netlink.h" + #include "conf-parser.h" typedef struct Link Link; -int link_configure_can(Link *link); +int can_set_netlink_message(Link *link, sd_netlink_message *m); CONFIG_PARSER_PROTOTYPE(config_parse_can_bitrate); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 8f70d5da3a..56eacdd0a1 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -411,14 +411,8 @@ void link_check_ready(Link *link) { if (!link->network) return (void) log_link_debug(link, "%s(): link is unmanaged.", __func__); - if (link->iftype == ARPHRD_CAN) { - /* let's shortcut things for CAN which doesn't need most of checks below. */ - if (!link->can_configured) - return (void) log_link_debug(link, "%s(): CAN device is not configured.", __func__); - - link_set_state(link, LINK_STATE_CONFIGURED); - return; - } + if (!link->tc_configured) + return (void) log_link_debug(link, "%s(): traffic controls are not configured.", __func__); if (link->set_link_messages > 0) return (void) log_link_debug(link, "%s(): link layer is configuring.", __func__); @@ -426,6 +420,12 @@ void link_check_ready(Link *link) { if (!link->activated) return (void) log_link_debug(link, "%s(): link is not activated.", __func__); + if (link->iftype == ARPHRD_CAN) { + /* let's shortcut things for CAN which doesn't need most of checks below. */ + link_set_state(link, LINK_STATE_CONFIGURED); + return; + } + if (!link->static_addresses_configured) return (void) log_link_debug(link, "%s(): static addresses are not configured.", __func__); @@ -461,9 +461,6 @@ void link_check_ready(Link *link) { if (!link->static_routing_policy_rules_configured) return (void) log_link_debug(link, "%s(): static routing policy rules are not configured.", __func__); - if (!link->tc_configured) - return (void) log_link_debug(link, "%s(): traffic controls are not configured.", __func__); - if (!link->sr_iov_configured) return (void) log_link_debug(link, "%s(): SR-IOV is not configured.", __func__); @@ -1028,14 +1025,19 @@ static int link_configure(Link *link) { if (r < 0) return r; + if (link->iftype == ARPHRD_CAN) { + /* let's shortcut things for CAN which doesn't need most of what's done below. */ + r = link_request_to_set_can(link); + if (r < 0) + return r; + + return link_request_to_activate(link); + } + r = link_configure_sr_iov(link); if (r < 0) return r; - if (link->iftype == ARPHRD_CAN) - /* let's shortcut things for CAN which doesn't need most of what's done below. */ - return link_configure_can(link); - r = link_set_sysctl(link); if (r < 0) return r; @@ -1588,10 +1590,6 @@ static int link_admin_state_down(Link *link) { return 0; if (link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_UP) { - if (streq_ptr(link->kind, "can") && !link->can_configured) - /* CAN device needs to be down on configure. */ - return 0; - log_link_info(link, "ActivationPolicy is \"always-on\", forcing link up"); return link_up(link); } diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index a3cc829ca6..aa30019a13 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -136,7 +136,6 @@ typedef struct Link { bool static_routing_policy_rules_configured:1; bool tc_configured:1; bool sr_iov_configured:1; - bool can_configured:1; bool activated:1; bool master_set:1; bool stacked_netdevs_created:1; diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index e9269d36ef..a66033b2a7 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -2,9 +2,11 @@ #include #include +#include #include "missing_network.h" #include "netlink-util.h" +#include "networkd-can.h" #include "networkd-link.h" #include "networkd-manager.h" #include "networkd-queue.h" @@ -16,6 +18,7 @@ static const char *const set_link_operation_table[_SET_LINK_OPERATION_MAX] = { [SET_LINK_BOND] = "bond configurations", [SET_LINK_BRIDGE] = "bridge configurations", [SET_LINK_BRIDGE_VLAN] = "bridge VLAN configurations", + [SET_LINK_CAN] = "CAN interface configurations", [SET_LINK_FLAGS] = "link flags", [SET_LINK_GROUP] = "interface group", [SET_LINK_MAC] = "MAC address", @@ -111,6 +114,10 @@ static int link_set_bridge_vlan_handler(sd_netlink *rtnl, sd_netlink_message *m, return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE_VLAN, true, NULL); } +static int link_set_can_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { + return set_link_handler_internal(rtnl, m, link, SET_LINK_CAN, false, NULL); +} + static int link_set_flags_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, true, get_link_default_handler); } @@ -161,7 +168,7 @@ static int link_configure( log_link_debug(link, "Setting %s", set_link_operation_to_string(op)); - if (op == SET_LINK_BOND) { + if (IN_SET(op, SET_LINK_BOND, SET_LINK_CAN)) { r = sd_rtnl_message_new_link(link->manager->rtnl, &req, RTM_NEWLINK, link->master_ifindex); if (r < 0) return log_link_debug_errno(link, r, "Could not allocate RTM_NEWLINK message: %m"); @@ -349,6 +356,11 @@ static int link_configure( return log_link_debug_errno(link, r, "Could not close IFLA_AF_SPEC container: %m"); break; + case SET_LINK_CAN: + r = can_set_netlink_message(link, req); + if (r < 0) + return r; + break; case SET_LINK_FLAGS: { unsigned ifi_change = 0, ifi_flags = 0; @@ -442,6 +454,16 @@ static bool link_is_ready_to_call_set_link(Request *req) { if (!link->master_set) return false; break; + case SET_LINK_CAN: + if (FLAGS_SET(link->flags, IFF_UP)) { + /* The CAN interface must be down to configure bitrate, etc... */ + r = link_down(link); + if (r < 0) { + link_enter_failed(link); + return false; + } + } + break; case SET_LINK_MASTER: { uint32_t m = 0; @@ -599,6 +621,19 @@ int link_request_to_set_bridge_vlan(Link *link) { return link_request_set_link(link, SET_LINK_BRIDGE_VLAN, link_set_bridge_vlan_handler, NULL); } +int link_request_to_set_can(Link *link) { + assert(link); + assert(link->network); + + if (link->iftype != ARPHRD_CAN) + return 0; + + if (!streq_ptr(link->kind, "can")) + return 0; + + return link_request_set_link(link, SET_LINK_CAN, link_set_can_handler, NULL); +} + int link_request_to_set_flags(Link *link) { assert(link); assert(link->network); diff --git a/src/network/networkd-setlink.h b/src/network/networkd-setlink.h index 463a2d114f..eb2633618b 100644 --- a/src/network/networkd-setlink.h +++ b/src/network/networkd-setlink.h @@ -10,7 +10,8 @@ typedef enum SetLinkOperation { SET_LINK_ADDRESS_GENERATION_MODE, /* Setting IPv6LL address generation mode. */ SET_LINK_BOND, /* Setting bond configs. */ SET_LINK_BRIDGE, /* Setting bridge configs. */ - SET_LINK_BRIDGE_VLAN, /* Setting bridge vlan configs. */ + SET_LINK_BRIDGE_VLAN, /* Setting bridge VLAN configs. */ + SET_LINK_CAN, /* Setting CAN interface configs. */ SET_LINK_FLAGS, /* Setting IFF_NOARP or friends. */ SET_LINK_GROUP, /* Setting interface group. */ SET_LINK_MAC, /* Setting MAC address. */ @@ -24,6 +25,7 @@ int link_request_to_set_addrgen_mode(Link *link); int link_request_to_set_bond(Link *link); int link_request_to_set_bridge(Link *link); int link_request_to_set_bridge_vlan(Link *link); +int link_request_to_set_can(Link *link); int link_request_to_set_flags(Link *link); int link_request_to_set_group(Link *link); int link_request_to_set_mac(Link *link); From 717ba5fc90159a09d27d2893f12984ee647e11ab Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 14:46:58 +0900 Subject: [PATCH 02/12] network: read the minimum and maximum MTU of the interface, and adjust requested MTU based on these values --- src/network/networkd-link.c | 28 +++++++++++++++++++++++----- src/network/networkd-link.h | 4 +++- src/network/networkd-setlink.c | 34 ++++++++++++++++++++++++++++------ 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 56eacdd0a1..71feb057eb 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2012,7 +2012,7 @@ static int link_update_hardware_address(Link *link, sd_netlink_message *message) } static int link_update_mtu(Link *link, sd_netlink_message *message) { - uint32_t mtu; + uint32_t mtu, min_mtu = 0, max_mtu = UINT32_MAX; int r; assert(link); @@ -2024,16 +2024,34 @@ static int link_update_mtu(Link *link, sd_netlink_message *message) { if (r < 0) return log_link_debug_errno(link, r, "rtnl: failed to read MTU in RTM_NEWLINK message: %m"); - if (mtu == 0 || link->mtu == mtu) + r = sd_netlink_message_read_u32(message, IFLA_MIN_MTU, &min_mtu); + if (r < 0 && r != -ENODATA) + return log_link_debug_errno(link, r, "rtnl: failed to read minimum MTU in RTM_NEWLINK message: %m"); + + r = sd_netlink_message_read_u32(message, IFLA_MAX_MTU, &max_mtu); + if (r < 0 && r != -ENODATA) + return log_link_debug_errno(link, r, "rtnl: failed to read maximum MTU in RTM_NEWLINK message: %m"); + + if (mtu == 0) return 0; + if (max_mtu == 0) + max_mtu = UINT32_MAX; + + link->min_mtu = min_mtu; + link->max_mtu = max_mtu; + if (link->original_mtu == 0) { link->original_mtu = mtu; - log_link_debug(link, "Saved original MTU: %" PRIu32, link->original_mtu); + log_link_debug(link, "Saved original MTU %" PRIu32" (min: %"PRIu32", max: %"PRIu32")", + link->original_mtu, link->min_mtu, link->max_mtu); } - if (link->mtu != 0) - log_link_debug(link, "MTU is changed: %"PRIu32" → %"PRIu32, link->mtu, mtu); + if (link->mtu == mtu) + return 0; + + log_link_debug(link, "MTU is changed: %"PRIu32" → %"PRIu32" (min: %"PRIu32", max: %"PRIu32")", + link->mtu, mtu, link->min_mtu, link->max_mtu); link->mtu = mtu; diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index aa30019a13..647c60e684 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -58,6 +58,9 @@ typedef struct Link { struct ether_addr permanent_mac; struct in6_addr ipv6ll_address; uint32_t mtu; + uint32_t min_mtu; + uint32_t max_mtu; + uint32_t original_mtu; sd_device *sd_device; char *driver; @@ -114,7 +117,6 @@ typedef struct Link { Address *dhcp_address, *dhcp_address_old; Set *dhcp_routes, *dhcp_routes_old; char *lease_file; - uint32_t original_mtu; unsigned dhcp4_messages; sd_ipv4acd *dhcp_acd; bool dhcp4_route_failed:1; diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index a66033b2a7..6207bcf23b 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -677,16 +677,38 @@ int link_request_to_set_master(Link *link) { int link_request_to_set_mtu(Link *link, uint32_t mtu) { Request *req = NULL; /* avoid false maybe-uninitialized warning */ + const char *origin; + uint32_t min_mtu; int r; assert(link); + assert(link->network); - /* IPv6 protocol requires a minimum MTU of IPV6_MTU_MIN(1280) bytes on the interface. Bump up - * MTU bytes to IPV6_MTU_MIN. */ - if (mtu < IPV6_MIN_MTU && link_ipv6_enabled(link)) { - log_link_warning(link, "Bumping MTU to " STRINGIFY(IPV6_MIN_MTU) ", as IPv6 is enabled " - "and requires a minimum MTU of " STRINGIFY(IPV6_MIN_MTU) " bytes"); - mtu = IPV6_MIN_MTU; + min_mtu = link->min_mtu; + origin = "the minimum MTU of the interface"; + if (link_ipv6_enabled(link)) { + /* IPv6 protocol requires a minimum MTU of IPV6_MTU_MIN(1280) bytes on the interface. Bump up + * MTU bytes to IPV6_MTU_MIN. */ + if (min_mtu < IPV6_MIN_MTU) { + min_mtu = IPV6_MIN_MTU; + origin = "the minimum IPv6 MTU"; + } + if (min_mtu < link->network->ipv6_mtu) { + min_mtu = link->network->ipv6_mtu; + origin = "the requested IPv6 MTU in IPv6MTUBytes="; + } + } + + if (mtu < min_mtu) { + log_link_warning(link, "Bumping the requested MTU %"PRIu32" to %s (%"PRIu32")", + mtu, origin, min_mtu); + mtu = min_mtu; + } + + if (mtu > link->max_mtu) { + log_link_warning(link, "Reducing the requested MTU %"PRIu32" to the interface's maximum MTU %"PRIu32".", + mtu, link->max_mtu); + mtu = link->max_mtu; } if (link->mtu == mtu) From e56e1a15b50e55789347d420facb004baaa9b4b6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 15:18:43 +0900 Subject: [PATCH 03/12] network: also adjust IPv6 MTU by the maximum MTU of the interface --- src/network/networkd-sysctl.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-sysctl.c b/src/network/networkd-sysctl.c index ee5fe5f93d..a67e10e812 100644 --- a/src/network/networkd-sysctl.c +++ b/src/network/networkd-sysctl.c @@ -184,6 +184,8 @@ static int link_set_ipv6_proxy_ndp(Link *link) { } int link_set_ipv6_mtu(Link *link) { + uint32_t mtu; + assert(link); /* Make this a NOP if IPv6 is not available */ @@ -199,7 +201,14 @@ int link_set_ipv6_mtu(Link *link) { if (link->network->ipv6_mtu == 0) return 0; - return sysctl_write_ip_property_uint32(AF_INET6, link->ifname, "mtu", link->network->ipv6_mtu); + mtu = link->network->ipv6_mtu; + if (mtu > link->max_mtu) { + log_link_warning(link, "Reducing requested IPv6 MTU %"PRIu32" to the interface's maximum MTU %"PRIu32".", + mtu, link->max_mtu); + mtu = link->max_mtu; + } + + return sysctl_write_ip_property_uint32(AF_INET6, link->ifname, "mtu", mtu); } static int link_set_ipv4_accept_local(Link *link) { From e95c06c97783a7c09e8fe4c429c54295c97d654a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 20:06:12 +0900 Subject: [PATCH 04/12] network: route: update error message --- src/network/networkd-route.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index c76cdbedee..721d49af75 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1451,7 +1451,7 @@ int route_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Li r = sd_netlink_message_get_errno(m); if (r < 0 && r != -EEXIST) { - log_link_message_warning_errno(link, m, r, "Could not set route with gateway"); + log_link_message_warning_errno(link, m, r, "Could not set route"); link_enter_failed(link); return 0; } From da4929eeb9df138fafcec552dd76c3426052567b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 7 Jun 2021 15:54:48 +0900 Subject: [PATCH 05/12] network: do not drop requests on carrier lost On carrier lost, then all requests which require carrier will not be processed. And they will be processed when the interface gained its carrier again. So, it is not necessary to drop requests here. --- src/network/networkd-link.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 71feb057eb..d970cd7a15 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1522,8 +1522,6 @@ static int link_carrier_lost(Link *link) { return r; } - link_drop_requests(link); - r = link_drop_config(link); if (r < 0) return r; From 899b0e5eec5196ecc13e19ea90a247633bfef3d8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 15:24:43 +0900 Subject: [PATCH 06/12] network: apply activation policy only when it is once activated Otherwise, e.g., if activation policy is always-up is set for bond slave, then setting bond configuration may fail. --- src/network/networkd-link.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index d970cd7a15..a894ef6e3b 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1567,7 +1567,7 @@ static int link_admin_state_up(Link *link) { if (!link->network) return 0; - if (link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_DOWN) { + if (link->activated && link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_DOWN) { log_link_info(link, "ActivationPolicy is \"always-off\", forcing link down"); return link_down(link); } @@ -1587,7 +1587,7 @@ static int link_admin_state_down(Link *link) { if (!link->network) return 0; - if (link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_UP) { + if (link->activated && link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_UP) { log_link_info(link, "ActivationPolicy is \"always-on\", forcing link up"); return link_up(link); } From ad82f0c32b0536a0cc7383a30900dd7652151d4c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 15:32:24 +0900 Subject: [PATCH 07/12] network: add brief comments about bound_to and bound_by list --- src/network/networkd-link.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index a894ef6e3b..5bf81a2b63 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -692,6 +692,9 @@ int link_handle_bound_to_list(Link *link) { assert(link); + /* If at least one interface in bound_to_links has carrier, then make this interface up. + * If all interfaces in bound_to_links do not, then make this interface down. */ + if (hashmap_isempty(link->bound_to_links)) return 0; @@ -718,6 +721,8 @@ static int link_handle_bound_by_list(Link *link) { assert(link); + /* Update up or down state of interfaces which depend on this interface's carrier state. */ + if (hashmap_isempty(link->bound_by_links)) return 0; From aa9117c0c4e2e9c25395d1aca1232870f7dca034 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 16:36:54 +0900 Subject: [PATCH 08/12] network: handle bound_by list even if IgnoreCarrierLoss=yes --- src/network/networkd-link.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 5bf81a2b63..2065cfd529 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1483,9 +1483,13 @@ static int link_carrier_gained(Link *link) { assert(link); + r = link_handle_bound_by_list(link); + if (r < 0) + return r; + if (link->iftype == ARPHRD_CAN) /* let's shortcut things for CAN which doesn't need most of what's done below. */ - return link_handle_bound_by_list(link); + return 0; r = wifi_get_info(link); if (r < 0) @@ -1506,7 +1510,7 @@ static int link_carrier_gained(Link *link) { return r; } - return link_handle_bound_by_list(link); + return 0; } static int link_carrier_lost(Link *link) { @@ -1514,12 +1518,16 @@ static int link_carrier_lost(Link *link) { assert(link); - if (link->network && link->network->ignore_carrier_loss) - return 0; + r = link_handle_bound_by_list(link); + if (r < 0) + return r; if (link->iftype == ARPHRD_CAN) /* let's shortcut things for CAN which doesn't need most of what's done below. */ - return link_handle_bound_by_list(link); + return 0; + + if (link->network && link->network->ignore_carrier_loss) + return 0; r = link_stop_engines(link, false); if (r < 0) { @@ -1538,7 +1546,7 @@ static int link_carrier_lost(Link *link) { return r; } - return link_handle_bound_by_list(link); + return 0; } int link_carrier_reset(Link *link) { From 68f5206349b90f4c33537751d0fc341ab64bb3d9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 16:59:41 +0900 Subject: [PATCH 09/12] network: use request queue to handle bound_to list --- src/network/networkd-link.c | 4 +- src/network/networkd-queue.c | 11 +++++- src/network/networkd-queue.h | 1 + src/network/networkd-setlink.c | 67 ++++++++++++++++++++++++++++++++-- src/network/networkd-setlink.h | 3 ++ 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 2065cfd529..d49ae05623 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -708,9 +708,9 @@ int link_handle_bound_to_list(Link *link) { } if (!required_up && link_is_up) - return link_down(link); + return link_request_to_bring_up_or_down(link, /* up = */ false); if (required_up && !link_is_up) - return link_up(link); + return link_request_to_bring_up_or_down(link, /* up = */ true); return 0; } diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c index acda4994f0..dfb4a2bf12 100644 --- a/src/network/networkd-queue.c +++ b/src/network/networkd-queue.c @@ -49,6 +49,7 @@ static void request_free_object(RequestType type, void *object) { routing_policy_rule_free(object); break; case REQUEST_TYPE_SET_LINK: + case REQUEST_TYPE_UP_DOWN: break; default: assert_not_reached("invalid request type."); @@ -125,6 +126,8 @@ static void request_hash_func(const Request *req, struct siphash *state) { case REQUEST_TYPE_SET_LINK: siphash24_compress(&req->set_link_operation, sizeof(req->set_link_operation), state); break; + case REQUEST_TYPE_UP_DOWN: + break; default: assert_not_reached("invalid request type."); } @@ -170,6 +173,8 @@ static int request_compare_func(const struct Request *a, const struct Request *b return routing_policy_rule_compare_func(a->rule, b->rule); case REQUEST_TYPE_SET_LINK: return CMP(a->set_link_operation, b->set_link_operation); + case REQUEST_TYPE_UP_DOWN: + return 0; default: assert_not_reached("invalid request type."); } @@ -201,7 +206,8 @@ int link_queue_request( assert(IN_SET(type, REQUEST_TYPE_ACTIVATE_LINK, REQUEST_TYPE_DHCP_SERVER, - REQUEST_TYPE_SET_LINK) || + REQUEST_TYPE_SET_LINK, + REQUEST_TYPE_UP_DOWN) || object); assert(type == REQUEST_TYPE_DHCP_SERVER || netlink_handler); @@ -296,6 +302,9 @@ int manager_process_requests(sd_event_source *s, void *userdata) { case REQUEST_TYPE_SET_LINK: r = request_process_set_link(req); break; + case REQUEST_TYPE_UP_DOWN: + r = request_process_link_up_or_down(req); + break; default: return -EINVAL; } diff --git a/src/network/networkd-queue.h b/src/network/networkd-queue.h index 4fb49c48bc..c3d7f05079 100644 --- a/src/network/networkd-queue.h +++ b/src/network/networkd-queue.h @@ -35,6 +35,7 @@ typedef enum RequestType { REQUEST_TYPE_ROUTE, REQUEST_TYPE_ROUTING_POLICY_RULE, REQUEST_TYPE_SET_LINK, + REQUEST_TYPE_UP_DOWN, _REQUEST_TYPE_MAX, _REQUEST_TYPE_INVALID = -EINVAL, } RequestType; diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 6207bcf23b..a5aeb7636a 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -802,6 +802,10 @@ static int link_down_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link return link_up_or_down_handler_internal(rtnl, m, link, false, false); } +static const char *up_or_down(bool up) { + return up ? "up" : "down"; +} + static int link_up_or_down(Link *link, bool up, link_netlink_message_handler_t callback) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; int r; @@ -811,7 +815,7 @@ static int link_up_or_down(Link *link, bool up, link_netlink_message_handler_t c assert(link->manager->rtnl); assert(callback); - log_link_debug(link, "Bringing link %s", up ? "up" : "down"); + log_link_debug(link, "Bringing link %s", up_or_down(up)); r = sd_rtnl_message_new_link(link->manager->rtnl, &req, RTM_SETLINK, link->ifindex); if (r < 0) @@ -869,7 +873,7 @@ int request_process_activation(Request *req) { r = link_up_or_down(link, up, req->netlink_handler); if (r < 0) - return log_link_error_errno(link, r, "Failed to bring %s: %m", up ? "up" : "down"); + return log_link_error_errno(link, r, "Failed to bring %s: %m", up_or_down(up)); return 1; } @@ -884,7 +888,6 @@ int link_request_to_activate(Link *link) { switch (link->network->activation_policy) { case ACTIVATION_POLICY_BOUND: - /* FIXME: also use request queue to handle the list. */ r = link_handle_bound_to_list(link); if (r < 0) return r; @@ -917,3 +920,61 @@ int link_request_to_activate(Link *link) { log_link_debug(link, "Requested to activate link"); return 0; } + +static bool link_is_ready_to_bring_up_or_down(Link *link) { + assert(link); + + if (link->state == LINK_STATE_UNMANAGED) + return true; + + if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) + return false; + + if (link->set_link_messages > 0) + return false; + + if (!link->activated) + return false; + + return true; +} + +int request_process_link_up_or_down(Request *req) { + Link *link; + bool up; + int r; + + assert(req); + assert(req->link); + assert(req->type == REQUEST_TYPE_UP_DOWN); + + link = req->link; + up = PTR_TO_INT(req->userdata); + + if (!link_is_ready_to_bring_up_or_down(link)) + return 0; + + r = link_up_or_down(link, up, req->netlink_handler); + if (r < 0) + return log_link_error_errno(link, r, "Failed to bring %s: %m", up_or_down(up)); + + return 1; +} + +int link_request_to_bring_up_or_down(Link *link, bool up) { + Request *req; + int r; + + assert(link); + + r = link_queue_request(link, REQUEST_TYPE_UP_DOWN, NULL, false, NULL, + up ? link_up_handler : link_down_handler, &req); + if (r < 0) + return log_link_error_errno(link, r, "Failed to request to bring %s link: %m", + up_or_down(up)); + + req->userdata = INT_TO_PTR(up); + + log_link_debug(link, "Requested to bring link %s", up_or_down(up)); + return 0; +} diff --git a/src/network/networkd-setlink.h b/src/network/networkd-setlink.h index eb2633618b..8caee1a1c0 100644 --- a/src/network/networkd-setlink.h +++ b/src/network/networkd-setlink.h @@ -41,3 +41,6 @@ int link_down(Link *link); int request_process_activation(Request *req); int link_request_to_activate(Link *link); + +int request_process_link_up_or_down(Request *req); +int link_request_to_bring_up_or_down(Link *link, bool up); From 712fd5d25afad3d7190115df8d5fe7453d06ad54 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 17:02:00 +0900 Subject: [PATCH 10/12] network: use request queue to handle always-up or -down activation policy --- src/network/networkd-link.c | 8 ++++---- src/network/networkd-setlink.c | 4 ---- src/network/networkd-setlink.h | 1 - 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index d49ae05623..66a3740e93 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1581,8 +1581,8 @@ static int link_admin_state_up(Link *link) { return 0; if (link->activated && link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_DOWN) { - log_link_info(link, "ActivationPolicy is \"always-off\", forcing link down"); - return link_down(link); + log_link_info(link, "ActivationPolicy is \"always-off\", forcing link down."); + return link_request_to_bring_up_or_down(link, /* up = */ false); } /* We set the ipv6 mtu after the device mtu, but the kernel resets @@ -1601,8 +1601,8 @@ static int link_admin_state_down(Link *link) { return 0; if (link->activated && link->network->activation_policy == ACTIVATION_POLICY_ALWAYS_UP) { - log_link_info(link, "ActivationPolicy is \"always-on\", forcing link up"); - return link_up(link); + log_link_info(link, "ActivationPolicy is \"always-on\", forcing link up."); + return link_request_to_bring_up_or_down(link, /* up = */ true); } return 0; diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index a5aeb7636a..bf54b5c8bf 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -835,10 +835,6 @@ static int link_up_or_down(Link *link, bool up, link_netlink_message_handler_t c return 0; } -int link_up(Link *link) { - return link_up_or_down(link, true, link_up_handler); -} - int link_down(Link *link) { return link_up_or_down(link, false, link_down_handler); } diff --git a/src/network/networkd-setlink.h b/src/network/networkd-setlink.h index 8caee1a1c0..0dc7415257 100644 --- a/src/network/networkd-setlink.h +++ b/src/network/networkd-setlink.h @@ -36,7 +36,6 @@ int link_configure_mtu(Link *link); int request_process_set_link(Request *req); -int link_up(Link *link); int link_down(Link *link); int request_process_activation(Request *req); From 2a057a734073ebe7fd7441337059d4d224c9e5e9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 6 Jun 2021 17:09:42 +0900 Subject: [PATCH 11/12] network: make several link settings critical --- src/network/networkd-setlink.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index bf54b5c8bf..00fed02403 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -103,15 +103,15 @@ static int link_set_addrgen_mode_handler(sd_netlink *rtnl, sd_netlink_message *m } static int link_set_bond_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return set_link_handler_internal(rtnl, m, link, SET_LINK_BOND, true, NULL); + return set_link_handler_internal(rtnl, m, link, SET_LINK_BOND, false, NULL); } static int link_set_bridge_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE, true, NULL); + return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE, false, NULL); } static int link_set_bridge_vlan_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE_VLAN, true, NULL); + return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE_VLAN, false, NULL); } static int link_set_can_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { @@ -119,11 +119,11 @@ static int link_set_can_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *l } static int link_set_flags_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, true, get_link_default_handler); + return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, false, get_link_default_handler); } static int link_set_group_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return set_link_handler_internal(rtnl, m, link, SET_LINK_GROUP, true, NULL); + return set_link_handler_internal(rtnl, m, link, SET_LINK_GROUP, false, NULL); } static int link_set_mac_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { @@ -131,7 +131,7 @@ static int link_set_mac_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *l } static int link_set_master_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return set_link_handler_internal(rtnl, m, link, SET_LINK_MASTER, true, get_link_master_handler); + return set_link_handler_internal(rtnl, m, link, SET_LINK_MASTER, false, get_link_master_handler); } static int link_set_mtu_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { From baa95d2274179e680c4731a74f514e2651722ad2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 7 Jun 2021 16:26:10 +0900 Subject: [PATCH 12/12] network: do not process requests which conditionalized with link flags while the flags are updating E.g. nexthop requires IFF_UP flag, but the currently stored flag may be outdated if we called link_down(). This makes such requests pending if at least one of the flags are updating. --- src/network/networkd-bridge-mdb.c | 3 ++ src/network/networkd-dhcp-common.c | 3 ++ src/network/networkd-dhcp-server.c | 3 ++ src/network/networkd-link.c | 9 ++++-- src/network/networkd-link.h | 1 + src/network/networkd-nexthop.c | 4 ++- src/network/networkd-setlink.c | 50 +++++++++++++++++++++++++----- 7 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/network/networkd-bridge-mdb.c b/src/network/networkd-bridge-mdb.c index 3bf1e068f0..faf3dbd0b7 100644 --- a/src/network/networkd-bridge-mdb.c +++ b/src/network/networkd-bridge-mdb.c @@ -258,6 +258,9 @@ static bool bridge_mdb_is_ready_to_configure(Link *link) { if (!IN_SET(master->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) return false; + if (master->set_flags_messages > 0) + return false; + if (!link_has_carrier(master)) return false; diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c index 8357b2b99d..249d780887 100644 --- a/src/network/networkd-dhcp-common.c +++ b/src/network/networkd-dhcp-common.c @@ -116,6 +116,9 @@ static int link_configure_and_start_dhcp_delayed(Link *link) { return r; } + if (link->set_flags_messages > 0) + return 0; + if (!link_has_carrier(link)) return 0; diff --git a/src/network/networkd-dhcp-server.c b/src/network/networkd-dhcp-server.c index 055aa82ba2..8c0cd37f3f 100644 --- a/src/network/networkd-dhcp-server.c +++ b/src/network/networkd-dhcp-server.c @@ -499,6 +499,9 @@ static bool dhcp_server_is_ready_to_configure(Link *link) { if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) return false; + if (link->set_flags_messages > 0) + return false; + if (!link_has_carrier(link)) return false; diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 66a3740e93..6933c8890f 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -148,8 +148,13 @@ bool link_is_ready_to_configure(Link *link, bool allow_unmanaged) { if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) return false; - if (!link_has_carrier(link) && !link->network->configure_without_carrier) - return false; + if (!link->network->configure_without_carrier) { + if (link->set_flags_messages > 0) + return false; + + if (!link_has_carrier(link)) + return false; + } if (link->set_link_messages > 0) return false; diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index 647c60e684..780bdee185 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -98,6 +98,7 @@ typedef struct Link { unsigned tc_messages; unsigned sr_iov_messages; unsigned set_link_messages; + unsigned set_flags_messages; unsigned create_stacked_netdev_messages; unsigned create_stacked_netdev_after_configured_messages; diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 954790f832..4bc29ca057 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -757,9 +757,11 @@ static bool nexthop_is_ready_to_configure(Link *link, const NextHop *nexthop) { } else { Link *l; - /* TODO: fdb nexthop does not require IFF_UP. The condition below needs to be updated + /* TODO: fdb nexthop does not require IFF_UP. The conditions below needs to be updated * when fdb nexthop support is added. See rtm_to_nh_config() in net/ipv4/nexthop.c of * kernel. */ + if (link->set_flags_messages > 0) + return false; if (!FLAGS_SET(link->flags, IFF_UP)) return false; diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 00fed02403..2c3803e02c 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -38,6 +38,15 @@ static int get_link_master_handler(sd_netlink *rtnl, sd_netlink_message *m, Link return 0; } +static int get_link_update_flag_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { + assert(link); + assert(link->set_flags_messages > 0); + + link->set_flags_messages--; + + return get_link_default_handler(rtnl, m, link); +} + static int set_link_handler_internal( sd_netlink *rtnl, sd_netlink_message *m, @@ -56,7 +65,7 @@ static int set_link_handler_internal( link->set_link_messages--; if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) - return 0; + goto on_error; r = sd_netlink_message_get_errno(m); if (r < 0) { @@ -67,7 +76,7 @@ static int set_link_handler_internal( if (!ignore) link_enter_failed(link); - return 0; + goto on_error; } log_link_debug(link, "%s set.", set_link_operation_to_string(op)); @@ -76,7 +85,7 @@ static int set_link_handler_internal( r = link_call_getlink(link, get_link_handler); if (r < 0) { link_enter_failed(link); - return 0; + goto on_error; } } @@ -84,6 +93,14 @@ static int set_link_handler_internal( link_check_ready(link); return 1; + +on_error: + if (op == SET_LINK_FLAGS) { + assert(link->set_flags_messages > 0); + link->set_flags_messages--; + } + + return 0; } static int link_set_addrgen_mode_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { @@ -119,7 +136,7 @@ static int link_set_can_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *l } static int link_set_flags_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, false, get_link_default_handler); + return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, false, get_link_update_flag_handler); } static int link_set_group_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { @@ -455,6 +472,8 @@ static bool link_is_ready_to_call_set_link(Request *req) { return false; break; case SET_LINK_CAN: + /* Do not check link->set_flgas_messages here, as it is ok even if link->flags + * is outdated, and checking the counter causes a deadlock. */ if (FLAGS_SET(link->flags, IFF_UP)) { /* The CAN interface must be down to configure bitrate, etc... */ r = link_down(link); @@ -478,6 +497,8 @@ static bool link_is_ready_to_call_set_link(Request *req) { return false; m = link->network->bond->ifindex; + /* Do not check link->set_flgas_messages here, as it is ok even if link->flags + * is outdated, and checking the counter causes a deadlock. */ if (FLAGS_SET(link->flags, IFF_UP)) { /* link must be down when joining to bond master. */ r = link_down(link); @@ -523,6 +544,9 @@ int request_process_set_link(Request *req) { return log_link_error_errno(req->link, r, "Failed to set %s: %m", set_link_operation_to_string(req->set_link_operation)); + if (req->set_link_operation == SET_LINK_FLAGS) + req->link->set_flags_messages++; + return 1; } @@ -770,7 +794,7 @@ static int link_up_or_down_handler_internal(sd_netlink *rtnl, sd_netlink_message assert(link); if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) - return 0; + goto on_error; r = sd_netlink_message_get_errno(m); if (r < 0) @@ -778,11 +802,23 @@ static int link_up_or_down_handler_internal(sd_netlink *rtnl, sd_netlink_message "Could not bring up interface, ignoring" : "Could not bring down interface, ignoring"); + r = link_call_getlink(link, get_link_update_flag_handler); + if (r < 0) { + link_enter_failed(link); + goto on_error; + } + if (check_ready) { link->activated = true; link_check_ready(link); } + return 1; + +on_error: + assert(link->set_flags_messages > 0); + link->set_flags_messages--; + return 0; } @@ -906,7 +942,7 @@ int link_request_to_activate(Link *link) { link->activated = false; - r = link_queue_request(link, REQUEST_TYPE_ACTIVATE_LINK, NULL, false, NULL, + r = link_queue_request(link, REQUEST_TYPE_ACTIVATE_LINK, NULL, false, &link->set_flags_messages, up ? link_activate_up_handler : link_activate_down_handler, &req); if (r < 0) return log_link_error_errno(link, r, "Failed to request to activate link: %m"); @@ -963,7 +999,7 @@ int link_request_to_bring_up_or_down(Link *link, bool up) { assert(link); - r = link_queue_request(link, REQUEST_TYPE_UP_DOWN, NULL, false, NULL, + r = link_queue_request(link, REQUEST_TYPE_UP_DOWN, NULL, false, &link->set_flags_messages, up ? link_up_handler : link_down_handler, &req); if (r < 0) return log_link_error_errno(link, r, "Failed to request to bring %s link: %m",