Re: workqueue for pr_input

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Sat, Jan 28, 2017 at 9:11 PM, Darren Reed <darrenr%netbsd.org@localhost> wrote:
> On 27/01/2017 11:51 PM, Ryota Ozaki wrote:
>>
>> On Thu, Jan 26, 2017 at 10:29 AM, Darren Reed <darrenr%netbsd.org@localhost> wrote:
>>>
>>> Apologies all for the blob of text :(
>>>
>>> I resent with better formatting but that's likely in a spam queue
>>> somewhere...
>>>
>>> So I'll try and summarise in a readable fashion:
>>>
>>> (1) patch introduces new locks for per-packet processing when the
>>> architectural goal is lock-less. What would this patch look like if done
>>> to be lock-less?
>>> (2) queuing has a memory cost. There are DoS attacks introduced that
>>> need to be mitigated.
>>> (4) does workqueue and this patch suggest that <sys/queue.h> needs
>>> enhancing for optimal processing of lists?
>>
>> http://www.netbsd.org/~ozaki-r/workqueue-pr_input-percpu.diff
>>
>> As per your comment, I changed the patch: (i) introduced percpu queues
>> and removed the lock and (ii) introduced the hard limit to each queue
>> to mitigate DoS, which is the same as other input queues such as
>> pktqueue and if_input percpuq.
>
>
> Looking better.
>
> In wqinit_work(), the mutex_enter/exit is inside a loop.
> What's being protected here?

Whole processing of pr_input protocols that aren't MP-safe yet.

>
> In wqinput_input(), there's a "wwl->wwl_dropped++".
> How do I see these when I do "netstat -s"?

I added sysctl nodes, instead of netstat -s, which are commonly used
for statistics of input queue such as pktqueue and percpuq.

>
>>> (3) do all packets need this workqueue approach or only some? Can
>>> selective queuing help with (2)?
>>
>> Workqueue is used only for ICMP/ICMPv6/CARP and packets on fast paths
>> aren't affected by the change. Even for the affected packets, the
>> influence by the change is small now by the above improvement.
>>
>> We might be able to use workqueue selectively per packet types say
>> ICMP message types, but there is a concern; it will introduce packet
>> reordering between packets (e.g., ICMP messages). I think we should
>> avoid the behavior, though I'm not sure the impact of the reordering.
>> And I don't so much like to introduce such an exceptional behavior
>> in the middle of the packet handling (pr_input is modestly a feasible
>> place to do, I think).
>
>
> I'm confused as to why the ordering of ICMP messages might be a problem.
> I don't think I've ever seen anyone suggest that this is necessary but maybe
> I just haven't read the correct section in host requirements.
> Can you please explain in more detail?

Hm, I think I overestimated the impact of the reordering; it can even happen
on the network and so fearing reordering in the host doesn't make sense.
Please ignore my claim about it.

That said, I'm still not inclined to introduce selective queuing. I think
it's an optimization and we can do it later if we find we really need it.

Thanks,
  ozaki-r