From f0ad7aedd93b9f0b4214ba000d96911aba061683 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 10 Jun 2021 06:00:44 +0900 Subject: [PATCH 01/10] network: use link_get_by_name() --- src/network/networkd-manager-bus.c | 10 ++-------- src/network/networkd-route.c | 8 ++------ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/network/networkd-manager-bus.c b/src/network/networkd-manager-bus.c index 1c6230b3932..4be09d98d84 100644 --- a/src/network/networkd-manager-bus.c +++ b/src/network/networkd-manager-bus.c @@ -14,7 +14,6 @@ #include "networkd-manager-bus.h" #include "networkd-manager.h" #include "path-util.h" -#include "socket-netlink.h" #include "strv.h" #include "user-util.h" @@ -60,19 +59,14 @@ static int method_get_link_by_name(sd_bus_message *message, void *userdata, sd_b _cleanup_free_ char *path = NULL; Manager *manager = userdata; const char *name; - int index, r; Link *link; + int r; r = sd_bus_message_read(message, "s", &name); if (r < 0) return r; - index = resolve_ifname(&manager->rtnl, name); - if (index < 0) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_LINK, "Link %s cannot be resolved", name); - - link = hashmap_get(manager->links, INT_TO_PTR(index)); - if (!link) + if (link_get_by_name(manager, name, &link) < 0) return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_LINK, "Link %s not known", name); r = sd_bus_message_new_method_return(message, &reply); diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 721d49af752..c68382433c6 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -13,7 +13,6 @@ #include "networkd-queue.h" #include "networkd-route.h" #include "parse-util.h" -#include "socket-netlink.h" #include "string-table.h" #include "string-util.h" #include "strv.h" @@ -708,13 +707,10 @@ int link_has_route(Link *link, const Route *route) { Link *l; if (m->ifname) { - r = resolve_interface(&link->manager->rtnl, m->ifname); - if (r < 0) + if (link_get_by_name(link->manager, m->ifname, &l) < 0) return false; - m->ifindex = r; - if (link_get(link->manager, m->ifindex, &l) < 0) - return false; + m->ifindex = l->ifindex; } else l = link; From f6e491547d92e86aa728a3851cc2007db57ea10e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 10 Jun 2021 06:01:44 +0900 Subject: [PATCH 02/10] netlink: move resolve_ifname() or friends to netlink-util.[ch] --- src/libsystemd/sd-netlink/netlink-util.c | 39 +++++++++++++++++++++++ src/libsystemd/sd-netlink/netlink-util.h | 3 ++ src/network/networkctl.c | 10 +++--- src/nspawn/nspawn-network.c | 12 +++---- src/resolve/resolvectl.c | 4 +-- src/shared/socket-netlink.c | 40 +----------------------- src/shared/socket-netlink.h | 6 ---- 7 files changed, 56 insertions(+), 58 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index 868763bb9c0..0650f0b4752 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -6,6 +6,7 @@ #include "memory-util.h" #include "netlink-internal.h" #include "netlink-util.h" +#include "parse-util.h" #include "strv.h" int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { @@ -302,6 +303,44 @@ int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name) { return ret; } +int rtnl_resolve_ifname(sd_netlink **rtnl, const char *name) { + int r; + + /* Like if_nametoindex, but resolves "alternative names" too. */ + + assert(name); + + r = if_nametoindex(name); + if (r > 0) + return r; + + return rtnl_resolve_link_alternative_name(rtnl, name); +} + +int rtnl_resolve_interface(sd_netlink **rtnl, const char *name) { + int r; + + /* Like rtnl_resolve_ifname, but resolves interface numbers too. */ + + assert(name); + + r = parse_ifindex(name); + if (r > 0) + return r; + assert(r < 0); + + return rtnl_resolve_ifname(rtnl, name); +} + +int rtnl_resolve_interface_or_warn(sd_netlink **rtnl, const char *name) { + int r; + + r = rtnl_resolve_interface(rtnl, name); + if (r < 0) + return log_error_errno(r, "Failed to resolve interface \"%s\": %m", name); + return r; +} + int rtnl_get_link_info(sd_netlink **rtnl, int ifindex, unsigned short *ret_iftype, unsigned *ret_flags) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL, *reply = NULL; unsigned short iftype; diff --git a/src/libsystemd/sd-netlink/netlink-util.h b/src/libsystemd/sd-netlink/netlink-util.h index c676c07135e..a4cdaaf8bd4 100644 --- a/src/libsystemd/sd-netlink/netlink-util.h +++ b/src/libsystemd/sd-netlink/netlink-util.h @@ -92,6 +92,9 @@ int rtnl_set_link_alternative_names(sd_netlink **rtnl, int ifindex, char * const int rtnl_set_link_alternative_names_by_ifname(sd_netlink **rtnl, const char *ifname, char * const *alternative_names); int rtnl_delete_link_alternative_names(sd_netlink **rtnl, int ifindex, char * const *alternative_names); int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name); +int rtnl_resolve_ifname(sd_netlink **rtnl, const char *name); +int rtnl_resolve_interface(sd_netlink **rtnl, const char *name); +int rtnl_resolve_interface_or_warn(sd_netlink **rtnl, const char *name); int rtnl_get_link_info(sd_netlink **rtnl, int ifindex, unsigned short *ret_iftype, unsigned *ret_flags); int rtnl_log_parse_error(int r); diff --git a/src/network/networkctl.c b/src/network/networkctl.c index a3ba15672b4..343dea3ed64 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -2665,7 +2665,7 @@ static int link_up_down(int argc, char *argv[], void *userdata) { return log_oom(); for (int i = 1; i < argc; i++) { - index = resolve_interface_or_warn(&rtnl, argv[i]); + index = rtnl_resolve_interface_or_warn(&rtnl, argv[i]); if (index < 0) return index; @@ -2703,7 +2703,7 @@ static int link_delete(int argc, char *argv[], void *userdata) { return log_oom(); for (int i = 1; i < argc; i++) { - index = resolve_interface_or_warn(&rtnl, argv[i]); + index = rtnl_resolve_interface_or_warn(&rtnl, argv[i]); if (index < 0) return index; @@ -2748,7 +2748,7 @@ static int link_renew(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to connect system bus: %m"); for (int i = 1; i < argc; i++) { - index = resolve_interface_or_warn(&rtnl, argv[i]); + index = rtnl_resolve_interface_or_warn(&rtnl, argv[i]); if (index < 0) return index; @@ -2782,7 +2782,7 @@ static int link_force_renew(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to connect system bus: %m"); for (int i = 1; i < argc; i++) { - int index = resolve_interface_or_warn(&rtnl, argv[i]); + int index = rtnl_resolve_interface_or_warn(&rtnl, argv[i]); if (index < 0) return index; @@ -2827,7 +2827,7 @@ static int verb_reconfigure(int argc, char *argv[], void *userdata) { return log_oom(); for (int i = 1; i < argc; i++) { - index = resolve_interface_or_warn(&rtnl, argv[i]); + index = rtnl_resolve_interface_or_warn(&rtnl, argv[i]); if (index < 0) return index; diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index 95e4b0213b5..1f0d2747578 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -272,7 +272,7 @@ int setup_veth(const char *machine_name, if (r < 0) return r; - u = if_nametoindex(n); /* We don't need to use resolve_ifname() here because the + u = if_nametoindex(n); /* We don't need to use rtnl_resolve_ifname() here because the * name we assigned is always the main name. */ if (u == 0) return log_error_errno(errno, "Failed to resolve interface %s: %m", n); @@ -330,7 +330,7 @@ static int join_bridge(sd_netlink *rtnl, const char *veth_name, const char *brid assert(veth_name); assert(bridge_name); - bridge_ifi = resolve_interface(&rtnl, bridge_name); + bridge_ifi = rtnl_resolve_interface(&rtnl, bridge_name); if (bridge_ifi < 0) return bridge_ifi; @@ -475,7 +475,7 @@ int test_network_interface_initialized(const char *name) { /* udev should be around. */ - ifi = resolve_interface_or_warn(NULL, name); + ifi = rtnl_resolve_interface_or_warn(NULL, name); if (ifi < 0) return ifi; @@ -515,7 +515,7 @@ int move_network_interfaces(int netns_fd, char **ifaces) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; int ifi; - ifi = resolve_interface_or_warn(&rtnl, *i); + ifi = rtnl_resolve_interface_or_warn(&rtnl, *i); if (ifi < 0) return ifi; @@ -554,7 +554,7 @@ int setup_macvlan(const char *machine_name, pid_t pid, char **ifaces) { struct ether_addr mac; int ifi; - ifi = resolve_interface_or_warn(&rtnl, *i); + ifi = rtnl_resolve_interface_or_warn(&rtnl, *i); if (ifi < 0) return ifi; @@ -640,7 +640,7 @@ int setup_ipvlan(const char *machine_name, pid_t pid, char **ifaces) { _cleanup_free_ char *n = NULL, *a = NULL; int ifi; - ifi = resolve_interface_or_warn(&rtnl, *i); + ifi = rtnl_resolve_interface_or_warn(&rtnl, *i); if (ifi < 0) return ifi; diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index e652c82ffcb..23f4ff1077b 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -119,7 +119,7 @@ int ifname_mangle(const char *s) { if (!iface) return log_oom(); - ifi = resolve_interface(NULL, iface); + ifi = rtnl_resolve_interface(NULL, iface); if (ifi < 0) { if (ifi == -ENODEV && arg_ifindex_permissive) { log_debug("Interface '%s' not found, but -f specified, ignoring.", iface); @@ -2018,7 +2018,7 @@ static int verb_status(int argc, char **argv, void *userdata) { STRV_FOREACH(ifname, argv + 1) { int ifindex, q; - ifindex = resolve_interface(&rtnl, *ifname); + ifindex = rtnl_resolve_interface(&rtnl, *ifname); if (ifindex < 0) { log_warning_errno(ifindex, "Failed to resolve interface \"%s\", ignoring: %m", *ifname); continue; diff --git a/src/shared/socket-netlink.c b/src/shared/socket-netlink.c index 4a7007d06e5..0c044378265 100644 --- a/src/shared/socket-netlink.c +++ b/src/shared/socket-netlink.c @@ -16,44 +16,6 @@ #include "socket-util.h" #include "string-util.h" -int resolve_ifname(sd_netlink **rtnl, const char *name) { - int r; - - /* Like if_nametoindex, but resolves "alternative names" too. */ - - assert(name); - - r = if_nametoindex(name); - if (r > 0) - return r; - - return rtnl_resolve_link_alternative_name(rtnl, name); -} - -int resolve_interface(sd_netlink **rtnl, const char *name) { - int r; - - /* Like resolve_ifname, but resolves interface numbers too. */ - - assert(name); - - r = parse_ifindex(name); - if (r > 0) - return r; - assert(r < 0); - - return resolve_ifname(rtnl, name); -} - -int resolve_interface_or_warn(sd_netlink **rtnl, const char *name) { - int r; - - r = resolve_interface(rtnl, name); - if (r < 0) - return log_error_errno(r, "Failed to resolve interface \"%s\": %m", name); - return r; -} - int socket_address_parse(SocketAddress *a, const char *s) { _cleanup_free_ char *n = NULL; char *e; @@ -338,7 +300,7 @@ int in_addr_port_ifindex_name_from_string_auto( return -EINVAL; /* We want to return -EINVAL for syntactically invalid names, * and -ENODEV for valid but nonexistent interfaces. */ - ifindex = resolve_interface(NULL, m + 1); + ifindex = rtnl_resolve_interface(NULL, m + 1); if (ifindex < 0) return ifindex; diff --git a/src/shared/socket-netlink.h b/src/shared/socket-netlink.h index eac599159db..6256a831bac 100644 --- a/src/shared/socket-netlink.h +++ b/src/shared/socket-netlink.h @@ -1,16 +1,10 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once -#include "sd-netlink.h" - #include "in-addr-util.h" #include "macro.h" #include "socket-util.h" -int resolve_ifname(sd_netlink **rtnl, const char *name); -int resolve_interface(sd_netlink **rtnl, const char *name); -int resolve_interface_or_warn(sd_netlink **rtnl, const char *name); - int make_socket_fd(int log_level, const char* address, int type, int flags); int socket_address_parse(SocketAddress *a, const char *s); From 57bd6aa7019734f599e23bdd75d6a75629df3aa6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 10 Jun 2021 18:16:28 +0900 Subject: [PATCH 03/10] netlink: check input name is valid before calling netlink method --- src/libsystemd/sd-netlink/netlink-util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index 0650f0b4752..4012725f4cd 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -274,6 +274,9 @@ int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name) { assert(name); + if (!ifname_valid_full(name, IFNAME_VALID_ALTERNATIVE)) + return -EINVAL; + if (!rtnl) rtnl = &our_rtnl; if (!*rtnl) { From afdf6c3b6040ef43b05428b834f0f302c8ce9a1b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 10 Jun 2021 18:17:47 +0900 Subject: [PATCH 04/10] netlink: make rtnl_resolve_link_alternative_name() optionally return the main interface name --- src/libsystemd/sd-netlink/netlink-util.c | 21 +++++++++++++++------ src/libsystemd/sd-netlink/netlink-util.h | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index 4012725f4cd..1211145fbf8 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -267,13 +267,15 @@ int rtnl_set_link_alternative_names_by_ifname(sd_netlink **rtnl, const char *ifn return 0; } -int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name) { +int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name, char **ret) { _cleanup_(sd_netlink_unrefp) sd_netlink *our_rtnl = NULL; _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL, *reply = NULL; - int r, ret; + int r, ifindex; assert(name); + /* This returns ifindex and the main interface name. */ + if (!ifname_valid_full(name, IFNAME_VALID_ALTERNATIVE)) return -EINVAL; @@ -299,11 +301,18 @@ int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name) { if (r < 0) return r; - r = sd_rtnl_message_link_get_ifindex(reply, &ret); + r = sd_rtnl_message_link_get_ifindex(reply, &ifindex); if (r < 0) return r; - assert(ret > 0); - return ret; + assert(ifindex > 0); + + if (ret) { + r = sd_netlink_message_read_string_strdup(message, IFLA_IFNAME, ret); + if (r < 0) + return r; + } + + return ifindex; } int rtnl_resolve_ifname(sd_netlink **rtnl, const char *name) { @@ -317,7 +326,7 @@ int rtnl_resolve_ifname(sd_netlink **rtnl, const char *name) { if (r > 0) return r; - return rtnl_resolve_link_alternative_name(rtnl, name); + return rtnl_resolve_link_alternative_name(rtnl, name, NULL); } int rtnl_resolve_interface(sd_netlink **rtnl, const char *name) { diff --git a/src/libsystemd/sd-netlink/netlink-util.h b/src/libsystemd/sd-netlink/netlink-util.h index a4cdaaf8bd4..a5d725655dd 100644 --- a/src/libsystemd/sd-netlink/netlink-util.h +++ b/src/libsystemd/sd-netlink/netlink-util.h @@ -91,7 +91,7 @@ int rtnl_get_link_alternative_names(sd_netlink **rtnl, int ifindex, char ***ret) int rtnl_set_link_alternative_names(sd_netlink **rtnl, int ifindex, char * const *alternative_names); int rtnl_set_link_alternative_names_by_ifname(sd_netlink **rtnl, const char *ifname, char * const *alternative_names); int rtnl_delete_link_alternative_names(sd_netlink **rtnl, int ifindex, char * const *alternative_names); -int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name); +int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name, char **ret); int rtnl_resolve_ifname(sd_netlink **rtnl, const char *name); int rtnl_resolve_interface(sd_netlink **rtnl, const char *name); int rtnl_resolve_interface_or_warn(sd_netlink **rtnl, const char *name); From bd44a727f736d21dd47b5dfbdbe79d21acbdc2a4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Jun 2021 23:27:20 +0900 Subject: [PATCH 05/10] sd-device: introduce sd_device_new_from_ifname/ifindex() --- src/libsystemd/libsystemd.sym | 2 + src/libsystemd/sd-device/sd-device.c | 80 ++++++++++++++++++---------- src/systemd/sd-device.h | 2 + 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 1606981e1e1..3730db6eef9 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -759,4 +759,6 @@ global: sd_device_get_usec_initialized; sd_device_trigger_with_uuid; sd_device_get_trigger_uuid; + sd_device_new_from_ifname; + sd_device_new_from_ifindex; } LIBSYSTEMD_248; diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index f31416da530..b5e7a1a44d1 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -14,10 +14,12 @@ #include "dirent-util.h" #include "fd-util.h" #include "fileio.h" +#include "format-util.h" #include "fs-util.h" #include "hashmap.h" #include "id128-util.h" #include "macro.h" +#include "netlink-util.h" #include "parse-util.h" #include "path-util.h" #include "set.h" @@ -245,6 +247,52 @@ _public_ int sd_device_new_from_devnum(sd_device **ret, char type, dev_t devnum) return sd_device_new_from_syspath(ret, syspath); } +static int device_new_from_main_ifname(sd_device **ret, const char *ifname) { + const char *syspath; + + assert(ret); + assert(ifname); + + syspath = strjoina("/sys/class/net/", ifname); + return sd_device_new_from_syspath(ret, syspath); +} + +_public_ int sd_device_new_from_ifname(sd_device **ret, const char *ifname) { + _cleanup_free_ char *main_name = NULL; + int r; + + assert_return(ret, -EINVAL); + assert_return(ifname, -EINVAL); + + r = parse_ifindex(ifname); + if (r > 0) + return sd_device_new_from_ifindex(ret, r); + + if (ifname_valid(ifname)) { + r = device_new_from_main_ifname(ret, ifname); + if (r >= 0) + return r; + } + + r = rtnl_resolve_link_alternative_name(NULL, ifname, &main_name); + if (r < 0) + return r; + + return device_new_from_main_ifname(ret, main_name); +} + +_public_ int sd_device_new_from_ifindex(sd_device **ret, int ifindex) { + char ifname[IF_NAMESIZE + 1]; + + assert_return(ret, -EINVAL); + assert_return(ifindex > 0, -EINVAL); + + if (!format_ifname(ifindex, ifname)) + return -ENODEV; + + return device_new_from_main_ifname(ret, ifname); +} + static int device_strjoin_new( const char *a, const char *b, @@ -643,37 +691,13 @@ _public_ int sd_device_new_from_device_id(sd_device **ret, const char *id) { } case 'n': { - _cleanup_(sd_device_unrefp) sd_device *device = NULL; - _cleanup_close_ int sk = -1; - struct ifreq ifr = {}; int ifindex; - r = ifr.ifr_ifindex = parse_ifindex(id + 1); - if (r < 0) - return r; + ifindex = parse_ifindex(id + 1); + if (ifindex < 0) + return ifindex; - sk = socket_ioctl_fd(); - if (sk < 0) - return sk; - - r = ioctl(sk, SIOCGIFNAME, &ifr); - if (r < 0) - return -errno; - - r = sd_device_new_from_subsystem_sysname(&device, "net", ifr.ifr_name); - if (r < 0) - return r; - - r = sd_device_get_ifindex(device, &ifindex); - if (r < 0) - return r; - - /* this is racey, so we might end up with the wrong device */ - if (ifr.ifr_ifindex != ifindex) - return -ENODEV; - - *ret = TAKE_PTR(device); - return 0; + return sd_device_new_from_ifindex(ret, ifindex); } case '+': { diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index 04599ea3821..7ab0a8334eb 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -62,6 +62,8 @@ int sd_device_new_from_devnum(sd_device **ret, char type, dev_t devnum); int sd_device_new_from_subsystem_sysname(sd_device **ret, const char *subsystem, const char *sysname); int sd_device_new_from_device_id(sd_device **ret, const char *id); int sd_device_new_from_stat_rdev(sd_device **ret, const struct stat *st); +int sd_device_new_from_ifname(sd_device **ret, const char *ifname); +int sd_device_new_from_ifindex(sd_device **ret, int ifindex); int sd_device_get_parent(sd_device *child, sd_device **ret); int sd_device_get_parent_with_subsystem_devtype(sd_device *child, const char *subsystem, const char *devtype, sd_device **ret); From 0ac655a63b55ab6381dd536d821b3505507cf6d7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Jun 2021 23:33:50 +0900 Subject: [PATCH 06/10] tree-wide: use sd_device_new_from_ifindex/ifname() --- src/libsystemd-network/dhcp-identifier.c | 6 ++---- src/network/networkctl.c | 4 +--- src/network/networkd-link.c | 4 +--- src/nspawn/nspawn-network.c | 10 ++-------- 4 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c index 953fef19fa4..c3b47ba3c90 100644 --- a/src/libsystemd-network/dhcp-identifier.c +++ b/src/libsystemd-network/dhcp-identifier.c @@ -167,14 +167,12 @@ int dhcp_identifier_set_iaid( const char *name = NULL; uint64_t id; uint32_t id32; + int r; if (detect_container() <= 0) { /* not in a container, udev will be around */ - char ifindex_str[1 + DECIMAL_STR_MAX(int)]; - int r; - xsprintf(ifindex_str, "n%d", ifindex); - if (sd_device_new_from_device_id(&device, ifindex_str) >= 0) { + if (sd_device_new_from_ifindex(&device, ifindex) >= 0) { r = sd_device_get_is_initialized(device); if (r < 0) return r; diff --git a/src/network/networkctl.c b/src/network/networkctl.c index 343dea3ed64..1c2cb7691a4 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -753,9 +753,7 @@ static int acquire_link_info(sd_bus *bus, sd_netlink *rtnl, char **patterns, Lin links[c].needs_freeing = true; - char devid[2 + DECIMAL_STR_MAX(int)]; - xsprintf(devid, "n%i", links[c].ifindex); - (void) sd_device_new_from_device_id(&links[c].sd_device, devid); + (void) sd_device_new_from_ifindex(&links[c].sd_device, links[c].ifindex); acquire_ether_link_info(&fd, &links[c]); acquire_wlan_link_info(&links[c]); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 911fb9b302c..c3af71b48fc 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1394,7 +1394,6 @@ static int link_initialized(Link *link, sd_device *device) { static int link_check_initialized(Link *link) { _cleanup_(sd_device_unrefp) sd_device *device = NULL; - char ifindex_str[2 + DECIMAL_STR_MAX(int)]; int r; assert(link); @@ -1404,8 +1403,7 @@ static int link_check_initialized(Link *link) { return link_initialized_and_synced(link); /* udev should be around */ - xsprintf(ifindex_str, "n%d", link->ifindex); - r = sd_device_new_from_device_id(&device, ifindex_str); + r = sd_device_new_from_ifindex(&device, link->ifindex); if (r < 0) { log_link_debug_errno(link, r, "Could not find device, waiting for device initialization: %m"); return 0; diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index 1f0d2747578..97e2756658b 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -467,20 +467,14 @@ int remove_bridge(const char *bridge_name) { int test_network_interface_initialized(const char *name) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; - int ifi, r; - char ifi_str[2 + DECIMAL_STR_MAX(int)]; + int r; if (path_is_read_only_fs("/sys")) return 0; /* udev should be around. */ - ifi = rtnl_resolve_interface_or_warn(NULL, name); - if (ifi < 0) - return ifi; - - sprintf(ifi_str, "n%i", ifi); - r = sd_device_new_from_device_id(&d, ifi_str); + r = sd_device_new_from_ifname(&d, name); if (r < 0) return log_error_errno(r, "Failed to get device %s: %m", name); From 27fce94ae6dca53da00595ffb6b1f443c09e7064 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Jun 2021 23:48:50 +0900 Subject: [PATCH 07/10] nspawn: path_is_read_only_fs() may return negative errno And we usually assume /sys is not read only on error. --- src/nspawn/nspawn-network.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index 97e2756658b..023b1e7e1a8 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -469,7 +469,7 @@ int test_network_interface_initialized(const char *name) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; int r; - if (path_is_read_only_fs("/sys")) + if (path_is_read_only_fs("/sys") > 0) return 0; /* udev should be around. */ From 0299deab53d2a087727a5d04c1500c322c48b63e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Jun 2021 23:41:00 +0900 Subject: [PATCH 08/10] sd-dhcp: do not use detect_container() to guess udev is running or not --- src/libsystemd-network/dhcp-identifier.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c index c3b47ba3c90..bffd9968249 100644 --- a/src/libsystemd-network/dhcp-identifier.c +++ b/src/libsystemd-network/dhcp-identifier.c @@ -12,6 +12,7 @@ #include "network-util.h" #include "siphash24.h" #include "sparse-endian.h" +#include "stat-util.h" #include "stdio-util.h" #include "udev-util.h" #include "virt.h" @@ -169,8 +170,8 @@ int dhcp_identifier_set_iaid( uint32_t id32; int r; - if (detect_container() <= 0) { - /* not in a container, udev will be around */ + if (path_is_read_only_fs("/sys") <= 0) { + /* udev should be around */ if (sd_device_new_from_ifindex(&device, ifindex) >= 0) { r = sd_device_get_is_initialized(device); From 8d71f2b3a6c9ef26a3a39fcaa4281776e9420976 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 10 Jun 2021 01:09:09 +0900 Subject: [PATCH 09/10] dhcp: do not use ifindex when generating iaid in tests --- src/libsystemd-network/dhcp-identifier.c | 3 ++- src/libsystemd-network/dhcp-identifier.h | 2 +- src/libsystemd-network/dhcp-internal.h | 4 ++++ src/libsystemd-network/dhcp6-internal.h | 4 ++++ src/libsystemd-network/fuzz-dhcp6-client.c | 1 + src/libsystemd-network/sd-dhcp-client.c | 16 ++++++++++++++-- src/libsystemd-network/sd-dhcp6-client.c | 14 +++++++++++++- src/libsystemd-network/test-dhcp-client.c | 21 +++++++-------------- src/libsystemd-network/test-dhcp6-client.c | 3 ++- 9 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c index bffd9968249..d1ea992d43c 100644 --- a/src/libsystemd-network/dhcp-identifier.c +++ b/src/libsystemd-network/dhcp-identifier.c @@ -161,6 +161,7 @@ int dhcp_identifier_set_iaid( const uint8_t *mac, size_t mac_len, bool legacy_unstable_byteorder, + bool use_mac, void *_id) { /* name is a pointer to memory in the sd_device struct, so must * have the same scope */ @@ -170,7 +171,7 @@ int dhcp_identifier_set_iaid( uint32_t id32; int r; - if (path_is_read_only_fs("/sys") <= 0) { + if (path_is_read_only_fs("/sys") <= 0 && !use_mac) { /* udev should be around */ if (sd_device_new_from_ifindex(&device, ifindex) >= 0) { diff --git a/src/libsystemd-network/dhcp-identifier.h b/src/libsystemd-network/dhcp-identifier.h index 1d9a9c55ba9..6c24af0326a 100644 --- a/src/libsystemd-network/dhcp-identifier.h +++ b/src/libsystemd-network/dhcp-identifier.h @@ -59,4 +59,4 @@ int dhcp_identifier_set_duid_llt(struct duid *duid, usec_t t, const uint8_t *add int dhcp_identifier_set_duid_ll(struct duid *duid, const uint8_t *addr, size_t addr_len, uint16_t arp_type, size_t *len); int dhcp_identifier_set_duid_en(struct duid *duid, size_t *len); int dhcp_identifier_set_duid_uuid(struct duid *duid, size_t *len); -int dhcp_identifier_set_iaid(int ifindex, const uint8_t *mac, size_t mac_len, bool legacy_unstable_byteorder, void *_id); +int dhcp_identifier_set_iaid(int ifindex, const uint8_t *mac, size_t mac_len, bool legacy_unstable_byteorder, bool use_mac, void *_id); diff --git a/src/libsystemd-network/dhcp-internal.h b/src/libsystemd-network/dhcp-internal.h index d8c42757aa6..16999971d2a 100644 --- a/src/libsystemd-network/dhcp-internal.h +++ b/src/libsystemd-network/dhcp-internal.h @@ -30,6 +30,8 @@ typedef struct DHCPServerData { extern const struct hash_ops dhcp_option_hash_ops; +typedef struct sd_dhcp_client sd_dhcp_client; + int dhcp_network_bind_raw_socket(int ifindex, union sockaddr_union *link, uint32_t xid, const uint8_t *mac_addr, size_t mac_addr_len, const uint8_t *bcast_addr, size_t bcast_addr_len, @@ -62,6 +64,8 @@ void dhcp_packet_append_ip_headers(DHCPPacket *packet, be32_t source_addr, int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len, bool checksum, uint16_t port); +void dhcp_client_set_test_mode(sd_dhcp_client *client, bool test_mode); + /* If we are invoking callbacks of a dhcp-client, ensure unreffing the * client from the callback doesn't destroy the object we are working * on */ diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h index 34e1f61cb08..f0f814957f2 100644 --- a/src/libsystemd-network/dhcp6-internal.h +++ b/src/libsystemd-network/dhcp6-internal.h @@ -91,6 +91,8 @@ typedef struct DHCP6IA { LIST_HEAD(DHCP6Address, addresses); } DHCP6IA; +typedef struct sd_dhcp6_client sd_dhcp6_client; + int dhcp6_option_append(uint8_t **buf, size_t *buflen, uint16_t code, size_t optlen, const void *optval); int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, const DHCP6IA *ia); @@ -118,6 +120,8 @@ int dhcp6_message_type_from_string(const char *s) _pure_; const char *dhcp6_message_status_to_string(int s) _const_; int dhcp6_message_status_from_string(const char *s) _pure_; +void dhcp6_client_set_test_mode(sd_dhcp6_client *client, bool test_mode); + #define log_dhcp6_client_errno(client, error, fmt, ...) \ log_interface_prefix_full_errno( \ "DHCPv6 client: ", \ diff --git a/src/libsystemd-network/fuzz-dhcp6-client.c b/src/libsystemd-network/fuzz-dhcp6-client.c index 7ebe01286da..f62ab468df3 100644 --- a/src/libsystemd-network/fuzz-dhcp6-client.c +++ b/src/libsystemd-network/fuzz-dhcp6-client.c @@ -33,6 +33,7 @@ static void fuzz_client(const uint8_t *data, size_t size, bool is_information_re assert_se(sd_dhcp6_client_set_ifindex(client, 42) == 0); assert_se(sd_dhcp6_client_set_local_address(client, &address) >= 0); assert_se(sd_dhcp6_client_set_information_request(client, is_information_request_enabled) == 0); + dhcp6_client_set_test_mode(client, true); assert_se(sd_dhcp6_client_start(client) >= 0); diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index a15ff8af33a..ff021f4eae1 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -117,6 +117,9 @@ struct sd_dhcp_client { sd_dhcp_lease *lease; usec_t start_delay; int ip_service_type; + + /* Ignore ifindex when generating iaid. See dhcp_identifier_set_iaid(). */ + bool test_mode; }; static const uint8_t default_req_opts[] = { @@ -466,7 +469,8 @@ static int dhcp_client_set_iaid_duid_internal( else { r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, - true, + /* legacy_unstable_byteorder = */ true, + /* use_mac = */ client->test_mode, &client->client_id.ns.iaid); if (r < 0) return log_dhcp_client_errno(client, r, "Failed to set IAID: %m"); @@ -555,6 +559,12 @@ int sd_dhcp_client_set_duid_llt( return dhcp_client_set_iaid_duid_internal(client, false, false, 0, DUID_TYPE_LLT, NULL, 0, llt_time); } +void dhcp_client_set_test_mode(sd_dhcp_client *client, bool test_mode) { + assert(client); + + client->test_mode = test_mode; +} + int sd_dhcp_client_set_hostname( sd_dhcp_client *client, const char *hostname) { @@ -860,7 +870,9 @@ static int client_message_init( client->client_id.type = 255; r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, - true, &client->client_id.ns.iaid); + /* legacy_unstable_byteorder = */ true, + /* use_mac = */ client->test_mode, + &client->client_id.ns.iaid); if (r < 0) return r; diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 855a61bb216..e8c47f429a5 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -84,6 +84,9 @@ struct sd_dhcp6_client { usec_t information_refresh_time_usec; OrderedHashmap *extra_options; OrderedHashmap *vendor_options; + + /* Ignore ifindex when generating iaid. See dhcp_identifier_set_iaid(). */ + bool test_mode; }; static const uint16_t default_req_opts[] = { @@ -398,6 +401,12 @@ int sd_dhcp6_client_set_iaid(sd_dhcp6_client *client, uint32_t iaid) { return 0; } +void dhcp6_client_set_test_mode(sd_dhcp6_client *client, bool test_mode) { + assert(client); + + client->test_mode = test_mode; +} + int sd_dhcp6_client_get_iaid(sd_dhcp6_client *client, uint32_t *iaid) { assert_return(client, -EINVAL); assert_return(iaid, -EINVAL); @@ -1073,7 +1082,10 @@ static int client_ensure_iaid(sd_dhcp6_client *client) { if (client->iaid_set) return 0; - r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, true, &iaid); + r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, + /* legacy_unstable_byteorder = */ true, + /* use_mac = */ client->test_mode, + &iaid); if (r < 0) return r; diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c index b464d3d700a..6e0fdcd769a 100644 --- a/src/libsystemd-network/test-dhcp-client.c +++ b/src/libsystemd-network/test-dhcp-client.c @@ -145,20 +145,11 @@ static void test_checksum(void) { static void test_dhcp_identifier_set_iaid(void) { uint32_t iaid_legacy; be32_t iaid; - int ifindex; - for (;;) { - char ifname[IFNAMSIZ]; - - /* try to find an ifindex which does not exist. I causes dhcp_identifier_set_iaid() - * to hash the MAC address. */ - pseudo_random_bytes(&ifindex, sizeof(ifindex)); - if (ifindex > 0 && !if_indextoname(ifindex, ifname)) - break; - } - - assert_se(dhcp_identifier_set_iaid(ifindex, mac_addr, sizeof(mac_addr), true, &iaid_legacy) >= 0); - assert_se(dhcp_identifier_set_iaid(ifindex, mac_addr, sizeof(mac_addr), false, &iaid) >= 0); + assert_se(dhcp_identifier_set_iaid(42, mac_addr, sizeof(mac_addr), /* legacy = */ true, + /* use_mac = */ true, &iaid_legacy) >= 0); + assert_se(dhcp_identifier_set_iaid(42, mac_addr, sizeof(mac_addr), /* legacy = */ false, + /* use_mac = */ true, &iaid) >= 0); /* we expect, that the MAC address was hashed. The legacy value is in native * endianness. */ @@ -180,7 +171,7 @@ static int check_options(uint8_t code, uint8_t len, const void *option, void *us size_t duid_len; assert_se(dhcp_identifier_set_duid_en(&duid, &duid_len) >= 0); - assert_se(dhcp_identifier_set_iaid(42, mac_addr, ETH_ALEN, true, &iaid) >= 0); + assert_se(dhcp_identifier_set_iaid(42, mac_addr, ETH_ALEN, true, /* use_mac = */ true, &iaid) >= 0); assert_se(len == sizeof(uint8_t) + sizeof(uint32_t) + duid_len); assert_se(len == 19); @@ -299,6 +290,7 @@ static void test_discover_message(sd_event *e) { assert_se(sd_dhcp_client_set_ifindex(client, 42) >= 0); assert_se(sd_dhcp_client_set_mac(client, mac_addr, bcast_addr, ETH_ALEN, ARPHRD_ETHER) >= 0); + dhcp_client_set_test_mode(client, true); assert_se(sd_dhcp_client_set_request_option(client, 248) >= 0); @@ -516,6 +508,7 @@ static void test_addr_acq(sd_event *e) { assert_se(sd_dhcp_client_set_ifindex(client, 42) >= 0); assert_se(sd_dhcp_client_set_mac(client, mac_addr, bcast_addr, ETH_ALEN, ARPHRD_ETHER) >= 0); + dhcp_client_set_test_mode(client, true); assert_se(sd_dhcp_client_set_callback(client, test_addr_acq_acquired, e) >= 0); diff --git a/src/libsystemd-network/test-dhcp6-client.c b/src/libsystemd-network/test-dhcp6-client.c index 5d0c5ed2a21..a72c13684de 100644 --- a/src/libsystemd-network/test-dhcp6-client.c +++ b/src/libsystemd-network/test-dhcp6-client.c @@ -13,6 +13,7 @@ #include "sd-dhcp6-client.h" #include "sd-event.h" +#include "dhcp-identifier.h" #include "dhcp6-internal.h" #include "dhcp6-lease-internal.h" #include "dhcp6-protocol.h" @@ -24,7 +25,6 @@ #include "strv.h" #include "tests.h" #include "time-util.h" -#include "virt.h" static struct ether_addr mac_addr = { .ether_addr_octet = {'A', 'B', 'C', '1', '2', '3'} @@ -952,6 +952,7 @@ static int test_client_solicit(sd_event *e) { sizeof (mac_addr), ARPHRD_ETHER) >= 0); assert_se(sd_dhcp6_client_set_fqdn(client, "host.lab.intra") == 1); + dhcp6_client_set_test_mode(client, true); assert_se(sd_dhcp6_client_get_information_request(client, &val) >= 0); assert_se(val == 0); From af7b405dff9c32bc76cbc673103194e4d5ac9918 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Jun 2021 23:47:59 +0900 Subject: [PATCH 10/10] sd-dhcp: refuse to set iaid if we cannot find the interface --- src/libsystemd-network/dhcp-identifier.c | 35 +++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c index d1ea992d43c..3313e53f74c 100644 --- a/src/libsystemd-network/dhcp-identifier.c +++ b/src/libsystemd-network/dhcp-identifier.c @@ -163,34 +163,37 @@ int dhcp_identifier_set_iaid( bool legacy_unstable_byteorder, bool use_mac, void *_id) { + /* name is a pointer to memory in the sd_device struct, so must * have the same scope */ _cleanup_(sd_device_unrefp) sd_device *device = NULL; const char *name = NULL; - uint64_t id; uint32_t id32; + uint64_t id; int r; if (path_is_read_only_fs("/sys") <= 0 && !use_mac) { /* udev should be around */ - if (sd_device_new_from_ifindex(&device, ifindex) >= 0) { - r = sd_device_get_is_initialized(device); - if (r < 0) - return r; - if (r == 0) - /* not yet ready */ - return -EBUSY; + r = sd_device_new_from_ifindex(&device, ifindex); + if (r < 0) + return r; - r = device_is_renaming(device); - if (r < 0) - return r; - if (r > 0) - /* device is under renaming */ - return -EBUSY; + r = sd_device_get_is_initialized(device); + if (r < 0) + return r; + if (r == 0) + /* not yet ready */ + return -EBUSY; - name = net_get_name_persistent(device); - } + r = device_is_renaming(device); + if (r < 0) + return r; + if (r > 0) + /* device is under renaming */ + return -EBUSY; + + name = net_get_name_persistent(device); } if (name)