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

Ignore unrecognized NDISC options (as per RFC 4861). #210

Closed

Conversation

progval
Copy link
Contributor

@progval progval commented May 14, 2018

https://tools.ietf.org/html/rfc4861#section-4

Future versions of this protocol may define new option types.
Receivers MUST silently ignore any options they do not recognize
and continue processing the message.

@@ -228,7 +228,7 @@ impl<'a> Repr<'a> {
let opt = NdiscOption::new_checked(packet.payload())?;
match opt.option_type() {
NdiscOptionType::SourceLinkLayerAddr => Some(opt.link_layer_addr()),
_ => { return Err(Error::Unrecognized); }
_ => {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Very minor nit: we use () for the do-nothing case. r=me once this is fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

@progval progval force-pushed the ignore-unknown-ndisc-options branch from 04e3e68 to ecedf6d Compare May 14, 2018 16:49
@whitequark
Copy link
Contributor

@m-labs-homu r+
Thanks!

@m-labs-homu
Copy link

📌 Commit ecedf6d has been approved by whitequark

m-labs-homu pushed a commit that referenced this pull request May 14, 2018
@m-labs-homu
Copy link

⌛ Testing commit ecedf6d with merge ca079bf...

@m-labs-homu
Copy link

💔 Test failed - status-travis

Copy link
Collaborator

@dlrobertson dlrobertson left a comment

Choose a reason for hiding this comment

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

Good catch. Use None instead of ().

@progval progval force-pushed the ignore-unknown-ndisc-options branch from ecedf6d to 3398964 Compare May 14, 2018 17:10
@whitequark
Copy link
Contributor

@m-labs-homu r+

@m-labs-homu
Copy link

📌 Commit 3398964 has been approved by whitequark

@m-labs-homu
Copy link

⌛ Testing commit 3398964 with merge 1bde6e7...

@m-labs-homu
Copy link

☀️ Test successful - status-travis
Approved by: whitequark
Pushing 1bde6e7 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants