Date: Mon, 25 Nov 2002 10:32:19 -0800 From: "Sam Leffler" <sam@errno.com> To: "Robert Watson" <rwatson@FreeBSD.ORG>, "Andrew Gallatin" <gallatin@cs.duke.edu> Cc: "Luigi Rizzo" <rizzo@icir.org>, <current@FreeBSD.ORG> Subject: Re: mbuf header bloat ? Message-ID: <232901c294b0$feda0090$52557f42@errno.com> References: <Pine.NEB.3.96L.1021125111737.36232C-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> (1) When packet headers are copied using m_copy_pkthdr(), different
> consumers have different expectations for what the resulting semantics
> are for m_tag data -- some want it duplicated, others want it moved.
> In practice, it is only ever moved, so consumers that expect
> duplication are in for a surprise. We need to re-implement the packet
> header copying code so that it can generate a failure (because it
> involves allocation), and separate the duplicate and move abstractions
> to get clean semantics. I exchanged some e-mail with Sam Leffler on
> the topic, and apparently OpenBSD has already made these changes, or
> similar ones, and we should do the same for 5.1.
>
As I explained to you; the handling of mtags mimics what was there for the
aux mbufs. I did this intentionally to avoid changes that might introduce
subtle problems. My intent was to cleanup this stuff after 5.0 releases by
replacing the pkthdr copy macros with separate DUP+MOVE macros ala openbsd.
I did this in my original implemention but discarded it when I did
the -current integration.
> (2) m_tag's don't have a notion that the data carried in a tag is
> multi-dimmensional and may require special destructor behavior. While
> it does centralize copying and free'ing of data, it handles this
> purely with bcopy, malloc, and free, which is not appropriate for use
> with MAC labels, since they may contain a variety of per-policy data
> that may require special handling (reference count management, etc).
> I tried putting tag-specific release/free/... code in the m_tag
> central routines, and it looks like that would work, although it
> eventually would lead to a lot of junk in the m_tag code. We might
> want to consider m_tag entry free/copy pointers of some sort, but I'm
> not sure if we want to go there. Adding the MAC stuff to the
> m_tag_{free,copy,...} calls won't break the ABI, whereas adding free
> and copy pointers to the tags themselves would.
>
I don't believe it's necessary to overload the basic mtag structure but
instead introduce a specific cookie that enables a more general mechanism
that would be suitable for your needs.
> (3) Not all code generating packets properly initializes m_tag field. The
> one example I've come across so far is the use of m_getcl() to grab
> mbufs with an attached cluster -- it never initializes the SLIST
> properly, as far as I can tell. Right now that's used in device
> drivers, and also in the BPF packet generation code. If the header is
> zero'd, this may be OK due to an implicit proper initialization, but
> this is concerning. We need to do more work to normalize packet
> handling.
>
I don't see this problem; m_getcl appears to do the right thing.
> (4) Code still exists that improperly hand-copies mbuf packet header data.
> if_loop.c contains some particular bogus code that also triggers a
> panic in the MAC code for the same reason. If m_tag data ever passes
> through if_loop and hits the re-alignment case introduced by KAME, the
> system will panic when the tag data is free'd. This code all needs to
> be normalized, and proper semantics must be enforced.
>
I don't see this problem; looutput looks to do the right thing. FWIW I've
passed mbufs w/ mtags through the loopback interface.
<...other stuff omitted...>
Please contact me directly about the problems so we can resolve them
immediately.
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?232901c294b0$feda0090$52557f42>