Date: Sun, 12 Oct 2014 15:49:52 +0000 (UTC) From: Robert Watson <rwatson@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r272984 - in head/sys: netinet netinet6 Message-ID: <201410121549.s9CFnqwg020160@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rwatson Date: Sun Oct 12 15:49:52 2014 New Revision: 272984 URL: https://svnweb.freebsd.org/changeset/base/272984 Log: When deciding whether to call m_pullup() even though there is adequate data in an mbuf, use M_WRITABLE() instead of a direct test of M_EXT; the latter both unnecessarily exposes mbuf-allocator internals in the protocol stack and is also insufficient to catch all cases of non-writability. (NB: m_pullup() does not actually guarantee that a writable mbuf is returned, so further refinement of all of these code paths continues to be required.) Reviewed by: bz MFC after: 3 days Sponsored by: EMC / Isilon Storage Division Differential Revision: https://reviews.freebsd.org/D900 Modified: head/sys/netinet/igmp.c head/sys/netinet/ip_mroute.c head/sys/netinet/ip_output.c head/sys/netinet6/icmp6.c head/sys/netinet6/ip6_mroute.c head/sys/netinet6/ip6_output.c Modified: head/sys/netinet/igmp.c ============================================================================== --- head/sys/netinet/igmp.c Sun Oct 12 13:12:06 2014 (r272983) +++ head/sys/netinet/igmp.c Sun Oct 12 15:49:52 2014 (r272984) @@ -1466,7 +1466,7 @@ igmp_input(struct mbuf **mp, int *offp, minlen += IGMP_V3_QUERY_MINLEN; else minlen += IGMP_MINLEN; - if ((m->m_flags & M_EXT || m->m_len < minlen) && + if ((!M_WRITABLE(m) || m->m_len < minlen) && (m = m_pullup(m, minlen)) == 0) { IGMPSTAT_INC(igps_rcv_tooshort); return (IPPROTO_DONE); @@ -1557,7 +1557,7 @@ igmp_input(struct mbuf **mp, int *offp, */ igmpv3len = iphlen + IGMP_V3_QUERY_MINLEN + srclen; - if ((m->m_flags & M_EXT || + if ((!M_WRITABLE(m) || m->m_len < igmpv3len) && (m = m_pullup(m, igmpv3len)) == NULL) { IGMPSTAT_INC(igps_rcv_tooshort); Modified: head/sys/netinet/ip_mroute.c ============================================================================== --- head/sys/netinet/ip_mroute.c Sun Oct 12 13:12:06 2014 (r272983) +++ head/sys/netinet/ip_mroute.c Sun Oct 12 15:49:52 2014 (r272984) @@ -121,7 +121,6 @@ __FBSDID("$FreeBSD$"); #endif #define VIFI_INVALID ((vifi_t) -1) -#define M_HASCL(m) ((m)->m_flags & M_EXT) static VNET_DEFINE(uint32_t, last_tv_sec); /* last time we processed this */ #define V_last_tv_sec VNET(last_tv_sec) @@ -1304,7 +1303,7 @@ X_ip_mforward(struct ip *ip, struct ifne } mb0 = m_copypacket(m, M_NOWAIT); - if (mb0 && (M_HASCL(mb0) || mb0->m_len < hlen)) + if (mb0 && (!M_WRITABLE(mb0) || mb0->m_len < hlen)) mb0 = m_pullup(mb0, hlen); if (mb0 == NULL) { free(rte, M_MRTABLE); @@ -1544,7 +1543,7 @@ ip_mdq(struct mbuf *m, struct ifnet *ifp int hlen = ip->ip_hl << 2; struct mbuf *mm = m_copy(m, 0, hlen); - if (mm && (M_HASCL(mm) || mm->m_len < hlen)) + if (mm && (!M_WRITABLE(mm) || mm->m_len < hlen)) mm = m_pullup(mm, hlen); if (mm == NULL) return ENOBUFS; @@ -1665,7 +1664,7 @@ phyint_send(struct ip *ip, struct vif *v * so that ip_output() only scribbles on the copy. */ mb_copy = m_copypacket(m, M_NOWAIT); - if (mb_copy && (M_HASCL(mb_copy) || mb_copy->m_len < hlen)) + if (mb_copy && (!M_WRITABLE(mb_copy) || mb_copy->m_len < hlen)) mb_copy = m_pullup(mb_copy, hlen); if (mb_copy == NULL) return; Modified: head/sys/netinet/ip_output.c ============================================================================== --- head/sys/netinet/ip_output.c Sun Oct 12 13:12:06 2014 (r272983) +++ head/sys/netinet/ip_output.c Sun Oct 12 15:49:52 2014 (r272984) @@ -1365,7 +1365,7 @@ ip_mloopback(struct ifnet *ifp, struct m * modify the pack in order to generate checksums. */ copym = m_dup(m, M_NOWAIT); - if (copym != NULL && (copym->m_flags & M_EXT || copym->m_len < hlen)) + if (copym != NULL && (!M_WRITABLE(copym) || copym->m_len < hlen)) copym = m_pullup(copym, hlen); if (copym != NULL) { /* If needed, compute the checksum and mark it as valid. */ Modified: head/sys/netinet6/icmp6.c ============================================================================== --- head/sys/netinet6/icmp6.c Sun Oct 12 13:12:06 2014 (r272983) +++ head/sys/netinet6/icmp6.c Sun Oct 12 15:49:52 2014 (r272984) @@ -63,6 +63,8 @@ #include <sys/cdefs.h> __FBSDID("$FreeBSD$"); +#define MBUF_PRIVATE /* XXXRW: Optimisation tries to avoid M_EXT mbufs */ + #include "opt_inet.h" #include "opt_inet6.h" #include "opt_ipsec.h" @@ -581,7 +583,7 @@ icmp6_input(struct mbuf **mp, int *offp, /* Give up remote */ break; } - if ((n->m_flags & M_EXT) != 0 + if (!M_WRITABLE(n) || n->m_len < off + sizeof(struct icmp6_hdr)) { struct mbuf *n0 = n; int n0len; Modified: head/sys/netinet6/ip6_mroute.c ============================================================================== --- head/sys/netinet6/ip6_mroute.c Sun Oct 12 13:12:06 2014 (r272983) +++ head/sys/netinet6/ip6_mroute.c Sun Oct 12 15:49:52 2014 (r272984) @@ -126,9 +126,6 @@ __FBSDID("$FreeBSD$"); static MALLOC_DEFINE(M_MRTABLE6, "mf6c", "multicast forwarding cache entry"); -/* XXX: this is a very common idiom; move to <sys/mbuf.h> ? */ -#define M_HASCL(m) ((m)->m_flags & M_EXT) - static int ip6_mdq(struct mbuf *, struct ifnet *, struct mf6c *); static void phyint_send(struct ip6_hdr *, struct mif6 *, struct mbuf *); static int register_send(struct ip6_hdr *, struct mif6 *, struct mbuf *); @@ -1128,7 +1125,7 @@ X_ip6_mforward(struct ip6_hdr *ip6, stru * Pullup packet header if needed before storing it, * as other references may modify it in the meantime. */ - if (mb0 && (M_HASCL(mb0) || mb0->m_len < sizeof(struct ip6_hdr))) + if (mb0 && (!M_WRITABLE(mb0) || mb0->m_len < sizeof(struct ip6_hdr))) mb0 = m_pullup(mb0, sizeof(struct ip6_hdr)); if (mb0 == NULL) { free(rte, M_MRTABLE6); @@ -1397,7 +1394,7 @@ ip6_mdq(struct mbuf *m, struct ifnet *if mm = m_copy(m, 0, sizeof(struct ip6_hdr)); if (mm && - (M_HASCL(mm) || + (!M_WRITABLE(mm) || mm->m_len < sizeof(struct ip6_hdr))) mm = m_pullup(mm, sizeof(struct ip6_hdr)); if (mm == NULL) @@ -1527,7 +1524,7 @@ phyint_send(struct ip6_hdr *ip6, struct */ mb_copy = m_copy(m, 0, M_COPYALL); if (mb_copy && - (M_HASCL(mb_copy) || mb_copy->m_len < sizeof(struct ip6_hdr))) + (!M_WRITABLE(mb_copy) || mb_copy->m_len < sizeof(struct ip6_hdr))) mb_copy = m_pullup(mb_copy, sizeof(struct ip6_hdr)); if (mb_copy == NULL) { return; Modified: head/sys/netinet6/ip6_output.c ============================================================================== --- head/sys/netinet6/ip6_output.c Sun Oct 12 13:12:06 2014 (r272983) +++ head/sys/netinet6/ip6_output.c Sun Oct 12 15:49:52 2014 (r272984) @@ -1196,7 +1196,7 @@ ip6_insertfraghdr(struct mbuf *m0, struc for (mlast = n; mlast->m_next; mlast = mlast->m_next) ; - if ((mlast->m_flags & M_EXT) == 0 && + if (M_WRITABLE(mlast) && M_TRAILINGSPACE(mlast) >= sizeof(struct ip6_frag)) { /* use the trailing space of the last mbuf for the fragment hdr */ *frghdrp = (struct ip6_frag *)(mtod(mlast, caddr_t) + @@ -2918,7 +2918,7 @@ ip6_mloopback(struct ifnet *ifp, struct * is in an mbuf cluster, so that we can safely override the IPv6 * header portion later. */ - if ((copym->m_flags & M_EXT) != 0 || + if (!M_WRITABLE(copym) || copym->m_len < sizeof(struct ip6_hdr)) { copym = m_pullup(copym, sizeof(struct ip6_hdr)); if (copym == NULL)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201410121549.s9CFnqwg020160>