Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 2/2] mac80211: Support LIVE_ADDRESS_CHANGE feature
To:     Johannes Berg <johannes@sipsolutions.net>,
        James Prestwood <prestwoj@gmail.com>,
        linux-wireless@vger.kernel.org
References: <20190913195908.7871-1-prestwoj@gmail.com>
 <20190913195908.7871-2-prestwoj@gmail.com>
 <c6835b5c5d2a97fa82b0fb21f7b7f0056aa42e1b.camel@sipsolutions.net>
 <90ae00044bc0834d87d3f9fb75ce63dce4cfadd5.camel@gmail.com>
 <0b57c1288016310050ccd6233dda886fc4a89b02.camel@gmail.com>
 <f468a8d573ddf401d2084b76eb625fef5950f265.camel@sipsolutions.net>
 <6fa34e4c-5c81-4875-da29-cada1a078e2c@gmail.com>
 <6530a6b06176790c5a6949d6ffccf37b506975bd.camel@sipsolutions.net>
From:   Denis Kenzior <denkenz@gmail.com>
Message-ID: <864267ec-9158-940d-6e0e-db84a395888e@gmail.com>
Date:   Tue, 8 Oct 2019 10:53:42 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <6530a6b06176790c5a6949d6ffccf37b506975bd.camel@sipsolutions.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi Johannes,

On 10/8/19 10:52 AM, Johannes Berg wrote:
> Hi,
> 
>>> You could have two interfaces, one which is scanning right now, right?
>>> And then theoretically you don't care about the other one - it *should*
>>> be OK to remove/re-add (with new MAC address) the one that *isn't*
>>> scanning, right?
>>
>> Actually, I don't think you can?  Unless I'm missing something?  All the
>> scan state is stored on struct ieee80211_local, so if that struct is
>> allocated per phy as you point out below, then what you suggest is
>> currently not possible?
> 
> ?
> 
> The scan_req struct contains a reference to which interface is scanning,
> so it should very well be possible to have
> 
> phy0:
>   wlan0: IFF_UP & scanning
>   wlan1: IFF_UP & change MAC address all the time
> 
> just like it's possible to change the MAC address when wlan1 *isn't*
> IFF_UP even if wlan0 is scanning, right?
> 

Indeed.  But that is not what you were suggesting earlier with just 
checking local->scanning.  So if scan_req contains a wdev, then yes it 
should be possible to compare the scan_req->wdev to the interface being 
changed.

>>> But we don't have that granularity here for anything - you're just
>>> checking "sdata->local->something", and by going from sdata to local
>>> you've now checked the whole NIC, not just a single interface on that
>>> NIC.
>>
>> Right.  But that seems to be a limitation of mac80211 actually.  We
>> can't run two scans concurrently on different interfaces.  This is
>> rather unintuitive given that scan requests require an ifindex/wdev.
>>
>> Can this be changed / fixed in mac80211 actually?  I would expect that
>> if a card supports p2p and station simultaneously, then it can scan / go
>> offchannel on two interfaces simultaneously? Or not?  What can iwlwifi
>> do for example?
> 
> No, this typically cannot be fixed, and it doesn't really make sense.
> The NIC cannot possibly do two scans at a time since it has only a
> single radio resource :-)

So why is the scan request not per phy then?  And should mac address 
even affect the ongoing scan?  Can we simply change it with the scan 
ongoing?

> 
>> Given the above, I'm not sure I see anything wrong?  The switch/case can
>> probably be gotten rid of, but it actually makes things clear that only
>> station/p2p_device and adhoc are handled specially.
> 
> I just don't think they *should* be handled specially.
> 

Fair enough.

> Given your code now, you can have
> 
> phy0:
>   wlan0: STATION, IFF_UP & something is doing remain-on-channel
>   wlan1: STATION, IFF_UP
> 
> --> cannot change wlan1 MAC address
> 
> phy0:
>   wlan0: STATION, IFF_UP & something is doing remain-on-channel
> 
> wlan1: AP, IFF_UP
> 
> --> *can* change wlan1 MAC address
> 
> This doesn't really make much sense?
> 

Agreed.

Regards,
-Denis