From 48f6dc2dae2572864db373ddc556518c2ab2ad85 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Fri, 6 Jan 2017 16:44:49 -0800 Subject: [PATCH 1/6] Fix PIM DBG message Ticket: CM-13771 Reviewed By: CCR-5537 Testing Done: yes Fix to CM-13771 where DBG message was out of order. Signed-off-by: Chirag Shah --- pimd/pim_join.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pimd/pim_join.c b/pimd/pim_join.c index 028f77f532..cedce8165c 100644 --- a/pimd/pim_join.c +++ b/pimd/pim_join.c @@ -325,14 +325,6 @@ int pim_joinprune_send(struct interface *ifp, return -1; } - if (PIM_DEBUG_PIM_J_P) { - char dst_str[INET_ADDRSTRLEN]; - pim_inet4_dump("", upstream_addr, dst_str, sizeof(dst_str)); - zlog_debug("%s: sending %s(S,G)=%s to upstream=%s on interface %s", - __PRETTY_FUNCTION__, - send_join ? "Join" : "Prune", - up->sg_str, dst_str, ifp->name); - } if (PIM_INADDR_IS_ANY(upstream_addr)) { if (PIM_DEBUG_PIM_J_P) { @@ -366,6 +358,15 @@ int pim_joinprune_send(struct interface *ifp, if (pim_msg_size < 0) return pim_msg_size; + if (PIM_DEBUG_PIM_J_P) { + char dst_str[INET_ADDRSTRLEN]; + pim_inet4_dump("", upstream_addr, dst_str, sizeof(dst_str)); + zlog_debug("%s: sending %s(S,G)=%s to upstream=%s on interface %s", + __PRETTY_FUNCTION__, + send_join ? "Join" : "Prune", + up->sg_str, dst_str, ifp->name); + } + if (pim_msg_send(pim_ifp->pim_sock_fd, pim_ifp->primary_address, qpim_all_pim_routers_addr, From 5637da0501faa600273ecd7a2ed39cdbbe4531b1 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Wed, 11 Jan 2017 19:36:50 -0800 Subject: [PATCH 2/6] pimd: non-null register checksum incorrect Ticket: CM-12041 Reviewed By: sharpd, CCR-5556 Testing Done: Tested on Local setup generating PIM Register (Data/Null) and processing both Tx/Rx with correct checksum. Provided quagga debian to submitter and checksum cases passed on submitter setup. 1. PIM Register msg checksum only accounts for 8 bytes (4 bytes for PIM header and next 4 byetes before data payload). In PIM header checksum calculation checked PIM packet type (in this case REGISTER type) then only pass 8 bytes as length rather than full packet length. 2. PIM Register Rx path also handled with 8 byte and full pim lenth checksum. Signed-off-by: Chirag Shah --- pimd/pim_msg.c | 13 +++++++++++-- pimd/pim_pim.c | 38 +++++++++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/pimd/pim_msg.c b/pimd/pim_msg.c index 7ea7b1ad82..ab14b3ae5f 100644 --- a/pimd/pim_msg.c +++ b/pimd/pim_msg.c @@ -35,6 +35,7 @@ #include "pim_iface.h" #include "pim_rp.h" #include "pim_rpf.h" +#include "pim_register.h" void pim_msg_build_header(uint8_t *pim_msg, int pim_msg_size, uint8_t pim_msg_type) @@ -54,8 +55,16 @@ void pim_msg_build_header(uint8_t *pim_msg, int pim_msg_size, * Compute checksum */ - *(uint16_t *) PIM_MSG_HDR_OFFSET_CHECKSUM(pim_msg) = 0; - checksum = in_cksum(pim_msg, pim_msg_size); + *(uint16_t *) PIM_MSG_HDR_OFFSET_CHECKSUM (pim_msg) = 0; + /* + * The checksum for Registers is done only on the first 8 bytes of the packet, + * including the PIM header and the next 4 bytes, excluding the data packet portion + */ + if (pim_msg_type == PIM_MSG_TYPE_REGISTER) + checksum = in_cksum (pim_msg, PIM_MSG_REGISTER_LEN); + else + checksum = in_cksum (pim_msg, pim_msg_size); + *(uint16_t *) PIM_MSG_HDR_OFFSET_CHECKSUM(pim_msg) = checksum; } diff --git a/pimd/pim_pim.c b/pimd/pim_pim.c index f727d3e627..840004c375 100644 --- a/pimd/pim_pim.c +++ b/pimd/pim_pim.c @@ -197,13 +197,37 @@ int pim_pim_packet(struct interface *ifp, uint8_t *buf, size_t len) /* for computing checksum */ *(uint16_t *) PIM_MSG_HDR_OFFSET_CHECKSUM(pim_msg) = 0; - checksum = in_cksum(pim_msg, pim_msg_len); - if (checksum != pim_checksum) { - if (PIM_DEBUG_PIM_PACKETS) - zlog_debug("Ignoring PIM pkt from %s with invalid checksum: received=%x calculated=%x", - ifp->name, pim_checksum, checksum); - return -1; - } + if (pim_type == PIM_MSG_TYPE_REGISTER) + { + /* First 8 byte header checksum */ + checksum = in_cksum (pim_msg, PIM_MSG_REGISTER_LEN); + if (checksum != pim_checksum) + { + checksum = in_cksum (pim_msg, pim_msg_len); + if (checksum != pim_checksum) + { + if (PIM_DEBUG_PIM_PACKETS) + zlog_debug + ("Ignoring PIM pkt from %s with invalid checksum: received=%x calculated=%x", + ifp->name, pim_checksum, checksum); + + return -1; + } + } + } + else + { + checksum = in_cksum (pim_msg, pim_msg_len); + if (checksum != pim_checksum) + { + if (PIM_DEBUG_PIM_PACKETS) + zlog_debug + ("Ignoring PIM pkt from %s with invalid checksum: received=%x calculated=%x", + ifp->name, pim_checksum, checksum); + + return -1; + } + } if (PIM_DEBUG_PIM_PACKETS) { pim_inet4_dump("", ip_hdr->ip_src, src_str, sizeof(src_str)); From d62c5c03816d8dc525424b04c6b4b3c3ad23a29c Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Thu, 12 Jan 2017 17:15:44 -0800 Subject: [PATCH 3/6] pimd: messages to neighbors should have TTL = 1 Ticket:CM-12924 Reviewed By:shapd Testing Done: configure PIM neighbor, verify PIM hello packet dump for ttl to be 1. Set TTL to 1 for outgoing multicast control packets destine to ALL-PIM-ROUTERS as oppose to unicast mcast packets. Signed-off-by: Chirag Shah --- pimd/pim_pim.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/pimd/pim_pim.c b/pimd/pim_pim.c index 840004c375..c96cc187c7 100644 --- a/pimd/pim_pim.c +++ b/pimd/pim_pim.c @@ -557,6 +557,8 @@ pim_msg_send(int fd, struct in_addr src, socklen_t tolen; unsigned char buffer[10000]; unsigned char *msg_start; + uint8_t ttl = MAXTTL; + enum pim_msg_type pim_type = PIM_MSG_TYPE_HELLO; struct ip *ip; memset (buffer, 0, 10000); @@ -565,14 +567,36 @@ pim_msg_send(int fd, struct in_addr src, msg_start = buffer + sizeof (struct ip); memcpy (msg_start, pim_msg, pim_msg_size); - ip = (struct ip *)buffer; + /* TTL for packets destine to ALL-PIM-ROUTERS is 1 */ + pim_type = PIM_MSG_HDR_GET_TYPE (pim_msg); + switch (pim_type) + { + case PIM_MSG_TYPE_HELLO: + case PIM_MSG_TYPE_JOIN_PRUNE: + case PIM_MSG_TYPE_BOOTSTRAP: + case PIM_MSG_TYPE_ASSERT: + ttl = 1; + break; + case PIM_MSG_TYPE_REGISTER: + case PIM_MSG_TYPE_REG_STOP: + case PIM_MSG_TYPE_GRAFT: + case PIM_MSG_TYPE_GRAFT_ACK: + case PIM_MSG_TYPE_CANDIDATE: + ttl = IPDEFTTL; + break; + default: + ttl = MAXTTL; + break; + } + + ip = (struct ip *) buffer; ip->ip_id = htons (++ip_id); ip->ip_hl = 5; ip->ip_v = 4; ip->ip_p = PIM_IP_PROTO_PIM; ip->ip_src = src; ip->ip_dst = dst; - ip->ip_ttl = MAXTTL; + ip->ip_ttl = ttl; ip->ip_len = htons (sendlen); if (PIM_DEBUG_PIM_PACKETS) { From 429a291bf00f79ef0ffe80c1371acdc9e32921ee Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Sat, 21 Jan 2017 15:18:08 -0800 Subject: [PATCH 4/6] pimd: mroute entries unresolved IIF issue Ticket:CM-14056 Reviewed By:sharpd, CCR-5603 Testing Done: verified multiple ifdown/ifup event on submitter setup and dev setup with 2k s,g entries, ran pim-smoke. 1. during ifdown event, pim vif for bridge was not resetting vif_index to -1 due to errno received from kernel during vif del sequence. It could be timing issue where kernel may have delete prior to pimd sending request. For vif_del even kernel returns error, reset vif_index to -1 in pimd DB so next if up event VIF receives new vif_index and reprograms in kernel. 2. during mroute del sequence reset mfcc_parent to MAXVIF. 3. during mroute add check if parent mfcc_parent is MAXVIF then do not download to kernel such mroute entry. Signed-off-by: Chirag Shah --- pimd/pim_iface.c | 5 +---- pimd/pim_mroute.c | 25 +++++++++++++++++++++++++ pimd/pim_zlookup.c | 3 ++- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c index 7f64c9d5f2..c1289a3ba7 100644 --- a/pimd/pim_iface.c +++ b/pimd/pim_iface.c @@ -945,10 +945,7 @@ int pim_if_del_vif(struct interface *ifp) return -1; } - if (pim_mroute_del_vif(pim_ifp->mroute_vif_index)) { - /* pim_mroute_del_vif reported error */ - return -2; - } + pim_mroute_del_vif(pim_ifp->mroute_vif_index); /* Update vif_index diff --git a/pimd/pim_mroute.c b/pimd/pim_mroute.c index b1cdaf9b95..c2f06ab57b 100644 --- a/pimd/pim_mroute.c +++ b/pimd/pim_mroute.c @@ -763,6 +763,17 @@ int pim_mroute_add(struct channel_oil *c_oil, const char *name) __PRETTY_FUNCTION__); return -1; } + /* Do not install route if incoming interface is undefined. */ + if (c_oil->oil.mfcc_parent == MAXVIFS) + { + if (PIM_DEBUG_MROUTE) + { + char buf[1000]; + zlog_debug("%s(%s) %s Attempting to add vifi that is invalid to mroute table", + __PRETTY_FUNCTION__, name, pim_channel_oil_dump (c_oil, buf, sizeof(buf))); + } + return -2; + } /* The linux kernel *expects* the incoming * vif to be part of the outgoing list @@ -834,6 +845,18 @@ int pim_mroute_del (struct channel_oil *c_oil, const char *name) return -1; } + if (!c_oil->installed) + { + if (PIM_DEBUG_MROUTE) + { + char buf[1000]; + zlog_debug("%s %s: vifi %d for route is %s not installed, do not need to send del req. ", + __FILE__, __PRETTY_FUNCTION__, c_oil->oil.mfcc_parent, + pim_channel_oil_dump (c_oil, buf, sizeof(buf))); + } + return -2; + } + err = setsockopt(qpim_mroute_socket_fd, IPPROTO_IP, MRT_DEL_MFC, &c_oil->oil, sizeof(c_oil->oil)); if (err) { if (PIM_DEBUG_MROUTE) @@ -852,7 +875,9 @@ int pim_mroute_del (struct channel_oil *c_oil, const char *name) pim_channel_oil_dump (c_oil, buf, sizeof(buf))); } + /*reset incoming vifi and kernel installed flags*/ c_oil->installed = 0; + c_oil->oil.mfcc_parent = MAXVIFS; return 0; } diff --git a/pimd/pim_zlookup.c b/pimd/pim_zlookup.c index e1a8e820b7..7ec31c7d50 100644 --- a/pimd/pim_zlookup.c +++ b/pimd/pim_zlookup.c @@ -455,7 +455,8 @@ pim_zlookup_sg_statistics (struct channel_oil *c_oil) more.src = c_oil->oil.mfcc_origin; more.grp = c_oil->oil.mfcc_mcastgrp; - zlog_debug ("Sending Request for New Channel Oil Information(%s)", pim_str_sg_dump (&more)); + zlog_debug ("Sending Request for New Channel Oil Information(%s) VIIF %d", + pim_str_sg_dump (&more), c_oil->oil.mfcc_parent); } if (!ifp) From e81d9709ff979502ff7b28ea622d04407068e0c2 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Fri, 27 Jan 2017 11:33:01 -0800 Subject: [PATCH 5/6] pimd: ifdown sequnce stale report entry Ticket: CM-14652 Testing Done: Tested via sending IGMP report and flap port and verified pim upstream and mroute, the entry is deleted. Run pim-smoke Even after Report received port down event, IGMP entry alawys exists in upstream, mroute, kernel. The entry exist because it was recreated after delete due missing check if group has no more source list, mode is exclude, last source address was * means (*, G) so do not trigger to create entry. Signed-off-by: Chirag Shah Reviewed-by: Donald Sharp --- pimd/pim_igmpv3.c | 12 ++++++++---- pimd/pim_mroute.c | 7 +++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pimd/pim_igmpv3.c b/pimd/pim_igmpv3.c index f7a6cbd0ce..fb65665135 100644 --- a/pimd/pim_igmpv3.c +++ b/pimd/pim_igmpv3.c @@ -389,10 +389,14 @@ void igmp_source_delete(struct igmp_source *source) listnode_delete(group->group_source_list, source); igmp_source_free(source); - - if (group->group_filtermode_isexcl) { - group_exclude_fwd_anysrc_ifempty(group); - } + /* Group source list is empty and current source is * then + *,G group going away so do not trigger start */ + if (group->group_filtermode_isexcl && + (listcount (group->group_source_list) != 0) && + source->source_addr.s_addr != INADDR_ANY) + { + group_exclude_fwd_anysrc_ifempty (group); + } } static void source_delete_by_flag(struct list *source_list) diff --git a/pimd/pim_mroute.c b/pimd/pim_mroute.c index c2f06ab57b..4fae5b3ca5 100644 --- a/pimd/pim_mroute.c +++ b/pimd/pim_mroute.c @@ -734,6 +734,13 @@ int pim_mroute_del_vif(int vif_index) return -1; } + if (PIM_DEBUG_MROUTE) + { + struct interface *ifp = pim_if_find_by_vif_index (vif_index); + zlog_debug ("%s %s: Del Vif %d (%s) ", __FILE__, + __PRETTY_FUNCTION__, vif_index, ifp ? ifp->name : "NULL"); + } + memset(&vc, 0, sizeof(vc)); vc.vifc_vifi = vif_index; From 05ea5ceecf05f0b16dc076d047c87b97702ce956 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 14 Feb 2017 16:19:16 -0500 Subject: [PATCH 6/6] pimd: Allow IPDEFTTL to be used for omnios The omnios OS has no IPDEFTTL defined. Add the ability to handle it for this one case. Signed-off-by: Donald Sharp --- pimd/pim_pim.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pimd/pim_pim.c b/pimd/pim_pim.c index c96cc187c7..e4c654557a 100644 --- a/pimd/pim_pim.c +++ b/pimd/pim_pim.c @@ -567,6 +567,13 @@ pim_msg_send(int fd, struct in_addr src, msg_start = buffer + sizeof (struct ip); memcpy (msg_start, pim_msg, pim_msg_size); + /* + * Omnios apparently doesn't have a #define for IP default + * ttl that is the same as all other platforms. + */ +#ifndef IPDEFTTL +#define IPDEFTTL 64 +#endif /* TTL for packets destine to ALL-PIM-ROUTERS is 1 */ pim_type = PIM_MSG_HDR_GET_TYPE (pim_msg); switch (pim_type)