All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Kan Yan <kyan@google.com>
Cc: Johannes Berg <johannes@sipsolutions.net>,
	linux-wireless@vger.kernel.org,
	make-wifi-fast@lists.bufferbloat.net,
	Felix Fietkau <nbd@nbd.name>, Yibo Zhao <yiboz@codeaurora.org>
Subject: Re: [PATCH 1/2] mac80211: Implement Airtime-based Queue Limit (AQL)
Date: Sun, 06 Oct 2019 19:40:41 +0200	[thread overview]
Message-ID: <87pnj9n55y.fsf@toke.dk> (raw)
In-Reply-To: <CA+iem5vJFRxskyHOKf5K73X8aGH965P4hoiCj-wQtK-Z-47pdg@mail.gmail.com>

Kan Yan <kyan@google.com> writes:

> Hi Johannes,
>
> Thanks for help review this patch.  I will fix all style errors.
>
>> > +ieee80211_txq_airtime_check(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
>> Why is it necessary to call this, vs. just returning NULL when an SKB is
>> requested?
> This function is also called by ath10k, from ath10k_mac_tx_can_push(),
> which returns a boolean.
>
>> However, I'm not sure it *shouldn't* actually be per interface (i.e.
>> moving from
>>         local->aql_total_pending_airtime
>> to
>>
>>         sdata->aql_total_pending_airtime
>>
>> because you could have multiple channels etc. involved and then using a
>> single airtime limit across two interfaces that are actually on two
>> different channels (e.g. 2.4 and 5 GHz) doesn't make that much sense.
>>
>> Actually, it does make some sense as long as the NIC is actually
>> channel-hopping ... but that's in the process of changing now, there's
>> going to be hardware really soon (or perhaps already exists) that has
>> real dual-band capabilities...
>
> That's a good point.  I haven't thought about real simultaneous dual
> band chipset and such chipset do exists now. Is RSDB support coming to
> mac80211 soon? Just curious if it will be just virtual interfaces or
> something else. I chose "local" instead of "sdata" thinking about the
> case of several virtual interfaces (AP, STA, MESH) operates in the
> same channel, then the interface total could be a better choice.
>
> I am ok with moving the "aql_total_pending_airtime" into sdata, but
> afraid that's not the most optimal choice for the case of multiple
> virtual interfaces operates in the same channel.
> Maybe we could leave it in "local" for now. What do you think?

I'd lean towards keeping it in 'local' for consistency with all the
other airtime stuff. For now, I think having multiple SSIDs on the same
radio is more common than the reverse (multiple bands on a single
radio).

In particular, the per-group airtime fairness stuff is definitely
designed on the assumption that all BSSes share the same band.

So if and when we start supporting true multi-band devices we'll have to
change these things anyway. So might as well keep everything together so
it all gets fixed :)

-Toke


  parent reply	other threads:[~2019-10-06 17:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-04  6:21 [PATCH 0/2] Implement Airtime-based Queue Limit (AQL) Kan Yan
2019-10-04  6:21 ` [PATCH 1/2] mac80211: " Kan Yan
2019-10-04 12:30   ` Johannes Berg
2019-10-04 12:34   ` Johannes Berg
2019-10-04 15:44   ` Toke Høiland-Jørgensen
2019-10-05  2:08     ` Kan Yan
2019-10-05  2:19       ` Kan Yan
2019-10-06 17:40       ` Toke Høiland-Jørgensen [this message]
2019-10-07 19:23         ` Johannes Berg
2019-10-07 19:32           ` [Make-wifi-fast] " Dave Taht
2019-10-07 19:40           ` Toke Høiland-Jørgensen
2019-10-07 19:43             ` Johannes Berg
2019-10-10  2:35               ` Kan Yan
2019-10-04  6:21 ` [PATCH 2/2] ath10k: Enable " Kan Yan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pnj9n55y.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=johannes@sipsolutions.net \
    --cc=kyan@google.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=make-wifi-fast@lists.bufferbloat.net \
    --cc=nbd@nbd.name \
    --cc=yiboz@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.