kern/188421: ng_callout() timeouts trigger packets queuing and out of order packets

[Karim Fodil-Lemelin]

>Number:         188421
>Category:       kern
>Synopsis:       ng_callout() timeouts trigger packets queuing and out of order packets
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Apr 10 13:30:00 UTC 2014
>Closed-Date:
>Last-Modified:
>Originator:     Karim Fodil-Lemelin
>Release:        FreeBSD 7, FreeBSD 9
>Organization:
XipLink Inc.
>Environment:
FreeBSD pluto 7.4-RELEASE FreeBSD 7.4-RELEASE #0: Mon Apr 15 20:48:02 UTC 2013     kfl at pluto:/usr/src/sys/amd64/compile/SIMULATOR  amd64

>Description:
I have seen an issue in netgraph
where, if called, a callout routine registered by ng_callout() will trigger
packet queuing inside the worklist of netgraph since ng_callout() makes my
node suddenly a WRITER node (therefore non reentrant) for the duration of
the call.

So as soon as the callout function returns, all following packets will get
directly passed to the node again and when the ngintr thread gets executed
then only then will I get the queued packets. This introduces out of order
packets in the flow. I am using the current patch below to solve the issue
and I am wondering if there is anything wrong with it (and maybe contribute
back :).

>How-To-Repeat:
Have a constant stream of packets sent to a netgraph node. The node must at some point have registered a timeout function. Lets have it exceute every 10 miliseconds. Track the order (looking at ip id) of the packets as they come out. You should see out of order packets whenever the callout function is called.
>Fix:
Use the provided patch.

Patch attached with submission follows:

@@ -3632,7 +3632,12 @@ ng_callout(struct callout *c, node_p node, hook_p
hook, int ticks,

        if ((item = ng_alloc_item(NGQF_FN, NG_NOFLAGS)) == NULL)
                return (ENOMEM);

-       item->el_flags |= NGQF_WRITER;
+       if ((node->nd_flags & NGF_FORCE_WRITER) ||
+           (hook && (hook->hk_flags & HK_FORCE_WRITER)))

+         item->el_flags |= NGQF_WRITER;
+       else

+         item->el_flags |= NGQF_READER;
+
        NG_NODE_REF(node);              /* and one for the item */
        NGI_SET_NODE(item, node);
        if (hook) {


>Release-Note:
>Audit-Trail:
>Unformatted: