Date: Mon, 7 Oct 2002 15:52:49 -0700 From: "Sam Leffler" <sam@errno.com> To: "Julian Elischer" <julian@elischer.org> Cc: <freebsd-arch@FreeBSD.ORG>, <freebsd-net@FreeBSD.ORG> Subject: Re: CFR: m_tag patch Message-ID: <185201c26e54$43339f40$52557f42@errno.com> References: <Pine.BSF.4.21.0210071322090.34884-100000@InterJet.elischer.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> If we make this a 'standard' method of adding metadata to a packet, then
> I'd like to move netgraph to use it to. it's always better
> to use a standard method if it will do than to roll your own.
>
> however, there are some thiongs that would proclude me from doing so at
> tth moment. Thes are not major issues but they do give a little feeling
> of "banging a square peg into a round hole" if I try use what you have
> for replacing the netgraph metadata system.
>
> Issues:
>
> Firstlty, each netgraph module type is ignorant of
> all other types (except in some special cases). Each 'type'
> labels its structures, control messages, and in fact special metadata
> that it keeps on a packet, using a special maginc number. Each 'type'
> of netgraph node (e.g. ppp node, or frame-relay-mux node)
> defines a different 32 bit magic number (traditionally in netgraph
> it's just the 32 bit time since the epoch when the module was written).
>
> Metadata that is associated with a packet may only have meaning
> to a subset of the modules that touch that packet. so in effect
> teh maginc number is an API/ABI indicator. It specifies which API
> defines this metadata. (or in netgraph, this control message).
> A particular set of modules may want to know about a common API which
> includes some metadata thet they share. In tehis case it is permissable
> for them to agree about a common magic number to identify these
> things, stored in an include file special to that API interface.
>
> All modules that do not know about that maginc number (do not include
> that API include file) must ignore that metadata. In this way
> 3rd party modes can define their own metadata
> types, assuming that they do not have a magic-number collision with some
> other node type (pretty damned unlikely). Once they have seen that
> teh metadata belongs to an API they understand then, and only then will
> they look firther to try understand the type of the metadata.
> There is enough data (e.g. size info) to allow ignorant nodes to free or
> bypass
> unknown metadata.
>
>
> Your tags have a single 16 bit tag ID field.
> This is insufficient for my needs.
> I need to be able to store the API cookie which is a 32 bit
> unsigned number, and on top of that, I also need a 16 bit type field
> that specifies what the data is within teh given API and a 16 bit
> length to allow opaque handling.
>
> your stucture is:
> /*
> * Packet tags structure.
> */
> struct m_tag {
> SLIST_ENTRY(m_tag) m_tag_link; /* List of packet tags */
> u_int16_t m_tag_id; /* Tag ID */
> u_int16_t m_tag_len; /* Length of data */
> };
>
>
> and mine is:
>
> struct meta_field_header {
> u_long cookie; /* cookie for the field. Skip fields you don't
> * know about (same cookie as in messages) */
> u_short type; /* field ID */
> u_short len; /* total len of this field including extra
> * data */
> char data[0]; /* data starts here */
> };
>
> Basically I'd need to add a 32 bit API identifier
> to the metadata to allow me to use it. Since (for example)
> both ppp and frame relay could define a metadata of type "1".
>
> Since neither ppp nor frame realy knows of the other they can not
> co-operate on selecting command and metadata IDs. However
> we DO send ppp packets over frame relay links, so the chance that
> a packet has to pass through both node types is very real.
>
> I would define a 'global' API that defines a few characteristics that
> would be useful for all modules to know about.
> e.g. packet priority. (frame relay needs this to work right
> for example, but it needs to be understood at the physical
> layer so that hi-pri packets can bypass low-pri packets)
>
So all this is to say that you want m_tag_id to be u_int32_t. Having
another 16-bit field is immaterial to anything I've thought of but given
that switching to a 32-bit tag_id will require allocating an additional
32-bit item you can have that for free since there's little point in having
a 32-bit length. The obvious downside is that m_tag structs now go from 8
bytes to 12, but this is still a darn sight better than allocating a 256
byte mbuf.
> Your TAG IDS are centrally allocated.
> e.g. in mbuf.h:
> /* Packet tag types -- first ones are from NetBSD */
>
> #define PACKET_TAG_NONE 0 /* Nadda */
> #define PACKET_TAG_IPSEC_IN_DONE 1 /* IPsec applied, in */
> #define PACKET_TAG_IPSEC_OUT_DONE 2 /* IPsec applied, out */
> #define PACKET_TAG_IPSEC_IN_CRYPTO_DONE 3 /* NIC IPsec crypto done */
> #define PACKET_TAG_IPSEC_OUT_CRYPTO_NEEDED 4 /* NIC IPsec crypto req'ed
*/
> #define PACKET_TAG_IPSEC_IN_COULD_DO_CRYPTO 5 /* NIC notifies IPsec */
> #define PACKET_TAG_IPSEC_PENDING_TDB 6 /* Reminder to do IPsec */
> #define PACKET_TAG_BRIDGE 7 /* Bridge processing done
*/
> #define PACKET_TAG_GIF 8 /* GIF processing done */
> #define PACKET_TAG_GRE 9 /* GRE processing done */
> [etc.]
>
> If you allocated your API definitions corectly
> using my scheme, you might allocate a API number of
> 1034025045 for example to the IPSEC-CRYPTO interface.
> and that API would define it's own IDS for metadata.
> This would not be able to accidentally match with the Priority
> metadata used for frame relay (if you sent Ipsec over frame relay)
> because (for example) teh frame relay API number is 872148478
> /usr/include/netgraph/ng_frame_relay.h line 48
>
> #define NGM_FRAMERELAY_COOKIE 872148478
>
> You wouldn't have to know about frame relay and frame relay doesn't need
> to know about IPSEC, but it does know how to free the metadata if it
> needs to discard the packet.
>
If you allocate tag id's using your 32-bit time scheme then the fixed values
above would never be hit since they are all for impossible times and so
there'd be no conflict.
>
> (this is a contrived example because priority is a "BASE API" metadata
> type in netgraph, and the base API doesn't have a magic number at the
> moment but probably should have one. (certainly would in this case)
>
>
> As I mentionned before, it is also not clear to me that the metadata
> needs to be in linked list form, but I could live with it.
>
Sounds to me like the real issue for you is insuring unique m_tag_id values.
We're certainly less likely to have collisions with a 32-bit value than a
16-bit value and expanding this way gives you your "field ID" too. I guess
the question I have is whether the existing API's that search only by
"cookie" are sufficient for your needs. If so then I'm ok with changing
things. Otherwise we have an API incompatibility with openbsd that I'd like
to avoid.
Sam
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?185201c26e54$43339f40$52557f42>