Merge pull request #2944 from thbtcllt/master

fix zebra crash when a vrf interface changes with netns implementation for vrf

lib/if.c

@@ -371,37 +371,47 @@ struct interface *if_lookup_prefix(struct prefix *prefix, vrf_id_t vrf_id)
    one. */
 struct interface *if_get_by_name(const char *name, vrf_id_t vrf_id, int vty)
 {
-	struct interface *ifp;
+	struct interface *ifp = NULL;
 
-	ifp = if_lookup_by_name(name, vrf_id);
-	if (ifp)
-		return ifp;
-	/* Not Found on same VRF. If the interface command
-	 * was entered in vty without a VRF (passed as VRF_DEFAULT),
-	 * accept the ifp we found. If a vrf was entered and there is
-	 * a mismatch, reject it if from vty.
-	 */
-	ifp = if_lookup_by_name_all_vrf(name);
-	if (!ifp)
-		return if_create(name, vrf_id);
-	if (vty) {
-		if (vrf_id == VRF_DEFAULT)
+	if (vrf_is_mapped_on_netns(vrf_lookup_by_id(vrf_id))) {
+		ifp = if_lookup_by_name(name, vrf_id);
+		if (ifp)
 			return ifp;
-		return NULL;
-	}
-	/* if vrf backend uses NETNS, then
-	 * this should not be considered as an update
-	 * then create the new interface
-	 */
-	if (ifp->vrf_id != vrf_id && vrf_is_mapped_on_netns(
-					vrf_lookup_by_id(vrf_id)))
+		if (vty) {
+			/* If the interface command was entered in vty without a
+			 * VRF (passed as VRF_DEFAULT), search an interface with
+			 * this name in all VRs
+			 */
+			if (vrf_id == VRF_DEFAULT)
+				return if_lookup_by_name_all_vrf(name);
+			return NULL;
+		}
 		return if_create(name, vrf_id);
-	/* If it came from the kernel
-	 * or by way of zclient, believe it and update
-	 * the ifp accordingly.
-	 */
-	if_update_to_new_vrf(ifp, vrf_id);
-	return ifp;
+	}
+	/* vrf is based on vrf-lite */
+	ifp = if_lookup_by_name_all_vrf(name);
+	if (ifp) {
+		if (ifp->vrf_id == vrf_id)
+			return ifp;
+		/* Found a match on a different VRF. If the interface command
+		 * was entered in vty without a VRF (passed as VRF_DEFAULT),
+		 * accept the ifp we found. If a vrf was entered and there is a
+		 * mismatch, reject it if from vty. If it came from the kernel
+		 * or by way of zclient, believe it and update the ifp
+		 * accordingly.
+		 */
+		if (vty) {
+			if (vrf_id == VRF_DEFAULT)
+				return ifp;
+			return NULL;
+		}
+		/* If it came from the kernel or by way of zclient, believe it
+		 * and update the ifp accordingly.
+		 */
+		if_update_to_new_vrf(ifp, vrf_id);
+		return ifp;
+	}
+	return if_create(name, vrf_id);
 }
 
 void if_set_index(struct interface *ifp, ifindex_t ifindex)

lib/vrf.c

@@ -493,9 +493,15 @@ void vrf_init(int (*create)(struct vrf *), int (*enable)(struct vrf *),
 			  "vrf_init: failed to create the default VRF!");
 		exit(1);
 	}
