summaryrefslogtreecommitdiffstats
path: root/bfdd
diff options
context:
space:
mode:
authorIgor Ryzhov <iryzhov@nfware.com>2021-02-02 23:02:15 +0100
committerIgor Ryzhov <iryzhov@nfware.com>2021-02-03 23:22:29 +0100
commit6cfcb775ef5d6004857b3d68e2bf757cf2a7a871 (patch)
tree82ca006f97002907ab56b4a26f3cb4a6fa16e9a5 /bfdd
parentMerge pull request #7314 from kuldeepkash/multicast-sm-topo1 (diff)
downloadfrr-6cfcb775ef5d6004857b3d68e2bf757cf2a7a871.tar.xz
frr-6cfcb775ef5d6004857b3d68e2bf757cf2a7a871.zip
bfdd: fix session lookup
BFD key has optional fields "local" and "ifname" which can be empty when the BFD session is created. In this case, the hash key will be calculated with these fields filled with zeroes. Later, when we're looking for the BFD session using the key with fields "local" and "ifname" populated with actual values, the hash key will be different. To work around this issue, we're doing multiple hash lookups, first with full key, then with fields "local" and "ifname" filled with zeroes. But there may be another case when the initial key has the actual values for "local" and "ifname", but the key we're using for lookup has empty values. This case is covered for IPv4 by using additional hash walk with bfd_key_lookup_ignore_partial_walker function but is not covered for IPv6. Instead of introducing more hacks and workarounds, the following solution is proposed: - the hash key is always calculated in bfd_key_hash_do using only required fields - the hash data is compared in bfd_key_hash_cmp, taking into account the fact that fields "local" and "ifname" may be empty Using this solution, it's enough to make only one hash lookup. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Diffstat (limited to 'bfdd')
-rw-r--r--bfdd/bfd.c160
1 files changed, 45 insertions, 115 deletions
diff --git a/bfdd/bfd.c b/bfdd/bfd.c
index ca4bf9495..3e45bf0e0 100644
--- a/bfdd/bfd.c
+++ b/bfdd/bfd.c
@@ -1672,15 +1672,54 @@ static bool bfd_id_hash_cmp(const void *n1, const void *n2)
static unsigned int bfd_key_hash_do(const void *p)
{
const struct bfd_session *bs = p;
+ struct bfd_key key = bs->key;
- return jhash(&bs->key, sizeof(bs->key), 0);
+ /*
+ * Local address and interface name are optional and
+ * can be filled any time after session creation.
+ * Hash key should not depend on these fields.
+ */
+ memset(&key.local, 0, sizeof(key.local));
+ memset(key.ifname, 0, sizeof(key.ifname));
+
+ return jhash(&key, sizeof(key), 0);
}
static bool bfd_key_hash_cmp(const void *n1, const void *n2)
{
const struct bfd_session *bs1 = n1, *bs2 = n2;
- return memcmp(&bs1->key, &bs2->key, sizeof(bs1->key)) == 0;
+ if (bs1->key.family != bs2->key.family)
+ return false;
+ if (bs1->key.mhop != bs2->key.mhop)
+ return false;
+ if (memcmp(&bs1->key.peer, &bs2->key.peer, sizeof(bs1->key.peer)))
+ return false;
+ if (memcmp(bs1->key.vrfname, bs2->key.vrfname,
+ sizeof(bs1->key.vrfname)))
+ return false;
+
+ /*
+ * Local address is optional and can be empty.
+ * If both addresses are not empty and different,
+ * then the keys are different.
+ */
+ if (memcmp(&bs1->key.local, &zero_addr, sizeof(bs1->key.local))
+ && memcmp(&bs2->key.local, &zero_addr, sizeof(bs2->key.local))
+ && memcmp(&bs1->key.local, &bs2->key.local, sizeof(bs1->key.local)))
+ return false;
+
+ /*
+ * Interface name is optional and can be empty.
+ * If both names are not empty and different,
+ * then the keys are different.
+ */
+ if (bs1->key.ifname[0] && bs2->key.ifname[0]
+ && memcmp(bs1->key.ifname, bs2->key.ifname,
+ sizeof(bs1->key.ifname)))
+ return false;
+
+ return true;
}
@@ -1698,117 +1737,13 @@ struct bfd_session *bfd_id_lookup(uint32_t id)
return hash_lookup(bfd_id_hash, &bs);
}
-struct bfd_key_walk_partial_lookup {
- struct bfd_session *given;
- struct bfd_session *result;
-};
-
-/* ignore some parameters */
-static int bfd_key_lookup_ignore_partial_walker(struct hash_bucket *b,
- void *data)
-{
- struct bfd_key_walk_partial_lookup *ctx =
- (struct bfd_key_walk_partial_lookup *)data;
- struct bfd_session *given = ctx->given;
- struct bfd_session *parsed = b->data;
-
- if (given->key.family != parsed->key.family)
- return HASHWALK_CONTINUE;
- if (given->key.mhop != parsed->key.mhop)
- return HASHWALK_CONTINUE;
- if (memcmp(&given->key.peer, &parsed->key.peer,
- sizeof(struct in6_addr)))
- return HASHWALK_CONTINUE;
- if (memcmp(given->key.vrfname, parsed->key.vrfname, MAXNAMELEN))
- return HASHWALK_CONTINUE;
- ctx->result = parsed;
- /* ignore localaddr or interface */
- return HASHWALK_ABORT;
-}
-
struct bfd_session *bfd_key_lookup(struct bfd_key key)
{
- struct bfd_session bs, *bsp;
- struct bfd_key_walk_partial_lookup ctx;
- char peer_buf[INET6_ADDRSTRLEN];
-
- bs.key = key;
- bsp = hash_lookup(bfd_key_hash, &bs);
- if (bsp)
- return bsp;
-
- inet_ntop(bs.key.family, &bs.key.peer, peer_buf,
- sizeof(peer_buf));
- /* Handle cases where local-address is optional. */
- if (memcmp(&bs.key.local, &zero_addr, sizeof(bs.key.local))) {
- memset(&bs.key.local, 0, sizeof(bs.key.local));
- bsp = hash_lookup(bfd_key_hash, &bs);
- if (bsp) {
- if (bglobal.debug_peer_event) {
- char addr_buf[INET6_ADDRSTRLEN];
- inet_ntop(bs.key.family, &key.local, addr_buf,
- sizeof(addr_buf));
- zlog_debug(
- " peer %s found, but loc-addr %s ignored",
- peer_buf, addr_buf);
- }
- return bsp;
- }
- }
-
- bs.key = key;
- /* Handle cases where ifname is optional. */
- if (bs.key.ifname[0]) {
- memset(bs.key.ifname, 0, sizeof(bs.key.ifname));
- bsp = hash_lookup(bfd_key_hash, &bs);
- if (bsp) {
- if (bglobal.debug_peer_event)
- zlog_debug(" peer %s found, but ifp %s ignored",
- peer_buf, key.ifname);
- return bsp;
- }
- }
+ struct bfd_session bs;
- /* Handle cases where local-address and ifname are optional. */
- if (bs.key.family == AF_INET) {
- memset(&bs.key.local, 0, sizeof(bs.key.local));
- bsp = hash_lookup(bfd_key_hash, &bs);
- if (bsp) {
- if (bglobal.debug_peer_event) {
- char addr_buf[INET6_ADDRSTRLEN];
- inet_ntop(bs.key.family, &bs.key.local,
- addr_buf, sizeof(addr_buf));
- zlog_debug(
- " peer %s found, but ifp %s and loc-addr %s ignored",
- peer_buf, key.ifname, addr_buf);
- }
- return bsp;
- }
- }
bs.key = key;
- /* Handle case where a context more complex ctx is present.
- * input has no iface nor local-address, but a context may
- * exist.
- *
- * Only applies to IPv4, because IPv6 requires either
- * local-address or interface.
- */
- if (!bs.key.mhop && bs.key.family == AF_INET) {
- ctx.result = NULL;
- ctx.given = &bs;
- hash_walk(bfd_key_hash, &bfd_key_lookup_ignore_partial_walker,
- &ctx);
- /* change key */
- if (ctx.result) {
- bsp = ctx.result;
- if (bglobal.debug_peer_event)
- zlog_debug(
- " peer %s found, but ifp and/or loc-addr params ignored",
- peer_buf);
- }
- }
- return bsp;
+ return hash_lookup(bfd_key_hash, &bs);
}
/*
@@ -1832,16 +1767,11 @@ struct bfd_session *bfd_id_delete(uint32_t id)
struct bfd_session *bfd_key_delete(struct bfd_key key)
{
- struct bfd_session bs, *bsp;
+ struct bfd_session bs;
bs.key = key;
- bsp = hash_lookup(bfd_key_hash, &bs);
- if (bsp == NULL && key.ifname[0]) {
- memset(bs.key.ifname, 0, sizeof(bs.key.ifname));
- bsp = hash_lookup(bfd_key_hash, &bs);
- }
- return hash_release(bfd_key_hash, bsp);
+ return hash_release(bfd_key_hash, &bs);
}
/* Iteration functions. */