Re: [PATCH] add AllowRoaming station property

[Denis Kenzior <denkenz@gmail.com>]

Hi Pedro,

On 10/19/23 06:55, Pedro André wrote:
> Hi Denis,
> 
> First of all, thank you for your feedback.
> 

No worries, I'm a bit swamped these days so my responses are sometimes delayed. 
Just FYI.

> 
> There is a use case where a device can be connected to an infrastructure
> AP and hosting its own AP. In this specific case it would be ideal to
> disable roaming since swapping the infrastructure AP can lead to
> instability on devices connected to the AP being hosted. In this case,
> the device should act as if it is not able to roam, so I was under the
> impression it would be OK to not honor AP directed roaming.

So AP directed roaming is a bit of a unicorn.  I haven't really seen it used in 
practice.  However, when it is enabled, it is usually pretty aggressive.  If you 
don't honor the roam request you will likely get disconnected.  Don't know if 
this impacts your design.

> 
> This setting would also need to be dynamic (i.e. set during runtime),
> since roaming might be needed during the same session if the use case
> changes (i.e. we no longer need to host an AP).

Sure, that makes sense.  I do think we need the ability to track whether this 
feature is being actively used (i.e. by tracking the D-Bus client that set the 
property) and go back to the default behavior if the client crashes, etc. 
Here's an example of how this was handled in oFono:

https://git.kernel.org/pub/scm/network/ofono/ofono.git/tree/doc/modem-api.txt#n42
https://git.kernel.org/pub/scm/network/ofono/ofono.git/tree/src/modem.c#n1022

> 
> If we are in the middle of a roam, I would say we can let it finish and
> only after do we start honoring the AllowRoaming setting.

Ok, fair enough.

> 
> Furthermore, you raise a very valid point in regards to checking if we
> should roam once the property goes back to default. I can add this logic
> to the next iteration of the patch if you think it is worth it.
> 

Yes, I think that would be nice.

> It does indeed overlap entirely with the "DisableRoamingScan" setting.
> The only difference is that "AllowRoaming" can be set during runtime
> trough D-Bus. Having a dynamic property is ideal for our use case as
> described above. If you think it is preferable, we can also merge the two.

DisableRoamingScan is something that isn't used very widely.  We don't even have 
auto tests for it for example.  My vote would be to combine the two and put 
DisableRoamingScan on a path to depreciation and eventual removal.

Regards,
-Denis