On Sat, Apr 15, 2017 at 8:51 AM, Ben Pfaff <b...@ovn.org> wrote: > On Mon, Apr 03, 2017 at 04:10:27PM +0530, nusid...@redhat.com wrote: > > From: Numan Siddique <nusid...@redhat.com> > > > > OVN implements native DNS resolution which can be used to resolve the > > internal DNS names belonging to a logical datapath. > > > > To support this, a new table 'DNS' is added in the NB DB. A new column > > 'dns_records' is added in 'Logical_Switch' table which references to the > > 'DNS' table. > > > > Following flows are added for each logical switch if configured with > > DNS records in the 'dns_records' column > > - A logical flow in DNS_LOOKUP stage which uses the action 'dns_lookup' > > to transform the DNS query to DNS reply packet and advances > > to the next stage - DNS_RESPONSE. > > > > - A logical flow in DNS_RESPONSE stage which implements the DNS > responder > > by sending the DNS reply from previous stage back to the inport. > > > > Signed-off-by: Numan Siddique <nusid...@redhat.com> > > Thank you very much! > > I see a few uses of - in ovn-northd.8.xml. I think that you can > just write - instead. > > ovn-northd.c uses smap_count() in a few places to determine whether an > smap is nonempty. I would prefer !smap_is_empty(), instead. > > It looks to me like at least two places have code that check for at > least one DNS record in an ovn_datapath. I think that a helper function > could clarify and reduce the amount of code. > > For iteration with HMAP_FOR_EACH and similar constructs, please write a > space after HMAP_FOR_EACH, so that it looks a little more like the > formatting we use for a "for" statement in OVS. > > sync_dns_entries() has two different HMAP_FOR_EACH_WITH_HASH loops that > search for a dns_info struct with a particular uuid. I think that it > would be better to write a helper function, to save code and make the > intent clearer. > > sync_dns_entries() calls sbrec_dns_set_datapaths() and > sbrec_dns_set_records() in two places. I think that this could be done > in a single place, in the final loop (although it might require an extra > 'delete' member in struct dns_info) or it could be a helper function. > > Thanks again! >
Thanks Ben for the review and the comments. I have addressed all your comments from P2 and this patch and will submit the v5 shortly. (This patch anyway needs a re base because of conflict). Thanks again Numan > Ben > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
