Implement ipv6-address-token-id key (LP: #1737976) #161

slyon · 2020-08-28T08:10:51Z

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

Runs make check successfully.
Retains 100% code coverage (make check-coverage).
New/changed keys in YAML format are documented.
(Optional) Closes an open bug in Launchpad: https://bugs.launchpad.net/ubuntu/+source/nplan/+bug/1737976

…-mode This is being implemented for any prefix in systemd PR#16618: systemd/systemd#16618

codecov-commenter · 2020-08-28T08:12:04Z

Codecov Report

Merging #161 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            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

Impacted Files	Coverage Δ
src/networkd.c	`100.00% <100.00%> (ø)`
src/nm.c	`100.00% <100.00%> (ø)`
src/parse.c	`97.65% <100.00%> (+0.01%)`	⬆️
src/validation.c	`87.31% <100.00%> (+0.19%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd750e2...9b35145. Read the comment docs.

slyon · 2020-08-28T08:12:36Z

Hey @vorlonofportland I'd like to get your opinion on the the schema key wording: ipv6-address-token-id
I wasn't really sure how to name it... what do you think about it?

ghost · 2020-08-28T10:47:57Z

You could drop the id portion of the schema key. The noun in this case is token; there is no other identifier.

ipv6-address-token

slyon · 2020-08-28T10:58:01Z

You could drop the id portion of the schema key. The noun in this case is token; there is no other identifier.

ipv6-address-token

Yeah... I was thinking about this as well... but this token is basically the "IPv6 interface identifier", so I wanted to add the -id part.
Maybe ipv6-interface-id would be a good name?

ghost · 2020-08-28T11:06:58Z

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 ipv6-address-token.

Add: I shy away from attaching id to any configuration element that does not serve to uniquely identify the "thing" being configured. In this case, it's easy (and often done) to assign the same token value to every interface of a multi-homed system (e.g. router, firewall), so that the host portion of every interface on that system is, say, ::1. Since this configuration element does not need to be unique across interfaces in the same system, I would hesitate to attach id, since it doesn't function as an identifier at all.

src/parse.c

vorlonofportland · 2020-08-28T16:18:03Z

src/validation.c

@@ -195,6 +195,9 @@ validate_netdef_grammar(NetplanNetDefinition* nd, yaml_node_t* node, GError** er
            goto netdef_grammar_error;
    }

+    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);
+


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?

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.

vorlonofportland · 2020-08-28T16:26:23Z

doc/netplan.md

+:   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``.
+


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.

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.

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.

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

Ok, this looks good. I see all Steve's review comments have also been addressed, so I think this is good to go.

slyon added 3 commits August 27, 2020 16:49

Implement ipv6-address-token-id key (LP: #1737976)

0df6fbb

Add integration tests for ipv6-address-token-id

9af841b

networkd: update comment about stable-privacy ipv6-address-generation…

9a50273

…-mode This is being implemented for any prefix in systemd PR#16618: systemd/systemd#16618

slyon added the schema change This PR includes a change to netplan's YAML schema, which needs sign-off label Aug 28, 2020

slyon requested a review from vorlonofportland August 28, 2020 08:11

vorlonofportland requested changes Aug 28, 2020

View reviewed changes

slyon added 2 commits August 31, 2020 12:10

rename ipv6-address-token from ipv6-address-token-id

73da0f8

avoid unrelated whitespace changes

d556884

sil2100 approved these changes Aug 31, 2020

View reviewed changes

Merge branch 'master' into slyon/tokenized-ipv6-identifiers

9b35145

slyon merged commit ee50582 into master Aug 31, 2020

slyon deleted the slyon/tokenized-ipv6-identifiers branch September 3, 2020 09:48

ThomasCr mentioned this pull request Jun 10, 2021

IPv6 configuration troglobit/inadyn#52

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement ipv6-address-token-id key (LP: #1737976) #161

Implement ipv6-address-token-id key (LP: #1737976) #161

slyon commented Aug 28, 2020

codecov-commenter commented Aug 28, 2020 •

edited

Loading

slyon commented Aug 28, 2020

ghost commented Aug 28, 2020

slyon commented Aug 28, 2020 •

edited

Loading

ghost commented Aug 28, 2020 •

edited by ghost

Loading

vorlonofportland Aug 28, 2020

slyon Aug 31, 2020

vorlonofportland Aug 28, 2020

ghost Aug 28, 2020 •

edited by ghost

Loading

vorlonofportland Aug 28, 2020

slyon Aug 31, 2020

sil2100 left a comment

Implement ipv6-address-token-id key (LP: #1737976) #161

Implement ipv6-address-token-id key (LP: #1737976) #161

Conversation

slyon commented Aug 28, 2020

Description

Checklist

codecov-commenter commented Aug 28, 2020 • edited Loading

Codecov Report

slyon commented Aug 28, 2020

ghost commented Aug 28, 2020

slyon commented Aug 28, 2020 • edited Loading

ghost commented Aug 28, 2020 • edited by ghost Loading

vorlonofportland Aug 28, 2020

Choose a reason for hiding this comment

slyon Aug 31, 2020

Choose a reason for hiding this comment

vorlonofportland Aug 28, 2020

Choose a reason for hiding this comment

ghost Aug 28, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

vorlonofportland Aug 28, 2020

Choose a reason for hiding this comment

slyon Aug 31, 2020

Choose a reason for hiding this comment

sil2100 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 28, 2020 •

edited

Loading

slyon commented Aug 28, 2020 •

edited

Loading

ghost commented Aug 28, 2020 •

edited by ghost

Loading

ghost Aug 28, 2020 •

edited by ghost

Loading