Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 7 Oct 2002 00:29:35 -0700 (PDT)
From:      Julian Elischer <julian@elischer.org>
To:        Sam Leffler <sam@errno.com>
Cc:        Nate Lawson <nate@root.org>, freebsd-arch@FreeBSD.ORG, freebsd-net@FreeBSD.ORG
Subject:   Re: CFR: m_tag patch
Message-ID:  <Pine.BSF.4.21.0210062343510.22932-100000@InterJet.elischer.org>
In-Reply-To: <142f01c26dc1$6c4fa5b0$52557f42@errno.com>

next in thread | previous in thread | raw e-mail | index | archive | help


On Sun, 6 Oct 2002, Sam Leffler wrote:

> > 1. Is ordering important or is an SLIST sufficient for all cases?
> 
> Order is not important.

I do similar in Netgraph but could move to whatever scheme becomes
standard in the rest of the system. However I have some serious
comments.

In netgraph I have a separate structure for the packet that contains
a pointer to the mbuf chain as well a pointer to a malloc'd memory
buffer that can contain metadata such as is kept here.
the metadata is of the form [header][field][field][field]....

where each field was defined as:
struct meta_field_header {
        u_long  cookie;         /* cookie for the field. Skip fields you don't
                                 * know about (same cookie as in messgaes) */
        u_short type;           /* field ID within this cookie scheme */
        u_short len;            /* total len of this field including extra
                                 * data */
        char    data[0];        /* data starts here */
};

the header for metadata is:
struct ng_meta {
        char    priority;       /* -ve is less priority,  0 is default */
        char    discardability; /* higher is less valuable.. discard first */
        u_short allocated_len;  /* amount malloc'd */
        u_short used_len;       /* sum of all fields, options etc. */   
        u_short flags;          /* see below.. generic flags */
        struct meta_field_header options[0];    /* add as (if) needed */
}; 

/* Flags for meta-data */
#define NGMF_TEST       0x01    /* discard at the last moment before  sending */
#define NGMF_TRACE      0x02    /* trace when handing this data to a node */
 
One metadata collection is associated with each packet. similar to what
you do except that in 4.x Ipass it as an arguent and in 5.0
I have a packet header that is passed around that identifies both
data and metadata. ( I need the header for other reasons anyway)

I show this only to show that I have tackled the same problem with a
similar but different scheme. My thought was that a packet usually only
has at most a couple of tags, and the tags are usually short, so that it
was ok to malloc a bigger chunk and using the length fields walk them.
If you didn't have room you could malloc a bigger one and copy the
intitial fields, and then add your new ones at the end. It would
probably almost never happen..

I did however find a need to delete a tag once the packet passed out of
the scope of the module in question. (the "cookie" represents a 
particular "ABI" the "type" is only valid withing the ABI.
If you do not support a particular cookie type you cannot interpret the 
contents aof that field) Protocols and such have their own cookies.

I point this out as a feature because the TAG values need to only be
defined within their own ABI/API include files.

If a module that supports a particular cookie passes a packet out to
the rest of the world, it probably should invalidate some fields that
are set with that particular cookie, in case that packet should in some
way be redirected back towards a module that does support it, and that
might cause unexpected results.  For this reason I needed to 'remove'
fields. I ended leaving them in place and setting the cookie to a
special "invalid" cookie value that no-one would match. In a similar
vein, I can imagine wanting to remove one of the 'tags' in your lists.

Each module has a cookie field that is pretty much guaranteed 
to be unique (time since epoch when written) so in your terms,
the IP code would only know and recognise TAGS prepended 
with the IP cookie and would handle other tags as opaque data.
Similarly IPSEC code would only see into tags with the IPSEC cookie
prepended.. I don't think you should be 
defining the TAG values in a global file, but rather
each module should identify its own tags and  ignode others.
If two modules need to share data, a .h file can contain the
defineitions for that API and the cookie that identifies such tags,
and both modules would include that shared include.  That way 
the base code need not know about future improvements, which
has always been "difficult" :-)

> 
> > 2. Is it possible to attach the aux argument to the mbuf chain instead of
> > adding it as a new parameter to ip_output?
> >
> 
> The "aux argument" _was_ originally attached to the mbuf chain.  The change
> to add an extra arg to ip*_output was done to eliminate one of the biggest
> uses of the aux mbuf; the socket to use to get IPsec policy.  This is a
> performance win and worth doing independent of the aux->m_tag switch.
> 
> One could split the ip_output change out but doing it together avoids
> converting code that would just eventually be eliminated (unless you did the
> ip_output change first and then the m_tag switch).

I'm not sure but it's possible m_aux was invented so that ip_output()
would not change interfaces to match with was defined in some interface
method table. I think NetBSD added entries in their
interface method tables with varargs (yech) to get around some of this..


So in summary:
is it worth making it a linked list?
how  many tags do you see on packets, and how large are they?
CAn you use a sche,e suc as that outlined so that each module can
define its own tags privatly?




> 
>     Sam
> 
> 
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-net" in the body of the message
> 


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0210062343510.22932-100000>