*: introduce new rb-tree to optimize interface lookup by ifindex

Performance tests showed that, when running on a system with a large
number of interfaces, some daemons would spend a considerable amount
of time in the if_lookup_by_index() function. Introduce a new rb-tree
to solve this problem.

With this change, we need to use the if_set_index() function whenever
we want to change the ifindex of an interface. This is necessary to
ensure that the 'ifaces_by_index' rb-tree is updated accordingly. The
return value of all insert/remove operations in the interface rb-trees
is checked to ensure that an error is logged if a corruption is
detected.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

babeld/babel_interface.c

@@ -138,7 +138,7 @@ babel_interface_delete (int cmd, struct zclient *client, zebra_size_t length, vr
 
     /* To support pseudo interface do not free interface structure.  */
     /* if_delete(ifp); */
-    ifp->ifindex = IFINDEX_INTERNAL;
+    if_set_index(ifp, IFINDEX_INTERNAL);
 
     return 0;
 }

bgpd/bgp_zebra.c

@@ -238,7 +238,7 @@ static int bgp_interface_delete(int command, struct zclient *zclient,
 
 	bgp_update_interface_nbrs(bgp, ifp, NULL);
 
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, IFINDEX_INTERNAL);
 	return 0;
 }
 

eigrpd/eigrp_zebra.c

@@ -192,6 +192,7 @@ static int eigrp_interface_delete(int command, struct zclient *zclient,
 		eigrp_if_free(ifp->info,
 			      INTERFACE_DOWN_BY_ZEBRA);
 
+	if_set_index(ifp, IFINDEX_INTERNAL);
 	return 0;
 }
 

isisd/isis_zebra.c

@@ -128,7 +128,7 @@ static int isis_zebra_if_del(int command, struct zclient *zclient,
 	   in case there is configuration info attached to it. */
 	if_delete_retain(ifp);
 
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, IFINDEX_INTERNAL);
 
 	return 0;
 }

ldpd/ldp_zebra.c

@@ -288,7 +288,7 @@ ldp_interface_delete(int command, struct zclient *zclient, zebra_size_t length,
 
 	/* To support pseudo interface do not free interface structure.  */
 	/* if_delete(ifp); */
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, IFINDEX_INTERNAL);
 
 	ifp2kif(ifp, &kif);
 	main_imsg_compose_both(IMSG_IFSTATUS, &kif, sizeof(kif));

lib/if.c

@@ -41,7 +41,10 @@ DEFINE_MTYPE(LIB, CONNECTED_LABEL, "Connected interface label")
 DEFINE_MTYPE_STATIC(LIB, IF_LINK_PARAMS, "Informational Link Parameters")
 
 static int if_cmp_func(const struct interface *, const struct interface *);
+static int if_cmp_index_func(const struct interface *ifp1,
+			     const struct interface *ifp2);
 RB_GENERATE(if_name_head, interface, name_entry, if_cmp_func);
+RB_GENERATE(if_index_head, interface, index_entry, if_cmp_index_func);
 
 DEFINE_QOBJ_TYPE(interface)
 
@@ -118,6 +121,12 @@ static int if_cmp_func(const struct interface *ifp1,
 	return if_cmp_name_func((char *)ifp1->name, (char *)ifp2->name);
 }
 
