[OpenWrt-Devel] [PATCH] iwinfo: add channel survey

Jo-Philipp Wich jo at mein.io
Tue Jun 5 04:59:34 EDT 2018


Hi,

please find code comments inline below.

Do you plan to extend the Lua binding as well?

Also I wonder what the intended use case of this change is...
(lib)iwinfo was once meant to provide a common uniform subset of
wireless information across different driver backends such as
proprietary wl, madwifi, wext and nl80211.

Since the survey info here is only implemented for nl80211 and only
exposed in the C library interface (and the iwinfo cli) whats the
advantage compared to just using "iw" and native netlink?

Do you have some program depending on the libiwinfo C library?

Regards,
Jo

> Signed-off-by: Nick Hainke <vincent at systemli.org>
> ---
>  include/iwinfo.h | 11 +++++++++++
>  iwinfo_cli.c     | 31 ++++++++++++++++++++++++++++-
>  iwinfo_nl80211.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/include/iwinfo.h b/include/iwinfo.h
> index 929f697..004896e 100644
> --- a/include/iwinfo.h
> +++ b/include/iwinfo.h
> @@ -157,6 +157,16 @@ struct iwinfo_scanlist_entry {
>  	struct iwinfo_crypto_entry crypto;
>  };
>  
> +struct iwinfo_survey_entry {
> +	uint32_t frequency;
> +	int8_t noise;
> +	uint64_t channel_time;
> +	uint64_t channel_time_busy;
> +	uint64_t channel_time_ext_busy;
> +	uint64_t channel_time_rx;
> +	uint64_t channel_time_tx;
> +};
> +
>  struct iwinfo_country_entry {
>  	uint16_t iso3166;
>  	char ccode[4];
> @@ -203,6 +213,7 @@ struct iwinfo_ops {
>  	int (*bitrate)(const char *, int *);
>  	int (*signal)(const char *, int *);
>  	int (*noise)(const char *, int *);
> +	int (*survey)(const char *ifname, struct iwinfo_survey_entry *entry);

Please remove the parameter names from the function prototype.

>  	int (*quality)(const char *, int *);
>  	int (*quality_max)(const char *, int *);
>  	int (*mbssid_support)(const char *, int *);
> diff --git a/iwinfo_cli.c b/iwinfo_cli.c
> index 49c9035..3d8b82c 100644
> --- a/iwinfo_cli.c
> +++ b/iwinfo_cli.c
> @@ -116,6 +116,18 @@ static char * format_signal(int sig)
>  	return buf;
>  }
>  
> +static char * format_channel_time(uint64_t time)
> +{
> +	static char buf[30];
> +
> +	if (!time)
> +		snprintf(buf, sizeof(buf), "unknown");
> +	else
> +		snprintf(buf, sizeof(buf), "%llu ms", time);
> +
> +	return buf;
> +}
> +
>  static char * format_noise(int noise)
>  {
>  	static char buf[10];
> @@ -531,6 +543,19 @@ static char * print_phyname(const struct iwinfo_ops *iw, const char *ifname)
>  	return "?";
>  }
>  
> +static void print_survey(const struct iwinfo_ops *iw, const char *ifname)
> +{
> +	struct iwinfo_survey_entry entry;
> +	iw->survey(ifname, &entry);

You need to check iw->survey != NULL, otherwise iwinfo will segfault
with non-nl80211 driver backends.

> +	printf("%s\tESSID:\t\t\t\t%s\n", ifname, print_ssid(iw, ifname));
> +	printf("\tChannel:\t\t\t%s (%s)\n", print_channel(iw, ifname), format_frequency(entry.frequency));
> +	printf("\tNoise:\t\t\t\t%s\n", format_noise(entry.noise));
> +	printf("\tchannel Active Time:\t\t%s\n", format_channel_time(entry.channel_time));
> +	printf("\tChannel Busy Time:\t\t%s\n",format_channel_time(entry.channel_time_busy));
> +	printf("\tExtension Channel Busy Time:\t%s\n",format_channel_time(entry.channel_time_ext_busy));
> +	printf("\tChannel Receive Time:\t\t%s\n",format_channel_time(entry.channel_time_rx));
> +	printf("\tChannel Transmit Time:\t\t%s\n",format_channel_time(entry.channel_time_tx));
> +}
>  
>  static void print_info(const struct iwinfo_ops *iw, const char *ifname)
>  {
> @@ -805,6 +830,7 @@ int main(int argc, char **argv)
>  			"Usage:\n"
>  			"	iwinfo <device> info\n"
>  			"	iwinfo <device> scan\n"
> +			"	iwinfo <device> survey\n"
>  			"	iwinfo <device> txpowerlist\n"
>  			"	iwinfo <device> freqlist\n"
>  			"	iwinfo <device> assoclist\n"
> @@ -883,7 +909,10 @@ int main(int argc, char **argv)
>  					break;
>  
>  				case 's':
> -					print_scanlist(iw, argv[1]);
> +					if(argv[i][1] == 'c')
> +						print_scanlist(iw, argv[1]);
> +					else
> +						print_survey(iw, argv[1]);
>  					break;
>  
>  				case 't':
> diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c
> index ecd2d6a..2c741a3 100644
> --- a/iwinfo_nl80211.c
> +++ b/iwinfo_nl80211.c
> @@ -1357,6 +1357,63 @@ static int nl80211_get_signal(const char *ifname, int *buf)
>  	return -1;
>  }
>  
> +static int nl80211_get_channel_survey_cb(struct nl_msg *msg, void *arg)
> +{
> +	struct iwinfo_survey_entry *entry = arg;
> +	struct nlattr *tb[NL80211_ATTR_MAX + 1];
> +	struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
> +	struct nlattr *si[NL80211_SURVEY_INFO_MAX + 1];
> +
> +	static struct nla_policy sp[NL80211_SURVEY_INFO_MAX + 1] = {
> +			[NL80211_SURVEY_INFO_FREQUENCY] = { .type = NLA_U32 },
> +			[NL80211_SURVEY_INFO_NOISE]     = { .type = NLA_U8  },
> +	};
> +
> +	nla_parse(tb, NL80211_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
> +			  genlmsg_attrlen(gnlh, 0), NULL);
> +
> +	if (!tb[NL80211_ATTR_SURVEY_INFO])
> +		return NL_SKIP;
> +
> +	if (nla_parse_nested(si, NL80211_SURVEY_INFO_MAX,
> +						 tb[NL80211_ATTR_SURVEY_INFO], sp))
> +		return NL_SKIP;
> +
> +	if (si[NL80211_SURVEY_INFO_IN_USE])
> +	{
> +		si[NL80211_SURVEY_INFO_FREQUENCY] ?
> +				entry->frequency = (uint32_t)nla_get_u32(si[NL80211_SURVEY_INFO_FREQUENCY]) : 0;

Why the extra casts here? Are they needed? Also please use

  if (si[...])
      entry->... = nla_get_...();

instead of ternary expressions.

> +
> +		si[NL80211_SURVEY_INFO_NOISE] ?
> +				entry->noise = (int8_t)nla_get_u8(si[NL80211_SURVEY_INFO_NOISE]) : 0;
> +
> +		si[NL80211_SURVEY_INFO_CHANNEL_TIME] ?
> +			entry->channel_time = (uint64_t)nla_get_u64(si[NL80211_SURVEY_INFO_CHANNEL_TIME]) : 0;
> +
> +		si[NL80211_SURVEY_INFO_CHANNEL_TIME_BUSY] ?
> +			entry->channel_time_busy = (uint64_t)nla_get_u64(si[NL80211_SURVEY_INFO_CHANNEL_TIME_BUSY]) : 0;
> +
> +		si[NL80211_SURVEY_INFO_TIME_EXT_BUSY] ?
> +				entry->channel_time_ext_busy = (uint64_t)nla_get_u64(si[NL80211_SURVEY_INFO_TIME_EXT_BUSY]) : 0;
> +
> +		si[NL80211_SURVEY_INFO_TIME_RX] ?
> +				entry->channel_time_rx = (uint64_t)nla_get_u64(si[NL80211_SURVEY_INFO_TIME_RX]) : 0;
> +
> +		si[NL80211_SURVEY_INFO_TIME_TX] ?
> +				entry->channel_time_tx = (uint64_t)nla_get_u64(si[NL80211_SURVEY_INFO_TIME_TX]) : 0;
> +	}
> +
> +	return NL_SKIP;
> +}
> +
> +static int nl80211_get_channel_survey(const char *ifname, struct iwinfo_survey_entry *entry)
> +{
> +	if (nl80211_request(ifname, NL80211_CMD_GET_SURVEY, NLM_F_DUMP,
> +						nl80211_get_channel_survey_cb, entry))
> +		return -1;

Please add a blank line here.

> +	return 0;
> +}
> +
>  static int nl80211_get_noise_cb(struct nl_msg *msg, void *arg)
>  {
>  	int8_t *noise = arg;
> @@ -1384,7 +1441,6 @@ static int nl80211_get_noise_cb(struct nl_msg *msg, void *arg)
>  	return NL_SKIP;
>  }
>  
> -
>  static int nl80211_get_noise(const char *ifname, int *buf)
>  {
>  	int8_t noise = 0;
> @@ -2832,6 +2888,7 @@ const struct iwinfo_ops nl80211_ops = {
>  	.bitrate          = nl80211_get_bitrate,
>  	.signal           = nl80211_get_signal,
>  	.noise            = nl80211_get_noise,
> +	.survey           = nl80211_get_channel_survey,
>  	.quality          = nl80211_get_quality,
>  	.quality_max      = nl80211_get_quality_max,
>  	.mbssid_support   = nl80211_get_mbssid_support,
> 


_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/listinfo/openwrt-devel



More information about the openwrt-devel mailing list