Re: [RFC 2/4] station: unify scan cancelation

James Prestwood
2023-12-13T17:32:47Z

On 12/13/23 09:25, James Prestwood wrote:
> The scan cancelation was all being done manually so group all the
> automatic-type scans into a single cancel function. This includes
> hidden, quick, owe, and periodic scans.
>
> This is being done in order to avoid scan failures after DPP
> initiates a connection. This is ultimately done through
> __station_connect_network which has no cancelation logic. This
> patch will move the scan cancelation into station_enter_state
> when the state changes to CONNECTING. All the mentioned scans will
> be canceled at that point.
>
> DBus scans were left out intentionally as they are an explicit
> type of scan rather than one that IWD starts on its own.
> ---
> src/station.c | 96 +++++++++++++++++++++++----------------------------
> 1 file changed, 43 insertions(+), 53 deletions(-)
>
> diff --git a/src/station.c b/src/station.c
> index 10860808..95ffee05 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -279,16 +279,8 @@ static int station_autoconnect_next(struct station *station)
> bss->signal_strength);
>
> r = network_autoconnect(network, bss);
> - if (!r) {
> - if (station->quick_scan_id) {
> - scan_cancel(netdev_get_wdev_id(station->netdev),
> - station->quick_scan_id);
> - station->quick_scan_id = 0;
> - station_property_set_scanning(station, false);
> - }
> -
> + if (!r)
> return 0;
> - }
>
> l_debug("autoconnect: network_autoconnect: %s (%d)",
> strerror(-r), r);
> @@ -1542,6 +1534,36 @@ static void station_set_drop_unicast_l2_multicast(struct station *station,
>
> static void station_signal_agent_notify(struct station *station);
>
> +static void station_cancel_scans(struct station *station)
> +{
> + if (station->hidden_network_scan_id) {
> + scan_cancel(netdev_get_wdev_id(station->netdev),
> + station->hidden_network_scan_id);
> +
> + dbus_pending_reply(&station->hidden_pending,
> + dbus_error_failed(station->hidden_pending));
> + }
> +
> + if (station->quick_scan_id) {
> + scan_cancel(netdev_get_wdev_id(station->netdev),
> + station->quick_scan_id);
> + station->quick_scan_id = 0;
> + station_property_set_scanning(station, false);
> + }
> +
> + if (station->owe_hidden_scan_ids) {
> + void *ptr;
> +
> + while ((ptr = l_queue_pop_head(station->owe_hidden_scan_ids)))
> + scan_cancel(netdev_get_wdev_id(station->netdev),
> + L_PTR_TO_UINT(ptr));
> +
> + l_queue_destroy(station->owe_hidden_scan_ids, NULL);
Just realized I should set this to NULL here or we could reference it
somewhere else after its freed.
> + }
> +
> + periodic_scan_stop(station);
> +}
> +
> static void station_enter_state(struct station *station,
> enum station_state state)
> {
> @@ -1594,7 +1616,9 @@ static void station_enter_state(struct station *station,
> if (station->signal_agent)
> station_signal_agent_notify(station);
>
> - periodic_scan_stop(station);
> + /* Scans issued during a connection will fail */
> + station_cancel_scans(station);
> +
> break;
> case STATION_STATE_CONNECTED:
> l_dbus_object_add_interface(dbus,
> @@ -3539,27 +3563,15 @@ void station_connect_network(struct station *station, struct network *network,
> struct l_dbus *dbus = dbus_get_bus();
> int err;
>
> - /*
> - * If a hidden scan is not completed, station_is_busy would not
> - * indicate anything is going on so we need to cancel the scan and
> - * fail the connection now.
> - */
> - if (station->hidden_network_scan_id) {
> - scan_cancel(netdev_get_wdev_id(station->netdev),
> - station->hidden_network_scan_id);
> -
> - dbus_pending_reply(&station->hidden_pending,
> - dbus_error_failed(station->hidden_pending));
> - }
> -
> - if (station->quick_scan_id) {
> - scan_cancel(netdev_get_wdev_id(station->netdev),
> - station->quick_scan_id);
> - station->quick_scan_id = 0;
> - station_property_set_scanning(station, false);
> - }
> -
> if (station_is_busy(station)) {
> + /*
> + * All scans will be canceled when entering into a connecting
> + * state but since we have an async disconnect callback cancel
> + * them early so we don't potentially trigger a scan in the
> + * middle of connecting. This also takes care of failing any
> + * hidden network connection
> + */
> + station_cancel_scans(station);
> station_disconnect_onconnect(station, network, bss, message);
>
> return;
> @@ -4539,8 +4551,6 @@ static void station_free(struct station *station)
> station->netconfig = NULL;
> }
>
> - periodic_scan_stop(station);
> -
> if (station->signal_agent) {
> station_signal_agent_release(station->signal_agent,
> netdev_get_path(station->netdev));
> @@ -4551,10 +4561,6 @@ static void station_free(struct station *station)
> dbus_pending_reply(&station->connect_pending,
> dbus_error_aborted(station->connect_pending));
>
> - if (station->hidden_pending)
> - dbus_pending_reply(&station->hidden_pending,
> - dbus_error_aborted(station->hidden_pending));
> -
> if (station->disconnect_pending)
> dbus_pending_reply(&station->disconnect_pending,
> dbus_error_aborted(station->disconnect_pending));
> @@ -4567,23 +4573,7 @@ static void station_free(struct station *station)
> scan_cancel(netdev_get_wdev_id(station->netdev),
> station->dbus_scan_id);
>
> - if (station->quick_scan_id)
> - scan_cancel(netdev_get_wdev_id(station->netdev),
> - station->quick_scan_id);
> -
> - if (station->hidden_network_scan_id)
> - scan_cancel(netdev_get_wdev_id(station->netdev),
> - station->hidden_network_scan_id);
> -
> - if (station->owe_hidden_scan_ids) {
> - void *ptr;
> -
> - while ((ptr = l_queue_pop_head(station->owe_hidden_scan_ids)))
> - scan_cancel(netdev_get_wdev_id(station->netdev),
> - L_PTR_TO_UINT(ptr));
> -
> - l_queue_destroy(station->owe_hidden_scan_ids, NULL);
> - }
> + station_cancel_scans(station);
>
> station_roam_state_clear(station);
>