[ovs-dev] [PATCH v4 2/2 ovn] External IP based NAT: NORTHD changes to use applied/exempted external ip
Tony Liu
tonyliu0592 at hotmail.com
Sun Aug 9 18:09:23 UTC 2020
> -----Original Message-----
> From: dev <ovs-dev-bounces at openvswitch.org> On Behalf Of Numan Siddique
> Sent: Friday, August 7, 2020 2:38 AM
> To: Ankur Sharma <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.
Is this a generic recommendation or for certain cases?
Is it OK to add an ACL with "!=", like below?
ovn-nbctl acl-add 12b1681c-b3e7-4ec9-b324-e780d9dfdc0d from-lport 1005 'ip4.dst == 192.168.0.0/16 && inport != "d93619c3-dab9-4f6d-8261-4211f6937fd1"' drop
Thanks!
Tony
>
> 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://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list