LinuxKPI: 802.11: correct HT protection fields

[bz]

Summary

It seems during the initial buildup of the file, the defines were
either mixed or not flagged as "FIXME".
Define the values through to the net80211 definitions and also
annotate them by at least some standards reference.

MFC after: 3 days
Fixes: 6b4cac814e32f

[cc]

This patch also fixes the wrong/old values used in sys/compat/linuxkpi/common/include/linux/ieee80211.h. thanks

If I understand correctly, these defines are from the Table 8-130—HT Operation element fields and subfields (continued) on page 615. I quoted them below. I think it's better to use cleaner names for these defines in sys/net80211/ieee80211.h and use an indent similar to the Linux version. Then, later read will find them easy to understand.

Field	Definition
HT Protection	Indicates protection requirements of HT transmissions. See 9.23.3.

Encoding:

• Set to 0 for no protection mode
• Set to 1 for nonmember protection mode
• Set to 2 for 20 MHz protection mode
• Set to 3 for non-HT mixed mode

for example, changes in sys/net80211/ieee80211.h

/* bytes 2+3 */
#define	IEEE80211_HT_OPMODE		0x03	/* operating mode mask */
#define	IEEE80211_HTINFO_OPMODE_S	0               << remove this line, not used any more
#define		IEEE80211_HT_OPMODE_PROTECTION_NONE		0	/* for no protection mode */
#define		IEEE80211_HT_OPMODE_PROTECTION_NONMEMBER	1	/* for nonmember protection mode */
#define		IEEE80211_HT_OPMODE_PROTECTION_20MHZ		2	/* for 20 MHz protection mode */
#define		IEEE80211_HT_OPMODE_PROTECTION_NONHT_MIXED	3	/* for non-HT mixed mode */

[bz]

In D43658#996222, @cc wrote:

This patch also fixes the wrong/old values used in sys/compat/linuxkpi/common/include/linux/ieee80211.h. thanks

No, this patch ONLY fixes the wrong values there. The fact that they are defined to a native version is cosmetics.

If I understand correctly, these defines are from the Table 8-130—HT Operation element fields and subfields (continued) on page 615. I quoted them below. I think it's better to use cleaner names for these defines in sys/net80211/ieee80211.h and use an indent similar to the Linux version.

There's a lot to be said about the "native" net80211 version. But that's not part of this review request. I tend to not change the old stuff there if it is used (VHT is not used yet, so that's why there is cleanup going on there still). Simply MFCing it could be a pain without leaving compat defines. In this case it seems only iwn(4) would be affected so we could argue we can but I haven't checked all the other related fields and that's where the real pain comes in.

[cc]

In D43658#996657, @bz wrote:

In D43658#996222, @cc wrote:

This patch also fixes the wrong/old values used in sys/compat/linuxkpi/common/include/linux/ieee80211.h. thanks

No, this patch ONLY fixes the wrong values there. The fact that they are defined to a native version is cosmetics.

I meant this patch fixes the wrong/old values here, in the LinuxKPI. For example, IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED should have a value 3, not the old 0x02.

If I understand correctly, these defines are from the Table 8-130—HT Operation element fields and subfields (continued) on page 615. I quoted them below. I think it's better to use cleaner names for these defines in sys/net80211/ieee80211.h and use an indent similar to the Linux version.

There's a lot to be said about the "native" net80211 version. But that's not part of this review request. I tend to not change the old stuff there if it is used (VHT is not used yet, so that's why there is cleanup going on there still). Simply MFCing it could be a pain without leaving compat defines. In this case it seems only iwn(4) would be affected so we could argue we can but I haven't checked all the other related fields and that's where the real pain comes in.

I think a separate patch "just rename these defines in sys/net80211/ieee80211.h" shall be OK, and can MFC, because their corresponding values are correct. If you agree, I can submit a patch for review.

cc fbsd_src % rg -w "IEEE80211_HTINFO_OPMODE" | wc -l
       8
cc fbsd_src % rg -w "IEEE80211_HTINFO_OPMODE_PURE" | wc -l
       6
cc fbsd_src % rg -w "IEEE80211_HTINFO_OPMODE_PROTOPT" | wc -l
       7
cc fbsd_src % rg -w "IEEE80211_HTINFO_OPMODE_HT20PR" | wc -l
       5
cc fbsd_src % rg -w "IEEE80211_HTINFO_OPMODE_MIXED" | wc -l
       8

[bz]

If I understand correctly, these defines are from the Table 8-130—HT Operation element fields and subfields (continued) on page 615. I quoted them below. I think it's better to use cleaner names for these defines in sys/net80211/ieee80211.h and use an indent similar to the Linux version.

There's a lot to be said about the "native" net80211 version. But that's not part of this review request. I tend to not change the old stuff there if it is used (VHT is not used yet, so that's why there is cleanup going on there still). Simply MFCing it could be a pain without leaving compat defines. In this case it seems only iwn(4) would be affected so we could argue we can but I haven't checked all the other related fields and that's where the real pain comes in.

I think a separate patch "just rename these defines in sys/net80211/ieee80211.h" shall be OK, and can MFC, because their corresponding values are correct. If you agree, I can submit a patch for review.

The defines in net80211 are part of the public KPI and have been for a decade.
They are used at least in one device driver as well. I have not recently checked if there are any usable wlan(4) drivers in ports still supporting HT but that's due diligens to be done then as well.
If we are fine with breaking out-of-tree then we should probably just move the ones from LinuxKPI into net80211 and have a single definition of them.
But them we'll have inconsistent naming with the other fields for "HTINFO" etc. and that makes no sense either.
Hence my leaning of leaving the "legacy bits" as-are.

[cc]

In D43658#997148, @bz wrote:

If I understand correctly, these defines are from the Table 8-130—HT Operation element fields and subfields (continued) on page 615. I quoted them below. I think it's better to use cleaner names for these defines in sys/net80211/ieee80211.h and use an indent similar to the Linux version.

There's a lot to be said about the "native" net80211 version. But that's not part of this review request. I tend to not change the old stuff there if it is used (VHT is not used yet, so that's why there is cleanup going on there still). Simply MFCing it could be a pain without leaving compat defines. In this case it seems only iwn(4) would be affected so we could argue we can but I haven't checked all the other related fields and that's where the real pain comes in.

I think a separate patch "just rename these defines in sys/net80211/ieee80211.h" shall be OK, and can MFC, because their corresponding values are correct. If you agree, I can submit a patch for review.

The defines in net80211 are part of the public KPI and have been for a decade.
They are used at least in one device driver as well. I have not recently checked if there are any usable wlan(4) drivers in ports still supporting HT but that's due diligens to be done then as well.
If we are fine with breaking out-of-tree then we should probably just move the ones from LinuxKPI into net80211 and have a single definition of them.
But them we'll have inconsistent naming with the other fields for "HTINFO" etc. and that makes no sense either.
Hence my leaning of leaving the "legacy bits" as-are.

I thought the ports tree contains user-space applications. Are you talking about some user-space drivers in the ports tree?
I have searched around and find none of these "IEEE80211_HTINFO*" micros are used in ports tree "main" and branch "release/14.0.0", "release/13.2.0", "release/12.4.0", "release/13.1.0" and "release/12.3.0" from the past two years.
So I think the ports tree is out of concern and the only focus is in src tree.

I found drivers "mwl" and "iwn" each used one of these five micros. But to me, the current five micros are old and confusing. The point is we can make a small step and rename these five in net80211 (thus keep these LinuxKPI linked there). Still with "HTINFO" consistence, it looks to me is better than the current patch.
like this for example, changes in sys/net80211/ieee80211.h, and therefore some additional 30 name replaces.

/* bytes 2+3 */
/* 802.11-2012, Table 8-130-HT Operation element fields and subfields, HT Protection */
#define	IEEE80211_HTINFO_OPMODE		0x03	/* operating mode mask */
#define	IEEE80211_HTINFO_OPMODE_S	0               << remove this line, not used any more in stable/13 and stable/14
#define		IEEE80211_HTINFO_OPMODE_PROTECTION_NONE		0	/* for no protection mode */
#define		IEEE80211_HTINFO_OPMODE_PROTECTION_NONMEMBER	1	/* for nonmember protection mode */
#define		IEEE80211_HTINFO_OPMODE_PROTECTION_20MHZ	2	/* for 20 MHz protection mode */
#define		IEEE80211_HTINFO_OPMODE_PROTECTION_NONHT_MIXED	3	/* for non-HT mixed mode */

cc fbsd_src % cat ~/tmp/pattern.txt
IEEE80211_HTINFO_OPMODE
IEEE80211_HTINFO_OPMODE_S
IEEE80211_HTINFO_OPMODE_PURE
IEEE80211_HTINFO_OPMODE_PROTOPT
IEEE80211_HTINFO_OPMODE_HT20PR
IEEE80211_HTINFO_OPMODE_MIXED
cc fbsd_src % rg -w --file ~/tmp/pattern.txt | wc -l
      35
cc fbsd_src %

[cc]

I have created D43770 to clarify/illustrate my idea.

[cc]

With D43770 in mind, I think we can go ahead and commit this change.

[emaste]

I thought the ports tree contains user-space applications. Are you talking about some user-space drivers in the ports tree?

There are a number of kernel modules in the ports tree as well. drm-kmod is one well known example.

[cc]

In D43658#999086, @emaste wrote:

I thought the ports tree contains user-space applications. Are you talking about some user-space drivers in the ports tree?

There are a number of kernel modules in the ports tree as well. drm-kmod is one well known example.

Good to know about that. Thanks, appreciate it.