On 8 March 2016 at 00:06, Dave Taht <dave.taht@xxxxxxxxx> wrote:
> Dear Michal:
>
> Going through this patchset... (while watching it compile)
>
>
> + if (!local->hw.txq_cparams.target)
> + local->hw.txq_cparams.target = MS2TIME(5);
>
> MS2TIME(20) for now and/or add something saner to this than !*backlog
>
> target will not be a constant in the long run.
>
> + if (now - custom_codel_get_enqueue_time(skb) < p->target ||
> + !*backlog) {
> + /* went below - stay below for at least interval */
> + vars->first_above_time = 0;
> + return false;
> + }
>
>
> *backlog < some_sane_value_for_an_aggregate_for_this_station
>
> Unlike regular codel *backlog should be a ptr to the queuesize for
> this station, not the total queue.
>
> regular codel, by using the shared backlog for all queues, is trying
> to get to a 1 packet depth for all queues, (see commit:
> 865ec5523dadbedefbc5710a68969f686a28d928 ), and store the flow in the
> network, not the queue...
>
> BUT in wifi's case you want to provide good service to all stations,
> which is filling up an aggregate
> for each... (and varying the "sane_value_for_the_aggregate" to suit
> per sta service time requirements in a given round of all stations).
Hmm.. This actually makes me think that:
skb = codel_dequeue(flow, &flow->backlog, &flow->cvars,
&hw->txq_cparams, codel_get_time(), false);
is kind of wrong.
If you want to maintain per-sta aggregate-long queue this probably
needs a sta->backlog instead of flow->backlog (because you can have
multiple flows per station) in the first place.
Not quite sure about cvars though. Thoughts?
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
