From 82ddfaa591d989dd8767183a518a61256a4dc30c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 Aug 2024 14:55:41 +0900 Subject: [PATCH 1/6] network/routing-policy-rule: update hash and compare function for fib rule Let's manage fib rules with the logic used by the kernel. Should not change any behavior. --- src/network/networkd-routing-policy-rule.c | 197 ++++++++++----------- 1 file changed, 89 insertions(+), 108 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index c6b3779908..4e184607e6 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -157,145 +157,126 @@ static int routing_policy_rule_dup(const RoutingPolicyRule *src, RoutingPolicyRu static void routing_policy_rule_hash_func(const RoutingPolicyRule *rule, struct siphash *state) { assert(rule); + /* See rule_exists() in net/core/fib_rules.c of the kernel. */ siphash24_compress_typesafe(rule->family, state); + siphash24_compress_typesafe(rule->type, state); + siphash24_compress_typesafe(rule->table, state); + siphash24_compress_typesafe(rule->priority, state); + siphash24_compress_string(rule->iif, state); + siphash24_compress_string(rule->oif, state); + siphash24_compress_typesafe(rule->fwmark, state); + siphash24_compress_typesafe(rule->suppress_ifgroup, state); + siphash24_compress_typesafe(rule->suppress_prefixlen, state); + siphash24_compress_typesafe(rule->fwmask, state); + /* FRA_TUN_ID */ + /* fr_net (network namespace) */ + siphash24_compress_typesafe(rule->l3mdev, state); + siphash24_compress_typesafe(rule->uid_range, state); + siphash24_compress_typesafe(rule->ipproto, state); + siphash24_compress_typesafe(rule->protocol, state); + siphash24_compress_typesafe(rule->sport, state); + siphash24_compress_typesafe(rule->dport, state); - switch (rule->family) { - case AF_INET: - case AF_INET6: - in_addr_hash_func(&rule->from, rule->family, state); - siphash24_compress_typesafe(rule->from_prefixlen, state); - - siphash24_compress_boolean(rule->l3mdev, state); - - in_addr_hash_func(&rule->to, rule->family, state); - siphash24_compress_typesafe(rule->to_prefixlen, state); - - siphash24_compress_boolean(rule->invert_rule, state); - - siphash24_compress_typesafe(rule->tos, state); - siphash24_compress_typesafe(rule->type, state); - siphash24_compress_typesafe(rule->fwmark, state); - siphash24_compress_typesafe(rule->fwmask, state); - siphash24_compress_typesafe(rule->priority, state); - siphash24_compress_typesafe(rule->table, state); - siphash24_compress_typesafe(rule->suppress_prefixlen, state); - siphash24_compress_typesafe(rule->suppress_ifgroup, state); - - siphash24_compress_typesafe(rule->ipproto, state); - siphash24_compress_typesafe(rule->protocol, state); - siphash24_compress_typesafe(rule->sport, state); - siphash24_compress_typesafe(rule->dport, state); - siphash24_compress_typesafe(rule->uid_range, state); - - siphash24_compress_string(rule->iif, state); - siphash24_compress_string(rule->oif, state); - - break; - default: - /* treat any other address family as AF_UNSPEC */ - break; - } + /* See fib4_rule_compare() in net/ipv4/fib_rules.c, and fib6_rule_compare() in net/ipv6/fib6_rules.c. */ + siphash24_compress_typesafe(rule->from_prefixlen, state); + siphash24_compress_typesafe(rule->to_prefixlen, state); + siphash24_compress_typesafe(rule->tos, state); + /* FRA_FLOW (IPv4 only) */ + in_addr_hash_func(&rule->from, rule->family, state); + in_addr_hash_func(&rule->to, rule->family, state); } static int routing_policy_rule_compare_func(const RoutingPolicyRule *a, const RoutingPolicyRule *b) { int r; + assert(a); + assert(b); + r = CMP(a->family, b->family); if (r != 0) return r; - switch (a->family) { - case AF_INET: - case AF_INET6: - r = CMP(a->from_prefixlen, b->from_prefixlen); - if (r != 0) - return r; + r = CMP(a->type, b->type); + if (r != 0) + return r; - r = memcmp(&a->from, &b->from, FAMILY_ADDRESS_SIZE(a->family)); - if (r != 0) - return r; + r = CMP(a->table, b->table); + if (r != 0) + return r; - r = CMP(a->l3mdev, b->l3mdev); - if (r != 0) - return r; + r = CMP(a->priority, b->priority); + if (r != 0) + return r; - r = CMP(a->to_prefixlen, b->to_prefixlen); - if (r != 0) - return r; + r = strcmp_ptr(a->iif, b->iif); + if (r != 0) + return r; - r = memcmp(&a->to, &b->to, FAMILY_ADDRESS_SIZE(a->family)); - if (r != 0) - return r; + r = strcmp_ptr(a->oif, b->oif); + if (r != 0) + return r; - r = CMP(a->invert_rule, b->invert_rule); - if (r != 0) - return r; + r = CMP(a->fwmark, b->fwmark); + if (r != 0) + return r; - r = CMP(a->tos, b->tos); - if (r != 0) - return r; + r = CMP(a->suppress_ifgroup, b->suppress_ifgroup); + if (r != 0) + return r; - r = CMP(a->type, b->type); - if (r != 0) - return r; + r = CMP(a->suppress_prefixlen, b->suppress_prefixlen); + if (r != 0) + return r; - r = CMP(a->fwmark, b->fwmark); - if (r != 0) - return r; + r = CMP(a->fwmask, b->fwmask); + if (r != 0) + return r; - r = CMP(a->fwmask, b->fwmask); - if (r != 0) - return r; + r = CMP(a->l3mdev, b->l3mdev); + if (r != 0) + return r; - r = CMP(a->priority, b->priority); - if (r != 0) - return r; + r = memcmp(&a->uid_range, &b->uid_range, sizeof(a->uid_range)); + if (r != 0) + return r; - r = CMP(a->table, b->table); - if (r != 0) - return r; + r = CMP(a->ipproto, b->ipproto); + if (r != 0) + return r; - r = CMP(a->suppress_prefixlen, b->suppress_prefixlen); - if (r != 0) - return r; + r = CMP(a->protocol, b->protocol); + if (r != 0) + return r; - r = CMP(a->suppress_ifgroup, b->suppress_ifgroup); - if (r != 0) - return r; + r = memcmp(&a->sport, &b->sport, sizeof(a->sport)); + if (r != 0) + return r; - r = CMP(a->ipproto, b->ipproto); - if (r != 0) - return r; + r = memcmp(&a->dport, &b->dport, sizeof(a->dport)); + if (r != 0) + return r; - r = CMP(a->protocol, b->protocol); - if (r != 0) - return r; + r = CMP(a->from_prefixlen, b->from_prefixlen); + if (r != 0) + return r; - r = memcmp(&a->sport, &b->sport, sizeof(a->sport)); - if (r != 0) - return r; + r = CMP(a->to_prefixlen, b->to_prefixlen); + if (r != 0) + return r; - r = memcmp(&a->dport, &b->dport, sizeof(a->dport)); - if (r != 0) - return r; + r = CMP(a->tos, b->tos); + if (r != 0) + return r; - r = memcmp(&a->uid_range, &b->uid_range, sizeof(a->uid_range)); - if (r != 0) - return r; + r = memcmp(&a->from, &b->from, FAMILY_ADDRESS_SIZE(a->family)); + if (r != 0) + return r; - r = strcmp_ptr(a->iif, b->iif); - if (r != 0) - return r; + r = memcmp(&a->to, &b->to, FAMILY_ADDRESS_SIZE(a->family)); + if (r != 0) + return r; - r = strcmp_ptr(a->oif, b->oif); - if (r != 0) - return r; - - return 0; - default: - /* treat any other address family as AF_UNSPEC */ - return 0; - } + return 0; } static bool routing_policy_rule_equal(const RoutingPolicyRule *rule1, const RoutingPolicyRule *rule2) { From 0a3a3a2a1eb4d839971100cd4639ae98b4aa014c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 Aug 2024 15:25:04 +0900 Subject: [PATCH 2/6] network/routing-policy-rule: reorder elements of RoutingPolicyRule and add comments No functional change, just refactoring. --- src/network/networkd-routing-policy-rule.h | 55 +++++++++++----------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index 60524c00a0..23aa041180 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -20,37 +20,36 @@ typedef struct RoutingPolicyRule { NetworkConfigSource source; NetworkConfigState state; - bool invert_rule; - bool priority_set; - bool l3mdev; /* FRA_L3MDEV */ - - uint8_t tos; - uint8_t type; - uint8_t ipproto; /* FRA_IP_PROTO */ - uint8_t protocol; /* FRA_PROTOCOL */ - uint8_t to_prefixlen; - uint8_t from_prefixlen; - - uint32_t table; - uint32_t fwmark; - uint32_t fwmask; - uint32_t priority; - + /* struct fib_rule_hdr */ AddressFamily address_family; /* Specified by Family= */ int family; /* Automatically determined by From= or To= */ + uint8_t to_prefixlen; + uint8_t from_prefixlen; + uint8_t tos; + uint8_t type; /* a.k.a action */ + bool invert_rule; /* used for flags */ - char *iif; - char *oif; - - union in_addr_union to; - union in_addr_union from; - - struct fib_rule_port_range sport; - struct fib_rule_port_range dport; - struct fib_rule_uid_range uid_range; - - int suppress_prefixlen; - int32_t suppress_ifgroup; + /* attributes */ + union in_addr_union to; /* FRA_DST */ + union in_addr_union from; /* FRA_SRC */ + char *iif; /* FRA_IIFNAME */ + /* FRA_GOTO */ + bool priority_set; + uint32_t priority; /* FRA_PRIORITY */ + uint32_t fwmark; /* FRA_FWMARK */ + /* FRA_FLOW */ + /* FRA_TUN_ID */ + int32_t suppress_ifgroup; /* FRA_SUPPRESS_IFGROUP */ + int suppress_prefixlen; /* FRA_SUPPRESS_PREFIXLEN */ + uint32_t table; /* FRA_TABLE, also used in struct fib_rule_hdr */ + uint32_t fwmask; /* FRA_FWMASK */ + char *oif; /* FRA_OIFNAME */ + bool l3mdev; /* FRA_L3MDEV */ + struct fib_rule_uid_range uid_range; /* FRA_UID_RANGE */ + uint8_t protocol; /* FRA_PROTOCOL */ + uint8_t ipproto; /* FRA_IP_PROTO */ + struct fib_rule_port_range sport; /* FRA_SPORT_RANGE */ + struct fib_rule_port_range dport; /* FRA_DPORT_RANGE */ } RoutingPolicyRule; const char* fr_act_type_full_to_string(int t) _const_; From 933448defeff377c64ec6104917e388dab9a2177 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 Aug 2024 20:13:12 +0900 Subject: [PATCH 3/6] network/routing-policy-rule: use int32_t for suppress_prefixlen The kernel parses FRA_SUPPRESS_PREFIXLEN as uint32_t, but internally handled as signed integer and negative values as unset. Let's explicitly specify the size of the variable. No functional change, just refactoring. --- src/basic/parse-util.c | 4 ++-- src/basic/parse-util.h | 2 +- src/network/networkd-routing-policy-rule.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index 35d13eb38f..5ace2b9b11 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -714,7 +714,7 @@ int parse_ip_port_range(const char *s, uint16_t *low, uint16_t *high, bool allow return 0; } -int parse_ip_prefix_length(const char *s, int *ret) { +int parse_ip_prefix_length(const char *s, int32_t *ret) { unsigned l; int r; @@ -725,7 +725,7 @@ int parse_ip_prefix_length(const char *s, int *ret) { if (l > 128) return -ERANGE; - *ret = (int) l; + *ret = (int32_t) l; return 0; } diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h index c12988ef20..e4ae48366c 100644 --- a/src/basic/parse-util.h +++ b/src/basic/parse-util.h @@ -141,7 +141,7 @@ int parse_nice(const char *p, int *ret); int parse_ip_port(const char *s, uint16_t *ret); int parse_ip_port_range(const char *s, uint16_t *low, uint16_t *high, bool allow_zero); -int parse_ip_prefix_length(const char *s, int *ret); +int parse_ip_prefix_length(const char *s, int32_t *ret); int parse_oom_score_adjust(const char *s, int *ret); diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index 23aa041180..fbde298f05 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -40,7 +40,7 @@ typedef struct RoutingPolicyRule { /* FRA_FLOW */ /* FRA_TUN_ID */ int32_t suppress_ifgroup; /* FRA_SUPPRESS_IFGROUP */ - int suppress_prefixlen; /* FRA_SUPPRESS_PREFIXLEN */ + int32_t suppress_prefixlen; /* FRA_SUPPRESS_PREFIXLEN */ uint32_t table; /* FRA_TABLE, also used in struct fib_rule_hdr */ uint32_t fwmask; /* FRA_FWMASK */ char *oif; /* FRA_OIFNAME */ From 957d94c5bd6011c2f02bb903b970e3e561a20dc8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 Aug 2024 20:24:07 +0900 Subject: [PATCH 4/6] network/routing-policy-rule: manage all flags Currently, only FIB_RULE_INVERT flag can be configurable, but for simplicity and future extension, let's manage all flags. No functional change, just refactoring. --- src/network/networkd-json.c | 2 +- src/network/networkd-routing-policy-rule.c | 14 +++++--------- src/network/networkd-routing-policy-rule.h | 2 +- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-json.c b/src/network/networkd-json.c index 299e029922..0a57e6aee0 100644 --- a/src/network/networkd-json.c +++ b/src/network/networkd-json.c @@ -316,7 +316,7 @@ static int routing_policy_rule_append_json(RoutingPolicyRule *rule, sd_json_vari SD_JSON_BUILD_PAIR_UNSIGNED("FirewallMask", rule->fwmask), JSON_BUILD_PAIR_UNSIGNED_NON_ZERO("Table", rule->table), JSON_BUILD_PAIR_STRING_NON_EMPTY("TableString", table), - SD_JSON_BUILD_PAIR_BOOLEAN("Invert", rule->invert_rule), + SD_JSON_BUILD_PAIR_BOOLEAN("Invert", FLAGS_SET(rule->flags, FIB_RULE_INVERT)), SD_JSON_BUILD_PAIR_CONDITION(rule->suppress_prefixlen >= 0, "SuppressPrefixLength", SD_JSON_BUILD_UNSIGNED(rule->suppress_prefixlen)), SD_JSON_BUILD_PAIR_CONDITION(rule->suppress_ifgroup >= 0, diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 4e184607e6..dd6adb49c5 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -524,11 +524,9 @@ static int routing_policy_rule_set_netlink_message(const RoutingPolicyRule *rule return r; } - if (rule->invert_rule) { - r = sd_rtnl_message_routing_policy_rule_set_flags(m, FIB_RULE_INVERT); - if (r < 0) - return r; - } + r = sd_rtnl_message_routing_policy_rule_set_flags(m, rule->flags); + if (r < 0) + return r; if (rule->l3mdev) { r = sd_netlink_message_append_u8(m, FRA_L3MDEV, 1); @@ -929,13 +927,11 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man } } - unsigned flags; - r = sd_rtnl_message_routing_policy_rule_get_flags(message, &flags); + r = sd_rtnl_message_routing_policy_rule_get_flags(message, &tmp->flags); if (r < 0) { log_warning_errno(r, "rtnl: received rule message without valid flag, ignoring: %m"); return 0; } - tmp->invert_rule = flags & FIB_RULE_INVERT; r = sd_netlink_message_read_u32(message, FRA_FWMARK, &tmp->fwmark); if (r < 0 && r != -ENODATA) { @@ -1487,7 +1483,7 @@ int config_parse_routing_policy_rule_invert( return 0; } - n->invert_rule = r; + SET_FLAG(n->flags, FIB_RULE_INVERT, r); TAKE_PTR(n); return 0; diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index fbde298f05..3765549554 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -27,7 +27,7 @@ typedef struct RoutingPolicyRule { uint8_t from_prefixlen; uint8_t tos; uint8_t type; /* a.k.a action */ - bool invert_rule; /* used for flags */ + uint32_t flags; /* attributes */ union in_addr_union to; /* FRA_DST */ From 6c72e80a3c84a64fc14ba6a199630bac4a4886c3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 Aug 2024 16:14:31 +0900 Subject: [PATCH 5/6] network/routing-policy-rule: introduce ref and unref functions for RoutingPolicyRule No functional change, just refactoring and preparation for later change. --- src/network/networkd-network.c | 2 +- src/network/networkd-routing-policy-rule.c | 117 +++++++++++++-------- src/network/networkd-routing-policy-rule.h | 5 +- 3 files changed, 81 insertions(+), 43 deletions(-) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 4e4e5eb2a8..5335376e15 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -798,7 +798,7 @@ static Network *network_free(Network *network) { hashmap_free_with_destructor(network->prefixes_by_section, prefix_free); hashmap_free_with_destructor(network->route_prefixes_by_section, route_prefix_free); hashmap_free_with_destructor(network->pref64_prefixes_by_section, prefix64_free); - hashmap_free_with_destructor(network->rules_by_section, routing_policy_rule_free); + hashmap_free(network->rules_by_section); hashmap_free_with_destructor(network->dhcp_static_leases_by_section, dhcp_static_lease_free); ordered_hashmap_free_with_destructor(network->sr_iov_by_section, sr_iov_free); hashmap_free_with_destructor(network->qdiscs_by_section, qdisc_free); diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index dd6adb49c5..c68a1aa71a 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -43,17 +43,36 @@ assert_cc(__FR_ACT_MAX <= UINT8_MAX); DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(fr_act_type, int); DEFINE_STRING_TABLE_LOOKUP_TO_STRING(fr_act_type_full, int); -RoutingPolicyRule *routing_policy_rule_free(RoutingPolicyRule *rule) { - if (!rule) - return NULL; +static RoutingPolicyRule* routing_policy_rule_detach_impl(RoutingPolicyRule *rule) { + assert(rule); + assert(!!rule->manager + !!rule->network <= 1); if (rule->network) { assert(rule->section); hashmap_remove(rule->network->rules_by_section, rule->section); + rule->network = NULL; + return rule; } - if (rule->manager) + if (rule->manager) { set_remove(rule->manager->rules, rule); + rule->manager = NULL; + return rule; + } + + return NULL; +} + +static void routing_policy_rule_detach(RoutingPolicyRule *rule) { + assert(rule); + routing_policy_rule_unref(routing_policy_rule_detach_impl(rule)); +} + +static RoutingPolicyRule* routing_policy_rule_free(RoutingPolicyRule *rule) { + if (!rule) + return NULL; + + routing_policy_rule_detach_impl(rule); config_section_free(rule->section); free(rule->iif); @@ -62,7 +81,27 @@ RoutingPolicyRule *routing_policy_rule_free(RoutingPolicyRule *rule) { return mfree(rule); } -DEFINE_SECTION_CLEANUP_FUNCTIONS(RoutingPolicyRule, routing_policy_rule_free); +DEFINE_TRIVIAL_REF_UNREF_FUNC(RoutingPolicyRule, routing_policy_rule, routing_policy_rule_free); + +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + routing_policy_rule_section_hash_ops, + ConfigSection, + config_section_hash_func, + config_section_compare_func, + RoutingPolicyRule, + routing_policy_rule_detach); + +static void routing_policy_rule_hash_func(const RoutingPolicyRule *rule, struct siphash *state); +static int routing_policy_rule_compare_func(const RoutingPolicyRule *a, const RoutingPolicyRule *b); + +DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( + routing_policy_rule_hash_ops_detach, + RoutingPolicyRule, + routing_policy_rule_hash_func, + routing_policy_rule_compare_func, + routing_policy_rule_detach); + +DEFINE_SECTION_CLEANUP_FUNCTIONS(RoutingPolicyRule, routing_policy_rule_unref); static int routing_policy_rule_new(RoutingPolicyRule **ret) { RoutingPolicyRule *rule; @@ -72,6 +111,7 @@ static int routing_policy_rule_new(RoutingPolicyRule **ret) { return -ENOMEM; *rule = (RoutingPolicyRule) { + .n_ref = 1, .table = RT_TABLE_MAIN, .uid_range.start = UID_INVALID, .uid_range.end = UID_INVALID, @@ -86,7 +126,7 @@ static int routing_policy_rule_new(RoutingPolicyRule **ret) { } static int routing_policy_rule_new_static(Network *network, const char *filename, unsigned section_line, RoutingPolicyRule **ret) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *rule = NULL; + _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *rule = NULL; _cleanup_(config_section_freep) ConfigSection *n = NULL; int r; @@ -114,7 +154,7 @@ static int routing_policy_rule_new_static(Network *network, const char *filename rule->source = NETWORK_CONFIG_SOURCE_STATIC; rule->protocol = RTPROT_STATIC; - r = hashmap_ensure_put(&network->rules_by_section, &config_section_hash_ops, rule->section, rule); + r = hashmap_ensure_put(&network->rules_by_section, &routing_policy_rule_section_hash_ops, rule->section, rule); if (r < 0) return r; @@ -123,7 +163,7 @@ static int routing_policy_rule_new_static(Network *network, const char *filename } static int routing_policy_rule_dup(const RoutingPolicyRule *src, RoutingPolicyRule **ret) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *dest = NULL; + _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *dest = NULL; assert(src); assert(ret); @@ -132,7 +172,8 @@ static int routing_policy_rule_dup(const RoutingPolicyRule *src, RoutingPolicyRu if (!dest) return -ENOMEM; - /* Unset all pointers */ + /* Clear the reference counter and all pointers. */ + dest->n_ref = 1; dest->manager = NULL; dest->network = NULL; dest->section = NULL; @@ -289,13 +330,6 @@ static bool routing_policy_rule_equal(const RoutingPolicyRule *rule1, const Rout return routing_policy_rule_compare_func(rule1, rule2) == 0; } -DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( - routing_policy_rule_hash_ops, - RoutingPolicyRule, - routing_policy_rule_hash_func, - routing_policy_rule_compare_func, - routing_policy_rule_free); - static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *in, RoutingPolicyRule **ret) { RoutingPolicyRule *rule; @@ -336,20 +370,22 @@ static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *in, Rout return -ENOENT; } -static int routing_policy_rule_add(Manager *m, RoutingPolicyRule *rule) { +static int routing_policy_rule_attach(Manager *m, RoutingPolicyRule *rule) { int r; assert(m); assert(rule); assert(IN_SET(rule->family, AF_INET, AF_INET6)); + assert(!rule->manager); - r = set_ensure_put(&m->rules, &routing_policy_rule_hash_ops, rule); + r = set_ensure_put(&m->rules, &routing_policy_rule_hash_ops_detach, rule); if (r < 0) return r; if (r == 0) return -EEXIST; rule->manager = m; + routing_policy_rule_ref(rule); return 0; } @@ -762,7 +798,7 @@ static int link_request_routing_policy_rule(Link *link, RoutingPolicyRule *rule) assert(rule->source != NETWORK_CONFIG_SOURCE_FOREIGN); if (routing_policy_rule_get(link->manager, rule, &existing) < 0) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *tmp = NULL; + _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; r = routing_policy_rule_dup(rule, &tmp); if (r < 0) @@ -772,11 +808,11 @@ static int link_request_routing_policy_rule(Link *link, RoutingPolicyRule *rule) if (r < 0) return r; - r = routing_policy_rule_add(link->manager, tmp); + r = routing_policy_rule_attach(link->manager, tmp); if (r < 0) return r; - existing = TAKE_PTR(tmp); + existing = tmp; } else existing->source = rule->source; @@ -862,7 +898,7 @@ static bool routing_policy_rule_is_created_by_kernel(const RoutingPolicyRule *ru } int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *tmp = NULL; + _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; RoutingPolicyRule *rule = NULL; bool adjust_protocol = false; uint16_t type; @@ -1063,12 +1099,11 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man } else { routing_policy_rule_enter_configured(tmp); log_routing_policy_rule_debug(tmp, "Remembering", NULL, m); - r = routing_policy_rule_add(m, tmp); + r = routing_policy_rule_attach(m, tmp); if (r < 0) { log_warning_errno(r, "Could not remember foreign rule, ignoring: %m"); return 0; } - TAKE_PTR(tmp); } break; case RTM_DELRULE: @@ -1076,7 +1111,7 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man routing_policy_rule_enter_removed(rule); if (rule->state == 0) { log_routing_policy_rule_debug(rule, "Forgetting", NULL, m); - routing_policy_rule_free(rule); + routing_policy_rule_detach(rule); } else log_routing_policy_rule_debug(rule, "Removed", NULL, m); } else @@ -1138,7 +1173,7 @@ int config_parse_routing_policy_rule_tos( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1174,7 +1209,7 @@ int config_parse_routing_policy_rule_priority( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1218,7 +1253,7 @@ int config_parse_routing_policy_rule_table( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1255,7 +1290,7 @@ int config_parse_routing_policy_rule_fwmark_mask( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1291,7 +1326,7 @@ int config_parse_routing_policy_rule_prefix( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; union in_addr_union *buffer; uint8_t *prefixlen; @@ -1340,7 +1375,7 @@ int config_parse_routing_policy_rule_device( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1380,7 +1415,7 @@ int config_parse_routing_policy_rule_port_range( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; uint16_t low, high; int r; @@ -1425,7 +1460,7 @@ int config_parse_routing_policy_rule_ip_protocol( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1463,7 +1498,7 @@ int config_parse_routing_policy_rule_invert( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1501,7 +1536,7 @@ int config_parse_routing_policy_rule_l3mdev( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1539,7 +1574,7 @@ int config_parse_routing_policy_rule_family( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; AddressFamily a; int r; @@ -1579,7 +1614,7 @@ int config_parse_routing_policy_rule_uid_range( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; uid_t start, end; int r; @@ -1625,7 +1660,7 @@ int config_parse_routing_policy_rule_suppress_prefixlen( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -1665,7 +1700,7 @@ int config_parse_routing_policy_rule_suppress_ifgroup( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int32_t suppress_ifgroup; int r; @@ -1713,7 +1748,7 @@ int config_parse_routing_policy_rule_type( void *data, void *userdata) { - _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_unref_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r, t; @@ -1772,5 +1807,5 @@ void network_drop_invalid_routing_policy_rules(Network *network) { HASHMAP_FOREACH(rule, network->rules_by_section) if (routing_policy_rule_section_verify(rule) < 0) - routing_policy_rule_free(rule); + routing_policy_rule_detach(rule); } diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index 3765549554..7047cc4e9b 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -20,6 +20,8 @@ typedef struct RoutingPolicyRule { NetworkConfigSource source; NetworkConfigState state; + unsigned n_ref; + /* struct fib_rule_hdr */ AddressFamily address_family; /* Specified by Family= */ int family; /* Automatically determined by From= or To= */ @@ -54,7 +56,8 @@ typedef struct RoutingPolicyRule { const char* fr_act_type_full_to_string(int t) _const_; -RoutingPolicyRule *routing_policy_rule_free(RoutingPolicyRule *rule); +RoutingPolicyRule* routing_policy_rule_ref(RoutingPolicyRule *rule); +RoutingPolicyRule* routing_policy_rule_unref(RoutingPolicyRule *rule); void network_drop_invalid_routing_policy_rules(Network *network); From 93f91df671bba2d7eb9d4b2876ecf97d4c3b34fd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 19 Aug 2024 12:36:17 +0900 Subject: [PATCH 6/6] network/routing-policy-rule: drop unused argument --- src/network/networkd-routing-policy-rule.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index c68a1aa71a..8e7cc47118 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -462,14 +462,12 @@ static void log_routing_policy_rule_debug(const RoutingPolicyRule *rule, const c strna(rule->iif), strna(rule->oif), strna(table)); } -static int routing_policy_rule_set_netlink_message(const RoutingPolicyRule *rule, sd_netlink_message *m, Link *link) { +static int routing_policy_rule_set_netlink_message(const RoutingPolicyRule *rule, sd_netlink_message *m) { int r; assert(rule); assert(m); - /* link may be NULL. */ - if (rule->from_prefixlen > 0) { r = netlink_message_append_in_addr_union(m, FRA_SRC, rule->family, &rule->from); if (r < 0) @@ -616,7 +614,7 @@ static int routing_policy_rule_remove(RoutingPolicyRule *rule) { if (r < 0) return log_warning_errno(r, "Could not allocate netlink message: %m"); - r = routing_policy_rule_set_netlink_message(rule, m, NULL); + r = routing_policy_rule_set_netlink_message(rule, m); if (r < 0) return log_warning_errno(r, "Could not create netlink message: %m"); @@ -648,7 +646,7 @@ static int routing_policy_rule_configure(RoutingPolicyRule *rule, Link *link, Re if (r < 0) return r; - r = routing_policy_rule_set_netlink_message(rule, m, link); + r = routing_policy_rule_set_netlink_message(rule, m); if (r < 0) return r;