From: Ondřej Surý Date: Sun, 22 Mar 2026 11:14:03 +0000 (+0100) Subject: Replace void* data pointers with match enum in radix nodes X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=30ad2afa9baf90da6d072f88a9c7200d2949ef5a;p=thirdparty%2Fbind9.git Replace void* data pointers with match enum in radix nodes The node data[] array only ever held pointers to two static bools (dns_iptable_pos/neg). Replace with isc_radix_match_t enum (RADIX_UNSET/RADIX_ALLOW/RADIX_DENY) stored directly in the node. This eliminates the void* casts, the static bool variables, the isc_radix_destroyfunc_t callback (always NULL), and shrinks isc_radix_node_t from 80 to 64 bytes. Also use sa_family_t for the prefix family field. --- diff --git a/bin/named/server.c b/bin/named/server.c index 86e0f961a50..e5ffb7ede2a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3537,7 +3537,7 @@ create_mapped_acl(void) { isc_netaddr_fromin6(&addr, &in6); dns_acl_create(isc_g_mctx, 1, &acl); - dns_iptable_addprefix(acl->iptable, &addr, 96, true); + dns_iptable_addprefix(acl->iptable, &addr, 96, RADIX_ALLOW); named_g_mapped = acl; } diff --git a/lib/dns/acl.c b/lib/dns/acl.c index f783e2abf9a..c77c1c99790 100644 --- a/lib/dns/acl.c +++ b/lib/dns/acl.c @@ -55,36 +55,16 @@ dns_acl_create(isc_mem_t *mctx, int n, dns_acl_t **target) { *target = acl; } -/* - * Create a new ACL and initialize it with the value "any" or "none", - * depending on the value of the "neg" parameter. - * "any" is a positive iptable entry with bit length 0. - * "none" is the same as "!any". - */ -static void -dns_acl_anyornone(isc_mem_t *mctx, bool neg, dns_acl_t **target) { - dns_acl_t *acl = NULL; - - dns_acl_create(mctx, 0, &acl); - dns_iptable_addprefix(acl->iptable, NULL, 0, !neg); - - *target = acl; -} - -/* - * Create a new ACL that matches everything. - */ void dns_acl_any(isc_mem_t *mctx, dns_acl_t **target) { - dns_acl_anyornone(mctx, false, target); + dns_acl_create(mctx, 0, target); + dns_iptable_addprefix((*target)->iptable, NULL, 0, RADIX_ALLOW); } -/* - * Create a new ACL that matches nothing. - */ void dns_acl_none(isc_mem_t *mctx, dns_acl_t **target) { - dns_acl_anyornone(mctx, true, target); + dns_acl_create(mctx, 0, target); + dns_iptable_addprefix((*target)->iptable, NULL, 0, RADIX_DENY); } /* @@ -105,16 +85,16 @@ dns_acl_isanyornone(dns_acl_t *acl, bool pos) { return false; } - if (acl->iptable->radix->head->prefix.bitlen == 0 && - acl->iptable->radix->head->data[0] != NULL && - acl->iptable->radix->head->data[0] == - acl->iptable->radix->head->data[1] && - *(bool *)(acl->iptable->radix->head->data[0]) == pos) + isc_radix_node_t *head = acl->iptable->radix->head; + isc_radix_match_t expected = pos ? RADIX_ALLOW : RADIX_DENY; + + if (head->prefix.bitlen == 0 && head->match[RADIX_V4] == expected && + head->match[RADIX_V6] == expected) { return true; } - return false; /* All others */ + return false; } /* @@ -150,7 +130,7 @@ dns_acl_match(const isc_netaddr_t *reqaddr, const dns_name_t *reqsigner, const isc_netaddr_t *addr = reqaddr; isc_netaddr_t v4addr; isc_result_t result; - int match_num = -1; + int32_t match_num = -1; unsigned int i; REQUIRE(reqaddr != NULL); @@ -165,7 +145,7 @@ dns_acl_match(const isc_netaddr_t *reqaddr, const dns_name_t *reqsigner, /* Always match with host addresses. */ bitlen = (addr->family == AF_INET6) ? 128 : 32; - NETADDR_TO_PREFIX_T(addr, pfx, bitlen); + isc_prefix_from_netaddr(&pfx, addr, bitlen); /* Assume no match. */ *match = 0; @@ -177,7 +157,7 @@ dns_acl_match(const isc_netaddr_t *reqaddr, const dns_name_t *reqsigner, if (result == ISC_R_SUCCESS && node != NULL) { int fam = ISC_RADIX_FAMILY(&pfx); match_num = node->node_num[fam]; - if (*(bool *)node->data[fam]) { + if (node->match[fam] == RADIX_ALLOW) { *match = match_num; } else { *match = -match_num; @@ -263,7 +243,7 @@ dns_acl_match_port_transport(const isc_netaddr_t *reqaddr, isc_result_t dns_acl_merge(dns_acl_t *dest, dns_acl_t *source, bool pos) { unsigned int nelem, i; - int max_node = 0, nodes; + int32_t max_node = 0, nodes; /* Resize the element array if needed. */ if (dest->length + source->length > dest->alloc) { @@ -335,7 +315,7 @@ dns_acl_merge(dns_acl_t *dest, dns_acl_t *source, bool pos) { * node_count value is set correctly afterward. */ nodes = max_node + dns_acl_node_count(dest); - dns_iptable_merge(dest->iptable, source->iptable, pos); + dns_iptable_merge(dest->iptable, source->iptable, !pos); if (nodes > dns_acl_node_count(dest)) { dns_acl_node_count(dest) = nodes; } @@ -500,13 +480,12 @@ static void check_insecure(isc_radix_node_t *node, void *arg) { bool *found = arg; isc_prefix_t *prefix = &node->prefix; - void **data = node->data; /* * If all nonexistent or negative then this node is secure. */ - if ((data[0] == NULL || !*(bool *)data[0]) && - (data[1] == NULL || !*(bool *)data[1])) + if (node->match[RADIX_V4] != RADIX_ALLOW && + node->match[RADIX_V6] != RADIX_ALLOW) { return; } @@ -517,13 +496,13 @@ check_insecure(isc_radix_node_t *node, void *arg) { */ if (prefix->bitlen == 32 && htonl(prefix->add.sin.s_addr) == INADDR_LOOPBACK && - (data[1] == NULL || !*(bool *)data[1])) + node->match[RADIX_V6] != RADIX_ALLOW) { return; } if (prefix->bitlen == 128 && IN6_IS_ADDR_LOOPBACK(&prefix->add.sin6) && - (data[0] == NULL || !*(bool *)data[0])) + node->match[RADIX_V4] != RADIX_ALLOW) { return; } diff --git a/lib/dns/include/dns/acl.h b/lib/dns/include/dns/acl.h index 4796c43777a..07c13c03aaa 100644 --- a/lib/dns/include/dns/acl.h +++ b/lib/dns/include/dns/acl.h @@ -76,7 +76,7 @@ struct dns_aclelement { dns_geoip_elem_t geoip_elem; #endif /* HAVE_GEOIP2 */ dns_acl_t *nestedacl; - int node_num; + int32_t node_num; }; #define dns_acl_node_count(acl) acl->iptable->radix->num_added_node diff --git a/lib/dns/include/dns/iptable.h b/lib/dns/include/dns/iptable.h index 807e1691c68..91909023baf 100644 --- a/lib/dns/include/dns/iptable.h +++ b/lib/dns/include/dns/iptable.h @@ -47,15 +47,15 @@ dns_iptable_create(isc_mem_t *mctx, dns_iptable_t **target); void dns_iptable_addprefix(dns_iptable_t *tab, const isc_netaddr_t *addr, - uint16_t bitlen, bool pos); + uint16_t bitlen, isc_radix_match_t match); /* - * Add an IP prefix to an existing IP table + * Add an IP prefix to an existing IP table. */ void -dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, bool pos); +dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, bool negate); /* - * Merge one IP table into another one. + * Merge one IP table into another one, optionally negating allow entries. */ #if DNS_IPTABLE_TRACE diff --git a/lib/dns/iptable.c b/lib/dns/iptable.c index 486cb27039f..300f9231f50 100644 --- a/lib/dns/iptable.c +++ b/lib/dns/iptable.c @@ -37,28 +37,26 @@ dns_iptable_create(isc_mem_t *mctx, dns_iptable_t **target) { *target = tab; } -static bool dns_iptable_neg = false; -static bool dns_iptable_pos = true; - /* * Add an IP prefix to an existing IP table */ static void -iptable_addentry(dns_iptable_t *tab, isc_prefix_t *pfx, bool pos) { +iptable_addentry(dns_iptable_t *tab, isc_prefix_t *pfx, + isc_radix_match_t match) { isc_radix_node_t *node = NULL; isc_radix_insert(tab->radix, &node, NULL, pfx); /* Preserve first-match semantics: don't overwrite existing data */ int fam = ISC_RADIX_FAMILY(pfx); - if (node->data[fam] == NULL) { - node->data[fam] = pos ? &dns_iptable_pos : &dns_iptable_neg; + if (node->match[fam] == RADIX_UNSET) { + node->match[fam] = match; } } void dns_iptable_addprefix(dns_iptable_t *tab, const isc_netaddr_t *addr, - uint16_t bitlen, bool pos) { + uint16_t bitlen, isc_radix_match_t match) { INSIST(DNS_IPTABLE_VALID(tab)); INSIST(tab->radix != NULL); @@ -70,20 +68,20 @@ dns_iptable_addprefix(dns_iptable_t *tab, const isc_netaddr_t *addr, isc_prefix_t pfx4 = { .family = AF_INET, .bitlen = 0 }; isc_prefix_t pfx6 = { .family = AF_INET6, .bitlen = 0 }; - iptable_addentry(tab, &pfx4, pos); - iptable_addentry(tab, &pfx6, pos); + iptable_addentry(tab, &pfx4, match); + iptable_addentry(tab, &pfx6, match); return; } isc_prefix_t pfx; - NETADDR_TO_PREFIX_T(addr, pfx, bitlen); - iptable_addentry(tab, &pfx, pos); + isc_prefix_from_netaddr(&pfx, addr, bitlen); + iptable_addentry(tab, &pfx, match); } typedef struct { dns_iptable_t *tab; - bool pos; - int max_node; + bool negate; + int32_t max_node; } iptable_merge_ctx_t; static void @@ -102,10 +100,8 @@ iptable_merge_node(isc_radix_node_t *node, void *arg) { * just leave the negative nodes negative. */ for (size_t i = 0; i < RADIX_FAMILIES; i++) { - if (!ctx->pos && node->data[i] != NULL && - *(bool *)node->data[i]) - { - new_node->data[i] = &dns_iptable_neg; + if (ctx->negate && node->match[i] == RADIX_ALLOW) { + new_node->match[i] = RADIX_DENY; } if (node->node_num[i] > ctx->max_node) { ctx->max_node = node->node_num[i]; @@ -117,8 +113,8 @@ iptable_merge_node(isc_radix_node_t *node, void *arg) { * Merge one IP table into another one. */ void -dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, bool pos) { - iptable_merge_ctx_t ctx = { .tab = tab, .pos = pos }; +dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, bool negate) { + iptable_merge_ctx_t ctx = { .tab = tab, .negate = negate }; isc_radix_foreach(source->radix, iptable_merge_node, &ctx); @@ -132,7 +128,7 @@ dns__iptable_destroy(dns_iptable_t *dtab) { dtab->magic = 0; if (dtab->radix != NULL) { - isc_radix_destroy(dtab->radix, NULL); + isc_radix_destroy(dtab->radix); dtab->radix = NULL; } diff --git a/lib/isc/include/isc/bit.h b/lib/isc/include/isc/bit.h index 68f01bd5c94..8b8ea27eb83 100644 --- a/lib/isc/include/isc/bit.h +++ b/lib/isc/include/isc/bit.h @@ -14,6 +14,7 @@ #pragma once #include +#include #include #include diff --git a/lib/isc/include/isc/radix.h b/lib/isc/include/isc/radix.h index aa209be4017..a246af3e305 100644 --- a/lib/isc/include/isc/radix.h +++ b/lib/isc/include/isc/radix.h @@ -18,35 +18,38 @@ #include #include +#include #include typedef struct isc_prefix { - unsigned int family; /* AF_INET | AF_INET6 (0 means no prefix) */ - unsigned int bitlen; /* prefix length in bits */ + sa_family_t family; /* AF_INET | AF_INET6 (0 means no prefix) */ + uint8_t bitlen; /* prefix length in bits (max 128) */ union { struct in_addr sin; struct in6_addr sin6; } add; } isc_prefix_t; -#define NETADDR_TO_PREFIX_T(na, pt, bits) \ - { \ - memset(&(pt), 0, sizeof(pt)); \ - (pt).family = (na)->family; \ - (pt).bitlen = (bits); \ - if ((pt).family == AF_INET6) { \ - memmove(&(pt).add.sin6, &(na)->type.in6, \ - ((bits) + 7) / 8); \ - } else { \ - memmove(&(pt).add.sin, &(na)->type.in, \ - ((bits) + 7) / 8); \ - } \ +static inline void +isc_prefix_from_netaddr(isc_prefix_t *pfx, const isc_netaddr_t *na, + uint8_t bitlen) { + *pfx = (isc_prefix_t){ .family = na->family, .bitlen = bitlen }; + if (na->family == AF_INET6) { + memmove(&pfx->add.sin6, &na->type.in6, (bitlen + 7) / 8); + } else { + memmove(&pfx->add.sin, &na->type.in, (bitlen + 7) / 8); } +} -typedef void (*isc_radix_destroyfunc_t)(void *); +#define isc_prefix_touint8(prefix) ((uint8_t *)&(prefix)->add.sin) -#define isc_prefix_tochar(prefix) ((char *)&(prefix)->add.sin) -#define isc_prefix_touchar(prefix) ((u_char *)&(prefix)->add.sin) +/* + * Test whether bit 'n' (0 = MSB) is set in the byte array 'addr'. + */ +static inline bool +isc_prefix_bit_isset(const uint8_t *addr, unsigned int n) { + return (addr[n / 8] & (1 << (7 - n % 8))) != 0; +} /* * We need "first match" when we search the radix tree to preserve @@ -73,16 +76,19 @@ typedef void (*isc_radix_destroyfunc_t)(void *); #define ISC_RADIX_FAMILY(p) (((p)->family == AF_INET6) ? RADIX_V6 : RADIX_V4) +typedef enum { + RADIX_UNSET = 0, /* no entry for this address family */ + RADIX_ALLOW, /* positive match (allow) */ + RADIX_DENY, /* negative match (deny) */ +} isc_radix_match_t; + typedef struct isc_radix_node { struct isc_radix_node *left, *right; /* children */ - struct isc_radix_node *parent; /* may be used */ - void *data[RADIX_FAMILIES]; /* pointers to IPv4 - * and IPV6 data */ - isc_prefix_t prefix; /* inline prefix data; - * family==0 for glue */ - uint32_t bit; /* bit position */ - int node_num[RADIX_FAMILIES]; /* insertion order, - * or -1 for glue */ + struct isc_radix_node *parent; + isc_prefix_t prefix; /* family==0 for glue nodes */ + int32_t node_num[RADIX_FAMILIES]; /* insertion order, -1 = glue */ + isc_radix_match_t match[RADIX_FAMILIES]; + uint8_t bit; /* bit position in the key */ } isc_radix_node_t; typedef void (*isc_radix_foreachfunc_t)(isc_radix_node_t *node, void *arg); @@ -92,11 +98,11 @@ typedef void (*isc_radix_foreachfunc_t)(isc_radix_node_t *node, void *arg); typedef struct isc_radix_tree { unsigned int magic; - uint32_t maxbits; /* for IP, 32 bit addresses */ + uint8_t maxbits; isc_mem_t *mctx; isc_radix_node_t *head; - int num_active_node; /* for debugging purposes */ - int num_added_node; /* total number of nodes */ + int32_t num_active_node; /* for debugging purposes */ + int32_t num_added_node; /* total number of nodes */ } isc_radix_tree_t; isc_result_t @@ -141,7 +147,7 @@ isc_radix_remove(isc_radix_tree_t *radix, isc_radix_node_t *node); */ void -isc_radix_create(isc_mem_t *mctx, isc_radix_tree_t **target, int maxbits); +isc_radix_create(isc_mem_t *mctx, isc_radix_tree_t **target, uint8_t maxbits); /*%< * Create a radix tree with a maximum depth of 'maxbits'; * @@ -152,9 +158,9 @@ isc_radix_create(isc_mem_t *mctx, isc_radix_tree_t **target, int maxbits); */ void -isc_radix_destroy(isc_radix_tree_t *radix, isc_radix_destroyfunc_t func); +isc_radix_destroy(isc_radix_tree_t *radix); /*%< - * Destroy a radix tree optionally calling 'func' to clean up node data. + * Destroy a radix tree. * * Requires: * \li 'radix' to be valid. diff --git a/lib/isc/radix.c b/lib/isc/radix.c index feb8d92a195..928445ebf92 100644 --- a/lib/isc/radix.c +++ b/lib/isc/radix.c @@ -13,26 +13,25 @@ #include +#include #include #include #include #include -#define BIT_TEST(f, b) (((f) & (b)) != 0) - static int -comp_with_mask(void *addr, void *dest, u_int mask) { +comp_with_mask(void *addr, void *dest, unsigned int mask) { /* Mask length of zero matches everything */ if (mask == 0) { return 1; } if (memcmp(addr, dest, mask / 8) == 0) { - u_int n = mask / 8; - u_int m = ((~0U) << (8 - (mask % 8))); + unsigned int n = mask / 8; + unsigned int m = ((~0U) << (8 - (mask % 8))); if ((mask % 8) == 0 || - (((u_char *)addr)[n] & m) == (((u_char *)dest)[n] & m)) + (((uint8_t *)addr)[n] & m) == (((uint8_t *)dest)[n] & m)) { return 1; } @@ -41,7 +40,7 @@ comp_with_mask(void *addr, void *dest, u_int mask) { } static isc_radix_node_t * -radix_node_create(isc_mem_t *mctx, isc_prefix_t *prefix, uint32_t bit) { +radix_node_create(isc_mem_t *mctx, isc_prefix_t *prefix, uint8_t bit) { isc_radix_node_t *node = isc_mem_get(mctx, sizeof(*node)); *node = (isc_radix_node_t){ .bit = bit, @@ -54,7 +53,7 @@ radix_node_create(isc_mem_t *mctx, isc_prefix_t *prefix, uint32_t bit) { } void -isc_radix_create(isc_mem_t *mctx, isc_radix_tree_t **target, int maxbits) { +isc_radix_create(isc_mem_t *mctx, isc_radix_tree_t **target, uint8_t maxbits) { REQUIRE(target != NULL && *target == NULL); RUNTIME_CHECK(maxbits <= RADIX_MAXBITS); @@ -73,26 +72,17 @@ isc_radix_create(isc_mem_t *mctx, isc_radix_tree_t **target, int maxbits) { */ static void -clear_radix(isc_radix_tree_t *radix, isc_radix_destroyfunc_t func) { +clear_radix(isc_radix_tree_t *radix) { + REQUIRE(radix != NULL); + isc_radix_node_t *stack[RADIX_MAXBITS + 1]; isc_radix_node_t **sp = stack; isc_radix_node_t *cur = radix->head; - REQUIRE(radix != NULL); - while (cur != NULL) { isc_radix_node_t *l = cur->left; isc_radix_node_t *r = cur->right; - if (cur->prefix.family != 0) { - if (func != NULL) { - func(cur->data); - } - } else { - INSIST(cur->data[RADIX_V4] == NULL && - cur->data[RADIX_V6] == NULL); - } - isc_mem_put(radix->mctx, cur, sizeof(*cur)); radix->num_active_node--; @@ -114,22 +104,22 @@ clear_radix(isc_radix_tree_t *radix, isc_radix_destroyfunc_t func) { } void -isc_radix_destroy(isc_radix_tree_t *radix, isc_radix_destroyfunc_t func) { +isc_radix_destroy(isc_radix_tree_t *radix) { REQUIRE(radix != NULL); - clear_radix(radix, func); + clear_radix(radix); isc_mem_putanddetach(&radix->mctx, radix, sizeof(*radix)); } void isc_radix_foreach(isc_radix_tree_t *radix, isc_radix_foreachfunc_t func, void *arg) { + REQUIRE(radix != NULL); + REQUIRE(func != NULL); + isc_radix_node_t *stack[RADIX_MAXBITS + 1]; isc_radix_node_t **sp = stack; isc_radix_node_t *cur = radix->head; - REQUIRE(radix != NULL); - REQUIRE(func != NULL); - while (cur != NULL) { if (cur->prefix.family != 0) { func(cur, arg); @@ -168,8 +158,8 @@ isc_radix_search(isc_radix_tree_t *radix, isc_radix_node_t **target, } /* Walk the tree collecting candidate nodes. */ - u_char *addr = isc_prefix_touchar(prefix); - uint32_t bitlen = prefix->bitlen; + uint8_t *addr = isc_prefix_touint8(prefix); + uint8_t bitlen = prefix->bitlen; isc_radix_node_t *node = radix->head; while (node != NULL && node->bit < bitlen) { @@ -177,8 +167,7 @@ isc_radix_search(isc_radix_tree_t *radix, isc_radix_node_t **target, stack[cnt++] = node; } - if (BIT_TEST(addr[node->bit >> 3], 0x80 >> (node->bit & 0x07))) - { + if (isc_prefix_bit_isset(addr, node->bit)) { node = node->right; } else { node = node->left; @@ -198,8 +187,8 @@ isc_radix_search(isc_radix_tree_t *radix, isc_radix_node_t **target, continue; } - if (comp_with_mask(isc_prefix_tochar(&node->prefix), - isc_prefix_tochar(prefix), + if (comp_with_mask(isc_prefix_touint8(&node->prefix), + isc_prefix_touint8(prefix), node->prefix.bitlen)) { int fam = ISC_RADIX_FAMILY(prefix); @@ -237,7 +226,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, INSIST(prefix != NULL); - uint32_t bitlen = prefix->bitlen; + uint8_t bitlen = prefix->bitlen; if (radix->head == NULL) { node = radix_node_create(radix->mctx, prefix, bitlen); @@ -256,7 +245,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, radix->num_added_node + source->node_num[i]; } - node->data[i] = source->data[i]; + node->match[i] = source->match[i]; } } else { int next = ++radix->num_added_node; @@ -268,12 +257,12 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, return; } - u_char *addr = isc_prefix_touchar(prefix); + uint8_t *addr = isc_prefix_touint8(prefix); node = radix->head; while (node->bit < bitlen || node->prefix.family == 0) { if (node->bit < radix->maxbits && - BIT_TEST(addr[node->bit >> 3], 0x80 >> (node->bit & 0x07))) + isc_prefix_bit_isset(addr, node->bit)) { if (node->right == NULL) { break; @@ -292,23 +281,16 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, INSIST(node->prefix.family != 0); /* Find the first bit different. */ - u_char *test_addr = isc_prefix_touchar(&node->prefix); - uint32_t check_bit = (node->bit < bitlen) ? node->bit : bitlen; - uint32_t differ_bit = 0; + uint8_t *test_addr = isc_prefix_touint8(&node->prefix); + uint8_t check_bit = (node->bit < bitlen) ? node->bit : bitlen; + uint8_t differ_bit = 0; for (size_t i = 0; i * 8 < check_bit; i++) { uint8_t r = addr[i] ^ test_addr[i]; if (r == 0) { differ_bit = (i + 1) * 8; continue; } - uint8_t j; - for (j = 0; j < 8; j++) { - if (BIT_TEST(r, 0x80 >> j)) { - break; - } - } - INSIST(j < 8); - differ_bit = i * 8 + j; + differ_bit = i * 8 + (stdc_leading_zeros((unsigned int)r) - 24); break; } @@ -334,7 +316,8 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, node->node_num[i] = radix->num_added_node + source->node_num[i]; - node->data[i] = source->data[i]; + node->match[i] = + source->match[i]; } } } else { @@ -349,9 +332,9 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, } else { node->prefix = *prefix; } - INSIST(node->data[RADIX_V4] == NULL && + INSIST(node->match[RADIX_V4] == RADIX_UNSET && node->node_num[RADIX_V4] == -1 && - node->data[RADIX_V6] == NULL && + node->match[RADIX_V6] == RADIX_UNSET && node->node_num[RADIX_V6] == -1); if (source != NULL) { /* Merging node */ @@ -360,7 +343,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, if (source->node_num[i] != -1) { node->node_num[i] = source->node_num[i] + cur; - node->data[i] = source->data[i]; + node->match[i] = source->match[i]; } } } else { @@ -386,7 +369,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, if (source->node_num[i] != -1) { new_node->node_num[i] = source->node_num[i] + cur; - new_node->data[i] = source->data[i]; + new_node->match[i] = source->match[i]; } } } else { @@ -398,7 +381,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, INSIST(glue == NULL); new_node->parent = node; if (node->bit < radix->maxbits && - BIT_TEST(addr[node->bit >> 3], 0x80 >> (node->bit & 0x07))) + isc_prefix_bit_isset(addr, node->bit)) { INSIST(node->right == NULL); node->right = new_node; @@ -413,7 +396,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, if (bitlen == differ_bit) { INSIST(glue == NULL); if (bitlen < radix->maxbits && - BIT_TEST(test_addr[bitlen >> 3], 0x80 >> (bitlen & 0x07))) + isc_prefix_bit_isset(test_addr, bitlen)) { new_node->right = node; } else { @@ -434,7 +417,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, glue->parent = node->parent; radix->num_active_node++; if (differ_bit < radix->maxbits && - BIT_TEST(addr[differ_bit >> 3], 0x80 >> (differ_bit & 07))) + isc_prefix_bit_isset(addr, differ_bit)) { glue->right = new_node; glue->left = node; @@ -473,7 +456,8 @@ isc_radix_remove(isc_radix_tree_t *radix, isc_radix_node_t *node) { * make sure there is a prefix associated with it! */ memset(&node->prefix, 0, sizeof(node->prefix)); - memset(node->data, 0, sizeof(node->data)); + node->match[RADIX_V4] = RADIX_UNSET; + node->match[RADIX_V6] = RADIX_UNSET; return; } diff --git a/lib/isccfg/aclconf.c b/lib/isccfg/aclconf.c index af98ebbca69..b0840deaacb 100644 --- a/lib/isccfg/aclconf.c +++ b/lib/isccfg/aclconf.c @@ -744,7 +744,9 @@ cfg_acl_fromconfig(const cfg_obj_t *acl_data, const cfg_obj_t *cctx, * the nestedacl element, not the iptable entry. */ setpos = (nest_level != 0 || !neg); - dns_iptable_addprefix(iptab, &addr, bitlen, setpos); + dns_iptable_addprefix(iptab, &addr, bitlen, + setpos ? RADIX_ALLOW + : RADIX_DENY); if (nest_level > 0) { INSIST(dacl->length < dacl->alloc); @@ -813,7 +815,9 @@ cfg_acl_fromconfig(const cfg_obj_t *acl_data, const cfg_obj_t *cctx, if (strcasecmp(name, "any") == 0) { /* Iptable entry with zero bit length. */ setpos = (nest_level != 0 || !neg); - dns_iptable_addprefix(iptab, NULL, 0, setpos); + dns_iptable_addprefix(iptab, NULL, 0, + setpos ? RADIX_ALLOW + : RADIX_DENY); if (nest_level != 0) { INSIST(dacl->length < dacl->alloc); @@ -831,7 +835,9 @@ cfg_acl_fromconfig(const cfg_obj_t *acl_data, const cfg_obj_t *cctx, * "!none;". */ setpos = (nest_level != 0 || neg); - dns_iptable_addprefix(iptab, NULL, 0, setpos); + dns_iptable_addprefix(iptab, NULL, 0, + setpos ? RADIX_ALLOW + : RADIX_DENY); if (!neg) { dacl->has_negatives = !neg; diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index da05573cefc..49ab9e3ed16 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -840,7 +840,8 @@ setup_locals(isc_interface_t *interface, dns_acl_t *localhost, /* First add localhost address */ prefixlen = (netaddr->family == AF_INET) ? 32 : 128; - dns_iptable_addprefix(localhost->iptable, netaddr, prefixlen, true); + dns_iptable_addprefix(localhost->iptable, netaddr, prefixlen, + RADIX_ALLOW); /* Then add localnets prefix */ result = isc_netaddr_masktoprefixlen(&interface->netmask, &prefixlen); @@ -869,7 +870,8 @@ setup_locals(isc_interface_t *interface, dns_acl_t *localhost, return ISC_R_SUCCESS; } - dns_iptable_addprefix(localnets->iptable, netaddr, prefixlen, true); + dns_iptable_addprefix(localnets->iptable, netaddr, prefixlen, + RADIX_ALLOW); return ISC_R_SUCCESS; } diff --git a/tests/isc/radix_test.c b/tests/isc/radix_test.c index 8a4599f64ba..3b402223d22 100644 --- a/tests/isc/radix_test.c +++ b/tests/isc/radix_test.c @@ -38,68 +38,328 @@ prefix_from_str(const char *str, uint16_t bitlen, isc_prefix_t *pfx) { in_addr.s_addr = inet_addr(str); isc_netaddr_fromin(&netaddr, &in_addr); - NETADDR_TO_PREFIX_T(&netaddr, *pfx, bitlen); + isc_prefix_from_netaddr(pfx, &netaddr, bitlen); +} + +static void +prefix6_from_str(const char *str, uint16_t bitlen, isc_prefix_t *pfx) { + struct in6_addr in6; + isc_netaddr_t netaddr; + + assert_int_equal(inet_pton(AF_INET6, str, &in6), 1); + isc_netaddr_fromin6(&netaddr, &in6); + isc_prefix_from_netaddr(pfx, &netaddr, bitlen); } static isc_radix_node_t * -insert_prefix(isc_radix_tree_t *radix, const char *str, uint16_t bitlen, - void *data) { +insert_v4(isc_radix_tree_t *radix, const char *str, uint16_t bitlen, + isc_radix_match_t match) { isc_prefix_t pfx; isc_radix_node_t *node = NULL; prefix_from_str(str, bitlen, &pfx); + isc_radix_insert(radix, &node, NULL, &pfx); + node->match[RADIX_V4] = match; + + return node; +} + +static isc_radix_node_t * +insert_v6(isc_radix_tree_t *radix, const char *str, uint16_t bitlen, + isc_radix_match_t match) { + isc_prefix_t pfx; + isc_radix_node_t *node = NULL; + prefix6_from_str(str, bitlen, &pfx); isc_radix_insert(radix, &node, NULL, &pfx); - node->data[RADIX_V4] = data; + node->match[RADIX_V6] = match; return node; } -ISC_RUN_TEST_IMPL(isc_radix_remove) { +/* Search an empty tree. */ +ISC_RUN_TEST_IMPL(radix_search_empty) { + isc_radix_tree_t *radix = NULL; + isc_radix_node_t *node = NULL; + isc_prefix_t pfx; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + prefix_from_str("10.0.0.1", 32, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_NOTFOUND); + + isc_radix_destroy(radix); +} + +/* Search miss in a populated tree. */ +ISC_RUN_TEST_IMPL(radix_search_miss) { + isc_radix_tree_t *radix = NULL; + isc_radix_node_t *node = NULL; + isc_prefix_t pfx; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + insert_v4(radix, "10.0.0.0", 8, RADIX_ALLOW); + + prefix_from_str("192.168.1.1", 32, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_NOTFOUND); + + isc_radix_destroy(radix); +} + +/* Exact match: insert only one prefix, search for it exactly. */ +ISC_RUN_TEST_IMPL(radix_search_exact) { + isc_radix_tree_t *radix = NULL; + isc_radix_node_t *node = NULL; + isc_prefix_t pfx; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + insert_v4(radix, "10.1.0.0", 16, RADIX_DENY); + + prefix_from_str("10.1.0.0", 16, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + assert_int_equal(node->match[RADIX_V4], RADIX_DENY); + + isc_radix_destroy(radix); +} + +/* + * First-match wins over best-match: even though /16 is more specific, + * the /8 was inserted first and wins. + */ +ISC_RUN_TEST_IMPL(radix_search_best_match) { isc_radix_tree_t *radix = NULL; isc_radix_node_t *node = NULL; + isc_prefix_t pfx; UNUSED(state); - isc_radix_create(isc_g_mctx, &radix, 32); + isc_radix_create(isc_g_mctx, &radix, 128); - insert_prefix(radix, "1.1.1.1", 32, (void *)1); - insert_prefix(radix, "1.0.0.0", 8, (void *)2); - node = insert_prefix(radix, "1.1.1.0", 24, (void *)3); + insert_v4(radix, "10.0.0.0", 8, RADIX_ALLOW); + insert_v4(radix, "10.1.0.0", 16, RADIX_DENY); - isc_radix_remove(radix, node); + /* 10.1.2.3/32 matches both /8 and /16, but /8 was first. */ + prefix_from_str("10.1.2.3", 32, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + assert_int_equal(node->match[RADIX_V4], RADIX_ALLOW); - isc_radix_destroy(radix, NULL); + isc_radix_destroy(radix); } -ISC_RUN_TEST_IMPL(isc_radix_search) { +/* + * First-match semantics: when multiple prefixes match, the one that + * was inserted first (lowest node_num) wins. + */ +ISC_RUN_TEST_IMPL(radix_first_match) { isc_radix_tree_t *radix = NULL; isc_radix_node_t *node = NULL; isc_prefix_t pfx; UNUSED(state); - isc_radix_create(isc_g_mctx, &radix, 32); + isc_radix_create(isc_g_mctx, &radix, 128); - insert_prefix(radix, "3.3.3.0", 24, (void *)1); - insert_prefix(radix, "3.3.0.0", 16, (void *)2); + /* Insert /8 first, then /16. Both match 10.1.2.3. */ + insert_v4(radix, "10.0.0.0", 8, RADIX_ALLOW); + insert_v4(radix, "10.1.0.0", 16, RADIX_DENY); /* - * Search for 3.3.3.3/22 -- should match the /16 entry since - * 3.3.3.3 falls within 3.3.0.0/16 and the /24 doesn't cover /22. + * Search with /8 bitlen -- both /8 and /16 entries have + * prefixes that match within 8 bits, but /8 was inserted + * first so it should win. */ - prefix_from_str("3.3.3.3", 22, &pfx); - isc_result_t result = isc_radix_search(radix, &node, &pfx); - assert_int_equal(result, ISC_R_SUCCESS); - assert_ptr_equal(node->data[RADIX_V4], (void *)2); + prefix_from_str("10.1.0.0", 8, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + assert_int_equal(node->match[RADIX_V4], RADIX_ALLOW); + + isc_radix_destroy(radix); +} + +/* Duplicate insert: same prefix should preserve the first match value. */ +ISC_RUN_TEST_IMPL(radix_duplicate_insert) { + isc_radix_tree_t *radix = NULL; + isc_radix_node_t *node = NULL; + isc_prefix_t pfx; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + insert_v4(radix, "10.0.0.0", 8, RADIX_ALLOW); + + /* Second insert of same prefix returns the existing node. */ + isc_radix_node_t *dup = NULL; + prefix_from_str("10.0.0.0", 8, &pfx); + isc_radix_insert(radix, &dup, NULL, &pfx); + /* The match value should be unchanged (first insert wins). */ + assert_int_equal(dup->match[RADIX_V4], RADIX_ALLOW); + + prefix_from_str("10.0.0.1", 32, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + assert_int_equal(node->match[RADIX_V4], RADIX_ALLOW); + + isc_radix_destroy(radix); +} + +/* IPv6 prefix insert and search. */ +ISC_RUN_TEST_IMPL(radix_ipv6) { + isc_radix_tree_t *radix = NULL; + isc_radix_node_t *node = NULL; + isc_prefix_t pfx; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + insert_v6(radix, "2001:db8::", 32, RADIX_ALLOW); + insert_v6(radix, "2001:db8:1::", 48, RADIX_DENY); + + /* First-match: /32 was inserted first, so it wins. */ + prefix6_from_str("2001:db8:1::1", 128, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + assert_int_equal(node->match[RADIX_V6], RADIX_ALLOW); + + /* Address outside the prefixes. */ + node = NULL; + prefix6_from_str("2001:db9::1", 128, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_NOTFOUND); + + isc_radix_destroy(radix); +} + +/* + * Remove a leaf node: the node has no children, so it and its + * glue parent (if any) should be freed. + */ +ISC_RUN_TEST_IMPL(radix_remove_leaf) { + isc_radix_tree_t *radix = NULL; + isc_radix_node_t *node = NULL; + isc_prefix_t pfx; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + insert_v4(radix, "10.0.0.0", 8, RADIX_ALLOW); + isc_radix_node_t *leaf = insert_v4(radix, "10.1.0.0", 16, RADIX_DENY); + + isc_radix_remove(radix, leaf); + + /* The /8 should still be findable. */ + prefix_from_str("10.0.0.1", 32, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + assert_int_equal(node->match[RADIX_V4], RADIX_ALLOW); + + /* The /16 should be gone. */ + node = NULL; + prefix_from_str("10.1.0.1", 16, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + /* Should match the /8, not the removed /16. */ + assert_int_equal(node->match[RADIX_V4], RADIX_ALLOW); + + isc_radix_destroy(radix); +} + +/* + * Remove a node with two children: the node is converted to a + * glue node (prefix cleared) but stays in the tree. + */ +ISC_RUN_TEST_IMPL(radix_remove_internal) { + isc_radix_tree_t *radix = NULL; + isc_radix_node_t *node = NULL; + isc_prefix_t pfx; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + insert_v4(radix, "10.0.0.0", 8, RADIX_ALLOW); + isc_radix_node_t *mid = insert_v4(radix, "10.1.0.0", 16, RADIX_DENY); + insert_v4(radix, "10.1.1.0", 24, RADIX_ALLOW); + + /* Remove the /16 which sits between /8 and /24. */ + isc_radix_remove(radix, mid); + + /* The /24 should still be reachable. */ + prefix_from_str("10.1.1.1", 32, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + assert_int_equal(node->match[RADIX_V4], RADIX_ALLOW); + + /* A search for the /16 range should now fall back to /8. */ + node = NULL; + prefix_from_str("10.1.2.1", 32, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_SUCCESS); + assert_int_equal(node->match[RADIX_V4], RADIX_ALLOW); + + isc_radix_destroy(radix); +} + +/* Remove the only node in the tree. */ +ISC_RUN_TEST_IMPL(radix_remove_root) { + isc_radix_tree_t *radix = NULL; + isc_radix_node_t *node = NULL; + isc_prefix_t pfx; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + isc_radix_node_t *root = insert_v4(radix, "10.0.0.0", 8, RADIX_ALLOW); + isc_radix_remove(radix, root); + + prefix_from_str("10.0.0.1", 32, &pfx); + assert_int_equal(isc_radix_search(radix, &node, &pfx), ISC_R_NOTFOUND); + + isc_radix_destroy(radix); +} + +/* Test isc_radix_foreach iteration. */ +static void +count_nodes(isc_radix_node_t *node, void *arg) { + UNUSED(node); + int *count = arg; + (*count)++; +} + +ISC_RUN_TEST_IMPL(radix_foreach) { + isc_radix_tree_t *radix = NULL; + int count = 0; + + UNUSED(state); + + isc_radix_create(isc_g_mctx, &radix, 128); + + insert_v4(radix, "10.0.0.0", 8, RADIX_ALLOW); + insert_v4(radix, "10.1.0.0", 16, RADIX_DENY); + insert_v4(radix, "192.168.0.0", 16, RADIX_ALLOW); + + isc_radix_foreach(radix, count_nodes, &count); + assert_int_equal(count, 3); - isc_radix_destroy(radix, NULL); + isc_radix_destroy(radix); } ISC_TEST_LIST_START -ISC_TEST_ENTRY(isc_radix_remove) -ISC_TEST_ENTRY(isc_radix_search) +ISC_TEST_ENTRY(radix_search_empty) +ISC_TEST_ENTRY(radix_search_miss) +ISC_TEST_ENTRY(radix_search_exact) +ISC_TEST_ENTRY(radix_search_best_match) +ISC_TEST_ENTRY(radix_first_match) +ISC_TEST_ENTRY(radix_duplicate_insert) +ISC_TEST_ENTRY(radix_ipv6) +ISC_TEST_ENTRY(radix_remove_leaf) +ISC_TEST_ENTRY(radix_remove_internal) +ISC_TEST_ENTRY(radix_remove_root) +ISC_TEST_ENTRY(radix_foreach) ISC_TEST_LIST_END ISC_TEST_MAIN