Re: [OpenWrt-Devel] [PATCH 1/2] add htmodelist for scan results

2018-07-15 Thread Jo-Philipp Wich
Hi,

comments inline.

On 07/13/2018 10:57 AM, Yury Shvedov wrote:

> diff --git a/include/iwinfo.h b/include/iwinfo.h
> index 929f697..c3c25ff 100644
> --- a/include/iwinfo.h
> +++ b/include/iwinfo.h
> @@ -154,6 +154,7 @@ struct iwinfo_scanlist_entry {
>   uint8_t signal;
>   uint8_t quality;
>   uint8_t quality_max;
> +int htmodelist;

Do we really need a full int here? I think an u8 or u16 should suffice.

>   struct iwinfo_crypto_entry crypto;
>  };
>  
> diff --git a/iwinfo_cli.c b/iwinfo_cli.c
> index 49c9035..2d58020 100644
> --- a/iwinfo_cli.c
> +++ b/iwinfo_cli.c
> @@ -572,7 +572,7 @@ static void print_info(const struct iwinfo_ops *iw, const 
> char *ifname)
>  
>  static void print_scanlist(const struct iwinfo_ops *iw, const char *ifname)
>  {
> - int i, x, len;
> + int i, h, x, len;
>   char buf[IWINFO_BUFSIZE];
>   struct iwinfo_scanlist_entry *e;
>  
> @@ -603,8 +603,17 @@ static void print_scanlist(const struct iwinfo_ops *iw, 
> const char *ifname)
>   format_signal(e->signal - 0x100),
>   format_quality(e->quality),
>   format_quality_max(e->quality_max));
> - printf("  Encryption: %s\n\n",
> + printf("  Encryption: %s\n",
>   format_encryption(>crypto));
> + if (e->htmodelist)
> + {
> + printf("  HT Capabilities: ");
> + for (h = 0; h < ARRAY_SIZE(IWINFO_HTMODE_NAMES); h++)
> + if (e->htmodelist & (1 << h))
> + printf("%s ", IWINFO_HTMODE_NAMES[h]);
> + printf ("\n");
> + }
> + printf ("\n");

Please adhere to the coding style and remove the space between "printf"
and the opening paren.
>   }
>  }
>  
> diff --git a/iwinfo_lua.c b/iwinfo_lua.c
> index eebab8e..01581a3 100644
> --- a/iwinfo_lua.c
> +++ b/iwinfo_lua.c
> @@ -378,7 +378,7 @@ static int iwinfo_L_txpwrlist(lua_State *L, int 
> (*func)(const char *, char *, in
>  /* Wrapper for scan list */
>  static int iwinfo_L_scanlist(lua_State *L, int (*func)(const char *, char *, 
> int *))
>  {
> - int i, x, len = 0;
> + int i, x, h, len = 0;
>   char rv[IWINFO_BUFSIZE];
>   char macstr[18];
>   const char *ifname = luaL_checkstring(L, 1);
> @@ -432,6 +432,20 @@ static int iwinfo_L_scanlist(lua_State *L, int 
> (*func)(const char *, char *, int
>   iwinfo_L_cryptotable(L, >crypto);
>   lua_setfield(L, -2, "encryption");
>  
> + /* HT Modes */
> + if (e->htmodelist)
> + {
> + lua_newtable(L);
> + for (h = 0; h < 
> ARRAY_SIZE(IWINFO_HTMODE_NAMES); h++)
> + {
> + lua_pushboolean(L, e->htmodelist & (1 
> << h));
> + lua_setfield(L, -2, 
> IWINFO_HTMODE_NAMES[h]);
> + }
> + }
> + else
> + lua_pushnil(L);
> + lua_setfield(L, -2, "htmodelist");

Please move the lua_setfield() into the if-condition and drop the else case.

> +
>   lua_rawseti(L, -2, x);
>   }
>   }
> diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c
> index ecd2d6a..542ac7d 100644
> --- a/iwinfo_nl80211.c
> +++ b/iwinfo_nl80211.c
> @@ -1980,6 +1980,71 @@ struct nl80211_scanlist {
>   int len;
>  };
>  
> +static void nl80211_parse_ht_capa(struct iwinfo_scanlist_entry *e,
> +   unsigned char 
> *ie,
> +   int len)
> +{
> + int capa;
> + if (len < 2)
> + return;
> + e->htmodelist |= IWINFO_HTMODE_HT20;
> + capa = ie[0] | (ie[8] << 8);
> + if (capa & (1 << 1))
> + {
> + e->htmodelist |= IWINFO_HTMODE_HT40;
> + if (e->htmodelist & IWINFO_HTMODE_VHT20)
> + e->htmodelist |= IWINFO_HTMODE_VHT40;
> + }
> +}
> +static void nl80211_parse_vht_capa(struct iwinfo_scanlist_entry *e,
> +unsigned 
> char *ie,
> +int len)
> +{
> + int capa;
> + if (len < 4)
> + return;
> + capa = ie[0] | (ie[1] << 8) | (ie[2] << 16) | (ie[3] << 24);
> +
> + e->htmodelist |= IWINFO_HTMODE_VHT20;
> + if (e->htmodelist & IWINFO_HTMODE_HT40)
> + e->htmodelist |= IWINFO_HTMODE_VHT40;
> +
> + switch ((capa >> 2) & 3) {
> + /*case 0: neither 160 nor 80+80 */
> + case 1:
> + e->htmodelist |= IWINFO_HTMODE_VHT160;
> + break;
> + case 2:
> + 

[OpenWrt-Devel] [PATCH 1/2] add htmodelist for scan results

2018-07-13 Thread Yury Shvedov
The knowledge about HT and VHT modes could be useful for user experience.
So grab it via nl80211 and make it available in both C and LUA APIs,
and show htmodelist on CLI scan results.
---
 include/iwinfo.h |  1 +
 iwinfo_cli.c | 13 ++--
 iwinfo_lua.c | 16 +-
 iwinfo_nl80211.c | 77 
 4 files changed, 104 insertions(+), 3 deletions(-)

diff --git a/include/iwinfo.h b/include/iwinfo.h
index 929f697..c3c25ff 100644
--- a/include/iwinfo.h
+++ b/include/iwinfo.h
@@ -154,6 +154,7 @@ struct iwinfo_scanlist_entry {
uint8_t signal;
uint8_t quality;
uint8_t quality_max;
+int htmodelist;
struct iwinfo_crypto_entry crypto;
 };
 
diff --git a/iwinfo_cli.c b/iwinfo_cli.c
index 49c9035..2d58020 100644
--- a/iwinfo_cli.c
+++ b/iwinfo_cli.c
@@ -572,7 +572,7 @@ static void print_info(const struct iwinfo_ops *iw, const 
char *ifname)
 
 static void print_scanlist(const struct iwinfo_ops *iw, const char *ifname)
 {
-   int i, x, len;
+   int i, h, x, len;
char buf[IWINFO_BUFSIZE];
struct iwinfo_scanlist_entry *e;
 
@@ -603,8 +603,17 @@ static void print_scanlist(const struct iwinfo_ops *iw, 
const char *ifname)
format_signal(e->signal - 0x100),
format_quality(e->quality),
format_quality_max(e->quality_max));
-   printf("  Encryption: %s\n\n",
+   printf("  Encryption: %s\n",
format_encryption(>crypto));
+   if (e->htmodelist)
+   {
+   printf("  HT Capabilities: ");
+   for (h = 0; h < ARRAY_SIZE(IWINFO_HTMODE_NAMES); h++)
+   if (e->htmodelist & (1 << h))
+   printf("%s ", IWINFO_HTMODE_NAMES[h]);
+   printf ("\n");
+   }
+   printf ("\n");
}
 }
 
diff --git a/iwinfo_lua.c b/iwinfo_lua.c
index eebab8e..01581a3 100644
--- a/iwinfo_lua.c
+++ b/iwinfo_lua.c
@@ -378,7 +378,7 @@ static int iwinfo_L_txpwrlist(lua_State *L, int 
(*func)(const char *, char *, in
 /* Wrapper for scan list */
 static int iwinfo_L_scanlist(lua_State *L, int (*func)(const char *, char *, 
int *))
 {
-   int i, x, len = 0;
+   int i, x, h, len = 0;
char rv[IWINFO_BUFSIZE];
char macstr[18];
const char *ifname = luaL_checkstring(L, 1);
@@ -432,6 +432,20 @@ static int iwinfo_L_scanlist(lua_State *L, int 
(*func)(const char *, char *, int
iwinfo_L_cryptotable(L, >crypto);
lua_setfield(L, -2, "encryption");
 
+   /* HT Modes */
+   if (e->htmodelist)
+   {
+   lua_newtable(L);
+   for (h = 0; h < 
ARRAY_SIZE(IWINFO_HTMODE_NAMES); h++)
+   {
+   lua_pushboolean(L, e->htmodelist & (1 
<< h));
+   lua_setfield(L, -2, 
IWINFO_HTMODE_NAMES[h]);
+   }
+   }
+   else
+   lua_pushnil(L);
+   lua_setfield(L, -2, "htmodelist");
+
lua_rawseti(L, -2, x);
}
}
diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c
index ecd2d6a..542ac7d 100644
--- a/iwinfo_nl80211.c
+++ b/iwinfo_nl80211.c
@@ -1980,6 +1980,71 @@ struct nl80211_scanlist {
int len;
 };
 
+static void nl80211_parse_ht_capa(struct iwinfo_scanlist_entry *e,
+ unsigned char 
*ie,
+ int len)
+{
+   int capa;
+   if (len < 2)
+   return;
+   e->htmodelist |= IWINFO_HTMODE_HT20;
+   capa = ie[0] | (ie[8] << 8);
+   if (capa & (1 << 1))
+   {
+   e->htmodelist |= IWINFO_HTMODE_HT40;
+   if (e->htmodelist & IWINFO_HTMODE_VHT20)
+   e->htmodelist |= IWINFO_HTMODE_VHT40;
+   }
+}
+static void nl80211_parse_vht_capa(struct iwinfo_scanlist_entry *e,
+  unsigned 
char *ie,
+  int len)
+{
+   int capa;
+   if (len < 4)
+   return;
+   capa = ie[0] | (ie[1] << 8) | (ie[2] << 16) | (ie[3] << 24);
+
+   e->htmodelist |= IWINFO_HTMODE_VHT20;
+   if (e->htmodelist & IWINFO_HTMODE_HT40)
+   e->htmodelist |= IWINFO_HTMODE_VHT40;
+
+   switch ((capa >> 2) & 3) {
+   /*case 0: neither 160 nor 80+80 */
+   case 1:
+   e->htmodelist |= IWINFO_HTMODE_VHT160;
+   break;
+   case 2:
+   e->htmodelist |=