Skip to content

Updates Roaming and Band-Steering #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NilsRo wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Updates Roaming and Band-Steering #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ff1e72d
Optimized steering with different levels of aggressiveness
NilsRo Oct 20, 2024
efb5aa9
802.11 Mgmt: DEAUTH reason more accurate
NilsRo Nov 3, 2024
22e31ac
Add dynamic signal threshold to band-steering
NilsRo Nov 10, 2024
edd1f09
802.11 MBO - added reassoc_delay and reason code
NilsRo Nov 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions band_steering.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,24 @@

void usteer_band_steering_sta_update(struct sta_info *si)
{
if (si->signal < usteer_snr_to_signal(si->node, config.band_steering_min_snr))
if (si->connected == STA_NOT_CONNECTED) {
if (si->band_steering.signal_threshold != NO_SIGNAL) {
si->band_steering.signal_threshold = NO_SIGNAL;
}
return;
}
if (si->connected != STA_NOT_CONNECTED && si->band_steering.signal_threshold == NO_SIGNAL) {
si->band_steering.signal_threshold = si->signal;
MSG(DEBUG, "band steering station " MAC_ADDR_FMT " (%s) set threshold %d\n", MAC_ADDR_DATA(si->sta->addr), usteer_node_name(si->node), si->band_steering.signal_threshold);
return;
}

/* Adapt signal threshold to actual signal quality */
if (si->signal < si->band_steering.signal_threshold) {
si->band_steering.signal_threshold--;
MSG(DEBUG, "band steering station " MAC_ADDR_FMT " (%s) reduce threshold %d, signal: %d\n", MAC_ADDR_DATA(si->sta->addr), usteer_node_name(si->node), si->band_steering.signal_threshold, si->signal);
}
if (si->signal < usteer_snr_to_signal(si->node, config.band_steering_min_snr) || si->signal < si->band_steering.signal_threshold + config.band_steering_signal_threshold)
si->band_steering.below_snr = true;
}

Expand Down Expand Up @@ -60,6 +77,8 @@ void usteer_band_steering_perform_steer(struct usteer_local_node *ln)
{
unsigned int min_count = DIV_ROUND_UP(config.band_steering_interval, config.local_sta_update);
struct sta_info *si;
uint32_t disassoc_timer;
uint32_t validity_period;

if (!config.band_steering_interval)
return;
Expand Down Expand Up @@ -91,8 +110,24 @@ void usteer_band_steering_perform_steer(struct usteer_local_node *ln)
continue;
}

if (si->bss_transition)
usteer_ubus_band_steering_request(si);
/* Skip if in validity period */
if (current_time < si->roam_transition_request_validity_end)
continue;

if (si->bss_transition) {
si->roam_transition_request_validity_end = current_time + 10000;
validity_period = 10000 / usteer_local_node_get_beacon_interval(ln); /* ~ 10 seconds */
Copy link
Member

@KanjiMonster KanjiMonster May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that comment supposed to be at the line above? And where do the 10 seconds come from? They are not explained anywhere.

Copy link
Author

@NilsRo NilsRo May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, its for the line. The validity period is calculated by beacon interval so is not exactly 10s.

Copy link
Member

@KanjiMonster KanjiMonster May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then what does the the 10000 mean? Because if you what you get when you divide it by a number (that is presumably > 1) is 10 seconds, 10000 is something larger than 10 seconds, right?

Copy link
Author

@NilsRo NilsRo May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10000ms have to be split up by beacons (e.g. every 150ms). The interval could be set in hostap config. So if the result of the division has to be rounded the beacon interval count does not exactly reflecting 10s anymore.

Copy link
Member

@KanjiMonster KanjiMonster May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that means that validity_period is the number of beacons sent out during 10 seconds, right?

Also it still does not explain where the 10 seconds value come from. How was this chosen? Should this be configurable?

Copy link
Author

@NilsRo NilsRo May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and good question.

This is the common standard and also fixed value within e.g. DAWN. Its related to disassociation imminent timer, that if the disassociation is planned within 30s every 10s a new BSS_TM_REQ is send like a timer and to update the neighbor list if the STA moved to another place. Could make sense for experts (it is available in Cisco Controllers) to change it but not relevant for common usage in my opinion. Not everything should be configurable without need and testing. This can be done in another iteration but with this PR I do not plan to add more functionality.

if (si->sta->aggressiveness >= 2) {
if (!si->kick_time)
si->kick_time = current_time + config.roam_kick_delay;
if (si->sta->aggressiveness >= 3)
disassoc_timer = (si->kick_time - current_time) / usteer_local_node_get_beacon_interval(ln);
else
disassoc_timer = 0;
usteer_ubus_band_steering_request(si, 0, true, disassoc_timer, true, validity_period);
} else
usteer_ubus_band_steering_request(si, 0, false, 0, true, validity_period);
}

