[PATCH 1/5] netconfig: Split route add/del callbacks

[PATCH 1/5] netconfig: Split route add/del callbacks

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]

---
 src/netconfig.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/netconfig.c b/src/netconfig.c
index 84eea7f5..31cbeb2a 100644
--- a/src/netconfig.c
+++ b/src/netconfig.c
@@ -591,14 +591,25 @@ static void netconfig_ifaddr_ipv6_cmd_cb(int error, uint16_t type,
 	netconfig_ifaddr_ipv6_notify(type, data, len, user_data);
 }
 
-static void netconfig_route_cmd_cb(int error, uint16_t type,
+static void netconfig_route_add_cmd_cb(int error, uint16_t type,
 						const void *data, uint32_t len,
 						void *user_data)
 {
 	if (!error)
 		return;
 
-	l_error("netconfig: Route command failure. Error %d: %s",
+	l_error("netconfig: Failed to add route. Error %d: %s",
+						error, strerror(-error));
+}
+
+static void netconfig_route_del_cmd_cb(int error, uint16_t type,
+						const void *data, uint32_t len,
+						void *user_data)
+{
+	if (!error)
+		return;
+
+	l_error("netconfig: Failed to delete route. Error %d: %s",
 						error, strerror(-error));
 }
 
@@ -623,7 +634,7 @@ static bool netconfig_ipv4_routes_install(struct netconfig *netconfig,
 						ifaddr->prefix_len, network,
 						ifaddr->ip,
 						netconfig->rtm_protocol,
-						netconfig_route_cmd_cb,
+						netconfig_route_add_cmd_cb,
 						NULL, NULL)) {
 		l_error("netconfig: Failed to add subnet route.");
 
@@ -643,7 +654,7 @@ static bool netconfig_ipv4_routes_install(struct netconfig *netconfig,
 						ifaddr->ip,
 						ROUTE_PRIORITY_OFFSET,
 						netconfig->rtm_protocol,
-						netconfig_route_cmd_cb,
+						netconfig_route_add_cmd_cb,
 						NULL, NULL)) {
 		l_error("netconfig: Failed to add route for: %s gateway.",
 								gateway);
@@ -711,7 +722,7 @@ static bool netconfig_ipv6_routes_install(struct netconfig *netconfig)
 	if (!rtnl_route_ipv6_add_gateway(rtnl, netconfig->ifindex, gateway,
 						ROUTE_PRIORITY_OFFSET,
 						netconfig->rtm_v6_protocol,
-						netconfig_route_cmd_cb,
+						netconfig_route_add_cmd_cb,
 						NULL, NULL)) {
 		l_error("netconfig: Failed to add route for: %s gateway.",
 								gateway);
@@ -999,7 +1010,7 @@ static void netconfig_ipv6_select_and_uninstall(struct netconfig *netconfig)
 	if (!rtnl_route_ipv6_delete_gateway(rtnl, netconfig->ifindex,
 			gateway, ROUTE_PRIORITY_OFFSET,
 			netconfig->rtm_v6_protocol,
-			netconfig_route_cmd_cb, NULL, NULL)) {
+			netconfig_route_del_cmd_cb, NULL, NULL)) {
 		l_error("netconfig: Failed to delete route for: %s gateway.",
 								gateway);
 	}
-- 
2.13.6

[PATCH 2/5] station: Move 'connected' logic out of enter state func

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 3076 bytes --]

Previously, station state 'connected' used to identify an interface associated
with AP. With the introduction of netconfig, an interface is assumed to be
connected after the IP addresses have been assigned to it. If netconfig is
disabled, the behavior remains unchanged.

The change of station state by netconfig to 'connected' is implemented in
the follow up patches.
---
 src/station.c | 51 +++++++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/src/station.c b/src/station.c
index 72393d51..9997352e 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1162,23 +1162,6 @@ static void station_enter_state(struct station *station,
 
 		break;
 	case STATION_STATE_CONNECTED:
-		periodic_scan_stop(station);
-
-		if (!station->netconfig)
-			break;
-
-		if (station->state == STATION_STATE_ROAMING) {
-			netconfig_reconfigure(station->netconfig);
-
-			break;
-		}
-
-		netconfig_configure(station->netconfig,
-					network_get_settings(
-						station->connected_network),
-					netdev_get_address(
-							station->netdev));
-		break;
 	case STATION_STATE_DISCONNECTING:
 	case STATION_STATE_ROAMING:
 		break;
@@ -1326,6 +1309,28 @@ static void station_roam_failed(struct station *station)
 		station_roam_timeout_rearm(station, 60);
 }
 
+static void station_connected(struct station *station)
+{
+	periodic_scan_stop(station);
+
+	if (!station->netconfig) {
+		station_enter_state(station, STATION_STATE_CONNECTED);
+
+		return;
+	}
+
+	if (station->state == STATION_STATE_ROAMING) {
+		netconfig_reconfigure(station->netconfig);
+
+		return;
+	}
+
+	netconfig_configure(station->netconfig,
+				network_get_settings(
+						station->connected_network),
+				netdev_get_address(station->netdev));
+}
+
 static void station_reassociate_cb(struct netdev *netdev,
 					enum netdev_result result,
 					void *event_data,
@@ -1340,9 +1345,10 @@ static void station_reassociate_cb(struct netdev *netdev,
 
 	if (result == NETDEV_RESULT_OK) {
 		station_roamed(station);
-		station_enter_state(station, STATION_STATE_CONNECTED);
-	} else
+		station_connected(station);
+	} else {
 		station_roam_failed(station);
+	}
 }
 
 static void station_fast_transition_cb(struct netdev *netdev,
@@ -1359,9 +1365,10 @@ static void station_fast_transition_cb(struct netdev *netdev,
 
 	if (result == NETDEV_RESULT_OK) {
 		station_roamed(station);
-		station_enter_state(station, STATION_STATE_CONNECTED);
-	} else
+		station_connected(station);
+	} else {
 		station_roam_failed(station);
+	}
 }
 
 static void station_netdev_event(struct netdev *netdev, enum netdev_event event,
@@ -2280,7 +2287,7 @@ static void station_connect_cb(struct netdev *netdev, enum netdev_result result,
 	}
 
 	network_connected(station->connected_network);
-	station_enter_state(station, STATION_STATE_CONNECTED);
+	station_connected(station);
 }
 
 int __station_connect_network(struct station *station, struct network *network,
-- 
2.13.6

[PATCH 3/5] netconfig: Add netconfig event notifier

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 3663 bytes --]

The notifier allows to subscribe for the netconfig events such as ‘connected’.
---
 src/netconfig.c | 28 +++++++++++++++++++++-------
 src/netconfig.h | 11 ++++++++++-
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/src/netconfig.c b/src/netconfig.c
index 31cbeb2a..ec8d0af3 100644
--- a/src/netconfig.c
+++ b/src/netconfig.c
@@ -50,6 +50,9 @@ struct netconfig {
 	uint8_t rtm_v6_protocol;
 
 	const struct l_settings *active_settings;
+
+	netconfig_notify_func_t notify;
+	void *user_data;
 };
 
 struct netconfig_ifaddr {
@@ -595,11 +598,18 @@ static void netconfig_route_add_cmd_cb(int error, uint16_t type,
 						const void *data, uint32_t len,
 						void *user_data)
 {
-	if (!error)
-		return;
+	struct netconfig *netconfig = user_data;
 
-	l_error("netconfig: Failed to add route. Error %d: %s",
+	if (error) {
+		l_error("netconfig: Failed to add route. Error %d: %s",
 						error, strerror(-error));
+		return;
+	}
+
+	if (!netconfig->notify)
+		return;
+
+	netconfig->notify(NETCONFIG_EVENT_CONNECTED, netconfig->user_data);
 }
 
 static void netconfig_route_del_cmd_cb(int error, uint16_t type,
@@ -611,6 +621,7 @@ static void netconfig_route_del_cmd_cb(int error, uint16_t type,
 
 	l_error("netconfig: Failed to delete route. Error %d: %s",
 						error, strerror(-error));
+
 }
 
 static bool netconfig_ipv4_routes_install(struct netconfig *netconfig,
@@ -635,7 +646,7 @@ static bool netconfig_ipv4_routes_install(struct netconfig *netconfig,
 						ifaddr->ip,
 						netconfig->rtm_protocol,
 						netconfig_route_add_cmd_cb,
-						NULL, NULL)) {
+						netconfig, NULL)) {
 		l_error("netconfig: Failed to add subnet route.");
 
 		return false;
@@ -655,7 +666,7 @@ static bool netconfig_ipv4_routes_install(struct netconfig *netconfig,
 						ROUTE_PRIORITY_OFFSET,
 						netconfig->rtm_protocol,
 						netconfig_route_add_cmd_cb,
-						NULL, NULL)) {
+						netconfig, NULL)) {
 		l_error("netconfig: Failed to add route for: %s gateway.",
 								gateway);
 
@@ -723,7 +734,7 @@ static bool netconfig_ipv6_routes_install(struct netconfig *netconfig)
 						ROUTE_PRIORITY_OFFSET,
 						netconfig->rtm_v6_protocol,
 						netconfig_route_add_cmd_cb,
-						NULL, NULL)) {
+						netconfig, NULL)) {
 		l_error("netconfig: Failed to add route for: %s gateway.",
 								gateway);
 
@@ -1020,9 +1031,12 @@ static void netconfig_ipv6_select_and_uninstall(struct netconfig *netconfig)
 
 bool netconfig_configure(struct netconfig *netconfig,
 				const struct l_settings *active_settings,
-				const uint8_t *mac_address)
+				const uint8_t *mac_address,
+				netconfig_notify_func_t notify, void *user_data)
 {
 	netconfig->active_settings = active_settings;
+	netconfig->notify = notify;
+	netconfig->user_data = user_data;
 
 	l_dhcp_client_set_address(netconfig->dhcp_client, ARPHRD_ETHER,
 							mac_address, ETH_ALEN);
diff --git a/src/netconfig.h b/src/netconfig.h
index cacd384a..5a527950 100644
--- a/src/netconfig.h
+++ b/src/netconfig.h
@@ -22,9 +22,18 @@
 
 struct netconfig;
 
+enum netconfig_event {
+	NETCONFIG_EVENT_CONNECTED,
+};
+
+typedef void (*netconfig_notify_func_t)(enum netconfig_event event,
+							void *user_data);
+
 bool netconfig_configure(struct netconfig *netconfig,
 				const struct l_settings *active_settings,
-				const uint8_t *mac_address);
+				const uint8_t *mac_address,
+				netconfig_notify_func_t notify,
+				void *user_data);
 bool netconfig_reconfigure(struct netconfig *netconfig);
 bool netconfig_reset(struct netconfig *netconfig);
 
-- 
2.13.6

[PATCH 4/5] station: Subscribe to netconfig event notifier

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]

---
 src/station.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/station.c b/src/station.c
index 9997352e..46f2a02e 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1309,6 +1309,22 @@ static void station_roam_failed(struct station *station)
 		station_roam_timeout_rearm(station, 60);
 }
 
+static void station_netconfig_event_handler(enum netconfig_event event,
+							void *user_data)
+{
+	struct station *station = user_data;
+
+	switch (event) {
+	case NETCONFIG_EVENT_CONNECTED:
+		station_enter_state(station, STATION_STATE_CONNECTED);
+
+		break;
+	default:
+		l_error("station: Unsupported netconfig event: %d.", event);
+		break;
+	}
+}
+
 static void station_connected(struct station *station)
 {
 	periodic_scan_stop(station);
@@ -1328,7 +1344,9 @@ static void station_connected(struct station *station)
 	netconfig_configure(station->netconfig,
 				network_get_settings(
 						station->connected_network),
-				netdev_get_address(station->netdev));
+				netdev_get_address(station->netdev),
+				station_netconfig_event_handler,
+				station);
 }
 
 static void station_reassociate_cb(struct netdev *netdev,
-- 
2.13.6

[PATCH 5/5] station: Match debug output to what is seen on dbus

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

---
 src/station.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/station.c b/src/station.c
index 46f2a02e..dc9ec7a1 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1131,16 +1131,16 @@ static void station_enter_state(struct station *station,
 	struct l_dbus *dbus = dbus_get_bus();
 	bool disconnected;
 
-	l_debug("Old State: %s, new state: %s",
-			station_state_to_string(station->state),
-			station_state_to_string(state));
-
 	disconnected = !station_is_busy(station);
 
 	if ((disconnected && state > STATION_STATE_AUTOCONNECT_FULL) ||
-			(!disconnected && state != station->state))
+			(!disconnected && state != station->state)) {
 		l_dbus_property_changed(dbus, netdev_get_path(station->netdev),
 					IWD_STATION_INTERFACE, "State");
+		l_debug("Old State: %s, new state: %s",
+					station_state_to_string(station->state),
+					station_state_to_string(state));
+	}
 
 	switch (state) {
 	case STATION_STATE_AUTOCONNECT_QUICK:
-- 
2.13.6

Re: [PATCH 1/5] netconfig: Split route add/del callbacks

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 204 bytes --]

Hi Tim,

On 10/8/19 4:48 PM, Tim Kourt wrote:
> ---
>   src/netconfig.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 2/5] station: Move 'connected' logic out of enter state func

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 3804 bytes --]

