Re: [ovs-dev] [PATCH v2 2/2 ovn] External IP based NAT: NORTHD changes to use allowed/disallowed external ip

Mon, 29 Jun 2020 11:34:59 -0700 

On 6/28/20 9:34 PM, Ankur Sharma wrote:

From: Ankur Sharma <ankur.sha...@nutanix.com>

This patch has northd changes which consumes allowed/disallowed external ip
configuration per NAT rule in logical flow.

Allowed/Disallowed external ip range adds an additional match criteria in
snat/dnat/unsnat/undant logical flow rules.

For example, if an allowed_external_ip address set ("efgh")
is configured for following NAT rule.
TYPE             EXTERNAL_IP        LOGICAL_IP
snat             10.15.24.135       50.0.0.10

Then logical flow will look like following:
..(lr_out_snat)...match=(ip && .... && ip4.dst == $efgh), action=(ct_snat(...);)
...(lr_in_unsnat...)match=(ip && ..... && ip4.src == $efgh), action=(ct_snat;)

Signed-off-by: Ankur Sharma <ankur.sha...@nutanix.com>
---
  northd/ovn-northd.c |  82 +++++++++++++++++++++++++++++++++
  tests/ovn-northd.at | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 209 insertions(+)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index f49e4f2..3858de9 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8809,6 +8809,21 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
              struct in6_addr ipv6, mask_v6, v6_exact = IN6ADDR_EXACT_INIT;
              bool is_v6 = false;
              bool stateless = lrouter_nat_is_stateless(nat);
+            struct nbrec_address_set *allowed_external_ip =
+                                      nat->allowed_external_ip;
+            struct nbrec_address_set *disallowed_external_ip =
+                                      nat->disallowed_external_ip;
+
+            if (allowed_external_ip && disallowed_external_ip) {
+                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+                VLOG_INFO_RL(&rl, "Both allowed: %s and disallowed: %s "
+                             "external ips set for NAT, type: %s, "
+                             "logical_ip: %s, external_ip: %s",
+                             allowed_external_ip->name,
+                             disallowed_external_ip->name,
+                             nat->type, nat->logical_ip, nat->external_ip);


3 things:
1) This message should probably be a WARN instead of an INFO message.
2) You can refer to the NAT by UUID instead of type and IPs.
3) The message should say that the NAT is not being applied because of setting both allow and disallow IP addresses. 


+                continue;
+            }
char *error = ip_parse_masked(nat->external_ip, &ip, &mask); 
              if (error || mask != OVS_BE32_MAX) {
@@ -8896,6 +8911,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
                      ds_put_format(&match, "ip && ip%s.dst == %s",
                                    is_v6 ? "6" : "4",
                                    nat->external_ip);
+                    if (allowed_external_ip || disallowed_external_ip) {
+                        ds_put_format(&match, " && ip%s.src %s $%s",
+                                      is_v6 ? "6" : "4",
+                                      allowed_external_ip? "==" : "!=",
+                                      allowed_external_ip?
+                                      allowed_external_ip->name :
+                                      disallowed_external_ip->name);
+                    }
This block is repeated many times throughout this patch (with the only different being "src" vs "dst") and should be made into a function. 


+
                      if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
                         ds_put_format(&actions, "ip%s.dst=%s; next;",
                                       is_v6 ? "6" : "4", nat->logical_ip);
@@ -8925,6 +8949,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
                                        od->l3redirect_port->json_key);
                      }
+ if (allowed_external_ip || disallowed_external_ip) { 
+                        ds_put_format(&match, " && ip%s.src %s $%s",
+                                      is_v6 ? "6" : "4",
+                                      allowed_external_ip? "==" : "!=",
+                                      allowed_external_ip?
+                                      allowed_external_ip->name :
+                                      disallowed_external_ip->name);
+                    }
+
                      if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
                          ds_put_format(&actions, "ip%s.dst=%s; next;",
                                        is_v6 ? "6" : "4", nat->logical_ip);
@@ -8953,6 +8986,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
                      ds_put_format(&match, "ip && ip%s.dst == %s",
                                    is_v6 ? "6" : "4",
                                    nat->external_ip);
