Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wireless/80211: update the 80211 header #5755

Merged
merged 1 commit into from
Apr 18, 2022

Conversation

anchao
Copy link
Contributor

@anchao anchao commented Mar 16, 2022

Summary

wireless/80211: update the 80211 header

Upstream from:
https://github.com/freebsd/freebsd-src/blob/main/sys/net80211/ieee80211.h

Signed-off-by: chao.an anchao@xiaomi.com

Impact

N/A

Testing

CI-check

include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
Copy link
Contributor

@pkarashchenko pkarashchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update is quite strange to me because many of structures that are in Linux header file (pointed in PR description) have __packed attribute while similar named structures i this PR are not equipped with packed.

include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
@btashton
Copy link
Contributor

We cannot have a GPL header from Linux that has just been reformatted and had the Apache header slapped on it.

Unless I am not understanding correctly this needs to be deleted ASAP.

@pkarashchenko
Copy link
Contributor

The original file is introduced in:

commit 38948fb65460adc5062f46cb033b4af47ec09559
Author: Gregory Nutt <gnutt@nuttx.org>
Date:   Sun Apr 30 17:40:10 2017 -0600

    ieee802.11: Bring some BSD licensed header files in from FreeBSD.

So there is no problem with that. The issue is only about adding the changes from GPL taken as a source

@jerpelea
Copy link
Contributor

@btashton in my understanding those definitions come from the standard and were implemented in linux. We will have the same definitions from the same standard even if we do not take them from the linux header. I think that we should filter out if there is something specific to linux in this header.

@jerpelea
Copy link
Contributor

@pkarashchenko
Copy link
Contributor

Ok. I will take a look into BSD variant ASAP

@jerpelea
Copy link
Contributor

@pkarashchenko
Copy link
Contributor

pkarashchenko commented Apr 14, 2022

@jerpelea @xiaoxiang781216 @anchao
I took a look into BSD variant of a header. As it was expected it as similar definitions and types/enums. So from this perspective I do not have any preferences either sync with Linux or BSD variant. But Linux variant has also a code (inline functions) that we are trying to adopt with this PR and from this perspective I definitely vote to sync with BSD variant.

@pkarashchenko
Copy link
Contributor

Seems like initial file was taken from OpenBSD http://fxr.watson.org/fxr/source/net80211/ieee80211.h?v=OPENBSD
At least when I compare it with NuttX version with OpenBSD / NetBSD / FreeBSD the NetBSD has the best match.
Again, I think we should sync with FreeBSD as it is maintained and matches the best from license perspective

@anchao
Copy link
Contributor Author

anchao commented Apr 17, 2022

Seems like initial file was taken from OpenBSD http://fxr.watson.org/fxr/source/net80211/ieee80211.h?v=OPENBSD At least when I compare it with NuttX version with OpenBSD / NetBSD / FreeBSD the NetBSD has the best match. Again, I think we should sync with FreeBSD as it is maintained and matches the best from license perspective

@pkarashchenko I updated ieee80211.h from freebsd, please review again.
https://github.com/freebsd/freebsd-src/blob/main/sys/net80211/ieee80211.h

Copy link
Contributor

@pkarashchenko pkarashchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one major comment about missing end_packed_struct. Other are minor style. In general LGTM

include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
include/nuttx/wireless/ieee80211/ieee80211.h Outdated Show resolved Hide resolved
@anchao
Copy link
Contributor Author

anchao commented Apr 18, 2022

Only one major comment about missing end_packed_struct. Other are minor style. In general LGTM

All addressed! Thank you!

@xiaoxiang781216
Copy link
Contributor

macOS ci always fail to "wget --quiet https://static.dev.sifive.com/dev-tools/riscv64-unknown-elf-gcc-8.3.0-2019.08.0-x86_64-apple-darwin.tar.gz".
Since this patch isn't specific to macOS, let's merge this simple header file change patch instead.

@xiaoxiang781216 xiaoxiang781216 merged commit 73aa337 into apache:master Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants