lib, pimd: add address match mode to prefix lists

... the PIM code is kinda misusing prefix lists to match addresses.
Considering the weird semantics of access-lists, I can't fault it.
However, prefix lists aren't great at matching addresses by default,
since they try to match the prefix length too.  So, here's an "address
match mode" for prefix lists to get that to work more reasonably.

Fixes: #8492
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/lib/plist.c b/lib/plist.c
index 0ee02f8a0b5f09bb00a82feddd095c5ce6ef76ee..1ee855a594ec4ed863280c3dfe1f3a3ede6bd3bc 100644 (file)
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -750,7 +750,7 @@ static const char *prefix_list_type_str(struct prefix_list_entry *pentry)
 }
 
 static int prefix_list_entry_match(struct prefix_list_entry *pentry,
-                                  const struct prefix *p)
+                                  const struct prefix *p, bool address_mode)
 {
        int ret;
 
@@ -761,6 +761,9 @@ static int prefix_list_entry_match(struct prefix_list_entry *pentry,
        if (!ret)
                return 0;
 
+       if (address_mode)
+               return 1;
+
        /* In case of le nor ge is specified, exact match is performed. */
        if (!pentry->le && !pentry->ge) {
                if (pentry->prefix.prefixlen != p->prefixlen)
@@ -777,14 +780,15 @@ static int prefix_list_entry_match(struct prefix_list_entry *pentry,
        return 1;
 }
 
-enum prefix_list_type prefix_list_apply_which_prefix(
+enum prefix_list_type prefix_list_apply_ext(
        struct prefix_list *plist,
-       const struct prefix **which,
-       const void *object)
+       const struct prefix_list_entry **which,
+       union prefixconstptr object,
+       bool address_mode)
 {
        struct prefix_list_entry *pentry, *pbest = NULL;
 
-       const struct prefix *p = (const struct prefix *)object;
+       const struct prefix *p = object.p;
        const uint8_t *byte = p->u.val;
        size_t depth;
        size_t validbits = p->prefixlen;
@@ -809,7 +813,7 @@ enum prefix_list_type prefix_list_apply_which_prefix(
                     pentry = pentry->next_best) {
                        if (pbest && pbest->seq < pentry->seq)
                                continue;
-                       if (prefix_list_entry_match(pentry, p))
+                       if (prefix_list_entry_match(pentry, p, address_mode))
                                pbest = pentry;
                }
 
@@ -830,7 +834,7 @@ enum prefix_list_type prefix_list_apply_which_prefix(
                     pentry = pentry->next_best) {
                        if (pbest && pbest->seq < pentry->seq)
                                continue;
-                       if (prefix_list_entry_match(pentry, p))
+                       if (prefix_list_entry_match(pentry, p, address_mode))
                                pbest = pentry;
                }
                break;
@@ -838,7 +842,7 @@ enum prefix_list_type prefix_list_apply_which_prefix(
 
        if (which) {
                if (pbest)
-                       *which = &pbest->prefix;
+                       *which = pbest;
                else
                        *which = NULL;
        }

diff --git a/lib/plist.h b/lib/plist.h
index 57eb763a6838e051a76f443435d1763204bef12e..c9507df57c88235a0448402265d67a36a32fef12 100644 (file)
--- a/lib/plist.h
+++ b/lib/plist.h
@@ -37,6 +37,7 @@ enum prefix_list_type {
 };
 
 struct prefix_list;
+struct prefix_list_entry;
 
 struct orf_prefix {
        uint32_t seq;
@@ -63,12 +64,18 @@ extern struct prefix_list *prefix_list_lookup(afi_t, const char *);
  *
  * If no pointer is sent in, do not return anything.
  * If it is a empty plist return a NULL pointer.
+ *
+ * address_mode = the "prefix" being passed in is really an address, match
+ * regardless of prefix length (i.e. ge/le are ignored.)  prefix->prefixlen
+ * must be /32.
  */
 extern enum prefix_list_type
-prefix_list_apply_which_prefix(struct prefix_list *plist,
-                              const struct prefix **which,
-                              const void *object);
-#define prefix_list_apply(A, B) prefix_list_apply_which_prefix((A), NULL, (B))
+prefix_list_apply_ext(struct prefix_list *plist,
+                     const struct prefix_list_entry **matches,
+                     union prefixconstptr prefix,
+                     bool address_mode);
+#define prefix_list_apply(A, B) \
+       prefix_list_apply_ext((A), NULL, (B), false)
 
 extern struct prefix_list *prefix_bgp_orf_lookup(afi_t, const char *);
 extern struct stream *prefix_bgp_orf_entry(struct stream *,

diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c
index b6521132f70fe0fca585ecf64cef22b1df36b34d..56e192752819c28ecc677939c8ca2b97e68a7698 100644 (file)
--- a/pimd/pim_rp.c
+++ b/pimd/pim_rp.c
@@ -203,6 +203,26 @@ static struct rp_info *pim_rp_find_exact(struct pim_instance *pim,
        return NULL;
 }
 
+/*
+ * XXX: long-term issue:  we don't actually have a good "ip address-list"
+ * implementation.  ("access-list XYZ" is the closest but honestly it's
+ * kinda garbage.)
+ *
+ * So it's using a prefix-list to match an address here, which causes very
+ * unexpected results for the user since prefix-lists by default only match
+ * when the prefix length is an exact match too.  i.e. you'd have to add the
+ * "le 32" and do "ip prefix-list foo permit 10.0.0.0/24 le 32"
+ *
+ * To avoid this pitfall, this code uses "address_mode = true" for the prefix
+ * list match (this is the only user for that.)
+ *
+ * In the long run, we need to add a "ip address-list", but that's a wholly
+ * separate bag of worms, and existing configs using ip prefix-list would
+ * drop into the UX pitfall.
+ */
+
+#include "lib/plist_int.h"
+
 /*
  * Given a group, return the rp_info for that group
  */
@@ -213,7 +233,8 @@ struct rp_info *pim_rp_find_match_group(struct pim_instance *pim,
        struct rp_info *best = NULL;
        struct rp_info *rp_info;
        struct prefix_list *plist;
-       const struct prefix *p, *bp;
+       const struct prefix *bp;
+       const struct prefix_list_entry *entry;
        struct route_node *rn;
 
        bp = NULL;
@@ -221,19 +242,19 @@ struct rp_info *pim_rp_find_match_group(struct pim_instance *pim,
                if (rp_info->plist) {
                        plist = prefix_list_lookup(AFI_IP, rp_info->plist);
 
-                       if (prefix_list_apply_which_prefix(plist, &p, group)
-                           == PREFIX_DENY)
+                       if (prefix_list_apply_ext(plist, &entry, group, true)
+                           == PREFIX_DENY || !entry)
                                continue;
 
                        if (!best) {
                                best = rp_info;
-                               bp = p;
+                               bp = &entry->prefix;
                                continue;
                        }
 
-                       if (bp && bp->prefixlen < p->prefixlen) {
+                       if (bp && bp->prefixlen < entry->prefix.prefixlen) {
                                best = rp_info;
-                               bp = p;
+                               bp = &entry->prefix;
                        }
                }
        }