On Sun, 30 Jul 2006, Sam Leffler wrote:
I have a fair amount of experience with the linux model and it works ok.
The main complication I've seen is when a driver needs to process multiple
queues of packets things get more involved. This is seen in 802.11 drivers
where there are two q's, one for data frames and one for management frames.
With the current scheme you have two separate queues and the start method
handles prioritization by polling the mgt q before the data q. If instead
the packet is passed to the start method then it needs to be tagged in some
way so the it's prioritized properly. Otherwise you end up with multiple
start methods; one per type of packet. I suspect this will be ok but the
end result will be that we'll need to add a priority field to mbufs (unless
we pass it as an arge to the start method).
All this is certainly doable but I think just replacing one mechanism with
the other (as you specified) is insufficient.
Hmm. This is something that I had overlooked. I was loosely aware that the
if_sl code made use of multiple queues, but was under the impression that the
classification to queues occured purely in the SLIP code. Indeed, it does,
but structurally, SLIP is split over the link layer (if_output) and driver
layer (if_start), which I had forgotten. I take it from your comments that
802.11 also does this, which I was not aware of.
I'm a little uncomfortable with our current m_tag model, as it requires
significant numbers of additional allocations and frees for each packet, as
well as walking link lists. It's fine for occasional discretionary use (i.e.,
MAC labels), but I worry about cases where it is used with every packet, and
we start seeing moderately non-zero numbers of tags on every packet. I think
I would be more comfortable with an explicit queue identifier argument to
if_start, where the link layer and driver layer agree on how to identify
queues.
As a straw man, how would the following strike you:
int if_startmbuf(struct ifnet *ifp, struct mbuf *m, int ifqid);
where for most link layers, the value would be zero, but for some link
layer/driver combinations, it would identify a specific queue which the link
layer believes the mbuf should be assigned, if implemented?
Attached is a patch that maintains the current if_start, but adds
if_startmbuf. If a device driver implements if_startmbuf and the global
sysctl net.startmbuf_enabled is set to 1, then the if_startmbuf path in the
driver will be used. Otherwise, if_start is used. I have modified the
if_em driver to implement if_startmbuf also. If there is no packet backlog
in the if_snd queue, it directly places the packet in the transmit
descriptor ring. If there is a backlog, it uses the if_snd queue protected
by driver mutex, rather than a separate ifq mutex.
In some basic local micro-benchmarks, I saw a 5% improvement in UDP 0-byte
paylod PPS on UP, and a 10% improvement on SMP. I saw a 1.7% performance
improvement in the bulk serving of 1k files over HTTP. These are only
micro-benchmarks, and reflect a configuration in which the CPU is unable to
keep up with the output rate of the 1gbps ethernet card in the device, so
reductions in host CPU usage are immediately visible in increased output as
the CPU is able to better keep up with the network hardware. Other
configurations are also of interest of interesting, especially ones in
which the network device is unable to keep up with the CPU, resulting in
more queueing.
Conceptual review as well as banchmarking, etc, would be most welcome.
Why is the startmbuf knob global and not per-interface? Seems like you want
to convert drivers one at a time?
I may have under-described what I have implemented. The decision is currently
made based on two factors: a global frob, and per-interface definition of
if_startmbuf being non-zero. The global frob is intended to make it easy to
benchmark the difference. I should modify the patch so that the global frob
doesn't override the driver back to if_start in the event that if_startmbuf is
defined and if_start isn't. The global frob is intended to be removed in the
long run, and I intend for us to continue to support both the old and new
start methods for the forseeable future, since I don't intend to update every
device driver we have to the new method, at least not personally :-).
FWIW the original model was driven by the expectation that you could raise
the spl so the tx path was entirely synchronized from above. With the SMPng
work we're synchronizing transfer through each control layer. If the driver
softc lock (or similar) were exposed to upper layers we could possibly
return the "lock the tx path" model we had before and eliminate all the
locking your changes target. But that would be a big layering violation and
would add significant contention in the SMP case.
In some ways, what I propose comes to much the same thing: the change I
propose basically delegates the queueing and synchronization decisions to the
device driver, which might choose either to use the lock already in the ifq,
to use its own lock, or to use some other synchronization strategy. In the
case of if_em, I've implemented bypass of software queueing entirely in the
common case, but in the event that the hardware ring backs up, then we still
fall back to the if_snd queue, only we lock it using the device driver's
transmit path mutex. Delegating the synchronization down the stack comes with
risks, as device driver writers will inevitably take liberties: on the other
hand, it appears that devices are quite diverse, and those liberties have
advantages.
I think the key observation is that most network hardware today takes
packets directly from private queues so the fast path needs to push things
down to those queues w/ minimal overhead. This includes devices that
implement QoS in h/w w/ multiple queues.
Yes -- however, you're right that the link layer needs to be able to pass more
information down. I'd like it to be able to do so without an m_tag
allocation, though, which suggests (as you point out) an explicit argument to
if_startmbuf.
Thanks,
Robert N M Watson
Computer Laboratory
University of Cambridge
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
