[ovs-dev] Add new 'passthrough' nat_type
diff mbox series

Message ID DB8PR03MB5849A70B44E8AB8B5BC7F0D3E3190@DB8PR03MB5849.eurprd03.prod.outlook.com
State New
Headers show
Series
  • [ovs-dev] Add new 'passthrough' nat_type
Related show

Commit Message

Surya Rudra via dev May 31, 2019, 2:08 p.m. UTC
'passthrough' expects source network in 'logical_ip' column and destination network in 'external_ip' column.
For the traffic that goes from source to destination 'passthrough' disables NAT by adding a priority-100 flow with a match of 'ip && ip4.src == A && ip4.dst == B' and an action of 'next;'.

Signed-off-by: Rostyslav Fridman rostyslav_fridman@epam.com<mailto:rostyslav_fridman@epam.com>

I have created a PR with this patch at https://github.com/openvswitch/ovs/pull/285

---

Comments

Ben Pfaff June 4, 2019, 5:20 p.m. UTC | #1
On Fri, May 31, 2019 at 02:08:16PM +0000, Rostyslav Fridman via dev wrote:
> 'passthrough' expects source network in 'logical_ip' column and destination network in 'external_ip' column.
> For the traffic that goes from source to destination 'passthrough' disables NAT by adding a priority-100 flow with a match of 'ip && ip4.src == A && ip4.dst == B' and an action of 'next;'.
> 
> Signed-off-by: Rostyslav Fridman rostyslav_fridman@epam.com<mailto:rostyslav_fridman@epam.com>
> 
> I have created a PR with this patch at https://github.com/openvswitch/ovs/pull/285

This explains the semantics but not why they're useful.  Why would one
use a passthrough nat_type?
Surya Rudra via dev June 5, 2019, 9:27 a.m. UTC | #2
> This explains the semantics but not why they're useful.  Why would one use a passthrough nat_type?
Basically this replicates the behavior of iptables ACCEPT rules in nat table.

target     prot opt source               destination
ACCEPT     all  --  10.0.0.0/8          192.168.0.0/16         
MASQUERADE  all  --  10.0.0.0/8          anywhere

What I want to achieve is to be able to nat some network to all destinations except for specified in passthrough rule.
To be more precise, if I have some private subnet and I want it to be able to access Internet using NAT, but at the same time have a direct routing to subnet on a different node I will use a passthrough nat rule.
Ben Pfaff June 5, 2019, 6:18 p.m. UTC | #3
On Wed, Jun 05, 2019 at 09:27:38AM +0000, Rostyslav Fridman wrote:
> > This explains the semantics but not why they're useful.  Why would one use a passthrough nat_type?
> Basically this replicates the behavior of iptables ACCEPT rules in nat table.
> 
> target     prot opt source               destination
> ACCEPT     all  --  10.0.0.0/8          192.168.0.0/16         
> MASQUERADE  all  --  10.0.0.0/8          anywhere
> 
> What I want to achieve is to be able to nat some network to all destinations except for specified in passthrough rule.
> To be more precise, if I have some private subnet and I want it to be able to access Internet using NAT, but at the same time have a direct routing to subnet on a different node I will use a passthrough nat rule.

It is good to get this explanation in email.  Thank you.  Please add the
explanation to the commit message and the documentation as part of the
next version of the patch.

