On Wed, Oct 05, 2016 at 14:30 +0200, Mike Belopuhov wrote:
> On Wed, Oct 05, 2016 at 10:58 +0900, YASUOKA Masahiko wrote:
> > On Tue, 4 Oct 2016 17:27:12 +0200
> > Mike Belopuhov <[email protected]> wrote:
> > > On Tue, Oct 04, 2016 at 01:07 +0200, Vincent Gross wrote:
> > >> On Sat, 24 Sep 2016 10:58:10 +0200
> > >> Vincent Gross <[email protected]> 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.
I have come up with a code that looks like this:
struct mbuf *n;
int off;
n = m_getptr(m_head, skip + ETHER_HDR_LEN, &off);
if ((m_head->m_len == m_head->m_pkthdr.len) &&
((n = m_getptr(m_head, skip + ETHER_HDR_LEN, &off)) == m_head) &&
!ALIGNED_POINTER(mtod(n, char *) + off, uint32_t)) {
m_adj(m_head, skip - ETHER_ALIGN);
memmove(mtod(m_head, char *) - ETHER_ALIGN,
mtod(m_head, char *), m_head->m_len);
m_head->m_len -= ETHER_ALIGN;
} else if ((n = m_getptr(m_head, skip, &off)) != m_head) {
/* Move Ethernet header to the first mbuf */
m_adj(m_head, skip - ETHER_HDR_LEN - ETHER_ALIGN);
m_copydata(n, off, ETHER_HDR_LEN, mtod(m_head, char *) +
ETHER_ALIGN);
m_adj(m_head, ETHER_ALIGN);
/* See if we need to shift the data */
if (!ALIGNED_POINTER(mtod(n, char *) + ETHER_HDR_LEN, uint32_t)) {
memmove(mtod(n, char *), mtod(n, char *) + ETHER_HDR_LEN,
n->m_len - ETHER_HDR_LEN);
m_adj(n, -ETHER_HDR_LEN);
} else {
m_adj(n, ETHER_HDR_LEN);
}
} else {
m_adj(m_head, skip);
}
But then I've realised that ether_input simply does m_adj(m, ETHER_HDR_LEN)
and passes the mbuf on to the ipv4_input where we do mtod(m, struct ip *).
Meaning that neither this funny code nor a simple m_pullup of the Ethernet
header will work. Ethernet header as well as an IP header must reside in
a contiguous space and be well aligned. Sigh.
