From 496c870bac08fd14ae241d52b818d963d1e4eaa5 Mon Sep 17 00:00:00 2001 From: Nathan Bahr Date: Wed, 19 Aug 2020 14:26:41 -0500 Subject: [PATCH 1/3] pimd: fix IGMP receive handling IGMP packets received from a source that does not match the subnet of any configured addresses on the receive interface should be ignored. Also, find and use the correct IGMP socket object for the received IGMP packet. Signed-off-by: Nathan Bahr Signed-off-by: Jafar Al-Gharaibeh --- pimd/pim_iface.c | 8 ++++---- pimd/pim_iface.h | 2 +- pimd/pim_mroute.c | 50 ++++++++++++++++++++++------------------------- 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c index b25b6eaa8c..a49892389e 100644 --- a/pimd/pim_iface.c +++ b/pimd/pim_iface.c @@ -1503,14 +1503,14 @@ void pim_if_create_pimreg(struct pim_instance *pim) } } -int pim_if_connected_to_source(struct interface *ifp, struct in_addr src) +struct prefix *pim_if_connected_to_source(struct interface *ifp, struct in_addr src) { struct listnode *cnode; struct connected *c; struct prefix p; if (!ifp) - return 0; + return NULL; p.family = AF_INET; p.u.prefix4 = src; @@ -1519,11 +1519,11 @@ int pim_if_connected_to_source(struct interface *ifp, struct in_addr src) for (ALL_LIST_ELEMENTS_RO(ifp->connected, cnode, c)) { if ((c->address->family == AF_INET) && prefix_match(CONNECTED_PREFIX(c), &p)) { - return 1; + return CONNECTED_PREFIX(c); } } - return 0; + return NULL; } bool pim_if_is_vrf_device(struct interface *ifp) diff --git a/pimd/pim_iface.h b/pimd/pim_iface.h index 570bf5eac3..dae712d955 100644 --- a/pimd/pim_iface.h +++ b/pimd/pim_iface.h @@ -222,7 +222,7 @@ void pim_if_update_assert_tracking_desired(struct interface *ifp); void pim_if_create_pimreg(struct pim_instance *pim); -int pim_if_connected_to_source(struct interface *ifp, struct in_addr src); +struct prefix *pim_if_connected_to_source(struct interface *ifp, struct in_addr src); int pim_update_source_set(struct interface *ifp, struct in_addr source); bool pim_if_is_vrf_device(struct interface *ifp); diff --git a/pimd/pim_mroute.c b/pimd/pim_mroute.c index 9060b6a95a..0bccba397b 100644 --- a/pimd/pim_mroute.c +++ b/pimd/pim_mroute.c @@ -592,12 +592,9 @@ static int pim_mroute_msg(struct pim_instance *pim, const char *buf, struct pim_interface *pim_ifp; const struct ip *ip_hdr; const struct igmpmsg *msg; - char ip_src_str[INET_ADDRSTRLEN] = ""; - char ip_dst_str[INET_ADDRSTRLEN] = ""; - char src_str[INET_ADDRSTRLEN] = ""; - char grp_str[INET_ADDRSTRLEN] = ""; struct in_addr ifaddr; struct igmp_sock *igmp; + const struct prefix *connected_src; if (buf_size < (int)sizeof(struct ip)) return 0; @@ -617,34 +614,37 @@ static int pim_mroute_msg(struct pim_instance *pim, const char *buf, if (!ifp || !ifp->info) return 0; + connected_src = pim_if_connected_to_source(ifp, ip_hdr->ip_src); + + if (!connected_src) { + if (PIM_DEBUG_IGMP_PACKETS) { + zlog_debug("Recv IGMP packet on interface: %s from a non-connected source: %pI4", + ifp->name, &ip_hdr->ip_src); + } + return 0; + } + pim_ifp = ifp->info; - ifaddr = pim_find_primary_addr(ifp); - igmp = pim_igmp_sock_lookup_ifaddr(pim_ifp->igmp_socket_list, - ifaddr); + ifaddr = connected_src->u.prefix4; + igmp = pim_igmp_sock_lookup_ifaddr(pim_ifp->igmp_socket_list, ifaddr); if (PIM_DEBUG_MROUTE) { - pim_inet4_dump("", ip_hdr->ip_src, ip_src_str, - sizeof(ip_src_str)); - pim_inet4_dump("", ip_hdr->ip_dst, ip_dst_str, - sizeof(ip_dst_str)); - zlog_debug( - "%s(%s): igmp kernel upcall on %s(%p) for %s -> %s", + "%s(%s): igmp kernel upcall on %s(%p) for %pI4 -> %pI4", __func__, pim->vrf->name, ifp->name, igmp, - ip_src_str, ip_dst_str); + &ip_hdr->ip_src, &ip_hdr->ip_dst); } if (igmp) pim_igmp_packet(igmp, (char *)buf, buf_size); - + else if (PIM_DEBUG_IGMP_PACKETS) { + zlog_debug("No IGMP socket on interface: %s with connected source: %pFX", + ifp->name, connected_src); + } } else if (ip_hdr->ip_p) { if (PIM_DEBUG_MROUTE_DETAIL) { - pim_inet4_dump("", ip_hdr->ip_src, src_str, - sizeof(src_str)); - pim_inet4_dump("", ip_hdr->ip_dst, grp_str, - sizeof(grp_str)); zlog_debug( - "%s: no kernel upcall proto=%d src: %s dst: %s msg_size=%d", - __func__, ip_hdr->ip_p, src_str, grp_str, + "%s: no kernel upcall proto=%d src: %pI4 dst: %pI4 msg_size=%d", + __func__, ip_hdr->ip_p, &ip_hdr->ip_src, &ip_hdr->ip_dst, buf_size); } @@ -656,15 +656,11 @@ static int pim_mroute_msg(struct pim_instance *pim, const char *buf, if (!ifp) return 0; if (PIM_DEBUG_MROUTE) { - pim_inet4_dump("", msg->im_src, src_str, - sizeof(src_str)); - pim_inet4_dump("", msg->im_dst, grp_str, - sizeof(grp_str)); zlog_debug( - "%s: pim kernel upcall %s type=%d ip_p=%d from fd=%d for (S,G)=(%s,%s) on %s vifi=%d size=%d", + "%s: pim kernel upcall %s type=%d ip_p=%d from fd=%d for (S,G)=(%pI4,%pI4) on %s vifi=%d size=%d", __func__, igmpmsgtype2str[msg->im_msgtype], msg->im_msgtype, ip_hdr->ip_p, - pim->mroute_socket, src_str, grp_str, ifp->name, + pim->mroute_socket, &msg->im_src, &msg->im_dst, ifp->name, msg->im_vif, buf_size); } From e0d28b66f11f337d41fcc94a0a3009b8882d9bc7 Mon Sep 17 00:00:00 2001 From: Nathan Bahr Date: Wed, 19 Aug 2020 14:42:07 -0500 Subject: [PATCH 2/3] pimd: fix IGMP source address on transmit IGMP queries should contain the source address of the IGMP socket they are being sent from. Added binding the IGMP sockets to their specific source, otherwise interfaces with multiple addresses will send multiple queries using the same source, which is determined by the kernel. Signed-off-by: Nathan Bahr Reviewed-by: Jafar Al-Gharaibeh --- pimd/pim_igmp.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pimd/pim_igmp.c b/pimd/pim_igmp.c index 851b00b2ac..62954400f1 100644 --- a/pimd/pim_igmp.c +++ b/pimd/pim_igmp.c @@ -997,6 +997,7 @@ struct igmp_sock *pim_igmp_sock_add(struct list *igmp_sock_list, { struct pim_interface *pim_ifp; struct igmp_sock *igmp; + struct sockaddr_in sin; int fd; pim_ifp = ifp->info; @@ -1008,6 +1009,15 @@ struct igmp_sock *pim_igmp_sock_add(struct list *igmp_sock_list, return 0; } + sin.sin_family = AF_INET; + sin.sin_addr = ifaddr; + sin.sin_port = 0; + if (bind(fd, (struct sockaddr *) &sin, sizeof(sin)) != 0) { + zlog_warn("Could not bind IGMP socket for %s on %s", + inet_ntoa(ifaddr), ifp->name); + return 0; + } + igmp = igmp_sock_new(fd, ifaddr, ifp, mtrace_only); igmp_read_on(igmp); From 6e840890054febdac210c0bc658b7da37da061b2 Mon Sep 17 00:00:00 2001 From: Nathan Bahr Date: Mon, 24 Aug 2020 13:52:51 -0500 Subject: [PATCH 3/3] pimd: fix IGMP querier election Match by exact address rather than by prefix match to determine if we generated the IGMPP query. Othwerwise we will be ignoring IGMP queries coming from other hosts on the same subnet. Signed-off-by: Nathan Bahr Reviewed-by: Jafar Al-Gharaibeh --- pimd/pim_igmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pimd/pim_igmp.c b/pimd/pim_igmp.c index 62954400f1..7b882ed98c 100644 --- a/pimd/pim_igmp.c +++ b/pimd/pim_igmp.c @@ -310,7 +310,7 @@ static int igmp_recv_query(struct igmp_sock *igmp, int query_version, return 0; } - if (if_lookup_address(&from, AF_INET, ifp->vrf_id)) { + if (if_lookup_exact_address(&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);