* [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
* 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 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 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
* [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
* 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
* [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 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