[usteer] New aggressive roaming to support Intel Wifi Cards (and also, other devices)

From c43d6699e3a618a3269f3b299ee7673eb8f44099 Mon Sep 17 00:00:00 2001
From: Nils Hendrik Rottgardt <n.rottgardt@gmail.com>
Date: Sat, 19 Oct 2024 21:20:36 +0200
Subject: [usteer] New aggressive roaming to support Intel Wifi Cards
(and also
   other devices)

Intel Wifi Cards does not understand the actual implementation because
of missing disassociation_timer and disassociation_imminent = true. So
they and other devices send a BSS-TM-RESP with status=1 (error).
This patch with add some new options to correct this behavior and also
fix the wrong bss_transition_request call.

- Added aggressive roaming (disaccociation_timer) and corrected calling
bss_transition_request with disassociation_imminent = false.
- Added 3 new options in config to control aggressive roaming for all or
specific MAC addresses.

Signed-off-by: Nils Hendrik Rottgardt <n.rottgardt@gmail.com>
---
 band_steering.c                        | 11 ++++-
 local_node.c                           | 21 +++++++--
 main.c                                 |  2 +
 openwrt/usteer/files/etc/config/usteer |  9 ++++
 openwrt/usteer/files/etc/init.d/usteer |  4 +-
 policy.c                               | 20 +++++++--
 sta.c                                  | 26 +++++++++++
 ubus.c                                 | 61 +++++++++++++++++++-------
 usteer.h                               | 30 ++++++++++---
 9 files changed, 152 insertions(+), 32 deletions(-)

 struct sta_info *usteer_sta_info_get(struct sta *sta, struct
usteer_node *node, bool *create);
@@ -376,6 +389,9 @@ void config_get_node_up_script(struct blob_buf *buf);
 void config_set_ssid_list(struct blob_attr *data);
 void config_get_ssid_list(struct blob_buf *buf);
 
+void config_set_aggressive_mac_list(struct blob_attr *data);
+void config_get_aggressive_mac_list(struct blob_buf *buf);
+
 int usteer_interface_init(void);
 void usteer_interface_add(const char *name);
 void usteer_sta_node_cleanup(struct usteer_node *node);

Please keep white space and indent changes in a separate commit.
Regards,
Paul

