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

ng_icmpv6: Initial import #2555

Merged
merged 4 commits into from
May 1, 2015
Merged

ng_icmpv6: Initial import #2555

merged 4 commits into from
May 1, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Mar 7, 2015

Depends on #2454 and all its dependencies.

@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 7, 2015
@miri64 miri64 added this to the Network Stack Task Force milestone Mar 7, 2015
@miri64 miri64 mentioned this pull request Mar 7, 2015
Closed
36 tasks
@miri64 miri64 force-pushed the ng_icmpv6/feat/initial branch 4 times, most recently from 7b5d706 to 89a23fd Compare March 12, 2015 01:28
@miri64
Copy link
Member Author

miri64 commented Mar 12, 2015

Rebased to current #2454

@miri64 miri64 force-pushed the ng_icmpv6/feat/initial branch 2 times, most recently from 7f1b556 to 33de13f Compare March 15, 2015 14:23
@miri64
Copy link
Member Author

miri64 commented Mar 15, 2015

Rebased to current #2454

@miri64 miri64 mentioned this pull request Mar 16, 2015
Closed
@miri64 miri64 force-pushed the ng_icmpv6/feat/initial branch 2 times, most recently from e36047f to 43e113f Compare March 19, 2015 14:58
@miri64
Copy link
Member Author

miri64 commented Mar 19, 2015

Rebased to current #2454

This was referenced Mar 19, 2015
Merged
Merged
@miri64
Copy link
Member Author

miri64 commented Mar 25, 2015

Rebased to current #2454

@miri64
Copy link
Member Author

miri64 commented Mar 30, 2015

Rebased and adapted to changes in #2454. Since they were quite big, I squashed.

@miri64 miri64 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Mar 30, 2015
@miri64
Copy link
Member Author

miri64 commented Mar 30, 2015

(No longer WIP)

cgundogan
cgundogan reviewed Apr 28, 2015
View reviewed changes
sys/net/network_layer/ng_icmpv6/ng_icmpv6.c
break;
}

/* ICMPv6-all will be send send in ng_ipv6.c so only dispatch of
Copy link
Member

Choose a reason for hiding this comment

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

too many sends!

@miri64
Copy link
Member Author

miri64 commented Apr 29, 2015

Addressed comments

OlegHahm
OlegHahm reviewed Apr 29, 2015
View reviewed changes
sys/net/network_layer/ng_icmpv6/echo/ng_icmpv6_echo.c
return _echo_build(NG_ICMPV6_ECHO_REQ, id, seq, data, data_len);
}

ng_pktsnip_t *ng_icmpv6_echo_rep_build(uint16_t id, uint16_t seq, uint8_t *data, size_t data_len)
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to inline these functions now?

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 I need to expose the generic function for this. Anyways: just did that.

@OlegHahm OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Apr 30, 2015
OlegHahm
OlegHahm reviewed Apr 30, 2015
View reviewed changes
sys/net/network_layer/ng_icmpv6/echo/ng_icmpv6_echo.c

ng_pktbuf_hold(pkt, ng_netreg_num(NG_NETTYPE_IPV6, NG_NETREG_DEMUX_CTX_ALL) - 1);

/* ICMPv6 is not interested anymore so `- 1` */
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this comment make more sense if written right before ng_pktbuf_hold()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

@miri64
Copy link
Member Author

miri64 commented Apr 30, 2015

Addressed comments

@OlegHahm
Copy link
Member

ACK. Please squash and go if Travis agrees.

@miri64
Copy link
Member Author

miri64 commented Apr 30, 2015

Squashed

@miri64 miri64 mentioned this pull request Apr 30, 2015
Merged
@miri64 miri64 removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 1, 2015
@miri64
Copy link
Member Author

miri64 commented May 1, 2015

Had to fix something: since the types of the echo request/reply were now exposed, I needed to put them somewhere else than ng_icmpv6.h => introduced ng_icmpv6/types.h

@miri64
Copy link
Member Author

miri64 commented May 1, 2015

Please re-ACK.

@OlegHahm
Copy link
Member

OlegHahm commented May 1, 2015

I think that's fine. ACK and go.

OlegHahm added a commit that referenced this pull request May 1, 2015
@OlegHahm
Merge pull request #2555 from authmillenon/ng_icmpv6/feat/initial
d1bfa2f 
ng_icmpv6: Initial import
@OlegHahm OlegHahm merged commit d1bfa2f into RIOT-OS:master May 1, 2015
@miri64 miri64 deleted the ng_icmpv6/feat/initial branch May 1, 2015 11:45
@miri64 miri64 mentioned this pull request Feb 1, 2019
Merged
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.
Projects
None yet
Milestone
Network Stack Task Force
Development

Successfully merging this pull request may close these issues.
5 participants
@miri64 @OlegHahm @cgundogan @kaspar030 @haukepetersen