From 8e7a4c6e2b8196751e4ab689bc73d064bbee0e03 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 3 Aug 2017 10:56:30 -0400 Subject: [PATCH 1/2] pimd: Lookup S,G ifchannel after we create it There are situations where we receive a *,G with a S,G,RPT Prune embedded where we do not actually have any S,G yet(MSDP with multiple RP's with the same address). As such since we only need to lookup the S,G ifchannel once, do it after the recv_prune. Ticket: CM-17230 Signed-off-by: Donald Sharp --- pimd/pim_join.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pimd/pim_join.c b/pimd/pim_join.c index 9796f580ce..c60e5a65aa 100644 --- a/pimd/pim_join.c +++ b/pimd/pim_join.c @@ -306,14 +306,20 @@ int pim_joinprune_recv(struct interface *ifp, struct pim_neighbor *neigh, return -8; } - sg_ch = pim_ifchannel_find(ifp, &sg); - buf += addr_offset; starg_alone = 0; recv_prune(ifp, neigh, msg_holdtime, msg_upstream_addr.u.prefix4, &sg, msg_source_flags); + /* + * So if we are receiving a S,G,RPT prune + * before we have any data for that S,G + * We need to retrieve the sg_ch after + * we parse the prune. + */ + sg_ch = pim_ifchannel_find(ifp, &sg); + /* Received SG-RPT Prune delete oif from specific S,G */ if (starg_ch && sg_ch && (msg_source_flags & PIM_RPT_BIT_MASK) From f09eaf1c8e6f8b76974c38e85cd70495d94af15b Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 3 Aug 2017 18:05:47 -0400 Subject: [PATCH 2/2] pimd: Fix crash on iface down due to secondary address list The secondary address list was being added/removed as we went. I see no reason to have special bookkeeping for this list. Just add it on interface startup and then remove it on deletion. Removes some very specialized coding that was saving a very small amount of space. Signed-off-by: Donald Sharp --- pimd/pim_iface.c | 146 +++++++++++++++++++---------------------------- 1 file changed, 60 insertions(+), 86 deletions(-) diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c index 9fdbf7b3e3..1afcff7cb1 100644 --- a/pimd/pim_iface.c +++ b/pimd/pim_iface.c @@ -69,17 +69,14 @@ static void *if_list_clean(struct pim_interface *pim_ifp) { struct pim_ifchannel *ch; - if (pim_ifp->igmp_join_list) { + if (pim_ifp->igmp_join_list) list_delete(pim_ifp->igmp_join_list); - } - if (pim_ifp->igmp_socket_list) { + if (pim_ifp->igmp_socket_list) list_delete(pim_ifp->igmp_socket_list); - } - if (pim_ifp->pim_neighbor_list) { + if (pim_ifp->pim_neighbor_list) list_delete(pim_ifp->pim_neighbor_list); - } if (pim_ifp->upstream_switch_list) list_delete(pim_ifp->upstream_switch_list); @@ -96,6 +93,38 @@ static void *if_list_clean(struct pim_interface *pim_ifp) return 0; } +static void pim_sec_addr_free(struct pim_secondary_addr *sec_addr) +{ + XFREE(MTYPE_PIM_SEC_ADDR, sec_addr); +} + +static int pim_sec_addr_comp(const void *p1, const void *p2) +{ + const struct pim_secondary_addr *sec1 = p1; + const struct pim_secondary_addr *sec2 = p2; + + if (sec1->addr.family == AF_INET && sec2->addr.family == AF_INET6) + return -1; + + if (sec1->addr.family == AF_INET6 && sec2->addr.family == AF_INET) + return 1; + + if (sec1->addr.family == AF_INET) { + if (ntohl(sec1->addr.u.prefix4.s_addr) + < ntohl(sec2->addr.u.prefix4.s_addr)) + return -1; + + if (ntohl(sec1->addr.u.prefix4.s_addr) + > ntohl(sec2->addr.u.prefix4.s_addr)) + return 1; + } else { + return memcmp(&sec1->addr.u.prefix6, &sec2->addr.u.prefix6, + sizeof(struct in6_addr)); + } + + return 0; +} + struct pim_interface *pim_if_new(struct interface *ifp, int igmp, int pim) { struct pim_interface *pim_ifp; @@ -146,8 +175,8 @@ struct pim_interface *pim_if_new(struct interface *ifp, int igmp, int pim) /* list of struct igmp_sock */ pim_ifp->igmp_socket_list = list_new(); if (!pim_ifp->igmp_socket_list) { - zlog_err("%s %s: failure: igmp_socket_list=list_new()", - __FILE__, __PRETTY_FUNCTION__); + zlog_err("%s: failure: igmp_socket_list=list_new()", + __PRETTY_FUNCTION__); return if_list_clean(pim_ifp); } pim_ifp->igmp_socket_list->del = (void (*)(void *))igmp_sock_free; @@ -155,22 +184,31 @@ struct pim_interface *pim_if_new(struct interface *ifp, int igmp, int pim) /* list of struct pim_neighbor */ pim_ifp->pim_neighbor_list = list_new(); if (!pim_ifp->pim_neighbor_list) { - zlog_err("%s %s: failure: pim_neighbor_list=list_new()", - __FILE__, __PRETTY_FUNCTION__); + zlog_err("%s: failure: pim_neighbor_list=list_new()", + __PRETTY_FUNCTION__); return if_list_clean(pim_ifp); } pim_ifp->pim_neighbor_list->del = (void (*)(void *))pim_neighbor_free; pim_ifp->upstream_switch_list = list_new(); if (!pim_ifp->upstream_switch_list) { - zlog_err("%s %s: failure: upstream_switch_list=list_new()", - __FILE__, __PRETTY_FUNCTION__); + zlog_err("%s: failure: upstream_switch_list=list_new()", + __PRETTY_FUNCTION__); return if_list_clean(pim_ifp); } pim_ifp->upstream_switch_list->del = (void (*)(void *))pim_jp_agg_group_list_free; pim_ifp->upstream_switch_list->cmp = pim_jp_agg_group_list_cmp; + pim_ifp->sec_addr_list = list_new(); + if (!pim_ifp->sec_addr_list) { + zlog_err("%s: failure: secondary addresslist", + __PRETTY_FUNCTION__); + } + pim_ifp->sec_addr_list->del = (void (*)(void *))pim_sec_addr_free; + pim_ifp->sec_addr_list->cmp = + (int (*)(void *, void *))pim_sec_addr_comp; + RB_INIT(pim_ifchannel_rb, &pim_ifp->ifchannel_rb); ifp->info = pim_ifp; @@ -314,48 +352,12 @@ static int detect_primary_address_change(struct interface *ifp, return changed; } -static int pim_sec_addr_comp(const void *p1, const void *p2) -{ - const struct pim_secondary_addr *sec1 = p1; - const struct pim_secondary_addr *sec2 = p2; - - if (sec1->addr.family == AF_INET && sec2->addr.family == AF_INET6) - return -1; - - if (sec1->addr.family == AF_INET6 && sec2->addr.family == AF_INET) - return 1; - - if (sec1->addr.family == AF_INET) { - if (ntohl(sec1->addr.u.prefix4.s_addr) - < ntohl(sec2->addr.u.prefix4.s_addr)) - return -1; - - if (ntohl(sec1->addr.u.prefix4.s_addr) - > ntohl(sec2->addr.u.prefix4.s_addr)) - return 1; - } else { - return memcmp(&sec1->addr.u.prefix6, &sec2->addr.u.prefix6, - sizeof(struct in6_addr)); - } - - return 0; -} - -static void pim_sec_addr_free(struct pim_secondary_addr *sec_addr) -{ - XFREE(MTYPE_PIM_SEC_ADDR, sec_addr); -} - static struct pim_secondary_addr * pim_sec_addr_find(struct pim_interface *pim_ifp, struct prefix *addr) { struct pim_secondary_addr *sec_addr; struct listnode *node; - if (!pim_ifp->sec_addr_list) { - return NULL; - } - for (ALL_LIST_ELEMENTS_RO(pim_ifp->sec_addr_list, node, sec_addr)) { if (prefix_cmp(&sec_addr->addr, addr)) { return sec_addr; @@ -383,22 +385,9 @@ static int pim_sec_addr_add(struct pim_interface *pim_ifp, struct prefix *addr) return changed; } - if (!pim_ifp->sec_addr_list) { - pim_ifp->sec_addr_list = list_new(); - pim_ifp->sec_addr_list->del = - (void (*)(void *))pim_sec_addr_free; - pim_ifp->sec_addr_list->cmp = - (int (*)(void *, void *))pim_sec_addr_comp; - } - sec_addr = XCALLOC(MTYPE_PIM_SEC_ADDR, sizeof(*sec_addr)); - if (!sec_addr) { - if (list_isempty(pim_ifp->sec_addr_list)) { - list_free(pim_ifp->sec_addr_list); - pim_ifp->sec_addr_list = NULL; - } + if (!sec_addr) return changed; - } changed = 1; sec_addr->addr = *addr; @@ -411,15 +400,10 @@ static int pim_sec_addr_del_all(struct pim_interface *pim_ifp) { int changed = 0; - if (!pim_ifp->sec_addr_list) { - return changed; - } if (!list_isempty(pim_ifp->sec_addr_list)) { changed = 1; /* remove all nodes and free up the list itself */ list_delete_all_node(pim_ifp->sec_addr_list); - list_free(pim_ifp->sec_addr_list); - pim_ifp->sec_addr_list = NULL; } return changed; @@ -434,11 +418,9 @@ static int pim_sec_addr_update(struct interface *ifp) struct pim_secondary_addr *sec_addr; int changed = 0; - if (pim_ifp->sec_addr_list) { - for (ALL_LIST_ELEMENTS_RO(pim_ifp->sec_addr_list, node, - sec_addr)) { - sec_addr->flags |= PIM_SEC_ADDRF_STALE; - } + for (ALL_LIST_ELEMENTS_RO(pim_ifp->sec_addr_list, node, + sec_addr)) { + sec_addr->flags |= PIM_SEC_ADDRF_STALE; } for (ALL_LIST_ELEMENTS_RO(ifp->connected, node, ifc)) { @@ -459,20 +441,12 @@ static int pim_sec_addr_update(struct interface *ifp) } } - if (pim_ifp->sec_addr_list) { - /* Drop stale entries */ - for (ALL_LIST_ELEMENTS(pim_ifp->sec_addr_list, node, nextnode, - sec_addr)) { - if (sec_addr->flags & PIM_SEC_ADDRF_STALE) { - pim_sec_addr_del(pim_ifp, sec_addr); - changed = 1; - } - } - - /* If the list went empty free it up */ - if (list_isempty(pim_ifp->sec_addr_list)) { - list_free(pim_ifp->sec_addr_list); - pim_ifp->sec_addr_list = NULL; + /* Drop stale entries */ + for (ALL_LIST_ELEMENTS(pim_ifp->sec_addr_list, node, nextnode, + sec_addr)) { + if (sec_addr->flags & PIM_SEC_ADDRF_STALE) { + pim_sec_addr_del(pim_ifp, sec_addr); + changed = 1; } }