si->band_steering.below_snr = false;
}
Expand Down
24 changes: 20 additions & 4 deletions local_node.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,10 @@ usteer_handle_bss_tm_response(struct usteer_local_node *ln, struct blob_attr *ms
si->bss_transition_response.status_code = blobmsg_get_u8(tb[BSS_TM_RESPONSE_STATUS_CODE]);
si->bss_transition_response.timestamp = current_time;

if (si->bss_transition_response.status_code) {
if (si->bss_transition_response.status_code && si->kick_time && si->sta->aggressiveness) {
/* Cancel imminent kick in case BSS transition was rejected */
si->kick_time = 0;
MSG(VERBOSE, "Kick canceled because transition rejected by station " MAC_ADDR_FMT "\n", MAC_ADDR_DATA(si->sta->addr));
}

return 0;
Expand Down Expand Up @@ -748,7 +749,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, false, 0, true, validity_period, NULL);
}

/* Free pending queries we can not handle */
Expand Down Expand Up @@ -977,8 +978,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_aggressiveness_mac_list(struct blob_attr *data)
{
free(config.aggressiveness_mac_list);

if (data && blobmsg_len(data))
config.aggressiveness_mac_list = blob_memdup(data);
else
config.aggressiveness_mac_list = NULL;
}

void config_get_aggressiveness_mac_list(struct blob_buf *buf)
{
if (config.aggressiveness_mac_list)
blobmsg_add_blob(buf, config.aggressiveness_mac_list);
}

void usteer_local_nodes_init(struct ubus_context *ctx)
{
usteer_register_events(ctx);
ubus_lookup(ctx, "hostapd.*", node_list_cb, NULL);
Expand Down
5 changes: 4 additions & 1 deletion main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,14 @@ void usteer_init_defaults(void)
config.remote_update_interval = 1000;
config.initial_connect_delay = 0;
config.remote_node_timeout = 10;
config.aggressiveness = 3;
Copy link
Member

@KanjiMonster KanjiMonster May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default should be (IMHO) whatever is the current behavior. The new behavior should be opt-in, not opt-out.

Also maybe use an enum or defines for the different levels so its obvious what they mean?

Copy link
Author

@NilsRo NilsRo May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion it would be better to switch to a new default to have a better roaming for everybody. According the test results in the forum roaming behave much better with the new default which is aligned to DAWNs implementation. Passive roaming is too friendly for most devices.

Anyway, I can also see a benefit to stay at the old default that nothing is changed after an upgrade but the default should be the best behavior for most cases... So open for a discussion what would be the best for the start...could be also changed later if we stay on the old behavior for the beginning.

Copy link

@Djfe Djfe May 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it makes more sense to actually activate the new feature instead of leaving it deactivated. Usteer is not a default package anyway and people installing it expect it to actually make clients roam. This PR adds a new feature to improve roaming for certain clients like Intel wifi cards.
That sounds like a sensible new default to me. Leaving it deactivated leads to worse experience overall which this pr is trying to improve upon by implementing the required standard/messages for said clients.
If we don't activate a useful new part of this package then certain people won't know about it and never activate it since it's only opt-in.

Yes, it's a new kind of default and it's good to bring up that this might change how usteer works/handles certain situations (good feedback by @KanjiMonster).
But knowing how bad certain clients handle roaming on their own, I pledge for the new default.

Looking forward to hear how other people feel about this :)

Copy link

@domingues domingues Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree with @Djfe. This should come with new default., the release could have a breaking change note with some settings attached to be used by the people that want to keep the old behavior.

config.reassociation_delay = 30;

config.steer_reject_timeout = 60000;

config.band_steering_interval = 120000;
config.band_steering_interval = 30000;
config.band_steering_min_snr = -60;
config.band_steering_signal_threshold = 5;

config.link_measurement_interval = 30000;

Expand Down
19 changes: 19 additions & 0 deletions openwrt/usteer/files/etc/config/usteer
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
config usteer
# OpenWrt guide: https://openwrt.org/docs/guide-user/network/wifi/usteer

# The network interface for inter-AP communication
option 'network' 'lan'

Expand Down Expand Up @@ -71,6 +73,19 @@ config usteer
# Timeout (ms) for which a client will not be steered after rejecting a BSS-transition-request
#option steer_reject_timeout 60000

# Aggressiveness of BSS-transition-request to push a station to another node (AP or band)
# 0 = no active transition / 1 = passive BSS-transition-request
# 2 = BSS-transition-request with disassociation imminent
# 3 = BSS-transition-request with disassociation imminent and timer
# 4 = BSS-transition-request with disassociation imminent, timer and forced disassociation
#option aggressiveness 3

# Timeout (s in "1024ms") a station is requested to avoid reassociation after bss transition
#option reassociation_delay 30

# List of MACs (lower case) to set aggressiveness per station (ff:ff:ff:ff:ff,2)
#list aggressiveness_mac_list ''

# Timeout (in ms) after which a association following a disassociation is not seen
# as a roam
#option roam_process_timeout 5000
Expand Down Expand Up @@ -128,6 +143,10 @@ config usteer
# steered to a higher frequency band
#option band_steering_min_snr -60

# Difference that the signal must be better compared to signal was on connection to node.
# Avoids conflicts between roaming and band-steering policies.
# option band_steering_signal_threshold 5

# Interval (ms) the device is sent a link-measurement request to help assess
# the bi-directional link quality. Setting the interval to 0 disables link-measurements.
#option link_measurement_interval 30000
Expand Down
8 changes: 5 additions & 3 deletions openwrt/usteer/files/etc/init.d/usteer
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" aggressiveness
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" aggressiveness_mac_list
uci_option_to_json_string_array "$cfg" event_log_types

for opt in \
Expand All @@ -83,9 +85,9 @@ uci_usteer() {
min_connect_snr min_snr min_snr_kick_delay signal_diff_threshold \
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 \
band_steering_interval band_steering_min_snr link_measurement_interval \
roam_scan_snr roam_scan_interval roam_trigger_snr roam_trigger_interval \
link_measurement_interval \
band_steering_interval band_steering_min_snr band_steering_signal_threshold \
load_kick_threshold load_kick_delay load_kick_min_clients \
load_kick_reason_code
do
Expand Down
61 changes: 51 additions & 10 deletions policy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ static bool
usteer_roam_trigger_sm(struct usteer_local_node *ln, struct sta_info *si)
{
struct sta_info *candidate;
uint32_t disassoc_timer;
uint32_t validity_period;
struct uevent ev = {
.si_cur = si,
};
Expand All @@ -343,10 +345,9 @@ usteer_roam_trigger_sm(struct usteer_local_node *ln, struct sta_info *si)
/* Check if no node was found within roam_scan_tries tries */
if (config.roam_scan_tries && si->roam_tries >= config.roam_scan_tries) {
if (!config.roam_scan_timeout) {
/* Prepare to kick client */
usteer_roam_set_state(si, ROAM_TRIGGER_SCAN_DONE, &ev);
} else {
/* Kick in scan timeout */
/* Set timeout until roam_scans are paused */
si->roam_scan_timeout_start = current_time;
usteer_roam_set_state(si, ROAM_TRIGGER_IDLE, &ev);
}
Expand All @@ -363,16 +364,44 @@ usteer_roam_trigger_sm(struct usteer_local_node *ln, struct sta_info *si)
break;

case ROAM_TRIGGER_SCAN_DONE:
/* Roaming time over, switch back to ROAM_TRIGGER_IDLE */
if (si->roam_transition_start && current_time - si->roam_transition_start > config.roam_kick_delay) {
si->roam_transition_start = 0;
usteer_roam_set_state(si, ROAM_TRIGGER_IDLE, &ev);
break;
}

candidate = usteer_roam_sm_found_better_node(si, &ev, ROAM_TRIGGER_SCAN_DONE);
/* Kick back in case no better node is found */
if (!candidate) {
usteer_roam_set_state(si, ROAM_TRIGGER_IDLE, &ev);
break;
}

usteer_ubus_bss_transition_request(si, 1, false, false, 100, candidate->node);
si->kick_time = current_time + config.roam_kick_delay;
usteer_roam_set_state(si, ROAM_TRIGGER_IDLE, &ev);
if (!candidate->node->rrm_nr)
MSG(VERBOSE, "Candidates node rrm nr not returned from hostapd. Neighbor list empty!");
Copy link
Member

@KanjiMonster KanjiMonster May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the meaning of this check / message? Is the user supposed to do something about it?

Copy link
Author

@NilsRo NilsRo May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an bug in hostap daemon that it sometimes does not return the rrm numbers which breaks supported roaming. Log messages describes the the error and not the solution as this could be different in many cases even if it are not caused by the software itself. Should be documented in OpenWrt Wiki if published but anyhow the solution must be searched in hostap. Mostly fixed with a restart of the deamon or sometimes on its own.

Copy link
Member

@KanjiMonster KanjiMonster May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's a bug that breaks roaming does it mean it prevents STAs from connecting to a different SSID?

If it breaks stuff, then it should be probably INFO and not VERBOSE, and the message should say something of that effect. As a user, I wouldn't have any idea what rrm nr means or if that is important, so it would just be noise and confusing.

Also if it breaks stuff, shouldn't usteer then try to handle that? It doesn't make sense continuing if this won't work, right?

Copy link

@Djfe Djfe May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if it breaks stuff, shouldn't usteer then try to handle that? It doesn't make sense continuing if this won't work, right?

To me the bug @NilsRo found in hostapd sounds more like a follow up that can be tackled now that the behavior was noticed.
Sure, relying on something that might not work all the time can be a problem. But atleast this code change could help in finding the root cause. Or alert of an error, I agree with @KanjiMonster the message needs to be improved to be accessible by everyone.

@NilsRo Was this bug affecting usteer already before your PR and you just added a message so you could restart hostapd when it happened?

Where can the bug be reported, on this mailing list? https://w1.fi/

The rest could be better answered by NilsRo himself than me.

Copy link
Author

@NilsRo NilsRo May 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It described here in detail. openwrt/openwrt#16875

It breaks "controlled" steering also before the PR for DAWN, Usteer, ... The wifi controller software could not get the identifiers anymore to send the target AP to the STA. So the bss transition request if more or less a "I_DONT_LIKE_YOU" but without any additional information. I added the log message to inform the user about it. Sometimes it fixes on its own but sometimes the hostap daemon has to be restarted or whole AP.

What do you suggest to have a better understanding of the message? As it happens from time to time and fixes itself it could be irritating so I put it to VERBOSE. But anyway if this is broken in general it is not visible without changing the logging level....


if (!si->roam_transition_start)
si->roam_transition_start = current_time;
si->roam_transition_request_validity_end = current_time + 10000;
validity_period = 10000 / usteer_local_node_get_beacon_interval(ln); /* ~ 10 seconds */
if (si->sta->aggressiveness >= 2) {
if (!si->kick_time)
si->kick_time = current_time + config.roam_kick_delay;
if (si->sta->aggressiveness >= 3)
disassoc_timer = (si->kick_time - current_time) / usteer_local_node_get_beacon_interval(ln);
else
disassoc_timer = 0;
usteer_ubus_bss_transition_request(si, 1, true, disassoc_timer, true, validity_period, candidate->node);
/* Countdown end */
if (disassoc_timer < validity_period) {
si->roam_transition_start = 0;
usteer_roam_set_state(si, ROAM_TRIGGER_IDLE, &ev);
}
} else {
usteer_ubus_bss_transition_request(si, 1, false, 0, true, validity_period, candidate->node);
usteer_roam_set_state(si, ROAM_TRIGGER_IDLE, &ev);
}
break;
}

Expand All @@ -385,22 +414,34 @@ bool usteer_policy_can_perform_roam(struct sta_info *si)
if (si->connected != STA_CONNECTED)
return false;

/* Only trigger for STA with active roaming */
if (!si->sta->aggressiveness)
return false;

/* Skip on pending kick */
if (si->kick_time)
if (si->kick_time && si->kick_time <= current_time)
return false;

/* Skip if in validity period */
if (current_time < si->roam_transition_request_validity_end)
return false;

/* Skip on rejected transition */
if (si->bss_transition_response.status_code && current_time - si->bss_transition_response.timestamp < config.steer_reject_timeout)
return false;

/* Skip if transition accepted */
if (!si->bss_transition_response.status_code && current_time - si->bss_transition_response.timestamp < config.roam_kick_delay)
return false;

/* Skip on previous kick attempt */
if (current_time - si->roam_kick < config.roam_trigger_interval)
return false;

/* Skip if connection is established shorter than the trigger-interval */
if (current_time - si->connected_since < config.roam_trigger_interval)
return false;

return true;
}

Expand Down Expand Up @@ -483,7 +524,7 @@ usteer_local_node_snr_kick(struct usteer_local_node *ln)
ev.count = si->kick_count;
usteer_event(&ev);

usteer_ubus_kick_client(si);
usteer_ubus_kick_client(si, KICK_REASON_UNSPECIFIED);
return;
}
}
Expand Down Expand Up @@ -567,7 +608,7 @@ usteer_local_node_load_kick(struct usteer_local_node *ln)
ev.si_other = candidate;
ev.count = kick1->kick_count;