+static int if_cmp_index_func(const struct interface *ifp1,
+			     const struct interface *ifp2)
+{
+	return ifp1->ifindex - ifp2->ifindex;
+}
+
 /* Create new interface structure. */
 struct interface *if_create(const char *name, vrf_id_t vrf_id)
 {
@@ -130,11 +139,7 @@ struct interface *if_create(const char *name, vrf_id_t vrf_id)
 	assert(name);
 	strlcpy(ifp->name, name, sizeof(ifp->name));
 	ifp->vrf_id = vrf_id;
-	if (RB_INSERT(if_name_head, &vrf->ifaces_by_name, ifp))
+	IFNAME_RB_INSERT(vrf, ifp);
-		zlog_err(
-			"if_create(%s): corruption detected -- interface with this "
-			"name exists already in VRF %u!",
-			ifp->name, vrf_id);
 	ifp->connected = list_new();
 	ifp->connected->del = (void (*)(void *))connected_free;
 
@@ -156,16 +161,18 @@ void if_update_to_new_vrf(struct interface *ifp, vrf_id_t vrf_id)
 
 	/* remove interface from old master vrf list */
 	vrf = vrf_lookup_by_id(ifp->vrf_id);
-	if (vrf)
+	if (vrf) {
-		RB_REMOVE(if_name_head, &vrf->ifaces_by_name, ifp);
+		IFNAME_RB_REMOVE(vrf, ifp);
+		if (ifp->ifindex != IFINDEX_INTERNAL)
+			IFINDEX_RB_REMOVE(vrf, ifp);
+	}
 
 	ifp->vrf_id = vrf_id;
 	vrf = vrf_get(ifp->vrf_id, NULL);
-	if (RB_INSERT(if_name_head, &vrf->ifaces_by_name, ifp))
+
-		zlog_err(
+	IFNAME_RB_INSERT(vrf, ifp);
-			"%s(%s): corruption detected -- interface with this "
+	if (ifp->ifindex != IFINDEX_INTERNAL)
-			"name exists already in VRF %u!",
+		IFINDEX_RB_INSERT(vrf, ifp);
-			__func__, ifp->name, vrf_id);
 }
 
 
@@ -187,7 +194,9 @@ void if_delete(struct interface *ifp)
 {
 	struct vrf *vrf = vrf_lookup_by_id(ifp->vrf_id);
 
-	RB_REMOVE(if_name_head, &vrf->ifaces_by_name, ifp);
+	IFNAME_RB_REMOVE(vrf, ifp);
+	if (ifp->ifindex != IFINDEX_INTERNAL)
+		IFINDEX_RB_REMOVE(vrf, ifp);
 
 	if_delete_retain(ifp);
 
@@ -203,13 +212,10 @@ void if_delete(struct interface *ifp)
 struct interface *if_lookup_by_index(ifindex_t ifindex, vrf_id_t vrf_id)
 {
 	struct vrf *vrf = vrf_lookup_by_id(vrf_id);
-	struct interface *ifp;
+	struct interface if_tmp;
 
-	RB_FOREACH (ifp, if_name_head, &vrf->ifaces_by_name)
+	if_tmp.ifindex = ifindex;
-		if (ifp->ifindex == ifindex)
+	return RB_FIND(if_index_head, &vrf->ifaces_by_index, &if_tmp);
-			return ifp;
-
-	return NULL;
 }
 
 const char *ifindex2ifname(ifindex_t ifindex, vrf_id_t vrf_id)
@@ -378,6 +384,22 @@ struct interface *if_get_by_name(const char *name, vrf_id_t vrf_id, int vty)
 	return if_create(name, vrf_id);
 }
 
