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

Updates on IP address and prefix representation #1

Merged
merged 4 commits into from
Feb 21, 2024
Merged

Conversation

marenamat
Copy link
Collaborator

No description provided.

@marenamat marenamat requested a review from cabo as a code owner January 17, 2024 21:57
Copy link
Owner

@cabo cabo left a comment

Choose a reason for hiding this comment

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

Thanks!

Approved this with a few suggestions.

Addresses that have zones given cannot use tag 54/52.
Addresses with leading zeros cannot use tag 54/52.
Addresses that do not comply with {{RFC5952}} cannot use tag 54.
The encoder MAY normalize IPv6 addresses and prefixes that do not comply with {{RFC5952}}
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think I know what an encoder is here.
The conversion from stand-in tag to a legacy representation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a definition to the top.

Addresses with leading zeros cannot use tag 54/52.
Addresses that do not comply with {{RFC5952}} cannot use tag 54.
The encoder MAY normalize IPv6 addresses and prefixes that do not comply with {{RFC5952}}
but can be converted into the stand-in representation. The encoder implementation SHOULD be clear
Copy link
Owner

@cabo cabo Jan 18, 2024

Choose a reason for hiding this comment

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

We tend not to use RFC 2119 keywords for documentation requirements.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, thanks. (This is my first RFC, please be patient.)

but can be converted into the stand-in representation. The encoder implementation SHOULD be clear
about whether this normalization is employed or not.

If the schema specifies `ip-prefix`, the encoder MAY normalize prefixes with
Copy link
Owner

Choose a reason for hiding this comment

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

normalize with or prefixes with? (Maybe an in-line example would clarify this.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

clarified, hopefully

draft-bormann-cbor-yang-standin.md Outdated Show resolved Hide resolved
draft-bormann-cbor-yang-standin.md Outdated Show resolved Hide resolved
draft-bormann-cbor-yang-standin.md Show resolved Hide resolved

If the schema specifies a union between `ip-prefix` and `ip-address-and-prefix` (or a union of their subtypes),
the encoder MUST prefer the `ip-address-and-prefix` stand-in over `ip-prefix`
format. Specifically, it the schema allows to encode 2001:db8:1234::/48 both as
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
format. Specifically, it the schema allows to encode 2001:db8:1234::/48 both as
format. Specifically, if the schema allows to encode 2001:db8:1234::/48 both as

non-zero bits after the prefix end. The encoder implementation SHOULD be clear
about whether this normalization is employed or not.

If the schema specifies a union between `ip-prefix` and `ip-address-and-prefix` (or a union of their subtypes),
Copy link
Owner

@cabo cabo Jan 18, 2024

Choose a reason for hiding this comment

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

We probably need to address unions in general.
Put in #2

@cabo cabo merged commit 4a31bd5 into cabo:main Feb 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants