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

gnrc_ndp_internal: add capability to add external options to NAs #3747

Merged
merged 2 commits into from
Aug 31, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Aug 31, 2015

6LoWPAN-ND adds the address registration option (ARO) which can be part of either Neighbor Solicitations (NSs) or Neighbor Advertisements (NAs). The value of the status field in the advertised ARO is dependent on the ARO provided by an NS, so the value of the option must be set out of the scope of the send function. This API change allows for that in the least intrusive way, by allowing to add preallocated options to the send function.

@miri64 miri64 added Area: network Area: Networking Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. NSTF labels Aug 31, 2015
@miri64 miri64 added this to the Release 2015.08 milestone Aug 31, 2015

if (pkt == NULL) {
DEBUG("ndp internal: error allocating Target Link-layer address option.\n");
gnrc_pktbuf_release(pkt);
gnrc_pktbuf_release(ext_opts);
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't ext_opts rather be released on the caller side (if desired) by indicating an error as a return value? I am just thinking: The caller might want to reuse the pktsnip, if gnrc_ndp_opt_tl2a_build fails.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point...

Copy link
Member Author

Choose a reason for hiding this comment

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

On the other hand: the releases of pkt below release ext_opts, too. So this would be inconsistent.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, then I would opt for a good documentation for now, stating that ext_ops could be modified after the call to gnrc_ndp_internal_send_nbr_adv

@cgundogan
Copy link
Member

@miri64
Copy link
Member Author

miri64 commented Aug 31, 2015

Done

@miri64 miri64 force-pushed the gnrc_ndp_internal/api/ext-opts branch from 42e7619 to 4a075cd Compare August 31, 2015 14:37
@cgundogan
Copy link
Member

ACK, squash and GO.

@cgundogan cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 31, 2015
@miri64 miri64 force-pushed the gnrc_ndp_internal/api/ext-opts branch from 4a075cd to 82924c6 Compare August 31, 2015 16:03
@miri64
Copy link
Member Author

miri64 commented Aug 31, 2015

Squashed.

@cgundogan
Copy link
Member

and go

cgundogan added a commit that referenced this pull request Aug 31, 2015
…opts

gnrc_ndp_internal: add capability to add external options to NAs
@cgundogan cgundogan merged commit d1a0247 into RIOT-OS:master Aug 31, 2015
@miri64 miri64 deleted the gnrc_ndp_internal/api/ext-opts branch August 31, 2015 17:03
@miri64
Copy link
Member Author

miri64 commented Aug 31, 2015

Thanks for the quick review :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants