[ovs-dev] [PATCH v4 2/2 ovn] External IP based NAT: NORTHD changes to use applied/exempted external ip
Numan Siddique
numans at ovn.org
Mon Aug 10 06:01:20 UTC 2020
On Sun, Aug 9, 2020 at 11:39 PM Tony Liu <tonyliu0592 at hotmail.com> wrote:
>
>
>
> > -----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
This is a generic recommendation. The above ACL would also result in
many OF flows.
To handle cases like above, you can add a couple of ACLs like below
with high priority flow to allow the desired inport and low priority
ACL to drop all the traffic.
ovn-nbctl acl-add 12b1681c-b3e7-4ec9-b324-e780d9dfdc0d from-lport
1006 'ip4.dst == 192.168.0.0/16 && inport ==
"d93619c3-dab9-4f6d-8261-4211f6937fd1"' allow
ovn-nbctl acl-add 12b1681c-b3e7-4ec9-b324-e780d9dfdc0d from-lport
1005 'ip4.dst == 192.168.0.0/16"' drop
Thanks
Numan
>
> 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
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list