Patch
diff mbox series

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index aab8f6974..d269c872a 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -2384,6 +2384,15 @@  nd_ns {
           <code>flags.force_snat_for_lb == 1 &amp;&amp; ip</code> with an
           action <code>ct_snat(<var>B</var>);</code>.
         </p>
+        <p>
+          For each configuration in the OVN Northbound database, that asks
+          NOT to change the source IP address of a packet with address
+          <var>A</var> is going to destination <var>B</var> or NOT to change
+          the source IP address of a packet thath belongs to network <var>A</var>
+          that is going to destination <var>B</var>, a priority-100 flow with
+          a match of <code>ip &amp;&amp; ip4.src == <var>A</var> &amp;&amp;
+          ip4.dst == <var>B</var></code> and an action of <code>next;</code>.
+        </p>
         <p>
           For each configuration in the OVN Northbound database, that asks
           to change the source IP address of a packet from an IP address of
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index de0c06d4b..7d3c222b1 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -6002,6 +6002,10 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             nat = op->od->nbr->nat[i];
+            if (!strcmp(nat->type, "passthrough")) {
+                continue;
+            }
+
             ovs_be32 ip;
             if (!ip_parse(nat->external_ip, &ip) || !ip) {
                 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
@@ -6359,19 +6363,34 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             ovs_be32 ip, mask;
-            char *error = ip_parse_masked(nat->external_ip, &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 external ip %s for nat",
-                             nat->external_ip);
-                free(error);
-                continue;
+            /* Check the validity of nat->external_ip. 'external_ip' can
+             * be a subnet when the type is "passthrough". */
+            if (!strcmp(nat->type, "passthrough")) {
+                char *error = ip_parse_masked(nat->external_ip, &ip, &mask);
+                if (error) {
+                    static struct vlog_rate_limit rl
+                            = VLOG_RATE_LIMIT_INIT(5, 1);
+                    VLOG_WARN_RL(&rl, "bad external ip %s for passthrough",
+                                 nat->external_ip);
+                    free(error);
+                    continue;
+                }
+            } else {
+                char *error = ip_parse_masked(nat->external_ip, &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 external ip %s for nat",
+                                 nat->external_ip);
+                    free(error);
+                    continue;
+                }
             }
             /* Check the validity of nat->logical_ip. 'logical_ip' can
-             * be a subnet when the type is "snat". */
-            error = ip_parse_masked(nat->logical_ip, &ip, &mask);
-            if (!strcmp(nat->type, "snat")) {
+             * be a subnet when the type is "snat" or "passthrough". */
+            char *error = ip_parse_masked(nat->logical_ip, &ip, &mask);
+            if (!strcmp(nat->type, "snat") || !strcmp(nat->type, "passthrough")) {
                 if (error) {
                     static struct vlog_rate_limit rl =
                         VLOG_RATE_LIMIT_INIT(5, 1);
@@ -6546,6 +6565,17 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                               ds_cstr(&match), ds_cstr(&actions));
             }
+            /* Egress SNAT table: Skip packets that have a specific 'passthrough'
+             * rule. */
+            if (!strcmp(nat->type, "passthrough")) {
+                ds_clear(&match);
+                ds_put_format(&match, "ip && ip4.src == %s && ip4.dst == %s",
+                              nat->logical_ip,
+                              nat->external_ip);
+                ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 100,
+                              ds_cstr(&match), "next;");
+            }
+
             /* Egress SNAT table: Packets enter the egress pipeline with
              * source ip address that needs to be SNATted to a external ip
              * address. */
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 2c87cbba7..520783227 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@ 
{
     "name": "OVN_Northbound",
-    "version": "5.16.0",
-    "cksum": "923459061 23095",
+    "version": "5.16.1",
+    "cksum": "2460088216 23171",
     "tables": {
         "NB_Global": {
             "columns": {
@@ -343,6 +343,7 @@ 
                 "type": {"type": {"key": {"type": "string",
                                            "enum": ["set", ["dnat",
                                                              "snat",
+                                                             "passthrough",
                                                              "dnat_and_snat"
                                                                ]]}}},
                 "external_ids": {
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index cbaa9495f..203141a86 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -2029,6 +2029,14 @@ 
           <ref column="logical_ip"/> is SNATed into the IP address in
           <ref column="external_ip"/>.
         </li>
+        <li>
+          When <ref column="type"/> is <code>passthrough</code>, IP packets
+          with their source IP address that either matches the IP address
+          in <ref column="logical_ip"/> or is in the network provided by
+          <ref column="logical_ip"/> is not SNATed to destination IP address
+          in <ref column="external_ip"/> or destination network provided by
+          <ref column="external_ip"/> and is allowed to pass-through.
+        </li>
         <li>
           When <ref column="type"/> is <code>dnat_and_snat</code>, the
           externally visible IP address <ref column="external_ip"/> is
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index e86ab7f7a..a96abef44 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -3832,6 +3832,7 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
     const char *external_ip = ctx->argv[3];
     const char *logical_ip = ctx->argv[4];
     char *new_logical_ip = NULL;
+    char *new_external_ip = NULL;
     char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr);
     if (error) {
@@ -3840,20 +3841,32 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
     }
     if (strcmp(nat_type, "dnat") && strcmp(nat_type, "snat")
-            && strcmp(nat_type, "dnat_and_snat")) {
-        ctl_error(ctx, "%s: type must be one of \"dnat\", \"snat\" and "
+            && strcmp(nat_type, "dnat_and_snat") && strcmp(nat_type, "passthrough")) {
+        ctl_error(ctx, "%s: type must be one of \"dnat\", \"snat\", \"passthrough\" and "
                   "\"dnat_and_snat\".", nat_type);
         return;
     }
     ovs_be32 ipv4 = 0;
     unsigned int plen;
-    if (!ip_parse(external_ip, &ipv4)) {
-        ctl_error(ctx, "%s: should be an IPv4 address.", external_ip);
-        return;
+    if (strcmp("passthrough", nat_type)) {
+        if (!ip_parse(external_ip, &ipv4)) {
+            ctl_error(ctx, "%s: should be an IPv4 address.", external_ip);
+            return;
+        }
+        new_external_ip = xstrdup(external_ip);
+    } else {
+        error = ip_parse_cidr(external_ip, &ipv4, &plen);
+        if (error) {
+            free(error);
+            ctl_error(ctx, "%s: should be an IPv4 address or network.",
+                      external_ip);
+            return;
+        }
+        new_external_ip = normalize_ipv4_prefix(ipv4, plen);
     }
-    if (strcmp("snat", nat_type)) {
+    if (strcmp("snat", nat_type) && strcmp("passthrough", nat_type)) {
         if (!ip_parse(logical_ip, &ipv4)) {
             ctl_error(ctx, "%s: should be an IPv4 address.", logical_ip);
             return;
@@ -3911,9 +3924,9 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
     for (size_t i = 0; i < lr->n_nat; i++) {
         const struct nbrec_nat *nat = lr->nat[i];
         if (!strcmp(nat_type, nat->type)) {
-            if (!strcmp(is_snat ? new_logical_ip : external_ip,
+            if (!strcmp(is_snat ? new_logical_ip : new_external_ip,
                         is_snat ? nat->logical_ip : nat->external_ip)) {
-                if (!strcmp(is_snat ? external_ip : new_logical_ip,
+                if (!strcmp(is_snat ? new_external_ip : new_logical_ip,
                             is_snat ? nat->external_ip : nat->logical_ip)) {
                         if (may_exist) {
                             nbrec_nat_verify_logical_port(nat);
@@ -3925,7 +3938,7 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
                         }
                         ctl_error(ctx, "%s, %s: a NAT with this external_ip "
                                   "and logical_ip already exists",
-                                  external_ip, new_logical_ip);
+                                  new_external_ip, new_logical_ip);
                         free(new_logical_ip);
                         return;
                 } else {
@@ -3933,7 +3946,7 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
                               "already exists",
                               nat_type,
                               is_snat ? "logical_ip" : "external_ip",
-                              is_snat ? new_logical_ip : external_ip);
+                              is_snat ? new_logical_ip : new_external_ip);
                     free(new_logical_ip);
                     return;
                 }
@@ -3944,7 +3957,7 @@  nbctl_lr_nat_add(struct ctl_context *ctx)
     /* Create the NAT. */
     struct nbrec_nat *nat = nbrec_nat_insert(ctx->txn);
     nbrec_nat_set_type(nat, nat_type);
-    nbrec_nat_set_external_ip(nat, external_ip);
+    nbrec_nat_set_external_ip(nat, new_external_ip);
     nbrec_nat_set_logical_ip(nat, new_logical_ip);
     if (logical_port && external_mac) {
         nbrec_nat_set_logical_port(nat, logical_port);
@@ -3982,8 +3995,8 @@  nbctl_lr_nat_del(struct ctl_context *ctx)
     const char *nat_type = ctx->argv[2];
     if (strcmp(nat_type, "dnat") && strcmp(nat_type, "snat")
-            && strcmp(nat_type, "dnat_and_snat")) {
-        ctl_error(ctx, "%s: type must be one of \"dnat\", \"snat\" and "
+            && strcmp(nat_type, "dnat_and_snat") && strcmp(nat_type, "passthrough")) {
+        ctl_error(ctx, "%s: type must be one of \"dnat\", \"snat\", \"passthrough\" and "
                   "\"dnat_and_snat\".", nat_type);
         return;
     }
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 18c5c1d42..2eee466ee 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -404,7 +404,7 @@  dnl ---------------------------------------------------------------------
OVN_NBCTL_TEST([ovn_nbctl_nats], [NATs], [
AT_CHECK([ovn-nbctl lr-add lr0])
AT_CHECK([ovn-nbctl lr-nat-add lr0 snatt 30.0.0.2 192.168.1.2], [1], [],
-[ovn-nbctl: snatt: type must be one of "dnat", "snat" and "dnat_and_snat".
+[ovn-nbctl: snatt: type must be one of "dnat", "snat", "passthrough" and "dnat_and_snat".
])
AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.2a 192.168.1.2], [1], [],
[ovn-nbctl: 30.0.0.2a: should be an IPv4 address.