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;
Message-ID: <896183390a396e8e0508622eceb3664effcb3c30.camel@gmail.com>
Subject: Re: [RFC 0/4] Allow live MAC address change
From:   James Prestwood <prestwoj@gmail.com>
To:     Johannes Berg <johannes@sipsolutions.net>,
        linux-wireless@vger.kernel.org
Date:   Wed, 11 Sep 2019 12:20:55 -0700
In-Reply-To: <4909a428ee6fef2bf8b0e61841bc88062f534b13.camel@sipsolutions.net>
References: <20190904191155.28056-1-prestwoj@gmail.com>
         (sfid-20190904_221357_305070_478BF6CA) <d776271eac8b7cd24da6dbd21fb87186b30a0e7f.camel@sipsolutions.net>
         <4c43ea6a74cacc61184bc5b1387fecaa40711369.camel@gmail.com>
         (sfid-20190911_175613_316165_C021A0FB) <4909a428ee6fef2bf8b0e61841bc88062f534b13.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi Johannes,

> > Out of curiosity how this behavior is different than the power down
> > +
> > RTNL MAC change (the current way of doing things)? If you power
> > down
> > the device, change the MAC, then power up does that MAC get reset
> > after
> > a disconnection/failure?
> 
> No, of course not? But then you're explicitly issuing a command
> ("change
> the MAC address") that is supposed to affect state indefinitely, vs.
> issuing a command ("please connect") that isn't really meant to.

I see what your saying, but theses kind of state changes are all over
the place in other APIs, and undocumented: One example is
SCAN_FLAG_FLUSH clearing out the scanning state for all other
processes. I'm sure I could find more. If we documented this attribute
and behavior I don't see an issue.

This is also no different than changing the MAC via SET_INTERFACE and
having CMD_CONNECT fail; the MAC still persists but instead we payed an
extra 3ms for the same result.

I know 3ms doesn't seems like a lot but everything counts and from my
testing this is even a further 20% improvement to doing so with RTNL.
Plus the added simplicity to the userspace code/API. We have taken a
lot of time to optimize IWD's connection times, and everything counts.
The connection times are fast already, but when there is room for
improvement we will push for it, especially in situations like this
when the change is quite minimal and does not introduce much
complexity.

> 
> If there was one thing that we learned from wext, IMHO it was that
> keeping all the state in the kernel is bad for you, and it's much
> better
> to handle things if the state gets reset when you disconnect etc. In
> most places that's what we do now and I think it has served us well,
> so
> I'm very reluctant to mix things that need state in the kernel with
> those that don't.

I don't really agree that this change puts any more or less state in
the kernel. Even the current way of doing things userspace still needs
to maintain what it changed the MAC to, here is no different. And
again, documenting this attribute should leave no question about what
is happening.

> 
> (You might not remember wext, but you'd have to issue a bunch of
> commands in the right order, and it would keep all the state
> inbetween;

I was not around for that, but yes that sounds bad. The difference
though is we are not issuing a bunch of state changing commands in a
row to achieve a single goal. We are issuing one single command,
CMD_CONNECT or CMD_AUTHENTICATE.

Thanks,
James

> if you forgot to clear the BSSID after setting it, it'd be remembered
> and you couldn't connect to a new AP unless you reset it, etc.)
> 
> Thanks,
> johannes
>