Bootstrap quick refresh on startup ignored

[josvarix]

When receiving a bootstrap message on startup, the BSM is always dropped because the cand_rp_list is not empty. The cand_rp_list is always populated with at lease a static entry for SSM 232.0.0.0/8

https://github.com/troglobit/pimd/blob/25608aa840b5d12bdfecf8bf9852277cf01d44d5/src/pim_proto.c#L3246-L3252

Maybe add a check to see if there are any dynamic entries in cand_rp_list? Or use a timer on startup to allow during first 60 seconds or so?

[troglobit]

Good catch! What do you think about this patch?

diff --git a/src/pim_proto.c b/src/pim_proto.c
index 9d63557..1168247 100644
--- a/src/pim_proto.c
+++ b/src/pim_proto.c
@@ -3243,12 +3243,31 @@ int receive_pim_bootstrap(uint32_t src, uint32_t dst, char *msg, size_t len)
        return FALSE;
    }

-   /* Probably unicasted from the current DR */
+
+   /* We have a Cand-RP-list already, check for static ones ... */
    if (cand_rp_list) {
-       /* Hmmm, I do have a Cand-RP-list, but some neighbor has a
-        * different opinion and is unicasting it to me. Ignore this guy.
-        */
-       return FALSE;
+       struct cand_rp *rp;
+       int any = 0;
+
+       for (rp = cand_rp_list; rp; rp = rp->next) {
+       struct rp_grp_entry *rp_grp = rp->rp_grp_next;
+       struct grp_mask *grp = rp_grp->group;
+       rpentry_t *entry = rp->rpentry;
+
+       if (entry->adv_holdtime == (uint16_t)0xffffff)
+           continue; /* Skip configured entries */
+
+       if (!IN_PIM_SSM_RANGE(grp->group_addr)) {
+           logit(LOG_INFO, 0, "Ignoring PIM Bootstrap candidate, CRP %s for %s exists.",
+             inet_fmt(entry->address, s1, sizeof(s1)),
+             netname(grp->group_addr, grp->group_mask));
+           any = 1;
+           break;
+       }
+       }
+
+       if (any)
+       return FALSE;
    }

    for (vifi = 0; vifi < numvifs; vifi++) {

[josvarix]

It works. I don't know if it is necessary to test for IN_PIM_SSM_RANGE because all static entries have max adv_holdtime. Logging should probably be lower level because this will trigger every time a neighbor reboots.

[troglobit]

@josvarix Yeah, I'm still not sure if we should ignore all static/configured entries or just the hard-coded one for SSM, so I put all the code in just as a POC. If we just skip all static ones the second if()-statement, including the log messages, could be removed entirely and the code be much simpler.

(Btw. LOG_NOTICE is the default and LOG_INFO is below that, so should not be seen at all unless operator enabled -l info.)

[troglobit]

Reduced diff proposal. I think we'll go with this, unless you have any objections @josvarix?

            return FALSE;
        }

-       /* Probably unicasted from the current DR */
        if (cand_rp_list) {
-           /* Hmmm, I do have a Cand-RP-list, but some neighbor has a
-            * different opinion and is unicasting it to me. Ignore this guy.
-            */
-           return FALSE;
+           struct cand_rp *rp;
+
+           /* We have a Cand-RP-list already, check for static ones ... */
+           for (rp = cand_rp_list; rp; rp = rp->next) {
+               rpentry_t *entry = rp->rpentry;
+
+               /* Skip static/configured ones */
+               if (entry->adv_holdtime == (uint16_t)0xffffff)
+                   continue;
+
+               /* Ignore PIM Bootstrap candidate, dynamic CRP exists. */
+               return FALSE;
+           }
        }

        for (vifi = 0; vifi < numvifs; vifi++) {

[josvarix]

I think is better to ignore all static/configured entries. The purpose is only allow unicast bootstrap message if we do not already have received any previous bootstrap message on startup.

[troglobit]

@josvarix Agreed, thank you for the input (and the bug report)! :smiley: