On Thu, 2008-08-07 at 01:31 +0300, Tomas Winkler wrote: > On Fri, Aug 1, 2008 at 3:54 PM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote: > > On Fri, 2008-08-01 at 15:40 +0300, Tomas Winkler wrote: > >> > Right. So why are you saying we should have a separate qdisc for it? > >> > >> I need a sw queue for it. > > > > Sure, you need to buffer packets, but that's a whole different beast. > > So this is the most important point of this conversation... What do > you call this beast! Ok I see where we're going. You're thinking of "struct sk_buff_head" (which is part of most qdiscs) while I always thought you actually wanted a qdisc. > What I'm trying to say all the way along I don't need qdisc per se I > need like 80% of it. Right. > So it was already implemented in qdisc so it was utilized. > I'm not aware of anything else that is already implemented but you can > always open my eyes and I'm not trying to be cynical. Well there's struct sk_buff_head that allows you to queue up frames anywhere you want them to be, we use it for example for PS-buffered frames. Of course that means we need to implement it ourselves. > This is really theoretical question. Wireless has defined 4 AC stream > for simplicity > so you map 2 TIDs out of 8 into 4 AC stream. One possible view > of aggregation you don't map TID into AC stream but run it along to > get more fine grained. > Otherwise why don't we do <AC, RA> but <TID,RA> . Just a possible > view. after all it's mapped back to AC. Well 802.11 isn't really clear about this, it talks about QSTAs having no more than one frame per <TID,RA,SA> outstanding at a time. I don't have access to wi-fi documents but I suspect they add further constraints because it's not feasible to have that many packets buffered on the hardware. > >> Not scheduling mean not string to prioritize streams in SW. I guess it means RR. > > > > But that's entirely user dependent if we allow actual qdiscs. > > True but there is s always some sane default. So we should disallow changing qdiscs? I'm not entirely against that, but I think if we go that way we should force it in the code. > I suggest to start at some more productive point. > So what is important is that we agree on one thing that we need sw > queue/buffer (forget qdisc for now) for aggregation queue and the > question is how to implement it efficiently and how to resolve > cleanly the lack of requeueing in the new MQ. . Well we can clearly do requeuing as much as we like, but we lose the skb->cb over it. In fact, the master_start_xmit now makes that explicit by memsetting it to 0. I've looked into the Atheros driver, they way the implement it is that when frames for an aggregation session are passed to the ->tx() call they simply buffer them in software and when they have enough they enqueue all of them to the hardware queue for the particular AC they map into. This scales much better, with 16 TIDs and many stations you can that way possibly support a lot of concurrent aggregation sessions. Anyway, I'm confident you can work it out, I don't have time any more for it right now, as I'll announce in a separate email. I had a patch that mostly went the "just have queues per AC" route, but it was incomplete, if anybody wants to take a look it's at http://johannes.sipsolutions.net/patches/kernel/all/2008-08-04-12%3a26/006-fix-skb-cb-agg.patch Here's a recap of the current issues that I'm aware of: * select_queue cannot really look into any state because due to the lack of a single TX lock there's nothing to prevent it racing against the code starting/stopping the aggregation * you cannot re-queue and assume that skb->cb will be left intact, but that's codified now by clearing skb->cb on master_start_xmit * the current implementation of ieee80211_requeue is racy against select_queue, select_queue is called under RCU lock though so you can use synchronize_rcu to avoid the race, this requires moving ieee80211_requeue into process context though johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
