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

[WIP] ng_ndp: Initial import of router discovery #2612

Closed
wants to merge 14 commits into from
Closed

[WIP] ng_ndp: Initial import of router discovery #2612

wants to merge 14 commits into from

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Mar 16, 2015

Depends on #2555 and #2721 and all their dependencies.

This is still very WIP, but I wanted to publish it anyways.

@miri64 miri64 added Area: network Area: Networking State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: waiting for other PR State: The PR requires another PR to be merged first NSTF labels Mar 16, 2015
@miri64 miri64 added this to the Network Stack Task Force milestone Mar 16, 2015
@miri64 miri64 mentioned this pull request Mar 16, 2015
Closed
36 tasks
@miri64 miri64 force-pushed the ng_ndp/feat/initial branch 2 times, most recently from efa4adc to 6f1c927 Compare March 19, 2015 15:00
@miri64
Copy link
Member Author

miri64 commented Mar 19, 2015

Rebased to current #2555

@miri64
Copy link
Member Author

miri64 commented Mar 25, 2015

Rebased to current #2555

@miri64 miri64 force-pushed the ng_ndp/feat/initial branch 3 times, most recently from b41e672 to ee92c15 Compare March 30, 2015 01:58
@miri64
Copy link
Member Author

miri64 commented Mar 30, 2015

Rebased to current #2555

@OlegHahm OlegHahm force-pushed the master branch 3 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01
@miri64 miri64 force-pushed the ng_ndp/feat/initial branch 2 times, most recently from 28b65a3 to 5fbaf17 Compare April 1, 2015 16:11
@miri64
Copy link
Member Author

miri64 commented Apr 1, 2015

Rebased to current #2555 and #2721

@miri64 miri64 mentioned this pull request Apr 7, 2015
Merged
@miri64
Copy link
Member Author

miri64 commented Apr 10, 2015

Rebased to current #2555

@miri64 miri64 mentioned this pull request Apr 14, 2015
Merged
@miri64 miri64 force-pushed the ng_ndp/feat/initial branch 2 times, most recently from bca9153 to c793834 Compare April 21, 2015 16:00
@miri64
Copy link
Member Author

miri64 commented Apr 21, 2015

Rebased to current #2555.

@miri64
Copy link
Member Author

miri64 commented Apr 22, 2015

Rebased to current #2555 and squashed

Lotterleben
Lotterleben reviewed May 5, 2015
View reviewed changes
sys/net/network_layer/ng_ndp/ng_ndp.c
}
}

static bool _handle_pi_opt(kernel_pid_t iface, ng_pktsnip_t *pkt,
Copy link
Member

Choose a reason for hiding this comment

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

mhm.. is there a reason it's nowhere noted what this function actually does?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because this PR is still WIP :-) It handles the PI (prefix information) option, which disseminate a prefix through a sub-net.

@OlegHahm
Copy link
Member

Does this PR still depend on another one?

@miri64
Copy link
Member Author

miri64 commented May 10, 2015

Yes, but one that isn't open yet. This PR is WIP, it needs thorough clean-up and only implements router discovery. I'll open a PR for the Address Resolution in the next few days, but it will depent on #2908 and #2909

@miri64
Copy link
Member Author

miri64 commented May 10, 2015

Actually, the PR I spoke about is already open: #2910

Lotterleben
Lotterleben reviewed May 12, 2015
View reviewed changes
sys/include/net/ng_ipv6/nc.h
@@ -123,6 +124,11 @@ typedef struct {
uint8_t l2_addr_len; /**< Length of ng_ipv6_nc_t::l2_addr */
uint8_t flags; /**< Flags as defined above */
Copy link
Member

Choose a reason for hiding this comment

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

Why does this comment say as defined above, when the only definition “above” is actually in RFC 4861? When I first read that comment, I expected there to be a typedef'd enum which contained all possible flags, and the type of flags to be ng_ipv6_nc_config_flags or something rather than uint8_t... Would that make sense or am I missing something here?
(After skimming through RFC 4861, I'm assuming flags refers to the M and O flags...)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a problem in general to reference a group of flags in doxygen. The defines prefixed with NG_IPV6_NC_... in the document group "Flag definitions for ng_ipv6_ref_t are meant. I'm against using enums for flags (see #2614 (comment) on that note). Have you maybe a better solution for referencing them?

@miri64 miri64 changed the title [WIP] ng_ndp: Initial import [WIP] ng_ndp: Initial import of router discovery May 12, 2015
@miri64
Copy link
Member Author

miri64 commented May 12, 2015

This PR contains only router discovery. I will rebate it to #2910 as soon as that one reaches some maturity.

This was referenced May 16, 2015
Merged
Merged
@miri64
Copy link
Member Author

miri64 commented May 22, 2015

Superseded by #3049

@miri64 miri64 closed this May 22, 2015
@miri64 miri64 deleted the ng_ndp/feat/initial branch May 22, 2015 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@OlegHahm OlegHahm

Labels
Area: network Area: Networking Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: waiting for other PR State: The PR requires another PR to be merged first State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet
Projects
None yet
Milestone
Network Stack Task Force
Development

Successfully merging this pull request may close these issues.
4 participants
@miri64 @OlegHahm @Lotterleben @haukepetersen