tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: RFC: softint-based if_input



   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.

- 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.

- 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

- 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.


Home | Main Index | Thread Index | Old Index