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.

Sign up for GitHub

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

Jump to bottom

sys/net: Add ipv4/ipv6 pseudomodules #16966

Merged
merged 3 commits into from
Oct 20, 2021
Merged

sys/net: Add ipv4/ipv6 pseudomodules #16966

merged 3 commits into from
Oct 20, 2021

Conversation

yarrick
Copy link
Contributor

@yarrick yarrick commented Oct 8, 2021 •
edited
Loading

Contribution description

New pseudomodules ipv4 and ipv6 are enabled whenever a networking stack supports the relevant protocol.

Change how SOCK_HAS_IPV6 is set, is now done in one place. (The define can now be replaced with a ipv6 module check if that is preferred)

Testing procedure

  • tests/gnrc_sock_async_event still works

Issues/PRs references

Another step towards a shared ifconfig shell command implementation.

@github-actions github-actions bot added Area: build system Area: Build system Area: network Area: Networking Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework labels Oct 8, 2021
miri64
miri64 previously requested changes Oct 8, 2021
View reviewed changes
makefiles/pseudomodules.inc.mk Outdated Show resolved Hide resolved
@yarrick
sys/net: Add ipv6 pseudomodule
3df56d5 
Enabled when GNRC, lwIP or OpenWSN uses IPv6
@yarrick yarrick changed the title sys/net: Add net_ipv4/ipv6 pseudomodules sys/net: Add ipv4/ipv6 pseudomodules Oct 8, 2021
@miri64
Copy link
Member

miri64 commented Oct 8, 2021

@fjmolinas can you have a look if this makes sense for OpenWSN, please?

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: run tests If set, CI server will run tests on hardware for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 8, 2021
@yarrick
Copy link
Contributor Author

yarrick commented Oct 8, 2021 •
edited
Loading

Changed check of sock_% to sock sock_%, to fix build issue (and not match socket_zep)

@miri64 miri64 dismissed their stale review October 9, 2021 09:50

Issue was addressed

kfessel
kfessel reviewed Oct 11, 2021
View reviewed changes
Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

why does this remove the GNRC specification around seemingly gnrc specific code without making it more general?

sys/shell/commands/sc_gnrc_netif.c Outdated Show resolved Hide resolved
sys/shell/commands/sc_gnrc_netif.c Outdated Show resolved Hide resolved
sys/shell/commands/sc_gnrc_netif.c Outdated Show resolved Hide resolved
sys/shell/commands/sc_gnrc_netif.c Outdated Show resolved Hide resolved
sys/shell/commands/sc_gnrc_netif.c Outdated Show resolved Hide resolved
@yarrick
sys/net/sock: Set SOCK_HAS_IPV6 in one place
ff7bf09 
Based on ipv6 pseudomodule
@yarrick
sys/net: Add ipv4 pseudomodule
9844953 
Enabled when lwIP uses IPv4.
@kfessel
Copy link
Contributor

kfessel commented Oct 11, 2021 •
edited
Loading

i tested:
GNRC:
tests/gnrc_sock_async_event is working and is able to test part of this patch (gnrc) ✔️
LWIP:
tests/lwip built and had a look at riotbuild.h found SOCK_HAS_IPV6 ✅
openswn:
tests/pkg_openswn and tests\openswn_sock_udp built and had a look at riotbuild.h found SOCK_HAS_IPV6 ✅

lets have another ping @fjmolinas

@fjmolinas
Copy link
Contributor

@fjmolinas can you have a look if this makes sense for OpenWSN, please?

LGTM, sorry for the delay was on vacation.

@kfessel kfessel added the Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines label Oct 18, 2021
@kfessel kfessel added Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels Oct 18, 2021
@yarrick
Copy link
Contributor Author

yarrick commented Oct 20, 2021

Is there anything left to do here?

kfessel
kfessel approved these changes Oct 20, 2021
View reviewed changes
Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

🚀

@kfessel kfessel merged commit c9f4d11 into RIOT-OS:master Oct 20, 2021
@yarrick yarrick deleted the net_v6 branch October 28, 2021 07:50
@fjmolinas fjmolinas added this to the Release 2022.01 milestone Nov 18, 2021
@yarrick yarrick mentioned this pull request Sep 28, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 left review comments

@kfessel kfessel kfessel approved these changes

@cgundogan cgundogan Awaiting requested review from cgundogan

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann PeterKietzmann is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

Assignees
No one assigned
Labels
Area: build system Area: Build system Area: network Area: Networking Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
Release 2022.01
Development

Successfully merging this pull request may close these issues.
4 participants
@yarrick @miri64 @kfessel @fjmolinas