| Message ID | 20240823222339.328006-5-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/8] doc: Document station Affinities property | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-ci-gitlint | success | GitLint |
Hi James, On 8/23/24 5:23 PM, James Prestwood wrote: > This property will hold an array of object paths for > BasicServiceSet (BSS) objects. For the purpose of this patch > only the setter/getter and client watch is implemented. The > purpose of this array is to guide or loosly lock IWD to certain typo: 'loosely' > BSS's provided that some external client has more information > about the environment than what IWD takes into account for its > roaming decisions. > > For the time being, the array is limited to only the connected > BSS path, and any roams or disconnects will clear the array. > > The intended use case for this is if the device is stationary > an external client could reduce the likelihood of roaming by > setting the affinity to the current BSS. > --- > src/station.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 145 insertions(+) > > diff --git a/src/station.c b/src/station.c > index 30a1232a..c72d42ae 100644 > --- a/src/station.c > +++ b/src/station.c > @@ -128,6 +128,10 @@ struct station { > > uint64_t last_roam_scan; > > + struct l_queue *affinities; > + unsigned int affinity_watch; > + char *affinity_client; > + > bool preparing_roam : 1; > bool roam_scan_full : 1; > bool signal_low : 1; > @@ -449,6 +453,14 @@ static const char *station_get_bss_path(struct station *station, > return __network_path_append_bss(network_path, bss); > } > > +static bool match_bss_path(const void *data, const void *user_data) > +{ > + const char *path1 = data; > + const char *path2 = user_data; > + > + return !strcmp(path1, path2); > +} > + > static bool station_unregister_bss(struct station *station, > struct scan_bss *bss) > { > @@ -457,6 +469,8 @@ static bool station_unregister_bss(struct station *station, > if (L_WARN_ON(!path)) > return false; > > + l_queue_remove_if(station->affinities, match_bss_path, path); > + > return l_dbus_unregister_object(dbus_get_bus(), path); > } > > @@ -1740,6 +1754,13 @@ static void station_enter_state(struct station *station, > station_set_evict_nocarrier(station, true); > station_set_drop_neighbor_discovery(station, false); > station_set_drop_unicast_l2_multicast(station, false); > + > + if (station->affinity_watch) { > + l_dbus_remove_watch(dbus_get_bus(), > + station->affinity_watch); > + station->affinity_watch = 0; > + } > + Hmm, but not resetting affinities? > break; > case STATION_STATE_DISCONNECTING: > case STATION_STATE_NETCONFIG: > @@ -1747,6 +1768,12 @@ static void station_enter_state(struct station *station, > case STATION_STATE_ROAMING: > case STATION_STATE_FT_ROAMING: > case STATION_STATE_FW_ROAMING: > + if (station->affinity_watch) { > + l_dbus_remove_watch(dbus_get_bus(), > + station->affinity_watch); > + station->affinity_watch = 0; Okay. This reminds me that for hardware supporting FW roaming we may want to return NotCapable when setting Affinities. > + } > + > station_set_evict_nocarrier(station, false); > break; > } <snip> > +static void station_affinity_watch_destroy(void *user_data) > +{ > + struct station *station = user_data; > + > + l_free(station->affinity_client); > + station->affinity_client = NULL; > + > + l_queue_destroy(station->affinities, l_free); > + station->affinities = NULL; > + > + l_dbus_property_changed(dbus_get_bus(), > + netdev_get_path(station->netdev), > + IWD_STATION_INTERFACE, "Affinities"); What if the list was empty? > +} > + > +static struct l_dbus_message *station_property_set_affinities( > + struct l_dbus *dbus, > + struct l_dbus_message *message, > + struct l_dbus_message_iter *new_value, > + l_dbus_property_complete_cb_t complete, > + void *user_data) > +{ > + struct station *station = user_data; > + struct l_dbus_message_iter array; > + const char *sender = l_dbus_message_get_sender(message); > + const char *path; > + struct l_queue *new_affinities; > + > + if (!station->connected_network) > + return dbus_error_not_connected(message); > + > + if (!station->affinity_watch) { > + station->affinity_client = l_strdup(sender); > + station->affinity_watch = l_dbus_add_disconnect_watch(dbus, > + sender, > + station_affinity_disconnected_cb, > + station, > + station_affinity_watch_destroy); > + } else if (strcmp(station->affinity_client, sender)) { > + l_warn("Only one client may manage Affinities property"); > + return dbus_error_not_available(message); Hmm, we probably need a new error for 'permission denied or something' > + } > + > + if (!l_dbus_message_iter_get_variant(new_value, "ao", &array)) > + return dbus_error_invalid_args(message); So what happens to affinity_client / affinity_watch above? > + > + new_affinities = l_queue_new(); > + > + while (l_dbus_message_iter_next_entry(&array, &path)) { > + struct scan_bss *bss = l_dbus_object_get_data(dbus_get_bus(), > + path, > + IWD_BSS_INTERFACE); > + > + /* > + * TODO: For now only allow the affinities array to contain the > + * connected BSS path. Any other values will be rejected. > + * This could change in the future. > + */ > + if (bss != station->connected_bss) { > + l_queue_destroy(new_affinities, l_free); > + return dbus_error_invalid_args(message); same as above? You may want to make your life easier by just looking for an empty list or list of size 1 with the connected bss. > + } > + > + l_queue_push_head(new_affinities, l_strdup(path)); > + } > + > + l_queue_destroy(station->affinities, l_free); > + station->affinities = new_affinities; If the list is empty, should you unregister the watch? > + > + l_dbus_property_changed(dbus, netdev_get_path(station->netdev), > + IWD_STATION_INTERFACE, "Affinities"); > + > + complete(dbus, message, NULL); It is the other way around, we send the 'message_return' first, then the signal. Do you want to spuriously emit if the two lists are the same? > + > + return NULL; > +} > + > void station_foreach(station_foreach_func_t func, void *user_data) > { > const struct l_queue_entry *entry; Regards, -Denis
diff --git a/src/station.c b/src/station.c index 30a1232a..c72d42ae 100644 --- a/src/station.c +++ b/src/station.c @@ -128,6 +128,10 @@ struct station { uint64_t last_roam_scan; + struct l_queue *affinities; + unsigned int affinity_watch; + char *affinity_client; + bool preparing_roam : 1; bool roam_scan_full : 1; bool signal_low : 1; @@ -449,6 +453,14 @@ static const char *station_get_bss_path(struct station *station, return __network_path_append_bss(network_path, bss); } +static bool match_bss_path(const void *data, const void *user_data) +{ + const char *path1 = data; + const char *path2 = user_data; + + return !strcmp(path1, path2); +} + static bool station_unregister_bss(struct station *station, struct scan_bss *bss) { @@ -457,6 +469,8 @@ static bool station_unregister_bss(struct station *station, if (L_WARN_ON(!path)) return false; + l_queue_remove_if(station->affinities, match_bss_path, path); + return l_dbus_unregister_object(dbus_get_bus(), path); } @@ -1740,6 +1754,13 @@ static void station_enter_state(struct station *station, station_set_evict_nocarrier(station, true); station_set_drop_neighbor_discovery(station, false); station_set_drop_unicast_l2_multicast(station, false); + + if (station->affinity_watch) { + l_dbus_remove_watch(dbus_get_bus(), + station->affinity_watch); + station->affinity_watch = 0; + } + break; case STATION_STATE_DISCONNECTING: case STATION_STATE_NETCONFIG: @@ -1747,6 +1768,12 @@ static void station_enter_state(struct station *station, case STATION_STATE_ROAMING: case STATION_STATE_FT_ROAMING: case STATION_STATE_FW_ROAMING: + if (station->affinity_watch) { + l_dbus_remove_watch(dbus_get_bus(), + station->affinity_watch); + station->affinity_watch = 0; + } + station_set_evict_nocarrier(station, false); break; } @@ -4520,6 +4547,118 @@ static bool station_property_get_state(struct l_dbus *dbus, return true; } +static bool station_property_get_affinities(struct l_dbus *dbus, + struct l_dbus_message *message, + struct l_dbus_message_builder *builder, + void *user_data) +{ + struct station *station = user_data; + const struct l_queue_entry *e; + + if (!station->connected_network) + return false; + + l_dbus_message_builder_enter_array(builder, "o"); + + for (e = l_queue_get_entries(station->affinities); e; e = e->next) { + const char *path = e->data; + + l_dbus_message_builder_append_basic(builder, 'o', path); + } + + l_dbus_message_builder_leave_array(builder); + + return true; +} + +static void station_affinity_disconnected_cb(struct l_dbus *dbus, + void *user_data) +{ + struct station *station = user_data; + + l_dbus_remove_watch(dbus_get_bus(), station->affinity_watch); + station->affinity_watch = 0; + + l_debug("client that set affinity has disconnected"); +} + +static void station_affinity_watch_destroy(void *user_data) +{ + struct station *station = user_data; + + l_free(station->affinity_client); + station->affinity_client = NULL; + + l_queue_destroy(station->affinities, l_free); + station->affinities = NULL; + + l_dbus_property_changed(dbus_get_bus(), + netdev_get_path(station->netdev), + IWD_STATION_INTERFACE, "Affinities"); +} + +static struct l_dbus_message *station_property_set_affinities( + struct l_dbus *dbus, + struct l_dbus_message *message, + struct l_dbus_message_iter *new_value, + l_dbus_property_complete_cb_t complete, + void *user_data) +{ + struct station *station = user_data; + struct l_dbus_message_iter array; + const char *sender = l_dbus_message_get_sender(message); + const char *path; + struct l_queue *new_affinities; + + if (!station->connected_network) + return dbus_error_not_connected(message); + + if (!station->affinity_watch) { + station->affinity_client = l_strdup(sender); + station->affinity_watch = l_dbus_add_disconnect_watch(dbus, + sender, + station_affinity_disconnected_cb, + station, + station_affinity_watch_destroy); + } else if (strcmp(station->affinity_client, sender)) { + l_warn("Only one client may manage Affinities property"); + return dbus_error_not_available(message); + } + + if (!l_dbus_message_iter_get_variant(new_value, "ao", &array)) + return dbus_error_invalid_args(message); + + new_affinities = l_queue_new(); + + while (l_dbus_message_iter_next_entry(&array, &path)) { + struct scan_bss *bss = l_dbus_object_get_data(dbus_get_bus(), + path, + IWD_BSS_INTERFACE); + + /* + * TODO: For now only allow the affinities array to contain the + * connected BSS path. Any other values will be rejected. + * This could change in the future. + */ + if (bss != station->connected_bss) { + l_queue_destroy(new_affinities, l_free); + return dbus_error_invalid_args(message); + } + + l_queue_push_head(new_affinities, l_strdup(path)); + } + + l_queue_destroy(station->affinities, l_free); + station->affinities = new_affinities; + + l_dbus_property_changed(dbus, netdev_get_path(station->netdev), + IWD_STATION_INTERFACE, "Affinities"); + + complete(dbus, message, NULL); + + return NULL; +} + void station_foreach(station_foreach_func_t func, void *user_data) { const struct l_queue_entry *entry; @@ -4842,6 +4981,9 @@ static void station_free(struct station *station) l_queue_destroy(station->roam_bss_list, l_free); + if (station->affinity_watch) + l_dbus_remove_watch(dbus_get_bus(), station->affinity_watch); + l_free(station); } @@ -4878,6 +5020,9 @@ static void station_setup_interface(struct l_dbus_interface *interface) station_property_get_scanning, NULL); l_dbus_interface_property(interface, "State", 0, "s", station_property_get_state, NULL); + l_dbus_interface_property(interface, "Affinities", 0, "ao", + station_property_get_affinities, + station_property_set_affinities); } static void station_destroy_interface(void *user_data)