[ovs-dev] [PATCH v4 2/2 ovn] External IP based NAT: NORTHD changes to use applied/exempted external ip
svc.mail.git
svc.mail.git at nutanix.com
Thu Aug 13 01:17:05 UTC 2020
Hi Numan,
Sure, yes thats a valid concern.
I will try to address this in V5.
Regards,
Ankur
________________________________
From: Numan Siddique <numans at ovn.org>
Sent: Friday, August 7, 2020 2:37 AM
To: svc.mail.git <svc.mail.git at nutanix.com>
Cc: ovs-dev <ovs-dev at openvswitch.org>
Subject: Re: [ovs-dev] [PATCH v4 2/2 ovn] External IP based NAT: NORTHD changes to use applied/exempted external ip
On Wed, Aug 5, 2020 at 1:56 AM Ankur Sharma <svc.mail.git at nutanix.com> wrote:
>
> From: Ankur Sharma <ankur.sharma at nutanix.com>
>
> This patch has northd changes which consumes applied/exempted external ip
> configuration per NAT rule in logical flow.
>
> Applied/Exempted 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.sharma at nutanix.com>
> ---
> northd/ovn-northd.c | 61 +++++++++++++++++++++++++
> tests/ovn-northd.at | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 188 insertions(+)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 03c62ba..17ae6a6 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -8250,6 +8250,23 @@ lrouter_nat_is_stateless(const struct nbrec_nat *nat)
> return false;
> }
>
> +static inline void
> +lrouter_nat_add_ext_ip_match(struct ds *match, const struct nbrec_nat *nat,
> + bool is_v6, bool is_src)
> +{
> + struct nbrec_address_set *applied_ext_ips = nat->applied_ext_ips;
> + struct nbrec_address_set *exempted_ext_ips = nat->exempted_ext_ips;
> +
> + ds_put_format(match, " && ip%s.%s %s $%s",
> + is_v6 ? "6" : "4",
> + is_src ? "src" : "dst",
> + applied_ext_ips? "==" : "!=",
I Ankur,
I think this is a big problem here. We should not use "!=" in logical
flows, although OVN allows.
This results in lots of lots of OF flows. In my testing with the below
logical flow
table=5 (lr_in_unsnat ), priority=100 , match=(ip && ip4.dst
== 172.168.0.100 && inport == "lr0-public" &&
is_chassis_resident("cr-lr0-public") && ip4.src != $disallowed_range),
action=(ct_snat;)
table=1 (lr_out_snat ), priority=153 , match=(ip && ip4.src ==
10.0.0.0/24 && outport == "lr0-public" &&
is_chassis_resident("cr-lr0-public") && ip4.dst != $disallowed_range),
action=(ct_snat(172.168.0.100);)
If the address set 'disallowed_range' has 1 IP, it results in around
74 Of flows because of these 2 logical flows.
With 2 addresses in the addr set, it results in around 1200 OF flows.
With 3 addresses, it results in around 1500 OF flows
And with 4 addresses, it results in around 155622 OF flows.
I don't think this is acceptable. And we should not use the "!=" match.
The approach seems fine to me for 'applied_ext_ips', but not for
'exempted_ext_ips'.
I'd suggest exploring other alternatives.
Thanks
Numan
> + applied_ext_ips?
> + applied_ext_ips->name :
> + exempted_ext_ips->name);
> + return;
> +}
> +
> /* Builds the logical flow that replies to ARP requests for an 'ip_address'
> * owned by the router. The flow is inserted in table S_ROUTER_IN_IP_INPUT
> * with the given priority.
> @@ -9199,6 +9216,18 @@ 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 *applied_ext_ips =
> + nat->applied_ext_ips;
> + struct nbrec_address_set *exempted_ext_ips =
> + nat->exempted_ext_ips;
> +
> + if (applied_ext_ips && exempted_ext_ips) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_WARN_RL(&rl, "NAT rule: "UUID_FMT" not applied, since"
> + "both applied and exempt external ips set",
> + UUID_ARGS(&(nat->header_.uuid)));
> + continue;
> + }
>
> char *error = ip_parse_masked(nat->external_ip, &ip, &mask);
> if (error || mask != OVS_BE32_MAX) {
> @@ -9286,6 +9315,10 @@ 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 (applied_ext_ips || exempted_ext_ips) {
> + lrouter_nat_add_ext_ip_match(&match, nat, is_v6, true);
> + }
> +
> if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
> ds_put_format(&actions, "ip%s.dst=%s; next;",
> is_v6 ? "6" : "4", nat->logical_ip);
> @@ -9315,6 +9348,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> od->l3redirect_port->json_key);
> }
>
> + if (applied_ext_ips || exempted_ext_ips) {
> + lrouter_nat_add_ext_ip_match(&match, nat, is_v6, true);
> + }
> +
> if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
> ds_put_format(&actions, "ip%s.dst=%s; next;",
> is_v6 ? "6" : "4", nat->logical_ip);
> @@ -9343,6 +9380,10 @@ 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 (applied_ext_ips || exempted_ext_ips) {
> + lrouter_nat_add_ext_ip_match(&match, nat, is_v6, true);
> + }
> +
> ds_clear(&actions);
> if (dnat_force_snat_ip) {
> /* Indicate to the future tables that a DNAT has taken
> @@ -9386,6 +9427,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> ds_put_format(&match, " && is_chassis_resident(%s)",
> od->l3redirect_port->json_key);
> }
> +
> + if (applied_ext_ips || exempted_ext_ips) {
> + lrouter_nat_add_ext_ip_match(&match, nat, is_v6, true);
> + }
> +
> ds_clear(&actions);
>
> if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
> @@ -9467,6 +9513,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> ds_put_format(&match, " && is_chassis_resident(%s)",
> od->l3redirect_port->json_key);
> }
> +
> + if (applied_ext_ips || exempted_ext_ips) {
> + lrouter_nat_add_ext_ip_match(&match, nat, is_v6, false);
> + }
> +
> ds_clear(&actions);
> if (distributed) {
> ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ",
> @@ -9496,6 +9547,11 @@ 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 (applied_ext_ips || exempted_ext_ips) {
> + lrouter_nat_add_ext_ip_match(&match, nat, is_v6, false);
> + }
> +
> ds_clear(&actions);
>
> if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
> @@ -9536,6 +9592,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> ds_put_format(&match, " && is_chassis_resident(%s)",
> od->l3redirect_port->json_key);
> }
> +
> + if (applied_ext_ips || exempted_ext_ips) {
> + lrouter_nat_add_ext_ip_match(&match, nat, is_v6, false);
> + }
> +
> 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 7872d50..a97a776 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-applied 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-applied 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-applied 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
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=gdsHrMKI-VOHoN1ziUyqlIGzgVzIjMMz-NHIsocZYPE&m=GudDuwu2w1TEhl64FG5VaBxyq7pjGHloBgJarjm1IZ0&s=ULAUl7D1cfq4PvrjkMM48zDNsh1hQkTbH5sfK1WHSMQ&e=
>
More information about the dev
mailing list