Re: RFC: softint-based if_input

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

On Thu, Feb 4, 2016 at 3:52 AM, Taylor R Campbell
<campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
>    Date: Wed, 3 Feb 2016 14:55:31 +0900
>    From: Ryota Ozaki <ozaki-r%netbsd.org@localhost>
>
>    Here is a new patch:
>      http://www.netbsd.org/~ozaki-r/softint-if_input-percpuq2.diff
>    The diff from v1 is here:
>      http://www.netbsd.org/~ozaki-r/softint-if_input-percpuq2-diff.diff
>
> Some comments:
>
> - if_percpuq_softint blocks interrupts while it processes the whole
> queue.  It seems likely to me that this will limit throughput by
> preventing packets from coming in to the NIC at the same time as we're
> processing them in the kernel.

I measured both cases, one big splnet vs. splnet per packet,
and there were no noticeable differences. In my measurements, the
former was slightly better than the latter. Under high load incoming
packets exceeds the number of packets that if_percpuq_softint can
process. In that case, packets are dropped at percpuq. One big splnet
helps to suppress the driver fetches and queues too much packets to
percpuq.

That said, the difference is not big deal in favor of polling mode :)

>
> - if_percpuq_dequeue must be called under splnet, unlike
> if_percpuq_enqueue.  If you want this to be part of the contract, it
> should be documented or at least commented.  But I think you probably
> want to push splnet into if_percpuq_dequeue, for symmetry with
> if_percpuq_enqueue and, more importantly, to let most of
> if_percpuq_softint run without splnet.

Yeah, splnet in if_percpuq_dequeue is more symmetric. I adopt it.

>
> - In a few cases, you've changed
>
>         if_attach(ifp);
>         somethingelse_attach(ifp);
>
> into
>
>         if_initialize(ifp);
>         somethingelse_attach(ifp);
>         if_register(ifp);
>
> E.g., bcmeth does ether_ifattach(ifp) in the middle with your patch.
> Why?
>
> In answer to my own question: because you changed the intended API to
> split if_attach into if_initialize/if_register back in 2014, and
> deprecated if_attach, and I even participated in the thread about
> that, but evidently forgot.  OK!
>
> https://mail-index.netbsd.org/tech-kern/2014/12/10/msg018242.html

Anyway I have to write some documents/comments for driver developers.
Will do.

>
> - You seem to have changed all the USB drivers to skip the ifq and go
> straight into the network stack.  While this more or less makes sense
> as long as IPL_SOFTUSB = IPL_SOFTNET, as I noted before I expect that
> IPL_SOFTUSB will change fairly soon to be IPL_SOFTSERIAL instead, in
> which case you will need to go through the ifq and softnet softint.
> So I think you probably want to treat the USB drivers like all the
> other ones you're converting in bulk.

Sure. The change may add some overhead though, it doesn't matter much.
I'll change so.

Thanks!
  ozaki-r