Hi Tim,

On 10/8/19 4:48 PM, Tim Kourt wrote:
> Previously, station state 'connected' used to identify an interface associated
> with AP. With the introduction of netconfig, an interface is assumed to be
> connected after the IP addresses have been assigned to it. If netconfig is
> disabled, the behavior remains unchanged.
> 
> The change of station state by netconfig to 'connected' is implemented in
> the follow up patches.
> ---
>   src/station.c | 51 +++++++++++++++++++++++++++++----------------------
>   1 file changed, 29 insertions(+), 22 deletions(-)
> 
> diff --git a/src/station.c b/src/station.c
> index 72393d51..9997352e 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -1162,23 +1162,6 @@ static void station_enter_state(struct station *station,
>   
>   		break;
>   	case STATION_STATE_CONNECTED:
> -		periodic_scan_stop(station);
> -

Not sure why you're taking this out from here...

> -		if (!station->netconfig)
> -			break;
> -
> -		if (station->state == STATION_STATE_ROAMING) {
> -			netconfig_reconfigure(station->netconfig);
> -
> -			break;
> -		}
> -
> -		netconfig_configure(station->netconfig,
> -					network_get_settings(
> -						station->connected_network),
> -					netdev_get_address(
> -							station->netdev));
> -		break;
>   	case STATION_STATE_DISCONNECTING:
>   	case STATION_STATE_ROAMING:
>   		break;
> @@ -1326,6 +1309,28 @@ static void station_roam_failed(struct station *station)
>   		station_roam_timeout_rearm(station, 60);
>   }
>   
> +static void station_connected(struct station *station)
> +{
> +	periodic_scan_stop(station);
> +
> +	if (!station->netconfig) {
> +		station_enter_state(station, STATION_STATE_CONNECTED);
> +
> +		return;
> +	}
> +
> +	if (station->state == STATION_STATE_ROAMING) {
> +		netconfig_reconfigure(station->netconfig);
> +
> +		return;
> +	}

So we already have a station_roamed callback, I'd say just stuff this in 
there...

> +
> +	netconfig_configure(station->netconfig,
> +				network_get_settings(
> +						station->connected_network),
> +				netdev_get_address(station->netdev));
> +}
> +
>   static void station_reassociate_cb(struct netdev *netdev,
>   					enum netdev_result result,
>   					void *event_data,
> @@ -1340,9 +1345,10 @@ static void station_reassociate_cb(struct netdev *netdev,
>   
>   	if (result == NETDEV_RESULT_OK) {
>   		station_roamed(station);
> -		station_enter_state(station, STATION_STATE_CONNECTED);
> -	} else
> +		station_connected(station);
> +	} else {

This is confusing.  reassociate is used for roaming, so calling 
station_roamed and station_connected looks weird.  Why not handle all 
the netconfig_reconfigure parts in station_roamed and either call 
station_enter_state() there directly, or here...

>   		station_roam_failed(station);
> +	}
>   }
>   
>   static void station_fast_transition_cb(struct netdev *netdev,
> @@ -1359,9 +1365,10 @@ static void station_fast_transition_cb(struct netdev *netdev,
>   
>   	if (result == NETDEV_RESULT_OK) {
>   		station_roamed(station);
> -		station_enter_state(station, STATION_STATE_CONNECTED);
> -	} else
> +		station_connected(station);

Same comment as above

> +	} else {
>   		station_roam_failed(station);
> +	}
>   }
>   
>   static void station_netdev_event(struct netdev *netdev, enum netdev_event event,
> @@ -2280,7 +2287,7 @@ static void station_connect_cb(struct netdev *netdev, enum netdev_result result,
>   	}
>   
>   	network_connected(station->connected_network);
> -	station_enter_state(station, STATION_STATE_CONNECTED);
> +	station_connected(station);
>   }
>   
>   int __station_connect_network(struct station *station, struct network *network,
> 

Regards,
-Denis

Re: [PATCH 5/5] station: Match debug output to what is seen on dbus

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

Hi Tim,

On 10/8/19 4:48 PM, Tim Kourt wrote:
> ---
>   src/station.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/station.c b/src/station.c
> index 46f2a02e..dc9ec7a1 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -1131,16 +1131,16 @@ static void station_enter_state(struct station *station,
>   	struct l_dbus *dbus = dbus_get_bus();
>   	bool disconnected;
>   
> -	l_debug("Old State: %s, new state: %s",
> -			station_state_to_string(station->state),
> -			station_state_to_string(state));
> -
>   	disconnected = !station_is_busy(station);
>   
>   	if ((disconnected && state > STATION_STATE_AUTOCONNECT_FULL) ||
> -			(!disconnected && state != station->state))
> +			(!disconnected && state != station->state)) {
>   		l_dbus_property_changed(dbus, netdev_get_path(station->netdev),
>   					IWD_STATION_INTERFACE, "State");
> +		l_debug("Old State: %s, new state: %s",
> +					station_state_to_string(station->state),
> +					station_state_to_string(state));
> +	}

Hmm, I'm not so sure about this.  You can see the dbus signaling via 
test/monitor-iwd.  And knowing when we enter the various autoconnect 
states is useful...

>   
>   	switch (state) {
>   	case STATION_STATE_AUTOCONNECT_QUICK:
> 

Regards,
-Denis