All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] doc: diagnostic DBus interface definition
@ 2021-01-11 17:12 James Prestwood
  2021-01-11 17:12 ` [PATCH 2/8] netdev: add netdev_get_station James Prestwood
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: James Prestwood @ 2021-01-11 17:12 UTC (permalink / raw)
  To: iwd

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

---
 doc/diagnostics.txt | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 doc/diagnostics.txt

diff --git a/doc/diagnostics.txt b/doc/diagnostics.txt
new file mode 100644
index 00000000..2354d316
--- /dev/null
+++ b/doc/diagnostics.txt
@@ -0,0 +1,36 @@
+Station diagnostic hierarchy
+=================
+
+Service		net.connman.iwd
+Interface	net.connman.iwd.StationDiagnostic
+Object path	/net/connman/iwd/{phy0,phy1,...}/{1,2,...}
+
+Methods		dict GetDiagnostics()
+
+			Get all diagnostic information for this interface. The
+			diagnostics are contained in a single dictionary. Values
+			here are generally low level and not meant for general
+			purpose applications which could get by with the
+			existing Station interface or values which are volatile
+			and change too frequently to be represented as
+			properties. The values in the dictionary may come and
+			go depending on the state of IWD.
+
+			Below is a list of possible diagnostic dictionary
+			values:
+
+			ConnectedBss - MAC address of currently connected BSS.
+
+			RSSI -	The RSSI of the currently connected BSS.
+
+			RxRate - Receive rate in 100kbit/s
+
+			RxMCS - Receiving MCS index
+
+			TxRate - Transmission rate in 100kbit/s
+
+			TxMCS - Transmitting MCS index
+
+			Possible errors: net.connman.iwd.Busy
+					 net.connman.iwd.Failed
+					 net.connman.iwd.NotConnected
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/8] netdev: add netdev_get_station
  2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
@ 2021-01-11 17:12 ` James Prestwood
  2021-01-11 20:49   ` Denis Kenzior
  2021-01-11 17:12 ` [PATCH 3/8] netdev: update RSSI polling to use netdev_get_station James Prestwood
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: James Prestwood @ 2021-01-11 17:12 UTC (permalink / raw)
  To: iwd

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

This adds a generalized API for GET_STATION. This API handles
calling and parsing the results into a new structure,
netdev_station_info. This results structure will hold any
data needed by consumers of netdev_get_station.

For now only the RSSI is parsed as this is already being
done for RSSI polling/events. Looking further more info will
be added such as rx/tx rates and estimated throughput.
---
 src/netdev.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/netdev.h |  12 ++++++
 2 files changed, 115 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index 3f78afbf..3cda1c48 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -132,6 +132,11 @@ struct netdev {
 	void *set_powered_user_data;
 	netdev_destroy_func_t set_powered_destroy;
 
+	uint32_t get_station_cmd_id;
+	netdev_get_station_cb_t get_station_cb;
+	void *get_station_data;
+	netdev_destroy_func_t get_station_destroy;
+
 	struct watchlist station_watches;
 
 	struct l_io *pae_io;  /* for drivers without EAPoL over NL80211 */
@@ -636,6 +641,11 @@ static void netdev_free(void *data)
 		netdev->mac_change_cmd_id = 0;
 	}
 
+	if (netdev->get_station_cmd_id) {
+		l_genl_family_cancel(nl80211, netdev->get_station_cmd_id);
+		netdev->get_station_cmd_id = 0;
+	}
+
 	if (netdev->events_ready)
 		WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t,
 					netdev, NETDEV_WATCH_EVENT_DEL);
@@ -4022,6 +4032,99 @@ done:
 	return 0;
 }
 
