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

Lock down netnatm and mark as MPSAFE:

Browse Source 
- Introduce a subsystem mutex, natm_mtx, manipulated with accessor macros
  NATM_LOCK_INIT(), NATM_LOCK(), NATM_UNLOCK(), NATM_LOCK_ASSERT().  It
  protects the consistency of pcb-related data structures.  Finer grained
  locking is possible, but should be done in the context of specific
  measurements (as very little work is done in netnatm -- most is in the
  ATM device driver or socket layer, so there's probably not much
  contention).

- Remove GIANT_REQUIRED, mark as NETISR_MPSAFE, remove
  NET_NEEDS_GIANT("netnatm").

- Conditionally acquire Giant when entering network interfaces for
  ifp->if_ioctl() using IFF_LOCKGIANT(ifp)/IFF_UNLOCKGIANT(ifp) in order
  to coexist with non-MPSAFE atm ifnet drivers..

- De-spl.

MFC after:	2 weeks
Reviewed by:	harti, bms (various versions)
This commit is contained in:
rwatson 2005-07-18 16:55:46 +00:00
parent 32252b633f
commit e11e852d54
5 changed files with 77 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
sys/net/if_atmsubr.c
View File

@ -256,9 +256,6 @@ atm_input(struct ifnet *ifp, struct atm_pseudohdr *ah, struct mbuf *m,
{
int isr;
u_int16_t etype = ETHERTYPE_IP; /* default */
#ifdef NATM
int s;
#endif
if ((ifp->if_flags & IFF_UP) == 0) {
m_freem(m);
@ -284,13 +281,19 @@ atm_input(struct ifnet *ifp, struct atm_pseudohdr *ah, struct mbuf *m,
if (rxhand) {
#ifdef NATM
struct natmpcb *npcb = rxhand;
struct natmpcb *npcb;
s = splimp(); /* in case 2 atm cards @ diff lvls */
/*
* XXXRW: this use of 'rxhand' is not a very good idea, and
* was subject to races even before SMPng due to the release
* of spl here.
*/
NATM_LOCK();
npcb = rxhand;
npcb->npcb_inq++; /* count # in queue */
splx(s);
isr = NETISR_NATM;
m->m_pkthdr.rcvif = rxhand; /* XXX: overload */
NATM_UNLOCK();
#else
printf("atm_input: NATM detected but not "
"configured in kernel\n");

107
sys/netnatm/natm.c
View File

@ -2,6 +2,7 @@
/*-
*
* Copyright (c) 1996 Charles D. Cranor and Washington University.
* Copyright (c) 2005 Robert N. M. Watson
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -67,6 +68,8 @@ static const u_long natm5_recvspace = 16*1024;
static const u_long natm0_sendspace = 16*1024;
static const u_long natm0_recvspace = 16*1024;
struct mtx natm_mtx;
/*
* user requests
*/
@ -93,14 +96,10 @@ natm_usr_attach(struct socket *so, int proto, d_thread_t *p)
{
struct natmpcb *npcb;
int error = 0;
int s = SPLSOFTNET();
npcb = (struct natmpcb *)so->so_pcb;
if (npcb) {
error = EISCONN;
goto out;
}
KASSERT(npcb == NULL, ("natm_usr_attach: so_pcb != NULL"));
if (so->so_snd.sb_hiwat == 0 || so->so_rcv.sb_hiwat == 0) {
if (proto == PROTO_NATMAAL5)
@ -113,8 +112,7 @@ natm_usr_attach(struct socket *so, int proto, d_thread_t *p)
so->so_pcb = (caddr_t) (npcb = npcb_alloc(M_WAITOK));
npcb->npcb_socket = so;
out:
splx(s);
out:
return (error);
}
@ -123,10 +121,11 @@ natm_usr_detach(struct socket *so)
{
struct natmpcb *npcb;
int error = 0;
int s = SPLSOFTNET();
NATM_LOCK();
npcb = (struct natmpcb *)so->so_pcb;
if (npcb == NULL) {
/* XXXRW: Does this case ever actually happen? */
error = EINVAL;
goto out;
}
@ -140,7 +139,7 @@ natm_usr_detach(struct socket *so)
so->so_pcb = NULL;
sotryfree(so);
out:
splx(s);
NATM_UNLOCK();
return (error);
}
@ -152,11 +151,12 @@ natm_usr_connect(struct socket *so, struct sockaddr *nam, d_thread_t *p)
struct atmio_openvcc op;
struct ifnet *ifp;
int error = 0;
int s2, s = SPLSOFTNET();
int proto = so->so_proto->pr_protocol;
NATM_LOCK();
npcb = (struct natmpcb *)so->so_pcb;
if (npcb == NULL) {
/* XXXRW: Does this case ever actually happen? */
error = EINVAL;
goto out;
}
@ -198,6 +198,7 @@ natm_usr_connect(struct socket *so, struct sockaddr *nam, d_thread_t *p)
error = EADDRINUSE;
goto out;
}
NATM_UNLOCK();
/*
* open the channel
@ -211,20 +212,22 @@ natm_usr_connect(struct socket *so, struct sockaddr *nam, d_thread_t *p)
op.param.aal = (proto == PROTO_NATMAAL5) ? ATMIO_AAL_5 : ATMIO_AAL_0;
op.param.traffic = ATMIO_TRAFFIC_UBR;
s2 = splimp();
IFF_LOCKGIANT(ifp);
if (ifp->if_ioctl == NULL ||
ifp->if_ioctl(ifp, SIOCATMOPENVCC, (caddr_t)&op) != 0) {
splx(s2);
IFF_UNLOCKGIANT(ifp);
NATM_LOCK();
npcb_free(npcb, NPCB_REMOVE);
error = EIO;
goto out;
}
splx(s2);
IFF_UNLOCKGIANT(ifp);
NATM_LOCK();
soisconnected(so);
out:
splx(s);
NATM_UNLOCK();
return (error);
}
@ -235,10 +238,11 @@ natm_usr_disconnect(struct socket *so)
struct atmio_closevcc cl;
struct ifnet *ifp;
int error = 0;
int s2, s = SPLSOFTNET();
NATM_LOCK();
npcb = (struct natmpcb *)so->so_pcb;
if (npcb == NULL) {
/* XXXRW: Does this case ever actually happen? */
error = EINVAL;
goto out;
}
@ -255,16 +259,19 @@ natm_usr_disconnect(struct socket *so)
*/
cl.vpi = npcb->npcb_vpi;
cl.vci = npcb->npcb_vci;
s2 = splimp();
if (ifp->if_ioctl != NULL)
NATM_UNLOCK();
if (ifp->if_ioctl != NULL) {
IFF_LOCKGIANT(ifp);
ifp->if_ioctl(ifp, SIOCATMCLOSEVCC, (caddr_t)&cl);
splx(s2);
IFF_UNLOCKGIANT(ifp);
}
NATM_LOCK();
npcb_free(npcb, NPCB_REMOVE);
soisdisconnected(so);
out:
splx(s);
NATM_UNLOCK();
return (error);
}
@ -282,11 +289,12 @@ natm_usr_send(struct socket *so, int flags, struct mbuf *m,
struct natmpcb *npcb;
struct atm_pseudohdr *aph;
int error = 0;
int s = SPLSOFTNET();
int proto = so->so_proto->pr_protocol;
NATM_LOCK();
npcb = (struct natmpcb *)so->so_pcb;
if (npcb == NULL) {
/* XXXRW: Does this case ever actually happen? */
error = EINVAL;
goto out;
}
@ -301,7 +309,7 @@ natm_usr_send(struct socket *so, int flags, struct mbuf *m,
/*
* send the data. we must put an atm_pseudohdr on first
*/
M_PREPEND(m, sizeof(*aph), M_TRYWAIT);
M_PREPEND(m, sizeof(*aph), M_DONTWAIT);
if (m == NULL) {
error = ENOBUFS;
goto out;
@ -314,7 +322,7 @@ natm_usr_send(struct socket *so, int flags, struct mbuf *m,
error = atm_output(npcb->npcb_ifp, m, NULL, NULL);
out:
splx(s);
NATM_UNLOCK();
return (error);
}
@ -323,13 +331,13 @@ natm_usr_peeraddr(struct socket *so, struct sockaddr **nam)
{
struct natmpcb *npcb;
struct sockaddr_natm *snatm, ssnatm;
int error = 0;
int s = SPLSOFTNET();
NATM_LOCK();
npcb = (struct natmpcb *)so->so_pcb;
if (npcb == NULL) {
error = EINVAL;
goto out;
/* XXXRW: Does this case ever actually happen? */
NATM_UNLOCK();
return (EINVAL);
}
snatm = &ssnatm;
@ -340,11 +348,9 @@ natm_usr_peeraddr(struct socket *so, struct sockaddr **nam)
sizeof(snatm->snatm_if));
snatm->snatm_vci = npcb->npcb_vci;
snatm->snatm_vpi = npcb->npcb_vpi;
*nam = sodupsockaddr((struct sockaddr *)snatm, M_NOWAIT);
out:
splx(s);
return (error);
NATM_UNLOCK();
*nam = sodupsockaddr((struct sockaddr *)snatm, M_WAITOK);
return (0);
}
static int
@ -352,24 +358,22 @@ natm_usr_control(struct socket *so, u_long cmd, caddr_t arg,
struct ifnet *ifp, d_thread_t *p)
{
struct natmpcb *npcb;
int error = 0;
int s = SPLSOFTNET();
int error;
/*
* XXXRW: Does this case ever actually happen? And does it even matter
* given that npcb is unused?
*/
npcb = (struct natmpcb *)so->so_pcb;
if (npcb == NULL) {
error = EINVAL;
goto out;
}
if (npcb == NULL)
return (EINVAL);
splx(s);
if (ifp == NULL || ifp->if_ioctl == NULL) {
error = EOPNOTSUPP;
goto out;
}
return ((*ifp->if_ioctl)(ifp, cmd, arg));
if (ifp == NULL || ifp->if_ioctl == NULL)
return (EOPNOTSUPP);
out:
splx(s);
IFF_LOCKGIANT(ifp);
error = ((*ifp->if_ioctl)(ifp, cmd, arg));
IFF_UNLOCKGIANT(ifp);
return (error);
}
@ -407,6 +411,7 @@ struct pr_usrreqs natm_usrreqs = {
};
#else /* !FREEBSD_USRREQS */
#error "!FREEBSD_USRREQS not implemented - locking"
#if defined(__NetBSD__) || defined(__OpenBSD__)
int natm_usrreq(so, req, m, nam, control, p)
@ -690,31 +695,29 @@ struct proc *p;
void
natmintr(struct mbuf *m)
{
int s;
struct socket *so;
struct natmpcb *npcb;
GIANT_REQUIRED;
#ifdef DIAGNOSTIC
M_ASSERTPKTHDR(m);
#endif
NATM_LOCK();
npcb = (struct natmpcb *)m->m_pkthdr.rcvif; /* XXX: overloaded */
so = npcb->npcb_socket;
s = splimp(); /* could have atm devs @ different levels */
npcb->npcb_inq--;
splx(s);
if (npcb->npcb_flags & NPCB_DRAIN) {
m_freem(m);
if (npcb->npcb_inq == 0)
FREE(npcb, M_PCB); /* done! */
NATM_UNLOCK();
m_freem(m);
return;
}
if (npcb->npcb_flags & NPCB_FREE) {
NATM_UNLOCK();
m_freem(m); /* drop */
return;
}
@ -734,11 +737,13 @@ natmintr(struct mbuf *m)
#endif
sbappendrecord(&so->so_rcv, m);
sorwakeup(so);
NATM_UNLOCK();
} else {
#ifdef NATM_STAT
natm_sodropcnt++;
natm_sodropbytes += m->m_pkthdr.len;
#endif
NATM_UNLOCK();
m_freem(m);
}
}

7
sys/netnatm/natm.h
View File

@ -95,6 +95,7 @@ LIST_HEAD(npcblist, natmpcb);
/* global data structures */
extern struct mtx natm_mtx; /* global netnatm lock */
extern struct npcblist natm_pcbs; /* global list of pcbs */
#define NATM_STAT
#ifdef NATM_STAT
@ -104,6 +105,12 @@ extern u_int natm_sookcnt;
extern u_int natm_sookbytes; /* account of ok */
#endif
/* locking macros */
#define NATM_LOCK_INIT() mtx_init(&natm_mtx, "natm_mtx", NULL, MTX_DEF)
#define NATM_LOCK() mtx_lock(&natm_mtx)
#define NATM_UNLOCK() mtx_unlock(&natm_mtx)
#define NATM_LOCK_ASSERT() mtx_assert(&natm_mtx, MA_OWNED)
/* external functions */
/* natm_pcb.c */

8
sys/netnatm/natm_pcb.c
View File

@ -81,7 +81,8 @@ npcb_alloc(int wait)
void
npcb_free(struct natmpcb *npcb, int op)
{
int s = splimp();
NATM_LOCK_ASSERT();
if ((npcb->npcb_flags & NPCB_FREE) == 0) {
LIST_REMOVE(npcb, pcblist);
@ -94,8 +95,6 @@ npcb_free(struct natmpcb *npcb, int op)
FREE(npcb, M_PCB); /* kill it! */
}
}
splx(s);
}
@ -107,8 +106,8 @@ struct natmpcb *
npcb_add(struct natmpcb *npcb, struct ifnet *ifp, u_int16_t vci, u_int8_t vpi)
{
struct natmpcb *cpcb = NULL; /* current pcb */
int s = splimp();
NATM_LOCK_ASSERT();
/*
* lookup required
@ -147,7 +146,6 @@ npcb_add(struct natmpcb *npcb, struct ifnet *ifp, u_int16_t vci, u_int8_t vpi)
LIST_INSERT_HEAD(&natm_pcbs, cpcb, pcblist);
done:
splx(s);
return (cpcb);
}

5
sys/netnatm/natm_proto.c
View File

@ -56,8 +56,6 @@ extern struct domain natmdomain;
static void natm_init(void);
NET_NEEDS_GIANT("netnatm");
static struct protosw natmsw[] = {
{ SOCK_STREAM, &natmdomain, PROTO_NATMAAL5, PR_CONNREQUIRED,
0, 0, 0, 0,
@ -123,8 +121,9 @@ natm_init(void)
LIST_INIT(&natm_pcbs);
bzero(&natmintrq, sizeof(natmintrq));
natmintrq.ifq_maxlen = natmqmaxlen;
NATM_LOCK_INIT();
mtx_init(&natmintrq.ifq_mtx, "natm_inq", NULL, MTX_DEF);
netisr_register(NETISR_NATM, natmintr, &natmintrq, 0);
netisr_register(NETISR_NATM, natmintr, &natmintrq, NETISR_MPSAFE);
}
#if defined(__FreeBSD__)