[PATCH] mac80211: mesh: only warn if mesh peering is established

[PATCH] mac80211: mesh: only warn if mesh peering is established

[Markus Theil]

The following warning is triggered every time an unestablished mesh peer
gets dumped. This patch checks, if a peer link is established, when dum-
ping the airtime link metric.

[ 9563.022567] WARNING: CPU: 0 PID: 6287 at net/mac80211/mesh_hwmp.c:345
               airtime_link_metric_get+0xa2/0xb0 [mac80211]
[ 9563.022697] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3
[ 9563.022756] RIP: 0010:airtime_link_metric_get+0xa2/0xb0 [mac80211]
[ 9563.022838] Call Trace:
[ 9563.022897]  sta_set_sinfo+0x936/0xa10 [mac80211]
[ 9563.022964]  ieee80211_dump_station+0x6d/0x90 [mac80211]
[ 9563.023062]  nl80211_dump_station+0x154/0x2a0 [cfg80211]
[ 9563.023120]  netlink_dump+0x17b/0x370
[ 9563.023130]  netlink_recvmsg+0x2a4/0x480
[ 9563.023140]  ____sys_recvmsg+0xa6/0x160
[ 9563.023154]  ___sys_recvmsg+0x93/0xe0
[ 9563.023169]  __sys_recvmsg+0x7e/0xd0
[ 9563.023210]  do_syscall_64+0x4e/0x140
[ 9563.023217]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 net/mac80211/mesh_hwmp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 68af62306385..d69983370381 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -328,6 +328,9 @@ u32 airtime_link_metric_get(struct ieee80211_local *local,
 	unsigned long fail_avg =
 		ewma_mesh_fail_avg_read(&sta->mesh->fail_avg);
 
+	if (sta->mesh->plink_state != NL80211_PLINK_ESTAB)
+		return MAX_METRIC;
+
 	/* Try to get rate based on HW/SW RC algorithm.
 	 * Rate is returned in units of Kbps, correct this
 	 * to comply with airtime calculation units
-- 
2.24.0

Re: [PATCH] mac80211: mesh: only warn if mesh peering is established

[Johannes Berg]

On Tue, 2019-12-03 at 19:06 +0100, Markus Theil wrote:
> The following warning is triggered every time an unestablished mesh peer
> gets dumped. This patch checks, if a peer link is established, when dum-
> ping the airtime link metric.
> 
> [ 9563.022567] WARNING: CPU: 0 PID: 6287 at net/mac80211/mesh_hwmp.c:345
>                airtime_link_metric_get+0xa2/0xb0 [mac80211]
> [ 9563.022697] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3
> [ 9563.022756] RIP: 0010:airtime_link_metric_get+0xa2/0xb0 [mac80211]
> [ 9563.022838] Call Trace:
> [ 9563.022897]  sta_set_sinfo+0x936/0xa10 [mac80211]
> [ 9563.022964]  ieee80211_dump_station+0x6d/0x90 [mac80211]
> [ 9563.023062]  nl80211_dump_station+0x154/0x2a0 [cfg80211]
> [ 9563.023120]  netlink_dump+0x17b/0x370
> [ 9563.023130]  netlink_recvmsg+0x2a4/0x480
> [ 9563.023140]  ____sys_recvmsg+0xa6/0x160
> [ 9563.023154]  ___sys_recvmsg+0x93/0xe0
> [ 9563.023169]  __sys_recvmsg+0x7e/0xd0
> [ 9563.023210]  do_syscall_64+0x4e/0x140
> [ 9563.023217]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

OK, I can see how this happens.

However,

> +	if (sta->mesh->plink_state != NL80211_PLINK_ESTAB)
> +		return MAX_METRIC;
> +

I'm not really sure this is the right way to fix it?

I'm sure you observed this only when the link isn't established yet, but
it seems to me that even when a link is established it could still
happen?

Or are the frames that are necessary for link establishment enough to
always set the metric?

johannes

Re: [PATCH] mac80211: mesh: only warn if mesh peering is established

[Markus Theil]

On 12/13/19 10:13 AM, Johannes Berg wrote:
> On Tue, 2019-12-03 at 19:06 +0100, Markus Theil wrote:
>> The following warning is triggered every time an unestablished mesh peer
>> gets dumped. This patch checks, if a peer link is established, when dum-
>> ping the airtime link metric.
>>
>> [ 9563.022567] WARNING: CPU: 0 PID: 6287 at net/mac80211/mesh_hwmp.c:345
>>                airtime_link_metric_get+0xa2/0xb0 [mac80211]
>> [ 9563.022697] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3
>> [ 9563.022756] RIP: 0010:airtime_link_metric_get+0xa2/0xb0 [mac80211]
>> [ 9563.022838] Call Trace:
>> [ 9563.022897]  sta_set_sinfo+0x936/0xa10 [mac80211]
>> [ 9563.022964]  ieee80211_dump_station+0x6d/0x90 [mac80211]
>> [ 9563.023062]  nl80211_dump_station+0x154/0x2a0 [cfg80211]
>> [ 9563.023120]  netlink_dump+0x17b/0x370
>> [ 9563.023130]  netlink_recvmsg+0x2a4/0x480
>> [ 9563.023140]  ____sys_recvmsg+0xa6/0x160
>> [ 9563.023154]  ___sys_recvmsg+0x93/0xe0
>> [ 9563.023169]  __sys_recvmsg+0x7e/0xd0
>> [ 9563.023210]  do_syscall_64+0x4e/0x140
>> [ 9563.023217]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> OK, I can see how this happens.
>
> However,
>
>> +	if (sta->mesh->plink_state != NL80211_PLINK_ESTAB)
>> +		return MAX_METRIC;
>> +
> I'm not really sure this is the right way to fix it?
>
> I'm sure you observed this only when the link isn't established yet, but
> it seems to me that even when a link is established it could still
> happen?
>
> Or are the frames that are necessary for link establishment enough to
> always set the metric?
>
> johannes

The current mac80211 code initializes this moving average when setting
the peer link to established in sta_apply_mesh_params.

case NL80211_PLINK_ESTAB:
            ...
            ewma_mesh_tx_rate_avg_init(&sta->mesh->tx_rate_avg);
            /* init at low value */
            ewma_mesh_tx_rate_avg_add(&sta->mesh->tx_rate_avg, 10);
            break;

This ewma_mesh_tx_rate_avg_add is the only reference that I found in the
code. It seems, that this avg is only initialized and never updated
during the plink lifetime.

-- 
Markus Theil

Technische Universität Ilmenau, Fachgebiet Telematik/Rechnernetze
Postfach 100565
98684 Ilmenau, Germany

Phone: +49 3677 69-4582
Email: markus[dot]theil[at]tu-ilmenau[dot]de
Web: http://www.tu-ilmenau.de/telematik

Re: [PATCH] mac80211: mesh: only warn if mesh peering is established

[Markus Theil]

On 12/14/19 11:13 AM, Markus Theil wrote:
> On 12/13/19 10:13 AM, Johannes Berg wrote:
>> On Tue, 2019-12-03 at 19:06 +0100, Markus Theil wrote:
>>> The following warning is triggered every time an unestablished mesh peer
>>> gets dumped. This patch checks, if a peer link is established, when dum-
>>> ping the airtime link metric.
>>>
>>> [ 9563.022567] WARNING: CPU: 0 PID: 6287 at net/mac80211/mesh_hwmp.c:345
>>>                airtime_link_metric_get+0xa2/0xb0 [mac80211]
>>> [ 9563.022697] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3
>>> [ 9563.022756] RIP: 0010:airtime_link_metric_get+0xa2/0xb0 [mac80211]
>>> [ 9563.022838] Call Trace:
>>> [ 9563.022897]  sta_set_sinfo+0x936/0xa10 [mac80211]
>>> [ 9563.022964]  ieee80211_dump_station+0x6d/0x90 [mac80211]
>>> [ 9563.023062]  nl80211_dump_station+0x154/0x2a0 [cfg80211]
>>> [ 9563.023120]  netlink_dump+0x17b/0x370
>>> [ 9563.023130]  netlink_recvmsg+0x2a4/0x480
>>> [ 9563.023140]  ____sys_recvmsg+0xa6/0x160
>>> [ 9563.023154]  ___sys_recvmsg+0x93/0xe0
>>> [ 9563.023169]  __sys_recvmsg+0x7e/0xd0
>>> [ 9563.023210]  do_syscall_64+0x4e/0x140
>>> [ 9563.023217]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> OK, I can see how this happens.
>>
>> However,
>>
>>> +	if (sta->mesh->plink_state != NL80211_PLINK_ESTAB)
>>> +		return MAX_METRIC;
>>> +
>> I'm not really sure this is the right way to fix it?
>>
>> I'm sure you observed this only when the link isn't established yet, but
>> it seems to me that even when a link is established it could still
>> happen?
>>
>> Or are the frames that are necessary for link establishment enough to
>> always set the metric?
>>
>> johannes
> The current mac80211 code initializes this moving average when setting
> the peer link to established in sta_apply_mesh_params.
>
> case NL80211_PLINK_ESTAB:
>             ...
>             ewma_mesh_tx_rate_avg_init(&sta->mesh->tx_rate_avg);
>             /* init at low value */
>             ewma_mesh_tx_rate_avg_add(&sta->mesh->tx_rate_avg, 10);
>             break;
>
> This ewma_mesh_tx_rate_avg_add is the only reference that I found in the
> code. It seems, that this avg is only initialized and never updated
> during the plink lifetime.
I've overlooked that ieee80211s_update_metric updates the rate avg.