Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices

2016-09-30 Thread Toke Høiland-Jørgensen
Johannes Berg  writes:

>> > I kinda see the logic here - we really don't need to queue as much
>> > if we can't possibly transmit it out quickly - but it seems to me
>> > we should also throw in some kind of limit that's relative to the
>> > amount of memory you have on the system?
>> 
>> Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
>> carries a patch for that which just changes the hard-coded default to
>> another hard-coded default. Not sure how to get a good value to use,
>> though; and deciding on how large a fraction of memory to use for
>> packets starts smelling an awful lot like setting policy in the
>> kernel, doesn't it?
>
> Yeah, I agree it does seem awkward.
>
> Perhaps we should instead pick a low limit and let users change it
> more easily (i.e. not debugfs)? I don't know a good answer to this
> either.

Hmm, I'll talk it over with some of the LEDE people who are more used to
dealing with these sorts of memory-constrained devices than I am. Will
send a patch if we come up with a good solution :)

-Toke


Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices

2016-09-30 Thread Johannes Berg

> > I kinda see the logic here - we really don't need to queue as much
> > if we can't possibly transmit it out quickly - but it seems to me
> > we should also throw in some kind of limit that's relative to the
> > amount of memory you have on the system?
> 
> Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
> carries a patch for that which just changes the hard-coded default to
> another hard-coded default. Not sure how to get a good value to use,
> though; and deciding on how large a fraction of memory to use for
> packets starts smelling an awful lot like setting policy in the
> kernel, doesn't it?

Yeah, I agree it does seem awkward.

Perhaps we should instead pick a low limit and let users change it more
easily (i.e. not debugfs)? I don't know a good answer to this either.

johannes


Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices

2016-09-30 Thread Toke Høiland-Jørgensen
Johannes Berg  writes:

> On Fri, 2016-09-23 at 21:59 +0200, Toke Høiland-Jørgensen wrote:
>> Small devices can run out of memory from queueing too many packets.
>> If VHT is not supported by the PHY, having more than 4 MBytes of
>> total queue in the TXQ intermediate queues is not needed, and so we
>> can safely limit the memory usage in these cases and avoid OOM.
>
> I kinda see the logic here - we really don't need to queue as much if
> we can't possibly transmit it out quickly - but it seems to me we
> should also throw in some kind of limit that's relative to the amount
> of memory you have on the system?

Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently
carries a patch for that which just changes the hard-coded default to
another hard-coded default. Not sure how to get a good value to use,
though; and deciding on how large a fraction of memory to use for
packets starts smelling an awful lot like setting policy in the kernel,
doesn't it?

> I've applied these anyway though. I just don't like your assumption (b)
> much as the rationale for it.

Right, thanks. I'll come up with a better rationale next time ;)

-Toke


Re: [PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices

2016-09-30 Thread Johannes Berg
On Fri, 2016-09-23 at 21:59 +0200, Toke Høiland-Jørgensen wrote:
> Small devices can run out of memory from queueing too many packets.
> If VHT is not supported by the PHY, having more than 4 MBytes of
> total queue in the TXQ intermediate queues is not needed, and so we
> can safely limit the memory usage in these cases and avoid OOM.

I kinda see the logic here - we really don't need to queue as much if
we can't possibly transmit it out quickly - but it seems to me we
should also throw in some kind of limit that's relative to the amount
of memory you have on the system?

I've applied these anyway though. I just don't like your assumption (b)
much as the rationale for it.

johannes


[PATCH 3/3] mac80211: Set lower memory limit for non-VHT devices

2016-09-23 Thread Toke Høiland-Jørgensen
Small devices can run out of memory from queueing too many packets. If
VHT is not supported by the PHY, having more than 4 MBytes of total
queue in the TXQ intermediate queues is not needed, and so we can safely
limit the memory usage in these cases and avoid OOM.

Signed-off-by: Toke Høiland-Jørgensen 
---
 net/mac80211/tx.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 1ff08be..82f41fc 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1434,6 +1434,8 @@ int ieee80211_txq_setup_flows(struct ieee80211_local 
*local)
struct fq *fq = &local->fq;
int ret;
int i;
+   bool supp_vht = false;
+   enum nl80211_band band;
 
if (!local->ops->wake_tx_queue)
return 0;
@@ -1442,6 +1444,22 @@ int ieee80211_txq_setup_flows(struct ieee80211_local 
*local)
if (ret)
return ret;
 
+   /*
+* If the hardware doesn't support VHT, it is safe to limit the maximum
+* queue size. 4 Mbytes is 64 max-size aggregates in 802.11n.
+*/
+   for (band = 0; band < NUM_NL80211_BANDS; band++) {
+   struct ieee80211_supported_band *sband;
+
+   sband = local->hw.wiphy->bands[band];
+   if (!sband)
+   continue;
+
+   supp_vht = supp_vht || sband->vht_cap.vht_supported;
+   }
+   if (!supp_vht)
+   fq->memory_limit = 4 << 20; /* 4 Mbytes */
+
codel_params_init(&local->cparams);
local->cparams.interval = MS2TIME(100);
local->cparams.target = MS2TIME(20);
-- 
2.9.3