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

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 <redacted>
---
 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

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

Johannes Berg [off-list ref] writes:

On Fri, 2016-09-23 at 21:59 +0200, Toke Høiland-Jørgensen wrote:

quoted

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

quoted

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

Johannes Berg [off-list ref] writes:

quoted

quoted

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