+static bool netdev_parse_sta_info(struct l_genl_attr *attr,
+					struct netdev_station_info *info)
+{
+	uint16_t type, len;
+	const void *data;
+
+	while (l_genl_attr_next(attr, &type, &len, &data)) {
+		switch (type) {
+		case NL80211_STA_INFO_SIGNAL_AVG:
+			if (len != 1)
+				return false;
+
+			info->cur_rssi = *(const int8_t *) data;
+
+			break;
+		}
+	}
+
+	return true;
+}
+
+static void netdev_get_station_cb(struct l_genl_msg *msg, void *user_data)
+{
+	struct netdev *netdev = user_data;
+	struct l_genl_attr attr, nested;
+	uint16_t type, len;
+	const void *data;
+	struct netdev_station_info info;
+
+	netdev->get_station_cmd_id = 0;
+
+	if (!l_genl_attr_init(&attr, msg))
+		goto parse_error;
+
+	while (l_genl_attr_next(&attr, &type, &len, &data)) {
+		switch (type) {
+		case NL80211_ATTR_STA_INFO:
+			if (!l_genl_attr_recurse(&attr, &nested))
+				goto parse_error;
+
+			if (!netdev_parse_sta_info(&nested, &info))
+				goto parse_error;
+
+			break;
+		}
+	}
+
+	if (netdev->get_station_cb)
+		netdev->get_station_cb(netdev, &info, netdev->get_station_data);
+
+	return;
+
+parse_error:
+	if (netdev->get_station_cb)
+		netdev->get_station_cb(netdev, NULL, netdev->get_station_data);
+}
+
+static void netdev_get_station_destroy(void *user_data)
+{
+	struct netdev *netdev = user_data;
+
+	netdev->get_station_cmd_id = 0;
+
+	if (netdev->get_station_destroy)
+		netdev->get_station_destroy(netdev->get_station_data);
+}
+
+int netdev_get_station(struct netdev *netdev, const uint8_t *mac,
+			netdev_get_station_cb_t cb, void *user_data,
+			netdev_destroy_func_t destroy)
+{
+	struct l_genl_msg *msg;
+
+	if (netdev->get_station_cmd_id)
+		return -EBUSY;
+
+	msg = l_genl_msg_new_sized(NL80211_CMD_GET_STATION, 64);
+	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
+	l_genl_msg_append_attr(msg, NL80211_ATTR_MAC, ETH_ALEN, mac);
+
+	netdev->get_station_cmd_id = l_genl_family_send(nl80211, msg,
+						netdev_get_station_cb, netdev,
+						netdev_get_station_destroy);
+	if (!netdev->get_station_cmd_id)
+		return -EIO;
+
+	netdev->get_station_cb = cb;
+	netdev->get_station_data = user_data;
+	netdev->get_station_destroy = destroy;
+
+	return 0;
+}
+
 static int netdev_cqm_rssi_update(struct netdev *netdev)
 {
 	struct l_genl_msg *msg;
diff --git a/src/netdev.h b/src/netdev.h
index 65fdbaaf..5cf38076 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -114,6 +114,14 @@ typedef void (*netdev_station_watch_func_t)(struct netdev *netdev,
 					const uint8_t *mac, bool added,
 					void *user_data);
 
+struct netdev_station_info {
+	int8_t cur_rssi;
+};
+
+typedef void (*netdev_get_station_cb_t)(struct netdev *netdev,
+					struct netdev_station_info *info,
+					void *user_data);
+
 struct wiphy *netdev_get_wiphy(struct netdev *netdev);
 const uint8_t *netdev_get_address(struct netdev *netdev);
 uint32_t netdev_get_ifindex(struct netdev *netdev);
@@ -174,6 +182,10 @@ int netdev_neighbor_report_req(struct netdev *netdev,
 int netdev_set_rssi_report_levels(struct netdev *netdev, const int8_t *levels,
 					size_t levels_num);
 
+int netdev_get_station(struct netdev *netdev, const uint8_t *mac,
+			netdev_get_station_cb_t cb, void *user_data,
+			netdev_destroy_func_t destroy);
+
 void netdev_handshake_failed(struct handshake_state *hs, uint16_t reason_code);
 
 struct netdev *netdev_find(int ifindex);
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/8] netdev: update RSSI polling to use netdev_get_station
  2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
  2021-01-11 17:12 ` [PATCH 2/8] netdev: add netdev_get_station James Prestwood
@ 2021-01-11 17:12 ` James Prestwood
  2021-01-11 17:12 ` [PATCH 4/8] netdev: parse rates in netdev_get_station James Prestwood
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: James Prestwood @ 2021-01-11 17:12 UTC (permalink / raw)
  To: iwd

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

---
 src/netdev.c | 61 +++++++++-------------------------------------------
 1 file changed, 10 insertions(+), 51 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index 3cda1c48..1e54acec 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -124,7 +124,6 @@ struct netdev {
 	uint8_t cur_rssi_level_idx;
 	int8_t cur_rssi;
 	struct l_timeout *rssi_poll_timeout;
-	uint32_t rssi_poll_cmd_id;
 	uint8_t set_mac_once[6];
 
 	uint32_t set_powered_cmd_id;
@@ -379,47 +378,16 @@ static void netdev_set_rssi_level_idx(struct netdev *netdev)
 	netdev->cur_rssi_level_idx = new_level;
 }
 
-static void netdev_rssi_poll_cb(struct l_genl_msg *msg, void *user_data)
+static void netdev_rssi_poll_cb(struct netdev *netdev,
+				struct netdev_station_info *info,
+				void *user_data)
 {
-	struct netdev *netdev = user_data;
-	struct l_genl_attr attr, nested;
-	uint16_t type, len;
-	const void *data;
-	bool found;
 	uint8_t prev_rssi_level_idx = netdev->cur_rssi_level_idx;
 
-	netdev->rssi_poll_cmd_id = 0;
-
-	if (!l_genl_attr_init(&attr, msg))
-		goto done;
-
-	found = false;
-	while (l_genl_attr_next(&attr, &type, &len, &data)) {
-		if (type != NL80211_ATTR_STA_INFO)
-			continue;
-
-		found = true;
-		break;
-	}
-
-	if (!found || !l_genl_attr_recurse(&attr, &nested))
+	if (!info)
 		goto done;
 
-	found = false;
-	while (l_genl_attr_next(&nested, &type, &len, &data)) {
-		if (type != NL80211_STA_INFO_SIGNAL_AVG)
-			continue;
-
-		if (len != 1)
-			continue;
-
-		found = true;
-		netdev->cur_rssi = *(const int8_t *) data;
-		break;
-	}
-
-	if (!found)
-		goto done;
+	netdev->cur_rssi =  info->cur_rssi;
 
 	/*
 	 * Note we don't have to handle LOW_SIGNAL_THRESHOLD here.  The
@@ -441,16 +409,12 @@ done:
 static void netdev_rssi_poll(struct l_timeout *timeout, void *user_data)
 {
 	struct netdev *netdev = user_data;
-	struct l_genl_msg *msg;
-
-	msg = l_genl_msg_new_sized(NL80211_CMD_GET_STATION, 64);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_MAC, ETH_ALEN,
-							netdev->handshake->aa);
 
-	netdev->rssi_poll_cmd_id = l_genl_family_send(nl80211, msg,
-							netdev_rssi_poll_cb,
-							netdev, NULL);
+	if (netdev_get_station(netdev, netdev->handshake->aa,
+				netdev_rssi_poll_cb, NULL, NULL) < 0) {
+		/* Some problem occurred, rearm timer and try again */
+		l_timeout_modify(netdev->rssi_poll_timeout, 6);
+	}
 }
 
 /* To be called whenever operational or rssi_levels_num are updated */
@@ -472,11 +436,6 @@ static void netdev_rssi_polling_update(struct netdev *netdev)
 
 		l_timeout_remove(netdev->rssi_poll_timeout);
 		netdev->rssi_poll_timeout = NULL;
-
-		if (netdev->rssi_poll_cmd_id) {
-			l_genl_family_cancel(nl80211, netdev->rssi_poll_cmd_id);
-			netdev->rssi_poll_cmd_id = 0;
-		}
 	}
 }
 
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/8] netdev: parse rates in netdev_get_station
  2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
  2021-01-11 17:12 ` [PATCH 2/8] netdev: add netdev_get_station James Prestwood
  2021-01-11 17:12 ` [PATCH 3/8] netdev: update RSSI polling to use netdev_get_station James Prestwood
@ 2021-01-11 17:12 ` James Prestwood
  2021-01-11 17:12 ` [PATCH 5/8] dbus: add diagnostic interface definition James Prestwood
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: James Prestwood @ 2021-01-11 17:12 UTC (permalink / raw)
  To: iwd

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

---
 src/netdev.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/netdev.h |  4 ++++
 2 files changed, 62 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index 1e54acec..6fb33847 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -3991,11 +3991,50 @@ done:
 	return 0;
 }
 
+static bool netdev_parse_bitrate(struct l_genl_attr *attr, uint32_t *rate_out,
+					uint8_t *mcs_out)
+{
+	uint16_t type, len;
+	const void *data;
+	uint32_t rate = 0;
+	uint8_t mcs = 0;
+
+	while (l_genl_attr_next(attr, &type, &len, &data)) {
+		switch (type) {
+		case NL80211_RATE_INFO_BITRATE32:
+			if (len != 4)
+				return false;
+
+			l_debug("parsing bitrate");
+			rate = *(const uint32_t *) data;
+
+			break;
+
+		case NL80211_RATE_INFO_MCS:
+			if (len != 1)
+				return false;
+
+			mcs = *(const uint8_t *) data;
+
+			break;
+		}
+	}
+
+	if (!rate || !mcs)
+		return false;
+
+	*rate_out = rate;
+	*mcs_out = mcs;
+
+	return true;
+}
+
 static bool netdev_parse_sta_info(struct l_genl_attr *attr,
 					struct netdev_station_info *info)
 {
 	uint16_t type, len;
 	const void *data;
+	struct l_genl_attr nested;
 
 	while (l_genl_attr_next(attr, &type, &len, &data)) {
 		switch (type) {
@@ -4005,6 +4044,25 @@ static bool netdev_parse_sta_info(struct l_genl_attr *attr,
 
 			info->cur_rssi = *(const int8_t *) data;
 
+			break;
+		case NL80211_STA_INFO_RX_BITRATE:
+			if (!l_genl_attr_recurse(attr, &nested))
+				return false;
+
+			if (!netdev_parse_bitrate(&nested, &info->rx_bitrate,
+							&info->rx_mcs))
+				return false;
+
+			break;
+
+		case NL80211_STA_INFO_TX_BITRATE:
+			if (!l_genl_attr_recurse(attr, &nested))
+				return false;
+
+			if (!netdev_parse_bitrate(&nested, &info->tx_bitrate,
+							&info->tx_mcs))
+				return false;
+
 			break;
 		}
 	}
diff --git a/src/netdev.h b/src/netdev.h
index 5cf38076..c5d1ff53 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -116,6 +116,10 @@ typedef void (*netdev_station_watch_func_t)(struct netdev *netdev,
 
 struct netdev_station_info {
 	int8_t cur_rssi;
+	uint32_t rx_bitrate;
+	uint8_t rx_mcs;
+	uint32_t tx_bitrate;
+	uint8_t tx_mcs;
 };
 
 typedef void (*netdev_get_station_cb_t)(struct netdev *netdev,
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/8] dbus: add diagnostic interface definition
  2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
                   ` (2 preceding siblings ...)
  2021-01-11 17:12 ` [PATCH 4/8] netdev: parse rates in netdev_get_station James Prestwood
@ 2021-01-11 17:12 ` James Prestwood
  2021-01-11 20:52   ` Denis Kenzior
  2021-01-11 17:12 ` [PATCH 6/8] netdev: parse expected throughput in netdev_get_station James Prestwood
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: James Prestwood @ 2021-01-11 17:12 UTC (permalink / raw)
  To: iwd

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

---
 src/dbus.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/dbus.h b/src/dbus.h
index 045b1bcb..3e2018d8 100644
--- a/src/dbus.h
+++ b/src/dbus.h
@@ -39,6 +39,7 @@
 #define IWD_P2P_PEER_INTERFACE "net.connman.iwd.p2p.Peer"
 #define IWD_P2P_SERVICE_MANAGER_INTERFACE "net.connman.iwd.p2p.ServiceManager"
 #define IWD_P2P_WFD_INTERFACE "net.connman.iwd.p2p.Display"
+#define IWD_STATION_DIAGNOSTIC_INTERFACE "net.connman.iwd.StationDiagnostic"
 
 #define IWD_BASE_PATH "/net/connman/iwd"
 #define IWD_AGENT_MANAGER_PATH IWD_BASE_PATH
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/8] netdev: parse expected throughput in netdev_get_station
  2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
                   ` (3 preceding siblings ...)
  2021-01-11 17:12 ` [PATCH 5/8] dbus: add diagnostic interface definition James Prestwood
