Merge pull request #4322 from sworleys/Nexthop-Cmp

lib: Add nexthop_cmp

bgpd/bgp_nht.c

@@ -474,8 +474,7 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 				continue;
 
 			for (oldnh = bnc->nexthop; oldnh; oldnh = oldnh->next)
-				if (nexthop_same_no_recurse(oldnh, nexthop) &&
+				if (nexthop_same(oldnh, nexthop))
-				    nexthop_labels_match(oldnh, nexthop))
 					break;
 
 			if (!oldnh)

lib/nexthop.c

@@ -36,40 +36,132 @@
 DEFINE_MTYPE_STATIC(LIB, NEXTHOP, "Nexthop")
 DEFINE_MTYPE_STATIC(LIB, NH_LABEL, "Nexthop label")
 
-/* check if nexthops are same, non-recursive */
+static int _nexthop_labels_cmp(const struct nexthop *nh1,
-int nexthop_same_no_recurse(const struct nexthop *next1,
+			       const struct nexthop *nh2)
-			    const struct nexthop *next2)
 {
-	if (next1->type != next2->type)
+	const struct mpls_label_stack *nhl1 = NULL;
+	const struct mpls_label_stack *nhl2 = NULL;
+
+	nhl1 = nh1->nh_label;
+	nhl2 = nh2->nh_label;
+
+	/* No labels is a match */
+	if (!nhl1 && !nhl2)
 		return 0;
 
+	if (nhl1 && !nhl2)
+		return 1;
+
+	if (nhl2 && !nhl1)
+		return -1;
+
+	if (nhl1->num_labels > nhl2->num_labels)
+		return 1;
+
+	if (nhl1->num_labels < nhl2->num_labels)
+		return -1;
+
+	return memcmp(nhl1->label, nhl2->label, nhl1->num_labels);
+}
+
+static int _nexthop_g_addr_cmp(enum nexthop_types_t type,
+			       const union g_addr *addr1,
+			       const union g_addr *addr2)
+{
+	int ret = 0;
+
+	switch (type) {
+	case NEXTHOP_TYPE_IPV4:
+	case NEXTHOP_TYPE_IPV4_IFINDEX:
+		ret = IPV4_ADDR_CMP(&addr1->ipv4, &addr2->ipv4);
+		break;
+	case NEXTHOP_TYPE_IPV6:
+	case NEXTHOP_TYPE_IPV6_IFINDEX:
+		ret = IPV6_ADDR_CMP(&addr1->ipv6, &addr2->ipv6);
+		break;
+	case NEXTHOP_TYPE_IFINDEX:
+	case NEXTHOP_TYPE_BLACKHOLE:
+		/* No addr here */
+		break;
+	}
+
+	return ret;
+}
+
+static int _nexthop_gateway_cmp(const struct nexthop *nh1,
+				const struct nexthop *nh2)
+{
+	return _nexthop_g_addr_cmp(nh1->type, &nh1->gate, &nh2->gate);
+}
+
+static int _nexthop_source_cmp(const struct nexthop *nh1,
+			       const struct nexthop *nh2)
+{
+	return _nexthop_g_addr_cmp(nh1->type, &nh1->src, &nh2->src);
+}
+
+static int _nexthop_cmp_no_labels(const struct nexthop *next1,
+				  const struct nexthop *next2)
+{
+	int ret = 0;
+
+	if (next1->vrf_id < next2->vrf_id)
+		return -1;
+
+	if (next1->vrf_id > next2->vrf_id)
+		return 1;
+
+	if (next1->type < next2->type)
+		return -1;
+
+	if (next1->type > next2->type)
+		return 1;
+
 	switch (next1->type) {
 	case NEXTHOP_TYPE_IPV4:
-	case NEXTHOP_TYPE_IPV4_IFINDEX:
-		if (!IPV4_ADDR_SAME(&next1->gate.ipv4, &next2->gate.ipv4))
-			return 0;
-		if (next1->ifindex && (next1->ifindex != next2->ifindex))
-			return 0;
-		break;
-	case NEXTHOP_TYPE_IFINDEX:
-		if (next1->ifindex != next2->ifindex)
-			return 0;
-		break;
 	case NEXTHOP_TYPE_IPV6:
-		if (!IPV6_ADDR_SAME(&next1->gate.ipv6, &next2->gate.ipv6))
+		ret = _nexthop_gateway_cmp(next1, next2);
-			return 0;
+		if (ret != 0)
+			return ret;
 		break;
+	case NEXTHOP_TYPE_IPV4_IFINDEX:
 	case NEXTHOP_TYPE_IPV6_IFINDEX:
-		if (!IPV6_ADDR_SAME(&next1->gate.ipv6, &next2->gate.ipv6))
+		ret = _nexthop_gateway_cmp(next1, next2);
-			return 0;
+		if (ret != 0)
-		if (next1->ifindex != next2->ifindex)
+			return ret;
-			return 0;
+		/* Intentional Fall-Through */
+	case NEXTHOP_TYPE_IFINDEX:
+		if (next1->ifindex < next2->ifindex)
+			return -1;
+
+		if (next1->ifindex > next2->ifindex)
+			return 1;
 		break;
-	default:
+	case NEXTHOP_TYPE_BLACKHOLE:
-		/* do nothing */
+		if (next1->bh_type < next2->bh_type)
+			return -1;
+
+		if (next1->bh_type > next2->bh_type)
+			return 1;
 		break;
 	}
-	return 1;
+
+	ret = _nexthop_source_cmp(next1, next2);
+
+	return ret;
+}
+
+int nexthop_cmp(const struct nexthop *next1, const struct nexthop *next2)
+{
+	int ret = 0;
+
+	ret = _nexthop_cmp_no_labels(next1, next2);
+	if (ret != 0)
+		return ret;
+
+	ret = _nexthop_labels_cmp(next1, next2);
+
+	return ret;
 }
 
 int nexthop_same_firsthop(struct nexthop *next1, struct nexthop *next2)
