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

[Alexander Kabaev]

On Thu, 11 Jun 2009 13:27:12 -0700
Julian Elischer <julian at elischer.org> wrote:

> Marko Zec wrote:
> > 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...
> 
> I would add that I suspect that we may end up using it in other
> places as well outside of netgraph.
> 

When that happens then per-thread field can be revisited again. Blowing
the side of major kernel structure for the sake of subsystem is
unused by 90%+ percent of users is little too drastic IMHO.

I do second Pawel's opinion that you should look at osd for the time
being. After all it was invented for just this reason.
-- 
Alexander Kabaev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20090612/0737799a/signature.pgp