Subject: Re: fixing send(2) semantics (kern/29750)
To: None <tech-kern@netbsd.org>
From: Christos Zoulas <christos@tac.gw.com>
List: tech-kern
Date: 03/27/2005 18:03:25
In article <20050326235313.E48372AC9C@beowulf.gw.com>,
Christos Zoulas <christos@zoulas.com> wrote:
>
> Since all the non-tunneling interfaces do this, or a buggy version
> of this. Then you can have a different version for the tunnelling
> interfaces (bridge, gif, etc.). This will cleanup the code significantly.
>
> Also the ieee1394 appears to freeing the mbuf on failure, or am I
> mis-reading this?
Here's a patch that centralizes most of the IFQ_ENQUEUE uses into two
functions saving n copies of the same, or slightly altered incorrectly
code.
Index: if.c
===================================================================
RCS file: /cvsroot/src/sys/net/if.c,v
retrieving revision 1.154
diff -u -u -r1.154 if.c
--- if.c 26 Feb 2005 22:45:09 -0000 1.154
+++ if.c 27 Mar 2005 22:58:06 -0000
@@ -1728,6 +1728,66 @@
return (error);
}
+/*
+ * Queue message on interface, and start output if interface
+ * not yet active.
+ */
+int
+ifq_enqueue(struct ifnet *ifp, struct mbuf *m
+ ALTQ_COMMA ALTQ_DECL(struct altq_pktattr *pktattr))
+{
+ int len = m->m_pkthdr.len;
+ int mflags = m->m_flags;
+ int s = splnet();
+ int error;
+
+ IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
+ if (error) {
+ splx(s);
+ return error;
+ }
+ ifp->if_obytes += len;
+ if (mflags & M_MCAST)
+ ifp->if_omcasts++;
+ if ((ifp->if_flags & IFF_OACTIVE) == 0)
+ (*ifp->if_start)(ifp);
+ splx(s);
+ return error;
+}
+
+/*
+ * Queue message on interface, possibly using a second fast queue
+ */
+int
+ifq_enqueue2(struct ifnet *ifp, struct ifqueue *ifq, struct mbuf *m
+ ALTQ_COMMA ALTQ_DECL(struct altq_pktattr *pktattr))
+{
+ int error = 0;
+
+ if (ifq != NULL
+#ifdef ALTQ
+ && ALTQ_IS_ENABLED(&ifp->if_snd) == 0
+#endif
+ ) {
+ if (IF_QFULL(ifq)) {
+ IF_DROP(&ifp->if_snd);
+ m_freem(m);
+ if (error == 0)
+ error = ENOBUFS;
+ }
+ else
+ IF_ENQUEUE(ifq, m);
+ } else
+ IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
+ if (error != 0) {
+ ++ifp->if_oerrors;
+ return error;
+ }
+
+ return 0;
+}
+
+
#if defined(INET) || defined(INET6)
static void
sysctl_net_ifq_setup(struct sysctllog **clog,
Index: if.h
===================================================================
RCS file: /cvsroot/src/sys/net/if.h,v
retrieving revision 1.106
diff -u -u -r1.106 if.h
--- if.h 20 Mar 2005 07:47:29 -0000 1.106
+++ if.h 27 Mar 2005 22:58:07 -0000
@@ -645,6 +645,7 @@
#ifdef ALTQ
#define ALTQ_DECL(x) x
+#define ALTQ_COMMA ,
#define IFQ_ENQUEUE(ifq, m, pattr, err) \
do { \
@@ -708,6 +709,7 @@
} while (/*CONSTCOND*/ 0)
#else /* ! ALTQ */
#define ALTQ_DECL(x) /* nothing */
+#define ALTQ_COMMA
#define IFQ_ENQUEUE(ifq, m, pattr, err) \
do { \
@@ -788,6 +790,11 @@
int if_clone_create __P((const char *));
int if_clone_destroy __P((const char *));
+int ifq_enqueue(struct ifnet *, struct mbuf * ALTQ_COMMA
+ ALTQ_DECL(struct altq_pktattr *));
+int ifq_enqueue2(struct ifnet *, struct ifqueue *, struct mbuf * ALTQ_COMMA
+ ALTQ_DECL(struct altq_pktattr *));
+
int loioctl __P((struct ifnet *, u_long, caddr_t));
void loopattach __P((int));
int looutput __P((struct ifnet *,
Index: if_arcsubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_arcsubr.c,v
retrieving revision 1.46
diff -u -u -r1.46 if_arcsubr.c
--- if_arcsubr.c 26 Feb 2005 22:45:09 -0000 1.46
+++ if_arcsubr.c 27 Mar 2005 22:58:08 -0000
@@ -125,7 +125,7 @@
struct arccom *ac;
struct arc_header *ah;
struct arphdr *arph;
- int s, error, newencoding, len;
+ int error, newencoding;
u_int8_t atype, adst, myself;
int tfrags, sflag, fsflag, rsflag;
ALTQ_DECL(struct altq_pktattr pktattr;)
@@ -298,22 +298,9 @@
ah->arc_flag = rsflag;
ah->arc_seqid = ac->ac_seqid;
- len = m->m_pkthdr.len;
- s = splnet();
- /*
- * Queue message on interface, and start output if
- * interface not yet active.
- */
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
- /* mbuf is already freed */
- splx(s);
+ if ((error = ifq_enqueue(ifp, m ALTQ_COMMA
+ ALTQ_DECL(&pktattr))) != 0)
return (error);
- }
- ifp->if_obytes += len;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
m = m1;
sflag += 2;
@@ -360,24 +347,7 @@
ah->arc_shost = myself;
}
- len = m->m_pkthdr.len;
- s = splnet();
- /*
- * Queue message on interface, and start output if interface
- * not yet active.
- */
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
- /* mbuf is already freed */
- splx(s);
- return (error);
- }
- ifp->if_obytes += len;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
-
- return (error);
+ return ifq_enqueue(ifp, m ALTQ_COMMA ALTQ_DECL(&pktattr));
bad:
if (m1)
Index: if_atmsubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_atmsubr.c,v
retrieving revision 1.34
diff -u -u -r1.34 if_atmsubr.c
--- if_atmsubr.c 26 Feb 2005 22:45:09 -0000 1.34
+++ if_atmsubr.c 27 Mar 2005 22:58:08 -0000
@@ -105,7 +105,7 @@
struct rtentry *rt0;
{
u_int16_t etype = 0; /* if using LLC/SNAP */
- int s, error = 0, sz, len;
+ int error = 0, sz;
struct atm_pseudohdr atmdst, *ad;
struct mbuf *m = m0;
struct rtentry *rt;
@@ -223,23 +223,7 @@
}
}
- /*
- * Queue message on interface, and start output if interface
- * not yet active.
- */
-
- len = m->m_pkthdr.len;
- s = splnet();
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
- splx(s);
- return (error);
- }
- ifp->if_obytes += len;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
- return (error);
+ return ifq_enqueue(ifp, m ALTQ_COMMA ALTQ_DECL(&pktattr));
bad:
if (m)
Index: if_ecosubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ecosubr.c,v
retrieving revision 1.15
diff -u -u -r1.15 if_ecosubr.c
--- if_ecosubr.c 26 Feb 2005 22:45:09 -0000 1.15
+++ if_ecosubr.c 27 Mar 2005 22:58:08 -0000
@@ -175,11 +175,10 @@
struct rtentry *rt0)
{
struct eco_header ehdr, *eh;
- int error, s;
+ int error;
struct mbuf *m = m0, *mcopy = NULL;
struct rtentry *rt;
int hdrcmplt;
- size_t len;
int delay, count;
struct m_tag *mtag;
struct eco_retryparms *erp;
@@ -334,23 +333,7 @@
return (0);
#endif
- /*
- * Queue message on interface, and start output if interface
- * not yet active.
- */
-
- len = m->m_pkthdr.len;
- s = splnet();
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
- splx(s);
- return (error);
- }
- ifp->if_obytes += len;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
- return (error);
+ return ifq_enqueue(&ifp->if_snd, m ALTQ_COMMA ALTQ_DECL(&pktattr));
bad:
if (m)
@@ -882,22 +865,11 @@
struct eco_retry *er = arg;
struct mbuf *m;
struct ifnet *ifp;
- int s, error, len;
ifp = er->er_ifp;
m = er->er_packet;
len = m->m_pkthdr.len;
LIST_REMOVE(er, er_link);
- s = splnet();
- IFQ_ENQUEUE(&ifp->if_snd, m, NULL, error);
- if (error) {
- splx(s);
- /* XXX should defer again? */
- m_freem(m);
- }
- ifp->if_obytes += len;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
+ (void)ifq_enqueue(ifp, m ALTQ_COMMA ALTQ_DECL(NULL));
FREE(er, M_TEMP);
}
Index: if_ethersubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ethersubr.c,v
retrieving revision 1.121
diff -u -u -r1.121 if_ethersubr.c
--- if_ethersubr.c 18 Mar 2005 11:11:50 -0000 1.121
+++ if_ethersubr.c 27 Mar 2005 22:58:09 -0000
@@ -205,7 +205,7 @@
struct rtentry *rt0)
{
u_int16_t etype = 0;
- int s, len, error = 0, hdrcmplt = 0;
+ int error = 0, hdrcmplt = 0;
u_char esrc[6], edst[6];
struct mbuf *m = m0;
struct rtentry *rt;
@@ -218,7 +218,6 @@
#ifdef NETATALK
struct at_ifaddr *aa;
#endif /* NETATALK */
- short mflags;
#ifdef MBUFTRACE
m_claimm(m, ifp->if_mowner);
@@ -539,26 +538,7 @@
altq_etherclassify(&ifp->if_snd, m, &pktattr);
#endif
- mflags = m->m_flags;
- len = m->m_pkthdr.len;
- s = splnet();
- /*
- * Queue message on interface, and start output if interface
- * not yet active.
- */
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
- /* mbuf is already freed */
- splx(s);
- return (error);
- }
- ifp->if_obytes += len;
- if (mflags & M_MCAST)
- ifp->if_omcasts++;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
- return (error);
+ return ifq_enqueue(ifp, m ALTQ_COMMA ALTQ_DECL(&pktattr));
bad:
if (m)
Index: if_fddisubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_fddisubr.c,v
retrieving revision 1.54
diff -u -u -r1.54 if_fddisubr.c
--- if_fddisubr.c 26 Feb 2005 22:45:09 -0000 1.54
+++ if_fddisubr.c 27 Mar 2005 22:58:10 -0000
@@ -221,14 +221,13 @@
struct rtentry *rt0;
{
u_int16_t etype;
- int s, len, error = 0, hdrcmplt = 0;
+ int error = 0, hdrcmplt = 0;
u_char esrc[6], edst[6];
struct mbuf *m = m0;
struct rtentry *rt;
struct fddi_header *fh;
struct mbuf *mcopy = (struct mbuf *)0;
ALTQ_DECL(struct altq_pktattr pktattr;)
- short mflags;
MCLAIM(m, ifp->if_mowner);
if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING))
@@ -564,26 +563,8 @@
else
bcopy((caddr_t)FDDIADDR(ifp), (caddr_t)fh->fddi_shost,
sizeof(fh->fddi_shost));
- mflags = m->m_flags;
- len = m->m_pkthdr.len;
- s = splnet();
- /*
- * Queue message on interface, and start output if interface
- * not yet active.
- */
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
- /* mbuf is already freed */
- splx(s);
- return (error);
- }
- ifp->if_obytes += len;
- if (mflags & M_MCAST)
- ifp->if_omcasts++;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
- return (error);
+
+ return ifq_enqueue(ifp, m ALTQ_COMMA ALTQ_DECL(&pktattr));
bad:
if (m)
Index: if_hippisubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_hippisubr.c,v
retrieving revision 1.18
diff -u -u -r1.18 if_hippisubr.c
--- if_hippisubr.c 26 Feb 2005 22:45:09 -0000 1.18
+++ if_hippisubr.c 27 Mar 2005 22:58:10 -0000
@@ -96,7 +96,7 @@
{
u_int16_t htype;
u_int32_t ifield = 0;
- int s, len, error = 0;
+ int error = 0;
struct mbuf *m = m0;
struct rtentry *rt;
struct hippi_header *hh;
@@ -226,23 +226,7 @@
m_copyback(m, m->m_pkthdr.len, 8 - d2_len % 8, (caddr_t) buffer);
}
- len = m->m_pkthdr.len;
- s = splnet();
- /*
- * Queue message on interface, and start output if interface
- * not yet active.
- */
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
- /* mbuf is already free */
- splx(s);
- return (error);
- }
- ifp->if_obytes += len;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
- return (error);
+ return ifq_enqueue(ifp, m ALTQ_COMMA ALTQ_DECL(&pktattr));
bad:
if (m)
Index: if_ppp.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ppp.c,v
retrieving revision 1.96
diff -u -u -r1.96 if_ppp.c
--- if_ppp.c 26 Feb 2005 22:45:09 -0000 1.96
+++ if_ppp.c 27 Mar 2005 22:58:11 -0000
@@ -1024,24 +1024,10 @@
m0->m_nextpkt = NULL;
sc->sc_npqtail = &m0->m_nextpkt;
} else {
- if ((m0->m_flags & M_HIGHPRI)
-#ifdef ALTQ
- && ALTQ_IS_ENABLED(&sc->sc_if.if_snd) == 0
-#endif
- ) {
- ifq = &sc->sc_fastq;
- if (IF_QFULL(ifq) && dst->sa_family != AF_UNSPEC) {
- IF_DROP(ifq);
- splx(s);
- error = ENOBUFS;
- goto bad;
- } else {
- IF_ENQUEUE(ifq, m0);
- error = 0;
- }
- } else
- IFQ_ENQUEUE(&sc->sc_if.if_snd, m0, &pktattr, error);
- if (error) {
+ if (m0->m_flags & M_HIGHPRI)
+ ifq = &sc->sc_fastq;
+ if ((error = ifq_enqueue2(&sc->sc_if, ifq, m0
+ ALTQ_COMMA ALTQ_DECL(&pktattr))) != 0) {
splx(s);
sc->sc_if.if_oerrors++;
sc->sc_stats.ppp_oerrors++;
@@ -1093,23 +1079,10 @@
*/
*mpp = m->m_nextpkt;
m->m_nextpkt = NULL;
- if ((m->m_flags & M_HIGHPRI)
-#ifdef ALTQ
- && ALTQ_IS_ENABLED(&sc->sc_if.if_snd) == 0
-#endif
- ) {
+ if (m->m_flags & M_HIGHPRI)
ifq = &sc->sc_fastq;
- if (IF_QFULL(ifq)) {
- IF_DROP(ifq);
- m_freem(m);
- error = ENOBUFS;
- } else {
- IF_ENQUEUE(ifq, m);
- error = 0;
- }
- } else
- IFQ_ENQUEUE(&sc->sc_if.if_snd, m, NULL, error);
- if (error) {
+ if ((error = ifq_enqueue2(&sc->sc_if, ifq, m ALTQ_COMMA
+ ALTQ_DECL(NULL))) != 0) {
sc->sc_if.if_oerrors++;
sc->sc_stats.ppp_oerrors++;
}
Index: if_sl.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_sl.c,v
retrieving revision 1.90
diff -u -u -r1.90 if_sl.c
--- if_sl.c 6 Dec 2004 02:59:23 -0000 1.90
+++ if_sl.c 27 Mar 2005 22:58:12 -0000
@@ -433,6 +433,7 @@
{
struct sl_softc *sc = ifp->if_softc;
struct ip *ip;
+ struct ifqueue *ifq = NULL;
int s, error;
ALTQ_DECL(struct altq_pktattr pktattr;)
@@ -483,26 +484,13 @@
s = splnet();
#ifdef INET
- if ((ip->ip_tos & IPTOS_LOWDELAY) != 0
-#ifdef ALTQ
- && ALTQ_IS_ENABLED(&ifp->if_snd) == 0
-#endif
- ) {
- if (IF_QFULL(&sc->sc_fastq)) {
- IF_DROP(&sc->sc_fastq);
- m_freem(m);
- error = ENOBUFS;
- } else {
- IF_ENQUEUE(&sc->sc_fastq, m);
- error = 0;
- }
- } else
+ if ((ip->ip_tos & IPTOS_LOWDELAY) != 0)
+ ifq = &sc->sc_fastq;
#endif
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
+ if ((error = ifq_enqueue2(ifp, ifq, m ALTQ_COMMA
+ ALTQ_DECL(&pktattr))) != 0) {
splx(s);
- ifp->if_oerrors++;
- return (error);
+ return error;
}
sc->sc_lastpacket = time;
splx(s);
Index: if_spppsubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_spppsubr.c,v
retrieving revision 1.82
diff -u -u -r1.82 if_spppsubr.c
--- if_spppsubr.c 26 Feb 2005 22:45:09 -0000 1.82
+++ if_spppsubr.c 27 Mar 2005 22:58:14 -0000
@@ -689,7 +689,7 @@
struct sppp *sp = (struct sppp *) ifp;
struct ppp_header *h = NULL;
struct ifqueue *ifq = NULL; /* XXX */
- int s, len, rv = 0;
+ int s, error = 0;
u_int16_t protocol;
ALTQ_DECL(struct altq_pktattr pktattr;)
@@ -821,7 +821,7 @@
*/
protocol = htons(PPP_IP);
if (sp->state[IDX_IPCP] != STATE_OPENED)
- rv = ENETDOWN;
+ error = ENETDOWN;
}
break;
#endif
@@ -841,7 +841,7 @@
*/
protocol = htons(PPP_IPV6);
if (sp->state[IDX_IPV6CP] != STATE_OPENED)
- rv = ENETDOWN;
+ error = ENETDOWN;
}
break;
#endif
@@ -887,43 +887,21 @@
h->protocol = protocol;
}
- /*
- * Queue message on interface, and start output if interface
- * not yet active.
- */
- len = m->m_pkthdr.len;
- if (ifq != NULL
-#ifdef ALTQ
- && ALTQ_IS_ENABLED(&ifp->if_snd) == 0
-#endif
- ) {
- if (IF_QFULL(ifq)) {
- IF_DROP(&ifp->if_snd);
- m_freem(m);
- if (rv == 0)
- rv = ENOBUFS;
- }
- else
- IF_ENQUEUE(ifq, m);
- } else
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, rv);
- if (rv != 0) {
- ++ifp->if_oerrors;
- splx(s);
- return (rv);
- }
- if (! (ifp->if_flags & IFF_OACTIVE))
- (*ifp->if_start)(ifp);
+ error = ifq_enqueue2(ifp, ifq, m ALTQ_COMMA ALTQ_DECL(&pktattr));
- /*
- * Count output packets and bytes.
- * The packet length includes header + additional hardware framing
- * according to RFC 1333.
- */
- ifp->if_obytes += len + sp->pp_framebytes;
+ if (error == 0) {
+ /*
+ * Count output packets and bytes.
+ * The packet length includes header + additional hardware
+ * framing according to RFC 1333.
+ */
+ if (!(ifp->if_flags & IFF_OACTIVE))
+ (*ifp->if_start)(ifp);
+ ifp->if_obytes += m->m_pkthdr.len + sp->pp_framebytes;
+ }
splx(s);
- return (0);
+ return error;
}
void
Index: if_strip.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_strip.c,v
retrieving revision 1.58
diff -u -u -r1.58 if_strip.c
--- if_strip.c 26 Feb 2005 22:45:09 -0000 1.58
+++ if_strip.c 27 Mar 2005 22:58:16 -0000
@@ -871,21 +871,10 @@
splx(s);
s = splnet();
- if (ifq != NULL) {
- if (IF_QFULL(ifq)) {
- IF_DROP(ifq);
- m_freem(m);
- error = ENOBUFS;
- } else {
- IF_ENQUEUE(ifq, m);
- error = 0;
- }
- } else
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
+ if ((error = ifq_enqueue2(ifp, ifq, m ALTQ_COMMA
+ ALTQ_DECL(&pktattr))) != 0) {
splx(s);
- ifp->if_oerrors++;
- return (error);
+ return error;
}
sc->sc_lastpacket = time;
splx(s);
Index: if_tokensubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_tokensubr.c,v
retrieving revision 1.30
diff -u -u -r1.30 if_tokensubr.c
--- if_tokensubr.c 26 Feb 2005 22:45:09 -0000 1.30
+++ if_tokensubr.c 27 Mar 2005 22:58:16 -0000
@@ -213,7 +213,7 @@
struct rtentry *rt0;
{
u_int16_t etype;
- int s, len, error = 0;
+ int error = 0;
u_char edst[ISO88025_ADDR_LEN];
struct mbuf *m = m0;
struct rtentry *rt;
@@ -226,7 +226,6 @@
struct token_rif bcastrif;
size_t riflen = 0;
ALTQ_DECL(struct altq_pktattr pktattr;)
- short mflags;
if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING))
senderr(ENETDOWN);
@@ -519,27 +518,7 @@
send:
#endif
- mflags = m->m_flags;
- len = m->m_pkthdr.len;
- s = splnet();
- /*
- * Queue message on interface, and start output if interface
- * not yet active.
- */
- IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
- if (error) {
- /* mbuf is already freed */
- splx(s);
- return (error);
- }
- ifp->if_obytes += len;
- if (mflags & M_MCAST)
- ifp->if_omcasts++;
- if ((ifp->if_flags & IFF_OACTIVE) == 0)
- (*ifp->if_start)(ifp);
- splx(s);
- return (error);
-
+ return ifq_enqueue(ifp, m ALTQ_COMMA ALTQ_DECL(&pktattr));
bad:
if (m)
m_freem(m);