diff options
author | rwatson <rwatson@FreeBSD.org> | 2008-06-26 23:05:28 +0000 |
---|---|---|
committer | rwatson <rwatson@FreeBSD.org> | 2008-06-26 23:05:28 +0000 |
commit | 46dd6e44fc7f70ee8d82d41fb83bedfb2c7829c8 (patch) | |
tree | 1a406ca586e36376a4f6fc527aef4c5964c174b5 /sys | |
parent | 74854699d26c7fb406a5e0029641de33e278cbce (diff) | |
download | FreeBSD-src-46dd6e44fc7f70ee8d82d41fb83bedfb2c7829c8.zip FreeBSD-src-46dd6e44fc7f70ee8d82d41fb83bedfb2c7829c8.tar.gz |
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
Diffstat (limited to 'sys')
-rw-r--r-- | sys/net/if.c | 81 | ||||
-rw-r--r-- | sys/net/if_var.h | 8 |
2 files changed, 73 insertions, 16 deletions
diff --git a/sys/net/if.c b/sys/net/if.c index f124e7e..afc8243 100644 --- a/sys/net/if.c +++ b/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)) { diff --git a/sys/net/if_var.h b/sys/net/if_var.h index d738e32..2e3dc0d 100644 --- a/sys/net/if_var.h +++ b/sys/net/if_var.h @@ -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; |