Skip to content

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

Merged
cgundogan merged 2 commits intoRIOT-OS:masterfrom
miri64:gnrc_ndp_internal/api/ext-opts
Aug 31, 2015
Merged

gnrc_ndp_internal: add capability to add external options to NAs#3747
cgundogan merged 2 commits intoRIOT-OS:masterfrom
miri64:gnrc_ndp_internal/api/ext-opts

Conversation

@miri64
Copy link
Copy Markdown
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
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

@miri64
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

miri64 commented Aug 31, 2015

Squashed.

@cgundogan
Copy link
Copy Markdown
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
Copy Markdown
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