> Op 19 okt 2024, om 21:24 heeft Nils Rottgardt <n.rottgardt@gmail.com> het volgende geschreven:
> 
> From c43d6699e3a618a3269f3b299ee7673eb8f44099 Mon Sep 17 00:00:00 2001
> From: Nils Hendrik Rottgardt <n.rottgardt@gmail.com>
> Date: Sat, 19 Oct 2024 21:20:36 +0200
> Subject: [usteer] New aggressive roaming to support Intel Wifi Cards
> (and also
>    other devices)
> 
> Intel Wifi Cards does not understand the actual implementation because
> of missing disassociation_timer and disassociation_imminent = true. So
> they and other devices send a BSS-TM-RESP with status=1 (error).
> This patch with add some new options to correct this behavior and also
> fix the wrong bss_transition_request call.
> 
> - Added aggressive roaming (disaccociation_timer) and corrected calling
> bss_transition_request with disassociation_imminent = false.
> - Added 3 new options in config to control aggressive roaming for all or
> specific MAC addresses.
> 
> Signed-off-by: Nils Hendrik Rottgardt <n.rottgardt@gmail.com>
> ---
>  band_steering.c                        | 11 ++++-
>  local_node.c                           | 21 +++++++--
>  main.c                                 |  2 +
>  openwrt/usteer/files/etc/config/usteer |  9 ++++
>  openwrt/usteer/files/etc/init.d/usteer |  4 +-
>  policy.c                               | 20 +++++++--
>  sta.c                                  | 26 +++++++++++
>  ubus.c                                 | 61 +++++++++++++++++++-------
>  usteer.h                               | 30 ++++++++++---
>  9 files changed, 152 insertions(+), 32 deletions(-)
> 
> diff --git a/band_steering.c b/band_steering.c
> index 7fce1df..7b2ee00 100644
> --- a/band_steering.c
> +++ b/band_steering.c
> @@ -86,13 +86,20 @@ void usteer_band_steering_perform_steer(struct
> usteer_local_node *ln)
>              continue;
>  
>          /* Skip clients with insufficient SNR-state */
> -        if (si->band_steering.below_snr) {
> +        if (si->band_steering.below_snr)
> +        {
>              si->band_steering.below_snr = false;
>              continue;
>          }
>  
>          if (si->bss_transition)
> -            usteer_ubus_band_steering_request(si);
> +        {
> +            // usteer_ubus_band_steering_request(si, 0, false, 100,
> true, 100);
> +            if (si->sta->aggressive)
> +                usteer_ubus_band_steering_request(si, 0, true,
> config.aggressive_disassoc_timer, true, config.aggressive_disassoc_timer);
> +            else
> +                usteer_ubus_band_steering_request(si, 0, false, 0,
> true, 100);
> +        }
>  
>          si->band_steering.below_snr = false;
>      }
> diff --git a/local_node.c b/local_node.c
> index e74d945..6aa7008 100644
> --- a/local_node.c
> +++ b/local_node.c
> @@ -748,7 +748,7 @@ usteer_local_node_process_bss_tm_queries(struct
> uloop_timeout *timeout)
>          if (!si)
>              continue;
>  
> -        usteer_ubus_bss_transition_request(si, query->dialog_token,
> false, false, validity_period, NULL);
> +        usteer_ubus_bss_transition_request(si, query->dialog_token,
> config.aggressive_all, validity_period, true, validity_period, NULL);
>      }
>  
>      /* Free pending queries we can not handle */
> @@ -977,8 +977,23 @@ void config_get_ssid_list(struct blob_buf *buf)
>          blobmsg_add_blob(buf, config.ssid_list);
>  }
>  
> -void
> -usteer_local_nodes_init(struct ubus_context *ctx)
> +void config_set_aggressive_mac_list(struct blob_attr *data)
> +{
> +    free(config.aggressive_mac_list);
> +
> +    if (data && blobmsg_len(data))
> +        config.aggressive_mac_list = blob_memdup(data);
> +    else
> +        config.aggressive_mac_list = NULL;
> +}
> +
> +void config_get_aggressive_mac_list(struct blob_buf *buf)
> +{
> +    if (config.aggressive_mac_list)
> +        blobmsg_add_blob(buf, config.aggressive_mac_list);
> +}
> +
> +void usteer_local_nodes_init(struct ubus_context *ctx)
>  {
>      usteer_register_events(ctx);
>      ubus_lookup(ctx, "hostapd.*", node_list_cb, NULL);
> diff --git a/main.c b/main.c
> index 99aa6ad..b07b624 100644
> --- a/main.c
> +++ b/main.c
> @@ -96,6 +96,8 @@ void usteer_init_defaults(void)
>      config.remote_update_interval = 1000;
>      config.initial_connect_delay = 0;
>      config.remote_node_timeout = 10;
> +    config.aggressive_all = false;
> +    config.aggressive_disassoc_timer = 100;
>  
>      config.steer_reject_timeout = 60000;
>  
> diff --git a/openwrt/usteer/files/etc/config/usteer
> b/openwrt/usteer/files/etc/config/usteer
> index f53c338..2fe47f3 100644
> --- a/openwrt/usteer/files/etc/config/usteer
> +++ b/openwrt/usteer/files/etc/config/usteer
> @@ -71,6 +71,15 @@ config usteer
>      # Timeout (ms) for which a client will not be steered after
> rejecting a BSS-transition-request
>      #option steer_reject_timeout 60000
>  
> +    # Use aggressvice roaming to push clients to another AP
> +    #option aggressive_all 0
> +
> +    # List of MACs to enable aggressive roaming on. If not set all macs
> will handled aggressive
> +    #list aggressive_mac_list ''
> +
> +    # Disassociation imminent tuner - in aggresive mode the time a
> client has to roam away before disconnected hardly
> +    #option aggressive_disassoc_timer 100
> +
>      # Timeout (in ms) after which a association following a
> disassociation is not seen
>      # as a roam
>      #option roam_process_timeout 5000
> diff --git a/openwrt/usteer/files/etc/init.d/usteer
> b/openwrt/usteer/files/etc/init.d/usteer
> index 07fd99e..fdc8211 100755
> --- a/openwrt/usteer/files/etc/init.d/usteer
> +++ b/openwrt/usteer/files/etc/init.d/usteer
> @@ -69,8 +69,10 @@ uci_usteer() {
>      uci_option_to_json_bool "$cfg" local_mode
>      uci_option_to_json_bool "$cfg" load_kick_enabled
>      uci_option_to_json_bool "$cfg" assoc_steering
> +    uci_option_to_json_bool "$cfg" aggressive_all
>      uci_option_to_json_string "$cfg" node_up_script
>      uci_option_to_json_string_array "$cfg" ssid_list
> +    uci_option_to_json_string_array "$cfg" aggressive_mac_list
>      uci_option_to_json_string_array "$cfg" event_log_types
>  
>      for opt in \
> @@ -84,7 +86,7 @@ uci_usteer() {
>          initial_connect_delay steer_reject_timeout roam_process_timeout\
>          roam_kick_delay roam_scan_tries roam_scan_timeout \
>          roam_scan_snr roam_scan_interval \
> -        roam_trigger_snr roam_trigger_interval \
> +        roam_trigger_snr roam_trigger_interval aggressive_disassoc_timer\
>          band_steering_interval band_steering_min_snr
> link_measurement_interval \
>          load_kick_threshold load_kick_delay load_kick_min_clients \
>          load_kick_reason_code
> diff --git a/policy.c b/policy.c
> index 8c5d244..856c03e 100644
> --- a/policy.c
> +++ b/policy.c
> @@ -370,8 +370,18 @@ usteer_roam_trigger_sm(struct usteer_local_node
> *ln, struct sta_info *si)
>              break;
>          }
>  
> -        usteer_ubus_bss_transition_request(si, 1, false, false, 100,
> candidate->node);
> -        si->kick_time = current_time + config.roam_kick_delay;
> +        if (si->sta->aggressive)
> +        {
> +            // TODO: Disaccociation Timer noch konfigurierbar machen
> +            usteer_ubus_bss_transition_request(si, 1, true,
> config.aggressive_disassoc_timer, true,
> config.aggressive_disassoc_timer, candidate->node);
> +            si->roam_disassoc_time = current_time + (100 * 100);
> +        }
> +        else
> +        {
> +            usteer_ubus_bss_transition_request(si, 1, false, 0, true,
> 100, candidate->node);
> +            si->kick_time = current_time + config.roam_kick_delay;
> +        }
> +
>          usteer_roam_set_state(si, ROAM_TRIGGER_IDLE, &ev);
>          break;
>      }
> @@ -400,7 +410,11 @@ bool usteer_policy_can_perform_roam(struct sta_info
> *si)
>      /* Skip if connection is established shorter than the
> trigger-interval */
>      if (current_time - si->connected_since < config.roam_trigger_interval)
>          return false;
> -    
> +
> +    /* Skip on aggressive roaming in progress - wait 10s after
> disassociation event*/
> +    if (current_time - si->roam_disassoc_time < 10000)
> +        return false;
> +
>      return true;
>  }
>  
> diff --git a/sta.c b/sta.c
> index ed7e40e..ee8cbb1 100644
> --- a/sta.c
> +++ b/sta.c
> @@ -76,6 +76,30 @@ usteer_sta_info_timeout(struct usteer_timeout_queue
> *q, struct usteer_timeout *t
>      usteer_sta_info_del(si);
>  }
>  
> +static void
> +usteer_sta_update_aggressive(struct sta *sta)
> +{
> +    struct blob_attr *cur;
> +    int rem;
> +    char sta_mac[18];
> +    sprintf(sta_mac, MAC_ADDR_FMT, MAC_ADDR_DATA(sta->addr));
> +
> +    if (config.aggressive_all)
> +        sta->aggressive = true;
> +    else
> +    {
> +        sta->aggressive = false;
> +        blobmsg_for_each_attr(cur, config.aggressive_mac_list, rem)
> +        {
> +            if (strcmp(blobmsg_get_string(cur), sta_mac) != 0)
> +                continue;
> +
> +            sta->aggressive = true;
> +            break;
> +        }
> +    }
> +}
> +
>  struct sta_info *
>  usteer_sta_info_get(struct sta *sta, struct usteer_node *node, bool
> *create)
>  {
> @@ -105,6 +129,8 @@ usteer_sta_info_get(struct sta *sta, struct
> usteer_node *node, bool *create)
>      si->created = current_time;
>      *create = true;
>  
> +    usteer_sta_update_aggressive(sta);
> +
>      /* Node is by default not connected. */
>      usteer_sta_disconnected(si);
>  
> diff --git a/ubus.c b/ubus.c
> index 40daf74..918255c 100644
> --- a/ubus.c
> +++ b/ubus.c
> @@ -162,6 +162,9 @@ struct cfg_item {
>      _cfg(U32, remote_update_interval), \
>      _cfg(U32, remote_node_timeout), \
>      _cfg(BOOL, assoc_steering), \
> +    _cfg(BOOL, aggressive_all), \
> +    _cfg(ARRAY_CB, aggressive_mac_list), \
> +    _cfg(U32, aggressive_disassoc_timer), \
>      _cfg(I32, min_connect_snr), \
>      _cfg(I32, min_snr), \
>      _cfg(U32, min_snr_kick_delay), \
> @@ -668,11 +671,12 @@ usteer_ubus_disassoc_add_neighbors(struct sta_info
> *si)
>  }
>  
>  int usteer_ubus_bss_transition_request(struct sta_info *si,
> -                       uint8_t dialog_token,
> -                       bool disassoc_imminent,
> -                       bool abridged,
> -                       uint8_t validity_period,
> -                       struct usteer_node *target_node)
> +                                       uint8_t dialog_token,
> +                                       bool disassoc_imminent,
> +                                       uint8_t disassoc_timer,
> +                                       bool abridged,
> +                                       uint8_t validity_period,
> +                                       struct usteer_node *target_node)
>  {
>      struct usteer_local_node *ln = container_of(si->node, struct
> usteer_local_node, node);
>  
> @@ -680,17 +684,32 @@ int usteer_ubus_bss_transition_request(struct
> sta_info *si,
>      blobmsg_printf(&b, "addr", MAC_ADDR_FMT, MAC_ADDR_DATA(si->sta->addr));
>      blobmsg_add_u32(&b, "dialog_token", dialog_token);
>      blobmsg_add_u8(&b, "disassociation_imminent", disassoc_imminent);
> +    if (disassoc_imminent)
> +    {
> +        blobmsg_add_u32(&b, "disassociation_timer", disassoc_timer);
> +    }
>      blobmsg_add_u8(&b, "abridged", abridged);
>      blobmsg_add_u32(&b, "validity_period", validity_period);
> -    if (!target_node) {
> +    if (!target_node)
> +    {
> +        // Add all known neighbors if no specific target set
> +        MSG(DEBUG, "ROAMING requested for sta=" MAC_ADDR_FMT " without
> target\n", MAC_ADDR_DATA(si->sta->addr));
>          usteer_ubus_disassoc_add_neighbors(si);
> -    } else {
> +    }
> +    else
> +    {
> +        MSG(DEBUG, "ROAMING requested for sta=" MAC_ADDR_FMT " to %s
> with disassociation timer %i\n", MAC_ADDR_DATA(si->sta->addr),
> usteer_node_name(target_node), disassoc_timer);
>          usteer_ubus_disassoc_add_neighbor(si, target_node);
>      }
>      return ubus_invoke(ubus_ctx, ln->obj_id, "bss_transition_request",
> b.head, NULL, 0, 100);
>  }
>  
> -int usteer_ubus_band_steering_request(struct sta_info *si)
> +int usteer_ubus_band_steering_request(struct sta_info *si,
> +                                      uint8_t dialog_token,
> +                                      bool disassoc_imminent,
> +                                      uint8_t disassoc_timer,
> +                                      bool abridged,
> +                                      uint8_t validity_period)
>  {
>      struct usteer_local_node *ln = container_of(si->node, struct
> usteer_local_node, node);
>      struct usteer_node *node;
> @@ -698,21 +717,31 @@ int usteer_ubus_band_steering_request(struct
> sta_info *si)
>  
>      blob_buf_init(&b, 0);
>      blobmsg_printf(&b, "addr", MAC_ADDR_FMT, MAC_ADDR_DATA(si->sta->addr));
> -    blobmsg_add_u32(&b, "dialog_token", 0);
> -    blobmsg_add_u8(&b, "disassociation_imminent", false);
> -    blobmsg_add_u8(&b, "abridged", false);
> -    blobmsg_add_u32(&b, "validity_period", 100);
> +    blobmsg_add_u32(&b, "dialog_token", dialog_token);
> +    blobmsg_add_u8(&b, "disassociation_imminent", disassoc_imminent);
> +    if (disassoc_imminent)
> +    {
> +        blobmsg_add_u32(&b, "disassociation_timer", disassoc_timer);
> +    }
> +    blobmsg_add_u8(&b, "abridged", abridged);
> +    blobmsg_add_u32(&b, "validity_period", validity_period);
>  
>      c = blobmsg_open_array(&b, "neighbors");
> -    for_each_local_node(node) {
> +    for_each_local_node(node)
> +    {
>          if (!usteer_band_steering_is_target(ln, node))
>              continue;
> -    
> +        // TODO: Funktioniert nicht, Targets werden nicht korrekt
> ausgewiesen.
>          usteer_add_nr_entry(si->node, node);
>      }
>      blobmsg_close_array(&b, c);
> -
> -    return ubus_invoke(ubus_ctx, ln->obj_id, "bss_transition_request",
> b.head, NULL, 0, 100);
> +    if (sizeof(si->node) > 0)
> +    {
> +        MSG(DEBUG, "BAND STEERING requested for sta=" MAC_ADDR_FMT "
> with disassociation timer %i\n", MAC_ADDR_DATA(si->sta->addr),
> disassoc_timer);
> +        return ubus_invoke(ubus_ctx, ln->obj_id,
> "bss_transition_request", b.head, NULL, 0, 100);
> +    }
> +    else
> +        MSG(DEBUG, "BAND STEERING no targets found for sta="
> MAC_ADDR_FMT "\n", MAC_ADDR_DATA(si->sta->addr));
>  }
>  
>  int usteer_ubus_trigger_link_measurement(struct sta_info *si)
> diff --git a/usteer.h b/usteer.h
> index f692fb8..7a5739c 100644
> --- a/usteer.h
> +++ b/usteer.h
> @@ -170,6 +170,10 @@ struct usteer_config {
>      uint32_t remote_update_interval;
>      uint32_t remote_node_timeout;
>  
> +    bool aggressive_all;
> +    struct blob_attr *aggressive_mac_list;
> +    uint32_t aggressive_disassoc_timer;
> +
>      int32_t min_snr;
>      uint32_t min_snr_kick_delay;
>      int32_t min_connect_snr;
> @@ -190,7 +194,7 @@ struct usteer_config {
>      uint32_t roam_kick_delay;
>  
>      uint32_t band_steering_interval;
> -    int32_t band_steering_min_snr;
> +    int32_t band_steering_min_snr;
>  
>      uint32_t link_measurement_interval;
>  
> @@ -255,6 +259,7 @@ struct sta_info {
>      uint8_t roam_tries;
>      uint64_t roam_event;
>      uint64_t roam_kick;
> +    uint64_t roam_disassoc_time;
>      uint64_t roam_scan_start;
>      uint64_t roam_scan_timeout_start;
>  
> @@ -285,6 +290,8 @@ struct sta {
>      uint8_t seen_2ghz : 1;
>      uint8_t seen_5ghz : 1;
>  
> +    bool aggressive;
> +
>      uint8_t addr[6];
>  };
>  
> @@ -336,13 +343,19 @@ bool usteer_band_steering_is_target(struct
> usteer_local_node *ln, struct usteer_
>  void usteer_ubus_init(struct ubus_context *ctx);
>  void usteer_ubus_kick_client(struct sta_info *si);
>  int usteer_ubus_trigger_client_scan(struct sta_info *si);
> -int usteer_ubus_band_steering_request(struct sta_info *si);
> +int usteer_ubus_band_steering_request(struct sta_info *si,
> +                                      uint8_t dialog_token,
> +                                      bool disassoc_imminent,
> +                                      uint8_t disassoc_timer,
> +                                      bool abridged,
> +                                      uint8_t validity_period);
>  int usteer_ubus_bss_transition_request(struct sta_info *si,
> -                       uint8_t dialog_token,
> -                       bool disassoc_imminent,
> -                       bool abridged,
> -                       uint8_t validity_period,
> -                       struct usteer_node *target_node);
> +                                       uint8_t dialog_token,
> +                                       bool disassoc_imminent,
> +                                       uint8_t disassoc_timer,
> +                                       bool abridged,
> +                                       uint8_t validity_period,
> +                                       struct usteer_node *target_node);
>  
>  struct sta *usteer_sta_get(const uint8_t *addr, bool create);
>  struct sta_info *usteer_sta_info_get(struct sta *sta, struct
> usteer_node *node, bool *create);
> @@ -376,6 +389,9 @@ void config_get_node_up_script(struct blob_buf *buf);
>  void config_set_ssid_list(struct blob_attr *data);
>  void config_get_ssid_list(struct blob_buf *buf);
>  
> +void config_set_aggressive_mac_list(struct blob_attr *data);
> +void config_get_aggressive_mac_list(struct blob_buf *buf);
> +
>  int usteer_interface_init(void);
>  void usteer_interface_add(const char *name);
>  void usteer_sta_node_cleanup(struct usteer_node *node);
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
Nils Rottgardt <n.rottgardt@gmail.com> writes:

> diff --git a/band_steering.c b/band_steering.c
> index 7fce1df..7b2ee00 100644
> --- a/band_steering.c
> +++ b/band_steering.c
> @@ -86,13 +86,20 @@ void usteer_band_steering_perform_steer(struct
> usteer_local_node *ln)
>              continue;

Are you able to apply this patch?  Maybe email it to yourself and test
it before sending yet another broken patch to the list?

See also
https://docs.kernel.org/process/email-clients.html#thunderbird-gui
if you can't use an email client with sane defaults.

> -        if (si->band_steering.below_snr) {
> +        if (si->band_steering.below_snr)
> +        {

How interested do you think most reviewers are in repeating their
feedback?


Bjørn

Thanks to everybody for the replies to my first patch. I tried to cover 
everything and will resend it now with git itself. Hopefully everything 
is fine then. It is my first time creating patches combined with mailing 
lists and also contributing to OpenWRT so every comment is welcome.

Nils

Am 20.10.2024 um 11:48 schrieb Bjørn Mork:
> Nils Rottgardt <n.rottgardt@gmail.com> writes:
>
>> diff --git a/band_steering.c b/band_steering.c
>> index 7fce1df..7b2ee00 100644
>> --- a/band_steering.c
>> +++ b/band_steering.c
>> @@ -86,13 +86,20 @@ void usteer_band_steering_perform_steer(struct
>> usteer_local_node *ln)
>>               continue;
> Are you able to apply this patch?  Maybe email it to yourself and test
> it before sending yet another broken patch to the list?
>
> See also
> https://docs.kernel.org/process/email-clients.html#thunderbird-gui
> if you can't use an email client with sane defaults.
>
>> -        if (si->band_steering.below_snr) {
>> +        if (si->band_steering.below_snr)
>> +        {
> How interested do you think most reviewers are in repeating their
> feedback?
>
>
> Bjørn