linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: greearb@candelatech.com, linux-wireless@vger.kernel.org
Subject: Re: [PATCH] mac80211: do not iterate active interfaces when in re-configure
Date: Thu, 30 Jul 2020 13:48:43 +0200	[thread overview]
Message-ID: <a3a6a9303eeaf91f83bcbc413ad0782659218966.camel@sipsolutions.net> (raw)
In-Reply-To: <20200525165317.2269-1-greearb@candelatech.com> (sfid-20200525_185328_305694_D3007257)

On Mon, 2020-05-25 at 09:53 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> This appears to fix a problem where ath10k firmware would crash,


"appears to", heh

Really though, in general, you need to start thinking about mac80211 and
the drivers as two separate things. You've also submitted another patch
where you say "this iwlwifi thing requires mac80211 to change", and here
you're submitting a patch saying "this ath10k thing requires mac80211 to
change", but I don't see you considering much the fact that mac80211 is
actually used for both.

It'd be good to have a discussion of such things in the commit log for
changes that will affect multiple drivers, rather than focusing on a
single bug for a single driver. In general, not just in this patch.

> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 5db2cd0..186a696 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -831,7 +831,7 @@ static void __iterate_interfaces(struct ieee80211_local *local,
>  			break;
>  		}
>  		if (!(iter_flags & IEEE80211_IFACE_ITER_RESUME_ALL) &&
> -		    active_only && !(sdata->flags & IEEE80211_SDATA_IN_DRIVER))
> +		    (active_only && (local->in_reconfig || !(sdata->flags & IEEE80211_SDATA_IN_DRIVER))))
>  			continue;

Anyway, this seems wrong to me. If anything, it should skip those
interfaces that weren't re-added yet, but not all of them. I'm pretty
sure this would cause iwlwifi to misbehave with multiple interfaces, as
it sometimes relies on iterating to understand what else is going on
before configuring something.

It might even be that this can only be done subject to driver choice.

johannes


  reply	other threads:[~2020-07-30 11:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-25 16:53 [PATCH] mac80211: do not iterate active interfaces when in re-configure greearb
2020-07-30 11:48 ` Johannes Berg [this message]
2020-07-30 13:05   ` Ben Greear
2020-07-30 13:13     ` Johannes Berg
2020-07-30 13:27       ` Ben Greear
2020-07-30 13:41         ` Johannes Berg
2020-07-30 14:52           ` Ben Greear
2020-07-30 15:03             ` Johannes Berg
2020-09-16 22:28               ` Ben Greear
2020-09-21  8:50               ` Kalle Valo

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=a3a6a9303eeaf91f83bcbc413ad0782659218966.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=greearb@candelatech.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).