diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/nexthop.c | 87 | ||||
| -rw-r--r-- | lib/nexthop.h | 104 | ||||
| -rwxr-xr-x | lib/route_types.pl | 15 | ||||
| -rw-r--r-- | lib/zclient.c | 22 | ||||
| -rw-r--r-- | lib/zlog_5424.c | 12 |
5 files changed, 120 insertions, 120 deletions
diff --git a/lib/nexthop.c b/lib/nexthop.c index 332581fbd8..ee6c2b7ec0 100644 --- a/lib/nexthop.c +++ b/lib/nexthop.c @@ -772,68 +772,30 @@ unsigned int nexthop_level(const struct nexthop *nexthop) return rv; } -/* Only hash word-sized things, let cmp do the rest. */ -uint32_t nexthop_hash_quick(const struct nexthop *nexthop) +uint32_t nexthop_hash(const struct nexthop *nexthop) { uint32_t key = 0x45afe398; - int i; - key = jhash_3words(nexthop->type, nexthop->vrf_id, - nexthop->nh_label_type, key); - - if (nexthop->nh_label) { - int labels = nexthop->nh_label->num_labels; + /* type, vrf, ifindex, ip addresses - see nexthop.h */ + key = _nexthop_hash_bytes(nexthop, key); - i = 0; + key = jhash_1word(nexthop->flags & NEXTHOP_FLAGS_HASHED, key); - while (labels >= 3) { - key = jhash_3words(nexthop->nh_label->label[i], - nexthop->nh_label->label[i + 1], - nexthop->nh_label->label[i + 2], - key); - labels -= 3; - i += 3; - } - - if (labels >= 2) { - key = jhash_2words(nexthop->nh_label->label[i], - nexthop->nh_label->label[i + 1], - key); - labels -= 2; - i += 2; - } + if (nexthop->nh_label) { + const struct mpls_label_stack *ls = nexthop->nh_label; - if (labels >= 1) - key = jhash_1word(nexthop->nh_label->label[i], key); + /* num_labels itself isn't useful to hash, if the number of + * labels is different, the hash value will change just due to + * that already. + */ + key = jhash(ls->label, sizeof(ls->label[0]) * ls->num_labels, key); } - key = jhash_2words(nexthop->ifindex, - CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK), - key); - /* Include backup nexthops, if present */ if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { int backups = nexthop->backup_num; - i = 0; - - while (backups >= 3) { - key = jhash_3words(nexthop->backup_idx[i], - nexthop->backup_idx[i + 1], - nexthop->backup_idx[i + 2], key); - backups -= 3; - i += 3; - } - - while (backups >= 2) { - key = jhash_2words(nexthop->backup_idx[i], - nexthop->backup_idx[i + 1], key); - backups -= 2; - i += 2; - } - - if (backups >= 1) - key = jhash_1word(nexthop->backup_idx[i], key); + key = jhash(nexthop->backup_idx, sizeof(nexthop->backup_idx[0]) * backups, key); } if (nexthop->nh_srv6) { @@ -868,31 +830,6 @@ uint32_t nexthop_hash_quick(const struct nexthop *nexthop) return key; } - -#define GATE_SIZE 4 /* Number of uint32_t words in struct g_addr */ - -/* For a more granular hash */ -uint32_t nexthop_hash(const struct nexthop *nexthop) -{ - uint32_t gate_src_rmap_raw[GATE_SIZE * 3] = {}; - /* Get all the quick stuff */ - uint32_t key = nexthop_hash_quick(nexthop); - - assert(((sizeof(nexthop->gate) + sizeof(nexthop->src) - + sizeof(nexthop->rmap_src)) - / 3) - == (GATE_SIZE * sizeof(uint32_t))); - - memcpy(gate_src_rmap_raw, &nexthop->gate, GATE_SIZE); - memcpy(gate_src_rmap_raw + GATE_SIZE, &nexthop->src, GATE_SIZE); - memcpy(gate_src_rmap_raw + (2 * GATE_SIZE), &nexthop->rmap_src, - GATE_SIZE); - - key = jhash2(gate_src_rmap_raw, (GATE_SIZE * 3), key); - - return key; -} - void nexthop_copy_no_recurse(struct nexthop *copy, const struct nexthop *nexthop, struct nexthop *rparent) diff --git a/lib/nexthop.h b/lib/nexthop.h index 5dfb58d846..cea7c77e3e 100644 --- a/lib/nexthop.h +++ b/lib/nexthop.h @@ -8,6 +8,7 @@ #ifndef _LIB_NEXTHOP_H #define _LIB_NEXTHOP_H +#include "jhash.h" #include "prefix.h" #include "mpls.h" #include "vxlan.h" @@ -56,15 +57,48 @@ struct nexthop { struct nexthop *next; struct nexthop *prev; - /* - * What vrf is this nexthop associated with? + + /* begin of hashed data - all fields from here onwards are given to + * jhash() as one consecutive chunk. DO NOT create "padding holes". + * DO NOT insert pointers that need to be deep-hashed. + * + * static_assert() below needs to be updated when fields are added */ + char _hash_begin[0]; + + /* see above */ + enum nexthop_types_t type; + + /* What vrf is this nexthop associated with? */ vrf_id_t vrf_id; /* Interface index. */ ifindex_t ifindex; - enum nexthop_types_t type; + /* Type of label(s), if any */ + enum lsp_types_t nh_label_type; + + /* padding: keep 16 byte alignment here */ + + /* Nexthop address + * make sure all 16 bytes for IPv6 are zeroed when putting in an IPv4 + * address since the entire thing is hashed as-is + */ + union { + union g_addr gate; + enum blackhole_type bh_type; + }; + union g_addr src; + union g_addr rmap_src; /* Src is set via routemap */ + + /* end of hashed data - remaining fields in this struct are not + * directly fed into jhash(). Most of them are actually part of the + * hash but have special rules or handling attached. + */ + char _hash_end[0]; + + /* Weight of the nexthop ( for unequal cost ECMP ) */ + uint8_t weight; uint16_t flags; #define NEXTHOP_FLAG_ACTIVE (1 << 0) /* This nexthop is alive. */ @@ -82,18 +116,15 @@ struct nexthop { #define NEXTHOP_FLAG_EVPN (1 << 8) /* nexthop is EVPN */ #define NEXTHOP_FLAG_LINKDOWN (1 << 9) /* is not removed on link down */ + /* which flags are part of nexthop_hash(). Should probably be split + * off into a separate field... + */ +#define NEXTHOP_FLAGS_HASHED NEXTHOP_FLAG_ONLINK + #define NEXTHOP_IS_ACTIVE(flags) \ (CHECK_FLAG(flags, NEXTHOP_FLAG_ACTIVE) \ && !CHECK_FLAG(flags, NEXTHOP_FLAG_DUPLICATE)) - /* Nexthop address */ - union { - union g_addr gate; - enum blackhole_type bh_type; - }; - union g_addr src; - union g_addr rmap_src; /* Src is set via routemap */ - /* Nexthops obtained by recursive resolution. * * If the nexthop struct needs to be resolved recursively, @@ -104,15 +135,9 @@ struct nexthop { /* Recursive parent */ struct nexthop *rparent; - /* Type of label(s), if any */ - enum lsp_types_t nh_label_type; - /* Label(s) associated with this nexthop. */ struct mpls_label_stack *nh_label; - /* Weight of the nexthop ( for unequal cost ECMP ) */ - uint8_t weight; - /* Count and index of corresponding backup nexthop(s) in a backup list; * only meaningful if the HAS_BACKUP flag is set. */ @@ -138,6 +163,29 @@ struct nexthop { struct nexthop_srv6 *nh_srv6; }; +/* all hashed fields (including padding, if it is necessary to add) need to + * be listed in the static_assert below + */ + +#define S(field) sizeof(((struct nexthop *)NULL)->field) + +static_assert( + offsetof(struct nexthop, _hash_end) - offsetof(struct nexthop, _hash_begin) == + S(type) + S(vrf_id) + S(ifindex) + S(nh_label_type) + S(gate) + S(src) + S(rmap_src), + "struct nexthop contains padding, this can break things. insert _pad fields at appropriate places"); + +#undef S + +/* this is here to show exactly what is meant by the comments above about + * the hashing + */ +static inline uint32_t _nexthop_hash_bytes(const struct nexthop *nh, uint32_t seed) +{ + return jhash(&nh->_hash_begin, + offsetof(struct nexthop, _hash_end) - offsetof(struct nexthop, _hash_begin), + seed); +} + /* Utility to append one nexthop to another. */ #define NEXTHOP_APPEND(to, new) \ do { \ @@ -183,27 +231,11 @@ struct nexthop *nexthop_from_blackhole(enum blackhole_type bh_type, /* * Hash a nexthop. Suitable for use with hash tables. * - * This function uses the following values when computing the hash: - * - vrf_id - * - ifindex - * - type - * - gate - * - * nexthop - * The nexthop to hash - * - * Returns: - * 32-bit hash of nexthop + * Please double check the code on what is included in the hash, there was + * documentation here but it got outdated and the only thing worse than no + * doc is incorrect doc. */ uint32_t nexthop_hash(const struct nexthop *nexthop); -/* - * Hash a nexthop only on word-sized attributes: - * - vrf_id - * - ifindex - * - type - * - (some) flags - */ -uint32_t nexthop_hash_quick(const struct nexthop *nexthop); extern bool nexthop_same(const struct nexthop *nh1, const struct nexthop *nh2); extern bool nexthop_same_no_labels(const struct nexthop *nh1, diff --git a/lib/route_types.pl b/lib/route_types.pl index c75a866964..834cb822d2 100755 --- a/lib/route_types.pl +++ b/lib/route_types.pl @@ -127,9 +127,12 @@ printf "#define SHOW_ROUTE_V6_HEADER \\\n%s\n", codelist(@protosv6); print "\n"; sub collect { - my ($daemon, $ipv4, $ipv6, $any) = @_; + my ($daemon, $ipv4, $ipv6, $any, $ip_prot) = @_; my (@names, @help) = ((), ()); for my $p (@protos) { + next if ($ip_prot == 1 && $daemon eq "zebra" && $protodetail{$p}->{"cname"} eq "kernel"); + next if ($ip_prot == 1 && $daemon eq "zebra" && $protodetail{$p}->{"cname"} eq "connected"); + next if ($ip_prot == 1 && $daemon eq "zebra" && $protodetail{$p}->{"cname"} eq "local"); next if ($protodetail{$p}->{"daemon"} eq $daemon && $daemon ne "zebra"); next if ($protodetail{$p}->{"restrict2"} ne "" && $protodetail{$p}->{"restrict2"} ne $daemon); @@ -151,24 +154,24 @@ for my $daemon (sort keys %daemons) { next unless ($daemons{$daemon}->{"ipv4"} || $daemons{$daemon}->{"ipv6"}); printf "/* %s */\n", $daemon; if ($daemons{$daemon}->{"ipv4"} && $daemons{$daemon}->{"ipv6"}) { - my ($names, $help) = collect($daemon, 1, 1, 0); + my ($names, $help) = collect($daemon, 1, 1, 0, 0); printf "#define FRR_REDIST_STR_%s \\\n %s\n", uc $daemon, $names; printf "#define FRR_REDIST_HELP_STR_%s \\\n%s\n", uc $daemon, $help; - ($names, $help) = collect($daemon, 1, 0, 0); + ($names, $help) = collect($daemon, 1, 0, 0, 0); printf "#define FRR_IP_REDIST_STR_%s \\\n %s\n", uc $daemon, $names; printf "#define FRR_IP_REDIST_HELP_STR_%s \\\n%s\n", uc $daemon, $help; - ($names, $help) = collect($daemon, 0, 1, 0); + ($names, $help) = collect($daemon, 0, 1, 0, 0); printf "#define FRR_IP6_REDIST_STR_%s \\\n %s\n", uc $daemon, $names; printf "#define FRR_IP6_REDIST_HELP_STR_%s \\\n%s\n", uc $daemon, $help; if ($daemon eq "zebra") { - ($names, $help) = collect($daemon, 1, 0, 1); + ($names, $help) = collect($daemon, 1, 0, 1, 1); printf "#define FRR_IP_PROTOCOL_MAP_STR_%s \\\n %s\n", uc $daemon, $names; printf "#define FRR_IP_PROTOCOL_MAP_HELP_STR_%s \\\n%s\n", uc $daemon, $help; - ($names, $help) = collect($daemon, 0, 1, 1); + ($names, $help) = collect($daemon, 0, 1, 1, 1); printf "#define FRR_IP6_PROTOCOL_MAP_STR_%s \\\n %s\n", uc $daemon, $names; printf "#define FRR_IP6_PROTOCOL_MAP_HELP_STR_%s \\\n%s\n", uc $daemon, $help; } diff --git a/lib/zclient.c b/lib/zclient.c index d8c75c9029..5deea8f0cf 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -2300,7 +2300,27 @@ struct nexthop *nexthop_from_zapi_nexthop(const struct zapi_nexthop *znh) n->type = znh->type; n->vrf_id = znh->vrf_id; n->ifindex = znh->ifindex; - n->gate = znh->gate; + + /* only copy values that have meaning - make sure "spare bytes" are + * left zeroed for hashing (look at _nexthop_hash_bytes) + */ + switch (znh->type) { + case NEXTHOP_TYPE_BLACKHOLE: + n->bh_type = znh->bh_type; + break; + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: + n->gate.ipv4 = znh->gate.ipv4; + break; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: + n->gate.ipv6 = znh->gate.ipv6; + break; + case NEXTHOP_TYPE_IFINDEX: + /* nothing, ifindex is always copied */ + break; + } + n->srte_color = znh->srte_color; n->weight = znh->weight; diff --git a/lib/zlog_5424.c b/lib/zlog_5424.c index 4c60d4b405..6265ce3b1f 100644 --- a/lib/zlog_5424.c +++ b/lib/zlog_5424.c @@ -782,7 +782,7 @@ static void zlog_5424_cycle(struct zlog_cfg_5424 *zcf, int fd) } old = zcf->active ? &zcf->active->zt : NULL; - old = zlog_target_replace(old, &zlt->zt); + old = zlog_target_replace(old, zlt ? &zlt->zt : NULL); zcf->active = zlt; /* oldt->fd == fd happens for zlog_5424_apply_meta() */ @@ -1076,9 +1076,17 @@ bool zlog_5424_apply_dst(struct zlog_cfg_5424 *zcf) bool zlog_5424_apply_meta(struct zlog_cfg_5424 *zcf) { + int fd; + frr_with_mutex (&zcf->cfg_mtx) { if (zcf->active) - zlog_5424_cycle(zcf, zcf->active->fd); + fd = zcf->active->fd; + else if (zcf->prio_min != ZLOG_DISABLED) + fd = zlog_5424_open(zcf, -1); + else + fd = -1; + if (fd >= 0) + zlog_5424_cycle(zcf, fd); } return true; |