DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5AF9620665
Message-ID: <7ac1d7a5f25ebcd44107c3a19e0d8140f37227c6.camel@sipsolutions.net>
Subject: Re: Should we check netif_running in cfg80211_calculate_bi_data?
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Ben Greear <greearb@candelatech.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Date:   Tue, 23 Oct 2018 11:32:40 +0200
In-Reply-To: <8c876445-a065-ddb2-4dcf-e922f22a1b7d@candelatech.com> (sfid-20181022_222758_799406_E8076EAF)
References: <8c876445-a065-ddb2-4dcf-e922f22a1b7d@candelatech.com>
         (sfid-20181022_222758_799406_E8076EAF)
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Mon, 2018-10-22 at 13:27 -0700, Ben Greear wrote:
> I was testing on my 4.16 kernel with a bunch of VAPs and I had two configured
> for 100 beacon interval, and more at 240.  This failed for reasons I figured out
> (gcd logic), but even once I tried to configure the vaps for 240 interval they
> could not come up.  I am thinking maybe it was because I could only re-configure
> admin-up interfaces, and they couldn't come up due the gcd thing?
> 
> Anyway, while poking, I thought maybe the patch below would be helpful since
> we shouldn't care about admin-down interfaces in this case?
> 
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index fbc880e..56d7583 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -1541,6 +1541,9 @@ static void cfg80211_calculate_bi_data(struct wiphy *wiphy, u32 new_beacon_int,
>                  if (wdev->beacon_interval == *beacon_int_gcd)
>                          continue;
> 
> +               if (!netif_running(wdev->netdev))
> +                       continue;
> +

I don't think we'd ever get to this check, since those interfaces will
always have wdev->beacon_interval == 0, checked a few lines before this
code at the beginning of the loop? At least they should have, but a
quick check suggests that is true.

johannes