d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rename IFF_RUNNING to IFF_DRV_RUNNING, IFF_OACTIVE to IFF_DRV_OACTIVE,

Browse Source 
and move both flags from ifnet.if_flags to ifnet.if_drv_flags, making
and documenting the locking of these flags the responsibility of the
device driver, not the network stack.  The flags for these two fields
will be mutually exclusive so that they can be exposed to user space as
though they were stored in the same variable.

Provide #defines to provide the old names #ifndef _KERNEL, so that user
applications (such as ifconfig) can use the old flag names.  Using the
old names in a device driver will result in a compile error in order to
help device driver writers adopt the new model.

When exposing the interface flags to user space, via interface ioctls
or routing sockets, or the two fields together.  Since the driver flags
cannot currently be set for user space, no new logic is currently
required to handle this case.

Add some assertions that general purpose network stack routines, such
as if_setflags(), are not improperly used on driver-owned flags.

With this change, a large number of very minor network stack races are
closed, subject to correct device driver locking.  Most were likely
never triggered.

Driver sweep to follow; many thanks to pjd and bz for the line-by-line
review they gave this patch.

Reviewed by:	pjd, bz
MFC after:	7 days
This commit is contained in:
Robert Watson 2005-08-09 10:16:17 +00:00
parent 4c3cffeb64
commit 292ee7be1c
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=148886
4 changed files with 55 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
sys/net/if.c
View File

