Merge pull request #3776 from opensourcerouting/pbrd-interface-nexthops

pbrd: add support for interface nexthops

lib/nexthop_group.c

@@ -188,11 +188,25 @@ static int nhgc_cmp_helper(const char *a, const char *b)
 	return strcmp(a, b);
 }
 
+static int nhgc_addr_cmp_helper(const union sockunion *a, const union sockunion *b)
+{
+	if (!a && !b)
+		return 0;
+
+	if (a && !b)
+		return -1;
+
+	if (!a && b)
+		return 1;
+
+	return sockunion_cmp(a, b);
+}
+
 static int nhgl_cmp(struct nexthop_hold *nh1, struct nexthop_hold *nh2)
 {
 	int ret;
 
-	ret = sockunion_cmp(&nh1->addr, &nh2->addr);
+	ret = nhgc_addr_cmp_helper(nh1->addr, nh2->addr);
 	if (ret)
 		return ret;
 
@@ -209,6 +223,9 @@ static void nhgl_delete(struct nexthop_hold *nh)
 
 	XFREE(MTYPE_TMP, nh->nhvrf_name);
 
+	if (nh->addr)
+		sockunion_free(nh->addr);
+
 	XFREE(MTYPE_TMP, nh);
 }
 
@@ -292,8 +309,8 @@ static void nexthop_group_save_nhop(struct nexthop_group_cmd *nhgc,
 		nh->nhvrf_name = XSTRDUP(MTYPE_TMP, nhvrf_name);
 	if (intf)
 		nh->intf = XSTRDUP(MTYPE_TMP, intf);
-
+	if (addr)
-	nh->addr = *addr;
+		nh->addr = sockunion_dup(addr);
 
 	listnode_add_sort(nhgc->nhg_list, nh);
 }
@@ -308,7 +325,7 @@ static void nexthop_group_unsave_nhop(struct nexthop_group_cmd *nhgc,
 
 	for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_list, node, nh)) {
 		if (nhgc_cmp_helper(nhvrf_name, nh->nhvrf_name) == 0 &&
-		    sockunion_cmp(addr, &nh->addr) == 0 &&
+		    nhgc_addr_cmp_helper(addr, nh->addr) == 0 &&
 		    nhgc_cmp_helper(intf, nh->intf) == 0)
 			break;
 	}
@@ -320,13 +337,7 @@ static void nexthop_group_unsave_nhop(struct nexthop_group_cmd *nhgc,
 		return;
 
 	list_delete_node(nhgc->nhg_list, node);
-
+	nhgl_delete(nh);
-	if (nh->nhvrf_name)
-		XFREE(MTYPE_TMP, nh->nhvrf_name);
-	if (nh->intf)
-		XFREE(MTYPE_TMP, nh->intf);
-
-	XFREE(MTYPE_TMP, nh);
 }
 
 static bool nexthop_group_parse_nexthop(struct nexthop *nhop,
@@ -347,36 +358,45 @@ static bool nexthop_group_parse_nexthop(struct nexthop *nhop,
 
 	nhop->vrf_id = vrf->vrf_id;
 
+	if (intf) {
+		nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id);
+		if (nhop->ifindex == IFINDEX_INTERNAL)
+			return false;
+	}
+
+	if (addr) {
 		if (addr->sa.sa_family == AF_INET) {
 			nhop->gate.ipv4.s_addr = addr->sin.sin_addr.s_addr;
-		if (intf) {
+			if (intf)
 				nhop->type = NEXTHOP_TYPE_IPV4_IFINDEX;
-			nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id);
+			else
-			if (nhop->ifindex == IFINDEX_INTERNAL)
-				return false;
-		} else
 				nhop->type = NEXTHOP_TYPE_IPV4;
 		} else {
-		memcpy(&nhop->gate.ipv6, &addr->sin6.sin6_addr, 16);
+			nhop->gate.ipv6 = addr->sin6.sin6_addr;
-		if (intf) {
+			if (intf)
 				nhop->type = NEXTHOP_TYPE_IPV6_IFINDEX;
-			nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id);
+			else
-			if (nhop->ifindex == IFINDEX_INTERNAL)
-				return false;
-		} else
 				nhop->type = NEXTHOP_TYPE_IPV6;
 		}