@ 2021-01-11 17:12 ` James Prestwood
  2021-01-11 17:12 ` [PATCH 7/8] station: create StationDiagnostic interface James Prestwood
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: James Prestwood @ 2021-01-11 17:12 UTC (permalink / raw)
  To: iwd

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

---
 src/netdev.c | 8 ++++++++
 src/netdev.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index 6fb33847..ba5912fd 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -4064,6 +4064,14 @@ static bool netdev_parse_sta_info(struct l_genl_attr *attr,
 				return false;
 
 			break;
+
+		case NL80211_STA_INFO_EXPECTED_THROUGHPUT:
+			if (len != 4)
+				return false;
+
+			info->expected_throughput = *(const uint32_t *)data;
+
+			break;
 		}
 	}
 
diff --git a/src/netdev.h b/src/netdev.h
index c5d1ff53..afff33ab 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -120,6 +120,7 @@ struct netdev_station_info {
 	uint8_t rx_mcs;
 	uint32_t tx_bitrate;
 	uint8_t tx_mcs;
+	uint32_t expected_throughput;
 };
 
 typedef void (*netdev_get_station_cb_t)(struct netdev *netdev,
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/8] station: create StationDiagnostic interface
  2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
                   ` (4 preceding siblings ...)
  2021-01-11 17:12 ` [PATCH 6/8] netdev: parse expected throughput in netdev_get_station James Prestwood
@ 2021-01-11 17:12 ` James Prestwood
  2021-01-11 21:04   ` Denis Kenzior
  2021-01-11 17:12 ` [PATCH 8/8] test: add a script for GetDiagnostics James Prestwood
  2021-01-11 20:51 ` [PATCH 1/8] doc: diagnostic DBus interface definition Denis Kenzior
  7 siblings, 1 reply; 14+ messages in thread
From: James Prestwood @ 2021-01-11 17:12 UTC (permalink / raw)
  To: iwd

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

This interface sits aside the regular station interface but
provides low level connection details for diagnostic and
testing purposes.
---
 src/station.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/src/station.c b/src/station.c
index 1e3706af..8cdd5443 100644
--- a/src/station.c
+++ b/src/station.c
@@ -77,6 +77,7 @@ struct station {
 	struct l_dbus_message *hidden_pending;
 	struct l_dbus_message *disconnect_pending;
 	struct l_dbus_message *scan_pending;
+	struct l_dbus_message *get_station_pending;
 	struct signal_agent *signal_agent;
 	uint32_t dbus_scan_id;
 	uint32_t quick_scan_id;
@@ -3333,6 +3334,9 @@ static struct station *station_create(struct netdev *netdev)
 
 	l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
 					IWD_STATION_INTERFACE, station);
