Implement ipv6-address-token-id key (LP: #1737976)#161
Conversation
…-mode This is being implemented for any prefix in systemd PR#16618: systemd/systemd#16618
slyon
added
the
schema change
Codecov Report
@@ Coverage Diff @@
## master #161 +/- ##
==========================================
+ Coverage 93.12% 93.15% +0.03%
==========================================
Files 9 9
Lines 2618 2631 +13
==========================================
+ Hits 2438 2451 +13
Misses 180 180
Continue to review full report at Codecov.
|
|
Hey @vorlonofportland I'd like to get your opinion on the the schema key wording: |
|
You could drop the
|
Yeah... I was thinking about this as well... but this token is basically the "IPv6 interface identifier", so I wanted to add the |
|
That's actually not what a token is. A token allows the administrator to set the host portion of the address to a static value even though the rest of the configuration is some form of autoconfiguration. It has more to do with what address the interface winds up with than anything else. Also, interface ID is very ambiguous with other configuration elements. The only truly accurate and unambiguous name for this is Add: I shy away from attaching |
|
|
||
| if (nd->ip6_addr_gen_mode > NETPLAN_ADDRGEN_EUI64 && nd->ip6_addr_gen_token) | ||
| return yaml_error(node, error, "%s: ipv6-address-generation and ipv6-address-token-id are mutually exclusive", nd->id); | ||
|
|
There was a problem hiding this comment.
I'm uncomfortable with this being an inequality comparison. If other values are added to NetplanAddrGenMode in the future, what guarantees these checks are correct?
There was a problem hiding this comment.
Good point! I switched to a != NETPLAN_ADDRGEN_DEFAULT comparison, which makes it more explicit to fail if ipv6-address-generation is set in any way.
| : Define an IPv6 address token for creating a static interface identifier for | ||
| IPv6 Stateless Address Autoconfiguration. This implies a slightly modified | ||
| ``eui64`` mode and is mutually exclusive with ``ipv6-address-generation``. | ||
|
|
There was a problem hiding this comment.
I agree with the other PR comments: "token id" is redundant.
I also think that "token" itself is unclear; and at least in networkd, token is used to mean something other than just the id.
https://tools.ietf.org/html/rfc4862 mentions in Appendix B that they /used to/ use the term "interface token" but now say "interface identifier".
At https://developer.gnome.org/NetworkManager/stable/settings-ipv6.html, NetworkManager references draft-chown-6man-tokenised-ipv6-identifiers-02 for its definition of "token". But this also refers to "tokenized interface identifier" - so "interface identifier" is used consistently.
Therefore I recommend "ipv6-interface-identifier" as the key for this.
There was a problem hiding this comment.
The term used in iproute2, NetworkManager, systemd, etc. is "token." The token is not an interface identifier; labeling it as such is extremely misleading. The accurate key for this is, as stated, ipv6-address-token.
Ignore what I said previously. Apparently the terminology has been changed, as you stated. That's what I get for forgetting that technology and documentation are living entities; my apologies.
I would prefer something like ipv6-token-id, though it was said to be redundant earlier. I would prefer the term 'token' to be retained within the key, if only to provide some clarity/context.
There was a problem hiding this comment.
I agree it's important that this be discoverable for people who are looking for "token", given that this term is used throughout iproute2, NetworkManager, and networkd as you mention (though, not with a consistent meaning).
Given that the term 'token' is deprecated in the RFCs, however, I don't like ipv6-token-id as this propagates the idea of a 'token' as something that new users are expected to understand.
My suggestions are either to use the ipv6-interface-identifier and reference 'token' in the documentation; or use ipv6-address-token.
There was a problem hiding this comment.
Thank you very much @cluonbeam and @vorlonofportland for your input!
I'm going with ipv6-address-token then, to keep it in line with the related ipv6-address-generation setting.
sil2100
left a comment
sil2100
left a comment
There was a problem hiding this comment.
Ok, this looks good. I see all Steve's review comments have also been addressed, so I think this is good to go.
4 participants
Description
This allows to statically configure the IPv6 host ID (low 64 bits) when auto-generated IPv6 addressing is used (i.e. DHCPv6 stateless, SLAAC).
It introduces one new YAML key in the schema:
ipv6-address-token-id.Fixes LP: #1737976
Checklist
make checksuccessfully.make check-coverage).