Date: Thu, 28 Nov 2002 09:33:56 -0800 From: "Sam Leffler" <sam@errno.com> To: "Bosko Milekic" <bmilekic@unixdaemons.com> Cc: "Robert Watson" <rwatson@FreeBSD.ORG>, "Andrew Gallatin" <gallatin@cs.duke.edu>, "Luigi Rizzo" <rizzo@icir.org>, <current@FreeBSD.ORG> Subject: Re: mbuf header bloat ? Message-ID: <086401c29704$54c05770$52557f42@errno.com> References: <Pine.NEB.3.96L.1021125133654.42342C-100000@fledge.watson.org> <235101c294b2$e66ce160$52557f42@errno.com> <20021125171036.B75673@unixdaemons.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Mon, Nov 25, 2002 at 10:46:00AM -0800, Sam Leffler wrote:
> > > > I don't see this problem; looutput looks to do the right thing.
FWIW
> > I've
> > > > passed mbufs w/ mtags through the loopback interface.
> > >
> > > This refers specifically to the following code snippet:
> > >
> > > if (m && m->m_next != NULL && m->m_pkthdr.len < MCLBYTES) {
> > > struct mbuf *n;
> > >
> > > MGETHDR(n, M_DONTWAIT, MT_HEADER);
> > > if (!n)
> > > goto contiguousfail;
> > > MCLGET(n, M_DONTWAIT);
> > > if (! (n->m_flags & M_EXT)) {
> > > m_freem(n);
> > > goto contiguousfail;
> > > }
> > >
> > > m_copydata(m, 0, m->m_pkthdr.len, mtod(n, caddr_t));
> > > n->m_pkthdr = m->m_pkthdr;
> > > n->m_len = m->m_pkthdr.len;
> > > n->m_pkthdr.aux = m->m_pkthdr.aux;
> > > m->m_pkthdr.aux = (struct mbuf *)NULL;
> > > m_freem(m);
> > > m = n;
> > > }
> > >
> >
> > Something is wrong with your tree:
> >
> > ebb% grep aux ../sys/mbuf.h
> > ebb%
> >
> > The above code is correct in the repo as is the m_getcl code.
>
> While the 'aux' part of the code was in fact removed, what Robert is
> saying still applies. What happens is that this code 'manually'
> performs a mbuf to mbuf+cluster copy because of some pretty bogus
> assumption in the KAME code that expects the data to be contiguous in
> a single cluster.
>
> So when the 'n' mbuf is allocated and the cluster with it then a
> shallow copy of the packet header is done from mbuf 'm' (the old mbuf)
> to the newly allocated mbuf 'n'. What Robert is saying is that
> this copy breaks his MAC label semantics which require a deeper copy
> in this case and he is concerned that it may break the m_tag semantics
> too, especially given the fact that the old mbuf 'm' is then freed
> (doesn't this destroy the label?!). If that's indeed the case, then
> this copy remains bogus.
>
The real code (i.e. what was checked in when I replaced the aux mbufs w/
tags) doesn't do the above; it does the right thing; albeit using the binary
copy of the mbuf instead of using the appropriate macro.
Regardless Robert and I have been working through the details of revising
the system handling of pkthdr's.
Sam
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?086401c29704$54c05770$52557f42>