+void if_set_index(struct interface *ifp, ifindex_t ifindex)
+{
+	struct vrf *vrf = vrf_lookup_by_id(ifp->vrf_id);
+
+	if (ifp->ifindex == ifindex)
+		return;
+
+	if (ifp->ifindex != IFINDEX_INTERNAL)
+		IFINDEX_RB_REMOVE(vrf, ifp)
+
+	ifp->ifindex = ifindex;
+
+	if (ifp->ifindex != IFINDEX_INTERNAL)
+		IFINDEX_RB_INSERT(vrf, ifp)
+}
+
 /* Does interface up ? */
 int if_is_up(struct interface *ifp)
 {

lib/if.h

@@ -201,7 +201,7 @@ struct if_link_params {
 
 /* Interface structure */
 struct interface {
-	RB_ENTRY(interface) name_entry;
+	RB_ENTRY(interface) name_entry, index_entry;
 
 	/* Interface name.  This should probably never be changed after the
 	   interface is created, because the configuration info for this
@@ -214,7 +214,12 @@ struct interface {
 	char name[INTERFACE_NAMSIZ];
 
 	/* Interface index (should be IFINDEX_INTERNAL for non-kernel or
-	   deleted interfaces). */
+	   deleted interfaces).
+	   WARNING: the ifindex needs to be changed using the if_set_index()
+	   function. Failure to respect this will cause corruption in the data
+	   structure used to store the interfaces and if_lookup_by_index() will
+	   not work as expected.
+	 */
 	ifindex_t ifindex;
 #define IFINDEX_INTERNAL	0
 
@@ -285,8 +290,38 @@ struct interface {
 };
 RB_HEAD(if_name_head, interface);
 RB_PROTOTYPE(if_name_head, interface, name_entry, if_cmp_func);
+RB_HEAD(if_index_head, interface);
+RB_PROTOTYPE(if_index_head, interface, index_entry, if_cmp_func);
 DECLARE_QOBJ_TYPE(interface)
 
+#define IFNAME_RB_INSERT(vrf, ifp)                                             \
+	if (RB_INSERT(if_name_head, &vrf->ifaces_by_name, (ifp)))              \
+		zlog_err(                                                      \
+			"%s(%s): corruption detected -- interface with this "  \
+			"name exists already in VRF %u!",                      \
+			__func__, (ifp)->name, (ifp)->vrf_id);
+
+#define IFNAME_RB_REMOVE(vrf, ifp)                                             \
+	if (RB_REMOVE(if_name_head, &vrf->ifaces_by_name, (ifp)) == NULL)      \
+		zlog_err(                                                      \
+			"%s(%s): corruption detected -- interface with this "  \
+			"name doesn't exist in VRF %u!",                       \
+			__func__, (ifp)->name, (ifp)->vrf_id);
+
+#define IFINDEX_RB_INSERT(vrf, ifp)                                            \
+	if (RB_INSERT(if_index_head, &vrf->ifaces_by_index, (ifp)))            \
+		zlog_err(                                                      \
+			"%s(%u): corruption detected -- interface with this "  \
+			"ifindex exists already in VRF %u!",                   \
+			__func__, (ifp)->ifindex, (ifp)->vrf_id);
+
+#define IFINDEX_RB_REMOVE(vrf, ifp)                                            \
+	if (RB_REMOVE(if_index_head, &vrf->ifaces_by_index, (ifp)) == NULL)    \
+		zlog_err(                                                      \
+			"%s(%u): corruption detected -- interface with this "  \
+			"ifindex doesn't exist in VRF %u!",                    \
+			__func__, (ifp)->ifindex, (ifp)->vrf_id);
+
 /* called from the library code whenever interfaces are created/deleted
  * note: interfaces may not be fully realized at that point; also they
  * may not exist in the system (ifindex = IFINDEX_INTERNAL)
@@ -426,6 +461,7 @@ extern struct interface *if_lookup_by_name_all_vrf(const char *ifname);
 extern struct interface *if_lookup_by_name(const char *ifname, vrf_id_t vrf_id);
 extern struct interface *if_get_by_name(const char *ifname, vrf_id_t vrf_id,
 					int vty);
+extern void if_set_index(struct interface *ifp, ifindex_t ifindex);
 
 /* Delete the interface, but do not free the structure, and leave it in the
    interface list.  It is often advisable to leave the pseudo interface

lib/vrf.c

@@ -110,6 +110,7 @@ struct vrf *vrf_get(vrf_id_t vrf_id, const char *name)
 		vrf = XCALLOC(MTYPE_VRF, sizeof(struct vrf));
 		vrf->vrf_id = VRF_UNKNOWN;
 		RB_INIT(if_name_head, &vrf->ifaces_by_name);
+		RB_INIT(if_index_head, &vrf->ifaces_by_index);
 		QOBJ_REG(vrf, vrf);
 		new = 1;
 

lib/vrf.h

@@ -79,6 +79,7 @@ struct vrf {
 
 	/* Interfaces belonging to this VRF */
 	struct if_name_head ifaces_by_name;
+	struct if_index_head ifaces_by_index;
 
 	/* User data */
 	void *info;

lib/zclient.c

@@ -1331,7 +1331,7 @@ void zebra_interface_if_set_value(struct stream *s, struct interface *ifp)
 	u_char link_params_status = 0;
 
 	/* Read interface's index. */
-	ifp->ifindex = stream_getl(s);
+	if_set_index(ifp, stream_getl(s));
 	ifp->status = stream_getc(s);
 
 	/* Read interface's value. */

nhrpd/nhrp_interface.c

@@ -299,7 +299,7 @@ int nhrp_interface_delete(int cmd, struct zclient *client,
 		return 0;
 
 	debugf(NHRP_DEBUG_IF, "if-delete: %s", ifp->name);
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, ifp->ifindex);
 	nhrp_interface_update(ifp);
 	/* if_delete(ifp); */
 	return 0;

ospf6d/ospf6_zebra.c

@@ -126,7 +126,7 @@ static int ospf6_zebra_if_del(int command, struct zclient *zclient,
   ospf6_interface_if_del (ifp);
 #endif /*0*/
 
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, IFINDEX_INTERNAL);
 	return 0;
 }
 

ospfd/ospf_zebra.c

@@ -166,7 +166,7 @@ static int ospf_interface_delete(int command, struct zclient *zclient,
 		if (rn->info)
 			ospf_if_free((struct ospf_interface *)rn->info);
 
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, IFINDEX_INTERNAL);
 	return 0;
 }
 

