From af3378d48bc4c8e828fc8e1ed92dda78607f6fbc Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Tue, 13 Sep 2016 15:54:58 -0700 Subject: [PATCH 1/4] RouteSync::onMsg supports ipv6 route --- fpmsyncd/routesync.cpp | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index afe68bc76b6..dd44cee444e 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -26,31 +26,28 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) struct rtnl_route *route_obj = (struct rtnl_route *)obj; struct nl_addr *dip; char ifname[MAX_ADDR_SIZE + 1] = {0}; - uint32_t ipv4; - int prefix; dip = rtnl_route_get_dst(route_obj); - /* Supports IPv4 address only for now */ - if (rtnl_route_get_family(route_obj) != AF_INET) + nl_addr2str(dip, ifname, MAX_ADDR_SIZE); + /* Supports IPv4 or IPv6 address, otherwise return immediately */ + switch (rtnl_route_get_family(route_obj)) { - nl_addr2str(dip, ifname, MAX_ADDR_SIZE); - SWSS_LOG_INFO("%s: Unknown route family support: %s (object: %s)\n", - __FUNCTION__, ifname, nl_object_get_type(obj)); - return; + case AF_INET: + case AF_INET6: + break; + default: + SWSS_LOG_INFO("%s: Unknown route family support: %s (object: %s)\n", + __FUNCTION__, ifname, nl_object_get_type(obj)); + return; } - prefix = nl_addr_get_prefixlen(dip); - ipv4 = *(uint32_t*)nl_addr_get_binary_addr(dip); - IpPrefix destip(ipv4, prefix); - if (nlmsg_type == RTM_DELROUTE) { - m_routeTable.del(destip.to_string()); + m_routeTable.del(ifname); return; } else if (nlmsg_type != RTM_NEWROUTE) { - nl_addr2str(dip, ifname, MAX_ADDR_SIZE); SWSS_LOG_INFO("%s: Unknown message-type: %d for %s\n", __FUNCTION__, nlmsg_type, ifname); return; @@ -63,7 +60,7 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) std::vector fvVector; FieldValueTuple fv("blackhole", "true"); fvVector.push_back(fv); - m_routeTable.set(destip.to_string(), fvVector); + m_routeTable.set(ifname, fvVector); return; } case RTN_UNICAST: @@ -72,7 +69,6 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) case RTN_MULTICAST: case RTN_BROADCAST: case RTN_LOCAL: - nl_addr2str(dip, ifname, MAX_ADDR_SIZE); SWSS_LOG_INFO("%s: BUM routes aren't supported yet (%s)\n", __FUNCTION__, ifname); return; @@ -88,7 +84,6 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) struct nl_list_head *nhs = rtnl_route_get_nexthops(route_obj); if (!nhs) { - nl_addr2str(dip, ifname, MAX_ADDR_SIZE); SWSS_LOG_INFO("%s: Nexthop list is empty for %s\n", __FUNCTION__, ifname); return; @@ -129,5 +124,5 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) FieldValueTuple idx("ifname", ifnames); fvVector.push_back(nh); fvVector.push_back(idx); - m_routeTable.set(destip.to_string(), fvVector); + m_routeTable.set(ifname, fvVector); } From b61750f6efd35f74ad1137626a6e2f6391dd48c5 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Tue, 13 Sep 2016 18:21:08 -0700 Subject: [PATCH 2/4] Fix wrongly variable reuse --- fpmsyncd/routesync.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index dd44cee444e..2544a2183c1 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -25,10 +25,11 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) { struct rtnl_route *route_obj = (struct rtnl_route *)obj; struct nl_addr *dip; - char ifname[MAX_ADDR_SIZE + 1] = {0}; + char destipprefix[MAX_ADDR_SIZE + 1] = {0}; dip = rtnl_route_get_dst(route_obj); - nl_addr2str(dip, ifname, MAX_ADDR_SIZE); + nl_addr2str(dip, destipprefix, MAX_ADDR_SIZE); + SWSS_LOG_DEBUG("destipprefix=%s\n", destipprefix); /* Supports IPv4 or IPv6 address, otherwise return immediately */ switch (rtnl_route_get_family(route_obj)) { @@ -37,19 +38,19 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) break; default: SWSS_LOG_INFO("%s: Unknown route family support: %s (object: %s)\n", - __FUNCTION__, ifname, nl_object_get_type(obj)); + __FUNCTION__, destipprefix, nl_object_get_type(obj)); return; } if (nlmsg_type == RTM_DELROUTE) { - m_routeTable.del(ifname); + m_routeTable.del(destipprefix); return; } else if (nlmsg_type != RTM_NEWROUTE) { SWSS_LOG_INFO("%s: Unknown message-type: %d for %s\n", - __FUNCTION__, nlmsg_type, ifname); + __FUNCTION__, nlmsg_type, destipprefix); return; } @@ -60,7 +61,7 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) std::vector fvVector; FieldValueTuple fv("blackhole", "true"); fvVector.push_back(fv); - m_routeTable.set(ifname, fvVector); + m_routeTable.set(destipprefix, fvVector); return; } case RTN_UNICAST: @@ -70,7 +71,7 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) case RTN_BROADCAST: case RTN_LOCAL: SWSS_LOG_INFO("%s: BUM routes aren't supported yet (%s)\n", - __FUNCTION__, ifname); + __FUNCTION__, destipprefix); return; default: @@ -85,10 +86,11 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) if (!nhs) { SWSS_LOG_INFO("%s: Nexthop list is empty for %s\n", - __FUNCTION__, ifname); + __FUNCTION__, destipprefix); return; } + char ifname[IFNAMSIZ] = {0}; for (int i = 0; i < rtnl_route_get_nnexthops(route_obj); i++) { struct rtnl_nexthop *nexthop = rtnl_route_nexthop_n(route_obj, i); @@ -101,12 +103,13 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) nexthops += nh.to_string(); } - rtnl_link_i2name(m_link_cache, ifindex, ifname, MAX_ADDR_SIZE); + rtnl_link_i2name(m_link_cache, ifindex, ifname, IFNAMSIZ); + SWSS_LOG_DEBUG("ifname=%s\n", ifname); /* Cannot get ifname. Possibly interfaces get re-created. */ if (!strlen(ifname)) { rtnl_link_alloc_cache(m_nl_sock, AF_UNSPEC, &m_link_cache); - rtnl_link_i2name(m_link_cache, ifindex, ifname, MAX_ADDR_SIZE); + rtnl_link_i2name(m_link_cache, ifindex, ifname, IFNAMSIZ); if (!strlen(ifname)) strcpy(ifname, "unknown"); } @@ -124,5 +127,5 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) FieldValueTuple idx("ifname", ifnames); fvVector.push_back(nh); fvVector.push_back(idx); - m_routeTable.set(ifname, fvVector); + m_routeTable.set(destipprefix, fvVector); } From bbcd573d0afd02cc39c5aa64bb91d43d9424603f Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Tue, 13 Sep 2016 22:39:04 -0700 Subject: [PATCH 3/4] Refactor --- fpmsyncd/routesync.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 2544a2183c1..b7cdd58028b 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -29,17 +29,14 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) dip = rtnl_route_get_dst(route_obj); nl_addr2str(dip, destipprefix, MAX_ADDR_SIZE); - SWSS_LOG_DEBUG("destipprefix=%s\n", destipprefix); + SWSS_LOG_DEBUG("Receive new route message dest ip prefix: %s\n", destipprefix); /* Supports IPv4 or IPv6 address, otherwise return immediately */ - switch (rtnl_route_get_family(route_obj)) + auto family = rtnl_route_get_family(route_obj); + if (family != AF_INET && family != AF_INET6) { - case AF_INET: - case AF_INET6: - break; - default: - SWSS_LOG_INFO("%s: Unknown route family support: %s (object: %s)\n", - __FUNCTION__, destipprefix, nl_object_get_type(obj)); - return; + SWSS_LOG_INFO("%s: Unknown route family support: %s (object: %s)\n", + __FUNCTION__, destipprefix, nl_object_get_type(obj)); + return; } if (nlmsg_type == RTM_DELROUTE) @@ -104,7 +101,6 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) } rtnl_link_i2name(m_link_cache, ifindex, ifname, IFNAMSIZ); - SWSS_LOG_DEBUG("ifname=%s\n", ifname); /* Cannot get ifname. Possibly interfaces get re-created. */ if (!strlen(ifname)) { From 9bb3896433234fb085f71b4463326f9c7efa5320 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Tue, 13 Sep 2016 22:49:22 -0700 Subject: [PATCH 4/4] Fix SWSS_LOG_INFO arguments --- fpmsyncd/routesync.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index b7cdd58028b..88014b87014 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -34,8 +34,7 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) auto family = rtnl_route_get_family(route_obj); if (family != AF_INET && family != AF_INET6) { - SWSS_LOG_INFO("%s: Unknown route family support: %s (object: %s)\n", - __FUNCTION__, destipprefix, nl_object_get_type(obj)); + SWSS_LOG_INFO("Unknown route family support: %s (object: %s)\n", destipprefix, nl_object_get_type(obj)); return; } @@ -46,8 +45,7 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) } else if (nlmsg_type != RTM_NEWROUTE) { - SWSS_LOG_INFO("%s: Unknown message-type: %d for %s\n", - __FUNCTION__, nlmsg_type, destipprefix); + SWSS_LOG_INFO("Unknown message-type: %d for %s\n", nlmsg_type, destipprefix); return; } @@ -67,8 +65,7 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) case RTN_MULTICAST: case RTN_BROADCAST: case RTN_LOCAL: - SWSS_LOG_INFO("%s: BUM routes aren't supported yet (%s)\n", - __FUNCTION__, destipprefix); + SWSS_LOG_INFO("BUM routes aren't supported yet (%s)\n", destipprefix); return; default: @@ -82,8 +79,7 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) struct nl_list_head *nhs = rtnl_route_get_nexthops(route_obj); if (!nhs) { - SWSS_LOG_INFO("%s: Nexthop list is empty for %s\n", - __FUNCTION__, destipprefix); + SWSS_LOG_INFO("Nexthop list is empty for %s\n", destipprefix); return; }