mirror of
https://git.proxmox.com/git/mirror_frr
synced 2025-08-14 08:50:26 +00:00
lib: handle failure to change ifindex
This fixes a theoretical bug that could occur when trying to change an ifindex on an interface to that of an existing interface. We would remove the interface from the ifindex tree, and change the ifindex, but when we tried to reinsert the interface, the insert would fail. It was impossible to know if this failed due to the insertion / deletion macros capturing the result value of the underlying BSD tree macros. So we would effectively delete the interface. Instead of failing on insert, we just check if the prospective ifindex already exists and return -1 if it does. Macros have been changed to statement expressions so the result can be checked, and bubbled up. Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
This commit is contained in:
parent
f17a9d0a07
commit
67f8158620
26
lib/if.c
26
lib/if.c
@ -582,23 +582,39 @@ struct interface *if_get_by_ifindex(ifindex_t ifindex, vrf_id_t vrf_id)
|
||||
return NULL;
|
||||
}
|
||||
|
||||
void if_set_index(struct interface *ifp, ifindex_t ifindex)
|
||||
int if_set_index(struct interface *ifp, ifindex_t ifindex)
|
||||
{
|
||||
struct vrf *vrf;
|
||||
|
||||
if (ifp->ifindex == ifindex)
|
||||
return 0;
|
||||
|
||||
vrf = vrf_get(ifp->vrf_id, NULL);
|
||||
assert(vrf);
|
||||
|
||||
if (ifp->ifindex == ifindex)
|
||||
return;
|
||||
/*
|
||||
* If there is already an interface with this ifindex, we will collide
|
||||
* on insertion, so don't even try.
|
||||
*/
|
||||
if (if_lookup_by_ifindex(ifindex, ifp->vrf_id))
|
||||
return -1;
|
||||
|
||||
if (ifp->ifindex != IFINDEX_INTERNAL)
|
||||
IFINDEX_RB_REMOVE(vrf, ifp);
|
||||
|
||||
ifp->ifindex = ifindex;
|
||||
|
||||
if (ifp->ifindex != IFINDEX_INTERNAL)
|
||||
IFINDEX_RB_INSERT(vrf, ifp)
|
||||
if (ifp->ifindex != IFINDEX_INTERNAL) {
|
||||
/*
|
||||
* This should never happen, since we checked if there was
|
||||
* already an interface with the desired ifindex at the top of
|
||||
* the function. Nevertheless.
|
||||
*/
|
||||
if (IFINDEX_RB_INSERT(vrf, ifp))
|
||||
return -1;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
void if_set_name(struct interface *ifp, const char *name)
|
||||
|
75
lib/if.h
75
lib/if.h
@ -308,33 +308,58 @@ RB_HEAD(if_index_head, interface);
|
||||
RB_PROTOTYPE(if_index_head, interface, index_entry, if_cmp_index_func)
|
||||
DECLARE_QOBJ_TYPE(interface)
|
||||
|
||||
#define IFNAME_RB_INSERT(vrf, ifp) \
|
||||
if (RB_INSERT(if_name_head, &vrf->ifaces_by_name, (ifp))) \
|
||||
flog_err(EC_LIB_INTERFACE, \
|
||||
"%s(%s): corruption detected -- interface with this " \
|
||||
"name exists already in VRF %u!", \
|
||||
__func__, (ifp)->name, (ifp)->vrf_id);
|
||||
#define IFNAME_RB_INSERT(vrf, ifp) \
|
||||
({ \
|
||||
struct interface *_iz = \
|
||||
RB_INSERT(if_name_head, &vrf->ifaces_by_name, (ifp)); \
|
||||
if (_iz) \
|
||||
flog_err( \
|
||||
EC_LIB_INTERFACE, \
|
||||
"%s(%s): corruption detected -- interface with this " \
|
||||
"name exists already in VRF %u!", \
|
||||
__func__, (ifp)->name, (ifp)->vrf_id); \
|
||||
_iz; \
|
||||
})
|
||||
|
||||
#define IFNAME_RB_REMOVE(vrf, ifp) \
|
||||
if (RB_REMOVE(if_name_head, &vrf->ifaces_by_name, (ifp)) == NULL) \
|
||||
flog_err(EC_LIB_INTERFACE, \
|
||||
"%s(%s): corruption detected -- interface with this " \
|
||||
"name doesn't exist in VRF %u!", \
|
||||
__func__, (ifp)->name, (ifp)->vrf_id);
|
||||
#define IFNAME_RB_REMOVE(vrf, ifp) \
|
||||
({ \
|
||||
struct interface *_iz = \
|
||||
RB_REMOVE(if_name_head, &vrf->ifaces_by_name, (ifp)); \
|
||||
if (_iz == NULL) \
|
||||
flog_err( \
|
||||
EC_LIB_INTERFACE, \
|
||||
"%s(%s): corruption detected -- interface with this " \
|
||||
"name doesn't exist in VRF %u!", \
|
||||
__func__, (ifp)->name, (ifp)->vrf_id); \
|
||||
_iz; \
|
||||
})
|
||||
|
||||
#define IFINDEX_RB_INSERT(vrf, ifp) \
|
||||
if (RB_INSERT(if_index_head, &vrf->ifaces_by_index, (ifp))) \
|
||||
flog_err(EC_LIB_INTERFACE, \
|
||||
"%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) \
|
||||
flog_err(EC_LIB_INTERFACE, \
|
||||
"%s(%u): corruption detected -- interface with this " \
|
||||
"ifindex doesn't exist in VRF %u!", \
|
||||
__func__, (ifp)->ifindex, (ifp)->vrf_id);
|
||||
#define IFINDEX_RB_INSERT(vrf, ifp) \
|
||||
({ \
|
||||
struct interface *_iz = RB_INSERT( \
|
||||
if_index_head, &vrf->ifaces_by_index, (ifp)); \
|
||||
if (_iz) \
|
||||
flog_err( \
|
||||
EC_LIB_INTERFACE, \
|
||||
"%s(%u): corruption detected -- interface with this " \
|
||||
"ifindex exists already in VRF %u!", \
|
||||
__func__, (ifp)->ifindex, (ifp)->vrf_id); \
|
||||
_iz; \
|
||||
})
|
||||
|
||||
#define IFINDEX_RB_REMOVE(vrf, ifp) \
|
||||
({ \
|
||||
struct interface *_iz = RB_REMOVE( \
|
||||
if_index_head, &vrf->ifaces_by_index, (ifp)); \
|
||||
if (_iz == NULL) \
|
||||
flog_err( \
|
||||
EC_LIB_INTERFACE, \
|
||||
"%s(%u): corruption detected -- interface with this " \
|
||||
"ifindex doesn't exist in VRF %u!", \
|
||||
__func__, (ifp)->ifindex, (ifp)->vrf_id); \
|
||||
_iz; \
|
||||
})
|
||||
|
||||
#define FOR_ALL_INTERFACES(vrf, ifp) \
|
||||
if (vrf) \
|
||||
@ -502,7 +527,7 @@ extern struct interface *if_get_by_name(const char *ifname, vrf_id_t vrf_id);
|
||||
extern struct interface *if_get_by_ifindex(ifindex_t ifindex, vrf_id_t vrf_id);
|
||||
|
||||
/* Sets the index and adds to index list */
|
||||
extern void if_set_index(struct interface *ifp, ifindex_t ifindex);
|
||||
extern int if_set_index(struct interface *ifp, ifindex_t ifindex);
|
||||
/* Sets the name and adds to name list */
|
||||
extern void if_set_name(struct interface *ifp, const char *name);
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user