Re: [ovs-dev] [PATCH v6 ovn 1/2] controller: add ipv6 prefix delegation state machine
> "
>
> On Tue, Jan 14, 2020 at 4:37 PM Lorenzo Bianconi
> wrote:
> >
> > Introduce IPv6 Prefix delegation state machine according to RFC 3633
> > https://tools.ietf.org/html/rfc3633.
> > Add handle_dhcpv6_reply controller action to parse advertise/reply from
> > IPv6 delegation server. Advertise/reply are parsed running respectively:
> > - pinctrl_parse_dhcv6_advt
> > - pinctrl_parse_dhcv6_reply
> > The IPv6 requesting router starts sending dhcpv6 solicit through the logical
> > router port marked with ipv6_prefix_delegation set to true.
> > An IPv6 prefix will be requested for each logical router port marked
> > with "prefix" set to true in option column of logical router port table.
> > Save IPv6 prefix received by IPv6 delegation router in the options columns
> > of
> > SB port binding table in order to be reused by Router Advertisement
> > framework
> > run by ovn logical router pipeline.
> > IPv6 Prefix delegation state machine is enabled on Gateway Router or on
> > a Gateway Router Port
> >
> > Signed-off-by: Lorenzo Bianconi
>
>
> Hi Lorenzo,
> Thanks for the v6.
Hi Numan,
thx for the reivew and sorry for the delay
>
> I tested this series. Below are some comments
>
> - When I configured the prefix options on the router ports,
> ovn-controller hosting the gateway
>router port sends the PD solicit message and gets the prefix, but
> it never sends the renew/Solicit
>requests periodically. In the code I see that the next announce
> time is chosen. But looks like that is not
> working. I think it's better to use the "prefix lifetime option"
> value sent by the delegating server rather than
> current_time + random(3000) which this patch does.
renew message timeout is set based on t1 field of IA_PD option received
from the delation router. According to the RFC3633 (pag 8):
"The time at which the requesting router should contact the delegating router
from which the prefixes in the IA_PD were obtained to extend the lifetimes
of the prefixes delegated to the IA_PD"
Do you think we should use preferred-lifetime/valid-lifetime fields?
>
> - Suppose if a router port had received a prefix 'P1' and if I restart
> ovn-controller, ovn-controller
> sends the PD messages and gets another prefix 'P2'. Ideally
> ovn-controller should try to renew
> the existing prefix. Can you please check if it is possible to
> include the prefix option in the Request message ?
> Probably ovn-controller can send a Renew request if a router port
> has already a prefix ? If the delegating server,
> can't allocate the same prefix, ovn-controller can start the fresh
> process of sending Solicit message.
ack, I will fix it in v7
>
> - In the above case, I notice that when delegating server sends 'P2',
> ovn-controller doesn't update this new prefix
> in the port_binding.options:ipv6_ra_pd_list. And the patch 2 of this
> series, doesn't update the "ipv6_prefix column
> of logical router port. I think it's better for ovn-northd to just
> update/reset the "ipv6_prefix" column from the
> port_binding.options:ipv6_ra_pd_list, rather than appending the value.
ack, I will fix it in v7
>
> - The action handle_dhcp6_reply takes inner actions. But this is not
> documented.
>I think it's better not to include the inner actions. When the
> delegating server sends the "reply" message, ovn-controller
>doesn't need to send any reply. But since this action is used for
> handling the "Advertise" and "Reply" messages from
>the delegating router, it doesn't seem appropriate to include the
> inner actions.
>
> Please see some more comments below
>
>
> > ---
> > controller/ovn-controller.c | 1 +
> > controller/pinctrl.c| 612 +++-
> > controller/pinctrl.h| 2 +
> > include/ovn/actions.h | 8 +-
> > lib/actions.c | 22 ++
> > lib/ovn-l7.h| 19 ++
> > ovn-sb.xml | 8 +
> > tests/ovn.at| 6 +
> > utilities/ovn-trace.c | 3 +
> > 9 files changed, 673 insertions(+), 8 deletions(-)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 17744d416..d559e845e 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -2145,6 +2145,7 @@ main(int argc, char *argv[])
> > runtime_data = engine_get_data(_runtime_data);
> > if (runtime_data) {
> > pinctrl_run(ovnsb_idl_txn,
> > +ovnsb_idl_loop.idl,
> > sbrec_datapath_binding_by_key,
> > sbrec_port_binding_by_datapath,
> > sbrec_port_binding_by_key,
[...]
> > +pinctrl_prefixd_state_handler(const struct flow *ip_flow,
> > + struct in6_addr addr, unsigned aid,
> > + char prefix_len,
Re: [ovs-dev] [PATCH v6 ovn 1/2] controller: add ipv6 prefix delegation state machine
"
On Tue, Jan 14, 2020 at 4:37 PM Lorenzo Bianconi
wrote:
>
> Introduce IPv6 Prefix delegation state machine according to RFC 3633
> https://tools.ietf.org/html/rfc3633.
> Add handle_dhcpv6_reply controller action to parse advertise/reply from
> IPv6 delegation server. Advertise/reply are parsed running respectively:
> - pinctrl_parse_dhcv6_advt
> - pinctrl_parse_dhcv6_reply
> The IPv6 requesting router starts sending dhcpv6 solicit through the logical
> router port marked with ipv6_prefix_delegation set to true.
> An IPv6 prefix will be requested for each logical router port marked
> with "prefix" set to true in option column of logical router port table.
> Save IPv6 prefix received by IPv6 delegation router in the options columns of
> SB port binding table in order to be reused by Router Advertisement framework
> run by ovn logical router pipeline.
> IPv6 Prefix delegation state machine is enabled on Gateway Router or on
> a Gateway Router Port
>
> Signed-off-by: Lorenzo Bianconi
Hi Lorenzo,
Thanks for the v6.
I tested this series. Below are some comments
- When I configured the prefix options on the router ports,
ovn-controller hosting the gateway
router port sends the PD solicit message and gets the prefix, but
it never sends the renew/Solicit
requests periodically. In the code I see that the next announce
time is chosen. But looks like that is not
working. I think it's better to use the "prefix lifetime option"
value sent by the delegating server rather than
current_time + random(3000) which this patch does.
- Suppose if a router port had received a prefix 'P1' and if I restart
ovn-controller, ovn-controller
sends the PD messages and gets another prefix 'P2'. Ideally
ovn-controller should try to renew
the existing prefix. Can you please check if it is possible to
include the prefix option in the Request message ?
Probably ovn-controller can send a Renew request if a router port
has already a prefix ? If the delegating server,
can't allocate the same prefix, ovn-controller can start the fresh
process of sending Solicit message.
- In the above case, I notice that when delegating server sends 'P2',
ovn-controller doesn't update this new prefix
in the port_binding.options:ipv6_ra_pd_list. And the patch 2 of this
series, doesn't update the "ipv6_prefix column
of logical router port. I think it's better for ovn-northd to just
update/reset the "ipv6_prefix" column from the
port_binding.options:ipv6_ra_pd_list, rather than appending the value.
- The action handle_dhcp6_reply takes inner actions. But this is not
documented.
I think it's better not to include the inner actions. When the
delegating server sends the "reply" message, ovn-controller
doesn't need to send any reply. But since this action is used for
handling the "Advertise" and "Reply" messages from
the delegating router, it doesn't seem appropriate to include the
inner actions.
Please see some more comments below
> ---
> controller/ovn-controller.c | 1 +
> controller/pinctrl.c| 612 +++-
> controller/pinctrl.h| 2 +
> include/ovn/actions.h | 8 +-
> lib/actions.c | 22 ++
> lib/ovn-l7.h| 19 ++
> ovn-sb.xml | 8 +
> tests/ovn.at| 6 +
> utilities/ovn-trace.c | 3 +
> 9 files changed, 673 insertions(+), 8 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 17744d416..d559e845e 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2145,6 +2145,7 @@ main(int argc, char *argv[])
> runtime_data = engine_get_data(_runtime_data);
> if (runtime_data) {
> pinctrl_run(ovnsb_idl_txn,
> +ovnsb_idl_loop.idl,
> sbrec_datapath_binding_by_key,
> sbrec_port_binding_by_datapath,
> sbrec_port_binding_by_key,
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 452ca8a1c..2a37fc525 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -270,6 +270,20 @@ static void pinctrl_ip_mcast_handle_igmp(
> const struct match *md,
> struct ofpbuf *userdata);
>
> +static void init_ipv6_prefixd(void);
> +static void destroy_ipv6_prefixd(void);
> +static void ipv6_prefixd_wait(long long int timeout);
> +static void
> +prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn,
> + struct ovsdb_idl *ovnsb_idl,
> + struct ovsdb_idl_index *sbrec_port_binding_by_name,
> + const struct hmap *local_datapaths,
> + const struct sbrec_chassis *chassis,
> + const struct sset *active_tunnels)
> +OVS_REQUIRES(pinctrl_mutex);
> +static void
> +send_ipv6_prefixd(struct rconn *swconn, long
[ovs-dev] [PATCH v6 ovn 1/2] controller: add ipv6 prefix delegation state machine
Introduce IPv6 Prefix delegation state machine according to RFC 3633
https://tools.ietf.org/html/rfc3633.
Add handle_dhcpv6_reply controller action to parse advertise/reply from
IPv6 delegation server. Advertise/reply are parsed running respectively:
- pinctrl_parse_dhcv6_advt
- pinctrl_parse_dhcv6_reply
The IPv6 requesting router starts sending dhcpv6 solicit through the logical
router port marked with ipv6_prefix_delegation set to true.
An IPv6 prefix will be requested for each logical router port marked
with "prefix" set to true in option column of logical router port table.
Save IPv6 prefix received by IPv6 delegation router in the options columns of
SB port binding table in order to be reused by Router Advertisement framework
run by ovn logical router pipeline.
IPv6 Prefix delegation state machine is enabled on Gateway Router or on
a Gateway Router Port
Signed-off-by: Lorenzo Bianconi
---
controller/ovn-controller.c | 1 +
controller/pinctrl.c| 612 +++-
controller/pinctrl.h| 2 +
include/ovn/actions.h | 8 +-
lib/actions.c | 22 ++
lib/ovn-l7.h| 19 ++
ovn-sb.xml | 8 +
tests/ovn.at| 6 +
utilities/ovn-trace.c | 3 +
9 files changed, 673 insertions(+), 8 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 17744d416..d559e845e 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2145,6 +2145,7 @@ main(int argc, char *argv[])
runtime_data = engine_get_data(_runtime_data);
if (runtime_data) {
pinctrl_run(ovnsb_idl_txn,
+ovnsb_idl_loop.idl,
sbrec_datapath_binding_by_key,
sbrec_port_binding_by_datapath,
sbrec_port_binding_by_key,
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 452ca8a1c..2a37fc525 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -270,6 +270,20 @@ static void pinctrl_ip_mcast_handle_igmp(
const struct match *md,
struct ofpbuf *userdata);
+static void init_ipv6_prefixd(void);
+static void destroy_ipv6_prefixd(void);
+static void ipv6_prefixd_wait(long long int timeout);
+static void
+prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn,
+ struct ovsdb_idl *ovnsb_idl,
+ struct ovsdb_idl_index *sbrec_port_binding_by_name,
+ const struct hmap *local_datapaths,
+ const struct sbrec_chassis *chassis,
+ const struct sset *active_tunnels)
+OVS_REQUIRES(pinctrl_mutex);
+static void
+send_ipv6_prefixd(struct rconn *swconn, long long int *send_prefixd_time)
+OVS_REQUIRES(pinctrl_mutex);
static bool may_inject_pkts(void);
static void init_put_vport_bindings(void);
@@ -457,6 +471,7 @@ pinctrl_init(void)
init_put_mac_bindings();
init_send_garps_rarps();
init_ipv6_ras();
+init_ipv6_prefixd();
init_buffered_packets_map();
init_event_table();
ip_mcast_snoop_init();
@@ -544,6 +559,49 @@ set_actions_and_enqueue_msg(struct rconn *swconn,
ofpbuf_uninit();
}
+static struct shash ipv6_prefixd;
+
+enum {
+PREFIX_SOLICIT,
+PREFIX_PENDING,
+PREFIX_DONE,
+};
+
+struct ipv6_prefixd_state {
+long long int next_announce;
+struct in6_addr ipv6_addr;
+struct eth_addr ea;
+struct eth_addr cmac;
+int64_t port_key;
+int64_t metadata;
+struct in6_addr prefix;
+unsigned plife_time;
+unsigned vlife_time;
+unsigned aid;
+unsigned t1;
+unsigned t2;
+int8_t plen;
+int state;
+};
+
+static void
+init_ipv6_prefixd(void)
+{
+shash_init(_prefixd);
+}
+
+static void
+destroy_ipv6_prefixd(void)
+{
+struct shash_node *iter, *next;
+SHASH_FOR_EACH_SAFE (iter, next, _prefixd) {
+struct ipv6_prefixd_state *pfd = iter->data;
+free(pfd);
+shash_delete(_prefixd, iter);
+}
+shash_destroy(_prefixd);
+}
+
struct buffer_info {
struct ofpbuf ofpacts;
struct dp_packet *p;
@@ -967,6 +1025,259 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const
struct flow *ip_flow,
dp_packet_uninit();
}
+static void
+pinctrl_parse_dhcpv6_advt(struct rconn *swconn, const struct flow *ip_flow,
+ struct dp_packet *pkt_in, const struct match *md,
+ struct ofpbuf *userdata)
+{
+struct udp_header *udp_in = dp_packet_l4(pkt_in);
+size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in));
+unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1);
+uint8_t *data, *end = (uint8_t *)udp_in + dlen;
+int len = 0;
+
+data = xmalloc(dlen);
+/* skip DHCPv6 common header */
+in_dhcpv6_data += 4;
+while (in_dhcpv6_data < end) {
+