+	l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
+					IWD_STATION_DIAGNOSTIC_INTERFACE,
+					station);
 
 	if (netconfig_enabled)
 		station->netconfig = netconfig_new(netdev_get_ifindex(netdev));
@@ -3455,6 +3459,111 @@ static void station_destroy_interface(void *user_data)
 	station_free(station);
 }
 
+/*
+ * Helper to append a dictionary value. This will only work for basic types.
+ */
+static void append_dict(struct l_dbus_message_builder *builder,
+			const char *name, const char *type, const void *data)
+{
+	l_dbus_message_builder_enter_dict(builder, "sv");
+	l_dbus_message_builder_append_basic(builder, 's', name);
+	l_dbus_message_builder_enter_variant(builder, type);
+	l_dbus_message_builder_append_basic(builder, type[0], data);
+	l_dbus_message_builder_leave_variant(builder);
+	l_dbus_message_builder_leave_dict(builder);
+}
+
+static void station_build_diagnostic_dict(struct station *station,
+					struct l_dbus_message *msg,
+					struct netdev_station_info *info)
+{
+	struct l_dbus_message_builder *builder =
+					l_dbus_message_builder_new(msg);
+	int16_t rssi = (int16_t)info->cur_rssi;
+
+	l_dbus_message_builder_enter_array(builder, "{sv}");
+
+	append_dict(builder, "ConnectedBss", "s", util_address_to_string(
+						station->connected_bss->addr));
+	append_dict(builder, "RSSI", "n", &rssi);
+	append_dict(builder, "TxBitrate", "u", &info->tx_bitrate);
+	append_dict(builder, "TxMCS", "y", &info->tx_mcs);
+	append_dict(builder, "RxBitrate", "u", &info->rx_bitrate);
+	append_dict(builder, "RxMCS", "y", &info->rx_mcs);
+	append_dict(builder, "ExpectedThroughput", "u",
+				&info->expected_throughput);
+
+	l_dbus_message_builder_leave_array(builder);
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+}
+
+static void station_get_diagnostic_cb(struct netdev *netdev,
+					struct netdev_station_info *info,
+					void *user_data)
+{
+	struct station *station = user_data;
+	struct l_dbus_message *reply;
+
+	if (!info) {
+		reply = dbus_error_aborted(station->get_station_pending);
+		goto done;
+	}
+
+	reply = l_dbus_message_new_method_return(station->get_station_pending);
+
+	station_build_diagnostic_dict(station, reply, info);
+
+done:
+	dbus_pending_reply(&station->get_station_pending, reply);
+}
+
+static void station_get_diagnostic_destroy(void *user_data)
+{
+	struct station *station = user_data;
+	struct l_dbus_message *reply;
+
+	if (station->get_station_pending) {
+		reply = l_dbus_message_new_method_return(
+						station->get_station_pending);
+		dbus_pending_reply(&station->get_station_pending, reply);
+	}
+}
+
+static struct l_dbus_message *station_get_diagnostics(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct station *station = user_data;
+
+	/*
+	 * At this time all values depend on a connected state.
+	 */
+	if (station->state != STATION_STATE_CONNECTED)
+		return dbus_error_not_connected(message);
+
+	if (netdev_get_station(station->netdev, station->connected_bss->addr,
+				station_get_diagnostic_cb, station,
+				station_get_diagnostic_destroy) < 0)
+		return dbus_error_busy(message);
+
+	station->get_station_pending = l_dbus_message_ref(message);
+
+	return NULL;
+}
+
+static void station_setup_diagnostic_interface(
+					struct l_dbus_interface *interface)
+{
+	l_dbus_interface_method(interface, "GetDiagnostics", 0,
+				station_get_diagnostics, "a{sv}", "",
+				"diagnostics");
+}
+
+static void station_destroy_diagnostic_interface(void *user_data)
+{
+}
+
 static void station_netdev_watch(struct netdev *netdev,
 				enum netdev_watch_event event, void *userdata)
 {
@@ -3483,6 +3592,11 @@ static int station_init(void)
 	l_dbus_register_interface(dbus_get_bus(), IWD_STATION_INTERFACE,
 					station_setup_interface,
 					station_destroy_interface, false);
+	l_dbus_register_interface(dbus_get_bus(),
+					IWD_STATION_DIAGNOSTIC_INTERFACE,
+					station_setup_diagnostic_interface,
+					station_destroy_diagnostic_interface,
+					false);
 
 	if (!l_settings_get_uint(iwd_get_config(), "General",
 					"ManagementFrameProtection",
@@ -3521,6 +3635,8 @@ static int station_init(void)
 
 static void station_exit(void)
 {
+	l_dbus_unregister_interface(dbus_get_bus(),
+					IWD_STATION_DIAGNOSTIC_INTERFACE);
 	l_dbus_unregister_interface(dbus_get_bus(), IWD_STATION_INTERFACE);
 	netdev_watch_remove(netdev_watch);
 	l_queue_destroy(station_list, NULL);
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/8] test: add a script for GetDiagnostics
  2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
                   ` (5 preceding siblings ...)
  2021-01-11 17:12 ` [PATCH 7/8] station: create StationDiagnostic interface James Prestwood
@ 2021-01-11 17:12 ` James Prestwood
  2021-01-11 20:51 ` [PATCH 1/8] doc: diagnostic DBus interface definition Denis Kenzior
  7 siblings, 0 replies; 14+ messages in thread
From: James Prestwood @ 2021-01-11 17:12 UTC (permalink / raw)
  To: iwd

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

---
 test/get-diagnostics | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100755 test/get-diagnostics

diff --git a/test/get-diagnostics b/test/get-diagnostics
new file mode 100755
index 00000000..c508c58c
--- /dev/null
+++ b/test/get-diagnostics
@@ -0,0 +1,37 @@
+#!/usr/bin/python3
+
+import sys
+import dbus
+
+# Map dict keys to units. Units can be a function/lambda which should return the
+# entire value with units as a string.
+unit_map = {
+    "ConnectedBss" : None,
+    "RSSI" : "dBm",
+    "RxBitrate" : lambda k : str(100 * int(k)) + ' Kbps',
+    "RxMCS" : lambda i : str(int(i)),
+    "TxBitrate" : lambda k : str(100 * int(k)) + ' Kbps',
+    "TxMCS" : lambda i : str(int(i)),
+    "ExpectedThroughput" : "Kbps"
+}
+
+if (len(sys.argv) != 2):
+    print("Usage: %s <device>" % (sys.argv[0]))
+    sys.exit(1)
+
+bus = dbus.SystemBus()
+device = dbus.Interface(bus.get_object("net.connman.iwd", sys.argv[1]),
+                                    "net.connman.iwd.StationDiagnostic")
+diagnostics = device.GetDiagnostics()
+
+for key, value in diagnostics.items():
+    if key in unit_map:
+        units = unit_map[key]
+        if units is None:
+            units = ''
+        elif callable(units):
+            value = units(value)
+            units = ''
+
+        print('%s : %s %s' % (key, value, units))
+
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/8] netdev: add netdev_get_station
  2021-01-11 17:12 ` [PATCH 2/8] netdev: add netdev_get_station James Prestwood
@ 2021-01-11 20:49   ` Denis Kenzior
  2021-01-11 20:54     ` James Prestwood
  0 siblings, 1 reply; 14+ messages in thread
From: Denis Kenzior @ 2021-01-11 20:49 UTC (permalink / raw)
  To: iwd

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

Hi James,

On 1/11/21 11:12 AM, James Prestwood wrote:
> This adds a generalized API for GET_STATION. This API handles
> calling and parsing the results into a new structure,
> netdev_station_info. This results structure will hold any
> data needed by consumers of netdev_get_station.
> 
> For now only the RSSI is parsed as this is already being
> done for RSSI polling/events. Looking further more info will
> be added such as rx/tx rates and estimated throughput.
> ---
>   src/netdev.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/netdev.h |  12 ++++++
>   2 files changed, 115 insertions(+)
> 

<snip>

> +int netdev_get_station(struct netdev *netdev, const uint8_t *mac,
> +			netdev_get_station_cb_t cb, void *user_data,
> +			netdev_destroy_func_t destroy)
> +{
> +	struct l_genl_msg *msg;
> +
> +	if (netdev->get_station_cmd_id)
> +		return -EBUSY;
> +
> +	msg = l_genl_msg_new_sized(NL80211_CMD_GET_STATION, 64);
> +	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
> +	l_genl_msg_append_attr(msg, NL80211_ATTR_MAC, ETH_ALEN, mac);
> +
> +	netdev->get_station_cmd_id = l_genl_family_send(nl80211, msg,
> +						netdev_get_station_cb, netdev,
> +						netdev_get_station_destroy);
> +	if (!netdev->get_station_cmd_id)
> +		return -EIO;

I think you're leaking msg here

> +
> +	netdev->get_station_cb = cb;
> +	netdev->get_station_data = user_data;
> +	netdev->get_station_destroy = destroy;
> +
> +	return 0;
> +}
> +
>   static int netdev_cqm_rssi_update(struct netdev *netdev)
>   {
>   	struct l_genl_msg *msg;
> diff --git a/src/netdev.h b/src/netdev.h
> index 65fdbaaf..5cf38076 100644
> --- a/src/netdev.h
> +++ b/src/netdev.h
> @@ -114,6 +114,14 @@ typedef void (*netdev_station_watch_func_t)(struct netdev *netdev,
>   					const uint8_t *mac, bool added,
>   					void *user_data);
>   
> +struct netdev_station_info {
> +	int8_t cur_rssi;
> +};
> +
> +typedef void (*netdev_get_station_cb_t)(struct netdev *netdev,
> +					struct netdev_station_info *info,

const struct netdev_...?

> +					void *user_data);
> +
>   struct wiphy *netdev_get_wiphy(struct netdev *netdev);
>   const uint8_t *netdev_get_address(struct netdev *netdev);
>   uint32_t netdev_get_ifindex(struct netdev *netdev);
> @@ -174,6 +182,10 @@ int netdev_neighbor_report_req(struct netdev *netdev,
>   int netdev_set_rssi_report_levels(struct netdev *netdev, const int8_t *levels,
>   					size_t levels_num);
>   
> +int netdev_get_station(struct netdev *netdev, const uint8_t *mac,

Do you want to add a convenience method to grab the current station?  The mac 
parameter is for cases where you might have multiple ones, like AP.

> +			netdev_get_station_cb_t cb, void *user_data,
> +			netdev_destroy_func_t destroy);
> +
>   void netdev_handshake_failed(struct handshake_state *hs, uint16_t reason_code);
>   
>   struct netdev *netdev_find(int ifindex);
> 

Regards,
-Denis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/8] doc: diagnostic DBus interface definition
  2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
                   ` (6 preceding siblings ...)
  2021-01-11 17:12 ` [PATCH 8/8] test: add a script for GetDiagnostics James Prestwood
@ 2021-01-11 20:51 ` Denis Kenzior
  7 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2021-01-11 20:51 UTC (permalink / raw)
  To: iwd

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

Hi James,

On 1/11/21 11:12 AM, James Prestwood wrote:
> ---
>   doc/diagnostics.txt | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>   create mode 100644 doc/diagnostics.txt
> 

I added an [experimental] tag to this and applied, thanks.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/8] dbus: add diagnostic interface definition
  2021-01-11 17:12 ` [PATCH 5/8] dbus: add diagnostic interface definition James Prestwood
@ 2021-01-11 20:52   ` Denis Kenzior
  0 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2021-01-11 20:52 UTC (permalink / raw)
  To: iwd

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

Hi James,

On 1/11/21 11:12 AM, James Prestwood wrote:
> ---
>   src/dbus.h | 1 +
>   1 file changed, 1 insertion(+)
> 

Applied, thanks.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/8] netdev: add netdev_get_station
  2021-01-11 20:49   ` Denis Kenzior
@ 2021-01-11 20:54     ` James Prestwood
  2021-01-11 21:08       ` Denis Kenzior
  0 siblings, 1 reply; 14+ messages in thread
From: James Prestwood @ 2021-01-11 20:54 UTC (permalink / raw)
  To: iwd

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

Hi Denis,

> >   struct wiphy *netdev_get_wiphy(struct netdev *netdev);
> >   const uint8_t *netdev_get_address(struct netdev *netdev);
> >   uint32_t netdev_get_ifindex(struct netdev *netdev);
> > @@ -174,6 +182,10 @@ int netdev_neighbor_report_req(struct netdev
> > *netdev,
> >   int netdev_set_rssi_report_levels(struct netdev *netdev, const
> > int8_t *levels,
> >   					size_t levels_num);
> >   
> > +int netdev_get_station(struct netdev *netdev, const uint8_t *mac,
> 
> Do you want to add a convenience method to grab the current
> station?  The mac 
> parameter is for cases where you might have multiple ones, like AP.

I could, although its not too bad just passing in handshake->aa... But
if you want a current station API and a separate one for AP I don't
mind either way.

> 
> > +			netdev_get_station_cb_t cb, void *user_data,
> > +			netdev_destroy_func_t destroy);
> > +
> >   void netdev_handshake_failed(struct handshake_state *hs, uint16_t
> > reason_code);
> >   
> >   struct netdev *netdev_find(int ifindex);
> > 
> 
> Regards,
> -Denis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 7/8] station: create StationDiagnostic interface
  2021-01-11 17:12 ` [PATCH 7/8] station: create StationDiagnostic interface James Prestwood
