iwinfo: add basic IEEE 802.11be support

[cmonroe]

Add basic support for 802.11be / WiFi 7 to iwinfo.

[jow-]

This change breaks the ABI which I'd ideally like to avoid. If I am not mistaken, there should be two bytes of padding at the end of the struct, so maybe place the high mhz bits after the mss field. Also put the is_eht bitfield member directly after is_he to ensure that it ends up in the same byte.

Updated structure layout could look like:

struct iwinfo_rate_entry {
	uint32_t rate;
	int8_t mcs;
	uint8_t is_40mhz:1;
	uint8_t is_short_gi:1;
	uint8_t is_ht:1;
	uint8_t is_vht:1;
	uint8_t is_he:1;
	uint8_t is_eht:1;
	uint8_t he_gi;
	uint8_t he_dcm;
	uint8_t mhz_lo;
	uint8_t nss;
        uint8_t mhz_hi;
        uint8_t eht_gi;    
};

[jow-]

Assuming the updated structure, this will become:
p += snprintf(p, l, ", EHT-MCS %d, %dMHz", r->mcs, r->mhz_hi * 256 + r->mhz);

[jow-]

Please use uint8_t here

[jow-]

Remove this no-op statement

[jow-]

Remove this no-op statement

[jow-]

Remove this no-op statement

[jow-]

Assuming the updated structure layout, this should become

re->mhz_hi = 320 / 256, re->mhz = 320 % 256;

or

re->mhz_hi = 1, re->mhz = 64;

[jow-]

Assuming the updated structure layout, this should become
lua_pushnumber(L, r->mhz_hi * 256 + r->mhz);

[cmonroe]

Hi @jow-

Thanks for looking this over! I think I got all of your requested changes in the most recent push if you don't mind checking them. I tested that 20, 80, 160 and 320MHz bandwidths are still reported correctly via the CLI and ubus. In order for assoclist to display 320MHz correctly via ubus I've updated rpcd #5 as well.

Two questions, mostly for my own understanding if you have the time to share:

1. Is adding the is_eht flag in the middle of iwinfo_rate_entry OK because all of the defined bitfields end up in the same byte?
2. Does adding a new element not require an ABI change? The rpcd PR mentioned above will make rpcd reliant upon presence of mhz_hi and I wanted to make sure this is OK.

[jow-]

Hi,

Is adding the is_eht flag in the middle of iwinfo_rate_entry OK because all of the defined bitfields end up in the same byte?

Yes. The is_eht field is a single-bit bitfield member (due to the :1 suffix). Adjacent bitfield struct members with the same base type are packed into the same value, so a

struct {
  uint8_t foo:1;
  uint8_t bar:1;
  uint8_t baz:1;
}

... ends up being a struct with one byte holding three bits, while in contrast a

struct {
  uint8_t foo:1;
  uint8_t bar:1;
  bool something_else;
  uint8_t baz:1;
}

... ends up being three bytes large (first a byte holding two bits, then a byte holding a boolean 1 or 0 value, then another byte holding one bit).

Does adding a new element not require an ABI change?

It depends, in this particular case the structure size and layout did not change, so existing code built against an older libiwinfo loading a newer one will continue to work. It might not be able to make sense of the new values (e.g. wrongly report "64 MHz" for a 320 wide channel) but it will not crash trying to access nonexistent/invalid memory when the structures returned by iwinfo are larger than what the program expects.

The reason why we need to be somewhat careful with the ABI here is that some programs (e.g. LuCI) dlopen() libiwinfo at runtime, so the usual ABI handling mechanics in packages do not apply.

Here's my local testcase for the structure size:

@j7:~$ cat /tmp/structsize.c 
#include <stdint.h>
#include <stdio.h>

struct iwinfo_rate_entry_old {
	uint32_t rate;
	int8_t mcs;
	uint8_t is_40mhz:1;
	uint8_t is_short_gi:1;
	uint8_t is_ht:1;
	uint8_t is_vht:1;
	uint8_t is_he:1;
	uint8_t he_gi;
	uint8_t he_dcm;
	uint8_t mhz;
	uint8_t nss;
};

struct iwinfo_rate_entry_new {
	uint32_t rate;
	int8_t mcs;
	uint8_t is_40mhz:1;
	uint8_t is_short_gi:1;
	uint8_t is_ht:1;
	uint8_t is_vht:1;
	uint8_t is_he:1;
	uint8_t is_eht:1;
	uint8_t he_gi;
	uint8_t he_dcm;
	uint8_t mhz_lo;
	uint8_t nss;
        uint8_t mhz_hi;
        uint8_t eht_gi;
};


int main(int argc, char **argv) {
	printf("%zd, %zd\n",
		sizeof(struct iwinfo_rate_entry_old),
		sizeof(struct iwinfo_rate_entry_new));

	return 0;
}
jow@j7:~$ gcc -o /tmp/structsize /tmp/structsize.c && /tmp/structsize
12, 12
jow@j7:~$

The reason why it was possible in this case is that the original struct had two unused alignment padding bytes at the end.

[cmonroe]

Thanks for the super clear info, it is very helpful :)

The ABI change part makes a lot of sense now.. I was thinking about the problem backwards before your explanation.

Please let me know if any other changes are needed and thanks again for your help!

[rmandrad]

Successfully tested on Banana Pi4

[rmandrad]

@dangowrt is anyone monitoring the openwrt/rpcd repo ? @cmonroe has another pull related with 11be support but it doesn't have any feedback/approvers

[danpawlik]

LGTM.

Tested-By: Daniel Pawlik pawlik.dan@gmail.com

[danpawlik]

any chance to merge that @jow- ? I see it has been already approved by Daniel.

[systemcrash]

@cmonroe Can you also take care of the 240(/160+80) MHz case?

[cmonroe]

Daniel merged the changed from this PR via 4b7c47c and 268a662

[Neustradamus]

@cmonroe: Thanks :)

[Neustradamus]

Dear all, @cmonroe, @jow-, @rmandrad, @danpawlik, @systemcrash, @dangowrt,

There is a new PR linked to 11be done by @prokowsoftware here:

• luci-mod-network: add WiFi 7 (802.11be / EHT) advanced configuration options luci#8461

Can you look?

Thanks in advance.