+	} else
+		nhop->type = NEXTHOP_TYPE_IFINDEX;
 
 	return true;
 }
 
 DEFPY(ecmp_nexthops, ecmp_nexthops_cmd,
-      "[no] nexthop <A.B.C.D|X:X::X:X>$addr [INTERFACE]$intf [nexthop-vrf NAME$name]",
+      "[no] nexthop\
+        <\
+	  <A.B.C.D|X:X::X:X>$addr [INTERFACE$intf]\
+	  |INTERFACE$intf\
+	>\
+	[nexthop-vrf NAME$name]",
       NO_STR
       "Specify one of the nexthops in this ECMP group\n"
       "v4 Address\n"
       "v6 Address\n"
       "Interface to use\n"
+      "Interface to use\n"
       "If the nexthop is in a different vrf tell us\n"
       "The nexthop-vrf Name\n")
 {
@@ -385,13 +405,6 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd,
 	struct nexthop *nh;
 	bool legal;
 
-	/*
-	 * This is impossible to happen as that the cli parser refuses
-	 * to let you get here without an addr, but the SA system
-	 * does not understand this intricacy
-	 */
-	assert(addr);
-
 	legal = nexthop_group_parse_nexthop(&nhop, addr, intf, name);
 
 	if (nhop.type == NEXTHOP_TYPE_IPV6
@@ -482,7 +495,8 @@ static void nexthop_group_write_nexthop_internal(struct vty *vty,
 
 	vty_out(vty, "nexthop");
 
-	vty_out(vty, "%s", sockunion2str(&nh->addr, buf, sizeof(buf)));
+	if (nh->addr)
+		vty_out(vty, " %s", sockunion2str(nh->addr, buf, sizeof(buf)));
 
 	if (nh->intf)
 		vty_out(vty, " %s", nh->intf);
@@ -526,7 +540,7 @@ void nexthop_group_enable_vrf(struct vrf *vrf)
 			struct nexthop nhop;
 			struct nexthop *nh;
 
-			if (!nexthop_group_parse_nexthop(&nhop, &nhh->addr,
+			if (!nexthop_group_parse_nexthop(&nhop, nhh->addr,
 							 nhh->intf,
 							 nhh->nhvrf_name))
 				continue;
@@ -562,7 +576,7 @@ void nexthop_group_disable_vrf(struct vrf *vrf)
 			struct nexthop nhop;
 			struct nexthop *nh;
 
-			if (!nexthop_group_parse_nexthop(&nhop, &nhh->addr,
+			if (!nexthop_group_parse_nexthop(&nhop, nhh->addr,
 							 nhh->intf,
 							 nhh->nhvrf_name))
 				continue;
@@ -600,7 +614,7 @@ void nexthop_group_interface_state_change(struct interface *ifp,
 				struct nexthop nhop;
 
 				if (!nexthop_group_parse_nexthop(
-					    &nhop, &nhh->addr, nhh->intf,
+					    &nhop, nhh->addr, nhh->intf,
 					    nhh->nhvrf_name))
 					continue;
 

lib/nexthop_group.h

@@ -68,7 +68,7 @@ void copy_nexthops(struct nexthop **tnh, struct nexthop *nh,
 
 struct nexthop_hold {
 	char *nhvrf_name;
-	union sockunion addr;
+	union sockunion *addr;
 	char *intf;
 };
 

pbrd/pbr_nht.c

@@ -50,7 +50,7 @@ static void pbr_nht_install_nexthop_group(struct pbr_nexthop_group_cache *pnhgc,
 static void
 pbr_nht_uninstall_nexthop_group(struct pbr_nexthop_group_cache *pnhgc,
 				struct nexthop_group nhg,
-				enum nexthop_types_t nh_afi);
+				enum nexthop_types_t nh_type);
 
 /*
  * Nexthop refcount.
@@ -157,7 +157,7 @@ static bool pbr_nh_hash_equal(const void *arg1, const void *arg2)
 
 	switch (pbrnc1->nexthop->type) {
 	case NEXTHOP_TYPE_IFINDEX:
-		return true;
+		return pbrnc1->nexthop->ifindex == pbrnc2->nexthop->ifindex;
 	case NEXTHOP_TYPE_IPV4_IFINDEX:
 	case NEXTHOP_TYPE_IPV4:
 		return pbrnc1->nexthop->gate.ipv4.s_addr
@@ -264,6 +264,14 @@ void pbr_nhgroup_add_nexthop_cb(const struct nexthop_group_cmd *nhgc,
 
 	pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg);
 	pbr_map_check_nh_group_change(nhgc->name);
+
+	if (nhop->type == NEXTHOP_TYPE_IFINDEX) {
+		struct interface *ifp;
+
+		ifp = if_lookup_by_index(nhop->ifindex, nhop->vrf_id);
+		if (ifp)
+			pbr_nht_nexthop_interface_update(ifp);
+	}
 }
 
 void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc,
@@ -274,7 +282,7 @@ void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc,
 	struct pbr_nexthop_group_cache *pnhgc;
 	struct pbr_nexthop_cache pnhc_find = {};
 	struct pbr_nexthop_cache *pnhc;
-	enum nexthop_types_t nh_afi = nhop->type;
+	enum nexthop_types_t nh_type = nhop->type;
 
 	/* find pnhgc by name */
 	strlcpy(pnhgc_find.name, nhgc->name, sizeof(pnhgc_find.name));
@@ -296,7 +304,7 @@ void pbr_nhgroup_del_nexthop_cb(const struct nexthop_group_cmd *nhgc,
 	if (pnhgc->nhh->count)
 		pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg);
 	else
-		pbr_nht_uninstall_nexthop_group(pnhgc, nhgc->nhg, nh_afi);
+		pbr_nht_uninstall_nexthop_group(pnhgc, nhgc->nhg, nh_type);
 
 	pbr_map_check_nh_group_change(nhgc->name);
 }
@@ -372,22 +380,32 @@ void pbr_nht_route_removed_for_table(uint32_t table_id)
  *    - AFI_MAX on error
  */
 static afi_t pbr_nht_which_afi(struct nexthop_group nhg,
-			       enum nexthop_types_t nh_afi)
+			       enum nexthop_types_t nh_type)
 {
 	struct nexthop *nexthop;
 	afi_t install_afi = AFI_MAX;
 	bool v6, v4, bh;
 
+	if (nh_type) {
+		switch (nh_type) {
+		case NEXTHOP_TYPE_IPV4:
+		case NEXTHOP_TYPE_IPV4_IFINDEX:
+			return AFI_IP;
+		case NEXTHOP_TYPE_IPV6:
+		case NEXTHOP_TYPE_IPV6_IFINDEX:
+			return AFI_IP6;
+		case NEXTHOP_TYPE_IFINDEX:
+		case NEXTHOP_TYPE_BLACKHOLE:
+			return AFI_MAX;
+		}
+	}
+
 	v6 = v4 = bh = false;
 
-	if (!nh_afi) {
 	for (ALL_NEXTHOPS(nhg, nexthop)) {
-			nh_afi = nexthop->type;
+		nh_type = nexthop->type;
-			break;
-		}
-	}
 
-	switch (nh_afi) {
+		switch (nh_type) {
 		case NEXTHOP_TYPE_IFINDEX:
 			break;
 		case NEXTHOP_TYPE_IPV4:
@@ -402,9 +420,13 @@ static afi_t pbr_nht_which_afi(struct nexthop_group nhg,
 			break;
 		case NEXTHOP_TYPE_BLACKHOLE:
 			bh = true;
-		install_afi = AFI_MAX;
 			break;
 		}
+	}
+
+	/* Interface and/or blackhole nexthops only. */
+	if (!v4 && !v6)
+		install_afi = AFI_MAX;
 
 	if (!bh && v6 && v4)
 		DEBUGD(&pbr_dbg_nht,
@@ -423,9 +445,9 @@ static void pbr_nht_install_nexthop_group(struct pbr_nexthop_group_cache *pnhgc,
 					  struct nexthop_group nhg)
 {
 	afi_t install_afi;
-	enum nexthop_types_t nh_afi = 0;
+	enum nexthop_types_t nh_type = 0;
 
-	install_afi = pbr_nht_which_afi(nhg, nh_afi);
+	install_afi = pbr_nht_which_afi(nhg, nh_type);
 
 	route_add(pnhgc, nhg, install_afi);
 }
@@ -433,11 +455,11 @@ static void pbr_nht_install_nexthop_group(struct pbr_nexthop_group_cache *pnhgc,
 static void
 pbr_nht_uninstall_nexthop_group(struct pbr_nexthop_group_cache *pnhgc,
 				struct nexthop_group nhg,
-				enum nexthop_types_t nh_afi)
+				enum nexthop_types_t nh_type)
 {
 	afi_t install_afi;
 
-	install_afi = pbr_nht_which_afi(nhg, nh_afi);
+	install_afi = pbr_nht_which_afi(nhg, nh_type);
 
 	pnhgc->installed = false;
 	pnhgc->valid = false;
@@ -526,7 +548,7 @@ void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms)
 	struct listnode *node;
 	struct pbr_map_interface *pmi;
 	struct nexthop *nh;
-	enum nexthop_types_t nh_afi = 0;
+	enum nexthop_types_t nh_type = 0;
 
 	if (pbrm->valid && pbrms->nhs_installed && pbrm->incoming->count) {
 		for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, node, pmi))
@@ -542,13 +564,13 @@ void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms)
 	pnhgc = hash_lookup(pbr_nhg_hash, &find);
 
 	nh = pbrms->nhg->nexthop;
-	nh_afi = nh->type;
+	nh_type = nh->type;
 	lup.nexthop = nh;
 	pnhc = hash_lookup(pnhgc->nhh, &lup);
 	pnhc->parent = NULL;
 	hash_release(pnhgc->nhh, pnhc);
 	pbr_nh_delete(&pnhc);
-	pbr_nht_uninstall_nexthop_group(pnhgc, *pbrms->nhg, nh_afi);
+	pbr_nht_uninstall_nexthop_group(pnhgc, *pbrms->nhg, nh_type);
 
 	hash_release(pbr_nhg_hash, pnhgc);
 
@@ -653,6 +675,7 @@ bool pbr_nht_nexthop_group_valid(const char *name)
 
 struct pbr_nht_individual {
 	struct zapi_route *nhr;
+	struct interface *ifp;
 
 	uint32_t valid;
 };
@@ -716,6 +739,56 @@ void pbr_nht_nexthop_update(struct zapi_route *nhr)
 	hash_iterate(pbr_nhg_hash, pbr_nht_nexthop_update_lookup, nhr);
 }
 
+static void
+pbr_nht_individual_nexthop_interface_update_lookup(struct hash_backet *b,
+						   void *data)
+{
+	struct pbr_nexthop_cache *pnhc = b->data;
+	struct pbr_nht_individual *pnhi = data;
+	bool old_valid;
+
+	old_valid = pnhc->valid;
+
+	if (pnhc->nexthop->type == NEXTHOP_TYPE_IFINDEX
+	    && pnhc->nexthop->ifindex == pnhi->ifp->ifindex)
+		pnhc->valid = !!if_is_up(pnhi->ifp);
+
+	DEBUGD(&pbr_dbg_nht, "\tFound %s: old: %d new: %d", pnhi->ifp->name,
+	       old_valid, pnhc->valid);
+
+	if (pnhc->valid)
+		pnhi->valid += 1;
+}
+
+static void pbr_nht_nexthop_interface_update_lookup(struct hash_backet *b,
+						    void *data)
+{
+	struct pbr_nexthop_group_cache *pnhgc = b->data;
+	struct pbr_nht_individual pnhi;
+	bool old_valid;
+
+	old_valid = pnhgc->valid;
+
+	pnhi.ifp = data;
+	pnhi.valid = 0;
+	hash_iterate(pnhgc->nhh,
+		     pbr_nht_individual_nexthop_interface_update_lookup, &pnhi);
+
+	/*
+	 * If any of the specified nexthops are valid we are valid
+	 */
+	pnhgc->valid = !!pnhi.valid;
+
+	if (old_valid != pnhgc->valid)
+		pbr_map_check_nh_group_change(pnhgc->name);
+}
+
+void pbr_nht_nexthop_interface_update(struct interface *ifp)
+{
+	hash_iterate(pbr_nhg_hash, pbr_nht_nexthop_interface_update_lookup,
+		     ifp);
+}
+
 static uint32_t pbr_nhg_hash_key(void *arg)
 {
 	struct pbr_nexthop_group_cache *nhgc =

pbrd/pbr_nht.h

@@ -117,5 +117,10 @@ extern void pbr_nht_show_nexthop_group(struct vty *vty, const char *name);
  */
 extern void pbr_nht_nexthop_update(struct zapi_route *nhr);
 
+/*
+ * When we get a callback from zebra about an interface status update.
+ */
+extern void pbr_nht_nexthop_interface_update(struct interface *ifp);
+
 extern void pbr_nht_init(void);
 #endif

pbrd/pbr_vty.c

@@ -221,13 +221,19 @@ DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd,
 }
 
 DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
-      "[no] set nexthop <A.B.C.D|X:X::X:X>$addr [INTERFACE]$intf [nexthop-vrf NAME$name]",
+      "[no] set nexthop\
+        <\
+	  <A.B.C.D|X:X::X:X>$addr [INTERFACE$intf]\
+	  |INTERFACE$intf\
+	>\
+        [nexthop-vrf NAME$name]",
       NO_STR
       "Set for the PBR-MAP\n"
       "Specify one of the nexthops in this map\n"
       "v4 Address\n"
       "v6 Address\n"
       "Interface to use\n"
+      "Interface to use\n"
       "If the nexthop is in a different vrf tell us\n"
       "The nexthop-vrf Name\n")
 {
@@ -255,36 +261,28 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
 	memset(&nhop, 0, sizeof(nhop));
 	nhop.vrf_id = vrf->vrf_id;
 
-	/*
+	if (intf) {
-	 * Make SA happy.  CLIPPY is not going to give us a NULL
+		nhop.ifindex = ifname2ifindex(intf, vrf->vrf_id);
-	 * addr.
+		if (nhop.ifindex == IFINDEX_INTERNAL) {
-	 */
+			vty_out(vty,
-	assert(addr);
+				"Specified Intf %s does not exist in vrf: %s\n",
+				intf, vrf->name);
+			return CMD_WARNING_CONFIG_FAILED;
+		}
+	}
+
+	if (addr) {
 		if (addr->sa.sa_family == AF_INET) {
 			nhop.gate.ipv4.s_addr = addr->sin.sin_addr.s_addr;
-		if (intf) {
+			if (intf)
 				nhop.type = NEXTHOP_TYPE_IPV4_IFINDEX;
-			nhop.ifindex = ifname2ifindex(intf, vrf->vrf_id);
+			else
-			if (nhop.ifindex == IFINDEX_INTERNAL) {
-				vty_out(vty,
-					"Specified Intf %s does not exist in vrf: %s\n",
-					intf, vrf->name);
-				return CMD_WARNING_CONFIG_FAILED;
-			}
-		} else
 				nhop.type = NEXTHOP_TYPE_IPV4;
 		} else {
-		memcpy(&nhop.gate.ipv6, &addr->sin6.sin6_addr, 16);
+			nhop.gate.ipv6 = addr->sin6.sin6_addr;
-		if (intf) {
+			if (intf)
 				nhop.type = NEXTHOP_TYPE_IPV6_IFINDEX;
-			nhop.ifindex = ifname2ifindex(intf, vrf->vrf_id);
+			else {
-			if (nhop.ifindex == IFINDEX_INTERNAL) {
-				vty_out(vty,
-					"Specified Intf %s does not exist in vrf: %s\n",
-					intf, vrf->name);
-				return CMD_WARNING_CONFIG_FAILED;
-			}
-		} else {
 				if (IN6_IS_ADDR_LINKLOCAL(&nhop.gate.ipv6)) {
 					vty_out(vty,
 						"Specified a v6 LL with no interface, rejecting\n");
@@ -293,6 +291,8 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
 				nhop.type = NEXTHOP_TYPE_IPV6;
 			}
 		}
+	} else
+		nhop.type = NEXTHOP_TYPE_IFINDEX;
 
 	if (pbrms->nhg)
 		nh = nexthop_exists(pbrms->nhg, &nhop);
@@ -335,6 +335,14 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
 		pbr_map_check(pbrms);
 	}
 
+	if (nhop.type == NEXTHOP_TYPE_IFINDEX) {
+		struct interface *ifp;
+
+		ifp = if_lookup_by_index(nhop.ifindex, nhop.vrf_id);
+		if (ifp)
+			pbr_nht_nexthop_interface_update(ifp);
+	}
+
 	return CMD_SUCCESS;
 }
 

pbrd/pbr_zebra.c

@@ -75,6 +75,8 @@ static int interface_add(int command, struct zclient *zclient,
 	if (!ifp->info)
 		pbr_if_new(ifp);
 
+	pbr_nht_nexthop_interface_update(ifp);
+
 	return 0;
 }
 
@@ -144,6 +146,8 @@ static int interface_state_up(int command, struct zclient *zclient,
 	DEBUGD(&pbr_dbg_zebra,
 	       "%s: %s is up", __PRETTY_FUNCTION__, ifp->name);
 
+	pbr_nht_nexthop_interface_update(ifp);
+
 	return 0;
 }
 
@@ -157,6 +161,8 @@ static int interface_state_down(int command, struct zclient *zclient,
 	DEBUGD(&pbr_dbg_zebra,
 	       "%s: %s is down", __PRETTY_FUNCTION__, ifp->name);
 
+	pbr_nht_nexthop_interface_update(ifp);
+
 	return 0;
 }