@ 2021-01-11 21:04   ` Denis Kenzior
  0 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2021-01-11 21:04 UTC (permalink / raw)
  To: iwd

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

Hi James,

On 1/11/21 11:12 AM, James Prestwood wrote:
> This interface sits aside the regular station interface but
> provides low level connection details for diagnostic and
> testing purposes.
> ---
>   src/station.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 116 insertions(+)
> 

<snip>

> @@ -3455,6 +3459,111 @@ static void station_destroy_interface(void *user_data)
>   	station_free(station);
>   }
>   
> +/*
> + * Helper to append a dictionary value. This will only work for basic types.
> + */
> +static void append_dict(struct l_dbus_message_builder *builder,
> +			const char *name, const char *type, const void *data)
> +{
> +	l_dbus_message_builder_enter_dict(builder, "sv");
> +	l_dbus_message_builder_append_basic(builder, 's', name);
> +	l_dbus_message_builder_enter_variant(builder, type);
> +	l_dbus_message_builder_append_basic(builder, type[0], data);
> +	l_dbus_message_builder_leave_variant(builder);
> +	l_dbus_message_builder_leave_dict(builder);
> +}

Maybe this belongs in src/dbus.[ch] somewhere.

<snip>

> +static void station_get_diagnostic_cb(struct netdev *netdev,
> +					struct netdev_station_info *info,
> +					void *user_data)
> +{
> +	struct station *station = user_data;
> +	struct l_dbus_message *reply;
> +
> +	if (!info) {
> +		reply = dbus_error_aborted(station->get_station_pending);
> +		goto done;
> +	}
> +
> +	reply = l_dbus_message_new_method_return(station->get_station_pending);
> +
> +	station_build_diagnostic_dict(station, reply, info);

Not sure why you break this into a separate step?

> +
> +done:
> +	dbus_pending_reply(&station->get_station_pending, reply);
> +}
> +
> +static void station_get_diagnostic_destroy(void *user_data)
> +{
> +	struct station *station = user_data;
> +	struct l_dbus_message *reply;
> +
> +	if (station->get_station_pending) {
> +		reply = l_dbus_message_new_method_return(
> +						station->get_station_pending);

Hmm, an empty message?  Maybe use dbus_error_aborted instead?  See how device.c 
does this for example.

> +		dbus_pending_reply(&station->get_station_pending, reply);
> +	}
> +}
> +
> +static struct l_dbus_message *station_get_diagnostics(struct l_dbus *dbus,
> +						struct l_dbus_message *message,
> +						void *user_data)
> +{
> +	struct station *station = user_data;
> +
> +	/*
> +	 * At this time all values depend on a connected state.
> +	 */
> +	if (station->state != STATION_STATE_CONNECTED)
> +		return dbus_error_not_connected(message);
> +
> +	if (netdev_get_station(station->netdev, station->connected_bss->addr,
> +				station_get_diagnostic_cb, station,
> +				station_get_diagnostic_destroy) < 0)
> +		return dbus_error_busy(message);

Maybe use dbus_error_from_errno instead?

> +
> +	station->get_station_pending = l_dbus_message_ref(message);
> +
> +	return NULL;
> +}
> +

<snip>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/8] netdev: add netdev_get_station
  2021-01-11 20:54     ` James Prestwood