ripd/rip_interface.c

@@ -471,7 +471,7 @@ int rip_interface_delete(int command, struct zclient *zclient,
 
 	/* To support pseudo interface do not free interface structure.  */
 	/* if_delete(ifp); */
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, IFINDEX_INTERNAL);
 
 	return 0;
 }

ripngd/ripng_interface.c

@@ -299,7 +299,7 @@ int ripng_interface_delete(int command, struct zclient *zclient,
 
 	/* To support pseudo interface do not free interface structure.  */
 	/* if_delete(ifp); */
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, IFINDEX_INTERNAL);
 
 	return 0;
 }

zebra/if_ioctl.c

@@ -131,7 +131,7 @@ end:
 /* Get interface's index by ioctl. */
 static int if_get_index(struct interface *ifp)
 {
-	ifp->ifindex = if_nametoindex(ifp->name);
+	if_set_index(ifp, if_nametoindex(ifp->name));
 	return ifp->ifindex;
 }
 

zebra/if_ioctl_solaris.c

@@ -227,9 +227,9 @@ static int if_get_index(struct interface *ifp)
 
 /* OK we got interface index. */
 #ifdef ifr_ifindex
-	ifp->ifindex = lifreq.lifr_ifindex;
+	if_set_index(ifp, lifreq.lifr_ifindex);
 #else
-	ifp->ifindex = lifreq.lifr_index;
+	if_set_index(ifp, lifreq.lifr_index);
 #endif
 	return ifp->ifindex;
 }

zebra/if_netlink.c

@@ -93,7 +93,7 @@ static void set_ifindex(struct interface *ifp, ifindex_t ifi_index,
 			if_delete_update(oifp);
 		}
 	}
-	ifp->ifindex = ifi_index;
+	if_set_index(ifp, ifi_index);
 }
 
 /* Utility function to parse hardware link-layer address and update ifp */

zebra/interface.c

@@ -685,7 +685,7 @@ void if_delete_update(struct interface *ifp)
 	   while processing the deletion.  Each client daemon is responsible
 	   for setting ifindex to IFINDEX_INTERNAL after processing the
 	   interface deletion message. */
-	ifp->ifindex = IFINDEX_INTERNAL;
+	if_set_index(ifp, IFINDEX_INTERNAL);
 	ifp->node = NULL;
 
 	/* if the ifp is in a vrf, move it to default so vrf can be deleted if

zebra/kernel_socket.c

@@ -324,7 +324,7 @@ static int ifan_read(struct if_announcemsghdr *ifan)
 
 		/* Create Interface */
 		ifp = if_get_by_name(ifan->ifan_name, VRF_DEFAULT, 0);
-		ifp->ifindex = ifan->ifan_index;
+		if_set_index(ifp, ifan->ifan_index);
 
 		if_get_metric(ifp);
 		if_add_update(ifp);
@@ -528,7 +528,7 @@ int ifm_read(struct if_msghdr *ifm)
 		 * Fill in newly created interface structure, or larval
 		 * structure with ifindex IFINDEX_INTERNAL.
 		 */
-		ifp->ifindex = ifm->ifm_index;
+		if_set_index(ifp, ifm->ifm_index);
 
 #ifdef HAVE_BSD_IFI_LINK_STATE /* translate BSD kernel msg for link-state */
 		bsd_linkdetect_translate(ifm);