On Mon, Apr 03, 2017 at 04:10:15PM +0530, nusid...@redhat.com wrote: > From: Numan Siddique <nusid...@redhat.com> > > This patch adds a new OVN action 'dns_lookup' to support native DNS. > ovn-controller parses this action and adds a NXT_PACKET_IN2 > OF flow with 'pause' flag set. > > A new table 'DNS' is added in the SB DB to look up and resolve > the DNS queries. When a valid DNS packet is received by > ovn-controller, it looks up the DNS name in the 'DNS' table > and if successful, it frames a DNS reply, resumes the packet > and stores 1 in the 1-bit subfield. If the packet is invalid > or cannot be resolved, it resumes the packet without any > modifications and stores 0 in the 1-bit subfield. > > reg0[4] = dns_lookup(); next; > > An upcoming patch will use this action and adds logical flows. > > Signed-off-by: Numan Siddique <nusid...@redhat.com>
Thanks for revising this! DNS is important and it goes one step closer to offering everything that OpenStack needs from OVN. I think that Guru is planning to review this too, particularly regarding the database schema (and its documentation, I hope), but I'll offer some preliminary comments of my own. I tend to think that the DNS protocol declarations should go in lib/packets.h, where OVS generally keeps protocol declarations, instead of ovn/lib/ovn-util.h. I think that the ovn-trace implementation of dns_lookup should at least set the destination bit to 0, to indicate that the lookup failed. I suggest folding in the following incremental to make the code easier to read. (I haven't tested it.) Thanks, Ben. --8<--------------------------cut here-------------------------->8-- diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index 9c669c96884a..e349817e7f4c 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -662,6 +662,18 @@ exit: } static void +put_be16(struct ofpbuf *buf, ovs_be16 x) +{ + ofpbuf_put(buf, &x, sizeof x); +} + +static void +put_be32(struct ofpbuf *buf, ovs_be32 x) +{ + ofpbuf_put(buf, &x, sizeof x); +} + +static void pinctrl_handle_dns_lookup( struct dp_packet *pkt_in, struct ofputil_packet_in *pin, struct ofpbuf *userdata, struct ofpbuf *continuation, @@ -722,27 +734,19 @@ pinctrl_handle_dns_lookup( /* Extract the hostname. If the hostname is - 'www.ovn.org' it would be * encoded as (in hex) - 03 77 77 77 03 6f 76 63 03 6f 72 67 00. */ - bool hostname_present = true; while ((in_dns_data + idx) < end && in_dns_data[idx]) { uint8_t label_len = in_dns_data[idx++]; if (in_dns_data + idx + label_len > end) { - hostname_present = false; - break; - } - for (uint8_t i = 0; i < label_len; i++) { - ds_put_char(&hostname, in_dns_data[idx++]); + ds_destroy(&hostname); + goto exit; } + ds_put_buffer(&hostname, (const char *) in_dns_data + idx, label_len); + idx += label_len; ds_put_char(&hostname, '.'); } - if (!hostname_present) { - ds_destroy(&hostname); - goto exit; - } - idx++; ds_chomp(&hostname, '.'); - ds_put_char(&hostname, 0); in_dns_data += idx; /* Query should have TYPE and CLASS fields */ @@ -809,35 +813,25 @@ pinctrl_handle_dns_lookup( * be IP address of the domain name. */ ofpbuf_put(&dns_answer, in_hostname, idx); - ovs_be16 v = htons(DNS_QUERY_TYPE_A); - ofpbuf_put(&dns_answer, &v, sizeof(ovs_be16)); - v = htons(DNS_CLASS_IN); - ofpbuf_put(&dns_answer, &v, sizeof(ovs_be16)); - ovs_be32 ttl = htonl(DNS_DEFAULT_RR_TTL); - ofpbuf_put(&dns_answer, &ttl, sizeof(ovs_be32)); - v = htons(sizeof(ovs_be32)); /* Length of the RDATA field */ - ofpbuf_put(&dns_answer, &v, sizeof(ovs_be16)); - ofpbuf_put(&dns_answer, &ip_addrs.ipv4_addrs[i].addr, - sizeof(ovs_be32)); + put_be16(&dns_answer, htons(DNS_QUERY_TYPE_A)); + put_be16(&dns_answer, htons(DNS_CLASS_IN)); + put_be32(&dns_answer, htonl(DNS_DEFAULT_RR_TTL)); + put_be16(&dns_answer, htons(sizeof(ovs_be32))); + put_be32(&dns_answer, ip_addrs.ipv4_addrs[i].addr); ancount++; } } if (query_type == DNS_QUERY_TYPE_AAAA || - query_type == DNS_QUERY_TYPE_ANY) { + query_type == DNS_QUERY_TYPE_ANY) { for (size_t i = 0; i < ip_addrs.n_ipv6_addrs; i++) { ofpbuf_put(&dns_answer, in_hostname, idx); - ovs_be16 v = htons(DNS_QUERY_TYPE_AAAA); - ofpbuf_put(&dns_answer, &v, sizeof(ovs_be16)); - v = htons(DNS_CLASS_IN); - ofpbuf_put(&dns_answer, &v, sizeof(ovs_be16)); - ovs_be32 ttl = htonl(DNS_DEFAULT_RR_TTL); - ofpbuf_put(&dns_answer, &ttl, sizeof(ovs_be32)); - /* Length of the RDATA field */ - v = htons(sizeof(ip_addrs.ipv6_addrs[i].addr.s6_addr)); - ofpbuf_put(&dns_answer, &v, sizeof(ovs_be16)); - ofpbuf_put(&dns_answer, &ip_addrs.ipv6_addrs[i].addr.s6_addr, - sizeof(ip_addrs.ipv6_addrs[i].addr.s6_addr)); + put_be16(&dns_answer, htons(DNS_QUERY_TYPE_AAAA)); + put_be16(&dns_answer, htons(DNS_CLASS_IN)); + put_be32(&dns_answer, htonl(DNS_DEFAULT_RR_TTL)); + const struct in6_addr *ip6 = &ip_addrs.ipv6_addrs[i].addr; + put_be16(&dns_answer, htons(sizeof *ip6)); + ofpbuf_put(&dns_answer, ip6, sizeof *ip6); ancount++; } } _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev