svn commit: r194012 - in head: . sys/netgraph sys/sys

[Marko Zec]

On Thursday 11 June 2009 21:01:40 Pawel Jakub Dawidek wrote:
> On Thu, Jun 11, 2009 at 04:50:49PM +0000, Marko Zec wrote:
> > Author: zec
> > Date: Thu Jun 11 16:50:49 2009
> > New Revision: 194012
> > URL: http://svn.freebsd.org/changeset/base/194012
> >
> > Log:
> >   Introduce a mechanism for detecting calls from outbound path of the
> >   network stack when reentering the inbound path from netgraph, and
> >   force queueing of mbufs at the outbound netgraph node.
> >
> >   The mechanism relies on two components.  First, in netgraph nodes
> >   where outbound path of the network stack calls into netgraph, the
> >   current thread has to be appropriately marked using the new
> >   NG_OUTBOUND_THREAD_REF() macro before proceeding to call further
> >   into the netgraph topology, and unmarked using the
> >   NG_OUTBOUND_THREAD_UNREF() macro before returning to the caller.
> >   Second, netgraph nodes which can potentially reenter the network
> >   stack in the inbound path have to mark their inbound hooks using
> >   NG_HOOK_SET_TO_INBOUND() macro.  The netgraph framework will then
> >   detect when there is a danger of a call graph looping back from
> >   outbound to inbound path via netgraph, and defer handing off the
> >   mbufs to the "inbound" node to a worker thread with a clean stack.
> >
> >   In this first pass only the most obvious netgraph nodes have been
> >   updated to ensure no outbound to inbound calls can occur.  Nodes
> >   such as ng_ipfw, ng_gif etc. should be further examined whether a
> >   potential for outbound to inbound call looping exists.
> >
> >   This commit changes the layout of struct thread, but due to
> >   __FreeBSD_version number shortage a version bump has been omitted
> >   at this time, nevertheless kernel and modules have to be rebuilt.
>
> Are you sure Marko that you can't use sys/sys/osd.h instead of adding
> yet another field to the thread structure? Netgraph is optional
> component and optional components could take advantage of allocating
> stuff they need dynamically. The OSD (Object-Specific Data) KPI is
> designed for use by optional components - you can add your data to a
> thread, you can get it when you want and OSD will call your callback
> when thread dies, so you can clean up.
>
> Maybe you can't, but it's worth checking.

Hmm how much locking overhead do osd_set() / osd_get() methods introduce?  We 
have to bump the refcount on each entry to netgraph, and then check it 
potentially on each hop to next ng node, and finally drop the refcount when 
done with the function call into netgraph.  Accessing td_ng_outbound directly 
via curthread is as cheap as it gets performancewise as it requires no 
locking whatsoever...

Cheers,

Marko


> PS. Currently OSD works for threads and jails, but it is ready to be
>     extended to work with other object types, eg. vnodes, ifnets, etc.
>     Even if you can't use it in this particular case, keep it in mind,
>     as it might be useful for other vimage-related stuff.
>
> > Modified: head/sys/sys/proc.h
> > =========================================================================
> >===== --- head/sys/sys/proc.h	Thu Jun 11 16:48:59 2009	(r194011)
> > +++ head/sys/sys/proc.h	Thu Jun 11 16:50:49 2009	(r194012)
> > @@ -235,6 +235,7 @@ struct thread {
> >  	char		td_name[MAXCOMLEN + 1];	/* (*) Thread name. */
> >  	struct file	*td_fpop;	/* (k) file referencing cdev under op */
> >  	int		td_dbgflags;	/* (c) Userland debugger flags */
> > +	int		td_ng_outbound;	/* (k) Thread entered ng from above. */
> >  	struct osd	td_osd;		/* (k) Object specific data. */
> >  #define	td_endzero td_base_pri