@ -1029,6 +1029,8 @@ if_unroute(struct ifnet *ifp, int flag, int fam)
{
struct ifaddr *ifa;
KASSERT(flag == IFF_UP, ("if_unroute: flag != IFF_UP"));
ifp->if_flags &= ~flag;
getmicrotime(&ifp->if_lastchange);
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link)
@ -1052,6 +1054,8 @@ if_route(struct ifnet *ifp, int flag, int fam)
{
struct ifaddr *ifa;
KASSERT(flag == IFF_UP, ("if_route: flag != IFF_UP"));
ifp->if_flags |= flag;
getmicrotime(&ifp->if_lastchange);
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link)
@ -1229,7 +1233,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
struct ifreq *ifr;
struct ifstat *ifs;
int error = 0;
int new_flags;
int new_flags, temp_flags;
size_t namelen, onamelen;
char new_name[IFNAMSIZ];
struct ifaddr *ifa;
@ -1242,8 +1246,9 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
break;
case SIOCGIFFLAGS:
ifr->ifr_flags = ifp->if_flags & 0xffff;
ifr->ifr_flagshigh = ifp->if_flags >> 16;
temp_flags = ifp->if_flags | ifp->if_drv_flags;
ifr->ifr_flags = temp_flags & 0xffff;
ifr->ifr_flagshigh = temp_flags >> 16;
break;
case SIOCGIFCAP:
@ -1273,6 +1278,10 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
error = suser(td);
if (error)
return (error);
/*
* Currently, no driver owned flags pass the IFF_CANTCHANGE
* check, so we don't need special handling here yet.
*/
new_flags = (ifr->ifr_flags & 0xffff) |
(ifr->ifr_flagshigh << 16);
if (ifp->if_flags & IFF_SMART) {
@ -1610,10 +1619,12 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct thread *td)
}
/*
* The code common to hadling reference counted flags,
* The code common to handling reference counted flags,
* e.g., in ifpromisc() and if_allmulti().
* The "pflag" argument can specify a permanent mode flag,
* such as IFF_PPROMISC for promiscuous mode; should be 0 if none.
*
* Only to be used on stack-owned flags, not driver-owned flags.
*/
static int
if_setflag(struct ifnet *ifp, int flag, int pflag, int *refcount, int onswitch)
@ -1622,6 +1633,9 @@ if_setflag(struct ifnet *ifp, int flag, int pflag, int *refcount, int onswitch)
int error;
int oldflags, oldcount;
KASSERT((flag & (IFF_DRV_OACTIVE|IFF_DRV_RUNNING)) == 0,
("if_setflag: setting driver-ownded flag %d", flag));
/* Sanity checks to catch programming errors */
if (onswitch) {
if (*refcount < 0) {
@ -2258,7 +2272,7 @@ if_handoff(struct ifqueue *ifq, struct mbuf *m, struct ifnet *ifp, int adjust)
ifp->if_obytes += m->m_pkthdr.len + adjust;
if (m->m_flags & (M_BCAST|M_MCAST))
ifp->if_omcasts++;
active = ifp->if_flags & IFF_OACTIVE;
active = ifp->if_drv_flags & IFF_DRV_OACTIVE;
}
_IF_ENQUEUE(ifq, m);
IF_UNLOCK(ifq);

31
sys/net/if.h
View File

@ -109,17 +109,33 @@ struct if_data {
struct timeval ifi_lastchange; /* time of last administrative change */
};
/*
* Interface flags are of two types: network stack owned flags, and driver
* owned flags. Historically, these values were stored in the same ifnet
* flags field, but with the advent of fine-grained locking, they have been
* broken out such that the network stack is responsible for synchronizing
* the stack-owned fields, and the device driver the device-owned fields.
* Both halves can perform lockless reads of the other half's field, subject
* to accepting the involved races.
*
* Both sets of flags come from the same number space, and should not be
* permitted to conflict, as they are exposed to user space via a single
* field.
*
* For historical reasons, the old flag names for driver flags are exposed to
* user space.
*/
#define IFF_UP 0x1 /* interface is up */
#define IFF_BROADCAST 0x2 /* broadcast address valid */
#define IFF_DEBUG 0x4 /* turn on debugging */
#define IFF_LOOPBACK 0x8 /* is a loopback net */
#define IFF_POINTOPOINT 0x10 /* interface is point-to-point link */
#define IFF_SMART 0x20 /* interface manages own routes */
#define IFF_RUNNING 0x40 /* resources allocated */
#define IFF_DRV_RUNNING 0x40 /* resources allocated */
#define IFF_NOARP 0x80 /* no address resolution protocol */
#define IFF_PROMISC 0x100 /* receive all packets */
#define IFF_ALLMULTI 0x200 /* receive all multicast packets */
#define IFF_OACTIVE 0x400 /* tx hardware queue is full */
#define IFF_DRV_OACTIVE 0x400 /* tx hardware queue is full */
#define IFF_SIMPLEX 0x800 /* can't hear own transmissions */
#define IFF_LINK0 0x1000 /* per link layer defined bit */
#define IFF_LINK1 0x2000 /* per link layer defined bit */
@ -132,9 +148,18 @@ struct if_data {
#define IFF_STATICARP 0x80000 /* static ARP */
#define IFF_NEEDSGIANT 0x100000 /* hold Giant over if_start calls */
/*
* Old names for driver flags so that user space tools can continue to use
* the old names.
*/
#ifndef _KERNEL
#define IFF_RUNNING IFF_DRV_RUNNING
#define IFF_OACTIVE IFF_DRV_OACTIVE
#endif
/* flags set internally only: */
#define IFF_CANTCHANGE \
(IFF_BROADCAST|IFF_POINTOPOINT|IFF_RUNNING|IFF_OACTIVE|\
(IFF_BROADCAST|IFF_POINTOPOINT|IFF_DRV_RUNNING|IFF_DRV_OACTIVE|\
IFF_SIMPLEX|IFF_MULTICAST|IFF_ALLMULTI|IFF_SMART|IFF_PROMISC|\
IFF_POLLING)

6
sys/net/if_var.h
View File

@ -422,6 +422,10 @@ do { \
#define IFQ_INC_DROPS(ifq) ((ifq)->ifq_drops++)
#define IFQ_SET_MAXLEN(ifq, len) ((ifq)->ifq_maxlen = (len))
/*
* The IFF_DRV_OACTIVE test should really occur in the device driver, not in
* the handoff logic, as that flag is locked by the device driver.
*/
#define IFQ_HANDOFF_ADJ(ifp, m, adj, err) \
do { \
int len; \
@ -434,7 +438,7 @@ do { \
(ifp)->if_obytes += len + (adj); \
if (mflags & M_MCAST) \
(ifp)->if_omcasts++; \
if (((ifp)->if_flags & IFF_OACTIVE) == 0) \
if (((ifp)->if_drv_flags & IFF_DRV_OACTIVE) == 0) \
if_start(ifp); \
} \
} while (0)

5
sys/net/rtsock.c
View File

@ -859,7 +859,7 @@ rt_ifmsg(struct ifnet *ifp)
return;
ifm = mtod(m, struct if_msghdr *);
ifm->ifm_index = ifp->if_index;
ifm->ifm_flags = ifp->if_flags;
ifm->ifm_flags = ifp->if_flags | ifp->if_drv_flags;
ifm->ifm_data = ifp->if_data;
ifm->ifm_addrs = 0;
rt_dispatch(m, NULL);
@ -1124,7 +1124,7 @@ sysctl_iflist(int af, struct walkarg *w)
ifm = (struct if_msghdr *)w->w_tmem;
ifm->ifm_index = ifp->if_index;
ifm->ifm_flags = ifp->if_flags;
ifm->ifm_flags = ifp->if_flags | ifp->if_drv_flags;
ifm->ifm_data = ifp->if_data;
ifm->ifm_addrs = info.rti_addrs;
error = SYSCTL_OUT(w->w_req,(caddr_t)ifm, len);
@ -1178,6 +1178,7 @@ sysctl_ifmalist(int af, struct walkarg *w)
continue;
ifa = ifaddr_byindex(ifp->if_index);
info.rti_info[RTAX_IFP] = ifa ? ifa->ifa_addr : NULL;
TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
if (af && af != ifma->ifma_addr->sa_family)
continue;