@ 2021-01-11 21:08       ` Denis Kenzior
  0 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2021-01-11 21:08 UTC (permalink / raw)
  To: iwd

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

Hi James,

>>> +int netdev_get_station(struct netdev *netdev, const uint8_t *mac,
>>
>> Do you want to add a convenience method to grab the current
>> station?  The mac
>> parameter is for cases where you might have multiple ones, like AP.
> 
> I could, although its not too bad just passing in handshake->aa... But
> if you want a current station API and a separate one for AP I don't
> mind either way.
> 

I was just thinking that you probably can keep this method and just add a 
wrapper (netdev_get_current_station or something) that would just call 
netdev_get_station appropriately.  But I'm fine either way.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-01-11 21:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-11 17:12 [PATCH 1/8] doc: diagnostic DBus interface definition James Prestwood
2021-01-11 17:12 ` [PATCH 2/8] netdev: add netdev_get_station James Prestwood
2021-01-11 20:49   ` Denis Kenzior
2021-01-11 20:54     ` James Prestwood
2021-01-11 21:08       ` Denis Kenzior
2021-01-11 17:12 ` [PATCH 3/8] netdev: update RSSI polling to use netdev_get_station James Prestwood
2021-01-11 17:12 ` [PATCH 4/8] netdev: parse rates in netdev_get_station James Prestwood
2021-01-11 17:12 ` [PATCH 5/8] dbus: add diagnostic interface definition James Prestwood
2021-01-11 20:52   ` Denis Kenzior
2021-01-11 17:12 ` [PATCH 6/8] netdev: parse expected throughput in netdev_get_station James Prestwood
2021-01-11 17:12 ` [PATCH 7/8] station: create StationDiagnostic interface James Prestwood
2021-01-11 21:04   ` Denis Kenzior
2021-01-11 17:12 ` [PATCH 8/8] test: add a script for GetDiagnostics James Prestwood
2021-01-11 20:51 ` [PATCH 1/8] doc: diagnostic DBus interface definition Denis Kenzior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.