From ed6a66ca6c2065bc1fed93e00805676a5d3b981e Mon Sep 17 00:00:00 2001 From: Robert Watson Date: Mon, 5 Jan 2015 09:58:32 +0000 Subject: [PATCH] To ease changes to underlying mbuf structure and the mbuf allocator, reduce the knowledge of mbuf layout, and in particular constants such as M_EXT, MLEN, MHLEN, and so on, in mbuf consumers by unifying various alignment utility functions (M_ALIGN(), MH_ALIGN(), MEXT_ALIGN() in a single M_ALIGN() macro, implemented by a now-inlined m_align() function: - Move m_align() from uipc_mbuf.c to mbuf.h; mark as __inline. - Reimplement M_ALIGN(), MH_ALIGN(), and MEXT_ALIGN() using m_align(). - Update consumers around the tree to simply use M_ALIGN(). This change eliminates a number of cases where mbuf consumers must be aware of whether or not mbufs returned by the allocator use external storage, but also assumptions about the size of the returned mbuf. This will make it easier to introduce changes in how we use external storage, as well as features such as variable-size mbufs. Differential Revision: https://reviews.freebsd.org/D1436 Reviewed by: glebius, trasz, gnn, bz Sponsored by: EMC / Isilon Storage Division --- sys/dev/en/midway.c | 2 +- sys/dev/fatm/if_fatm.c | 2 +- sys/dev/iscsi_initiator/isc_soc.c | 4 +- sys/dev/patm/if_patm_rx.c | 6 +-- sys/kern/uipc_mbuf.c | 40 ++--------------- sys/net80211/ieee80211_freebsd.c | 2 +- sys/netinet/if_ether.c | 2 +- sys/netinet/igmp.c | 8 ++-- sys/netinet/ip_carp.c | 4 +- sys/netinet/sctp_os_bsd.h | 6 +-- sys/netinet/tcp_output.c | 2 +- sys/netinet6/ip6_output.c | 2 +- sys/netinet6/mld6.c | 4 +- sys/netinet6/nd6_nbr.c | 4 +- sys/sys/mbuf.h | 71 +++++++++++++++---------------- 15 files changed, 61 insertions(+), 98 deletions(-) diff --git a/sys/dev/en/midway.c b/sys/dev/en/midway.c index 3175d61477dd..bc3adf5cf558 100644 --- a/sys/dev/en/midway.c +++ b/sys/dev/en/midway.c @@ -1935,7 +1935,7 @@ en_mget(struct en_softc *sc, u_int pktlen) m->m_pkthdr.rcvif = NULL; m->m_pkthdr.len = pktlen; m->m_len = EN_RX1BUF; - MH_ALIGN(m, EN_RX1BUF); + M_ALIGN(m, EN_RX1BUF); if (m->m_len >= totlen) { m->m_len = totlen; diff --git a/sys/dev/fatm/if_fatm.c b/sys/dev/fatm/if_fatm.c index b3281fe1a47f..ec85d31ae570 100644 --- a/sys/dev/fatm/if_fatm.c +++ b/sys/dev/fatm/if_fatm.c @@ -1105,7 +1105,7 @@ fatm_supply_small_buffers(struct fatm_softc *sc) LIST_INSERT_HEAD(&sc->rbuf_free, rb, link); break; } - MH_ALIGN(m, SMALL_BUFFER_LEN); + M_ALIGN(m, SMALL_BUFFER_LEN); error = bus_dmamap_load(sc->rbuf_tag, rb->map, m->m_data, SMALL_BUFFER_LEN, dmaload_helper, &phys, BUS_DMA_NOWAIT); diff --git a/sys/dev/iscsi_initiator/isc_soc.c b/sys/dev/iscsi_initiator/isc_soc.c index 625e74b3b981..e77f0700c69e 100644 --- a/sys/dev/iscsi_initiator/isc_soc.c +++ b/sys/dev/iscsi_initiator/isc_soc.c @@ -110,7 +110,7 @@ isc_sendPDU(isc_session_t *sp, pduq_t *pq) | Add any AHS to the iSCSI hdr mbuf */ if((mh->m_len + pp->ahs_len) < MHLEN) { - MH_ALIGN(mh, mh->m_len + pp->ahs_len); + M_ALIGN(mh, mh->m_len + pp->ahs_len); bcopy(&pp->ipdu, mh->m_data, mh->m_len); bcopy(pp->ahs_addr, mh->m_data + mh->m_len, pp->ahs_len); mh->m_len += pp->ahs_len; @@ -119,7 +119,7 @@ isc_sendPDU(isc_session_t *sp, pduq_t *pq) panic("len AHS=%d too big, not impleneted yet", pp->ahs_len); } else { - MH_ALIGN(mh, mh->m_len); + M_ALIGN(mh, mh->m_len); bcopy(&pp->ipdu, mh->m_data, mh->m_len); } mh->m_pkthdr.len = mh->m_len; diff --git a/sys/dev/patm/if_patm_rx.c b/sys/dev/patm/if_patm_rx.c index 3df4d6a29b8d..c4822983c6a4 100644 --- a/sys/dev/patm/if_patm_rx.c +++ b/sys/dev/patm/if_patm_rx.c @@ -471,7 +471,7 @@ patm_rx_raw(struct patm_softc *sc, u_char *cell) default: case PATM_RAW_CELL: m->m_len = m->m_pkthdr.len = 53; - MH_ALIGN(m, 53); + M_ALIGN(m, 53); dst = mtod(m, u_char *); *dst++ = *cell++; *dst++ = *cell++; @@ -483,7 +483,7 @@ patm_rx_raw(struct patm_softc *sc, u_char *cell) case PATM_RAW_NOHEC: m->m_len = m->m_pkthdr.len = 52; - MH_ALIGN(m, 52); + M_ALIGN(m, 52); dst = mtod(m, u_char *); *dst++ = *cell++; *dst++ = *cell++; @@ -494,7 +494,7 @@ patm_rx_raw(struct patm_softc *sc, u_char *cell) case PATM_RAW_CS: m->m_len = m->m_pkthdr.len = 64; - MH_ALIGN(m, 64); + M_ALIGN(m, 64); dst = mtod(m, u_char *); *dst++ = *cell++; *dst++ = *cell++; diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c index 3880456bddf7..abeef08c4d1a 100644 --- a/sys/kern/uipc_mbuf.c +++ b/sys/kern/uipc_mbuf.c @@ -574,13 +574,8 @@ m_prepend(struct mbuf *m, int len, int how) m_move_pkthdr(mn, m); mn->m_next = m; m = mn; - if(m->m_flags & M_PKTHDR) { - if (len < MHLEN) - MH_ALIGN(m, len); - } else { - if (len < MLEN) - M_ALIGN(m, len); - } + if (len < M_SIZE(m)) + M_ALIGN(m, len); m->m_len = len; return (m); } @@ -1226,7 +1221,7 @@ m_split(struct mbuf *m0, int len0, int wait) goto extpacket; if (remain > MHLEN) { /* m can't be the lead packet */ - MH_ALIGN(n, 0); + M_ALIGN(n, 0); n->m_next = m_split(m, len, wait); if (n->m_next == NULL) { (void) m_free(n); @@ -1236,7 +1231,7 @@ m_split(struct mbuf *m0, int len0, int wait) return (n); } } else - MH_ALIGN(n, remain); + M_ALIGN(n, remain); } else if (remain == 0) { n = m->m_next; m->m_next = NULL; @@ -1887,33 +1882,6 @@ m_mbuftouio(struct uio *uio, struct mbuf *m, int len) return (0); } -/* - * Set the m_data pointer of a newly-allocated mbuf - * to place an object of the specified size at the - * end of the mbuf, longword aligned. - */ -void -m_align(struct mbuf *m, int len) -{ -#ifdef INVARIANTS - const char *msg = "%s: not a virgin mbuf"; -#endif - int adjust; - - if (m->m_flags & M_EXT) { - KASSERT(m->m_data == m->m_ext.ext_buf, (msg, __func__)); - adjust = m->m_ext.ext_size - len; - } else if (m->m_flags & M_PKTHDR) { - KASSERT(m->m_data == m->m_pktdat, (msg, __func__)); - adjust = MHLEN - len; - } else { - KASSERT(m->m_data == m->m_dat, (msg, __func__)); - adjust = MLEN - len; - } - - m->m_data += adjust &~ (sizeof(long)-1); -} - /* * Create a writable copy of the mbuf chain. While doing this * we compact the chain with a goal of producing a chain with diff --git a/sys/net80211/ieee80211_freebsd.c b/sys/net80211/ieee80211_freebsd.c index ab5b26eca825..87cda6079837 100644 --- a/sys/net80211/ieee80211_freebsd.c +++ b/sys/net80211/ieee80211_freebsd.c @@ -420,7 +420,7 @@ ieee80211_getmgtframe(uint8_t **frm, int headroom, int pktlen) * frames which all fit in MHLEN. */ if (m != NULL) - MH_ALIGN(m, len); + M_ALIGN(m, len); } else { m = m_getcl(M_NOWAIT, MT_DATA, M_PKTHDR); if (m != NULL) diff --git a/sys/netinet/if_ether.c b/sys/netinet/if_ether.c index f1d06562c7b7..840598d3042f 100644 --- a/sys/netinet/if_ether.c +++ b/sys/netinet/if_ether.c @@ -261,7 +261,7 @@ arprequest(struct ifnet *ifp, const struct in_addr *sip, m->m_len = sizeof(*ah) + 2 * sizeof(struct in_addr) + 2 * ifp->if_addrlen; m->m_pkthdr.len = m->m_len; - MH_ALIGN(m, m->m_len); + M_ALIGN(m, m->m_len); ah = mtod(m, struct arphdr *); bzero((caddr_t)ah, m->m_len); #ifdef MAC diff --git a/sys/netinet/igmp.c b/sys/netinet/igmp.c index 210b24f032bb..4ed25a5f677f 100644 --- a/sys/netinet/igmp.c +++ b/sys/netinet/igmp.c @@ -2212,7 +2212,7 @@ igmp_v1v2_queue_report(struct in_multi *inm, const int type) m = m_gethdr(M_NOWAIT, MT_DATA); if (m == NULL) return (ENOMEM); - MH_ALIGN(m, sizeof(struct ip) + sizeof(struct igmp)); + M_ALIGN(m, sizeof(struct ip) + sizeof(struct igmp)); m->m_pkthdr.len = sizeof(struct ip) + sizeof(struct igmp); @@ -2793,7 +2793,7 @@ igmp_v3_enqueue_group_record(struct ifqueue *ifq, struct in_multi *inm, if (m == NULL) { m = m_gethdr(M_NOWAIT, MT_DATA); if (m) - MH_ALIGN(m, IGMP_LEADINGSPACE); + M_ALIGN(m, IGMP_LEADINGSPACE); } if (m == NULL) return (-ENOMEM); @@ -2917,7 +2917,7 @@ igmp_v3_enqueue_group_record(struct ifqueue *ifq, struct in_multi *inm, if (m == NULL) { m = m_gethdr(M_NOWAIT, MT_DATA); if (m) - MH_ALIGN(m, IGMP_LEADINGSPACE); + M_ALIGN(m, IGMP_LEADINGSPACE); } if (m == NULL) return (-ENOMEM); @@ -3073,7 +3073,7 @@ igmp_v3_enqueue_filter_change(struct ifqueue *ifq, struct in_multi *inm) if (m == NULL) { m = m_gethdr(M_NOWAIT, MT_DATA); if (m) - MH_ALIGN(m, IGMP_LEADINGSPACE); + M_ALIGN(m, IGMP_LEADINGSPACE); } if (m == NULL) { CTR1(KTR_IGMPV3, diff --git a/sys/netinet/ip_carp.c b/sys/netinet/ip_carp.c index 72087646edad..97862733bc79 100644 --- a/sys/netinet/ip_carp.c +++ b/sys/netinet/ip_carp.c @@ -840,7 +840,7 @@ carp_send_ad_locked(struct carp_softc *sc) m->m_pkthdr.len = len; m->m_pkthdr.rcvif = NULL; m->m_len = len; - MH_ALIGN(m, m->m_len); + M_ALIGN(m, m->m_len); m->m_flags |= M_MCAST; ip = mtod(m, struct ip *); ip->ip_v = IPVERSION; @@ -892,7 +892,7 @@ carp_send_ad_locked(struct carp_softc *sc) m->m_pkthdr.len = len; m->m_pkthdr.rcvif = NULL; m->m_len = len; - MH_ALIGN(m, m->m_len); + M_ALIGN(m, m->m_len); m->m_flags |= M_MCAST; ip6 = mtod(m, struct ip6_hdr *); bzero(ip6, sizeof(*ip6)); diff --git a/sys/netinet/sctp_os_bsd.h b/sys/netinet/sctp_os_bsd.h index 9d5466307f50..53427ce85bc7 100644 --- a/sys/netinet/sctp_os_bsd.h +++ b/sys/netinet/sctp_os_bsd.h @@ -304,11 +304,7 @@ typedef struct callout sctp_os_timer_t; #define SCTP_BUF_RECVIF(m) (m->m_pkthdr.rcvif) #define SCTP_BUF_PREPEND M_PREPEND -#define SCTP_ALIGN_TO_END(m, len) if(m->m_flags & M_PKTHDR) { \ - MH_ALIGN(m, len); \ - } else if ((m->m_flags & M_EXT) == 0) { \ - M_ALIGN(m, len); \ - } +#define SCTP_ALIGN_TO_END(m, len) M_ALIGN(m, len) /* We make it so if you have up to 4 threads * writing based on the default size of diff --git a/sys/netinet/tcp_output.c b/sys/netinet/tcp_output.c index 160fadaea5cd..eb4cb47ba48e 100644 --- a/sys/netinet/tcp_output.c +++ b/sys/netinet/tcp_output.c @@ -1007,7 +1007,7 @@ send: #ifdef INET6 if (isipv6 && (MHLEN < hdrlen + max_linkhdr) && MHLEN >= hdrlen) { - MH_ALIGN(m, hdrlen); + M_ALIGN(m, hdrlen); } else #endif m->m_data += max_linkhdr; diff --git a/sys/netinet6/ip6_output.c b/sys/netinet6/ip6_output.c index 74eb72ed5075..ce68381e5628 100644 --- a/sys/netinet6/ip6_output.c +++ b/sys/netinet6/ip6_output.c @@ -2940,7 +2940,7 @@ ip6_splithdr(struct mbuf *m, struct ip6_exthdrs *exthdrs) return ENOBUFS; } m_move_pkthdr(mh, m); - MH_ALIGN(mh, sizeof(*ip6)); + M_ALIGN(mh, sizeof(*ip6)); m->m_len -= sizeof(*ip6); m->m_data += sizeof(*ip6); mh->m_next = m; diff --git a/sys/netinet6/mld6.c b/sys/netinet6/mld6.c index 8805edac9d86..d992d57de207 100644 --- a/sys/netinet6/mld6.c +++ b/sys/netinet6/mld6.c @@ -1818,7 +1818,7 @@ mld_v1_transmit_report(struct in6_multi *in6m, const int type) * that ether_output() does not need to allocate another mbuf * for the header in the most common case. */ - MH_ALIGN(mh, sizeof(struct ip6_hdr)); + M_ALIGN(mh, sizeof(struct ip6_hdr)); mh->m_pkthdr.len = sizeof(struct ip6_hdr) + sizeof(struct mld_hdr); mh->m_len = sizeof(struct ip6_hdr); @@ -3179,7 +3179,7 @@ mld_v2_encap_report(struct ifnet *ifp, struct mbuf *m) m_freem(m); return (NULL); } - MH_ALIGN(mh, sizeof(struct ip6_hdr) + sizeof(struct mldv2_report)); + M_ALIGN(mh, sizeof(struct ip6_hdr) + sizeof(struct mldv2_report)); mldreclen = m_length(m, NULL); CTR2(KTR_MLD, "%s: mldreclen is %d", __func__, mldreclen); diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c index 39a36e875c23..4841d72eef77 100644 --- a/sys/netinet6/nd6_nbr.c +++ b/sys/netinet6/nd6_nbr.c @@ -431,7 +431,7 @@ nd6_ns_output(struct ifnet *ifp, const struct in6_addr *daddr6, icmp6len = sizeof(*nd_ns); m->m_pkthdr.len = m->m_len = sizeof(*ip6) + icmp6len; - m->m_data += max_linkhdr; /* or MH_ALIGN() equivalent? */ + m->m_data += max_linkhdr; /* or M_ALIGN() equivalent? */ /* fill neighbor solicitation packet */ ip6 = mtod(m, struct ip6_hdr *); @@ -1003,7 +1003,7 @@ nd6_na_output_fib(struct ifnet *ifp, const struct in6_addr *daddr6_0, icmp6len = sizeof(*nd_na); m->m_pkthdr.len = m->m_len = sizeof(struct ip6_hdr) + icmp6len; - m->m_data += max_linkhdr; /* or MH_ALIGN() equivalent? */ + m->m_data += max_linkhdr; /* or M_ALIGN() equivalent? */ /* fill neighbor advertisement packet */ ip6 = mtod(m, struct ip6_hdr *); diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h index cbb2a1850c2e..28e1cc5db6f0 100644 --- a/sys/sys/mbuf.h +++ b/sys/sys/mbuf.h @@ -809,42 +809,6 @@ m_last(struct mbuf *m) KASSERT((((struct mbuf *)m)->m_flags & 0) == 0, \ ("%s: attempted use of a free mbuf!", __func__)) -/* - * Set the m_data pointer of a newly-allocated mbuf (m_get/MGET) to place an - * object of the specified size at the end of the mbuf, longword aligned. - */ -#define M_ALIGN(m, len) do { \ - KASSERT(!((m)->m_flags & (M_PKTHDR|M_EXT)), \ - ("%s: M_ALIGN not normal mbuf", __func__)); \ - KASSERT((m)->m_data == (m)->m_dat, \ - ("%s: M_ALIGN not a virgin mbuf", __func__)); \ - (m)->m_data += (MLEN - (len)) & ~(sizeof(long) - 1); \ -} while (0) - -/* - * As above, for mbufs allocated with m_gethdr/MGETHDR or initialized by - * M_DUP/MOVE_PKTHDR. - */ -#define MH_ALIGN(m, len) do { \ - KASSERT((m)->m_flags & M_PKTHDR && !((m)->m_flags & M_EXT), \ - ("%s: MH_ALIGN not PKTHDR mbuf", __func__)); \ - KASSERT((m)->m_data == (m)->m_pktdat, \ - ("%s: MH_ALIGN not a virgin mbuf", __func__)); \ - (m)->m_data += (MHLEN - (len)) & ~(sizeof(long) - 1); \ -} while (0) - -/* - * As above, for mbuf with external storage. - */ -#define MEXT_ALIGN(m, len) do { \ - KASSERT((m)->m_flags & M_EXT, \ - ("%s: MEXT_ALIGN not an M_EXT mbuf", __func__)); \ - KASSERT((m)->m_data == (m)->m_ext.ext_buf, \ - ("%s: MEXT_ALIGN not a virgin mbuf", __func__)); \ - (m)->m_data += ((m)->m_ext.ext_size - (len)) & \ - ~(sizeof(long) - 1); \ -} while (0) - /* * Return the address of the start of the buffer associated with an mbuf, * handling external storage, packet-header mbufs, and regular data mbufs. @@ -863,6 +827,41 @@ m_last(struct mbuf *m) ((m)->m_flags & M_PKTHDR) ? MHLEN : \ MLEN) +/* + * Set the m_data pointer of a newly allocated mbuf to place an object of the + * specified size at the end of the mbuf, longword aligned. + * + * NB: Historically, we had M_ALIGN(), MH_ALIGN(), and MEXT_ALIGN() as + * separate macros, each asserting that it was called at the proper moment. + * This required callers to themselves test the storage type and call the + * right one. Rather than require callers to be aware of those layout + * decisions, we centralize here. + */ +static __inline void +m_align(struct mbuf *m, int len) +{ +#ifdef INVARIANTS + const char *msg = "%s: not a virgin mbuf"; +#endif + int adjust; + + KASSERT(m->m_data == M_START(m), (msg, __func__)); + + if (m->m_flags & M_EXT) { + adjust = m->m_ext.ext_size - len; + } else if (m->m_flags & M_PKTHDR) { + adjust = MHLEN - len; + } else { + adjust = MLEN - len; + } + + m->m_data += adjust &~ (sizeof(long)-1); +} + +#define M_ALIGN(m, len) m_align(m, len) +#define MH_ALIGN(m, len) m_align(m, len) +#define MEXT_ALIGN(m, len) m_align(m, len) + /* * Compute the amount of space available before the current start of data in * an mbuf.