+                    if (allowed_external_ip || disallowed_external_ip) {
+                        ds_put_format(&match, " && ip%s.src %s $%s",
+                                      is_v6 ? "6" : "4",
+                                      allowed_external_ip? "==" : "!=",
+                                      allowed_external_ip?
+                                      allowed_external_ip->name :
+                                      disallowed_external_ip->name);
+                    }
+
                      ds_clear(&actions);
                      if (dnat_force_snat_ip) {
                          /* Indicate to the future tables that a DNAT has taken
@@ -8996,6 +9038,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
                          ds_put_format(&match, " && is_chassis_resident(%s)",
                                        od->l3redirect_port->json_key);
                      }
+
+                    if (allowed_external_ip || disallowed_external_ip) {
+                        ds_put_format(&match, " && ip%s.src %s $%s",
+                                      is_v6 ? "6" : "4",
+                                      allowed_external_ip? "==" : "!=",
+                                      allowed_external_ip?
+                                      allowed_external_ip->name :
+                                      disallowed_external_ip->name);
+                    }
+
                      ds_clear(&actions);
if (!strcmp(nat->type, "dnat_and_snat") && stateless) { 
@@ -9076,6 +9128,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
                      ds_put_format(&match, " && is_chassis_resident(%s)",
                                    od->l3redirect_port->json_key);
                  }
+
+                if (allowed_external_ip || disallowed_external_ip) {
+                    ds_put_format(&match, " && ip%s.dst %s $%s",
+                                  is_v6 ? "6" : "4",
+                                  allowed_external_ip? "==" : "!=",
+                                  allowed_external_ip?
+                                  allowed_external_ip->name :
+                                  disallowed_external_ip->name);
+                }
+
                  ds_clear(&actions);
                  if (distributed) {
                      ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ",
@@ -9105,6 +9167,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
                      ds_put_format(&match, "ip && ip%s.src == %s",
                                    is_v6 ? "6" : "4",
                                    nat->logical_ip);
+
+                    if (allowed_external_ip || disallowed_external_ip) {
+                        ds_put_format(&match, " && ip%s.dst %s $%s",
+                                      is_v6 ? "6" : "4",
+                                      allowed_external_ip? "==" : "!=",
+                                      allowed_external_ip?
+                                      allowed_external_ip->name :
+                                      disallowed_external_ip->name);
+                    }
+
                      ds_clear(&actions);
if (!strcmp(nat->type, "dnat_and_snat") && stateless) { 
@@ -9145,6 +9217,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
                          ds_put_format(&match, " && is_chassis_resident(%s)",
                                        od->l3redirect_port->json_key);
                      }
+
+                    if (allowed_external_ip || disallowed_external_ip) {
+                        ds_put_format(&match, " && ip%s.dst %s $%s",
+                                      is_v6 ? "6" : "4",
+                                      allowed_external_ip? "==" : "!=",
+                                      allowed_external_ip?
+                                      allowed_external_ip->name :
+                                      disallowed_external_ip->name);
+                    }
+
                      ds_clear(&actions);
                      if (distributed) {
                          ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ",
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 80bda0a..4d574b2 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1140,6 +1140,133 @@ ovn-nbctl lr-nat-del R1 dnat_and_snat  172.16.1.1
AT_CLEANUP +AT_SETUP([ovn -- check allowed/disallowed external dnat, snat and dnat_and_snat rules]) 
+ovn_start
+
+ovn-sbctl chassis-add gw1 geneve 127.0.0.1
+
+ovn-nbctl lr-add R1
+ovn-nbctl lrp-add R1 R1-S1 02:ac:10:01:00:01 172.16.1.1/24
+
+ovn-nbctl ls-add S1
+ovn-nbctl lsp-add S1 S1-R1
+ovn-nbctl lsp-set-type S1-R1 router
+ovn-nbctl lsp-set-addresses S1-R1 router
+ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1
+
+ovn-nbctl lrp-set-gateway-chassis R1-S1 gw1
+
+uuid=`ovn-sbctl --columns=_uuid --bare find Port_Binding logical_port=cr-R1-S1`
+echo "CR-LRP UUID is: " $uuid
+
+ovn-nbctl create Address_Set name=allowed_range addresses=\"1.1.1.1\"
+ovn-nbctl create Address_Set name=disallowed_range addresses=\"2.2.2.2\"
+
+# SNAT with ALLOWED_IPs
+ovn-nbctl lr-nat-add R1 snat  172.16.1.1 50.0.0.11
+ovn-nbctl --is-allowed lr-nat-update-ext-ip R1 snat 50.0.0.11 allowed_range
+
+OVS_WAIT_UNTIL([test 3 = `ovn-sbctl dump-flows R1 | grep lr_out_snat | wc -l`])
+
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_snat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst == $allowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_unsnat | grep "ip4.dst == 172.16.1.1" | 
grep "ip4.src == $allowed_range" | wc -l], [0], [1
+])
+
+# SNAT with DISALLOWED_IPs
+ovn-nbctl lr-nat-del R1 snat  50.0.0.11
+ovn-nbctl lr-nat-add R1 snat  172.16.1.1 50.0.0.11
+ovn-nbctl lr-nat-update-ext-ip R1 snat 50.0.0.11 disallowed_range
+
+OVS_WAIT_UNTIL([test 3 = `ovn-sbctl dump-flows R1 | grep lr_out_snat | \
+wc -l`])
+
+ovn-nbctl show R1
+ovn-sbctl dump-flows R1
+
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_snat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst != $disallowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_unsnat | grep "ip4.dst == 172.16.1.1" | 
grep "ip4.src != $disallowed_range" | wc -l], [0], [1
+])
+
+# Stateful FIP with ALLOWED_IPs
+ovn-nbctl lr-nat-del R1 snat  50.0.0.11
+ovn-nbctl lr-nat-add R1 dnat_and_snat  172.16.1.2 50.0.0.11
+ovn-nbctl --is-allowed lr-nat-update-ext-ip R1 dnat_and_snat 172.16.1.2 
allowed_range
+ovn-nbctl show R1
+ovn-sbctl dump-flows R1
+
+OVS_WAIT_UNTIL([test 3 = `ovn-sbctl dump-flows R1 | grep lr_out_snat | \
+wc -l`])
+
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_snat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst == $allowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_unsnat | grep "ip4.dst == 172.16.1.2" | 
grep "ip4.src == $allowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_undnat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst == $allowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_dnat | grep "ip4.dst == 172.16.1.2" | 
grep "ip4.src == $allowed_range" | wc -l], [0], [1
+])
+
+# Stateful FIP with DISALLOWED_IPs
+ovn-nbctl lr-nat-del R1 dnat_and_snat  172.16.1.2
+ovn-nbctl lr-nat-add R1 dnat_and_snat  172.16.1.2 50.0.0.11
+ovn-nbctl lr-nat-update-ext-ip R1 dnat_and_snat 172.16.1.2 disallowed_range
+ovn-nbctl show R1
+ovn-sbctl dump-flows R1
+
+OVS_WAIT_UNTIL([test 3 = `ovn-sbctl dump-flows R1 | grep lr_out_snat | \
+wc -l`])
+
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_snat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst != $disallowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_unsnat | grep "ip4.dst == 172.16.1.2" | 
grep "ip4.src != $disallowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_undnat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst != $disallowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_dnat | grep "ip4.dst == 172.16.1.2" | 
grep "ip4.src != $disallowed_range" | wc -l], [0], [1
+])
+
+# Stateless FIP with DISALLOWED_IPs
+ovn-nbctl lr-nat-del R1 dnat_and_snat  172.16.1.2
+ovn-nbctl --stateless lr-nat-add R1 dnat_and_snat  172.16.1.2 50.0.0.11
+ovn-nbctl --is-allowed lr-nat-update-ext-ip R1 dnat_and_snat 172.16.1.2 
allowed_range
+ovn-nbctl show R1
+ovn-sbctl dump-flows R1
+
+OVS_WAIT_UNTIL([test 3 = `ovn-sbctl dump-flows R1 | grep lr_out_snat | \
+wc -l`])
+
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_snat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst == $allowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_unsnat | grep "ip4.dst == 172.16.1.2" | 
grep "ip4.src == $allowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_undnat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst == $allowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_dnat | grep "ip4.dst == 172.16.1.2" | 
grep "ip4.src == $allowed_range" | wc -l], [0], [1
+])
+
+# Stateful FIP with DISALLOWED_IPs
+ovn-nbctl lr-nat-del R1 dnat_and_snat  172.16.1.2
+ovn-nbctl --stateless lr-nat-add R1 dnat_and_snat  172.16.1.2 50.0.0.11
+ovn-nbctl lr-nat-update-ext-ip R1 dnat_and_snat 172.16.1.2 disallowed_range
+ovn-nbctl show R1
+ovn-sbctl dump-flows R1
+
+OVS_WAIT_UNTIL([test 3 = `ovn-sbctl dump-flows R1 | grep lr_out_snat | \
+wc -l`])
+
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_snat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst != $disallowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_unsnat | grep "ip4.dst == 172.16.1.2" | 
grep "ip4.src != $disallowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_out_undnat | grep "ip4.src == 50.0.0.11" | 
grep "ip4.dst != $disallowed_range" | wc -l], [0], [1
+])
+AT_CHECK([ovn-sbctl dump-flows R1 | grep lr_in_dnat | grep "ip4.dst == 172.16.1.2" | 
grep "ip4.src != $disallowed_range" | wc -l], [0], [1
+])
+
+AT_CLEANUP
+
  AT_SETUP([ovn -- check Load balancer health check and Service Monitor sync])
  AT_SKIP_IF([test $HAVE_PYTHON = no])
  ovn_start



_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to