netconfig: Split route add/del callbacks

PATCH Applied Tim Kourt 2 participants 8 messages Oct 8, 2019 – Oct 9, 2019

Share & tools

Tim Kourt

author

2019-10-08 21:48 UTC #0 lore raw

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

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

src/netconfig.c +17 -6

index 84eea7f5..31cbeb2a 100644
@@ -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);
 	}

Signature · 1 line

2.13.6

Tim Kourt

author

2019-10-08 21:48 UTC #1 lore raw

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

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(-)

src/station.c +29 ~~-22~~

index 72393d51..9997352e 100644
@@ -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,

Signature · 1 line

2.13.6

Denis Kenzior

2019-10-09 19:29 UTC #2 lore raw

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

Hi Tim,

Quoted 16 lines · Tim Kourt

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(-)
src/station.c +0 -0
index 72393d51..9997352e 100644

src/station.c static void station_enter_state(struct station *station, · line 1166

Quoted 5 lines

src/station.c +0 -2

@@ -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...

Quoted 27 lines

src/station.c +5 ~~-15~~

-		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) {

src/station.c static void station_roam_failed(struct station *station) · line 1326

Quoted 10 lines

src/station.c +10 -0

+		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...

Quoted 11 lines

src/station.c +7 -0

+
+	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,

src/station.c static void station_reassociate_cb(struct netdev *netdev, · line 1349

Quoted 7 lines

src/station.c +2 -2

@@ -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...

src/station.c static void station_fast_transition_cb(struct netdev *netdev, · line 1368

Quoted 12 lines

src/station.c +2 -2

  		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

Quoted 8 lines

src/station.c +2 -0

+	} else {
  		station_roam_failed(station);
+	}
  }
 
  static void station_netdev_event(struct netdev *netdev, enum netdev_event event,

src/station.c static void station_connect_cb(struct netdev *netdev, enum netdev_result result, · line 2293

Quoted 8 lines

src/station.c +1 -1

@@ -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

Tim Kourt

author

2019-10-08 21:48 UTC #3 lore raw

PATCH 3/5 netconfig: Add netconfig event notifier

In reply to Tim Kourt

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(-)

src/netconfig.c +21 -7

index 31cbeb2a..ec8d0af3 100644
@@ -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);

src/netconfig.h +10 -1

index cacd384a..5a527950 100644
@@ -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);

Signature · 1 line

2.13.6

Tim Kourt

author

2019-10-08 21:48 UTC #4 lore raw

PATCH 4/5 station: Subscribe to netconfig event notifier

In reply to Tim Kourt

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

src/station.c +19 -1

index 9997352e..46f2a02e 100644
@@ -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,

Signature · 1 line

2.13.6

Tim Kourt

author

2019-10-08 21:48 UTC #5 lore raw

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

In reply to Tim Kourt

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

src/station.c +5 -5

index 46f2a02e..dc9ec7a1 100644
@@ -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:

Signature · 1 line

2.13.6

Denis Kenzior

2019-10-09 19:37 UTC #6 lore raw

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

Hi Tim,

Quoted 17 lines · Tim Kourt

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

src/station.c +0 -4

index 46f2a02e..dc9ec7a1 100644
@@ -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);

src/station.c static void station_enter_state(struct station *station, · line 1143

Quoted 10 lines

src/station.c +5 -1

 
  	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...

src/station.c static void station_enter_state(struct station *station,

Quoted 4 lines

src/station.c +0 -0

  	switch (state) {
  	case STATION_STATE_AUTOCONNECT_QUICK:

Regards,
-Denis

Denis Kenzior

Applied

2019-10-09 19:19 UTC #7 lore raw

In reply to Tim Kourt

Hi Tim,

Quoted 4 lines · Tim Kourt

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

Applied, thanks.

Regards,
-Denis