Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 21 Jun 2002 01:18:47 -0700
From:      Luigi Rizzo <rizzo@icir.org>
To:        julian@freebsd.org
Cc:        net@freebsd.org
Subject:   Re: removing global variables from the network stack
Message-ID:  <20020621011847.C77089@iguana.icir.org>
In-Reply-To: <20020619094420.C58636@iguana.icir.org>; from rizzo@icir.org on Wed, Jun 19, 2002 at 09:44:20AM -0700
References:  <20020619094420.C58636@iguana.icir.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Followup to my message, on which I got some valuable input from the
usual suspects -- thanks to those who replied.

Obviously an architecturally clean solution would be to import the
m_tag thing done in NetBSD/OpenBSD and modify the mbuf routines to
properly handle the tag chains.

As a temporary and much less intrusive workaround, I have come up
with the following solution which seems rather clean anyways:

  + It does not need any change in the system but in sys/sys/mbuf.h for
    the definition of the PACKET_TAG_* constants (which I have already
    committed);

  + it only involves simple modifications of the interested
    functions (callers and callees)

  + it is quite efficient because it does not require
    malloc'ing/freeing of the tags.

  + and it is similar in spirit to the m_tag approach so it will
    be hopefully easy to upgrade should we decide to go for m_tag
    at some point.

Comments please ?

I have used this to remove global variables which carried packet
state from DIVERT, FORWARD, DUMMYNET and it took very very little,
mostly deleting lines!

I'd like to commit this stuff soon. As a matter of fact, I also
thought of doing the commit in two steps -- first the infrastructure,
then applications of it, but it turns out that there is no
infrastructure other than the constants in mbuf.h :)

	cheers
	luigi
-------------------------------------------------------------------


	=== A PROPOSAL TO CARRY STATE ALONG WITH PACKETS ===

To carry state along with packets, the mbuf chains are prepended
with mbuf lookalikes (same as it was done for dummynet) which
carry the required info. These 'tags' all start with an m_hdr
containing:

        mh_type = MT_TAG;
        mh_flags = PACKET_TAG_foo;       /* the required type */
        mh_next = <pointer to the next tag or actual mbuf chain>

(MT_TAG is currently defined as MT_CONTROL, which already exists
and was unused. But we could maybe define a new mbuf type, MT_TAG,
and use it here).

Other m_hdr fields (mh_nextpkt, mh_data) are available for tag-specific
data.  mh_len is available too, but i prefer to keep it unused.

If a tag (such as dummynet ones) needs more room, you can use
a larger structure with the m_hdr at the beginning e.g.

        struct dn_pkt {
                struct m_hdr m_hdr;
                <dummynet specific info>
        }

It is important to notice that IT IS THE CALLER'S RESPONSIBILITY
to free the storage associated with the tags. This simplifies
handling in the callee, which can safely skip over the blocks it
is not interested in. Also, the callee cannot expect the info in
the tags to survive beyond the call -- it needs to copy the data
from the tags, not just take a reference.

=== TAG PROCESSING -- CALLEE ===

A routine which needs the info associated with the tags will have to
do something like this near the very beginning:

        while (m->m_type == MT_TAG) {
                switch(m->m_tag_id) {
                default:
                        printf("foo: unrecognised MT_TAG tag %d\n",
                                m->m_tag_id);
                        m = m->next;
                        break;

                case PACKET_TAG_IPFORWARD:
                        next_hop = (struct sockaddr_in *)m->m_hdr.mh_data;
                        m = m->m_next;
                        break;
                ...
                }
        }

with the simplest case (just ignore any tag which might be there) being

	while (m->m_type == MT_TAG)
		m = m->next;

=== TAG PROCESSING -- CALLER ===

A routine wishing to insert a tag in front of the chain can do
something like this:

        if (next_hop != NULL) {
                struct m_hdr tag;

                tag.mh_type = MT_TAG;
                tag.mh_flags = PACKET_TAG_IPFORWARD;
                tag.mh_next = m;
                tag.mh_data = (caddr_t)next_hop; /* tag-specific data */
                ...
                ip_input((struct mbuf *)&tag);
        }

Note that the tag is allocated on the stack, as per the rules
defined above, and the callee is not supposed to free it or
keep references to it.
Of course you should make sure NOT TO SEND TAGS to routines which are
unable to process them!

========================================================================

On Wed, Jun 19, 2002 at 09:44:20AM -0700, Luigi Rizzo wrote:
> Hi,
> I am trying to cleanup as much as possible the use of global
> variables in the network stack, both for code clarity, and
> to ease the use of this code in an SMP context.
...

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?20020621011847.C77089>