@@ -121,27 +213,12 @@ const char *nexthop_type_to_str(enum nexthop_types_t nh_type)
 /*
  * Check if the labels match for the 2 nexthops specified.
  */
-int nexthop_labels_match(const struct nexthop *nh1, const struct nexthop *nh2)
+bool nexthop_labels_match(const struct nexthop *nh1, const struct nexthop *nh2)
 {
-	const struct mpls_label_stack *nhl1, *nhl2;
+	if (_nexthop_labels_cmp(nh1, nh2) != 0)
+		return false;
 
-	nhl1 = nh1->nh_label;
+	return true;
-	nhl2 = nh2->nh_label;
-
-	/* No labels is a match */
-	if (!nhl1 && !nhl2)
-		return 1;
-
-	if (!nhl1 || !nhl2)
-		return 0;
-
-	if (nhl1->num_labels != nhl2->num_labels)
-		return 0;
-
-	if (memcmp(nhl1->label, nhl2->label, nhl1->num_labels))
-		return 0;
-
-	return 1;
 }
 
 struct nexthop *nexthop_new(void)
@@ -180,45 +257,28 @@ bool nexthop_same(const struct nexthop *nh1, const struct nexthop *nh2)
 	if (nh1 == nh2)
 		return true;
 
-	if (nh1->vrf_id != nh2->vrf_id)
+	if (nexthop_cmp(nh1, nh2) != 0)
 		return false;
 
-	if (nh1->type != nh2->type)
+	return true;
+}
+
+bool nexthop_same_no_labels(const struct nexthop *nh1,
+			    const struct nexthop *nh2)
+{
+	if (nh1 && !nh2)
 		return false;
 
-	switch (nh1->type) {
+	if (!nh1 && nh2)
-	case NEXTHOP_TYPE_IFINDEX:
+		return false;
-		if (nh1->ifindex != nh2->ifindex)
-			return false;
-		break;
-	case NEXTHOP_TYPE_IPV4:
-		if (nh1->gate.ipv4.s_addr != nh2->gate.ipv4.s_addr)
-			return false;
-		break;
-	case NEXTHOP_TYPE_IPV4_IFINDEX:
-		if (nh1->gate.ipv4.s_addr != nh2->gate.ipv4.s_addr)
-			return false;
-		if (nh1->ifindex != nh2->ifindex)
-			return false;
-		break;
-	case NEXTHOP_TYPE_IPV6:
-		if (memcmp(&nh1->gate.ipv6, &nh2->gate.ipv6, 16))
-			return false;
-		break;
-	case NEXTHOP_TYPE_IPV6_IFINDEX:
-		if (memcmp(&nh1->gate.ipv6, &nh2->gate.ipv6, 16))
-			return false;
-		if (nh1->ifindex != nh2->ifindex)
-			return false;
-		break;
-	case NEXTHOP_TYPE_BLACKHOLE:
-		if (nh1->bh_type != nh2->bh_type)
-			return false;
-		break;
-	}
 
-	/* Compare labels too (if present) */
+	if (nh1 == nh2)
-	return (!!nexthop_labels_match(nh1, nh2));
+		return true;
+
+	if (_nexthop_cmp_no_labels(nh1, nh2) != 0)
+		return false;
+
+	return true;
 }
 
 /* Update nexthop with label information. */

lib/nexthop.h

@@ -139,12 +139,13 @@ void nexthop_del_labels(struct nexthop *);
 uint32_t nexthop_hash(const struct nexthop *nexthop);
 
 extern bool nexthop_same(const struct nexthop *nh1, const struct nexthop *nh2);
+extern bool nexthop_same_no_labels(const struct nexthop *nh1,
+				   const struct nexthop *nh2);
+extern int nexthop_cmp(const struct nexthop *nh1, const struct nexthop *nh2);
 
 extern const char *nexthop_type_to_str(enum nexthop_types_t nh_type);
-extern int nexthop_same_no_recurse(const struct nexthop *next1,
+extern bool nexthop_labels_match(const struct nexthop *nh1,
-				   const struct nexthop *next2);
+				 const struct nexthop *nh2);
-extern int nexthop_labels_match(const struct nexthop *nh1,
-				const struct nexthop *nh2);
 extern int nexthop_same_firsthop(struct nexthop *next1, struct nexthop *next2);
 
 extern const char *nexthop2str(const struct nexthop *nexthop,

zebra/zebra_rib.c

@@ -2855,7 +2855,11 @@ void rib_delete(afi_t afi, safi_t safi, vrf_id_t vrf_id, int type,
 				break;
 			}
 			for (ALL_NEXTHOPS(re->ng, rtnh))
-				if (nexthop_same_no_recurse(rtnh, nh)) {
+				/*
+				 * No guarantee all kernel send nh with labels
+				 * on delete.
+				 */
+				if (nexthop_same_no_labels(rtnh, nh)) {
 					same = re;
 					break;
 				}