Re: [PATCH v2 6/9] netdev: add netdev_ft_reassociate

Denis Kenzior
2023-12-06T16:40:39Z

Hi James,

On 12/6/23 09:07, James Prestwood wrote:
> Essentially exposes (and renames) netdev_ft_tx_associate in order to
> be called similarly to netdev_reassociate/netdev_connect where a
> connect callback can be provided. This will fix the current bug where
> if association times out during FT IWD will hang and never transition
> to disconnected.
>

I like it!

Nitpick below...

> This also removes the calling of the FT_ROAMED event and instead just
> calls the connect callback (since its now set). This unifies the
> callback path for reassociation and FT roaming.
> ---
> src/netdev.c | 44 ++++++++++++++++++++++++++------------------
> src/netdev.h | 5 +++++
> 2 files changed, 31 insertions(+), 18 deletions(-)
>

<snip>

> diff --git a/src/netdev.c b/src/netdev.c
> index f2e887b4..7d52ffea 100644
> --- a/src/netdev.c
> +++ b/src/netdev.c
> @@ -1409,16 +1409,14 @@ static void netdev_connect_ok(struct netdev *netdev)
> scan_bss_free(netdev->fw_roam_bss);
>
> netdev->fw_roam_bss = NULL;
> - } else if (netdev->in_ft) {
> - if (netdev->event_filter)
> - netdev->event_filter(netdev, NETDEV_EVENT_FT_ROAMED,
> - NULL, netdev->user_data);
> - netdev->in_ft = false;
> } else if (netdev->connect_cb) {
> netdev->connect_cb(netdev, NETDEV_RESULT_OK, NULL,
> netdev->user_data);
> netdev->connect_cb = NULL;
> - }
> + netdev->in_ft = false;
> + netdev->in_reassoc = false;

Why do we set in_reassoc to false here? Shouldn't it be taken care of already
by the associate_event?

> + } else
> + l_warn("Connection event without a connect callback!");
>
> netdev_rssi_polling_update(netdev);
>

<snip>

> @@ -6256,7 +6265,6 @@ static int netdev_init(void)
> __eapol_set_install_pmk_func(netdev_set_pmk);
>
> __ft_set_tx_frame_func(netdev_tx_ft_frame);
> - __ft_set_tx_associate_func(netdev_ft_tx_associate);

This part probably belongs in the next patch or even patch 8.

>
> unicast_watch = l_genl_add_unicast_watch(genl, NL80211_GENL_NAME,
> netdev_unicast_notify,

Regards,
-Denis