2016-11-29 18:05:57

by Ben Greear

[permalink] [raw]
Subject: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.

From: Ben Greear <[email protected]>

Could be useful for debugging memory consumption issues,
and perhaps power-save as well.

Signed-off-by: Ben Greear <[email protected]>
---

This is against 4.7.10+

net/mac80211/debugfs.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index b251b2f7..0b49b43 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -162,6 +162,30 @@ static ssize_t hwflags_read(struct file *file, char __user *user_buf,
return rv;
}

+static ssize_t misc_read(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ieee80211_local *local = file->private_data;
+ size_t bufsz = 1000;
+ char *buf = kzalloc(bufsz, GFP_KERNEL);
+ char *pos = buf, *end = buf + bufsz - 1;
+ ssize_t rv;
+ int i;
+ int ln;
+
+ pos += scnprintf(pos, end - pos, "pending:\n");
+
+ for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
+ ln = skb_queue_len(&local->pending[i]);
+ pos += scnprintf(pos, end - pos, "[%i] %d\n",
+ i, ln);
+ }
+
+ rv = simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
+ kfree(buf);
+ return rv;
+}
+
static ssize_t queues_read(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
@@ -182,6 +206,7 @@ static ssize_t queues_read(struct file *file, char __user *user_buf,

DEBUGFS_READONLY_FILE_OPS(hwflags);
DEBUGFS_READONLY_FILE_OPS(queues);
+DEBUGFS_READONLY_FILE_OPS(misc);

/* statistics stuff */

@@ -250,6 +275,7 @@ void debugfs_hw_add(struct ieee80211_local *local)
DEBUGFS_ADD(total_ps_buffered);
DEBUGFS_ADD(wep_iv);
DEBUGFS_ADD(queues);
+ DEBUGFS_ADD(misc);
#ifdef CONFIG_PM
DEBUGFS_ADD_MODE(reset, 0200);
#endif
--
2.4.11


2016-11-29 18:05:58

by Ben Greear

[permalink] [raw]
Subject: [PATCH 2/2] mac80211: put upper bound on txqi queue length.

From: Ben Greear <[email protected]>

This fixes OOM when using pktgen to drive a wifi station at more than
the station can transmit. pktgen uses ndo_start_xmit instead of going
through the queue layer, so it will not back off when the queues are
stopped, and would thus cause packets to be added to the txqi->queue
until the system goes OOM and crashes.

Signed-off-by: Ben Greear <[email protected]>
---

This is against 4.7.10+, not sure if it is actually needed in latest kernel.

net/mac80211/tx.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index fbcb5fc..0573ab9 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1293,6 +1293,16 @@ static void ieee80211_drv_tx(struct ieee80211_local *local,
goto tx_normal;

ac = txq->ac;
+
+ if (atomic_read(&sdata->txqs_len[ac]) >=
+ (local->hw.txq_ac_max_pending * 2)) {
+ /* Must be that something is not paying attention to
+ * max-pending, like pktgen, so just drop this frame.
+ */
+ ieee80211_free_txskb(&local->hw, skb);
+ return;
+ }
+
txqi = to_txq_info(txq);
atomic_inc(&sdata->txqs_len[ac]);
if (atomic_read(&sdata->txqs_len[ac]) >= local->hw.txq_ac_max_pending)
--
2.4.11

2016-12-05 14:47:22

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.



On 12/05/2016 05:59 AM, Johannes Berg wrote:
> +static ssize_t misc_read(struct file *file, char __user *user_buf,
>> + size_t count, loff_t *ppos)
>> +{
>> + struct ieee80211_local *local = file->private_data;
>> + size_t bufsz = 1000;
>> + char *buf = kzalloc(bufsz, GFP_KERNEL);
>
> You need at most IEEE80211_MAX_QUEUES * 16 (==256) which I think you
> can put on the stack?

I actually run with 64 queues in my tree, and either way, I thought large-ish
things on the stack were frowned upon for systems that want to run smaller stacks?

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2016-12-05 14:05:18

by Michal Kazior

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: put upper bound on txqi queue length.

On 5 December 2016 at 14:56, Johannes Berg <[email protected]> wrot=
e:
> On Tue, 2016-11-29 at 10:05 -0800, [email protected] wrote:
>> From: Ben Greear <[email protected]>
>>
>> This fixes OOM when using pktgen to drive a wifi station at more than
>> the station can transmit. pktgen uses ndo_start_xmit instead of
>> going
>> through the queue layer, so it will not back off when the queues are
>> stopped, and would thus cause packets to be added to the txqi->queue
>> until the system goes OOM and crashes.
>>
>> Signed-off-by: Ben Greear <[email protected]>
>> ---
>>
>> This is against 4.7.10+, not sure if it is actually needed in latest
>> kernel.
>
> One would hope that fq_tin_enqueue() does something like that, but
> anyway the patch doesn't apply so I'm dropping it.

fq_tin_enqueue() drops "fat" flow head packet upon reaching packet
count limit (8192) or memory limit (4 or 16 mbytes depending on vht
availability) whichever is hit first.


Micha=C5=82

2016-12-05 14:54:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.

On Mon, 2016-12-05 at 06:47 -0800, Ben Greear wrote:
>
> On 12/05/2016 05:59 AM, Johannes Berg wrote:
> >
> > +static ssize_t misc_read(struct file *file, char __user *user_buf,
> > >
> > > +  size_t count, loff_t *ppos)
> > > +{
> > > + struct ieee80211_local *local = file->private_data;
> > > + size_t bufsz = 1000;
> > > + char *buf = kzalloc(bufsz, GFP_KERNEL);
> >
> > You need at most IEEE80211_MAX_QUEUES * 16 (==256) which I think
> > you
> > can put on the stack?
>
> I actually run with 64 queues in my tree,

Heh, well, in that case the 1000 is actually potentially too small for
you :) (it'll work because there never are many packets on the queues
though)

> and either way, I thought large-ish things on the stack were frowned
> upon for systems that want to run smaller stacks?

Yeah but the limit is more like 1KB :)

Maybe allocate, but actually do 16*NUM_QUEUES? The 1000 is just
arbitrarily magic in there.

johannes

2016-12-05 14:52:13

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.

+static ssize_t misc_read(struct file *file, char __user *user_buf,
> +  size_t count, loff_t *ppos)
> +{
> + struct ieee80211_local *local = file->private_data;
> + size_t bufsz = 1000;
> + char *buf = kzalloc(bufsz, GFP_KERNEL);

You need at most IEEE80211_MAX_QUEUES * 16 (==256) which I think you
can put on the stack?

johannes

2016-12-05 14:58:18

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.



On 12/05/2016 06:54 AM, Johannes Berg wrote:
> On Mon, 2016-12-05 at 06:47 -0800, Ben Greear wrote:
>>
>> On 12/05/2016 05:59 AM, Johannes Berg wrote:
>>>
>>> +static ssize_t misc_read(struct file *file, char __user *user_buf,
>>>>
>>>> + size_t count, loff_t *ppos)
>>>> +{
>>>> + struct ieee80211_local *local = file->private_data;
>>>> + size_t bufsz = 1000;
>>>> + char *buf = kzalloc(bufsz, GFP_KERNEL);
>>>
>>> You need at most IEEE80211_MAX_QUEUES * 16 (==256) which I think
>>> you
>>> can put on the stack?
>>
>> I actually run with 64 queues in my tree,
>
> Heh, well, in that case the 1000 is actually potentially too small for
> you :) (it'll work because there never are many packets on the queues
> though)
>
>> and either way, I thought large-ish things on the stack were frowned
>> upon for systems that want to run smaller stacks?
>
> Yeah but the limit is more like 1KB :)
>
> Maybe allocate, but actually do 16*NUM_QUEUES? The 1000 is just
> arbitrarily magic in there.

Sounds good, I'll respin it.

Thanks,
Ben

>
> johannes
>

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2016-12-05 13:57:14

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: put upper bound on txqi queue length.

On Tue, 2016-11-29 at 10:05 -0800, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> This fixes OOM when using pktgen to drive a wifi station at more than
> the station can transmit.  pktgen uses ndo_start_xmit instead of
> going
> through the queue layer, so it will not back off when the queues are
> stopped, and would thus cause packets to be added to the txqi->queue
> until the system goes OOM and crashes.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
>
> This is against 4.7.10+, not sure if it is actually needed in latest
> kernel.

One would hope that fq_tin_enqueue() does something like that, but
anyway the patch doesn't apply so I'm dropping it.

johannes