[ovs-dev] [PATCH v3 1/2] ovn-controller: Add 'dns_lookup' action

[Ben Pfaff]

On Mon, Mar 27, 2017 at 07:42:58PM +0530, nusiddiq at redhat.com wrote:
> From: Numan Siddique <nusiddiq at 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 <nusiddiq at redhat.com>

I didn't do a thorough review, but here's a suggested incremental for
the documentation.  (I started by noticing that a DNS request should
have udp.dst 53, not udp.src 53, and then got carried away a little.)

diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index e0e4f67b3bb6..692925aa9856 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1482,21 +1482,21 @@
           </p>
 
           <p>
-            When this action is applied to a valid DNS (UDP packet with
-            udp.src 53) request packet, it changes the packet into a DNS reply
-            if it is able to resolve the query and stores 1 in <var>R</var>.
-            It leaves the packet unchanged if it is not able to resolve and
-            stores 0 in <var>R</var>.
+            When this action is applied to a valid DNS request (a UDP packet
+            typically directed to port 53), it attempts to resolve the query
+            using the contents of the <ref table="DNS"/> table.  If it is
+            successful, it changes the packet into a DNS reply and stores 1 in
+            <var>R</var>.  If the action is applied to a non-DNS packet, an
+            invalid DNS request packet, or a valid DNS request for which the
+            <ref table="DNS"/> table does not supply an answer, it leaves the
+            packet unchanged and stores 0 in <var>R</var>.
           </p>
 
           <p>
-            When this action is applied to a non-DNS or an invalid
-            DNS request packet , it leaves the packet unchanged and stores
-            0 in <var>R</var>.
-          </p>
-
-          <p>
-            The contents of the <ref table="DNS"/> table control the DNS lookup.
+            Regardless of success, the action does not make any of the changes
+            to the flow that are necessary to direct the packet back to the
+            requester.  The logical pipeline can implement this behavior with
+            matches and actions in later tables.
           </p>
 
           <p>
@@ -1505,6 +1505,10 @@
               reg0[3] = dns_lookup();
             </code>
           </p>
+
+          <p>
+            <b>Prerequisite:</b> <code>udp</code>
+          </p>
         </dd>
       </dl>