Date: Sun, 30 Jul 2006 18:02:51 -0700 From: Sam Leffler <sam@errno.com> To: Robert Watson <rwatson@FreeBSD.org> Cc: arch@FreeBSD.org, net@FreeBSD.org Subject: Re: Changes in the network interface queueing handoff model Message-ID: <44CD56BB.6080405@errno.com> In-Reply-To: <20060730200929.J16341@fledge.watson.org> References: <20060730141642.D16341@fledge.watson.org> <44CCFC2C.20402@errno.com> <20060730200929.J16341@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Robert Watson wrote: > 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. There are several issues here but the basic one is, I believe, that we need to provide a per-packet notion of priority or TOS handling. The distinction between mgt frame and data in 802.11 drivers is a kludge; the right thing is to just use priority to get the desired effect. But separately 802.11 is aware of priority for WME so independent of mgt frames priority we still need a way to pass down an AC (access control). For 802.11 I was able to do this by encoding the value in the mbuf flags. If there were a field in the mbuf header this kludge could be removed. For other devices we still want a way to pass around the DiffServ bits or similar so things like vlan priority can be set w/o resorting to tagging each frame. Ideally prioritization work like what's done inside slip should be pulled out. Note that just slapping a field in the mbuf is a start but we also need to think about how to handle it up+down the stack so layers can honor existing priorty and/or filling priority for packets that aren't already classified. > > 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? mbuf tags are not a solution; too expensive. I think we need something in the mbuf header. > >>> 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. Or an addition to the mbuf header. Sam
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?44CD56BB.6080405>