All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Kevin Lund <kglund@google.com>
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org
Subject: Re: [PATCH 2/2] wifi: mwifiex: Stop rejecting connection attempts while connected
Date: Thu, 3 Aug 2023 18:21:07 -0700	[thread overview]
Message-ID: <ZMxSg1SmALzTSyGD@google.com> (raw)
In-Reply-To: <20230602225751.164525-2-kglund@google.com>

On Fri, Jun 02, 2023 at 04:57:51PM -0600, Kevin Lund wrote:
> Currently, the Marvell WiFi driver rejects any connection attmept while
> we are currently connected. This is poor logic, since there are several
> legitimate use-cases for initiating a connection attempt while
> connected, including re-association and BSS Transitioning. This logic
> means that it's impossible for userspace to request the driver to
> connect to a different BSS on the same ESS without explicitly requesting
> a disconnect first.
> 
> Remove the check from the driver so that we can complete BSS transitions
> on the first attempt.
> 
> Testing on Chrome OS has shown that this change resolves some issues
> with failed BSS transitions.
> 
> Signed-off-by: Kevin Lund <kglund@google.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 6 ------
>  1 file changed, 6 deletions(-)

I've been told this one might need an extra look, but first off, it's
marked Rejected, likely due to feedback on patch 1, so probably needs a
resubmit if it needs consideration:

https://patchwork.kernel.org/project/linux-wireless/patch/20230602225751.164525-2-kglund@google.com/

But, did you attempt any background work on this, to determine why it
exists, or what other mitigations are in place? For example, I see that
sme.c's cfg80211_connect() makes a similar check, but only rejects
things if the SSID is different. So with that understanding, it's a
reasonable guess to say that mwifiex would be OK with just relying on
the existing cfg80211 checks instead.

In other words, I think this patch may be OK, but probably could use a
bit more explanation.

Also, how does "BSS Transitioning" (in your description) fit in here?
IIUC, cfg80211_connect() doesn't support that, as it only allows
reassociation to the same BSSID.
(Or, I might be confused here.)

Brian

  reply	other threads:[~2023-08-04  1:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-02 22:57 [PATCH 1/2] wifi: cfg80211: Reject (re-)association to the same BSSID Kevin Lund
2023-06-02 22:57 ` [PATCH 2/2] wifi: mwifiex: Stop rejecting connection attempts while connected Kevin Lund
2023-08-04  1:21   ` Brian Norris [this message]
2023-08-07 22:35     ` Kevin Lund
2023-08-15  0:21       ` Brian Norris
2023-06-03  3:32 ` [PATCH 1/2] wifi: cfg80211: Reject (re-)association to the same BSSID kernel test robot
2023-06-03  5:55 ` kernel test robot
2023-06-05 16:42 ` Johannes Berg
2023-08-04 17:29   ` Kevin Lund

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=ZMxSg1SmALzTSyGD@google.com \
    --to=briannorris@chromium.org \
    --cc=johannes@sipsolutions.net \
    --cc=kglund@google.com \
    --cc=linux-wireless@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.