| Message ID | 20240318211252.62591-1-fnordahl@ubuntu.com |
|---|---|
| State | RFC |
| Headers | show |
| Series | [ovs-dev,RFC] controller: Always populate prefix-length for IPv6 PD. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Mar 18, 2024 at 10:12 PM Frode Nordahl <fnordahl@ubuntu.com> wrote: > > The prefix we obtain is used to fill the ``ipv6_ra_prefixes`` > option for configuration of instances through SLAAC. > > As discussed in RFC 7421 the interface identifier is 64 bits long, > and client implementations refrain from performing SLAAC with any > other prefix length. > > TODO: I'm pretty sure the origin of this behavior is tied to an > older RFC, attempt to track it down. > > The DHCPv6 server is at liberty to offer whichever prefix length it > wants, so make sure new requests are for a prefix length of 64 so > that it is suitable. > > TODO: We may need to make the default prefix-length to request > configurable? Setting this to 64 fixes an issue I see in my > network where the server will delegate a /62 by default, > which does not work well for SLAAC. > > A future improvement might be to adjust the requested prefix length > to the number of downstream LRPs we have and then allocate to the > downstream LRPs from that larger delegated prefix. > > TODO: We are free to request whatever prefix-length we want, and > the server is equally free to ignore our request and give us > something else. So we need some generic handling of > allocations where prefix-length < 64. Perhaps just split > the prefix up in /64 chunks and add them all to > ``ipv6_ra_prefixes`` ? > > TODO: tests > > QUESTION: From cursory view there appears to be an issue with > routing inside OVN after successful IPv6 Prefix > Delegation, it may be an issue with my setup, but > thought I might as well ask. Is the prefix delegation > code supposed to automatically handle internal routing? I apparently can't quite let go of this itch :) Responding to my own question, there does appear to be missing functionality in a few more areas to make IPv6 Prefix Delegation support complete: 1) We need to include the learned ipv6_prefix when building lr_in_ip_routing logical flows, something like this does it: diff --git a/lib/ovn-util.c b/lib/ovn-util.c index ee5cbcdc3..94ebf6488 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -344,6 +344,22 @@ extract_lrp_networks(const struct nbrec_logical_router_port *lrp, } } + for (int i = 0; i < lrp->n_ipv6_prefix; i++) { + struct in6_addr ip6; + unsigned int plen; + char *error; + + error = ipv6_parse_cidr(lrp->ipv6_prefix[i], &ip6, &plen); + if (!error) { + add_ipv6_netaddr(laddrs, ip6, plen); + } else { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_INFO_RL(&rl, "invalid syntax '%s' in ipv6_prefix", + lrp->networks[i]); + free(error); + } + } + --- I just piggy backed on extract_lrp_networks() here, but it might deserve its own extract_lrp_delegated_ipv6_prefix() or something like that for clarity? 2) Traffic is currently allowed to pass through port security, presumably due to ND/MAC learning, but we should probably let IPAM know about the learned prefixes so that dynamic addresses can be properly populated? 3) I need to manually add an external v6 default route, but as a user I would expect this to be learned through RA when IPv6 Prefix Delegation is enabled. Maybe we should make this automatic? > Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com> > --- > controller/pinctrl.c | 35 ++++++++++++++++++++++++----------- > 1 file changed, 24 insertions(+), 11 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 721a737de..8c4425a2e 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -1120,9 +1120,7 @@ compose_prefixd_packet(struct dp_packet *b, struct ipv6_prefixd_state *pfd) > if (pfd->uuid.len) { > payload += pfd->uuid.len + sizeof(struct dhcpv6_opt_header); > } > - if (ipv6_addr_is_set(&pfd->prefix)) { > - payload += sizeof(struct dhcpv6_opt_ia_prefix); > - } > + payload += sizeof(struct dhcpv6_opt_ia_prefix); > > eth_compose(b, (struct eth_addr) ETH_ADDR_C(33,33,00,01,00,02), pfd->ea, > ETH_TYPE_IPV6, IPV6_HEADER_LEN); > @@ -1174,23 +1172,38 @@ compose_prefixd_packet(struct dp_packet *b, struct ipv6_prefixd_state *pfd) > ia_pd->opt.code = htons(DHCPV6_OPT_IA_PD); > int opt_len = sizeof(struct dhcpv6_opt_ia_na) - > sizeof(struct dhcpv6_opt_header); > - if (ipv6_addr_is_set(&pfd->prefix)) { > - opt_len += sizeof(struct dhcpv6_opt_ia_prefix); > - } > + opt_len += sizeof(struct dhcpv6_opt_ia_prefix); > ia_pd->opt.len = htons(opt_len); > ia_pd->iaid = htonl(pfd->aid); > ia_pd->t1 = OVS_BE32_MAX; > ia_pd->t2 = OVS_BE32_MAX; > + struct dhcpv6_opt_ia_prefix *ia_prefix = > + (struct dhcpv6_opt_ia_prefix *)(ia_pd + 1); > + ia_prefix->opt.code = htons(DHCPV6_OPT_IA_PREFIX); > + ia_prefix->opt.len = htons(sizeof(struct dhcpv6_opt_ia_prefix) - > + sizeof(struct dhcpv6_opt_header)); > if (ipv6_addr_is_set(&pfd->prefix)) { > - struct dhcpv6_opt_ia_prefix *ia_prefix = > - (struct dhcpv6_opt_ia_prefix *)(ia_pd + 1); > - ia_prefix->opt.code = htons(DHCPV6_OPT_IA_PREFIX); > - ia_prefix->opt.len = htons(sizeof(struct dhcpv6_opt_ia_prefix) - > - sizeof(struct dhcpv6_opt_header)); > ia_prefix->plife_time = OVS_BE32_MAX; > ia_prefix->vlife_time = OVS_BE32_MAX; > ia_prefix->plen = pfd->plen; > ia_prefix->ipv6 = pfd->prefix; > + } else { > + /* The prefix we obtain is used to fill the ``ipv6_ra_prefixes`` > + * option for configuration of instances through SLAAC. > + * > + * As discussed in RFC 7421 the interface identifier is 64 bits long, > + * and client implementations refrain from performing SLAAC with any > + * other prefix length. > + * > + * The DHCPv6 server is at liberty to offer whichever prefix length it > + * wants, so make sure new requests are for a prefix length of 64 so > + * that it is suitable. > + * > + * A future improvement might be to adjust the requested prefix length > + * to the number of downstream LRPs we have and then allocate to the > + * downstream LRPs from that larger delegated prefix. */ > + ia_prefix->ipv6 = in6addr_any; > + ia_prefix->plen = 64; > } > > uint32_t csum = packet_csum_pseudoheader6(dp_packet_l3(b)); > -- > 2.43.0 >
On 3/19/24 10:14, Frode Nordahl wrote: > On Mon, Mar 18, 2024 at 10:12 PM Frode Nordahl <fnordahl@ubuntu.com> wrote: >> Hi Frode, Sorry for taking so long to reply. >> The prefix we obtain is used to fill the ``ipv6_ra_prefixes`` >> option for configuration of instances through SLAAC. >> >> As discussed in RFC 7421 the interface identifier is 64 bits long, >> and client implementations refrain from performing SLAAC with any >> other prefix length. >> >> TODO: I'm pretty sure the origin of this behavior is tied to an >> older RFC, attempt to track it down. >> >> The DHCPv6 server is at liberty to offer whichever prefix length it >> wants, so make sure new requests are for a prefix length of 64 so >> that it is suitable. >> >> TODO: We may need to make the default prefix-length to request >> configurable? Setting this to 64 fixes an issue I see in my >> network where the server will delegate a /62 by default, >> which does not work well for SLAAC. >> >> A future improvement might be to adjust the requested prefix length >> to the number of downstream LRPs we have and then allocate to the >> downstream LRPs from that larger delegated prefix. >> >> TODO: We are free to request whatever prefix-length we want, and >> the server is equally free to ignore our request and give us >> something else. So we need some generic handling of >> allocations where prefix-length < 64. Perhaps just split >> the prefix up in /64 chunks and add them all to >> ``ipv6_ra_prefixes`` ? >> >> TODO: tests >> >> QUESTION: From cursory view there appears to be an issue with >> routing inside OVN after successful IPv6 Prefix >> Delegation, it may be an issue with my setup, but >> thought I might as well ask. Is the prefix delegation >> code supposed to automatically handle internal routing? > > I apparently can't quite let go of this itch :) > I'm not aware of this feature being used actively in production (I might be wrong of course) so maybe that's why there are these loose ends. We should probably fix them though, especially if it's interesting for your own usage. > Responding to my own question, there does appear to be missing > functionality in a few more areas to make IPv6 Prefix Delegation > support complete: > 1) We need to include the learned ipv6_prefix when building > lr_in_ip_routing logical flows, something like this does it: > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index ee5cbcdc3..94ebf6488 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -344,6 +344,22 @@ extract_lrp_networks(const struct > nbrec_logical_router_port *lrp, > } > } > > + for (int i = 0; i < lrp->n_ipv6_prefix; i++) { > + struct in6_addr ip6; > + unsigned int plen; > + char *error; > + > + error = ipv6_parse_cidr(lrp->ipv6_prefix[i], &ip6, &plen); > + if (!error) { > + add_ipv6_netaddr(laddrs, ip6, plen); > + } else { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_INFO_RL(&rl, "invalid syntax '%s' in ipv6_prefix", > + lrp->networks[i]); > + free(error); > + } > + } > + > > --- > > I just piggy backed on extract_lrp_networks() here, but it might > deserve its own extract_lrp_delegated_ipv6_prefix() or something like > that for clarity? > Maybe, given that the rest of the network configuration doesn't change. > 2) Traffic is currently allowed to pass through port security, > presumably due to ND/MAC learning, but we should probably let IPAM > know about the learned prefixes so that dynamic addresses can be > properly populated? > Do you mean the DHCPv6 traffic or traffic using those IPs? If it's the latter it sounds a bit weird to me. I'd expect indeed that we need to inform IPAM about the learned prefixes. > 3) I need to manually add an external v6 default route, but as a user > I would expect this to be learned through RA when IPv6 Prefix > Delegation is enabled. Maybe we should make this automatic? > Sounds reasonable to me. >> Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com> >> --- >> controller/pinctrl.c | 35 ++++++++++++++++++++++++----------- >> 1 file changed, 24 insertions(+), 11 deletions(-) >> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> index 721a737de..8c4425a2e 100644 >> --- a/controller/pinctrl.c >> +++ b/controller/pinctrl.c >> @@ -1120,9 +1120,7 @@ compose_prefixd_packet(struct dp_packet *b, struct ipv6_prefixd_state *pfd) >> if (pfd->uuid.len) { >> payload += pfd->uuid.len + sizeof(struct dhcpv6_opt_header); >> } >> - if (ipv6_addr_is_set(&pfd->prefix)) { >> - payload += sizeof(struct dhcpv6_opt_ia_prefix); >> - } >> + payload += sizeof(struct dhcpv6_opt_ia_prefix); >> >> eth_compose(b, (struct eth_addr) ETH_ADDR_C(33,33,00,01,00,02), pfd->ea, >> ETH_TYPE_IPV6, IPV6_HEADER_LEN); >> @@ -1174,23 +1172,38 @@ compose_prefixd_packet(struct dp_packet *b, struct ipv6_prefixd_state *pfd) >> ia_pd->opt.code = htons(DHCPV6_OPT_IA_PD); >> int opt_len = sizeof(struct dhcpv6_opt_ia_na) - >> sizeof(struct dhcpv6_opt_header); >> - if (ipv6_addr_is_set(&pfd->prefix)) { >> - opt_len += sizeof(struct dhcpv6_opt_ia_prefix); >> - } >> + opt_len += sizeof(struct dhcpv6_opt_ia_prefix); >> ia_pd->opt.len = htons(opt_len); >> ia_pd->iaid = htonl(pfd->aid); >> ia_pd->t1 = OVS_BE32_MAX; >> ia_pd->t2 = OVS_BE32_MAX; >> + struct dhcpv6_opt_ia_prefix *ia_prefix = >> + (struct dhcpv6_opt_ia_prefix *)(ia_pd + 1); >> + ia_prefix->opt.code = htons(DHCPV6_OPT_IA_PREFIX); >> + ia_prefix->opt.len = htons(sizeof(struct dhcpv6_opt_ia_prefix) - >> + sizeof(struct dhcpv6_opt_header)); >> if (ipv6_addr_is_set(&pfd->prefix)) { >> - struct dhcpv6_opt_ia_prefix *ia_prefix = >> - (struct dhcpv6_opt_ia_prefix *)(ia_pd + 1); >> - ia_prefix->opt.code = htons(DHCPV6_OPT_IA_PREFIX); >> - ia_prefix->opt.len = htons(sizeof(struct dhcpv6_opt_ia_prefix) - >> - sizeof(struct dhcpv6_opt_header)); >> ia_prefix->plife_time = OVS_BE32_MAX; >> ia_prefix->vlife_time = OVS_BE32_MAX; >> ia_prefix->plen = pfd->plen; >> ia_prefix->ipv6 = pfd->prefix; >> + } else { >> + /* The prefix we obtain is used to fill the ``ipv6_ra_prefixes`` >> + * option for configuration of instances through SLAAC. >> + * >> + * As discussed in RFC 7421 the interface identifier is 64 bits long, >> + * and client implementations refrain from performing SLAAC with any >> + * other prefix length. >> + * >> + * The DHCPv6 server is at liberty to offer whichever prefix length it >> + * wants, so make sure new requests are for a prefix length of 64 so >> + * that it is suitable. >> + * >> + * A future improvement might be to adjust the requested prefix length >> + * to the number of downstream LRPs we have and then allocate to the >> + * downstream LRPs from that larger delegated prefix. */ >> + ia_prefix->ipv6 = in6addr_any; >> + ia_prefix->plen = 64; >> } >> >> uint32_t csum = packet_csum_pseudoheader6(dp_packet_l3(b)); Regards, Dumitru
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 721a737de..8c4425a2e 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -1120,9 +1120,7 @@ compose_prefixd_packet(struct dp_packet *b, struct ipv6_prefixd_state *pfd) if (pfd->uuid.len) { payload += pfd->uuid.len + sizeof(struct dhcpv6_opt_header); } - if (ipv6_addr_is_set(&pfd->prefix)) { - payload += sizeof(struct dhcpv6_opt_ia_prefix); - } + payload += sizeof(struct dhcpv6_opt_ia_prefix); eth_compose(b, (struct eth_addr) ETH_ADDR_C(33,33,00,01,00,02), pfd->ea, ETH_TYPE_IPV6, IPV6_HEADER_LEN); @@ -1174,23 +1172,38 @@ compose_prefixd_packet(struct dp_packet *b, struct ipv6_prefixd_state *pfd) ia_pd->opt.code = htons(DHCPV6_OPT_IA_PD); int opt_len = sizeof(struct dhcpv6_opt_ia_na) - sizeof(struct dhcpv6_opt_header); - if (ipv6_addr_is_set(&pfd->prefix)) { - opt_len += sizeof(struct dhcpv6_opt_ia_prefix); - } + opt_len += sizeof(struct dhcpv6_opt_ia_prefix); ia_pd->opt.len = htons(opt_len); ia_pd->iaid = htonl(pfd->aid); ia_pd->t1 = OVS_BE32_MAX; ia_pd->t2 = OVS_BE32_MAX; + struct dhcpv6_opt_ia_prefix *ia_prefix = + (struct dhcpv6_opt_ia_prefix *)(ia_pd + 1); + ia_prefix->opt.code = htons(DHCPV6_OPT_IA_PREFIX); + ia_prefix->opt.len = htons(sizeof(struct dhcpv6_opt_ia_prefix) - + sizeof(struct dhcpv6_opt_header)); if (ipv6_addr_is_set(&pfd->prefix)) { - struct dhcpv6_opt_ia_prefix *ia_prefix = - (struct dhcpv6_opt_ia_prefix *)(ia_pd + 1); - ia_prefix->opt.code = htons(DHCPV6_OPT_IA_PREFIX); - ia_prefix->opt.len = htons(sizeof(struct dhcpv6_opt_ia_prefix) - - sizeof(struct dhcpv6_opt_header)); ia_prefix->plife_time = OVS_BE32_MAX; ia_prefix->vlife_time = OVS_BE32_MAX; ia_prefix->plen = pfd->plen; ia_prefix->ipv6 = pfd->prefix; + } else { + /* The prefix we obtain is used to fill the ``ipv6_ra_prefixes`` + * option for configuration of instances through SLAAC. + * + * As discussed in RFC 7421 the interface identifier is 64 bits long, + * and client implementations refrain from performing SLAAC with any + * other prefix length. + * + * The DHCPv6 server is at liberty to offer whichever prefix length it + * wants, so make sure new requests are for a prefix length of 64 so + * that it is suitable. + * + * A future improvement might be to adjust the requested prefix length + * to the number of downstream LRPs we have and then allocate to the + * downstream LRPs from that larger delegated prefix. */ + ia_prefix->ipv6 = in6addr_any; + ia_prefix->plen = 64; } uint32_t csum = packet_csum_pseudoheader6(dp_packet_l3(b));
The prefix we obtain is used to fill the ``ipv6_ra_prefixes`` option for configuration of instances through SLAAC. As discussed in RFC 7421 the interface identifier is 64 bits long, and client implementations refrain from performing SLAAC with any other prefix length. TODO: I'm pretty sure the origin of this behavior is tied to an older RFC, attempt to track it down. The DHCPv6 server is at liberty to offer whichever prefix length it wants, so make sure new requests are for a prefix length of 64 so that it is suitable. TODO: We may need to make the default prefix-length to request configurable? Setting this to 64 fixes an issue I see in my network where the server will delegate a /62 by default, which does not work well for SLAAC. A future improvement might be to adjust the requested prefix length to the number of downstream LRPs we have and then allocate to the downstream LRPs from that larger delegated prefix. TODO: We are free to request whatever prefix-length we want, and the server is equally free to ignore our request and give us something else. So we need some generic handling of allocations where prefix-length < 64. Perhaps just split the prefix up in /64 chunks and add them all to ``ipv6_ra_prefixes`` ? TODO: tests QUESTION: From cursory view there appears to be an issue with routing inside OVN after successful IPv6 Prefix Delegation, it may be an issue with my setup, but thought I might as well ask. Is the prefix delegation code supposed to automatically handle internal routing? Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com> --- controller/pinctrl.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-)