On Wed, Oct 05, 2016 at 10:58 +0900, YASUOKA Masahiko wrote:
> On Tue, 4 Oct 2016 17:27:12 +0200
> Mike Belopuhov <m...@belopuhov.com> wrote:
> > On Tue, Oct 04, 2016 at 01:07 +0200, Vincent Gross wrote:
> >> On Sat, 24 Sep 2016 10:58:10 +0200
> >> Vincent Gross <vgr...@openbsd.org> wrote:
> >>
> >> > Hi,
> >> >
> >> [snip]
> >> >
> >> > Aside from the mbuf issue, is this Ok ?
> >>
> >> I will go back on the mbuff stuff later.
> >>
> >> Diff rebased, ok anyone ?
> >>
> >> Index: net/if_vxlan.c
> >> ===================================================================
> >> RCS file: /cvs/src/sys/net/if_vxlan.c,v
> >> retrieving revision 1.48
> >> diff -u -p -r1.48 if_vxlan.c
> >> --- net/if_vxlan.c 30 Sep 2016 10:22:05 -0000 1.48
> >> +++ net/if_vxlan.c 3 Oct 2016 23:12:42 -0000
> >> @@ -638,7 +749,9 @@ vxlan_lookup(struct mbuf *m, struct udph
> >> if (m->m_pkthdr.len < skip + sizeof(struct ether_header))
> >> return (EINVAL);
> >>
> >> - m_adj(m, skip);
> >> + m_adj(m, skip - ETHER_ALIGN);
> >> + m = m_pullup(m, ETHER_HDR_LEN + ETHER_ALIGN);
> >> + m_adj(m, ETHER_ALIGN);
> >> ifp = &sc->sc_ac.ac_if;
> >>
> >> #if NBRIDGE > 0
> >
> > I think this chunk is correct. First you ensure that m->m_data
> > points to a contiguous and well-aligned ethernet header and then
> > you trim the alignment so that mtod() points directly at it.
>
> Isn't it possible that m_pullup() may return non aligned pointer?
>
Do you mean if m->m_data pointer can be not aligned properly after
an m_pullup? Of course, if the header layout is such that m_pullup
doesn't need to move data around and at the same time the header
that we're going to m_adj[ust] to is not aligned properly, this is
going to be a problem.
I wrote a test program (attached) that illustrates the change in
behavior when compiled w/o any options versus when compiled with
-DTEST3 (cc -DTEST3 -o mbuf mbuf.c)
Meaning that m_pullup might not be enough in a generic case.
#include <sys/types.h>
#include <sys/mbuf.h>
#include <errno.h>
#include <err.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#define panic(x...) errx(1, x)
unsigned int
min(unsigned int a, unsigned int b)
{
return (a < b ? a : b);
}
void
m_extfree(struct mbuf *m)
{
free(m->m_ext.ext_buf);
m->m_flags &= ~(M_EXT|M_EXTWR);
}
struct mbuf *
m_free(struct mbuf *m)
{
struct mbuf *n;
if (m == NULL)
return (NULL);
n = m->m_next;
if (m->m_flags & M_EXT)
m_extfree(m);
free(m);
return (n);
}
struct mbuf *
m_freem(struct mbuf *m)
{
struct mbuf *n;
if (m == NULL)
return (NULL);
n = m->m_nextpkt;
do
m = m_free(m);
while (m != NULL);
return (n);
}
struct mbuf *
m_get(int nowait, int type)
{
struct mbuf *m;
m = calloc(1, sizeof(*m));
if (m == NULL)
return (NULL);
m->m_type = type;
m->m_next = NULL;
m->m_nextpkt = NULL;
m->m_data = m->m_dat;
m->m_flags = 0;
return (m);
}
struct mbuf *
m_inithdr(struct mbuf *m)
{
/* keep in sync with m_gethdr */
m->m_next = NULL;
m->m_nextpkt = NULL;
m->m_data = m->m_pktdat;
m->m_flags = M_PKTHDR;
memset(&m->m_pkthdr, 0, sizeof(m->m_pkthdr));
return (m);
}
struct mbuf *
m_gethdr(int nowait, int type)
{
struct mbuf *m;
m = calloc(1, sizeof(*m));
if (m == NULL)
return (NULL);
m->m_type = type;
return (m_inithdr(m));
}
struct mbuf *
m_clget(struct mbuf *m, int how, u_int pktlen)
{
struct mbuf *m0 = NULL;
caddr_t buf;
if (m == NULL) {
m0 = m_gethdr(how, MT_DATA);
if (m0 == NULL)
return (NULL);
m = m0;
}
buf = calloc(1, pktlen);
if (buf == NULL) {
if (m0)
m_freem(m0);
return (NULL);
}
MEXTADD(m, buf, pktlen, M_EXTWR, MEXTFREE_POOL, NULL);
return (m);
}
struct mbuf *
m_getptr(struct mbuf *m, int loc, int *off)
{
while (loc >= 0) {
/* Normal end of search */
if (m->m_len > loc) {
*off = loc;
return (m);
} else {
loc -= m->m_len;
if (m->m_next == NULL) {
if (loc == 0) {
/* Point at the end of valid data */
*off = m->m_len;
return (m);
} else {
return (NULL);
}
} else {
m = m->m_next;
}
}
}
return (NULL);
}
int
m_leadingspace(struct mbuf *m)
{
if (M_READONLY(m))
return 0;
return (m->m_flags & M_EXT ? m->m_data - m->m_ext.ext_buf :
m->m_flags & M_PKTHDR ? m->m_data - m->m_pktdat :
m->m_data - m->m_dat);
}
int
m_trailingspace(struct mbuf *m)
{
if (M_READONLY(m))
return 0;
return (m->m_flags & M_EXT ? m->m_ext.ext_buf +
m->m_ext.ext_size - (m->m_data + m->m_len) :
&m->m_dat[MLEN] - (m->m_data + m->m_len));
}
void
m_copydata(struct mbuf *m, int off, int len, caddr_t cp)
{
unsigned count;
if (off < 0)
panic("m_copydata: off %d < 0", off);
if (len < 0)
panic("m_copydata: len %d < 0", len);
if ((m = m_getptr(m, off, &off)) == NULL)
panic("m_copydata: short mbuf chain");
while (len > 0) {
if (m == NULL)
panic("m_copydata: null mbuf");
count = min(m->m_len - off, len);
memmove(cp, mtod(m, caddr_t) + off, count);
len -= count;
cp += count;
off = 0;
m = m->m_next;
}
}
int
m_copyback(struct mbuf *m0, int off, int len, const void *_cp, int wait)
{
int mlen, totlen = 0;
struct mbuf *m = m0, *n;
caddr_t cp = (caddr_t)_cp;
int error = 0;
if (m0 == NULL)
return (0);
while (off > (mlen = m->m_len)) {
off -= mlen;
totlen += mlen;
if (m->m_next == NULL) {
if ((n = m_get(wait, m->m_type)) == NULL) {
error = ENOBUFS;
goto out;
}
if (off + len > MLEN) {
MCLGETI(n, wait, NULL, off + len);
if (!(n->m_flags & M_EXT)) {
m_free(n);
error = ENOBUFS;
goto out;
}
}
memset(mtod(n, caddr_t), 0, off);
n->m_len = len + off;
m->m_next = n;
}
m = m->m_next;
}
while (len > 0) {
/* extend last packet to be filled fully */
if (m->m_next == NULL && (len > m->m_len - off))
m->m_len += min(len - (m->m_len - off),
M_TRAILINGSPACE(m));
mlen = min(m->m_len - off, len);
memmove(mtod(m, caddr_t) + off, cp, mlen);
cp += mlen;
len -= mlen;
totlen += mlen + off;
if (len == 0)
break;
off = 0;
if (m->m_next == NULL) {
if ((n = m_get(wait, m->m_type)) == NULL) {
error = ENOBUFS;
goto out;
}
if (len > MLEN) {
MCLGETI(n, wait, NULL, len);
if (!(n->m_flags & M_EXT)) {
m_free(n);
error = ENOBUFS;
goto out;
}
}
n->m_len = len;
m->m_next = n;
}
m = m->m_next;
}
out:
if (((m = m0)->m_flags & M_PKTHDR) && (m->m_pkthdr.len < totlen))
m->m_pkthdr.len = totlen;
return (error);
}
void
m_adj(struct mbuf *mp, int req_len)
{
int len = req_len;
struct mbuf *m;
int count;
if ((m = mp) == NULL)
return;
if (len >= 0) {
/*
* Trim from head.
*/
while (m != NULL && len > 0) {
if (m->m_len <= len) {
len -= m->m_len;
m->m_len = 0;
m = m->m_next;
} else {
m->m_len -= len;
m->m_data += len;
len = 0;
}
}
if (mp->m_flags & M_PKTHDR)
mp->m_pkthdr.len -= (req_len - len);
} else {
/*
* Trim from tail. Scan the mbuf chain,
* calculating its length and finding the last mbuf.
* If the adjustment only affects this mbuf, then just
* adjust and return. Otherwise, rescan and truncate
* after the remaining size.
*/
len = -len;
count = 0;
for (;;) {
count += m->m_len;
if (m->m_next == NULL)
break;
m = m->m_next;
}
if (m->m_len >= len) {
m->m_len -= len;
if (mp->m_flags & M_PKTHDR)
mp->m_pkthdr.len -= len;
return;
}
count -= len;
if (count < 0)
count = 0;
/*
* Correct length for chain is "count".
* Find the mbuf with last data, adjust its length,
* and toss data from remaining mbufs on chain.
*/
m = mp;
if (m->m_flags & M_PKTHDR)
m->m_pkthdr.len = count;
for (; m; m = m->m_next) {
if (m->m_len >= count) {
m->m_len = count;
break;
}
count -= m->m_len;
}
while ((m = m->m_next) != NULL)
m->m_len = 0;
}
}
struct mbuf *
m_pullup(struct mbuf *n, int len)
{
struct mbuf *m;
int count;
/*
* If first mbuf has no cluster, and has room for len bytes
* without shifting current data, pullup into it,
* otherwise allocate a new mbuf to prepend to the chain.
*/
if ((n->m_flags & M_EXT) == 0 && n->m_next &&
n->m_data + len < &n->m_dat[MLEN]) {
if (n->m_len >= len)
return (n);
m = n;
n = n->m_next;
len -= m->m_len;
} else if ((n->m_flags & M_EXT) != 0 && len > MHLEN && n->m_next &&
n->m_data + len < &n->m_ext.ext_buf[n->m_ext.ext_size]) {
if (n->m_len >= len)
return (n);
m = n;
n = n->m_next;
len -= m->m_len;
} else {
if (len > MAXMCLBYTES)
goto bad;
MGET(m, M_DONTWAIT, n->m_type);
if (m == NULL)
goto bad;
if (len > MHLEN) {
MCLGETI(m, M_DONTWAIT, NULL, len);
if ((m->m_flags & M_EXT) == 0) {
m_free(m);
goto bad;
}
}
m->m_len = 0;
if (n->m_flags & M_PKTHDR)
M_MOVE_PKTHDR(m, n);
}
do {
count = min(len, n->m_len);
memcpy(mtod(m, caddr_t) + m->m_len, mtod(n, caddr_t),
count);
len -= count;
m->m_len += count;
n->m_len -= count;
if (n->m_len)
n->m_data += count;
else
n = m_free(n);
} while (len > 0 && n);
if (len > 0) {
(void)m_free(m);
goto bad;
}
m->m_next = n;
return (m);
bad:
m_freem(n);
return (NULL);
}
void
m_hexdump(struct mbuf *m)
{
char *cp, *desc, mark;
int i, len, leading, trailing;
while (m != NULL) {
mark = ' ';
len = MLEN;
cp = m->m_dat;
desc = "MBUF";
if (m->m_flags & M_PKTHDR) {
len = MHLEN;
cp = m->m_pktdat;
desc = "PKTHDR";
} else if (m->m_flags & M_EXT) {
len = m->m_ext.ext_size;
cp = m->m_ext.ext_buf;
desc = "CLUSTER";
}
leading = m_leadingspace(m);
trailing = m_trailingspace(m);
printf("=== %s: len %d, total %d, leading(-) %d, trailing(+)
%d\n",
desc, m->m_len, len, leading, trailing);
for (i = 0; i < len; i++) {
if (i % 16 == 0)
printf("%02x:", i);
if (leading > 0 && i < leading )
mark = '-';
else if (trailing > 0 && i >= len - trailing)
mark = '+';
printf(" %c%02x", mark, (unsigned char)*(cp + i));
if (i != 0 && (i % 16) == 15)
printf("\n");
mark = ' ';
}
printf("\n");
m = m->m_next;
}
}
int
main(void)
{
struct mbuf *m_head, *m1;
struct {
uint32_t h1_1;
uint32_t h1_2;
#ifdef TEST3
uint16_t h1_3;
#endif
} __packed hdr1;
struct {
#if 1
uint16_t h2_pad[0]; /* no padding */
#else
uint16_t h2_pad[1]; /* simulate ETHER_ALIGN */
#endif
uint8_t h2_1;
uint8_t h2_2;
} __packed hdr2;
uint8_t data[64];
MGETHDR(m_head, M_DONTWAIT, MT_DATA);
if (m_head == NULL)
err(1, "MGETHDR");
MGET(m1, M_DONTWAIT, MT_DATA);
if (m1 == NULL)
err(1, "MGET");
m_head->m_next = m1;
memset(&hdr1, 0, sizeof(hdr1));
hdr1.h1_1 = htonl(0x11);
hdr1.h1_2 = htonl(0x12);
#ifdef TEST3
hdr1.h1_3 = htons(0x13);
#endif
memset(&hdr2, 0, sizeof(hdr2));
hdr2.h2_1 = 0x21;
hdr2.h2_2 = 0x22;
memset(data, 0xee, sizeof(data));
#if 1
/* Simulate empty first mbuf after an m_adj */
/* Put HDR1 into the first mbuf */
m_head->m_len = sizeof(hdr1);
if (m_copyback(m_head, 0, sizeof(hdr1), &hdr1, M_DONTWAIT))
err(1, "m_copyback hdr1");
/* Put HDR2 into the second mbuf */
m1->m_len = sizeof(hdr2);
if (m_copyback(m_head, sizeof(hdr1), sizeof(hdr2), &hdr2, M_DONTWAIT))
err(1, "m_copyback hdr2");
/* Put data into the second mbuf after an HDR2 */
m1->m_len += sizeof(data);
if (m_copyback(m_head, sizeof(hdr1) + sizeof(hdr2), sizeof(data), data,
M_DONTWAIT))
err(1, "m_copyback data");
#else
/* Simulate a proper mbuf chain even after an m_adj */
/* Put HDR1 and HDR2 into the first mbuf */
m_head->m_len = sizeof(hdr1) + sizeof(hdr2);
if (m_copyback(m_head, 0, sizeof(hdr1), &hdr1, M_DONTWAIT))
err(1, "m_copyback hdr1");
if (m_copyback(m_head, sizeof(hdr1), sizeof(hdr2), &hdr2, M_DONTWAIT))
err(1, "m_copyback hdr2");
/* Put data into the second mbuf after an HDR2 */
m1->m_len = sizeof(data);
if (m_copyback(m_head, sizeof(hdr1) + sizeof(hdr2), sizeof(data), data,
M_DONTWAIT))
err(1, "m_copyback data");
#endif
m_head->m_pkthdr.len = m_head->m_len + m1->m_len;
printf("\n*** SETUP ***\n\n");
m_hexdump(m_head);
#if 0 /* This leaves first mbuf empty */
m_adj(m_head, sizeof(hdr1));
#else
m_adj(m_head, sizeof(hdr1) - 2);
m_head = m_pullup(m_head, sizeof(hdr2) + 2);
m_adj(m_head, 2);
#endif
printf("\n*** RESULT ***\n\n");
m_hexdump(m_head);
return (0);
}
