6:42 p.m.
Move the netconfig_reset calls to inside
station_reset_connection_state() instead of having each caller do
netconfig_reset() followed by station_reset_connection_state(). One of
the callers was missing the netconfig_reset() and this was resulting in
the "netconfig: Failed to start DHCPv4 client for interface %u" warning.
---
src/station.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/station.c b/src/station.c
index 934815f2..61649c74 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1501,6 +1501,9 @@ static void station_reset_connection_state(struct station *station)
if (!network)
return;
+ if (station->netconfig)
+ netconfig_reset(station->netconfig);
+
if (station->state == STATION_STATE_CONNECTED ||
station->state == STATION_STATE_CONNECTING ||
station->state == STATION_STATE_CONNECTING_AUTO ||
@@ -1530,9 +1533,6 @@ static void station_disassociated(struct station *station)
{
l_debug("%u", netdev_get_ifindex(station->netdev));
- if (station->netconfig)
- netconfig_reset(station->netconfig);
-
station_reset_connection_state(station);
station_enter_state(station, STATION_STATE_DISCONNECTED);
@@ -2930,9 +2930,6 @@ static void station_disconnect_onconnect(struct station *station,
return;
}
- if (station->netconfig)
- netconfig_reset(station->netconfig);
-
station_reset_connection_state(station);
station_enter_state(station, STATION_STATE_DISCONNECTING);
@@ -3234,9 +3231,6 @@ int station_disconnect(struct station *station)
if (!station->connected_bss)
return -ENOTCONN;
- if (station->netconfig)
- netconfig_reset(station->netconfig);
-
/*
* If the disconnect somehow fails we won't know if we're still
* connected so we may as well indicate now that we're no longer
--
2.30.2
9:51 p.m.
Hi Andrew,
On 9/29/21 11:42 AM, Andrew Zaborowski wrote:
Move the netconfig_reset calls to inside
station_reset_connection_state() instead of having each caller do
This is on purpose. There should be no need to call netconfig_reset if
netconfig_configure wasn't called in the first place.
netconfig_reset() followed by station_reset_connection_state(). One
of
the callers was missing the netconfig_reset() and this was resulting in
the "netconfig: Failed to start DHCPv4 client for interface %u" warning.
You need to figure out why calling netconfig_load_settings causes side-effects
and fix that instead.
---
src/station.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
Regards,
-Denis
12:05 a.m.
Hi Denis,
On Wed, 29 Sept 2021 at 22:06, Denis Kenzior <denkenz(a)gmail.com> wrote:
On 9/29/21 11:42 AM, Andrew Zaborowski wrote:
> Move the netconfig_reset calls to inside
> station_reset_connection_state() instead of having each caller do
This is on purpose. There should be no need to call netconfig_reset if
netconfig_configure wasn't called in the first place.
In this case it *was* called, then for some reason the driver
deauthenticated when we requested a scan (from_ap is false), netdev
emitted a NETDEV_EVENT_DISCONNECT_BY_SME and we end up in
station_connect_cb. You're right in that station_connect_cb also
handles failures before the netconfig_configure call, so if you want
to avoid the potentially unneeded netconfig_reset() I can move it
inside station_disconnect_event to avoid bigger changes.
Best regards
12:07 a.m.
Hi Andrew,
On 9/29/21 5:05 PM, Andrew Zaborowski wrote:
Hi Denis,
On Wed, 29 Sept 2021 at 22:06, Denis Kenzior <denkenz(a)gmail.com> wrote:
> On 9/29/21 11:42 AM, Andrew Zaborowski wrote:
>> Move the netconfig_reset calls to inside
>> station_reset_connection_state() instead of having each caller do
>
> This is on purpose. There should be no need to call netconfig_reset if
> netconfig_configure wasn't called in the first place.
In this case it *was* called, then for some reason the driver
deauthenticated when we requested a scan (from_ap is false), netdev
That would be useful info and should be in the commit description ;)
emitted a NETDEV_EVENT_DISCONNECT_BY_SME and we end up in
station_connect_cb. You're right in that station_connect_cb also
Then that sounds like a bug or another side-effect of delaying the CONNECTED
state until netconfig is done. NETDEV_EVENT_DISCONNECT_BY_SME should result
in station_disassociated() and not station_connect_cb()
handles failures before the netconfig_configure call, so if you want
to avoid the potentially unneeded netconfig_reset() I can move it
inside station_disconnect_event to avoid bigger changes.
Regards,
-Denis
12:20 a.m.
On Thu, 30 Sept 2021 at 00:12, Denis Kenzior <denkenz(a)gmail.com> wrote:
On 9/29/21 5:05 PM, Andrew Zaborowski wrote:
> On Wed, 29 Sept 2021 at 22:06, Denis Kenzior <denkenz(a)gmail.com> wrote:
>> On 9/29/21 11:42 AM, Andrew Zaborowski wrote:
>>> Move the netconfig_reset calls to inside
>>> station_reset_connection_state() instead of having each caller do
>>
>> This is on purpose. There should be no need to call netconfig_reset if
>> netconfig_configure wasn't called in the first place.
>
> In this case it *was* called, then for some reason the driver
> deauthenticated when we requested a scan (from_ap is false), netdev
That would be useful info and should be in the commit description ;)
> emitted a NETDEV_EVENT_DISCONNECT_BY_SME and we end up in
> station_connect_cb. You're right in that station_connect_cb also
Then that sounds like a bug or another side-effect of delaying the CONNECTED
state until netconfig is done. NETDEV_EVENT_DISCONNECT_BY_SME should result
in station_disassociated() and not station_connect_cb()
I don't remember what the reasoning was, but station_connect_cb does
handle some mechanisms that seem desirable here also, like trying the
next bss from the autoconnect list immediately and blacklisting BSSes.
In this case it seems the problem was triggered by a driver problem
(or our misunderstanding on when the radio is free for scanning) but
if a specific BSS always fails during netconfig then we should
blacklist it.
Best regards
12:46 a.m.
Hi Andrew,
I don't remember what the reasoning was, but station_connect_cb
does
handle some mechanisms that seem desirable here also, like trying the
next bss from the autoconnect list immediately and blacklisting BSSes.
The connect failed path is generally used if the connection simply fails.
Either due to an invalid password, or due to handshake timeout, or some other
reason where the status_code can be returned by netdev. Once we're successfully
connected, and the driver/kernel decides to disconnect us due to a firmware
crash/bug/whatever, then I'm not sure blacklisting the bss should be done.
Besides, we already sent the dbus message reply, etc.
Now, maybe some elements of what the connect failed path does might be needed in
this failure case. But to determine that we would need to know why the
disconnect happens? How often? Is it every time? Sometimes? Once in a blue moon,
etc.
In this case it seems the problem was triggered by a driver problem
(or our misunderstanding on when the radio is free for scanning) but
if a specific BSS always fails during netconfig then we should
blacklist it.
Sure, but then these actions should be taken in the station_disassociated path,
or a brand new path specifically for this case.
Regards,
-Denis
1:14 a.m.
On Thu, 30 Sept 2021 at 00:57, Denis Kenzior <denkenz(a)gmail.com> wrote:
> I don't remember what the reasoning was, but
station_connect_cb does
> handle some mechanisms that seem desirable here also, like trying the
> next bss from the autoconnect list immediately and blacklisting BSSes.
The connect failed path is generally used if the connection simply fails.
Either due to an invalid password, or due to handshake timeout, or some other
reason where the status_code can be returned by netdev. Once we're successfully
connected, and the driver/kernel decides to disconnect us due to a firmware
crash/bug/whatever, then I'm not sure blacklisting the bss should be done.
Besides, we already sent the dbus message reply, etc.
Now, maybe some elements of what the connect failed path does might be needed in
this failure case. But to determine that we would need to know why the
disconnect happens? How often? Is it every time? Sometimes? Once in a blue moon,
etc.
Unfortunately this applies to most failure modes in a connection (at
802.11 level or at IP level -- for the user they're the same thing)
and I think the binary blacklisted/not blacklisted model we use
doesn't really work. IIRC my initial patches used a different model
where the blacklist was a kind of penalty list where BSSes and/or
networks that fail more often are pushed further in the priority list.
In the end when we've tried all the BSSes in range we have nothing to
lose by retrying some of the ones we've already tried, we just don't
want to put them before other BSSes. This can be done in a way that
over time the list converges to sorted by how often a network/bss
fails.
This also allows you to learn from some very clear cues that we're not
handling, like a user manually switching to another network.
Best regards
1:09 a.m.
Hi Andrew,
> Now, maybe some elements of what the connect failed path does
might be needed in
> this failure case. But to determine that we would need to know why the
> disconnect happens? How often? Is it every time? Sometimes? Once in a blue moon,
> etc.
Unfortunately this applies to most failure modes in a connection (at
802.11 level or at IP level -- for the user they're the same thing)
I agree, but details matter. So what is it you're proposing exactly?
and I think the binary blacklisted/not blacklisted model we use
doesn't really work. IIRC my initial patches used a different model
where the blacklist was a kind of penalty list where BSSes and/or
networks that fail more often are pushed further in the priority list.
I remember, but we have since moved on from that model of autoconnect. We
autoconnect networks now, not individual BSSes. Eventually we will move to a
network priority model in the future where the user will specify the priority of
a given network for autoconnect. Blacklisting within the network is a different
topic. Also note that we have two levels of blacklisting, so it isn't quite as
simple as you describe.
In the end when we've tried all the BSSes in range we have
nothing to
lose by retrying some of the ones we've already tried, we just don't
want to put them before other BSSes. This can be done in a way that
We do that already.
over time the list converges to sorted by how often a network/bss
fails.
This also allows you to learn from some very clear cues that we're not
handling, like a user manually switching to another network.
Regards,
-Denis
232
days inactive
232
days old
7 comments
2 participants
participants (2)
-
Andrew Zaborowski -
Denis Kenzior