[ovs-dev] [PATCH v3 5/5] ovn: DNAT and SNAT on a gateway router.

Ben Pfaff blp at ovn.org
Thu Jun 2 21:48:16 UTC 2016


On Thu, May 19, 2016 at 01:02:34PM -0700, Gurucharan Shetty wrote:
> For traffic from physical space to virtual space we need DNAT.
> The DNAT happens in the gateway router and reaches the logical
> port. The return traffic should be unDNATed.
> 
> Traffic originating in virtual space heading to physical space
> should be SNATed. The return traffic is unSNATted.
> 
> East-west traffic with the public destination IP address needs
> a DNAT. This traffic is punted to the l3 gateway where DNAT
> takes place. This traffic is also SNATed and eventually loops back to
> its destination. The SNAT is needed because we need the reverse traffic
> to go back to the l3 gateway and not short-circuit directly to the source.
> 
> This commit introduces 4 new logical actions.
> 1. ct_snat: To send the packet through SNAT zone to unSNAT packets.
> 2. ct_snat(IP): To SNAT to the provided IP address.
> 3. ct_dnat: To send the packet throgh DNAT zone to unDNAT packets.
> 4. ct_dnat(IP): To DNAT to the provided IP.
> 
> This commit only provides the ability to do IP based NAT. This will
> eventually be enhanced to do PORT based NAT too.
> 
> Command hints:
> 
> Consider a distributed router "R1" that has switch foo (192.168.1.0/24)
> with a lport foo1 (192.168.1.2) and bar (192.168.2.0/24) with lport bar1
> (192.168.2.2) connected to it. You connect "R1" to
> a gateway router "R2" via a switch "join" in (20.0.0.0/24) network.
> 
> R2 has a switch "alice" (172.16.1.0/24) connected to it (to simulate
> external network).
> 
> case: Add pure DNAT (north-south)
> 
> Add a DNAT rule in R2:
> ovn-nbctl set logical_router R2 dnat:"30.0.0.2"=192.168.1.2
> 
> Now alice1 should be able to ping 192.168.1.2 via 30.0.0.2.
> 
> case2 : Add pure SNAT (south-north)
> 
> Add a SNAT rule in R2:
> 
> ovn-nbctl set logical_router R2 snat:"192.168.1.0/24"=30.0.0.1
> 
> (You need a static route in R1 to send packets destined to outside
> world to go through R2)
> 
> When foo1 pings alice1, alice1 receives traffic from 30.0.0.1
> 
> case3 : SNAT and DNAT (east-west traffic)
> 
> Add a SNAT rule for bar1
> ovn-nbctl set logical_router R2 snat:"192.168.2.2"=30.0.0.3
> 
> When bar1 pings 30.0.0.2, the traffic jumps to the gateway router
> and loops back to foo1 with a source ip address of 30.0.0.3
> 
> Signed-off-by: Gurucharan Shetty <guru at ovn.org>

Thanks for working on this!

The examples above are helpful.  Is there someplace in the documentation
where it would be appropriate to include them?

Can you add some test cases for action parsing to "ovn -- action
parsing" in ovn.at?  (As an aside, I like to try to make sure that the
action parsing test cases exercise all the possible syntax errors;
usually that's easy.)

Please mention the "ip" prerequisite in the ovn-sb.xml documentation.

I see a typo in the ovn-northd.8.xml documentation,
s/Northdbound/Northbound/ in a few places, e.g.:
+          For each configuration in the OVN Northdbound database, that asks
...
+      changed based on the configuration in the OVN Northdbound database.
...
+          For each configuration in the OVN Northdbound database, that asks

It appears to me that there is a lot of duplicated code in ovn-northd.c
for the different kinds of NAT.  For example, I think that the following
would condense the SNAT code a good deal with less code (it does not
update the comments properly though):

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 3ce88a7..279d641 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2028,6 +2028,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
 
         /* Packets are allowed by default. */
         ovn_lflow_add(lflows, od, S_ROUTER_IN_SNAT, 0, "1", "next;");
+        ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 0, "1", "next;");
 
         /* SNAT rules are only valid on non-distributed routers. */
         if (!smap_get(&od->nbr->options, "chassis")) {
@@ -2060,6 +2061,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             ovn_lflow_add(lflows, od, S_ROUTER_IN_SNAT, 100,
                           match, "ct_snat; next;");
             free(match);
+
+            char *actions;
+            match = xasprintf("ip && ip4.src == %s", node->key);
+            actions = xasprintf("ct_snat(%s);", node->value);
+            ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 100,
+                          match, actions);
+            free(match);
+            free(actions);
         }
     }
 
@@ -2330,52 +2339,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 0, "1", "output;");
     }
 
-    /* Egress SNAT. Packets enter the pipeline with source ip address
-     * that needs to be SNATted to a virtual ip address. */
-    HMAP_FOR_EACH (od, key_node, datapaths) {
-        if (!od->nbr) {
-            continue;
-        }
-
-        /* Packets are allowed by default. */
-        ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 0, "1", "next;");
-
-        /* SNAT rules are only valid on non-distributed routers. */
-        if (!smap_get(&od->nbr->options, "chassis")) {
-            continue;
-        }
-
-        struct smap_node *node;
-        SMAP_FOR_EACH(node, &od->nbr->snat) {
-            char *match, *actions;
-            ovs_be32 ip, mask;
-
-            char *error = ip_parse_masked(node->key, &ip, &mask);
-            if (error) {
-                static struct vlog_rate_limit rl
-                        = VLOG_RATE_LIMIT_INIT(5, 1);
-                VLOG_WARN_RL(&rl, "bad ip or ip network %s in snat key",
-                             node->key);
-                continue;
-            }
-
-            error = ip_parse_masked(node->value, &ip, &mask);
-            if (error || mask != OVS_BE32_MAX) {
-                static struct vlog_rate_limit rl
-                        = VLOG_RATE_LIMIT_INIT(5, 1);
-                VLOG_WARN_RL(&rl, "bad ip address %s for snat", node->value);
-                continue;
-            }
-
-            match = xasprintf("ip && ip4.src == %s", node->key);
-            actions = xasprintf("ct_snat(%s);", node->value);
-            ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 100,
-                          match, actions);
-            free(match);
-            free(actions);
-        }
-    }
-
     /* Logical router egress table 1: Delivery (priority 100).
      *
      * Priority 100 rules deliver packets to enabled logical ports. */

It appears that the ingress DNAT code could also be shortened by
factoring out the code for parsing IP addresses into a helper.

Do you think that "non-distributed" is a good term?  If it is a standard
term, then it's OK, but, if not, then negative terms are usually better
replaced by positive, for example perhaps "centralized" instead.

The documentation in ovn-sb.xml talks about recirculation.  The person
looking at OVN logical flows is a couple of steps removed from
understanding what this means or the consequences.  I wonder whether
there is a simple way to explain the meaning or why it is important.

Thanks again for implementing the gateway.

Acked-by: Ben Pfaff <blp at ovn.org>



More information about the dev mailing list