On Mon, Oct 12, 2020 at 2:36 PM Lorenzo Bianconi
<[email protected]> wrote:
>
> Introduce RENEW/REBIND message support to OVN IPv6 PD support
> according to RFC-3633 [0] in order to renew IPv6 prefixes when
> T1/T2 are elapsed
>
> [0] https://tools.ietf.org/html/rfc3633
>
> Acked-by: Mark Michelson <[email protected]>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
Thanks Lorenzo and Mark.
I applied this patch to master with one change. Please see below.
Thanks
Numan
> ---
> Changes since v1:
> - fixed compilation warning
> ---
> controller/pinctrl.c | 131 +++++++++++++++++++++++++++++++++----------
> lib/ovn-l7.h | 3 +
> tests/system-ovn.at | 18 ++++--
> 3 files changed, 118 insertions(+), 34 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 0a7020533..c1ef1610f 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -573,11 +573,22 @@ enum {
> PREFIX_REQUEST,
> PREFIX_PENDING,
> PREFIX_DONE,
> + PREFIX_RENEW,
> + PREFIX_REBIND,
> };
>
> struct ipv6_prefixd_state {
> long long int next_announce;
> + long long int last_complete;
> long long int last_used;
> + /* IPv6 PD server info */
> + struct in6_addr server_addr;
> + struct eth_addr sa;
> + /* server_id_info */
> + struct {
> + uint8_t *data;
> + uint8_t len;
> + } uuid;
> struct in6_addr ipv6_addr;
> struct eth_addr ea;
> struct eth_addr cmac;
> @@ -781,20 +792,26 @@ out:
> static void
> pinctrl_prefixd_state_handler(const struct flow *ip_flow,
> struct in6_addr addr, unsigned aid,
> + struct eth_addr sa, struct in6_addr
> server_addr,
> char prefix_len, unsigned t1, unsigned t2,
> - unsigned plife_time, unsigned vlife_time)
> + unsigned plife_time, unsigned vlife_time,
> + uint8_t *uuid, uint8_t uuid_len)
> {
> struct ipv6_prefixd_state *pfd;
>
> pfd = pinctrl_find_prefixd_state(ip_flow, aid);
> if (pfd) {
> pfd->state = PREFIX_PENDING;
> - pfd->plife_time = plife_time;
> - pfd->vlife_time = vlife_time;
> + pfd->server_addr = server_addr;
> + pfd->sa = sa;
> + pfd->uuid.data = uuid;
> + pfd->uuid.len = uuid_len;
> + pfd->plife_time = plife_time * 1000;
> + pfd->vlife_time = vlife_time * 1000;
> pfd->plen = prefix_len;
> pfd->prefix = addr;
> - pfd->t1 = t1;
> - pfd->t2 = t2;
> + pfd->t1 = t1 * 1000;
> + pfd->t2 = t2 * 1000;
> notify_pinctrl_main();
> }
> }
> @@ -804,19 +821,21 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in,
> const struct flow *ip_flow)
> OVS_REQUIRES(pinctrl_mutex)
> {
> + struct eth_header *eth = dp_packet_eth(pkt_in);
> + struct ip6_hdr *in_ip = dp_packet_l3(pkt_in);
> struct udp_header *udp_in = dp_packet_l4(pkt_in);
> unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1);
> size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in));
> unsigned t1 = 0, t2 = 0, vlife_time = 0, plife_time = 0;
> - uint8_t *end = (uint8_t *)udp_in + dlen;
> - uint8_t prefix_len = 0;
> - struct in6_addr ipv6;
> + uint8_t *end = (uint8_t *)udp_in + dlen, *uuid = NULL;
> + uint8_t prefix_len = 0, uuid_len = 0;
> + struct in6_addr ipv6 = in6addr_any;
> bool status = false;
> unsigned aid = 0;
>
> - memset(&ipv6, 0, sizeof (struct in6_addr));
> /* skip DHCPv6 common header */
> in_dhcpv6_data += 4;
> +
> while (in_dhcpv6_data < end) {
> struct dhcpv6_opt_header *in_opt =
> (struct dhcpv6_opt_header *)in_dhcpv6_data;
> @@ -867,14 +886,20 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in,
> }
> break;
> }
> + case DHCPV6_OPT_SERVER_ID_CODE:
> + uuid_len = ntohs(in_opt->len);
> + uuid = xmalloc(uuid_len);
> + memcpy(uuid, in_opt + 1, uuid_len);
> + break;
> default:
> break;
> }
> in_dhcpv6_data += opt_len;
> }
> if (status) {
> - pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, prefix_len,
> - t1, t2, plife_time, vlife_time);
> + pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src,
> + in_ip->ip6_src, prefix_len, t1, t2,
> + plife_time, vlife_time, uuid,
> uuid_len);
> }
If status is false and uuid is allocated memory, there is a chance of
memory leak.
I applied this patch with the below incremental change.
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index c1ef1610f2..7099687ebc 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -900,6 +900,8 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in,
pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src,
in_ip->ip6_src, prefix_len, t1, t2,
plife_time, vlife_time, uuid, uuid_len);
+ } else if (uuid) {
+ free(uuid);
}
}
> }
>
> @@ -904,27 +929,42 @@ pinctrl_handle_dhcp6_server(struct rconn *swconn, const
> struct flow *ip_flow,
> }
>
> static void
> -compose_prefixd_solicit(struct dp_packet *b,
> - struct ipv6_prefixd_state *pfd,
> - const struct eth_addr eth_dst,
> - const struct in6_addr *ipv6_dst)
> +compose_prefixd_packet(struct dp_packet *b, struct ipv6_prefixd_state *pfd)
> {
> - eth_compose(b, eth_dst, pfd->ea, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
> + struct in6_addr ipv6_dst;
> + struct eth_addr eth_dst;
>
> int payload = sizeof(struct dhcpv6_opt_server_id) +
> sizeof(struct dhcpv6_opt_ia_na);
> + if (pfd->uuid.len) {
> + payload += pfd->uuid.len + sizeof(struct dhcpv6_opt_header);
> + ipv6_dst = pfd->server_addr;
> + eth_dst = pfd->sa;
> + } else {
> + eth_dst = (struct eth_addr) ETH_ADDR_C(33,33,00,01,00,02);
> + ipv6_parse("ff02::1:2", &ipv6_dst);
> + }
> if (ipv6_addr_is_set(&pfd->prefix)) {
> payload += sizeof(struct dhcpv6_opt_ia_prefix);
> }
> +
> + eth_compose(b, eth_dst, pfd->ea, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
> +
> int len = UDP_HEADER_LEN + 4 + payload;
> struct udp_header *udp_h = compose_ipv6(b, IPPROTO_UDP, &pfd->ipv6_addr,
> - ipv6_dst, 0, 0, 255, len);
> + &ipv6_dst, 0, 0, 255, len);
> udp_h->udp_len = htons(len);
> udp_h->udp_csum = 0;
> packet_set_udp_port(b, htons(546), htons(547));
>
> unsigned char *dhcp_hdr = (unsigned char *)(udp_h + 1);
> - *dhcp_hdr = DHCPV6_MSG_TYPE_SOLICIT;
> + if (pfd->state == PREFIX_RENEW) {
> + *dhcp_hdr = DHCPV6_MSG_TYPE_RENEW;
> + } else if (pfd->state == PREFIX_REBIND) {
> + *dhcp_hdr = DHCPV6_MSG_TYPE_REBIND;
> + } else {
> + *dhcp_hdr = DHCPV6_MSG_TYPE_SOLICIT;
> + }
>
> struct dhcpv6_opt_server_id *opt_client_id =
> (struct dhcpv6_opt_server_id *)(dhcp_hdr + 4);
> @@ -935,11 +975,21 @@ compose_prefixd_solicit(struct dp_packet *b,
> opt_client_id->hw_type = htons(DHCPV6_HW_TYPE_ETH);
> opt_client_id->mac = pfd->cmac;
>
> + unsigned char *ptr = (unsigned char *)(opt_client_id + 1);
> + if (pfd->uuid.len) {
> + struct dhcpv6_opt_header *in_opt = (struct dhcpv6_opt_header *)ptr;
> + in_opt->code = htons(DHCPV6_OPT_SERVER_ID_CODE);
> + in_opt->len = htons(pfd->uuid.len);
> +
> + ptr += sizeof *in_opt;
> + memcpy(ptr, pfd->uuid.data, pfd->uuid.len);
> + ptr += pfd->uuid.len;
> + }
> +
> if (!ipv6_addr_is_set(&pfd->prefix)) {
> pfd->aid = random_uint16();
> }
> - struct dhcpv6_opt_ia_na *ia_pd =
> - (struct dhcpv6_opt_ia_na *)(opt_client_id + 1);
> + struct dhcpv6_opt_ia_na *ia_pd = (struct dhcpv6_opt_ia_na *)ptr;
> ia_pd->opt.code = htons(DHCPV6_OPT_IA_PD);
> int opt_len = sizeof(struct dhcpv6_opt_ia_na) -
> sizeof(struct dhcpv6_opt_header);
> @@ -981,16 +1031,15 @@ ipv6_prefixd_send(struct rconn *swconn, struct
> ipv6_prefixd_state *pfd)
> return pfd->next_announce;
> }
>
> + if (pfd->state == PREFIX_DONE) {
> + goto out;
> + }
> +
> uint64_t packet_stub[256 / 8];
> struct dp_packet packet;
>
> - struct eth_addr eth_dst;
> - eth_dst = (struct eth_addr) ETH_ADDR_C(33,33,00,01,00,02);
> - struct in6_addr ipv6_dst;
> - ipv6_parse("ff02::1:2", &ipv6_dst);
> -
> dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
> - compose_prefixd_solicit(&packet, pfd, eth_dst, &ipv6_dst);
> + compose_prefixd_packet(&packet, pfd);
>
> uint64_t ofpacts_stub[4096 / 8];
> struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
> @@ -1019,8 +1068,9 @@ ipv6_prefixd_send(struct rconn *swconn, struct
> ipv6_prefixd_state *pfd)
> queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
> dp_packet_uninit(&packet);
> ofpbuf_uninit(&ofpacts);
> +
> +out:
> pfd->next_announce = cur_time + random_range(IPV6_PREFIXD_TIMEOUT);
> - pfd->state = PREFIX_SOLICIT;
>
> return pfd->next_announce;
> }
> @@ -1031,10 +1081,28 @@ static bool ipv6_prefixd_should_inject(void)
>
> SHASH_FOR_EACH (iter, &ipv6_prefixd) {
> struct ipv6_prefixd_state *pfd = iter->data;
> + long long int cur_time = time_msec();
> +
> if (pfd->state == PREFIX_SOLICIT) {
> return true;
> }
> - if (pfd->state && pfd->next_announce < time_msec()) {
> + if (pfd->state == PREFIX_DONE &&
> + cur_time > pfd->last_complete + pfd->t1) {
> + pfd->state = PREFIX_RENEW;
> + return true;
> + }
> + if (pfd->state == PREFIX_RENEW &&
> + cur_time > pfd->last_complete + pfd->t2) {
> + pfd->state = PREFIX_REBIND;
> + if (pfd->uuid.len) {
> + free(pfd->uuid.data);
> + pfd->uuid.len = 0;
> + }
> + return true;
> + }
> + if (pfd->state == PREFIX_REBIND &&
> + cur_time > pfd->last_complete + pfd->vlife_time) {
> + pfd->state = PREFIX_SOLICIT;
> return true;
> }
> }
> @@ -1120,7 +1188,8 @@ fill_ipv6_prefix_state(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> struct smap options;
>
> pfd->state = PREFIX_DONE;
> - pfd->next_announce = time_msec() + pfd->t1 * 1000;
> + pfd->last_complete = time_msec();
> + pfd->next_announce = pfd->last_complete + pfd->t1;
> ipv6_string_mapped(prefix_str, &pfd->prefix);
> smap_clone(&options, &pb->options);
> smap_add_format(&options, "ipv6_ra_pd_list", "%d:%s/%d",
> @@ -1219,6 +1288,10 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> SHASH_FOR_EACH_SAFE (iter, next, &ipv6_prefixd) {
> struct ipv6_prefixd_state *pfd = iter->data;
> if (pfd->last_used + IPV6_PREFIXD_STALE_TIMEOUT < time_msec()) {
> + if (pfd->uuid.len) {
> + free(pfd->uuid.data);
> + pfd->uuid.len = 0;
> + }
> free(pfd);
> shash_delete(&ipv6_prefixd, iter);
> }
> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> index 18c59896d..30a795531 100644
> --- a/lib/ovn-l7.h
> +++ b/lib/ovn-l7.h
> @@ -189,6 +189,9 @@ struct dhcp_opt6_header {
> #define DHCPV6_MSG_TYPE_ADVT 2
> #define DHCPV6_MSG_TYPE_REQUEST 3
> #define DHCPV6_MSG_TYPE_CONFIRM 4
> +#define DHCPV6_MSG_TYPE_RENEW 5
> +#define DHCPV6_MSG_TYPE_REBIND 6
> +
> #define DHCPV6_MSG_TYPE_REPLY 7
> #define DHCPV6_MSG_TYPE_DECLINE 9
> #define DHCPV6_MSG_TYPE_INFO_REQ 11
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 420610f89..60bd20fd4 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -4768,19 +4768,28 @@ AT_CHECK([ovn-nbctl get logical_router_port rp-sw1
> ipv6_prefix | cut -c3-16], [0
> [2001:1db8:3333]
> ])
>
> -kill $(pidof dibbler-server)
> -
> prefix=$(ovn-nbctl list logical_router_port rp-public | awk -F/
> '/ipv6_prefix/{print substr($1,25,9)}' | sed 's/://g')
> ovn-nbctl set logical_router_port rp-sw0 options:prefix=false
> ovn-nbctl set logical_router_port rp-sw1 options:prefix=false
>
> -NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[95:4]]=0x${prefix} >
> public.pcap &])
> +# Renew message
> +NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x05 and
> ip6[[113:4]]=0x${prefix} > renew.pcap &])
> +# Reply message with Status OK
> +NS_CHECK_EXEC([server], [tcpdump -c 1 -nni s1 ip6[[48:1]]=0x07 and
> ip6[[81:4]]=0x${prefix} and ip6[[98:1]]=0x0d and ip6[[101:2]]=0x0000 >
> reply.pcap &])
> +
> +OVS_WAIT_UNTIL([
> + total_pkts=$(cat renew.pcap | wc -l)
> + test "${total_pkts}" = "1"
> +])
>
> OVS_WAIT_UNTIL([
> - total_pkts=$(cat public.pcap | wc -l)
> + total_pkts=$(cat reply.pcap | wc -l)
> test "${total_pkts}" = "1"
> ])
>
> +kill $(pidof dibbler-server)
> +kill $(pidof tcpdump)
> +
> ovn-nbctl set logical_router_port rp-sw0 options:prefix=false
> ovn-nbctl clear logical_router_port rp-sw0 ipv6_prefix
> OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix
> | cut -c3-16)" = "[2001:1db8:3333]"])
> @@ -4788,7 +4797,6 @@ AT_CHECK([ovn-nbctl get logical_router_port rp-sw0
> ipv6_prefix | cut -c3-16], [0
> []
> ])
>
> -kill $(pidof tcpdump)
> kill $(pidof ovn-controller)
>
> as ovn-sb
> --
> 2.26.2
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
