| Message ID | 20231205175203.1935692-3-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] auto-t: add explicit stop() to IWD class | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-ci-gitlint | success | GitLint |
Hi James, On 12/5/23 11:52, James Prestwood wrote: > Not sure how we want to address this, but FT is special in that it > has no connect_cb but can still trigger an associate timeout. > Currently if FT times out associating IWD will hang indefinitely. Do our auto tests cover this case? I wonder why they didn't flag this earlier? > > This behavior can be tested with the prior autotest patches (without > this patch applied). > > Fixes: 30c6a10f28 ("netdev: Separate connect_failed and disconnected paths") > --- > src/netdev.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/netdev.c b/src/netdev.c > index f2e887b4..2d1120c4 100644 > --- a/src/netdev.c > +++ b/src/netdev.c > @@ -3045,7 +3045,13 @@ static void netdev_associate_event(struct l_genl_msg *msg, > * out. The failed connection must be explicitly > * initiated here. > */ > - netdev_connect_failed(netdev, > + if (!netdev->ap) { if (netdev->in_ft) ? > + if (netdev->event_filter) > + netdev->event_filter(netdev, > + NETDEV_EVENT_DISCONNECT_BY_SME, > + NULL, netdev->user_data); > + } else > + netdev_connect_failed(netdev, > NETDEV_RESULT_ASSOCIATION_FAILED, > status_code); > return; I wonder if we should make ft_associate use netdev_reassociate? Regards, -Denis
Hi Denis, On 12/5/23 13:37, Denis Kenzior wrote: > Hi James, > > On 12/5/23 11:52, James Prestwood wrote: >> Not sure how we want to address this, but FT is special in that it >> has no connect_cb but can still trigger an associate timeout. >> Currently if FT times out associating IWD will hang indefinitely. > > Do our auto tests cover this case? I wonder why they didn't flag this > earlier? They don't (only auth timeouts), but patches 1/2 add an associate timeout test. > >> >> This behavior can be tested with the prior autotest patches (without >> this patch applied). >> >> Fixes: 30c6a10f28 ("netdev: Separate connect_failed and disconnected >> paths") >> --- >> src/netdev.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/src/netdev.c b/src/netdev.c >> index f2e887b4..2d1120c4 100644 >> --- a/src/netdev.c >> +++ b/src/netdev.c >> @@ -3045,7 +3045,13 @@ static void netdev_associate_event(struct >> l_genl_msg *msg, >> * out. The failed connection must be explicitly >> * initiated here. >> */ >> - netdev_connect_failed(netdev, >> + if (!netdev->ap) { > > if (netdev->in_ft) ? > >> + if (netdev->event_filter) >> + netdev->event_filter(netdev, >> + NETDEV_EVENT_DISCONNECT_BY_SME, >> + NULL, netdev->user_data); >> + } else >> + netdev_connect_failed(netdev, >> NETDEV_RESULT_ASSOCIATION_FAILED, >> status_code); >> return; > > I wonder if we should make ft_associate use netdev_reassociate? So pass station_connect_cb into ft_associate, then to netdev? That seems reasonable. > Regards, > -Denis
Hi James, > They don't (only auth timeouts), but patches 1/2 add an associate timeout test. Ah, I guess I should have looked at those too, whoops :) >> >> I wonder if we should make ft_associate use netdev_reassociate? > > So pass station_connect_cb into ft_associate, then to netdev? That seems > reasonable. > Yep. Probably need a few additional checks to make sure we don't trigger any offloading behavior. Regards, -Denis
diff --git a/src/netdev.c b/src/netdev.c index f2e887b4..2d1120c4 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -3045,7 +3045,13 @@ static void netdev_associate_event(struct l_genl_msg *msg, * out. The failed connection must be explicitly * initiated here. */ - netdev_connect_failed(netdev, + if (!netdev->ap) { + if (netdev->event_filter) + netdev->event_filter(netdev, + NETDEV_EVENT_DISCONNECT_BY_SME, + NULL, netdev->user_data); + } else + netdev_connect_failed(netdev, NETDEV_RESULT_ASSOCIATION_FAILED, status_code); return;
Not sure how we want to address this, but FT is special in that it has no connect_cb but can still trigger an associate timeout. Currently if FT times out associating IWD will hang indefinitely. This behavior can be tested with the prior autotest patches (without this patch applied). Fixes: 30c6a10f28 ("netdev: Separate connect_failed and disconnected paths") --- src/netdev.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)