usteer_ubus_kick_client(kick1);
usteer_ubus_kick_client(kick1, config.load_kick_reason_code);

out:
usteer_event(&ev);
Expand All @@ -582,7 +623,7 @@ usteer_local_node_perform_kick(struct usteer_local_node *ln)
if (!si->kick_time || si->kick_time > current_time)
continue;

usteer_ubus_kick_client(si);
usteer_ubus_kick_client(si, KICK_REASON_BSS_TRANSITION);
}
}

Expand Down
26 changes: 26 additions & 0 deletions sta.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_aggressiveness(struct sta *sta)
{
struct blob_attr *cur;
int rem;
char sta_mac[18];
char config_entry[20];
char config_mac[18];

sprintf(sta_mac, MAC_ADDR_FMT, MAC_ADDR_DATA(sta->addr));
sta->aggressiveness = config.aggressiveness;
blobmsg_for_each_attr(cur, config.aggressiveness_mac_list, rem) {
strcpy(config_entry, blobmsg_get_string(cur));
if (strlen(config_entry) != 19)
continue;
strncpy(config_mac, config_entry, 18);
config_mac[17] = '\0';
if (strcmp(config_mac, sta_mac) != 0)
continue;
sta->aggressiveness = config_entry[18] - '0';
break;
}
}

struct sta_info *
usteer_sta_info_get(struct sta *sta, struct usteer_node *node, bool *create)
{
Expand Down Expand Up @@ -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_aggressiveness(sta);

/* Node is by default not connected. */
usteer_sta_disconnected(si);

Expand Down
Loading