From f31fa08057f40ed5872daab7e566d6992b6c0cbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 09:56:04 +0200 Subject: [PATCH 01/12] sd-network: modernize log error messages SYNTHETIC_ERRNO() is used where appropriate. Splits in message format strings are removed, so that it's easier to read/grep/copy them. --- src/libsystemd-network/sd-dhcp-client.c | 65 ++++++++++--------------- src/libsystemd-network/sd-radv.c | 3 +- src/network/networkd.c | 2 +- 3 files changed, 30 insertions(+), 40 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 854eca5a584..36b87fe84ed 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -401,8 +401,8 @@ int sd_dhcp_client_set_client_id( * the client-id. The caller is advised to account for that. */ if ((type == ARPHRD_ETHER && data_len != ETH_ALEN) || (type == ARPHRD_INFINIBAND && data_len != 8)) - log_dhcp_client(client, "Changing client ID to hardware type %u with " - "unexpected address length %zu", + log_dhcp_client(client, + "Changing client ID to hardware type %u with unexpected address length %zu", type, data_len); client->client_id.type = type; @@ -1563,10 +1563,9 @@ static int client_handle_offer(sd_dhcp_client *client, DHCPMessage *offer, size_ } r = dhcp_option_parse(offer, len, dhcp_lease_parse_options, lease, NULL); - if (r != DHCP_OFFER) { - log_dhcp_client(client, "received message was not an OFFER, ignoring"); - return -ENOMSG; - } + if (r != DHCP_OFFER) + return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG), + "received message was not an OFFER, ignoring"); lease->next_server = offer->siaddr; lease->address = offer->yiaddr; @@ -1576,19 +1575,16 @@ static int client_handle_offer(sd_dhcp_client *client, DHCPMessage *offer, size_ if (lease->address == 0 || lease->server_address == 0 || - lease->lifetime == 0) { - log_dhcp_client(client, "received lease lacks address, server address or lease lifetime, ignoring"); - return -ENOMSG; - } + lease->lifetime == 0) + return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG), + "received lease lacks address, server address or lease lifetime, ignoring"); if (!lease->have_subnet_mask) { r = dhcp_lease_set_default_subnet_mask(lease); - if (r < 0) { - log_dhcp_client(client, - "received lease lacks subnet mask, " - "and a fallback one cannot be generated, ignoring"); - return -ENOMSG; - } + if (r < 0) + return log_dhcp_client_errno( + client, SYNTHETIC_ERRNO(ENOMSG), + "received lease lacks subnet mask, and a fallback one cannot be generated, ignoring"); } sd_dhcp_lease_unref(client->lease); @@ -1611,14 +1607,13 @@ static int client_handle_forcerenew(sd_dhcp_client *client, DHCPMessage *force, #if 0 log_dhcp_client(client, "FORCERENEW"); - return 0; #else /* FIXME: Ignore FORCERENEW requests until we implement RFC3118 (Authentication for DHCP * Messages) and/or RFC6704 (Forcerenew Nonce Authentication), as unauthenticated FORCERENEW * requests causes a security issue (TALOS-2020-1142, CVE-2020-13529). */ - log_dhcp_client(client, "Received FORCERENEW, ignoring."); - return -ENOMSG; + return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG), + "Received FORCERENEW, ignoring."); #endif } @@ -1657,15 +1652,13 @@ static int client_handle_ack(sd_dhcp_client *client, DHCPMessage *ack, size_t le } r = dhcp_option_parse(ack, len, dhcp_lease_parse_options, lease, &error_message); - if (r == DHCP_NAK) { - log_dhcp_client(client, "NAK: %s", strna(error_message)); - return -EADDRNOTAVAIL; - } + if (r == DHCP_NAK) + return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(EADDRNOTAVAIL), + "NAK: %s", strna(error_message)); - if (r != DHCP_ACK) { - log_dhcp_client(client, "received message was not an ACK, ignoring"); - return -ENOMSG; - } + if (r != DHCP_ACK) + return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG), + "received message was not an ACK, ignoring"); lease->next_server = ack->siaddr; @@ -1673,20 +1666,16 @@ static int client_handle_ack(sd_dhcp_client *client, DHCPMessage *ack, size_t le if (lease->address == INADDR_ANY || lease->server_address == INADDR_ANY || - lease->lifetime == 0) { - log_dhcp_client(client, "received lease lacks address, server " - "address or lease lifetime, ignoring"); - return -ENOMSG; - } + lease->lifetime == 0) + return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG), + "received lease lacks address, server address or lease lifetime, ignoring"); if (lease->subnet_mask == INADDR_ANY) { r = dhcp_lease_set_default_subnet_mask(lease); - if (r < 0) { - log_dhcp_client(client, - "received lease lacks subnet mask, " - "and a fallback one cannot be generated, ignoring"); - return -ENOMSG; - } + if (r < 0) + return log_dhcp_client_errno( + client, SYNTHETIC_ERRNO(ENOMSG), + "received lease lacks subnet mask, and a fallback one cannot be generated, ignoring"); } r = SD_DHCP_CLIENT_EVENT_IP_ACQUIRE; diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index a35311efb99..9506759bb57 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -1144,7 +1144,8 @@ int sd_radv_pref64_prefix_set_prefix( r = pref64_prefix_length_to_plc(prefixlen, &prefixlen_code); if (r < 0) - return log_radv_errno(NULL, r, "Unsupported PREF64 prefix length %u. Valid lengths are 32, 40, 48, 56, 64 and 96", prefixlen); + return log_radv_errno(NULL, r, + "Unsupported PREF64 prefix length %u. Valid lengths are 32, 40, 48, 56, 64 and 96", prefixlen); if (lifetime_usec == USEC_INFINITY || DIV_ROUND_UP(lifetime_usec, 8 * USEC_PER_SEC) >= UINT64_C(1) << 13) return -EINVAL; diff --git a/src/network/networkd.c b/src/network/networkd.c index 68760e8ff4d..46c2c743467 100644 --- a/src/network/networkd.c +++ b/src/network/networkd.c @@ -87,7 +87,7 @@ static int run(int argc, char *argv[]) { r = manager_setup(m); if (r < 0) - return log_error_errno(r, "Could not setup manager: %m"); + return log_error_errno(r, "Could not set up manager: %m"); r = manager_parse_config_file(m); if (r < 0) From 3b6cabd806c6eb5eec4ac21c903a5033bea94e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 10:08:12 +0200 Subject: [PATCH 02/12] basic/parse-util: add helper to parse bounded unsigned values "parse_range" is already used for stuff like "a-b", so use "bounded" here to avoid confusion. --- src/basic/parse-util.c | 15 +++++++++++++++ src/basic/parse-util.h | 3 ++- src/test/test-parse-util.c | 26 ++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index b8fbe3f1c77..d39082ed155 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -425,6 +425,21 @@ int safe_atou_full(const char *s, unsigned base, unsigned *ret_u) { return 0; } +int safe_atou_bounded(const char *s, unsigned min, unsigned max, unsigned *ret) { + unsigned v; + int r; + + r = safe_atou(s, &v); + if (r < 0) + return r; + + if (v < min || v > max) + return -ERANGE; + + *ret = v; + return 0; +} + int safe_atoi(const char *s, int *ret_i) { unsigned base = 0; char *x = NULL; diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h index ca0252005ac..e586077efb5 100644 --- a/src/basic/parse-util.h +++ b/src/basic/parse-util.h @@ -30,11 +30,12 @@ int parse_fd(const char *t); #define SAFE_ATO_MASK_FLAGS(base) ((base) & ~SAFE_ATO_ALL_FLAGS) int safe_atou_full(const char *s, unsigned base, unsigned *ret_u); - static inline int safe_atou(const char *s, unsigned *ret_u) { return safe_atou_full(s, 0, ret_u); } +int safe_atou_bounded(const char *s, unsigned min, unsigned max, unsigned *ret); + int safe_atoi(const char *s, int *ret_i); int safe_atolli(const char *s, long long int *ret_i); diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c index 1ba8c8987fd..7a485f390fb 100644 --- a/src/test/test-parse-util.c +++ b/src/test/test-parse-util.c @@ -417,6 +417,32 @@ TEST(parse_range) { assert_se(upper == 9999); } +TEST(safe_atou_bounded) { + int r; + unsigned x; + + r = safe_atou_bounded("12345", 12, 20000, &x); + assert_se(r == 0); + assert_se(x == 12345); + + r = safe_atou_bounded("12", 12, 20000, &x); + assert_se(r == 0); + assert_se(x == 12); + + r = safe_atou_bounded("20000", 12, 20000, &x); + assert_se(r == 0); + assert_se(x == 20000); + + r = safe_atou_bounded("-1", 12, 20000, &x); + assert_se(r == -ERANGE); + + r = safe_atou_bounded("11", 12, 20000, &x); + assert_se(r == -ERANGE); + + r = safe_atou_bounded("20001", 12, 20000, &x); + assert_se(r == -ERANGE); +} + TEST(safe_atolli) { int r; long long l; From 851cdffd1bfe7a1f2d3dfc0d343003aab6b44620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 12:06:19 +0200 Subject: [PATCH 03/12] network: use a common helper to parse bounded ranges This compresses repetetive code and makes it easier to add new options in networkd. The formatting of error messages becomes uniform. The error message always specifies the rvalue literally, instead of using a "descriptive name". This makes the message much easier to handle for the user. I opted to add just one parser, and wrap it with inline functions to proxy the type. This is less verbose than copying functions for each type separately, and the compiler should be able to get rid of the inline wrapper almost entirely. asserts are reordered to use the same order as the parameter list. This makes the code easier to read. No functional change intended, apart from the difference in error message formatting. --- src/network/netdev/bond.c | 47 +++----------- src/network/netdev/bridge.c | 53 ++++------------ src/network/netdev/geneve.c | 51 ++++------------ src/network/netdev/l2tp-tunnel.c | 59 ++++++------------ src/network/netdev/macvlan.c | 25 ++------ src/network/netdev/tunnel.c | 102 ++++++++++--------------------- src/shared/conf-parser.c | 38 ++++++++++++ src/shared/conf-parser.h | 94 ++++++++++++++++++++++++++++ 8 files changed, 220 insertions(+), 249 deletions(-) diff --git a/src/network/netdev/bond.c b/src/network/netdev/bond.c index 601bff0a9c0..704a4c6b4c1 100644 --- a/src/network/netdev/bond.c +++ b/src/network/netdev/bond.c @@ -303,32 +303,18 @@ int config_parse_ad_actor_sys_prio( const char *rvalue, void *data, void *userdata) { - Bond *b = userdata; - uint16_t v; - int r; assert(filename); assert(lvalue); assert(rvalue); assert(data); - r = safe_atou16(rvalue, &v); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse actor system priority '%s', ignoring: %m", rvalue); - return 0; - } + Bond *b = ASSERT_PTR(userdata); - if (v == 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Failed to parse actor system priority '%s'. Range is [1,65535], ignoring.", - rvalue); - return 0; - } - - b->ad_actor_sys_prio = v; - - return 0; + return config_parse_uint16_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 1, UINT16_MAX, true, + &b->ad_actor_sys_prio); } int config_parse_ad_user_port_key( @@ -342,31 +328,18 @@ int config_parse_ad_user_port_key( const char *rvalue, void *data, void *userdata) { - Bond *b = userdata; - uint16_t v; - int r; assert(filename); assert(lvalue); assert(rvalue); assert(data); - r = safe_atou16(rvalue, &v); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse user port key '%s', ignoring: %m", rvalue); - return 0; - } + Bond *b = ASSERT_PTR(userdata); - if (v > 1023) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Failed to parse user port key '%s'. Range is [0…1023], ignoring.", rvalue); - return 0; - } - - b->ad_user_port_key = v; - - return 0; + return config_parse_uint16_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, 1023, /* ignoring= */ true, + &b->ad_user_port_key); } int config_parse_ad_actor_system( diff --git a/src/network/netdev/bridge.c b/src/network/netdev/bridge.c index b65c3b49fc2..3783139d158 100644 --- a/src/network/netdev/bridge.c +++ b/src/network/netdev/bridge.c @@ -189,36 +189,22 @@ int config_parse_bridge_igmp_version( void *data, void *userdata) { - Bridge *b = userdata; - uint8_t u; - int r; - assert(filename); assert(lvalue); assert(rvalue); assert(data); + Bridge *b = ASSERT_PTR(userdata); + if (isempty(rvalue)) { b->igmp_version = 0; /* 0 means unset. */ return 0; } - r = safe_atou8(rvalue, &u); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse bridge's multicast IGMP version number '%s', ignoring assignment: %m", - rvalue); - return 0; - } - if (!IN_SET(u, 2, 3)) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid bridge's multicast IGMP version number '%s', ignoring assignment.", rvalue); - return 0; - } - - b->igmp_version = u; - - return 0; + return config_parse_uint8_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 2, 3, true, + &b->igmp_version); } int config_parse_bridge_port_priority( @@ -233,33 +219,16 @@ int config_parse_bridge_port_priority( void *data, void *userdata) { - uint16_t i; - int r; - assert(filename); assert(lvalue); assert(rvalue); - assert(data); - /* This is used in networkd-network-gperf.gperf. */ + uint16_t *prio = ASSERT_PTR(data); - r = safe_atou16(rvalue, &i); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse bridge port priority, ignoring: %s", rvalue); - return 0; - } - - if (i > LINK_BRIDGE_PORT_PRIORITY_MAX) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Bridge port priority is larger than maximum %u, ignoring: %s", - LINK_BRIDGE_PORT_PRIORITY_MAX, rvalue); - return 0; - } - - *((uint16_t *)data) = i; - - return 0; + return config_parse_uint16_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, LINK_BRIDGE_PORT_PRIORITY_MAX, true, + prio); } static void bridge_init(NetDev *n) { diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c index 7d5a8679d14..83c73aff077 100644 --- a/src/network/netdev/geneve.c +++ b/src/network/netdev/geneve.c @@ -116,29 +116,17 @@ int config_parse_geneve_vni( void *data, void *userdata) { - Geneve *v = userdata; - uint32_t f; - int r; - assert(filename); assert(lvalue); assert(rvalue); assert(data); - r = safe_atou32(rvalue, &f); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse Geneve VNI '%s'.", rvalue); - return 0; - } + Geneve *v = ASSERT_PTR(userdata); - if (f > GENEVE_VID_MAX){ - log_syntax(unit, LOG_WARNING, filename, line, 0, "Geneve VNI out is of range '%s'.", rvalue); - return 0; - } - - v->id = f; - - return 0; + return config_parse_uint32_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, GENEVE_VID_MAX, true, + &v->id); } int config_parse_geneve_address( @@ -230,35 +218,22 @@ int config_parse_geneve_ttl( void *data, void *userdata) { - Geneve *v = userdata; - unsigned f; - int r; - assert(filename); assert(lvalue); assert(rvalue); assert(data); - if (streq(rvalue, "inherit")) + Geneve *v = ASSERT_PTR(userdata); + + if (streq(rvalue, "inherit")) { v->inherit = true; - else { - r = safe_atou(rvalue, &f); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse Geneve TTL '%s', ignoring assignment: %m", rvalue); - return 0; - } - - if (f > 255) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid Geneve TTL '%s'. TTL must be <= 255. Ignoring assignment.", rvalue); - return 0; - } - - v->ttl = f; + return 0; } - return 0; + return config_parse_uint8_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, UINT8_MAX, true, + &v->ttl); } static int netdev_geneve_verify(NetDev *netdev, const char *filename) { diff --git a/src/network/netdev/l2tp-tunnel.c b/src/network/netdev/l2tp-tunnel.c index 547de2418c4..17d5391cd38 100644 --- a/src/network/netdev/l2tp-tunnel.c +++ b/src/network/netdev/l2tp-tunnel.c @@ -626,30 +626,16 @@ int config_parse_l2tp_tunnel_id( void *data, void *userdata) { - uint32_t *id = data, k; - int r; - assert(filename); assert(lvalue); assert(rvalue); - assert(data); - r = safe_atou32(rvalue, &k); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse L2TP tunnel id. Ignoring assignment: %s", rvalue); - return 0; - } + uint32_t *id = ASSERT_PTR(data); - if (k == 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid L2TP tunnel id. Ignoring assignment: %s", rvalue); - return 0; - } - - *id = k; - - return 0; + return config_parse_uint32_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 1, UINT32_MAX, true, + id); } int config_parse_l2tp_session_id( @@ -664,40 +650,29 @@ int config_parse_l2tp_session_id( void *data, void *userdata) { - _cleanup_(l2tp_session_free_or_set_invalidp) L2tpSession *session = NULL; - L2tpTunnel *t = userdata; - uint32_t k; - int r; - assert(filename); assert(section); assert(lvalue); assert(rvalue); assert(data); + L2tpTunnel *t = ASSERT_PTR(userdata); + _cleanup_(l2tp_session_free_or_set_invalidp) L2tpSession *session = NULL; + int r; + r = l2tp_session_new_static(t, filename, section_line, &session); if (r < 0) return log_oom(); - r = safe_atou32(rvalue, &k); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse L2TP session id. Ignoring assignment: %s", rvalue); - return 0; - } + uint32_t *id = streq(lvalue, "SessionId") ? &session->session_id : &session->peer_session_id; - if (k == 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid L2TP session id. Ignoring assignment: %s", rvalue); - return 0; - } - - if (streq(lvalue, "SessionId")) - session->session_id = k; - else - session->peer_session_id = k; - - session = NULL; + r = config_parse_uint32_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 1, UINT32_MAX, true, + id); + if (r <= 0) + return r; + TAKE_PTR(session); return 0; } diff --git a/src/network/netdev/macvlan.c b/src/network/netdev/macvlan.c index 1114bb0cb1b..6083cd2b44e 100644 --- a/src/network/netdev/macvlan.c +++ b/src/network/netdev/macvlan.c @@ -84,36 +84,23 @@ int config_parse_macvlan_broadcast_queue_size( void *data, void *userdata) { - MacVlan *m = ASSERT_PTR(userdata); - uint32_t v; - int r; - assert(filename); assert(section); assert(lvalue); assert(rvalue); assert(data); + MacVlan *m = ASSERT_PTR(userdata); + if (isempty(rvalue)) { m->bc_queue_length = UINT32_MAX; return 0; } - r = safe_atou32(rvalue, &v); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse BroadcastMulticastQueueLength=%s, ignoring assignment: %m", rvalue); - return 0; - } - - if (v == UINT32_MAX) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid BroadcastMulticastQueueLength=%s, ignoring assignment: %m", rvalue); - return 0; - } - - m->bc_queue_length = v; - return 0; + return config_parse_uint32_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, UINT32_MAX - 1, true, + &m->bc_queue_length); } static void macvlan_done(NetDev *n) { diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index 881be3c405b..ced04c77eca 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -972,33 +972,26 @@ int config_parse_encap_limit( void *data, void *userdata) { - Tunnel *t = ASSERT_PTR(userdata); - int k, r; - assert(filename); assert(rvalue); + Tunnel *t = ASSERT_PTR(userdata); + int r; + if (streq(rvalue, "none")) { - t->flags |= IP6_TNL_F_IGN_ENCAP_LIMIT; t->encap_limit = 0; + t->flags |= IP6_TNL_F_IGN_ENCAP_LIMIT; return 0; } - r = safe_atoi(rvalue, &k); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse Tunnel Encapsulation Limit option, ignoring assignment: %s", rvalue); - return 0; - } - - if (k > 255 || k < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid Tunnel Encapsulation value, ignoring assignment: %d", k); - return 0; - } - - t->encap_limit = k; + r = config_parse_uint8_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, UINT8_MAX, true, + &t->encap_limit); + if (r <= 0) + return r; t->flags &= ~IP6_TNL_F_IGN_ENCAP_LIMIT; + return 0; } @@ -1051,32 +1044,21 @@ int config_parse_erspan_version( void *data, void *userdata) { - uint8_t n, *v = ASSERT_PTR(data); - int r; - assert(filename); assert(lvalue); assert(rvalue); + uint8_t *v = ASSERT_PTR(data); + if (isempty(rvalue)) { *v = 1; /* defaults to 1 */ return 0; } - r = safe_atou8(rvalue, &n); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse erspan version \"%s\", ignoring: %m", rvalue); - return 0; - } - if (!IN_SET(n, 0, 1, 2)) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid erspan version \"%s\", which must be 0, 1 or 2, ignoring.", rvalue); - return 0; - } - - *v = n; - return 0; + return config_parse_uint8_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, 2, true, + v); } int config_parse_erspan_index( @@ -1091,32 +1073,21 @@ int config_parse_erspan_index( void *data, void *userdata) { - uint32_t n, *v = ASSERT_PTR(data); - int r; - assert(filename); assert(lvalue); assert(rvalue); + uint32_t *v = ASSERT_PTR(data); + if (isempty(rvalue)) { *v = 0; /* defaults to 0 */ return 0; } - r = safe_atou32(rvalue, &n); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse erspan index \"%s\", ignoring: %m", rvalue); - return 0; - } - if (n >= 0x100000) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid erspan index \"%s\", which must be less than 0x100000, ignoring.", rvalue); - return 0; - } - - *v = n; - return 0; + return config_parse_uint32_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, 0x100000 - 1, true, + v); } int config_parse_erspan_direction( @@ -1131,12 +1102,12 @@ int config_parse_erspan_direction( void *data, void *userdata) { - uint8_t *v = ASSERT_PTR(data); - assert(filename); assert(lvalue); assert(rvalue); + uint8_t *v = ASSERT_PTR(data); + if (isempty(rvalue) || streq(rvalue, "ingress")) *v = 0; /* defaults to ingress */ else if (streq(rvalue, "egress")) @@ -1160,32 +1131,21 @@ int config_parse_erspan_hwid( void *data, void *userdata) { - uint16_t n, *v = ASSERT_PTR(data); - int r; - assert(filename); assert(lvalue); assert(rvalue); + uint16_t *v = ASSERT_PTR(data); + if (isempty(rvalue)) { *v = 0; /* defaults to 0 */ return 0; } - r = safe_atou16(rvalue, &n); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse erspan hwid \"%s\", ignoring: %m", rvalue); - return 0; - } - if (n >= 64) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid erspan index \"%s\", which must be less than 64, ignoring.", rvalue); - return 0; - } - - *v = n; - return 0; + return config_parse_uint16_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, 63, true, + v); } static void netdev_tunnel_init(NetDev *netdev) { diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index af5aac7b291..a60588ad9ee 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -1908,6 +1908,44 @@ int config_parse_in_addr_non_null( return 0; } +int config_parse_unsigned_bounded( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *name, + const char *value, + unsigned min, + unsigned max, + bool ignoring, + unsigned *ret) { + + int r; + + assert(filename); + assert(name); + assert(value); + assert(ret); + + r = safe_atou_bounded(value, min, max, ret); + if (r == -ERANGE) + log_syntax(unit, LOG_WARNING, filename, line, r, + "Invalid '%s=%s', allowed range is %u..%u%s.", + name, value, min, max, ignoring ? ", ignoring" : ""); + else if (r < 0) + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse '%s=%s'%s: %m", + name, value, ignoring ? ", ignoring" : ""); + + if (r >= 0) + return 1; /* Return 1 if something was set */ + else if (ignoring) + return 0; + else + return r; +} + DEFINE_CONFIG_PARSE(config_parse_percent, parse_percent, "Failed to parse percent value"); DEFINE_CONFIG_PARSE(config_parse_permyriad, parse_permyriad, "Failed to parse permyriad value"); DEFINE_CONFIG_PARSE_PTR(config_parse_sec_fix_0, parse_sec_fix_0, usec_t, "Failed to parse time value"); diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 140883969cc..f1b03a8d152 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -385,3 +385,97 @@ typedef enum ConfigParseStringFlags { \ return free_and_replace(*enums, xs); \ } + +int config_parse_unsigned_bounded( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *name, + const char *value, + unsigned min, + unsigned max, + bool ignoring, + unsigned *ret); + +static inline int config_parse_uint32_bounded( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *name, + const char *value, + uint32_t min, + uint32_t max, + bool ignoring, + uint32_t *ret) { + + unsigned t; + int r; + + r = config_parse_unsigned_bounded( + unit, filename, line, section, section_line, name, value, + min, max, ignoring, + &t); + if (r <= 0) + return r; + assert(t <= UINT32_MAX); + *ret = t; + return 1; +} + +static inline int config_parse_uint16_bounded( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *name, + const char *value, + uint16_t min, + uint16_t max, + bool ignoring, + uint16_t *ret) { + + unsigned t; + int r; + + r = config_parse_unsigned_bounded( + unit, filename, line, section, section_line, name, value, + min, max, ignoring, + &t); + if (r <= 0) + return r; + assert(t <= UINT16_MAX); + *ret = t; + return 1; +} + +static inline int config_parse_uint8_bounded( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *name, + const char *value, + uint8_t min, + uint8_t max, + bool ignoring, + uint8_t *ret) { + + unsigned t; + int r; + + r = config_parse_unsigned_bounded( + unit, filename, line, section, section_line, name, value, + min, max, ignoring, + &t); + if (r <= 0) + return r; + assert(t <= UINT8_MAX); + *ret = t; + return 1; +} From 257cebb67a979b408f8d0814559717ae3a491812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 12:11:34 +0200 Subject: [PATCH 04/12] network/netdev: use ASSERT_PTR() more, adjust indentation --- src/network/netdev/bareudp.c | 20 ++-------- src/network/netdev/bond.c | 26 ++++--------- src/network/netdev/bridge.c | 14 +++---- src/network/netdev/fou-tunnel.c | 12 ++---- src/network/netdev/geneve.c | 40 +++++++++---------- src/network/netdev/macsec.c | 58 ++++++++++++---------------- src/network/netdev/tunnel.c | 68 ++++++++++----------------------- src/network/netdev/xfrm.c | 14 ++----- 8 files changed, 86 insertions(+), 166 deletions(-) diff --git a/src/network/netdev/bareudp.c b/src/network/netdev/bareudp.c index 24d3afb8771..539f8486e5c 100644 --- a/src/network/netdev/bareudp.c +++ b/src/network/netdev/bareudp.c @@ -21,15 +21,11 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_bare_udp_iftype, bare_udp_protocol, BareUD "Failed to parse EtherType="); static int netdev_bare_udp_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - BareUDP *u; - int r; - assert(netdev); assert(m); - u = BAREUDP(netdev); - - assert(u); + BareUDP *u = ASSERT_PTR(BAREUDP(netdev)); + int r; r = sd_netlink_message_append_u16(m, IFLA_BAREUDP_ETHERTYPE, htobe16(u->iftype)); if (r < 0) @@ -43,14 +39,10 @@ static int netdev_bare_udp_fill_message_create(NetDev *netdev, Link *link, sd_ne } static int netdev_bare_udp_verify(NetDev *netdev, const char *filename) { - BareUDP *u; - assert(netdev); assert(filename); - u = BAREUDP(netdev); - - assert(u); + BareUDP *u = ASSERT_PTR(BAREUDP(netdev)); if (u->dest_port == 0) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), @@ -64,13 +56,9 @@ static int netdev_bare_udp_verify(NetDev *netdev, const char *filename) { } static void bare_udp_init(NetDev *netdev) { - BareUDP *u; - assert(netdev); - u = BAREUDP(netdev); - - assert(u); + BareUDP *u = ASSERT_PTR(BAREUDP(netdev)); u->iftype = _BARE_UDP_PROTOCOL_INVALID; } diff --git a/src/network/netdev/bond.c b/src/network/netdev/bond.c index 704a4c6b4c1..6bedf0cd3ea 100644 --- a/src/network/netdev/bond.c +++ b/src/network/netdev/bond.c @@ -57,16 +57,12 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_bond_arp_all_targets, bond_arp_all_targets DEFINE_CONFIG_PARSE_ENUM(config_parse_bond_primary_reselect, bond_primary_reselect, BondPrimaryReselect, "Failed to parse bond primary reselect"); static int netdev_bond_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - Bond *b; - int r; - assert(netdev); assert(!link); assert(m); - b = BOND(netdev); - - assert(b); + Bond *b = ASSERT_PTR(BOND(netdev)); + int r; if (b->mode != _NETDEV_BOND_MODE_INVALID) { r = sd_netlink_message_append_u8(m, IFLA_BOND_MODE, b->mode); @@ -237,14 +233,14 @@ int config_parse_arp_ip_target_address( void *data, void *userdata) { - Bond *b = userdata; - int r; - assert(filename); assert(lvalue); assert(rvalue); assert(data); + Bond *b = ASSERT_PTR(BOND(userdata)); + int r; + if (isempty(rvalue)) { b->arp_ip_targets = ordered_set_free(b->arp_ip_targets); return 0; @@ -382,23 +378,17 @@ int config_parse_ad_actor_system( } static void bond_done(NetDev *netdev) { - Bond *b; - assert(netdev); - b = BOND(netdev); - assert(b); + + Bond *b = ASSERT_PTR(BOND(netdev)); ordered_set_free(b->arp_ip_targets); } static void bond_init(NetDev *netdev) { - Bond *b; - assert(netdev); - b = BOND(netdev); - - assert(b); + Bond *b = ASSERT_PTR(BOND(netdev)); b->mode = _NETDEV_BOND_MODE_INVALID; b->xmit_hash_policy = _NETDEV_BOND_XMIT_HASH_POLICY_INVALID; diff --git a/src/network/netdev/bridge.c b/src/network/netdev/bridge.c index 3783139d158..19638a871f8 100644 --- a/src/network/netdev/bridge.c +++ b/src/network/netdev/bridge.c @@ -46,10 +46,10 @@ static int netdev_bridge_set_handler(sd_netlink *rtnl, sd_netlink_message *m, Ne } static int netdev_bridge_post_create_message(NetDev *netdev, sd_netlink_message *req) { - Bridge *b; - int r; + assert(netdev); - assert_se(b = BRIDGE(netdev)); + Bridge *b = ASSERT_PTR(BRIDGE(netdev)); + int r; r = sd_netlink_message_open_container(req, IFLA_LINKINFO); if (r < 0) @@ -231,12 +231,8 @@ int config_parse_bridge_port_priority( prio); } -static void bridge_init(NetDev *n) { - Bridge *b; - - b = BRIDGE(n); - - assert(b); +static void bridge_init(NetDev *netdev) { + Bridge *b = ASSERT_PTR(BRIDGE(netdev)); b->mcast_querier = -1; b->mcast_snooping = -1; diff --git a/src/network/netdev/fou-tunnel.c b/src/network/netdev/fou-tunnel.c index 7baec1529f4..2786bf6bb88 100644 --- a/src/network/netdev/fou-tunnel.c +++ b/src/network/netdev/fou-tunnel.c @@ -24,12 +24,12 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_fou_encap_type, fou_encap_type, FooOverUDP "Failed to parse Encapsulation="); static int netdev_fill_fou_tunnel_message(NetDev *netdev, sd_netlink_message *m) { - FouTunnel *t; + assert(netdev); + + FouTunnel *t = ASSERT_PTR(FOU(netdev)); uint8_t encap_type; int r; - assert_se(t = FOU(netdev)); - r = sd_netlink_message_append_u16(m, FOU_ATTR_PORT, htobe16(t->port)); if (r < 0) return r; @@ -225,14 +225,10 @@ int config_parse_fou_tunnel_address( } static int netdev_fou_tunnel_verify(NetDev *netdev, const char *filename) { - FouTunnel *t; - assert(netdev); assert(filename); - t = FOU(netdev); - - assert(t); + FouTunnel *t = ASSERT_PTR(FOU(netdev)); switch (t->fou_encap_type) { case NETDEV_FOO_OVER_UDP_ENCAP_DIRECT: diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c index 83c73aff077..f5f325711cd 100644 --- a/src/network/netdev/geneve.c +++ b/src/network/netdev/geneve.c @@ -28,13 +28,11 @@ DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(geneve_df, GeneveDF, NETDEV_GENEVE_DF_YE DEFINE_CONFIG_PARSE_ENUM(config_parse_geneve_df, geneve_df, GeneveDF, "Failed to parse Geneve IPDoNotFragment= setting"); static int netdev_geneve_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - Geneve *v; - int r; - assert(netdev); assert(m); - v = GENEVE(netdev); + Geneve *v = ASSERT_PTR(GENEVE(netdev)); + int r; if (v->id <= GENEVE_VID_MAX) { r = sd_netlink_message_append_u32(m, IFLA_GENEVE_ID, v->id); @@ -141,24 +139,26 @@ int config_parse_geneve_address( void *data, void *userdata) { - Geneve *v = userdata; - union in_addr_union *addr = data, buffer; - int r, f; - assert(filename); assert(lvalue); assert(rvalue); assert(data); + Geneve *v = ASSERT_PTR(userdata); + union in_addr_union *addr = data, buffer; + int r, f; + r = in_addr_from_string_auto(rvalue, &f, &buffer); if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, "geneve '%s' address is invalid, ignoring assignment: %s", lvalue, rvalue); + log_syntax(unit, LOG_WARNING, filename, line, r, + "geneve '%s' address is invalid, ignoring assignment: %s", lvalue, rvalue); return 0; } r = in_addr_is_multicast(f, &buffer); if (r > 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "geneve invalid multicast '%s' address, ignoring assignment: %s", lvalue, rvalue); + log_syntax(unit, LOG_WARNING, filename, line, 0, + "geneve invalid multicast '%s' address, ignoring assignment: %s", lvalue, rvalue); return 0; } @@ -180,15 +180,15 @@ int config_parse_geneve_flow_label( void *data, void *userdata) { - Geneve *v = userdata; - uint32_t f; - int r; - assert(filename); assert(lvalue); assert(rvalue); assert(data); + Geneve *v = ASSERT_PTR(userdata); + uint32_t f; + int r; + r = safe_atou32(rvalue, &f); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse Geneve flow label '%s'.", rvalue); @@ -237,28 +237,22 @@ int config_parse_geneve_ttl( } static int netdev_geneve_verify(NetDev *netdev, const char *filename) { - Geneve *v = GENEVE(netdev); - assert(netdev); - assert(v); assert(filename); + Geneve *v = ASSERT_PTR(GENEVE(netdev)); + if (v->id > GENEVE_VID_MAX) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), "%s: Geneve without valid VNI (or Virtual Network Identifier) configured. Ignoring.", filename); - return 0; } static void geneve_init(NetDev *netdev) { - Geneve *v; - assert(netdev); - v = GENEVE(netdev); - - assert(v); + Geneve *v = ASSERT_PTR(GENEVE(netdev)); v->id = GENEVE_VID_MAX + 1; v->geneve_df = _NETDEV_GENEVE_DF_INVALID; diff --git a/src/network/netdev/macsec.c b/src/network/netdev/macsec.c index 0da3dd4bd22..6469a97b29e 100644 --- a/src/network/netdev/macsec.c +++ b/src/network/netdev/macsec.c @@ -356,13 +356,11 @@ static int netdev_macsec_configure_receive_association(NetDev *netdev, ReceiveAs } static int macsec_receive_channel_handler(sd_netlink *rtnl, sd_netlink_message *m, ReceiveChannel *c) { - NetDev *netdev; - int r; - assert(c); assert(c->macsec); - netdev = NETDEV(c->macsec); + NetDev *netdev = ASSERT_PTR(NETDEV(c->macsec)); + int r; assert(netdev->state != _NETDEV_STATE_INVALID); @@ -474,15 +472,13 @@ static int netdev_macsec_configure_transmit_association(NetDev *netdev, Transmit } static int netdev_macsec_configure(NetDev *netdev, Link *link) { + assert(netdev); + + MACsec *s = ASSERT_PTR(MACSEC(netdev)); TransmitAssociation *a; ReceiveChannel *c; - MACsec *s; int r; - assert(netdev); - s = MACSEC(netdev); - assert(s); - ORDERED_HASHMAP_FOREACH(a, s->transmit_associations_by_section) { r = netdev_macsec_configure_transmit_association(netdev, a); if (r < 0) @@ -499,15 +495,11 @@ static int netdev_macsec_configure(NetDev *netdev, Link *link) { } static int netdev_macsec_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - MACsec *v; - int r; - assert(netdev); assert(m); - v = MACSEC(netdev); - - assert(v); + MACsec *v = ASSERT_PTR(MACSEC(netdev)); + int r; if (v->port > 0) { r = sd_netlink_message_append_u16(m, IFLA_MACSEC_PORT, v->port); @@ -540,19 +532,19 @@ int config_parse_macsec_port( void *data, void *userdata) { - _cleanup_(macsec_receive_association_free_or_set_invalidp) ReceiveAssociation *b = NULL; - _cleanup_(macsec_receive_channel_free_or_set_invalidp) ReceiveChannel *c = NULL; - MACsec *s = userdata; - uint16_t port; - void *dest; - int r; - assert(filename); assert(section); assert(lvalue); assert(rvalue); assert(data); + MACsec *s = ASSERT_PTR(userdata); + _cleanup_(macsec_receive_association_free_or_set_invalidp) ReceiveAssociation *b = NULL; + _cleanup_(macsec_receive_channel_free_or_set_invalidp) ReceiveChannel *c = NULL; + uint16_t port; + void *dest; + int r; + /* This parses port used to make Secure Channel Identifier (SCI) */ if (streq(section, "MACsec")) @@ -601,17 +593,17 @@ int config_parse_macsec_hw_address( void *data, void *userdata) { - _cleanup_(macsec_receive_association_free_or_set_invalidp) ReceiveAssociation *b = NULL; - _cleanup_(macsec_receive_channel_free_or_set_invalidp) ReceiveChannel *c = NULL; - MACsec *s = userdata; - int r; - assert(filename); assert(section); assert(lvalue); assert(rvalue); assert(data); + MACsec *s = ASSERT_PTR(userdata); + _cleanup_(macsec_receive_association_free_or_set_invalidp) ReceiveAssociation *b = NULL; + _cleanup_(macsec_receive_channel_free_or_set_invalidp) ReceiveChannel *c = NULL; + int r; + if (streq(section, "MACsecReceiveChannel")) r = macsec_receive_channel_new_static(s, filename, section_line, &c); else @@ -645,18 +637,18 @@ int config_parse_macsec_packet_number( void *data, void *userdata) { - _cleanup_(macsec_transmit_association_free_or_set_invalidp) TransmitAssociation *a = NULL; - _cleanup_(macsec_receive_association_free_or_set_invalidp) ReceiveAssociation *b = NULL; - MACsec *s = userdata; - uint32_t val, *dest; - int r; - assert(filename); assert(section); assert(lvalue); assert(rvalue); assert(data); + MACsec *s = ASSERT_PTR(userdata); + _cleanup_(macsec_transmit_association_free_or_set_invalidp) TransmitAssociation *a = NULL; + _cleanup_(macsec_receive_association_free_or_set_invalidp) ReceiveAssociation *b = NULL; + uint32_t val, *dest; + int r; + if (streq(section, "MACsecTransmitAssociation")) r = macsec_transmit_association_new_static(s, filename, section_line, &a); else diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index ced04c77eca..a6985753d4b 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -207,11 +207,9 @@ static int netdev_ipip_sit_fill_message_create(NetDev *netdev, Link *link, sd_ne assert(m); if (netdev->kind == NETDEV_KIND_IPIP) - t = IPIP(netdev); + t = ASSERT_PTR(IPIP(netdev)); else - t = SIT(netdev); - - assert(t); + t = ASSERT_PTR(SIT(netdev)); if (t->external) { r = sd_netlink_message_append_flag(m, IFLA_IPTUN_COLLECT_METADATA); @@ -268,8 +266,8 @@ static int netdev_ipip_sit_fill_message_create(NetDev *netdev, Link *link, sd_ne if (r < 0) return r; - /* u16 is deliberate here, even though we're passing a netmask that can never be >128. The kernel is - * expecting to receive the prefixlen as a u16. + /* u16 is deliberate here, even though we're passing a netmask that can never be + * >128. The kernel is expecting to receive the prefixlen as a u16. */ r = sd_netlink_message_append_u16(m, IFLA_IPTUN_6RD_PREFIXLEN, t->sixrd_prefixlen); if (r < 0) @@ -304,20 +302,18 @@ static int netdev_gre_erspan_fill_message_create(NetDev *netdev, Link *link, sd_ switch (netdev->kind) { case NETDEV_KIND_GRE: - t = GRE(netdev); + t = ASSERT_PTR(GRE(netdev)); break; case NETDEV_KIND_ERSPAN: - t = ERSPAN(netdev); + t = ASSERT_PTR(ERSPAN(netdev)); break; case NETDEV_KIND_GRETAP: - t = GRETAP(netdev); + t = ASSERT_PTR(GRETAP(netdev)); break; default: assert_not_reached(); } - assert(t); - if (t->external) { r = sd_netlink_message_append_flag(m, IFLA_GRE_COLLECT_METADATA); if (r < 0) @@ -441,10 +437,8 @@ static int netdev_gre_erspan_fill_message_create(NetDev *netdev, Link *link, sd_ static int netdev_ip6gre_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { union in_addr_union local; - uint32_t ikey = 0; - uint32_t okey = 0; - uint16_t iflags = 0; - uint16_t oflags = 0; + uint32_t ikey = 0, okey = 0; + uint16_t iflags = 0, oflags = 0; Tunnel *t; int r; @@ -452,11 +446,9 @@ static int netdev_ip6gre_fill_message_create(NetDev *netdev, Link *link, sd_netl assert(m); if (netdev->kind == NETDEV_KIND_IP6GRE) - t = IP6GRE(netdev); + t = ASSERT_PTR(IP6GRE(netdev)); else - t = IP6GRETAP(netdev); - - assert(t); + t = ASSERT_PTR(IP6GRETAP(netdev)); if (t->external) { r = sd_netlink_message_append_flag(m, IFLA_GRE_COLLECT_METADATA); @@ -544,11 +536,9 @@ static int netdev_vti_fill_message_create(NetDev *netdev, Link *link, sd_netlink assert(m); if (netdev->kind == NETDEV_KIND_VTI) - t = VTI(netdev); + t = ASSERT_PTR(VTI(netdev)); else - t = VTI6(netdev); - - assert(t); + t = ASSERT_PTR(VTI6(netdev)); if (link || t->assign_to_loopback) { r = sd_netlink_message_append_u32(m, IFLA_VTI_LINK, link ? link->ifindex : LOOPBACK_IFINDEX); @@ -587,17 +577,13 @@ static int netdev_vti_fill_message_create(NetDev *netdev, Link *link, sd_netlink } static int netdev_ip6tnl_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - union in_addr_union local; - uint8_t proto; - Tunnel *t; - int r; - assert(netdev); assert(m); - t = IP6TNL(netdev); - - assert(t); + union in_addr_union local; + uint8_t proto; + Tunnel *t = ASSERT_PTR(IP6TNL(netdev)); + int r; switch (t->ip6tnl_mode) { case NETDEV_IP6_TNL_MODE_IP6IP6: @@ -673,13 +659,9 @@ static int netdev_ip6tnl_fill_message_create(NetDev *netdev, Link *link, sd_netl } static int netdev_tunnel_is_ready_to_create(NetDev *netdev, Link *link) { - Tunnel *t; - assert(netdev); - t = TUNNEL(netdev); - - assert(t); + Tunnel *t = ASSERT_PTR(TUNNEL(netdev)); if (t->independent) return true; @@ -688,14 +670,10 @@ static int netdev_tunnel_is_ready_to_create(NetDev *netdev, Link *link) { } static int netdev_tunnel_verify(NetDev *netdev, const char *filename) { - Tunnel *t; - assert(netdev); assert(filename); - t = TUNNEL(netdev); - - assert(t); + Tunnel *t = ASSERT_PTR(TUNNEL(netdev)); if (netdev->kind == NETDEV_KIND_IP6TNL && t->ip6tnl_mode == _NETDEV_IP6_TNL_MODE_INVALID) @@ -1149,13 +1127,7 @@ int config_parse_erspan_hwid( } static void netdev_tunnel_init(NetDev *netdev) { - Tunnel *t; - - assert(netdev); - - t = TUNNEL(netdev); - - assert(t); + Tunnel *t = ASSERT_PTR(TUNNEL(netdev)); t->local_type = _NETDEV_LOCAL_ADDRESS_TYPE_INVALID; t->pmtudisc = -1; diff --git a/src/network/netdev/xfrm.c b/src/network/netdev/xfrm.c index a961d8fef24..a3042831722 100644 --- a/src/network/netdev/xfrm.c +++ b/src/network/netdev/xfrm.c @@ -6,15 +6,12 @@ #include "xfrm.h" static int xfrm_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *message) { - Xfrm *x; - int r; - assert(netdev); assert(message); - x = XFRM(netdev); + Xfrm *x = ASSERT_PTR(XFRM(netdev)); + int r; - assert(x); assert(link || x->independent); r = sd_netlink_message_append_u32(message, IFLA_XFRM_LINK, link ? link->ifindex : LOOPBACK_IFINDEX); @@ -29,19 +26,14 @@ static int xfrm_fill_message_create(NetDev *netdev, Link *link, sd_netlink_messa } static int xfrm_verify(NetDev *netdev, const char *filename) { - Xfrm *x; - assert(netdev); assert(filename); - x = XFRM(netdev); - - assert(x); + Xfrm *x = ASSERT_PTR(XFRM(netdev)); if (x->if_id == 0) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), "%s: Xfrm interface ID cannot be zero.", filename); - return 0; } From 04c2a002e6147f8073b9578b14f4a536767b5568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 12:13:56 +0200 Subject: [PATCH 05/12] network/vxlan: avoid unneccesary temporary variables parse_ip_port_range() DTRT and only sets the output on success. --- src/network/netdev/vxlan.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/network/netdev/vxlan.c b/src/network/netdev/vxlan.c index 589161938ae..c91ed24cda1 100644 --- a/src/network/netdev/vxlan.c +++ b/src/network/netdev/vxlan.c @@ -286,25 +286,18 @@ int config_parse_port_range( void *data, void *userdata) { - VxLan *v = userdata; - uint16_t low, high; - int r; - assert(filename); assert(lvalue); assert(rvalue); assert(data); - r = parse_ip_port_range(rvalue, &low, &high); - if (r < 0) { + VxLan *v = ASSERT_PTR(userdata); + int r; + + r = parse_ip_port_range(rvalue, &v->port_range.low, &v->port_range.high); + if (r < 0) log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse VXLAN port range '%s'. Port should be greater than 0 and less than 65535.", rvalue); - return 0; - } - - v->port_range.low = low; - v->port_range.high = high; - return 0; } From 1b2733b4129c132d598ec96a07b4f93386ad4e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 12:43:16 +0200 Subject: [PATCH 06/12] shared/ip-procotol-list: generalize and rework parse_ip_protocol() Optionally, accept protocols that don't have a known name. Avoid any allocations in the common case. Return more granular error codes: -ERANGE for negative values, -EOPNOTSUPP if the protocol is a valid number, but we don't know the protocol, and -EINVAL only if it's not a numerical string. --- src/shared/ip-protocol-list.c | 43 ++++++++++++++++++------------- src/shared/ip-protocol-list.h | 7 ++++- src/test/test-ip-protocol-list.c | 44 +++++++++++++++++--------------- 3 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/shared/ip-protocol-list.c b/src/shared/ip-protocol-list.c index 21d88f26607..14155b679ab 100644 --- a/src/shared/ip-protocol-list.c +++ b/src/shared/ip-protocol-list.c @@ -37,33 +37,40 @@ int ip_protocol_from_name(const char *name) { return sc->id; } -int parse_ip_protocol(const char *s) { - _cleanup_free_ char *str = NULL; - int i, r; +int parse_ip_protocol_full(const char *s, bool relaxed) { + int r, p; assert(s); if (isempty(s)) return IPPROTO_IP; - /* Do not use strdupa() here, as the input string may come from * - * command line or config files. */ - str = strdup(s); - if (!str) - return -ENOMEM; - - i = ip_protocol_from_name(ascii_strlower(str)); - if (i >= 0) - return i; - - r = safe_atoi(str, &i); - if (r < 0) + /* People commonly use lowercase protocol names, which we can look up very quickly, so let's try that + * first. */ + r = ip_protocol_from_name(s); + if (r >= 0) return r; - if (!ip_protocol_to_name(i)) - return -EINVAL; + /* Do not use strdupa() here, as the input string may come from command line or config files. */ + _cleanup_free_ char *t = strdup(s); + if (!t) + return -ENOMEM; - return i; + r = ip_protocol_from_name(ascii_strlower(t)); + if (r >= 0) + return r; + + r = safe_atoi(t, &p); + if (r < 0) + return r; + if (p < 0) + return -ERANGE; + + /* If @relaxed, we don't check that we have a name for the protocol. */ + if (!relaxed && !ip_protocol_to_name(p)) + return -EPROTONOSUPPORT; + + return p; } const char *ip_protocol_to_tcp_udp(int id) { diff --git a/src/shared/ip-protocol-list.h b/src/shared/ip-protocol-list.h index b40ec083016..a0875ef234c 100644 --- a/src/shared/ip-protocol-list.h +++ b/src/shared/ip-protocol-list.h @@ -1,9 +1,14 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include + const char *ip_protocol_to_name(int id); int ip_protocol_from_name(const char *name); -int parse_ip_protocol(const char *s); +int parse_ip_protocol_full(const char *s, bool relaxed); +static inline int parse_ip_protocol(const char *s) { + return parse_ip_protocol_full(s, false); +} const char *ip_protocol_to_tcp_udp(int id); int ip_protocol_from_tcp_udp(const char *ip_protocol); diff --git a/src/test/test-ip-protocol-list.c b/src/test/test-ip-protocol-list.c index 018441d497a..dfff015f53e 100644 --- a/src/test/test-ip-protocol-list.c +++ b/src/test/test-ip-protocol-list.c @@ -17,13 +17,13 @@ static void test_int(int i) { assert_se(ip_protocol_from_name(ip_protocol_to_name(parse_ip_protocol(str))) == i); } -static void test_int_fail(int i) { +static void test_int_fail(int i, int error) { char str[DECIMAL_STR_MAX(int)]; assert_se(!ip_protocol_to_name(i)); xsprintf(str, "%i", i); - assert_se(parse_ip_protocol(str) == -EINVAL); + assert_se(parse_ip_protocol(str) == error); } static void test_str(const char *s) { @@ -31,39 +31,41 @@ static void test_str(const char *s) { assert_se(streq(ip_protocol_to_name(parse_ip_protocol(s)), s)); } -static void test_str_fail(const char *s) { +static void test_str_fail(const char *s, int error) { assert_se(ip_protocol_from_name(s) == -EINVAL); - assert_se(parse_ip_protocol(s) == -EINVAL); -} - -static void test_parse_ip_protocol_one(const char *s, int expected) { - assert_se(parse_ip_protocol(s) == expected); + assert_se(parse_ip_protocol(s) == error); } TEST(integer) { test_int(IPPROTO_TCP); test_int(IPPROTO_DCCP); - test_int_fail(-1); - test_int_fail(1024 * 1024); + test_int_fail(-1, -ERANGE); + test_int_fail(1024 * 1024, -EPROTONOSUPPORT); } TEST(string) { test_str("sctp"); test_str("udp"); - test_str_fail("hoge"); - test_str_fail("-1"); - test_str_fail("1000000000"); + test_str_fail("hoge", -EINVAL); + test_str_fail("-1", -ERANGE); + test_str_fail("1000000000", -EPROTONOSUPPORT); } TEST(parse_ip_protocol) { - test_parse_ip_protocol_one("sctp", IPPROTO_SCTP); - test_parse_ip_protocol_one("ScTp", IPPROTO_SCTP); - test_parse_ip_protocol_one("ip", IPPROTO_IP); - test_parse_ip_protocol_one("", IPPROTO_IP); - test_parse_ip_protocol_one("1", 1); - test_parse_ip_protocol_one("0", 0); - test_parse_ip_protocol_one("-10", -EINVAL); - test_parse_ip_protocol_one("100000000", -EINVAL); + assert_se(parse_ip_protocol("sctp") == IPPROTO_SCTP); + assert_se(parse_ip_protocol("ScTp") == IPPROTO_SCTP); + assert_se(parse_ip_protocol("ip") == IPPROTO_IP); + assert_se(parse_ip_protocol("") == IPPROTO_IP); + assert_se(parse_ip_protocol("1") == 1); + assert_se(parse_ip_protocol("0") == 0); + assert_se(parse_ip_protocol("-10") == -ERANGE); + assert_se(parse_ip_protocol("100000000") == -EPROTONOSUPPORT); +} + +TEST(parse_ip_protocol_full) { + assert_se(parse_ip_protocol_full("-1", true) == -ERANGE); + assert_se(parse_ip_protocol_full("0", true) == 0); + assert_se(parse_ip_protocol_full("11", true) == 11); } DEFINE_TEST_MAIN(LOG_INFO); From a893c121edcab567cabf17184a1ae041d0ce2468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 12:48:07 +0200 Subject: [PATCH 07/12] network/fou-tunnel: simplify parsing of protocol number Previously, we would call parse_ip_protocol(), which internally calls safe_atoi(), and then call safe_atou(). This isn't terrible, but it's also slightly confusing. Use parse_ip_protocol_full() to avoid the second call. --- src/network/netdev/fou-tunnel.c | 35 ++++++++++++++------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/network/netdev/fou-tunnel.c b/src/network/netdev/fou-tunnel.c index 2786bf6bb88..12e8e462a21 100644 --- a/src/network/netdev/fou-tunnel.c +++ b/src/network/netdev/fou-tunnel.c @@ -156,37 +156,32 @@ int config_parse_ip_protocol( void *data, void *userdata) { - uint8_t *ret = ASSERT_PTR(data); - unsigned protocol; - /* linux/fou.h defines the netlink field as one byte, so we need to reject protocols numbers that - * don't fit in one byte. */ - int r; - assert(filename); assert(section); assert(lvalue); assert(rvalue); - r = parse_ip_protocol(rvalue); - if (r >= 0) - protocol = r; - else { - r = safe_atou(rvalue, &protocol); - if (r < 0) - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse IP protocol '%s' for FooOverUDP tunnel, " - "ignoring assignment: %m", rvalue); + uint8_t *proto = ASSERT_PTR(data); + int r; + + r = parse_ip_protocol_full(rvalue, /* relaxed= */ true); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse '%s=%s', ignoring: %m", + lvalue, rvalue); return 0; } - if (protocol > UINT8_MAX) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "IP protocol '%s' for FooOverUDP tunnel out of range, " - "ignoring assignment: %m", rvalue); + if (r > UINT8_MAX) { + /* linux/fou.h defines the netlink field as one byte, so we need to reject + * protocols numbers that don't fit in one byte. */ + log_syntax(unit, LOG_WARNING, filename, line, r, + "Invalid '%s=%s', allowed range is 0..255, ignoring.", + lvalue, rvalue); return 0; } - *ret = protocol; + *proto = r; return 0; } From af14281d2c12a2942552765508e61f8445abb483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 20:42:43 +0200 Subject: [PATCH 08/12] network: refusing parsing negative flow labels The docs for FlowLabel= said that the range is 0..1048575, but the code did not reject negative numbers. --- src/network/netdev/tunnel.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index a6985753d4b..84da73c711e 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -909,7 +909,8 @@ int config_parse_ipv6_flowlabel( void *userdata) { Tunnel *t = ASSERT_PTR(userdata); - int k, r; + uint32_t k; + int r; assert(filename); assert(rvalue); @@ -920,21 +921,15 @@ int config_parse_ipv6_flowlabel( return 0; } - r = safe_atoi(rvalue, &k); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse tunnel IPv6 flowlabel, ignoring assignment: %s", rvalue); - return 0; - } - - if (k > 0xFFFFF) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid tunnel IPv6 flowlabel, ignoring assignment: %s", rvalue); - return 0; - } - + r = config_parse_uint32_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, 0xFFFFF, true, + &k); + if (r <= 0) + return r; t->ipv6_flowlabel = htobe32(k) & IP6_FLOWINFO_FLOWLABEL; t->flags &= ~IP6_TNL_F_USE_ORIG_FLOWLABEL; + return 0; } From 6fded8dced402eafc5069201f6f4c0fd0e27adf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2023 23:54:18 +0200 Subject: [PATCH 09/12] network/netdev: fix resetting of 'inherit' field We have two fields: inherit and ttl, and ttl is ignored if inherit is true. Setting TTL=inherit and later TTL=n would not work because we didn't unset inherit. --- src/network/netdev/geneve.c | 8 +++++++- src/network/netdev/vxlan.c | 33 +++++++++++++-------------------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c index f5f325711cd..c60fe6974d5 100644 --- a/src/network/netdev/geneve.c +++ b/src/network/netdev/geneve.c @@ -224,16 +224,22 @@ int config_parse_geneve_ttl( assert(data); Geneve *v = ASSERT_PTR(userdata); + int r; if (streq(rvalue, "inherit")) { v->inherit = true; + v->ttl = 0; /* unset the unused ttl field for clarity */ return 0; } - return config_parse_uint8_bounded( + r = config_parse_uint8_bounded( unit, filename, line, section, section_line, lvalue, rvalue, 0, UINT8_MAX, true, &v->ttl); + if (r <= 0) + return r; + v->inherit = false; + return 0; } static int netdev_geneve_verify(NetDev *netdev, const char *filename) { diff --git a/src/network/netdev/vxlan.c b/src/network/netdev/vxlan.c index c91ed24cda1..b7d3ee50859 100644 --- a/src/network/netdev/vxlan.c +++ b/src/network/netdev/vxlan.c @@ -351,34 +351,27 @@ int config_parse_vxlan_ttl( void *data, void *userdata) { - VxLan *v = userdata; - unsigned f; - int r; - assert(filename); assert(lvalue); assert(rvalue); assert(data); - if (streq(rvalue, "inherit")) + VxLan *v = ASSERT_PTR(userdata); + int r; + + if (streq(rvalue, "inherit")) { v->inherit = true; - else { - r = safe_atou(rvalue, &f); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Failed to parse VXLAN TTL '%s', ignoring assignment: %m", rvalue); - return 0; - } - - if (f > 255) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid VXLAN TTL '%s'. TTL must be <= 255. Ignoring assignment.", rvalue); - return 0; - } - - v->ttl = f; + v->ttl = 0; /* unset the unused ttl field for clarity */ + return 0; } + r = config_parse_unsigned_bounded( + unit, filename, line, section, section_line, lvalue, rvalue, + 0, UINT8_MAX, true, + &v->ttl); + if (r <= 0) + return r; + v->inherit = false; return 0; } From 6c9935ba87de2b46259e11722687b02349b9d1fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 17 Sep 2023 00:14:29 +0200 Subject: [PATCH 10/12] network/netdev: align tables --- src/network/netdev/geneve.c | 4 ++-- src/network/netdev/netdev-util.c | 10 +++++----- src/network/netdev/tunnel.c | 2 +- src/network/netdev/vxlan.c | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c index c60fe6974d5..5615f5bdcea 100644 --- a/src/network/netdev/geneve.c +++ b/src/network/netdev/geneve.c @@ -19,8 +19,8 @@ #define DEFAULT_GENEVE_DESTINATION_PORT 6081 static const char* const geneve_df_table[_NETDEV_GENEVE_DF_MAX] = { - [NETDEV_GENEVE_DF_NO] = "no", - [NETDEV_GENEVE_DF_YES] = "yes", + [NETDEV_GENEVE_DF_NO] = "no", + [NETDEV_GENEVE_DF_YES] = "yes", [NETDEV_GENEVE_DF_INHERIT] = "inherit", }; diff --git a/src/network/netdev/netdev-util.c b/src/network/netdev/netdev-util.c index 23506246ce2..6229992a1bf 100644 --- a/src/network/netdev/netdev-util.c +++ b/src/network/netdev/netdev-util.c @@ -6,11 +6,11 @@ #include "string-table.h" static const char * const netdev_local_address_type_table[_NETDEV_LOCAL_ADDRESS_TYPE_MAX] = { - [NETDEV_LOCAL_ADDRESS_IPV4LL] = "ipv4_link_local", - [NETDEV_LOCAL_ADDRESS_IPV6LL] = "ipv6_link_local", - [NETDEV_LOCAL_ADDRESS_DHCP4] = "dhcp4", - [NETDEV_LOCAL_ADDRESS_DHCP6] = "dhcp6", - [NETDEV_LOCAL_ADDRESS_SLAAC] = "slaac", + [NETDEV_LOCAL_ADDRESS_IPV4LL] = "ipv4_link_local", + [NETDEV_LOCAL_ADDRESS_IPV6LL] = "ipv6_link_local", + [NETDEV_LOCAL_ADDRESS_DHCP4] = "dhcp4", + [NETDEV_LOCAL_ADDRESS_DHCP6] = "dhcp6", + [NETDEV_LOCAL_ADDRESS_SLAAC] = "slaac", }; DEFINE_STRING_TABLE_LOOKUP(netdev_local_address_type, NetDevLocalAddressType); diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index 84da73c711e..bafe696ae5f 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -25,7 +25,7 @@ static const char* const ip6tnl_mode_table[_NETDEV_IP6_TNL_MODE_MAX] = { [NETDEV_IP6_TNL_MODE_IP6IP6] = "ip6ip6", - [NETDEV_IP6_TNL_MODE_IPIP6] = "ipip6", + [NETDEV_IP6_TNL_MODE_IPIP6] = "ipip6", [NETDEV_IP6_TNL_MODE_ANYIP6] = "any", }; diff --git a/src/network/netdev/vxlan.c b/src/network/netdev/vxlan.c index b7d3ee50859..2582ae8cab4 100644 --- a/src/network/netdev/vxlan.c +++ b/src/network/netdev/vxlan.c @@ -14,8 +14,8 @@ #include "vxlan.h" static const char* const df_table[_NETDEV_VXLAN_DF_MAX] = { - [NETDEV_VXLAN_DF_NO] = "no", - [NETDEV_VXLAN_DF_YES] = "yes", + [NETDEV_VXLAN_DF_NO] = "no", + [NETDEV_VXLAN_DF_YES] = "yes", [NETDEV_VXLAN_DF_INHERIT] = "inherit", }; From 117843fe95f091df1ddc87aaa60f1a2a8698d120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2023 12:52:26 +0200 Subject: [PATCH 11/12] network: make DEFINE_NETDEV_CAST() assert on input and output The macro used to return NULL if input was NULL or had the wrong type. Now it asserts that input is nonnull and it has the expected type. There are a few places where a missing or mismatched type was OK, but in a majority of places, we would do both of the asserts. In various places we'd only do one, but that was by ommission/mistake. So moving the asserts into the macro allows us to save some lines. --- src/network/netdev/bareudp.c | 10 +--- src/network/netdev/batadv.c | 16 ++---- src/network/netdev/bond.c | 13 ++--- src/network/netdev/bridge.c | 6 +- src/network/netdev/fou-tunnel.c | 16 +----- src/network/netdev/geneve.c | 10 +--- src/network/netdev/ipoib.c | 17 +----- src/network/netdev/ipvlan.c | 31 ++-------- src/network/netdev/l2tp-tunnel.c | 49 ++++------------ src/network/netdev/macsec.c | 37 ++++-------- src/network/netdev/macvlan.c | 39 +++---------- src/network/netdev/netdev.h | 15 +++-- src/network/netdev/tunnel.c | 37 +++++------- src/network/netdev/veth.c | 28 ++------- src/network/netdev/vlan.c | 29 ++-------- src/network/netdev/vrf.c | 9 +-- src/network/netdev/vxcan.c | 26 ++------- src/network/netdev/vxlan.c | 33 +++-------- src/network/netdev/wireguard.c | 98 ++++++++------------------------ src/network/netdev/wlan.c | 42 ++------------ src/network/netdev/xfrm.c | 6 +- src/network/networkd-network.c | 2 - src/network/networkd-route.c | 14 ++--- 23 files changed, 137 insertions(+), 446 deletions(-) diff --git a/src/network/netdev/bareudp.c b/src/network/netdev/bareudp.c index 539f8486e5c..1df886573ba 100644 --- a/src/network/netdev/bareudp.c +++ b/src/network/netdev/bareudp.c @@ -21,10 +21,9 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_bare_udp_iftype, bare_udp_protocol, BareUD "Failed to parse EtherType="); static int netdev_bare_udp_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - assert(netdev); assert(m); - BareUDP *u = ASSERT_PTR(BAREUDP(netdev)); + BareUDP *u = BAREUDP(netdev); int r; r = sd_netlink_message_append_u16(m, IFLA_BAREUDP_ETHERTYPE, htobe16(u->iftype)); @@ -39,10 +38,9 @@ static int netdev_bare_udp_fill_message_create(NetDev *netdev, Link *link, sd_ne } static int netdev_bare_udp_verify(NetDev *netdev, const char *filename) { - assert(netdev); assert(filename); - BareUDP *u = ASSERT_PTR(BAREUDP(netdev)); + BareUDP *u = BAREUDP(netdev); if (u->dest_port == 0) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), @@ -56,9 +54,7 @@ static int netdev_bare_udp_verify(NetDev *netdev, const char *filename) { } static void bare_udp_init(NetDev *netdev) { - assert(netdev); - - BareUDP *u = ASSERT_PTR(BAREUDP(netdev)); + BareUDP *u = BAREUDP(netdev); u->iftype = _BARE_UDP_PROTOCOL_INVALID; } diff --git a/src/network/netdev/batadv.c b/src/network/netdev/batadv.c index 7e97619657a..26da0231d45 100644 --- a/src/network/netdev/batadv.c +++ b/src/network/netdev/batadv.c @@ -16,9 +16,7 @@ #include "string-util.h" static void batadv_init(NetDev *n) { - BatmanAdvanced *b; - - b = BATADV(n); + BatmanAdvanced *b = BATADV(n); /* Set defaults */ b->aggregation = true; @@ -115,11 +113,9 @@ static int netdev_batman_set_handler(sd_netlink *rtnl, sd_netlink_message *m, Ne } static int netdev_batadv_post_create_message(NetDev *netdev, sd_netlink_message *message) { - BatmanAdvanced *b; + BatmanAdvanced *b = BATADV(netdev); int r; - assert_se(b = BATADV(netdev)); - r = sd_netlink_message_append_u32(message, BATADV_ATTR_MESH_IFINDEX, netdev->ifindex); if (r < 0) return r; @@ -188,14 +184,10 @@ static int netdev_batadv_post_create(NetDev *netdev, Link *link) { } static int netdev_batadv_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - BatmanAdvanced *b; - int r; - - assert(netdev); assert(m); - b = BATADV(netdev); - assert(b); + BatmanAdvanced *b = BATADV(netdev); + int r; r = sd_netlink_message_append_string(m, IFLA_BATADV_ALGO_NAME, batadv_routing_algorithm_kernel_to_string(b->routing_algorithm)); if (r < 0) diff --git a/src/network/netdev/bond.c b/src/network/netdev/bond.c index 6bedf0cd3ea..4d75a0d6bf2 100644 --- a/src/network/netdev/bond.c +++ b/src/network/netdev/bond.c @@ -57,11 +57,10 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_bond_arp_all_targets, bond_arp_all_targets DEFINE_CONFIG_PARSE_ENUM(config_parse_bond_primary_reselect, bond_primary_reselect, BondPrimaryReselect, "Failed to parse bond primary reselect"); static int netdev_bond_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - assert(netdev); assert(!link); assert(m); - Bond *b = ASSERT_PTR(BOND(netdev)); + Bond *b = BOND(netdev); int r; if (b->mode != _NETDEV_BOND_MODE_INVALID) { @@ -238,7 +237,7 @@ int config_parse_arp_ip_target_address( assert(rvalue); assert(data); - Bond *b = ASSERT_PTR(BOND(userdata)); + Bond *b = BOND(userdata); int r; if (isempty(rvalue)) { @@ -378,17 +377,13 @@ int config_parse_ad_actor_system( } static void bond_done(NetDev *netdev) { - assert(netdev); - - Bond *b = ASSERT_PTR(BOND(netdev)); + Bond *b = BOND(netdev); ordered_set_free(b->arp_ip_targets); } static void bond_init(NetDev *netdev) { - assert(netdev); - - Bond *b = ASSERT_PTR(BOND(netdev)); + Bond *b = BOND(netdev); b->mode = _NETDEV_BOND_MODE_INVALID; b->xmit_hash_policy = _NETDEV_BOND_XMIT_HASH_POLICY_INVALID; diff --git a/src/network/netdev/bridge.c b/src/network/netdev/bridge.c index 19638a871f8..3e394edadfd 100644 --- a/src/network/netdev/bridge.c +++ b/src/network/netdev/bridge.c @@ -46,9 +46,7 @@ static int netdev_bridge_set_handler(sd_netlink *rtnl, sd_netlink_message *m, Ne } static int netdev_bridge_post_create_message(NetDev *netdev, sd_netlink_message *req) { - assert(netdev); - - Bridge *b = ASSERT_PTR(BRIDGE(netdev)); + Bridge *b = BRIDGE(netdev); int r; r = sd_netlink_message_open_container(req, IFLA_LINKINFO); @@ -232,7 +230,7 @@ int config_parse_bridge_port_priority( } static void bridge_init(NetDev *netdev) { - Bridge *b = ASSERT_PTR(BRIDGE(netdev)); + Bridge *b = BRIDGE(netdev); b->mcast_querier = -1; b->mcast_snooping = -1; diff --git a/src/network/netdev/fou-tunnel.c b/src/network/netdev/fou-tunnel.c index 12e8e462a21..3bf41a8bf29 100644 --- a/src/network/netdev/fou-tunnel.c +++ b/src/network/netdev/fou-tunnel.c @@ -24,9 +24,7 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_fou_encap_type, fou_encap_type, FooOverUDP "Failed to parse Encapsulation="); static int netdev_fill_fou_tunnel_message(NetDev *netdev, sd_netlink_message *m) { - assert(netdev); - - FouTunnel *t = ASSERT_PTR(FOU(netdev)); + FouTunnel *t = FOU(netdev); uint8_t encap_type; int r; @@ -128,7 +126,6 @@ static int netdev_fou_tunnel_create(NetDev *netdev) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; int r; - assert(netdev); assert(FOU(netdev)); r = netdev_create_fou_tunnel_message(netdev, &m); @@ -220,10 +217,9 @@ int config_parse_fou_tunnel_address( } static int netdev_fou_tunnel_verify(NetDev *netdev, const char *filename) { - assert(netdev); assert(filename); - FouTunnel *t = ASSERT_PTR(FOU(netdev)); + FouTunnel *t = FOU(netdev); switch (t->fou_encap_type) { case NETDEV_FOO_OVER_UDP_ENCAP_DIRECT: @@ -254,13 +250,7 @@ static int netdev_fou_tunnel_verify(NetDev *netdev, const char *filename) { } static void fou_tunnel_init(NetDev *netdev) { - FouTunnel *t; - - assert(netdev); - - t = FOU(netdev); - - assert(t); + FouTunnel *t = FOU(netdev); t->fou_encap_type = NETDEV_FOO_OVER_UDP_ENCAP_DIRECT; } diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c index 5615f5bdcea..bc655ec7ffd 100644 --- a/src/network/netdev/geneve.c +++ b/src/network/netdev/geneve.c @@ -28,10 +28,9 @@ DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(geneve_df, GeneveDF, NETDEV_GENEVE_DF_YE DEFINE_CONFIG_PARSE_ENUM(config_parse_geneve_df, geneve_df, GeneveDF, "Failed to parse Geneve IPDoNotFragment= setting"); static int netdev_geneve_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - assert(netdev); assert(m); - Geneve *v = ASSERT_PTR(GENEVE(netdev)); + Geneve *v = GENEVE(netdev); int r; if (v->id <= GENEVE_VID_MAX) { @@ -243,10 +242,9 @@ int config_parse_geneve_ttl( } static int netdev_geneve_verify(NetDev *netdev, const char *filename) { - assert(netdev); assert(filename); - Geneve *v = ASSERT_PTR(GENEVE(netdev)); + Geneve *v = GENEVE(netdev); if (v->id > GENEVE_VID_MAX) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), @@ -256,9 +254,7 @@ static int netdev_geneve_verify(NetDev *netdev, const char *filename) { } static void geneve_init(NetDev *netdev) { - assert(netdev); - - Geneve *v = ASSERT_PTR(GENEVE(netdev)); + Geneve *v = GENEVE(netdev); v->id = GENEVE_VID_MAX + 1; v->geneve_df = _NETDEV_GENEVE_DF_INVALID; diff --git a/src/network/netdev/ipoib.c b/src/network/netdev/ipoib.c index 5dd9286d57f..d5fe299b7b4 100644 --- a/src/network/netdev/ipoib.c +++ b/src/network/netdev/ipoib.c @@ -12,29 +12,18 @@ assert_cc((int) IP_OVER_INFINIBAND_MODE_DATAGRAM == (int) IPOIB_MODE_DATAGRAM); assert_cc((int) IP_OVER_INFINIBAND_MODE_CONNECTED == (int) IPOIB_MODE_CONNECTED); static void netdev_ipoib_init(NetDev *netdev) { - IPoIB *ipoib; - - assert(netdev); - - ipoib = IPOIB(netdev); - - assert(ipoib); + IPoIB *ipoib = IPOIB(netdev); ipoib->mode = _IP_OVER_INFINIBAND_MODE_INVALID; ipoib->umcast = -1; } static int netdev_ipoib_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - IPoIB *ipoib; - int r; - - assert(netdev); assert(link); assert(m); - ipoib = IPOIB(netdev); - - assert(ipoib); + IPoIB *ipoib = IPOIB(netdev); + int r; if (ipoib->pkey > 0) { r = sd_netlink_message_append_u16(m, IFLA_IPOIB_PKEY, ipoib->pkey); diff --git a/src/network/netdev/ipvlan.c b/src/network/netdev/ipvlan.c index 058eadebd7a..05d5d010f65 100644 --- a/src/network/netdev/ipvlan.c +++ b/src/network/netdev/ipvlan.c @@ -14,19 +14,12 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_ipvlan_mode, ipvlan_mode, IPVlanMode, "Fai DEFINE_CONFIG_PARSE_ENUM(config_parse_ipvlan_flags, ipvlan_flags, IPVlanFlags, "Failed to parse ipvlan flags"); static int netdev_ipvlan_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *req) { - IPVlan *m; - int r; - assert(netdev); assert(link); assert(netdev->ifname); - if (netdev->kind == NETDEV_KIND_IPVLAN) - m = IPVLAN(netdev); - else - m = IPVTAP(netdev); - - assert(m); + IPVlan *m = netdev->kind == NETDEV_KIND_IPVLAN ? IPVLAN(netdev) : IPVTAP(netdev); + int r; if (m->mode != _NETDEV_IPVLAN_MODE_INVALID) { r = sd_netlink_message_append_u16(req, IFLA_IPVLAN_MODE, m->mode); @@ -43,17 +36,8 @@ static int netdev_ipvlan_fill_message_create(NetDev *netdev, Link *link, sd_netl return 0; } -static void ipvlan_init(NetDev *n) { - IPVlan *m; - - assert(n); - - if (n->kind == NETDEV_KIND_IPVLAN) - m = IPVLAN(n); - else - m = IPVTAP(n); - - assert(m); +static void ipvlan_init(NetDev *netdev) { + IPVlan *m = ASSERT_PTR(netdev)->kind == NETDEV_KIND_IPVLAN ? IPVLAN(netdev) : IPVTAP(netdev); m->mode = _NETDEV_IPVLAN_MODE_INVALID; m->flags = _NETDEV_IPVLAN_FLAGS_INVALID; @@ -80,13 +64,10 @@ const NetDevVTable ipvtap_vtable = { }; IPVlanMode link_get_ipvlan_mode(Link *link) { - IPVlan *ipvlan; - assert(link); - ipvlan = IPVLAN(link->netdev); - if (!ipvlan) + if (!link->netdev || link->netdev->kind != NETDEV_KIND_IPVLAN) return _NETDEV_IPVLAN_MODE_INVALID; - return ipvlan->mode; + return IPVLAN(link->netdev)->mode; } diff --git a/src/network/netdev/l2tp-tunnel.c b/src/network/netdev/l2tp-tunnel.c index 17d5391cd38..8b9406bb081 100644 --- a/src/network/netdev/l2tp-tunnel.c +++ b/src/network/netdev/l2tp-tunnel.c @@ -92,15 +92,13 @@ static int l2tp_session_new_static(L2tpTunnel *t, const char *filename, unsigned } static int netdev_l2tp_create_message_tunnel(NetDev *netdev, union in_addr_union *local_address, sd_netlink_message **ret) { + assert(local_address); + _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; uint16_t encap_type; - L2tpTunnel *t; + L2tpTunnel *t = L2TP(netdev); int r; - assert(netdev); - assert(local_address); - assert_se(t = L2TP(netdev)); - r = sd_genl_message_new(netdev->manager->genl, L2TP_GENL_NAME, L2TP_CMD_TUNNEL_CREATE, &m); if (r < 0) return r; @@ -278,13 +276,11 @@ static int link_get_l2tp_local_address(Link *link, L2tpTunnel *t, union in_addr_ static int l2tp_get_local_address(NetDev *netdev, union in_addr_union *ret) { Link *link = NULL; - L2tpTunnel *t; + L2tpTunnel *t = L2TP(netdev); Address *a = NULL; int r; - assert(netdev); assert(netdev->manager); - assert_se(t = L2TP(netdev)); if (t->local_ifname) { r = link_get_by_name(netdev->manager, t->local_ifname, &link); @@ -403,16 +399,11 @@ static int l2tp_create_session(NetDev *netdev, L2tpSession *session) { static int l2tp_create_tunnel_handler(sd_netlink *rtnl, sd_netlink_message *m, NetDev *netdev) { L2tpSession *session; - L2tpTunnel *t; + L2tpTunnel *t = L2TP(netdev); int r; - assert(netdev); assert(netdev->state != _NETDEV_STATE_INVALID); - t = L2TP(netdev); - - assert(t); - r = sd_netlink_message_get_errno(m); if (r == -EEXIST) log_netdev_info(netdev, "netdev exists, using existing without changing its parameters"); @@ -434,12 +425,9 @@ static int l2tp_create_tunnel_handler(sd_netlink *rtnl, sd_netlink_message *m, N static int l2tp_create_tunnel(NetDev *netdev) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; union in_addr_union local_address; - L2tpTunnel *t; + L2tpTunnel *t = L2TP(netdev); int r; - assert(netdev); - assert_se(t = L2TP(netdev)); - r = l2tp_get_local_address(netdev, &local_address); if (r < 0) return log_netdev_error_errno(netdev, r, "Could not find local address."); @@ -757,13 +745,7 @@ int config_parse_l2tp_session_name( } static void l2tp_tunnel_init(NetDev *netdev) { - L2tpTunnel *t; - - assert(netdev); - - t = L2TP(netdev); - - assert(t); + L2tpTunnel *t = L2TP(netdev); t->l2tp_encap_type = NETDEV_L2TP_ENCAPTYPE_UDP; t->udp6_csum_rx = true; @@ -797,15 +779,10 @@ static int l2tp_session_verify(L2tpSession *session) { } static int netdev_l2tp_tunnel_verify(NetDev *netdev, const char *filename) { - L2tpTunnel *t; - L2tpSession *session; - - assert(netdev); assert(filename); - t = L2TP(netdev); - - assert(t); + L2tpTunnel *t = L2TP(netdev); + L2tpSession *session; if (!IN_SET(t->family, AF_INET, AF_INET6)) return log_netdev_error_errno(netdev, SYNTHETIC_ERRNO(EINVAL), @@ -830,13 +807,7 @@ static int netdev_l2tp_tunnel_verify(NetDev *netdev, const char *filename) { } static void l2tp_tunnel_done(NetDev *netdev) { - L2tpTunnel *t; - - assert(netdev); - - t = L2TP(netdev); - - assert(t); + L2tpTunnel *t = L2TP(netdev); ordered_hashmap_free_with_destructor(t->sessions_by_section, l2tp_session_free); free(t->local_ifname); diff --git a/src/network/netdev/macsec.c b/src/network/netdev/macsec.c index 6469a97b29e..6d17d45059a 100644 --- a/src/network/netdev/macsec.c +++ b/src/network/netdev/macsec.c @@ -472,9 +472,7 @@ static int netdev_macsec_configure_transmit_association(NetDev *netdev, Transmit } static int netdev_macsec_configure(NetDev *netdev, Link *link) { - assert(netdev); - - MACsec *s = ASSERT_PTR(MACSEC(netdev)); + MACsec *s = MACSEC(netdev); TransmitAssociation *a; ReceiveChannel *c; int r; @@ -495,10 +493,9 @@ static int netdev_macsec_configure(NetDev *netdev, Link *link) { } static int netdev_macsec_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - assert(netdev); assert(m); - MACsec *v = ASSERT_PTR(MACSEC(netdev)); + MACsec *v = MACSEC(netdev); int r; if (v->port > 0) { @@ -1118,6 +1115,8 @@ static int macsec_receive_association_verify(ReceiveAssociation *a) { } static int netdev_macsec_verify(NetDev *netdev, const char *filename) { + assert(filename); + MACsec *v = MACSEC(netdev); TransmitAssociation *a; ReceiveAssociation *n; @@ -1126,10 +1125,6 @@ static int netdev_macsec_verify(NetDev *netdev, const char *filename) { bool use_for_encoding; int r; - assert(netdev); - assert(v); - assert(filename); - ORDERED_HASHMAP_FOREACH(c, v->receive_channels_by_section) { r = macsec_receive_channel_verify(c); if (r < 0) @@ -1184,30 +1179,18 @@ static int netdev_macsec_verify(NetDev *netdev, const char *filename) { } static void macsec_init(NetDev *netdev) { - MACsec *v; - - assert(netdev); - - v = MACSEC(netdev); - - assert(v); + MACsec *v = MACSEC(netdev); v->encrypt = -1; } static void macsec_done(NetDev *netdev) { - MACsec *t; + MACsec *v = MACSEC(netdev); - assert(netdev); - - t = MACSEC(netdev); - - assert(t); - - ordered_hashmap_free_with_destructor(t->receive_channels, macsec_receive_channel_free); - ordered_hashmap_free_with_destructor(t->receive_channels_by_section, macsec_receive_channel_free); - ordered_hashmap_free_with_destructor(t->transmit_associations_by_section, macsec_transmit_association_free); - ordered_hashmap_free_with_destructor(t->receive_associations_by_section, macsec_receive_association_free); + ordered_hashmap_free_with_destructor(v->receive_channels, macsec_receive_channel_free); + ordered_hashmap_free_with_destructor(v->receive_channels_by_section, macsec_receive_channel_free); + ordered_hashmap_free_with_destructor(v->transmit_associations_by_section, macsec_transmit_association_free); + ordered_hashmap_free_with_destructor(v->receive_associations_by_section, macsec_receive_association_free); } const NetDevVTable macsec_vtable = { diff --git a/src/network/netdev/macvlan.c b/src/network/netdev/macvlan.c index 6083cd2b44e..203807e3a56 100644 --- a/src/network/netdev/macvlan.c +++ b/src/network/netdev/macvlan.c @@ -13,20 +13,13 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_macvlan_mode, macvlan_mode, MacVlanMode, "Failed to parse macvlan mode"); static int netdev_macvlan_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *req) { - MacVlan *m; - int r; - assert(netdev); - assert(link); assert(netdev->ifname); + assert(link); assert(link->network); - if (netdev->kind == NETDEV_KIND_MACVLAN) - m = MACVLAN(netdev); - else - m = MACVTAP(netdev); - - assert(m); + MacVlan *m = netdev->kind == NETDEV_KIND_MACVLAN ? MACVLAN(netdev) : MACVTAP(netdev); + int r; if (m->mode == NETDEV_MACVLAN_MODE_SOURCE && !set_isempty(m->match_source_mac)) { const struct ether_addr *mac_addr; @@ -103,32 +96,14 @@ int config_parse_macvlan_broadcast_queue_size( &m->bc_queue_length); } -static void macvlan_done(NetDev *n) { - MacVlan *m; - - assert(n); - - if (n->kind == NETDEV_KIND_MACVLAN) - m = MACVLAN(n); - else - m = MACVTAP(n); - - assert(m); +static void macvlan_done(NetDev *netdev) { + MacVlan *m = ASSERT_PTR(netdev)->kind == NETDEV_KIND_MACVLAN ? MACVLAN(netdev) : MACVTAP(netdev); set_free(m->match_source_mac); } -static void macvlan_init(NetDev *n) { - MacVlan *m; - - assert(n); - - if (n->kind == NETDEV_KIND_MACVLAN) - m = MACVLAN(n); - else - m = MACVTAP(n); - - assert(m); +static void macvlan_init(NetDev *netdev) { + MacVlan *m = ASSERT_PTR(netdev)->kind == NETDEV_KIND_MACVLAN ? MACVLAN(netdev) : MACVTAP(netdev); m->mode = _NETDEV_MACVLAN_MODE_INVALID; m->bc_queue_length = UINT32_MAX; diff --git a/src/network/netdev/netdev.h b/src/network/netdev/netdev.h index 49eadbb7a44..cb8cc8c6a9d 100644 --- a/src/network/netdev/netdev.h +++ b/src/network/netdev/netdev.h @@ -182,14 +182,13 @@ extern const NetDevVTable * const netdev_vtable[_NETDEV_KIND_MAX]; #define NETDEV_VTABLE(n) ((n)->kind != _NETDEV_KIND_INVALID ? netdev_vtable[(n)->kind] : NULL) /* For casting a netdev into the various netdev kinds */ -#define DEFINE_NETDEV_CAST(UPPERCASE, MixedCase) \ - static inline MixedCase* UPPERCASE(NetDev *n) { \ - if (_unlikely_(!n || \ - n->kind != NETDEV_KIND_##UPPERCASE) || \ - n->state == _NETDEV_STATE_INVALID) \ - return NULL; \ - \ - return (MixedCase*) n; \ +#define DEFINE_NETDEV_CAST(UPPERCASE, MixedCase) \ + static inline MixedCase* UPPERCASE(NetDev *n) { \ + assert(n); \ + assert(n->kind == NETDEV_KIND_##UPPERCASE); \ + assert(n->state < _NETDEV_STATE_MAX); \ + \ + return (MixedCase*) n; \ } /* For casting the various netdev kinds into a netdev */ diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index bafe696ae5f..db84e7cf6ee 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -199,17 +199,11 @@ static int tunnel_get_local_address(Tunnel *t, Link *link, union in_addr_union * } static int netdev_ipip_sit_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - union in_addr_union local; - Tunnel *t; - int r; - - assert(netdev); assert(m); - if (netdev->kind == NETDEV_KIND_IPIP) - t = ASSERT_PTR(IPIP(netdev)); - else - t = ASSERT_PTR(SIT(netdev)); + union in_addr_union local; + Tunnel *t = ASSERT_PTR(netdev)->kind == NETDEV_KIND_IPIP ? IPIP(netdev) : SIT(netdev); + int r; if (t->external) { r = sd_netlink_message_append_flag(m, IFLA_IPTUN_COLLECT_METADATA); @@ -302,13 +296,13 @@ static int netdev_gre_erspan_fill_message_create(NetDev *netdev, Link *link, sd_ switch (netdev->kind) { case NETDEV_KIND_GRE: - t = ASSERT_PTR(GRE(netdev)); + t = GRE(netdev); break; case NETDEV_KIND_ERSPAN: - t = ASSERT_PTR(ERSPAN(netdev)); + t = ERSPAN(netdev); break; case NETDEV_KIND_GRETAP: - t = ASSERT_PTR(GRETAP(netdev)); + t = GRETAP(netdev); break; default: assert_not_reached(); @@ -446,9 +440,9 @@ static int netdev_ip6gre_fill_message_create(NetDev *netdev, Link *link, sd_netl assert(m); if (netdev->kind == NETDEV_KIND_IP6GRE) - t = ASSERT_PTR(IP6GRE(netdev)); + t = IP6GRE(netdev); else - t = ASSERT_PTR(IP6GRETAP(netdev)); + t = IP6GRETAP(netdev); if (t->external) { r = sd_netlink_message_append_flag(m, IFLA_GRE_COLLECT_METADATA); @@ -527,18 +521,13 @@ static int netdev_ip6gre_fill_message_create(NetDev *netdev, Link *link, sd_netl } static int netdev_vti_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - union in_addr_union local; - uint32_t ikey, okey; - Tunnel *t; - int r; - assert(netdev); assert(m); - if (netdev->kind == NETDEV_KIND_VTI) - t = ASSERT_PTR(VTI(netdev)); - else - t = ASSERT_PTR(VTI6(netdev)); + union in_addr_union local; + uint32_t ikey, okey; + Tunnel *t = netdev->kind == NETDEV_KIND_VTI ? VTI(netdev) : VTI6(netdev); + int r; if (link || t->assign_to_loopback) { r = sd_netlink_message_append_u32(m, IFLA_VTI_LINK, link ? link->ifindex : LOOPBACK_IFINDEX); @@ -582,7 +571,7 @@ static int netdev_ip6tnl_fill_message_create(NetDev *netdev, Link *link, sd_netl union in_addr_union local; uint8_t proto; - Tunnel *t = ASSERT_PTR(IP6TNL(netdev)); + Tunnel *t = IP6TNL(netdev); int r; switch (t->ip6tnl_mode) { diff --git a/src/network/netdev/veth.c b/src/network/netdev/veth.c index fb00e6667fb..e0f5b4ebb1a 100644 --- a/src/network/netdev/veth.c +++ b/src/network/netdev/veth.c @@ -10,17 +10,12 @@ #include "veth.h" static int netdev_veth_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - struct hw_addr_data hw_addr; - Veth *v; - int r; - - assert(netdev); assert(!link); assert(m); - v = VETH(netdev); - - assert(v); + struct hw_addr_data hw_addr; + Veth *v = VETH(netdev); + int r; r = sd_netlink_message_open_container(m, VETH_INFO_PEER); if (r < 0) @@ -57,14 +52,9 @@ static int netdev_veth_fill_message_create(NetDev *netdev, Link *link, sd_netlin } static int netdev_veth_verify(NetDev *netdev, const char *filename) { - Veth *v; - - assert(netdev); assert(filename); - v = VETH(netdev); - - assert(v); + Veth *v = VETH(netdev); if (!v->ifname_peer) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), @@ -74,14 +64,8 @@ static int netdev_veth_verify(NetDev *netdev, const char *filename) { return 0; } -static void veth_done(NetDev *n) { - Veth *v; - - assert(n); - - v = VETH(n); - - assert(v); +static void veth_done(NetDev *netdev) { + Veth *v = VETH(netdev); free(v->ifname_peer); } diff --git a/src/network/netdev/vlan.c b/src/network/netdev/vlan.c index 5eb36ef6801..2390206993b 100644 --- a/src/network/netdev/vlan.c +++ b/src/network/netdev/vlan.c @@ -10,17 +10,12 @@ #include "vlan.h" static int netdev_vlan_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *req) { - struct ifla_vlan_flags flags = {}; - VLan *v; - int r; - - assert(netdev); assert(link); assert(req); - v = VLAN(netdev); - - assert(v); + struct ifla_vlan_flags flags = {}; + VLan *v = VLAN(netdev); + int r; r = sd_netlink_message_append_u16(req, IFLA_VLAN_ID, v->id); if (r < 0) @@ -180,14 +175,9 @@ int config_parse_vlan_qos_maps( } static int netdev_vlan_verify(NetDev *netdev, const char *filename) { - VLan *v; - - assert(netdev); assert(filename); - v = VLAN(netdev); - - assert(v); + VLan *v = VLAN(netdev); if (v->id == VLANID_INVALID) { log_netdev_warning(netdev, "VLAN without valid Id (%"PRIu16") configured in %s.", v->id, filename); @@ -197,12 +187,8 @@ static int netdev_vlan_verify(NetDev *netdev, const char *filename) { return 0; } -static void vlan_done(NetDev *n) { - VLan *v; - - v = VLAN(n); - - assert(v); +static void vlan_done(NetDev *netdev) { + VLan *v = VLAN(netdev); set_free(v->egress_qos_maps); set_free(v->ingress_qos_maps); @@ -211,9 +197,6 @@ static void vlan_done(NetDev *n) { static void vlan_init(NetDev *netdev) { VLan *v = VLAN(netdev); - assert(netdev); - assert(v); - v->id = VLANID_INVALID; v->protocol = -1; v->gvrp = -1; diff --git a/src/network/netdev/vrf.c b/src/network/netdev/vrf.c index 05ef3ff13d3..b75ec2bcc68 100644 --- a/src/network/netdev/vrf.c +++ b/src/network/netdev/vrf.c @@ -7,16 +7,11 @@ #include "vrf.h" static int netdev_vrf_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - Vrf *v; - int r; - - assert(netdev); assert(!link); assert(m); - v = VRF(netdev); - - assert(v); + Vrf *v = VRF(netdev); + int r; r = sd_netlink_message_append_u32(m, IFLA_VRF_TABLE, v->table); if (r < 0) diff --git a/src/network/netdev/vxcan.c b/src/network/netdev/vxcan.c index 83269b07071..c0343f45b62 100644 --- a/src/network/netdev/vxcan.c +++ b/src/network/netdev/vxcan.c @@ -6,16 +6,11 @@ #include "vxcan.h" static int netdev_vxcan_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - VxCan *v; - int r; - - assert(netdev); assert(!link); assert(m); - v = VXCAN(netdev); - - assert(v); + VxCan *v = VXCAN(netdev); + int r; r = sd_netlink_message_open_container(m, VXCAN_INFO_PEER); if (r < 0) @@ -35,14 +30,9 @@ static int netdev_vxcan_fill_message_create(NetDev *netdev, Link *link, sd_netli } static int netdev_vxcan_verify(NetDev *netdev, const char *filename) { - VxCan *v; - - assert(netdev); assert(filename); - v = VXCAN(netdev); - - assert(v); + VxCan *v = VXCAN(netdev); if (!v->ifname_peer) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), @@ -51,14 +41,8 @@ static int netdev_vxcan_verify(NetDev *netdev, const char *filename) { return 0; } -static void vxcan_done(NetDev *n) { - VxCan *v; - - assert(n); - - v = VXCAN(n); - - assert(v); +static void vxcan_done(NetDev *netdev) { + VxCan *v = VXCAN(netdev); free(v->ifname_peer); } diff --git a/src/network/netdev/vxlan.c b/src/network/netdev/vxlan.c index 2582ae8cab4..b11fdbbd0de 100644 --- a/src/network/netdev/vxlan.c +++ b/src/network/netdev/vxlan.c @@ -37,16 +37,11 @@ static int vxlan_get_local_address(VxLan *v, Link *link, int *ret_family, union } static int netdev_vxlan_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - union in_addr_union local; - int local_family, r; - VxLan *v; - - assert(netdev); assert(m); - v = VXLAN(netdev); - - assert(v); + union in_addr_union local; + int local_family, r; + VxLan *v = VXLAN(netdev); if (v->vni <= VXLAN_VID_MAX) { r = sd_netlink_message_append_u32(m, IFLA_VXLAN_ID, v->vni); @@ -376,12 +371,10 @@ int config_parse_vxlan_ttl( } static int netdev_vxlan_verify(NetDev *netdev, const char *filename) { - VxLan *v = VXLAN(netdev); - - assert(netdev); - assert(v); assert(filename); + VxLan *v = VXLAN(netdev); + if (v->vni > VXLAN_VID_MAX) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), "%s: VXLAN without valid VNI (or VXLAN Segment ID) configured. Ignoring.", @@ -409,13 +402,7 @@ static int netdev_vxlan_verify(NetDev *netdev, const char *filename) { } static int netdev_vxlan_is_ready_to_create(NetDev *netdev, Link *link) { - VxLan *v; - - assert(netdev); - - v = VXLAN(netdev); - - assert(v); + VxLan *v = VXLAN(netdev); if (v->independent) return true; @@ -424,13 +411,7 @@ static int netdev_vxlan_is_ready_to_create(NetDev *netdev, Link *link) { } static void vxlan_init(NetDev *netdev) { - VxLan *v; - - assert(netdev); - - v = VXLAN(netdev); - - assert(v); + VxLan *v = VXLAN(netdev); v->local_type = _NETDEV_LOCAL_ADDRESS_TYPE_INVALID; v->vni = VXLAN_VID_MAX + 1; diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 0e992f9ad1b..14f664e9830 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -228,13 +228,9 @@ static int wireguard_set_interface(NetDev *netdev) { WireguardPeer *peer_start; bool sent_once = false; uint32_t serial; - Wireguard *w; + Wireguard *w = WIREGUARD(netdev); int r; - assert(netdev); - w = WIREGUARD(netdev); - assert(w); - for (peer_start = w->peers; peer_start || !sent_once; ) { uint16_t i = 0; @@ -428,11 +424,7 @@ static int peer_resolve_endpoint(WireguardPeer *peer) { } static void wireguard_resolve_endpoints(NetDev *netdev) { - Wireguard *w; - - assert(netdev); - w = WIREGUARD(netdev); - assert(w); + Wireguard *w = WIREGUARD(netdev); LIST_FOREACH(peers, peer, w->peers) if (peer_resolve_endpoint(peer) == -ENOBUFS) @@ -441,7 +433,6 @@ static void wireguard_resolve_endpoints(NetDev *netdev) { } static int netdev_wireguard_post_create(NetDev *netdev, Link *link) { - assert(netdev); assert(WIREGUARD(netdev)); (void) wireguard_set_interface(netdev); @@ -537,11 +528,7 @@ int config_parse_wireguard_private_key( void *data, void *userdata) { - Wireguard *w; - - assert(data); - w = WIREGUARD(data); - assert(w); + Wireguard *w = WIREGUARD(data); return wireguard_decode_key_and_warn(rvalue, w->private_key, unit, filename, line, lvalue); } @@ -558,12 +545,8 @@ int config_parse_wireguard_private_key_file( void *data, void *userdata) { + Wireguard *w = WIREGUARD(data); _cleanup_free_ char *path = NULL; - Wireguard *w; - - assert(data); - w = WIREGUARD(data); - assert(w); if (isempty(rvalue)) { w->private_key_file = mfree(w->private_key_file); @@ -592,14 +575,10 @@ int config_parse_wireguard_peer_key( void *data, void *userdata) { + Wireguard *w = WIREGUARD(data); _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; - Wireguard *w; int r; - assert(data); - w = WIREGUARD(data); - assert(w); - r = wireguard_peer_new_static(w, filename, section_line, &peer); if (r < 0) return log_oom(); @@ -626,15 +605,11 @@ int config_parse_wireguard_preshared_key_file( void *data, void *userdata) { + Wireguard *w = WIREGUARD(data); _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; _cleanup_free_ char *path = NULL; - Wireguard *w; int r; - assert(data); - w = WIREGUARD(data); - assert(w); - r = wireguard_peer_new_static(w, filename, section_line, &peer); if (r < 0) return log_oom(); @@ -669,19 +644,15 @@ int config_parse_wireguard_allowed_ips( void *data, void *userdata) { + assert(rvalue); + + Wireguard *w = WIREGUARD(data); _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; union in_addr_union addr; unsigned char prefixlen; int r, family; - Wireguard *w; WireguardIPmask *ipmask; - assert(rvalue); - assert(data); - - w = WIREGUARD(data); - assert(w); - r = wireguard_peer_new_static(w, filename, section_line, &peer); if (r < 0) return log_oom(); @@ -751,21 +722,18 @@ int config_parse_wireguard_endpoint( void *data, void *userdata) { + assert(filename); + assert(rvalue); + assert(userdata); + + Wireguard *w = WIREGUARD(userdata); _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; _cleanup_free_ char *host = NULL; union in_addr_union addr; const char *p; uint16_t port; - Wireguard *w; int family, r; - assert(filename); - assert(rvalue); - assert(userdata); - - w = WIREGUARD(userdata); - assert(w); - r = wireguard_peer_new_static(w, filename, section_line, &peer); if (r < 0) return log_oom(); @@ -846,17 +814,13 @@ int config_parse_wireguard_keepalive( void *data, void *userdata) { + assert(rvalue); + + Wireguard *w = WIREGUARD(data); _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; uint16_t keepalive = 0; - Wireguard *w; int r; - assert(rvalue); - assert(data); - - w = WIREGUARD(data); - assert(w); - r = wireguard_peer_new_static(w, filename, section_line, &peer); if (r < 0) return log_oom(); @@ -927,18 +891,14 @@ int config_parse_wireguard_peer_route_table( void *data, void *userdata) { + Wireguard *w = WIREGUARD(userdata); _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; - NetDev *netdev = ASSERT_PTR(userdata); - Wireguard *w; int r; assert(filename); assert(lvalue); assert(rvalue); - assert(netdev->manager); - - w = WIREGUARD(netdev); - assert(w); + assert(NETDEV(w)->manager); r = wireguard_peer_new_static(w, filename, section_line, &peer); if (r < 0) @@ -957,7 +917,7 @@ int config_parse_wireguard_peer_route_table( return 0; } - r = manager_get_route_table_from_string(netdev->manager, rvalue, &peer->route_table); + r = manager_get_route_table_from_string(NETDEV(w)->manager, rvalue, &peer->route_table); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse %s=, ignoring assignment: %s", @@ -1051,21 +1011,13 @@ int config_parse_wireguard_peer_route_priority( } static void wireguard_init(NetDev *netdev) { - Wireguard *w; - - assert(netdev); - w = WIREGUARD(netdev); - assert(w); + Wireguard *w = WIREGUARD(netdev); w->flags = WGDEVICE_F_REPLACE_PEERS; } static void wireguard_done(NetDev *netdev) { - Wireguard *w; - - assert(netdev); - w = WIREGUARD(netdev); - assert(w); + Wireguard *w = WIREGUARD(netdev); explicit_bzero_safe(w->private_key, WG_KEY_LEN); free(w->private_key_file); @@ -1124,13 +1076,9 @@ static int wireguard_peer_verify(WireguardPeer *peer) { } static int wireguard_verify(NetDev *netdev, const char *filename) { - Wireguard *w; + Wireguard *w = WIREGUARD(netdev); int r; - assert(netdev); - w = WIREGUARD(netdev); - assert(w); - r = wireguard_read_key_file(w->private_key_file, w->private_key); if (r < 0) return log_netdev_error_errno(netdev, r, diff --git a/src/network/netdev/wlan.c b/src/network/netdev/wlan.c index 816e1064009..904e40fe81c 100644 --- a/src/network/netdev/wlan.c +++ b/src/network/netdev/wlan.c @@ -12,38 +12,20 @@ #include "wlan.h" static void wlan_done(NetDev *netdev) { - WLan *w; - - assert(netdev); - - w = WLAN(netdev); - - assert(w); + WLan *w = WLAN(netdev); w->wiphy_name = mfree(w->wiphy_name); } static void wlan_init(NetDev *netdev) { - WLan *w; - - assert(netdev); - - w = WLAN(netdev); - - assert(w); + WLan *w = WLAN(netdev); w->wiphy_index = UINT32_MAX; w->wds = -1; } static int wlan_get_wiphy(NetDev *netdev, Wiphy **ret) { - WLan *w; - - assert(netdev); - - w = WLAN(netdev); - - assert(w); + WLan *w = WLAN(netdev); if (w->wiphy_name) return wiphy_get_by_name(netdev->manager, w->wiphy_name, ret); @@ -56,17 +38,10 @@ static int wlan_is_ready_to_create(NetDev *netdev, Link *link) { } static int wlan_fill_message(NetDev *netdev, sd_netlink_message *m) { + WLan *w = WLAN(netdev); Wiphy *wiphy; - WLan *w; int r; - assert(netdev); - assert(m); - - w = WLAN(netdev); - - assert(w); - r = wlan_get_wiphy(netdev, &wiphy); if (r < 0) return r; @@ -145,15 +120,10 @@ static int wlan_create(NetDev *netdev) { } static int wlan_verify(NetDev *netdev, const char *filename) { - WLan *w; + WLan *w = WLAN(netdev); - assert(netdev); assert(filename); - w = WLAN(netdev); - - assert(w); - if (w->iftype == NL80211_IFTYPE_UNSPECIFIED) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), "%s: WLAN interface type is not specified, ignoring.", @@ -179,7 +149,7 @@ int config_parse_wiphy( void *data, void *userdata) { - WLan *w = ASSERT_PTR(userdata); + WLan *w = WLAN(userdata); int r; assert(filename); diff --git a/src/network/netdev/xfrm.c b/src/network/netdev/xfrm.c index a3042831722..905bfc0bdf0 100644 --- a/src/network/netdev/xfrm.c +++ b/src/network/netdev/xfrm.c @@ -6,10 +6,9 @@ #include "xfrm.h" static int xfrm_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *message) { - assert(netdev); assert(message); - Xfrm *x = ASSERT_PTR(XFRM(netdev)); + Xfrm *x = XFRM(netdev); int r; assert(link || x->independent); @@ -26,10 +25,9 @@ static int xfrm_fill_message_create(NetDev *netdev, Link *link, sd_netlink_messa } static int xfrm_verify(NetDev *netdev, const char *filename) { - assert(netdev); assert(filename); - Xfrm *x = ASSERT_PTR(XFRM(netdev)); + Xfrm *x = XFRM(netdev); if (x->if_id == 0) return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index e0a8be6b544..94506c975de 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -209,8 +209,6 @@ int network_verify(Network *network) { else continue; - assert(m); - if (m->mode == NETDEV_MACVLAN_MODE_PASSTHRU) network->link_local = ADDRESS_FAMILY_NO; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 6cefd3a9bf5..023d81b5784 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -889,15 +889,14 @@ static bool route_by_kernel(const Route *route) { } static void link_unmark_wireguard_routes(Link *link) { - Route *route, *existing; - Wireguard *w; - assert(link); - w = WIREGUARD(link->netdev); - if (!w) + if (!link->netdev || link->netdev->kind != NETDEV_KIND_WIREGUARD) return; + Route *route, *existing; + Wireguard *w = WIREGUARD(link->netdev); + SET_FOREACH(route, w->routes) if (route_get(NULL, link, route, &existing) >= 0) route_unmark(existing); @@ -1527,7 +1526,6 @@ static int link_request_static_route(Link *link, Route *route) { static int link_request_wireguard_routes(Link *link, bool only_ipv4) { NetDev *netdev; - Wireguard *w; Route *route; int r; @@ -1539,9 +1537,7 @@ static int link_request_wireguard_routes(Link *link, bool only_ipv4) { if (netdev_get(link->manager, link->ifname, &netdev) < 0) return 0; - w = WIREGUARD(netdev); - if (!w) - return 0; + Wireguard *w = WIREGUARD(netdev); SET_FOREACH(route, w->routes) { if (only_ipv4 && route->family != AF_INET) From f75921c7fd36be2139cae921bd6510c8a33182c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2023 12:56:37 +0200 Subject: [PATCH 12/12] netdev/wireguard: define iterator variable in the loop --- src/network/netdev/wireguard.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 14f664e9830..c89577609d4 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -225,13 +225,12 @@ cancel: static int wireguard_set_interface(NetDev *netdev) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL; WireguardIPmask *mask_start = NULL; - WireguardPeer *peer_start; bool sent_once = false; uint32_t serial; Wireguard *w = WIREGUARD(netdev); int r; - for (peer_start = w->peers; peer_start || !sent_once; ) { + for (WireguardPeer *peer_start = w->peers; peer_start || !sent_once; ) { uint16_t i = 0; message = sd_netlink_message_unref(message);