From 1e9044be8d4325fa82f01d72eb6c8581dcd6fd06 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 23 Jun 2021 16:35:44 +0200 Subject: [PATCH] *: clean up ifp-by-local-address function(s) Most users of if_lookup_address_exact only cared about whether the address is any local address. Split that off into a separate function. For the users that actually need the ifp - which I'm about to add a few of - change it to prefer returning interfaces that are UP. (Function name changed due to slight change in behavior re. UP state, to avoid possible bugs from this change.) Signed-off-by: David Lamparter --- lib/if.c | 43 +++++++++++++++++++++++++++-------------- lib/if.h | 8 +++++++- pimd/pim_bsm.c | 4 ++-- pimd/pim_igmp.c | 2 +- pimd/pim_igmp_mtrace.c | 4 ++-- pimd/pim_register.c | 2 +- pimd/pim_rp.c | 9 --------- ripd/rip_snmp.c | 2 +- staticd/static_routes.c | 8 ++++---- staticd/static_zebra.c | 8 ++------ 10 files changed, 49 insertions(+), 41 deletions(-) diff --git a/lib/if.c b/lib/if.c index a40f04f5a2..6bfbdf9147 100644 --- a/lib/if.c +++ b/lib/if.c @@ -455,36 +455,51 @@ static struct interface *if_lookup_by_index_all_vrf(ifindex_t ifindex) return NULL; } -/* Lookup interface by IP address. */ -struct interface *if_lookup_exact_address(const void *src, int family, +/* Lookup interface by IP address. + * + * supersedes if_lookup_exact_address(), which didn't care about up/down + * state. but all users we have either only care if the address is local + * (=> use if_address_is_local() please), or care about UP interfaces before + * anything else + * + * to accept only UP interfaces, check if_is_up() on the returned ifp. + */ +struct interface *if_lookup_address_local(const void *src, int family, vrf_id_t vrf_id) { struct vrf *vrf = vrf_lookup_by_id(vrf_id); struct listnode *cnode; - struct interface *ifp; + struct interface *ifp, *best_down = NULL; struct prefix *p; struct connected *c; + if (family != AF_INET && family != AF_INET6) + return NULL; + FOR_ALL_INTERFACES (vrf, ifp) { for (ALL_LIST_ELEMENTS_RO(ifp->connected, cnode, c)) { p = c->address; - if (p && (p->family == family)) { - if (family == AF_INET) { - if (IPV4_ADDR_SAME( - &p->u.prefix4, + if (!p || p->family != family) + continue; + + if (family == AF_INET) { + if (!IPV4_ADDR_SAME(&p->u.prefix4, (struct in_addr *)src)) - return ifp; - } else if (family == AF_INET6) { - if (IPV6_ADDR_SAME( - &p->u.prefix6, + continue; + } else if (family == AF_INET6) { + if (!IPV6_ADDR_SAME(&p->u.prefix6, (struct in6_addr *)src)) - return ifp; - } + continue; } + + if (if_is_up(ifp)) + return ifp; + if (!best_down) + best_down = ifp; } } - return NULL; + return best_down; } /* Lookup interface by IP address. */ diff --git a/lib/if.h b/lib/if.h index 1012bf5557..5f0dc27555 100644 --- a/lib/if.h +++ b/lib/if.h @@ -511,7 +511,7 @@ extern void if_update_to_new_vrf(struct interface *, vrf_id_t vrf_id); extern struct interface *if_lookup_by_index(ifindex_t, vrf_id_t vrf_id); extern struct interface *if_vrf_lookup_by_index_next(ifindex_t ifindex, vrf_id_t vrf_id); -extern struct interface *if_lookup_exact_address(const void *matchaddr, +extern struct interface *if_lookup_address_local(const void *matchaddr, int family, vrf_id_t vrf_id); extern struct connected *if_lookup_address(const void *matchaddr, int family, vrf_id_t vrf_id); @@ -520,6 +520,12 @@ extern struct interface *if_lookup_prefix(const struct prefix *prefix, size_t if_lookup_by_hwaddr(const uint8_t *hw_addr, size_t addrsz, struct interface ***result, vrf_id_t vrf_id); +static inline bool if_address_is_local(const void *matchaddr, int family, + vrf_id_t vrf_id) +{ + return if_lookup_address_local(matchaddr, family, vrf_id) != NULL; +} + struct vrf; extern struct interface *if_lookup_by_name_vrf(const char *name, struct vrf *vrf); extern struct interface *if_lookup_by_name(const char *ifname, vrf_id_t vrf_id); diff --git a/pimd/pim_bsm.c b/pimd/pim_bsm.c index a3a3426f39..6c4c7eeb5a 100644 --- a/pimd/pim_bsm.c +++ b/pimd/pim_bsm.c @@ -1323,8 +1323,8 @@ int pim_bsm_process(struct interface *ifp, struct ip *ip_hdr, uint8_t *buf, return -1; } } - } else if (if_lookup_exact_address(&ip_hdr->ip_dst, AF_INET, - pim->vrf->vrf_id)) { + } else if (if_address_is_local(&ip_hdr->ip_dst, AF_INET, + pim->vrf->vrf_id)) { /* Unicast BSM received - if ucast bsm not enabled on * the interface, drop it */ diff --git a/pimd/pim_igmp.c b/pimd/pim_igmp.c index 795c96c838..cd905b3cbd 100644 --- a/pimd/pim_igmp.c +++ b/pimd/pim_igmp.c @@ -324,7 +324,7 @@ static int igmp_recv_query(struct igmp_sock *igmp, int query_version, return 0; } - if (if_lookup_exact_address(&from, AF_INET, ifp->vrf_id)) { + if (if_address_is_local(&from, AF_INET, ifp->vrf_id)) { if (PIM_DEBUG_IGMP_PACKETS) zlog_debug("Recv IGMP query on interface: %s from ourself %s", ifp->name, from_str); diff --git a/pimd/pim_igmp_mtrace.c b/pimd/pim_igmp_mtrace.c index 73af44fc46..42101bc48e 100644 --- a/pimd/pim_igmp_mtrace.c +++ b/pimd/pim_igmp_mtrace.c @@ -596,8 +596,8 @@ int igmp_mtrace_recv_qry_req(struct igmp_sock *igmp, struct ip *ip_hdr, * if applicable */ if (!IPV4_CLASS_DE(ntohl(ip_hdr->ip_dst.s_addr))) - if (!if_lookup_exact_address(&ip_hdr->ip_dst, AF_INET, - pim->vrf->vrf_id)) + if (!if_address_is_local(&ip_hdr->ip_dst, AF_INET, + pim->vrf->vrf_id)) return mtrace_forward_packet(pim, ip_hdr); if (igmp_msg_len < (int)sizeof(struct igmp_mtrace)) { diff --git a/pimd/pim_register.c b/pimd/pim_register.c index e2538da36f..cc0dace7c2 100644 --- a/pimd/pim_register.c +++ b/pimd/pim_register.c @@ -327,7 +327,7 @@ int pim_register_recv(struct interface *ifp, struct in_addr dest_addr, #define PIM_MSG_REGISTER_BIT_RESERVED_LEN 4 ip_hdr = (struct ip *)(tlv_buf + PIM_MSG_REGISTER_BIT_RESERVED_LEN); - if (!pim_rp_check_is_my_ip_address(pim, dest_addr)) { + if (!if_address_is_local(&dest_addr, AF_INET, pim->vrf->vrf_id)) { if (PIM_DEBUG_PIM_REG) { char dest[INET_ADDRSTRLEN]; diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c index f2a969e04a..1651387384 100644 --- a/pimd/pim_rp.c +++ b/pimd/pim_rp.c @@ -1178,15 +1178,6 @@ int pim_rp_config_write(struct pim_instance *pim, struct vty *vty, return count; } -bool pim_rp_check_is_my_ip_address(struct pim_instance *pim, - struct in_addr dest_addr) -{ - if (if_lookup_exact_address(&dest_addr, AF_INET, pim->vrf->vrf_id)) - return true; - - return false; -} - void pim_rp_show_information(struct pim_instance *pim, struct vty *vty, bool uj) { struct rp_info *rp_info; diff --git a/ripd/rip_snmp.c b/ripd/rip_snmp.c index 824cbd8cf1..436dc4de0e 100644 --- a/ripd/rip_snmp.c +++ b/ripd/rip_snmp.c @@ -257,7 +257,7 @@ static struct interface *rip2IfLookup(struct variable *v, oid name[], oid2in_addr(name + v->namelen, sizeof(struct in_addr), addr); - return if_lookup_exact_address((void *)addr, AF_INET, + return if_lookup_address_local((void *)addr, AF_INET, VRF_DEFAULT); } else { len = *length - v->namelen; diff --git a/staticd/static_routes.c b/staticd/static_routes.c index 60f384e517..45c42ddcef 100644 --- a/staticd/static_routes.c +++ b/staticd/static_routes.c @@ -199,14 +199,14 @@ bool static_add_nexthop_validate(const char *nh_vrf_name, switch (type) { case STATIC_IPV4_GATEWAY: case STATIC_IPV4_GATEWAY_IFNAME: - if (if_lookup_exact_address(&ipaddr->ipaddr_v4, AF_INET, - vrf->vrf_id)) + if (if_address_is_local(&ipaddr->ipaddr_v4, AF_INET, + vrf->vrf_id)) return false; break; case STATIC_IPV6_GATEWAY: case STATIC_IPV6_GATEWAY_IFNAME: - if (if_lookup_exact_address(&ipaddr->ipaddr_v6, AF_INET6, - vrf->vrf_id)) + if (if_address_is_local(&ipaddr->ipaddr_v6, AF_INET6, + vrf->vrf_id)) return false; break; default: diff --git a/staticd/static_zebra.c b/staticd/static_zebra.c index 538fae28aa..38b3c93d76 100644 --- a/staticd/static_zebra.c +++ b/staticd/static_zebra.c @@ -162,14 +162,10 @@ static bool static_nexthop_is_local(vrf_id_t vrfid, struct prefix *addr, int family) { if (family == AF_INET) { - if (if_lookup_exact_address(&addr->u.prefix4, - AF_INET, - vrfid)) + if (if_address_is_local(&addr->u.prefix4, AF_INET, vrfid)) return true; } else if (family == AF_INET6) { - if (if_lookup_exact_address(&addr->u.prefix6, - AF_INET6, - vrfid)) + if (if_address_is_local(&addr->u.prefix6, AF_INET6, vrfid)) return true; } return false;