-	if (vrf_is_backend_netns())
+	if (vrf_is_backend_netns()) {
+		struct ns *ns;
+
 		strlcpy(default_vrf->data.l.netns_name,
 			VRF_DEFAULT_NAME, NS_NAMSIZ);
+		ns = ns_lookup(ns_get_default_id());
+		ns->vrf_ctxt = default_vrf;
+		default_vrf->ns_ctxt = ns;
+	}
 
 	/* Enable the default VRF. */
 	if (!vrf_enable(default_vrf)) {
@@ -711,8 +717,6 @@ int vrf_is_mapped_on_netns(struct vrf *vrf)
 {
 	if (!vrf || vrf->data.l.netns_name[0] == '\0')
 		return 0;
-	if (vrf->vrf_id == VRF_DEFAULT)
-		return 0;
 	return 1;
 }
 

zebra/if_netlink.c

@@ -1044,67 +1044,6 @@ int netlink_interface_addr(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 	return 0;
 }
 
-/* helper function called by if_netlink_change
- * to delete interfaces in case the interface moved
- * to an other netns
- */
-static void if_netlink_check_ifp_instance_consistency(uint16_t cmd,
-						     struct interface *ifp,
-						     ns_id_t ns_id)
-{
-	struct interface *other_ifp;
-
-	/*
-	 * look if interface name is also found on other netns
-	 * - only if vrf backend is netns
-	 * - do not concern lo interface
-	 * - then remove previous one
-	 * - for new link case, check found interface is not active
-	 */
-	if (!vrf_is_backend_netns() ||
-	    !strcmp(ifp->name, "lo"))
-		return;
-	other_ifp = if_lookup_by_name_not_ns(ns_id, ifp->name);
-	if (!other_ifp)
-		return;
-	/* because previous interface may be inactive,
-	 * interface is moved back to default vrf
-	 * then one may find the same pointer; ignore
-	 */
-	if (other_ifp == ifp)
-		return;
-	if ((cmd == RTM_NEWLINK)
-	    && (CHECK_FLAG(other_ifp->status, ZEBRA_INTERFACE_ACTIVE)))
-		return;
-	if (IS_ZEBRA_DEBUG_KERNEL && cmd == RTM_NEWLINK) {
-		zlog_debug("RTM_NEWLINK %s(%u, VRF %u) replaces %s(%u, VRF %u)\n",
-			   ifp->name,
-			   ifp->ifindex,
-			   ifp->vrf_id,
-			   other_ifp->name,
-			   other_ifp->ifindex,
-			   other_ifp->vrf_id);
-	} else	if (IS_ZEBRA_DEBUG_KERNEL && cmd == RTM_DELLINK) {
-		zlog_debug("RTM_DELLINK %s(%u, VRF %u) is replaced by %s(%u, VRF %u)\n",
-			   ifp->name,
-			   ifp->ifindex,
-			   ifp->vrf_id,
-			   other_ifp->name,
-			   other_ifp->ifindex,
-			   other_ifp->vrf_id);
-	}
-	/* the found interface replaces the current one
-	 * remove it
-	 */
-	if (cmd == RTM_DELLINK)
-		if_delete(ifp);
-	else
-		if_delete(other_ifp);
-	/* the found interface is replaced by the current one
-	 * suppress it
-	 */
-}
-
 int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 {
 	int len;
@@ -1278,8 +1217,6 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 			if (IS_ZEBRA_IF_BRIDGE_SLAVE(ifp))
 				zebra_l2if_update_bridge_slave(ifp,
 							       bridge_ifindex);
-			if_netlink_check_ifp_instance_consistency(RTM_NEWLINK,
-								  ifp, ns_id);
 		} else if (ifp->vrf_id != vrf_id) {
 			/* VRF change for an interface. */
 			if (IS_ZEBRA_DEBUG_KERNEL)
@@ -1353,8 +1290,6 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 			if (IS_ZEBRA_IF_BRIDGE_SLAVE(ifp) || was_bridge_slave)
 				zebra_l2if_update_bridge_slave(ifp,
 							       bridge_ifindex);
-			if_netlink_check_ifp_instance_consistency(RTM_NEWLINK,
-								  ifp, ns_id);
 		}
 	} else {
 		/* Delete interface notification from kernel */
@@ -1378,8 +1313,6 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 
 		if (!IS_ZEBRA_IF_VRF(ifp))
 			if_delete_update(ifp);
-		if_netlink_check_ifp_instance_consistency(RTM_DELLINK,
-							  ifp, ns_id);
 	}
 
 	return 0;

zebra/interface.c

@@ -202,7 +202,6 @@ struct interface *if_link_per_ns(struct zebra_ns *ns, struct interface *ifp)
 	if (rn->info) {
 		ifp = (struct interface *)rn->info;
 		route_unlock_node(rn); /* get */
-		ifp->node = rn;
 		return ifp;
 	}
 
@@ -253,30 +252,6 @@ struct interface *if_lookup_by_name_per_ns(struct zebra_ns *ns,
 	return NULL;
 }
 
-/* this function must be used only if the vrf backend
- * is a netns backend
- */
-struct interface *if_lookup_by_name_not_ns(ns_id_t ns_id,
-					   const char *ifname)
-{
-	struct interface *ifp;
-	struct ns *ns;
-
-	RB_FOREACH (ns, ns_head, &ns_tree) {
-		if (ns->ns_id == ns_id)
-			continue;
-		/* if_delete_update has removed interface
-		 * from zns->if_table
-		 * so to look for interface, use the vrf list
-		 */
-		ifp = if_lookup_by_name(ifname, (vrf_id_t)ns->ns_id);
-		if (!ifp)
-			continue;
-		return ifp;
-	}
-	return NULL;
-}
-
 const char *ifindex2ifname_per_ns(struct zebra_ns *zns, unsigned int ifindex)
 {
 	struct interface *ifp;
@@ -753,8 +728,12 @@ void if_delete_update(struct interface *ifp)
 	ifp->node = NULL;
 
 	/* if the ifp is in a vrf, move it to default so vrf can be deleted if
-	 * desired */
-	if (ifp->vrf_id)
+	 * desired. This operation is not done for netns implementation to avoid
+	 * collision with interface with the same name in the default vrf (can
+	 * occur with this implementation whereas it is not possible with
+	 * vrf-lite).
+	 */
+	if (ifp->vrf_id && !vrf_is_backend_netns())
 		if_handle_vrf_change(ifp, VRF_DEFAULT);
 
 	/* Reset some zebra interface params to default values. */

zebra/interface.h

@@ -328,8 +328,6 @@ extern void zebra_if_init(void);
 extern struct interface *if_lookup_by_index_per_ns(struct zebra_ns *, uint32_t);
 extern struct interface *if_lookup_by_name_per_ns(struct zebra_ns *,
 						  const char *);
-extern struct interface *if_lookup_by_name_not_ns(ns_id_t ns_id,
-						  const char *ifname);
 extern struct interface *if_link_per_ns(struct zebra_ns *, struct interface *);
 extern const char *ifindex2ifname_per_ns(struct zebra_ns *, unsigned int);