Introduce locking around use of ifindex_table, whose use was previously
unsynchronized. While races were extremely rare, we've now had a couple of reports of panics in environments involving large numbers of IPSEC tunnels being added very quickly on an active system. - Add accessor functions ifnet_byindex(), ifaddr_byindex(), ifdev_byindex() to replace existing accessor macros. These functions now acquire the ifnet lock before derefencing the table. - Add IFNET_WLOCK_ASSERT(). - Add static accessor functions ifnet_setbyindex(), ifdev_setbyindex(), which set values in the table either asserting of acquiring the ifnet lock. - Use accessor functions throughout if.c to modify and read ifindex_table. - Rework ifnet attach/detach to lock around ifindex_table modification. Note that these changes simply close races around use of ifindex_table, and make no attempt to solve the probem of disappearing ifnets. Further refinement of this work, including with respect to ifindex_table resizing, is still required. In a future change, the ifnet lock should be converted from a mutex to an rwlock in order to reduce contention. Reviewed and tested by: brooks
This commit is contained in:
parent
a54eadd8c4
commit
02f4879d3a
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=180042
81
sys/net/if.c
81
sys/net/if.c
@ -133,7 +133,6 @@ extern void nd6_setmtu(struct ifnet *);
|
||||
#endif
|
||||
|
||||
int if_index = 0;
|
||||
struct ifindex_entry *ifindex_table = NULL;
|
||||
int ifqmaxlen = IFQ_MAXLEN;
|
||||
struct ifnethead ifnet; /* depend on static init XXX */
|
||||
struct ifgrouphead ifg_head;
|
||||
@ -144,6 +143,11 @@ static if_com_free_t *if_com_free[256];
|
||||
static int if_indexlim = 8;
|
||||
static struct knlist ifklist;
|
||||
|
||||
/*
|
||||
* Table of ifnet/cdev by index. Locked with ifnet_lock.
|
||||
*/
|
||||
static struct ifindex_entry *ifindex_table = NULL;
|
||||
|
||||
static void filt_netdetach(struct knote *kn);
|
||||
static int filt_netdev(struct knote *kn, long hint);
|
||||
|
||||
@ -160,6 +164,57 @@ MALLOC_DEFINE(M_IFNET, "ifnet", "interface internals");
|
||||
MALLOC_DEFINE(M_IFADDR, "ifaddr", "interface address");
|
||||
MALLOC_DEFINE(M_IFMADDR, "ether_multi", "link-level multicast address");
|
||||
|
||||
struct ifnet *
|
||||
ifnet_byindex(u_short idx)
|
||||
{
|
||||
struct ifnet *ifp;
|
||||
|
||||
IFNET_RLOCK();
|
||||
ifp = ifindex_table[idx].ife_ifnet;
|
||||
IFNET_RUNLOCK();
|
||||
return (ifp);
|
||||
}
|
||||
|
||||
static void
|
||||
ifnet_setbyindex(u_short idx, struct ifnet *ifp)
|
||||
{
|
||||
|
||||
IFNET_WLOCK_ASSERT();
|
||||
|
||||
ifindex_table[idx].ife_ifnet = ifp;
|
||||
}
|
||||
|
||||
struct ifaddr *
|
||||
ifaddr_byindex(u_short idx)
|
||||
{
|
||||
struct ifaddr *ifa;
|
||||
|
||||
IFNET_RLOCK();
|
||||
ifa = ifnet_byindex(idx)->if_addr;
|
||||
IFNET_RUNLOCK();
|
||||
return (ifa);
|
||||
}
|
||||
|
||||
struct cdev *
|
||||
ifdev_byindex(u_short idx)
|
||||
{
|
||||
struct cdev *cdev;
|
||||
|
||||
IFNET_RLOCK();
|
||||
cdev = ifindex_table[idx].ife_dev;
|
||||
IFNET_RUNLOCK();
|
||||
return (cdev);
|
||||
}
|
||||
|
||||
static void
|
||||
ifdev_setbyindex(u_short idx, struct cdev *cdev)
|
||||
{
|
||||
|
||||
IFNET_WLOCK();
|
||||
ifindex_table[idx].ife_dev = cdev;
|
||||
IFNET_WUNLOCK();
|
||||
}
|
||||
|
||||
static d_open_t netopen;
|
||||
static d_close_t netclose;
|
||||
static d_ioctl_t netioctl;
|
||||
@ -298,8 +353,8 @@ if_init(void *dummy __unused)
|
||||
TAILQ_INIT(&ifg_head);
|
||||
knlist_init(&ifklist, NULL, NULL, NULL, NULL);
|
||||
if_grow(); /* create initial table */
|
||||
ifdev_byindex(0) = make_dev(&net_cdevsw, 0,
|
||||
UID_ROOT, GID_WHEEL, 0600, "network");
|
||||
ifdev_setbyindex(0, make_dev(&net_cdevsw, 0, UID_ROOT, GID_WHEEL,
|
||||
0600, "network"));
|
||||
if_clone_init();
|
||||
}
|
||||
|
||||
@ -360,7 +415,9 @@ if_alloc(u_char type)
|
||||
return (NULL);
|
||||
}
|
||||
}
|
||||
ifnet_byindex(ifp->if_index) = ifp;
|
||||
IFNET_WLOCK();
|
||||
ifnet_setbyindex(ifp->if_index, ifp);
|
||||
IFNET_WUNLOCK();
|
||||
IF_ADDR_LOCK_INIT(ifp);
|
||||
|
||||
return (ifp);
|
||||
@ -394,17 +451,18 @@ if_free_type(struct ifnet *ifp, u_char type)
|
||||
return;
|
||||
}
|
||||
|
||||
IF_ADDR_LOCK_DESTROY(ifp);
|
||||
|
||||
ifnet_byindex(ifp->if_index) = NULL;
|
||||
IFNET_WLOCK();
|
||||
ifnet_setbyindex(ifp->if_index, NULL);
|
||||
|
||||
/* XXX: should be locked with if_findindex() */
|
||||
while (if_index > 0 && ifnet_byindex(if_index) == NULL)
|
||||
if_index--;
|
||||
IFNET_WUNLOCK();
|
||||
|
||||
if (if_com_free[type] != NULL)
|
||||
if_com_free[type](ifp->if_l2com, type);
|
||||
|
||||
IF_ADDR_LOCK_DESTROY(ifp);
|
||||
free(ifp, M_IFNET);
|
||||
};
|
||||
|
||||
@ -454,10 +512,9 @@ if_attach(struct ifnet *ifp)
|
||||
mac_ifnet_create(ifp);
|
||||
#endif
|
||||
|
||||
ifdev_byindex(ifp->if_index) = make_dev(&net_cdevsw,
|
||||
unit2minor(ifp->if_index),
|
||||
UID_ROOT, GID_WHEEL, 0600, "%s/%s",
|
||||
net_cdevsw.d_name, ifp->if_xname);
|
||||
ifdev_setbyindex(ifp->if_index, make_dev(&net_cdevsw,
|
||||
unit2minor(ifp->if_index), UID_ROOT, GID_WHEEL, 0600, "%s/%s",
|
||||
net_cdevsw.d_name, ifp->if_xname));
|
||||
make_dev_alias(ifdev_byindex(ifp->if_index), "%s%d",
|
||||
net_cdevsw.d_name, ifp->if_index);
|
||||
|
||||
@ -706,7 +763,7 @@ if_detach(struct ifnet *ifp)
|
||||
*/
|
||||
ifp->if_addr = NULL;
|
||||
destroy_dev(ifdev_byindex(ifp->if_index));
|
||||
ifdev_byindex(ifp->if_index) = NULL;
|
||||
ifdev_setbyindex(ifp->if_index, NULL);
|
||||
|
||||
/* We can now free link ifaddr. */
|
||||
if (!TAILQ_EMPTY(&ifp->if_addrhead)) {
|
||||
|
@ -636,6 +636,7 @@ extern struct mtx ifnet_lock;
|
||||
mtx_init(&ifnet_lock, "ifnet", NULL, MTX_DEF | MTX_RECURSE)
|
||||
#define IFNET_WLOCK() mtx_lock(&ifnet_lock)
|
||||
#define IFNET_WUNLOCK() mtx_unlock(&ifnet_lock)
|
||||
#define IFNET_WLOCK_ASSERT() mtx_assert(&ifnet_lock, MA_OWNED)
|
||||
#define IFNET_RLOCK() IFNET_WLOCK()
|
||||
#define IFNET_RUNLOCK() IFNET_WUNLOCK()
|
||||
|
||||
@ -644,17 +645,16 @@ struct ifindex_entry {
|
||||
struct cdev *ife_dev;
|
||||
};
|
||||
|
||||
#define ifnet_byindex(idx) ifindex_table[(idx)].ife_ifnet
|
||||
struct ifnet *ifnet_byindex(u_short idx);
|
||||
/*
|
||||
* Given the index, ifaddr_byindex() returns the one and only
|
||||
* link-level ifaddr for the interface. You are not supposed to use
|
||||
* it to traverse the list of addresses associated to the interface.
|
||||
*/
|
||||
#define ifaddr_byindex(idx) ifnet_byindex(idx)->if_addr
|
||||
#define ifdev_byindex(idx) ifindex_table[(idx)].ife_dev
|
||||
struct ifaddr *ifaddr_byindex(u_short idx);
|
||||
struct cdev *ifdev_byindex(u_short idx);
|
||||
|
||||
extern struct ifnethead ifnet;
|
||||
extern struct ifindex_entry *ifindex_table;
|
||||
extern int ifqmaxlen;
|
||||
extern struct ifnet *loif; /* first loopback interface */
|
||||
extern int if_index;
|
||||
|
Loading…
Reference in New Issue
Block a user