Hi,

I have been working on CQM RSSI notification support for the brcmfmac 
driver so that roaming can be properly offloaded to iwd. I want to ask 
about this particular behaviour in iwd's station code:

     static void station_roamed(struct station *station)
     {
         /*
          * New signal high/low notification should occur on the next
          * beacon from new AP.
          */
         station->signal_low = false;
         station->roam_min_time.tv_sec = 0;
         station->roam_no_orig_ap = false;
         station->roam_scan_full = false;

         ...
     }

When I look at the kernel API, this doesn't seem quite right to me. At 
least, there doesn't seem to be any onus on the driver to repeat itself. 
This led me to experience the following scenario during my testing:

     - station is connected to a BSS
     - driver notifies RSSI LOW
     - iwd roams to a new BSS
     - RSSI happens to remain LOW
     - driver doesn't notify RSSI LOW, because it never went HIGH

This leads to us getting stuck with a low RSSI, even though iwd should 
probably try roaming again because it was never informed that the RSSI 
went HIGH. In practice, it means that we might sooner disconnect 
entirely rather than roam to better BSS.

Would it not be more prudent to continue assuming that signal_low == 
true, and schedule another roam in station_roamed() to cover for this 
scenario? Something like what is already done in station_roam_failed():

     static void station_roam_failed(struct station *station)
     {
         ...

     delayed_retry:
         /*
          * If we're still connected to the old BSS, only clear 
preparing_roam
          * and reattempt in 60 seconds if signal level is still low at that
          * time.
          */
         station->preparing_roam = false;
         station->roam_scan_full = false;
         station->ap_directed_roaming = false;

         if (station->signal_low)
             station_roam_timeout_rearm(station, 60);
     }

If you like the suggestion then I can prepare a patch. But perhaps I 
have misunderstood the requirements of the CQM RSSI notification API in 
the kernel, and it should indeed always send a notification upon 
association to a new BSS, even if that happens to be redundant (e.g. LOW 
followed by LOW). In any case I would be happy to have some 
clarification on this.

Kind regards,
Alvin