public inbox for linux-wireless@vger.kernel.org
 help / color / mirror / Atom feed
From: Felix Fietkau <nbd@nbd.name>
To: Georgi Valkov <gvalkov@abv.bg>
Cc: linux-wireless@vger.kernel.org, johannes@sipsolutions.net,
	openwrt-devel@lists.openwrt.org
Subject: Re: [PATCH v2] mac80211: fix regression in sta connection monitor
Date: Sat, 26 Sep 2020 08:11:17 +0200	[thread overview]
Message-ID: <3762d906-31ff-8386-b9ea-d7b06ea07397@nbd.name> (raw)
In-Reply-To: <FD88F09D-5FEA-4A15-B586-95843830F737@abv.bg>


Hi Georgi,

thanks for testing and for your insight into this issue.

On 2020-09-26 06:41, Georgi Valkov wrote:
> Hi Felix!
> 
> With your latest suggestion, it takes between 10 and 17 hours for the connection to drop, then long five minutes to reconnect.
> Notice the order of code execution in the original code of ieee80211_sta_tx_notify():
> probe_send_count is always cleared when ack is true. But before clearing probe_send_count, we need to check if ieee80211_is_any_nullfunc() returns true and probe_send_count > 0: if yes, we must call ieee80211_queue_work(). You cleared probe_send_count ahead of time, so the condition to run ieee80211_queue_work() will never be met.
The condition in ieee80211_sta_tx_notify may not be met, but
ieee80211_queue_work is called from ieee80211_tx_status_ext in that
case. Any idea why that is not enough?

> To spare your time, I did spend one week to find the cause, then another learning every detail about the code and testing various solutions, including those you proposed. While I do not have experience with mac80211’s design, I’m quite good at preserving the exact behaviour during large scale refactoring. And in my fix I tried changing as little as possible to keep the patch small, preserving both your changes and the original design behaviour.
And the reason I'm still proposing changes to it is because your patch
does not take into account no-skb or 802.3 tx status.
I'll try to make a v3 patch that leaves more of the original code intact
at the cost of a little more duplication.

- Felix

  reply	other threads:[~2020-09-26  6:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-22 10:24 [PATCH v2] mac80211: fix regression in sta connection monitor Felix Fietkau
2020-09-26  4:41 ` Georgi Valkov
2020-09-26  6:11   ` Felix Fietkau [this message]
2020-09-26  7:31     ` Georgi Valkov

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=3762d906-31ff-8386-b9ea-d7b06ea07397@nbd.name \
    --to=nbd@nbd.name \
    --cc=gvalkov@abv.bg \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=openwrt-devel@lists.openwrt.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox