Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 16 Apr 2014 17:59:26 -0400
From:      Karim Fodil-Lemelin <fodillemlinkarim@gmail.com>
To:        freebsd-net@freebsd.org
Subject:   Re: Preventing ng_callout() timeouts to trigger packet queuing
Message-ID:  <534EFD3E.3010006@gmail.com>
In-Reply-To: <53483B8B.7030409@freebsd.org>
References:  <53459C96.5040304@gmail.com> <5345BAE7.4010501@gmail.com> <53483B8B.7030409@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi Julian,

The code behaves as you described. The intent is clearly there and I can 
confirm the behavior is correct for incoming packets. At this point it 
looks like turning off net.isr.direct makes a difference too so I'm 
trying to understand the relationship between the system's queuing 
behavior and netgraph's. The only common ground I can find at this point 
is they both use the "swi1: net" thread.

There may be a higher level design concern with forcing WRITER on 
callout items. It seems that, in some cases, it is desirable to prevent 
queuing packets if a timer event (ng_callout) can execute 
asynchronously, but I digress.

Regards,

Karim.

On 11/04/2014 2:59 PM, Julian Elischer wrote:
> disclaimer: I'm not looking at the code now.. I want to go to bed:  :-)
>
> When I wrote that code, the idea was that even a direct node execution 
> should become a queuing operation if there was already something else 
> on the queue.  so in that model packets were not supposed to get 
> re-ordered. does that not still work?
>
> Either that, or you need to explain the problem to me a bit better..
>
>
> On 4/10/14, 5:25 AM, Karim Fodil-Lemelin wrote:
>> Hi,
>>
>> Below is a revised patch for this issue. It accounts for nodes or 
>> hooks that explicitly need to be queuing:
>>
>> @@ -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) {
>>
>> Regards,
>>
>> Karim.
>>
>> On 09/04/2014 3:16 PM, Karim Fodil-Lemelin wrote:
>>> Hi List,
>>>
>>> I'm calling out to the general wisdom ... 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 :):
>>>
>>>
>>> @@ -3632,7 +3632,7 @@ 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;
>>> +       item->el_flags = NGQF_READER;
>>>         NG_NODE_REF(node);              /* and one for the item */
>>>         NGI_SET_NODE(item, node);
>>>         if (hook) {
>>>
>>>
>>> Best regards,
>>>
>>> Karim.
>>> _______________________________________________
>>> freebsd-net@freebsd.org mailing list
>>> http://lists.freebsd.org/mailman/listinfo/freebsd-net
>>> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
>>
>> _______________________________________________
>> freebsd-net@freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-net
>> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
>>
>
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?534EFD3E.3010006>