netdev - Re: [RFC/RFT] mac80211: implement fq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+BoTQ=ASe2o7kfiC8q6YOkNp9q6DF3Yc3XZgL=scVsw9VdqrA@mail.gmail.com>
Date:	Tue, 8 Mar 2016 11:57:41 +0100
From:	Michal Kazior <michal.kazior@...to.com>
To:	Dave Taht <dave.taht@...il.com>
Cc:	linux-wireless <linux-wireless@...r.kernel.org>,
	Johannes Berg <johannes@...solutions.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
	Felix Fietkau <nbd@...nwrt.org>,
	Tim Shepard <shep@...m.mit.edu>
Subject: Re: [RFC/RFT] mac80211: implement fq_codel for software queuing

On 8 March 2016 at 00:06, Dave Taht <dave.taht@...il.com> 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ł