| Message ID | 1570220071-16483-3-git-send-email-ankur.sharma@nutanix.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ALLOW Stateless NAT operations | expand |
On Sat, Oct 5, 2019 at 1:45 AM Ankur Sharma <ankur.sharma@nutanix.com> wrote: > > For dnat_and_snat rules which are meant to be stateless > instead of using ct_snat/dnat OVN actions, we will use > ipv4.src/ipv4.dst. > > This actions will do 1:1 mapping to inner ip to external ip, > while recalculating the checksums. > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> Hi Ankur, There is a checkpatch warning - WARNING: Line is 80 characters long (recommended limit is 79). Please address it. Please see below for few comments. Thanks Numan > --- > northd/ovn-northd.8.xml | 34 +++++++++++++++---- > northd/ovn-northd.c | 86 +++++++++++++++++++++++++++++++++++++++++++------ > tests/ovn-northd.at | 50 ++++++++++++++++++++++++++++ > 3 files changed, 154 insertions(+), 16 deletions(-) > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 0f4f1c1..7d5d102 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -1718,7 +1718,9 @@ icmp6 { > to change the source IP address of a packet from <var>A</var> to > <var>B</var>, a priority-90 flow matches <code>ip && > ip4.dst == <var>B</var></code> with an action > - <code>ct_snat; </code>. > + <code>ct_snat; </code>. If the NAT rule is of type dnat_and_snat > + and has <code>is_stateless=true</code> in the options, then the action > + would be <code>replace_dst_ip(<var>B</var>)</code>. I think you need to update the documentation as you removed the action - replace_dst_ip in v2. > </p> > > <p> > @@ -1738,7 +1740,10 @@ icmp6 { > <var>B</var>, a priority-100 flow matches <code>ip && > ip4.dst == <var>B</var> && inport == <var>GW</var></code>, > where <var>GW</var> is the logical router gateway port, with an > - action <code>ct_snat;</code>. > + action <code>ct_snat;</code>. If the NAT rule is of type > + dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_dst_ip Same here and other places too. > + (<var>B</var>)</code>. > </p> > > <p> > @@ -1858,7 +1863,10 @@ icmp6 { > Gateway router is configured to force SNAT any DNATed packet, > the above action will be replaced by > <code>flags.force_snat_for_dnat = 1; flags.loopback = 1; > - ct_dnat(<var>B</var>);</code>. > + ct_dnat(<var>B</var>);</code>. If the NAT rule is of type > + dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_dst_ip > + (<var>B</var>)</code>. > </li> > > <li> > @@ -1890,7 +1898,10 @@ icmp6 { > <var>B</var>, a priority-100 flow matches <code>ip && > ip4.dst == <var>B</var> && inport == <var>GW</var></code>, > where <var>GW</var> is the logical router gateway port, with an > - action <code>ct_dnat(<var>B</var>);</code>. > + action <code>ct_dnat(<var>B</var>);</code>. If the NAT rule is of > + type dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_dst_ip > + (<var>B</var>)</code>. > </p> > > <p> > @@ -2553,7 +2564,10 @@ nd_ns { > matches <code>ip && ip4.src == <var>B</var> > && outport == <var>GW</var></code>, where <var>GW</var> > is the logical router gateway port, with an action > - <code>ct_dnat;</code>. > + <code>ct_dnat;</code>. If the NAT rule is of type > + dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_src_ip > + (<var>B</var>)</code>. > </p> > > <p> > @@ -2611,7 +2625,10 @@ nd_ns { > <code>ip && ip4.src == <var>A</var></code> with an action > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > is calculated based on the mask of <var>A</var>, with matches > - having larger masks getting higher priorities. > + having larger masks getting higher priorities. If the NAT rule is > + of type dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_src_ip > + (<var>B</var>)</code>. > </p> > <p> > A priority-0 logical flow with match <code>1</code> has actions > @@ -2634,7 +2651,10 @@ nd_ns { > logical router gateway port, with an action > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > is calculated based on the mask of <var>A</var>, with matches > - having larger masks getting higher priorities. > + having larger masks getting higher priorities. If the NAT rule > + is of type dnat_and_snat and has <code>is_stateless=true</code> > + in the options, then the action would be <code>replace_src_ip > + (<var>B</var>)</code>. > </p> > > <p> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index f393ceb..4036392 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -6314,6 +6314,18 @@ copy_ra_to_sb(struct ovn_port *op, const char *address_mode) > smap_destroy(&options); > } > > +static inline bool > +lrouter_nat_is_stateless(const struct nbrec_nat *nat) > +{ > + const char *is_stateless = smap_get(&nat->options, "is_stateless"); > + > + if (is_stateless && !strcmp(is_stateless, "true")) { > + return true; > + } > + > + return false; > +} > + > static void > build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > struct hmap *lflows, struct shash *meter_groups) > @@ -7052,6 +7064,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > nat = od->nbr->nat[i]; > > ovs_be32 ip, mask; > + bool is_stateless = lrouter_nat_is_stateless(nat); > > char *error = ip_parse_masked(nat->external_ip, &ip, &mask); > if (error || mask != OVS_BE32_MAX) { > @@ -7117,15 +7130,26 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > if (!od->l3dgw_port) { > /* Gateway router. */ > ds_clear(&match); > + ds_clear(&actions); > ds_put_format(&match, "ip && ip4.dst == %s", > nat->external_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.dst=%s; next;", > + nat->logical_ip); > + } else { > + ds_put_cstr(&actions, "ct_snat;"); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 90, > - ds_cstr(&match), "ct_snat;"); > + ds_cstr(&match), ds_cstr(&actions)); > } else { > /* Distributed router. */ > > /* Traffic received on l3dgw_port is subject to NAT. */ > ds_clear(&match); > + ds_clear(&actions); > + > ds_put_format(&match, "ip && ip4.dst == %s" > " && inport == %s", > nat->external_ip, > @@ -7136,8 +7160,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&match, " && is_chassis_resident(%s)", > od->l3redirect_port->json_key); > } > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.dst=%s; next;", > + nat->logical_ip); > + } else { > + ds_put_cstr(&actions, "ct_snat;"); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 100, > - ds_cstr(&match), "ct_snat;"); > + ds_cstr(&match), ds_cstr(&actions)); > > /* Traffic received on other router ports must be > * redirected to the central instance of the l3dgw_port > @@ -7172,8 +7204,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&actions, > "flags.force_snat_for_dnat = 1; "); > } > - ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", > - nat->logical_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "flags.loopback = 1; " > + "ip4.dst=%s; next;", > + nat->logical_ip); > + } else { > + ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", > + nat->logical_ip); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, > ds_cstr(&match), ds_cstr(&actions)); > } else { > @@ -7192,8 +7232,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > od->l3redirect_port->json_key); > } > ds_clear(&actions); > - ds_put_format(&actions, "ct_dnat(%s);", > - nat->logical_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.dst=%s; next;", > + nat->logical_ip); > + } else { > + ds_put_format(&actions, "ct_dnat(%s);", > + nat->logical_ip); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, > ds_cstr(&match), ds_cstr(&actions)); > > @@ -7235,7 +7282,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", > ETH_ADDR_ARGS(mac)); > } > - ds_put_format(&actions, "ct_dnat;"); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.src=%s; next;", > + nat->external_ip); > + } else { > + ds_put_format(&actions, "ct_dnat;"); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_OUT_UNDNAT, 100, > ds_cstr(&match), ds_cstr(&actions)); > } > @@ -7251,7 +7305,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&match, "ip && ip4.src == %s", > nat->logical_ip); > ds_clear(&actions); > - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.src=%s; next;", > + nat->external_ip); > + } else { > + ds_put_format(&actions, "ct_snat(%s);", > + nat->external_ip); > + } > > /* The priority here is calculated such that the > * nat->logical_ip with the longest mask gets a higher > @@ -7280,7 +7341,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", > ETH_ADDR_ARGS(mac)); > } > - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.src=%s; next;", > + nat->external_ip); > + } else { > + ds_put_format(&actions, "ct_snat(%s);", > + nat->external_ip); > + } > > /* The priority here is calculated such that the > * nat->logical_ip with the longest mask gets a higher > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 42033d5..64511a9 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -966,3 +966,53 @@ OVS_WAIT_UNTIL([ovn-sbctl get Port_Binding ${uuid} options:redirect-type], [0], > ]) > > AT_CLEANUP > + Can you add a new test case or modify an existing test case in ovn.at to make use of this feature ? > +AT_SETUP([ovn -- check stateless dnat_and_snat rule]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +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 lr-nat-add R1 dnat_and_snat 172.16.1.1 50.0.0.11 > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], [0], [2 > +]) There is a chance that the test case might fail on slower systems if ovn-northd is not fast enough to add the necessary logical flows. For the above first check, I would suggest to use - OVS_WAIT_UNTIL. > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [2 > +]) > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [0 > +]) > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [0 > +]) > + > +ovn-nbctl lr-nat-del R1 dnat_and_snat 172.16.1.1 > + > +ovn-nbctl --stateless lr-nat-add R1 dnat_and_snat 172.16.1.1 50.0.0.11 > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], [0], [0 > +]) > + Same here. Please use OVS_WAIT_UNTIL. > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [0 > +]) > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [2 > +]) > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [2 > +]) > + > +AT_CLEANUP > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Numan, Submitted a v3 yesterday, addressing all the review comments. Please take a look. Regards, Ankur -----Original Message----- From: Numan Siddique <numans@ovn.org> Sent: Monday, October 28, 2019 6:40 AM To: Ankur Sharma <ankur.sharma@nutanix.com> Cc: ovs-dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v2 2/2 ovn] OVN: Use ipv4.src and ipv4.dst actions for NAT rules On Sat, Oct 5, 2019 at 1:45 AM Ankur Sharma <ankur.sharma@nutanix.com> wrote: > > For dnat_and_snat rules which are meant to be stateless instead of > using ct_snat/dnat OVN actions, we will use ipv4.src/ipv4.dst. > > This actions will do 1:1 mapping to inner ip to external ip, while > recalculating the checksums. > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> Hi Ankur, There is a checkpatch warning - WARNING: Line is 80 characters long (recommended limit is 79). Please address it. Please see below for few comments. Thanks Numan > --- > northd/ovn-northd.8.xml | 34 +++++++++++++++---- > northd/ovn-northd.c | 86 +++++++++++++++++++++++++++++++++++++++++++------ > tests/ovn-northd.at | 50 ++++++++++++++++++++++++++++ > 3 files changed, 154 insertions(+), 16 deletions(-) > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index > 0f4f1c1..7d5d102 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -1718,7 +1718,9 @@ icmp6 { > to change the source IP address of a packet from <var>A</var> to > <var>B</var>, a priority-90 flow matches <code>ip && > ip4.dst == <var>B</var></code> with an action > - <code>ct_snat; </code>. > + <code>ct_snat; </code>. If the NAT rule is of type dnat_and_snat > + and has <code>is_stateless=true</code> in the options, then the action > + would be <code>replace_dst_ip(<var>B</var>)</code>. I think you need to update the documentation as you removed the action - replace_dst_ip in v2. > </p> > > <p> > @@ -1738,7 +1740,10 @@ icmp6 { > <var>B</var>, a priority-100 flow matches <code>ip && > ip4.dst == <var>B</var> && inport == <var>GW</var></code>, > where <var>GW</var> is the logical router gateway port, with an > - action <code>ct_snat;</code>. > + action <code>ct_snat;</code>. If the NAT rule is of type > + dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_dst_ip Same here and other places too. > + (<var>B</var>)</code>. > </p> > > <p> > @@ -1858,7 +1863,10 @@ icmp6 { > Gateway router is configured to force SNAT any DNATed packet, > the above action will be replaced by > <code>flags.force_snat_for_dnat = 1; flags.loopback = 1; > - ct_dnat(<var>B</var>);</code>. > + ct_dnat(<var>B</var>);</code>. If the NAT rule is of type > + dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_dst_ip > + (<var>B</var>)</code>. > </li> > > <li> > @@ -1890,7 +1898,10 @@ icmp6 { > <var>B</var>, a priority-100 flow matches <code>ip && > ip4.dst == <var>B</var> && inport == <var>GW</var></code>, > where <var>GW</var> is the logical router gateway port, with an > - action <code>ct_dnat(<var>B</var>);</code>. > + action <code>ct_dnat(<var>B</var>);</code>. If the NAT rule is of > + type dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_dst_ip > + (<var>B</var>)</code>. > </p> > > <p> > @@ -2553,7 +2564,10 @@ nd_ns { > matches <code>ip && ip4.src == <var>B</var> > && outport == <var>GW</var></code>, where <var>GW</var> > is the logical router gateway port, with an action > - <code>ct_dnat;</code>. > + <code>ct_dnat;</code>. If the NAT rule is of type > + dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_src_ip > + (<var>B</var>)</code>. > </p> > > <p> > @@ -2611,7 +2625,10 @@ nd_ns { > <code>ip && ip4.src == <var>A</var></code> with an action > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > is calculated based on the mask of <var>A</var>, with matches > - having larger masks getting higher priorities. > + having larger masks getting higher priorities. If the NAT rule is > + of type dnat_and_snat and has <code>is_stateless=true</code> in the > + options, then the action would be <code>replace_src_ip > + (<var>B</var>)</code>. > </p> > <p> > A priority-0 logical flow with match <code>1</code> has > actions @@ -2634,7 +2651,10 @@ nd_ns { > logical router gateway port, with an action > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > is calculated based on the mask of <var>A</var>, with matches > - having larger masks getting higher priorities. > + having larger masks getting higher priorities. If the NAT rule > + is of type dnat_and_snat and has <code>is_stateless=true</code> > + in the options, then the action would be <code>replace_src_ip > + (<var>B</var>)</code>. > </p> > > <p> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index > f393ceb..4036392 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -6314,6 +6314,18 @@ copy_ra_to_sb(struct ovn_port *op, const char *address_mode) > smap_destroy(&options); > } > > +static inline bool > +lrouter_nat_is_stateless(const struct nbrec_nat *nat) { > + const char *is_stateless = smap_get(&nat->options, > +"is_stateless"); > + > + if (is_stateless && !strcmp(is_stateless, "true")) { > + return true; > + } > + > + return false; > +} > + > static void > build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > struct hmap *lflows, struct shash *meter_groups) > @@ -7052,6 +7064,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > nat = od->nbr->nat[i]; > > ovs_be32 ip, mask; > + bool is_stateless = lrouter_nat_is_stateless(nat); > > char *error = ip_parse_masked(nat->external_ip, &ip, &mask); > if (error || mask != OVS_BE32_MAX) { @@ -7117,15 +7130,26 > @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > if (!od->l3dgw_port) { > /* Gateway router. */ > ds_clear(&match); > + ds_clear(&actions); > ds_put_format(&match, "ip && ip4.dst == %s", > nat->external_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.dst=%s; next;", > + nat->logical_ip); > + } else { > + ds_put_cstr(&actions, "ct_snat;"); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 90, > - ds_cstr(&match), "ct_snat;"); > + ds_cstr(&match), > + ds_cstr(&actions)); > } else { > /* Distributed router. */ > > /* Traffic received on l3dgw_port is subject to NAT. */ > ds_clear(&match); > + ds_clear(&actions); > + > ds_put_format(&match, "ip && ip4.dst == %s" > " && inport == %s", > nat->external_ip, @@ -7136,8 > +7160,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&match, " && is_chassis_resident(%s)", > od->l3redirect_port->json_key); > } > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.dst=%s; next;", > + nat->logical_ip); > + } else { > + ds_put_cstr(&actions, "ct_snat;"); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 100, > - ds_cstr(&match), "ct_snat;"); > + ds_cstr(&match), > + ds_cstr(&actions)); > > /* Traffic received on other router ports must be > * redirected to the central instance of the > l3dgw_port @@ -7172,8 +7204,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&actions, > "flags.force_snat_for_dnat = 1; "); > } > - ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", > - nat->logical_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "flags.loopback = 1; " > + "ip4.dst=%s; next;", > + nat->logical_ip); > + } else { > + ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", > + nat->logical_ip); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, > ds_cstr(&match), ds_cstr(&actions)); > } else { > @@ -7192,8 +7232,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > od->l3redirect_port->json_key); > } > ds_clear(&actions); > - ds_put_format(&actions, "ct_dnat(%s);", > - nat->logical_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.dst=%s; next;", > + nat->logical_ip); > + } else { > + ds_put_format(&actions, "ct_dnat(%s);", > + nat->logical_ip); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, > ds_cstr(&match), > ds_cstr(&actions)); > > @@ -7235,7 +7282,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", > ETH_ADDR_ARGS(mac)); > } > - ds_put_format(&actions, "ct_dnat;"); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.src=%s; next;", > + nat->external_ip); > + } else { > + ds_put_format(&actions, "ct_dnat;"); > + } > + > ovn_lflow_add(lflows, od, S_ROUTER_OUT_UNDNAT, 100, > ds_cstr(&match), ds_cstr(&actions)); > } > @@ -7251,7 +7305,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&match, "ip && ip4.src == %s", > nat->logical_ip); > ds_clear(&actions); > - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.src=%s; next;", > + nat->external_ip); > + } else { > + ds_put_format(&actions, "ct_snat(%s);", > + nat->external_ip); > + } > > /* The priority here is calculated such that the > * nat->logical_ip with the longest mask gets a > higher @@ -7280,7 +7341,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", > ETH_ADDR_ARGS(mac)); > } > - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); > + > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > + ds_put_format(&actions, "ip4.src=%s; next;", > + nat->external_ip); > + } else { > + ds_put_format(&actions, "ct_snat(%s);", > + nat->external_ip); > + } > > /* The priority here is calculated such that the > * nat->logical_ip with the longest mask gets a > higher diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index > 42033d5..64511a9 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -966,3 +966,53 @@ OVS_WAIT_UNTIL([ovn-sbctl get Port_Binding > ${uuid} options:redirect-type], [0], > ]) > > AT_CLEANUP > + Can you add a new test case or modify an existing test case in ovn.at to make use of this feature ? > +AT_SETUP([ovn -- check stateless dnat_and_snat rule]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) 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 lr-nat-add R1 dnat_and_snat 172.16.1.1 50.0.0.11 > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], [0], [2 > +]) There is a chance that the test case might fail on slower systems if ovn-northd is not fast enough to add the necessary logical flows. For the above first check, I would suggest to use - OVS_WAIT_UNTIL. > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [2 > +]) > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [0 > +]) > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [0 > +]) > + > +ovn-nbctl lr-nat-del R1 dnat_and_snat 172.16.1.1 > + > +ovn-nbctl --stateless lr-nat-add R1 dnat_and_snat 172.16.1.1 > +50.0.0.11 AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], > +[0], [0 > +]) > + Same here. Please use OVS_WAIT_UNTIL. > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [0 > +]) > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [2 > +]) > + > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [2 > +]) > + > +AT_CLEANUP > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch. > org_mailman_listinfo_ovs-2Ddev&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=mZw > X9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=CouZqk7agKapXA85Hmee871NM2d > rC0etOpAwwwF_Qvw&s=yGzdgaPfGMkQpXg_3NmyQCOqkqeJPvomW3QqTsWghPQ&e=
On Thu, Oct 31, 2019 at 2:02 AM Ankur Sharma <ankur.sharma@nutanix.com> wrote: > > Hi Numan, > > Submitted a v3 yesterday, addressing all the review comments. > Please take a look. Hi Ankur, Can you please rebase your v3 patches ? They don't apply on top of master. Thanks Numan > > Regards, > Ankur > > -----Original Message----- > From: Numan Siddique <numans@ovn.org> > Sent: Monday, October 28, 2019 6:40 AM > To: Ankur Sharma <ankur.sharma@nutanix.com> > Cc: ovs-dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v2 2/2 ovn] OVN: Use ipv4.src and ipv4.dst actions for NAT rules > > On Sat, Oct 5, 2019 at 1:45 AM Ankur Sharma <ankur.sharma@nutanix.com> wrote: > > > > For dnat_and_snat rules which are meant to be stateless instead of > > using ct_snat/dnat OVN actions, we will use ipv4.src/ipv4.dst. > > > > This actions will do 1:1 mapping to inner ip to external ip, while > > recalculating the checksums. > > > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> > > > Hi Ankur, > > There is a checkpatch warning - WARNING: Line is 80 characters long (recommended limit is 79). > Please address it. > > Please see below for few comments. > > Thanks > Numan > > > --- > > northd/ovn-northd.8.xml | 34 +++++++++++++++---- > > northd/ovn-northd.c | 86 +++++++++++++++++++++++++++++++++++++++++++------ > > tests/ovn-northd.at | 50 ++++++++++++++++++++++++++++ > > 3 files changed, 154 insertions(+), 16 deletions(-) > > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index > > 0f4f1c1..7d5d102 100644 > > --- a/northd/ovn-northd.8.xml > > +++ b/northd/ovn-northd.8.xml > > @@ -1718,7 +1718,9 @@ icmp6 { > > to change the source IP address of a packet from <var>A</var> to > > <var>B</var>, a priority-90 flow matches <code>ip && > > ip4.dst == <var>B</var></code> with an action > > - <code>ct_snat; </code>. > > + <code>ct_snat; </code>. If the NAT rule is of type dnat_and_snat > > + and has <code>is_stateless=true</code> in the options, then the action > > + would be <code>replace_dst_ip(<var>B</var>)</code>. > > I think you need to update the documentation as you removed the action > - replace_dst_ip in v2. > > > > </p> > > > > <p> > > @@ -1738,7 +1740,10 @@ icmp6 { > > <var>B</var>, a priority-100 flow matches <code>ip && > > ip4.dst == <var>B</var> && inport == <var>GW</var></code>, > > where <var>GW</var> is the logical router gateway port, with an > > - action <code>ct_snat;</code>. > > + action <code>ct_snat;</code>. If the NAT rule is of type > > + dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_dst_ip > > Same here and other places too. > > > + (<var>B</var>)</code>. > > </p> > > > > <p> > > @@ -1858,7 +1863,10 @@ icmp6 { > > Gateway router is configured to force SNAT any DNATed packet, > > the above action will be replaced by > > <code>flags.force_snat_for_dnat = 1; flags.loopback = 1; > > - ct_dnat(<var>B</var>);</code>. > > + ct_dnat(<var>B</var>);</code>. If the NAT rule is of type > > + dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_dst_ip > > + (<var>B</var>)</code>. > > </li> > > > > <li> > > @@ -1890,7 +1898,10 @@ icmp6 { > > <var>B</var>, a priority-100 flow matches <code>ip && > > ip4.dst == <var>B</var> && inport == <var>GW</var></code>, > > where <var>GW</var> is the logical router gateway port, with an > > - action <code>ct_dnat(<var>B</var>);</code>. > > + action <code>ct_dnat(<var>B</var>);</code>. If the NAT rule is of > > + type dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_dst_ip > > + (<var>B</var>)</code>. > > </p> > > > > <p> > > @@ -2553,7 +2564,10 @@ nd_ns { > > matches <code>ip && ip4.src == <var>B</var> > > && outport == <var>GW</var></code>, where <var>GW</var> > > is the logical router gateway port, with an action > > - <code>ct_dnat;</code>. > > + <code>ct_dnat;</code>. If the NAT rule is of type > > + dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_src_ip > > + (<var>B</var>)</code>. > > </p> > > > > <p> > > @@ -2611,7 +2625,10 @@ nd_ns { > > <code>ip && ip4.src == <var>A</var></code> with an action > > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > > is calculated based on the mask of <var>A</var>, with matches > > - having larger masks getting higher priorities. > > + having larger masks getting higher priorities. If the NAT rule is > > + of type dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_src_ip > > + (<var>B</var>)</code>. > > </p> > > <p> > > A priority-0 logical flow with match <code>1</code> has > > actions @@ -2634,7 +2651,10 @@ nd_ns { > > logical router gateway port, with an action > > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > > is calculated based on the mask of <var>A</var>, with matches > > - having larger masks getting higher priorities. > > + having larger masks getting higher priorities. If the NAT rule > > + is of type dnat_and_snat and has <code>is_stateless=true</code> > > + in the options, then the action would be <code>replace_src_ip > > + (<var>B</var>)</code>. > > </p> > > > > <p> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index > > f393ceb..4036392 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -6314,6 +6314,18 @@ copy_ra_to_sb(struct ovn_port *op, const char *address_mode) > > smap_destroy(&options); > > } > > > > +static inline bool > > +lrouter_nat_is_stateless(const struct nbrec_nat *nat) { > > + const char *is_stateless = smap_get(&nat->options, > > +"is_stateless"); > > + > > + if (is_stateless && !strcmp(is_stateless, "true")) { > > + return true; > > + } > > + > > + return false; > > +} > > + > > static void > > build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > struct hmap *lflows, struct shash *meter_groups) > > @@ -7052,6 +7064,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > nat = od->nbr->nat[i]; > > > > ovs_be32 ip, mask; > > + bool is_stateless = lrouter_nat_is_stateless(nat); > > > > char *error = ip_parse_masked(nat->external_ip, &ip, &mask); > > if (error || mask != OVS_BE32_MAX) { @@ -7117,15 +7130,26 > > @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > if (!od->l3dgw_port) { > > /* Gateway router. */ > > ds_clear(&match); > > + ds_clear(&actions); > > ds_put_format(&match, "ip && ip4.dst == %s", > > nat->external_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.dst=%s; next;", > > + nat->logical_ip); > > + } else { > > + ds_put_cstr(&actions, "ct_snat;"); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 90, > > - ds_cstr(&match), "ct_snat;"); > > + ds_cstr(&match), > > + ds_cstr(&actions)); > > } else { > > /* Distributed router. */ > > > > /* Traffic received on l3dgw_port is subject to NAT. */ > > ds_clear(&match); > > + ds_clear(&actions); > > + > > ds_put_format(&match, "ip && ip4.dst == %s" > > " && inport == %s", > > nat->external_ip, @@ -7136,8 > > +7160,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&match, " && is_chassis_resident(%s)", > > od->l3redirect_port->json_key); > > } > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.dst=%s; next;", > > + nat->logical_ip); > > + } else { > > + ds_put_cstr(&actions, "ct_snat;"); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 100, > > - ds_cstr(&match), "ct_snat;"); > > + ds_cstr(&match), > > + ds_cstr(&actions)); > > > > /* Traffic received on other router ports must be > > * redirected to the central instance of the > > l3dgw_port @@ -7172,8 +7204,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&actions, > > "flags.force_snat_for_dnat = 1; "); > > } > > - ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", > > - nat->logical_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "flags.loopback = 1; " > > + "ip4.dst=%s; next;", > > + nat->logical_ip); > > + } else { > > + ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", > > + nat->logical_ip); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, > > ds_cstr(&match), ds_cstr(&actions)); > > } else { > > @@ -7192,8 +7232,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > od->l3redirect_port->json_key); > > } > > ds_clear(&actions); > > - ds_put_format(&actions, "ct_dnat(%s);", > > - nat->logical_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.dst=%s; next;", > > + nat->logical_ip); > > + } else { > > + ds_put_format(&actions, "ct_dnat(%s);", > > + nat->logical_ip); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, > > ds_cstr(&match), > > ds_cstr(&actions)); > > > > @@ -7235,7 +7282,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", > > ETH_ADDR_ARGS(mac)); > > } > > - ds_put_format(&actions, "ct_dnat;"); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.src=%s; next;", > > + nat->external_ip); > > + } else { > > + ds_put_format(&actions, "ct_dnat;"); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_OUT_UNDNAT, 100, > > ds_cstr(&match), ds_cstr(&actions)); > > } > > @@ -7251,7 +7305,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&match, "ip && ip4.src == %s", > > nat->logical_ip); > > ds_clear(&actions); > > - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.src=%s; next;", > > + nat->external_ip); > > + } else { > > + ds_put_format(&actions, "ct_snat(%s);", > > + nat->external_ip); > > + } > > > > /* The priority here is calculated such that the > > * nat->logical_ip with the longest mask gets a > > higher @@ -7280,7 +7341,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", > > ETH_ADDR_ARGS(mac)); > > } > > - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.src=%s; next;", > > + nat->external_ip); > > + } else { > > + ds_put_format(&actions, "ct_snat(%s);", > > + nat->external_ip); > > + } > > > > /* The priority here is calculated such that the > > * nat->logical_ip with the longest mask gets a > > higher diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index > > 42033d5..64511a9 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -966,3 +966,53 @@ OVS_WAIT_UNTIL([ovn-sbctl get Port_Binding > > ${uuid} options:redirect-type], [0], > > ]) > > > > AT_CLEANUP > > + > > Can you add a new test case or modify an existing test case in ovn.at to make use of this feature ? > > > +AT_SETUP([ovn -- check stateless dnat_and_snat rule]) > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) 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 lr-nat-add R1 dnat_and_snat 172.16.1.1 50.0.0.11 > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], [0], [2 > > +]) > > There is a chance that the test case might fail on slower systems if ovn-northd is not fast enough to add the necessary logical flows. > > For the above first check, I would suggest to use - OVS_WAIT_UNTIL. > > > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [2 > > +]) > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [0 > > +]) > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [0 > > +]) > > + > > +ovn-nbctl lr-nat-del R1 dnat_and_snat 172.16.1.1 > > + > > +ovn-nbctl --stateless lr-nat-add R1 dnat_and_snat 172.16.1.1 > > +50.0.0.11 AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], > > +[0], [0 > > +]) > > + > > Same here. Please use OVS_WAIT_UNTIL. > > > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [0 > > +]) > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [2 > > +]) > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [2 > > +]) > > + > > +AT_CLEANUP > > -- > > 1.8.3.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch. > > org_mailman_listinfo_ovs-2Ddev&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=mZw > > X9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=CouZqk7agKapXA85Hmee871NM2d > > rC0etOpAwwwF_Qvw&s=yGzdgaPfGMkQpXg_3NmyQCOqkqeJPvomW3QqTsWghPQ&e= > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Numan, Just submitted V4, please take a look. Regards, Ankur -----Original Message----- From: Numan Siddique <numans@ovn.org> Sent: Thursday, October 31, 2019 10:39 AM To: Ankur Sharma <ankur.sharma@nutanix.com> Cc: ovs-dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v2 2/2 ovn] OVN: Use ipv4.src and ipv4.dst actions for NAT rules On Thu, Oct 31, 2019 at 2:02 AM Ankur Sharma <ankur.sharma@nutanix.com> wrote: > > Hi Numan, > > Submitted a v3 yesterday, addressing all the review comments. > Please take a look. Hi Ankur, Can you please rebase your v3 patches ? They don't apply on top of master. Thanks Numan > > Regards, > Ankur > > -----Original Message----- > From: Numan Siddique <numans@ovn.org> > Sent: Monday, October 28, 2019 6:40 AM > To: Ankur Sharma <ankur.sharma@nutanix.com> > Cc: ovs-dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v2 2/2 ovn] OVN: Use ipv4.src and > ipv4.dst actions for NAT rules > > On Sat, Oct 5, 2019 at 1:45 AM Ankur Sharma <ankur.sharma@nutanix.com> wrote: > > > > For dnat_and_snat rules which are meant to be stateless instead of > > using ct_snat/dnat OVN actions, we will use ipv4.src/ipv4.dst. > > > > This actions will do 1:1 mapping to inner ip to external ip, while > > recalculating the checksums. > > > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> > > > Hi Ankur, > > There is a checkpatch warning - WARNING: Line is 80 characters long (recommended limit is 79). > Please address it. > > Please see below for few comments. > > Thanks > Numan > > > --- > > northd/ovn-northd.8.xml | 34 +++++++++++++++---- > > northd/ovn-northd.c | 86 +++++++++++++++++++++++++++++++++++++++++++------ > > tests/ovn-northd.at | 50 ++++++++++++++++++++++++++++ > > 3 files changed, 154 insertions(+), 16 deletions(-) > > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index > > 0f4f1c1..7d5d102 100644 > > --- a/northd/ovn-northd.8.xml > > +++ b/northd/ovn-northd.8.xml > > @@ -1718,7 +1718,9 @@ icmp6 { > > to change the source IP address of a packet from <var>A</var> to > > <var>B</var>, a priority-90 flow matches <code>ip && > > ip4.dst == <var>B</var></code> with an action > > - <code>ct_snat; </code>. > > + <code>ct_snat; </code>. If the NAT rule is of type dnat_and_snat > > + and has <code>is_stateless=true</code> in the options, then the action > > + would be <code>replace_dst_ip(<var>B</var>)</code>. > > I think you need to update the documentation as you removed the action > - replace_dst_ip in v2. > > > > </p> > > > > <p> > > @@ -1738,7 +1740,10 @@ icmp6 { > > <var>B</var>, a priority-100 flow matches <code>ip && > > ip4.dst == <var>B</var> && inport == <var>GW</var></code>, > > where <var>GW</var> is the logical router gateway port, with an > > - action <code>ct_snat;</code>. > > + action <code>ct_snat;</code>. If the NAT rule is of type > > + dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_dst_ip > > Same here and other places too. > > > + (<var>B</var>)</code>. > > </p> > > > > <p> > > @@ -1858,7 +1863,10 @@ icmp6 { > > Gateway router is configured to force SNAT any DNATed packet, > > the above action will be replaced by > > <code>flags.force_snat_for_dnat = 1; flags.loopback = 1; > > - ct_dnat(<var>B</var>);</code>. > > + ct_dnat(<var>B</var>);</code>. If the NAT rule is of type > > + dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_dst_ip > > + (<var>B</var>)</code>. > > </li> > > > > <li> > > @@ -1890,7 +1898,10 @@ icmp6 { > > <var>B</var>, a priority-100 flow matches <code>ip && > > ip4.dst == <var>B</var> && inport == <var>GW</var></code>, > > where <var>GW</var> is the logical router gateway port, with an > > - action <code>ct_dnat(<var>B</var>);</code>. > > + action <code>ct_dnat(<var>B</var>);</code>. If the NAT rule is of > > + type dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_dst_ip > > + (<var>B</var>)</code>. > > </p> > > > > <p> > > @@ -2553,7 +2564,10 @@ nd_ns { > > matches <code>ip && ip4.src == <var>B</var> > > && outport == <var>GW</var></code>, where <var>GW</var> > > is the logical router gateway port, with an action > > - <code>ct_dnat;</code>. > > + <code>ct_dnat;</code>. If the NAT rule is of type > > + dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_src_ip > > + (<var>B</var>)</code>. > > </p> > > > > <p> > > @@ -2611,7 +2625,10 @@ nd_ns { > > <code>ip && ip4.src == <var>A</var></code> with an action > > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > > is calculated based on the mask of <var>A</var>, with matches > > - having larger masks getting higher priorities. > > + having larger masks getting higher priorities. If the NAT rule is > > + of type dnat_and_snat and has <code>is_stateless=true</code> in the > > + options, then the action would be <code>replace_src_ip > > + (<var>B</var>)</code>. > > </p> > > <p> > > A priority-0 logical flow with match <code>1</code> has > > actions @@ -2634,7 +2651,10 @@ nd_ns { > > logical router gateway port, with an action > > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > > is calculated based on the mask of <var>A</var>, with matches > > - having larger masks getting higher priorities. > > + having larger masks getting higher priorities. If the NAT rule > > + is of type dnat_and_snat and has <code>is_stateless=true</code> > > + in the options, then the action would be <code>replace_src_ip > > + (<var>B</var>)</code>. > > </p> > > > > <p> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index > > f393ceb..4036392 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -6314,6 +6314,18 @@ copy_ra_to_sb(struct ovn_port *op, const char *address_mode) > > smap_destroy(&options); > > } > > > > +static inline bool > > +lrouter_nat_is_stateless(const struct nbrec_nat *nat) { > > + const char *is_stateless = smap_get(&nat->options, > > +"is_stateless"); > > + > > + if (is_stateless && !strcmp(is_stateless, "true")) { > > + return true; > > + } > > + > > + return false; > > +} > > + > > static void > > build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > struct hmap *lflows, struct shash > > *meter_groups) @@ -7052,6 +7064,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > nat = od->nbr->nat[i]; > > > > ovs_be32 ip, mask; > > + bool is_stateless = lrouter_nat_is_stateless(nat); > > > > char *error = ip_parse_masked(nat->external_ip, &ip, &mask); > > if (error || mask != OVS_BE32_MAX) { @@ -7117,15 > > +7130,26 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > if (!od->l3dgw_port) { > > /* Gateway router. */ > > ds_clear(&match); > > + ds_clear(&actions); > > ds_put_format(&match, "ip && ip4.dst == %s", > > nat->external_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.dst=%s; next;", > > + nat->logical_ip); > > + } else { > > + ds_put_cstr(&actions, "ct_snat;"); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 90, > > - ds_cstr(&match), "ct_snat;"); > > + ds_cstr(&match), > > + ds_cstr(&actions)); > > } else { > > /* Distributed router. */ > > > > /* Traffic received on l3dgw_port is subject to NAT. */ > > ds_clear(&match); > > + ds_clear(&actions); > > + > > ds_put_format(&match, "ip && ip4.dst == %s" > > " && inport == %s", > > nat->external_ip, @@ -7136,8 > > +7160,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap > > +*ports, > > ds_put_format(&match, " && is_chassis_resident(%s)", > > od->l3redirect_port->json_key); > > } > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.dst=%s; next;", > > + nat->logical_ip); > > + } else { > > + ds_put_cstr(&actions, "ct_snat;"); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 100, > > - ds_cstr(&match), "ct_snat;"); > > + ds_cstr(&match), > > + ds_cstr(&actions)); > > > > /* Traffic received on other router ports must be > > * redirected to the central instance of the > > l3dgw_port @@ -7172,8 +7204,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&actions, > > "flags.force_snat_for_dnat = 1; "); > > } > > - ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", > > - nat->logical_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "flags.loopback = 1; " > > + "ip4.dst=%s; next;", > > + nat->logical_ip); > > + } else { > > + ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", > > + nat->logical_ip); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, > > ds_cstr(&match), ds_cstr(&actions)); > > } else { > > @@ -7192,8 +7232,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > od->l3redirect_port->json_key); > > } > > ds_clear(&actions); > > - ds_put_format(&actions, "ct_dnat(%s);", > > - nat->logical_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.dst=%s; next;", > > + nat->logical_ip); > > + } else { > > + ds_put_format(&actions, "ct_dnat(%s);", > > + nat->logical_ip); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, > > ds_cstr(&match), > > ds_cstr(&actions)); > > > > @@ -7235,7 +7282,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", > > ETH_ADDR_ARGS(mac)); > > } > > - ds_put_format(&actions, "ct_dnat;"); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.src=%s; next;", > > + nat->external_ip); > > + } else { > > + ds_put_format(&actions, "ct_dnat;"); > > + } > > + > > ovn_lflow_add(lflows, od, S_ROUTER_OUT_UNDNAT, 100, > > ds_cstr(&match), ds_cstr(&actions)); > > } > > @@ -7251,7 +7305,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&match, "ip && ip4.src == %s", > > nat->logical_ip); > > ds_clear(&actions); > > - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.src=%s; next;", > > + nat->external_ip); > > + } else { > > + ds_put_format(&actions, "ct_snat(%s);", > > + nat->external_ip); > > + } > > > > /* The priority here is calculated such that the > > * nat->logical_ip with the longest mask gets a > > higher @@ -7280,7 +7341,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", > > ETH_ADDR_ARGS(mac)); > > } > > - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); > > + > > + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { > > + ds_put_format(&actions, "ip4.src=%s; next;", > > + nat->external_ip); > > + } else { > > + ds_put_format(&actions, "ct_snat(%s);", > > + nat->external_ip); > > + } > > > > /* The priority here is calculated such that the > > * nat->logical_ip with the longest mask gets a > > higher diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index > > 42033d5..64511a9 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -966,3 +966,53 @@ OVS_WAIT_UNTIL([ovn-sbctl get Port_Binding > > ${uuid} options:redirect-type], [0], > > ]) > > > > AT_CLEANUP > > + > > Can you add a new test case or modify an existing test case in ovn.at to make use of this feature ? > > > +AT_SETUP([ovn -- check stateless dnat_and_snat rule]) > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) 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 lr-nat-add R1 dnat_and_snat 172.16.1.1 50.0.0.11 > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], [0], [2 > > +]) > > There is a chance that the test case might fail on slower systems if ovn-northd is not fast enough to add the necessary logical flows. > > For the above first check, I would suggest to use - OVS_WAIT_UNTIL. > > > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [2 > > +]) > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [0 > > +]) > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [0 > > +]) > > + > > +ovn-nbctl lr-nat-del R1 dnat_and_snat 172.16.1.1 > > + > > +ovn-nbctl --stateless lr-nat-add R1 dnat_and_snat 172.16.1.1 > > +50.0.0.11 AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc > > +-l], [0], [0 > > +]) > > + > > Same here. Please use OVS_WAIT_UNTIL. > > > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [0 > > +]) > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [2 > > +]) > > + > > +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [2 > > +]) > > + > > +AT_CLEANUP > > -- > > 1.8.3.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch. > > org_mailman_listinfo_ovs-2Ddev&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=m > > Zw > > X9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=CouZqk7agKapXA85Hmee871NM > > 2d rC0etOpAwwwF_Qvw&s=yGzdgaPfGMkQpXg_3NmyQCOqkqeJPvomW3QqTsWghPQ&e= > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch. > org_mailman_listinfo_ovs-2Ddev&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=mZw > X9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=FoT09vI9qbn00jTWguXBPw_kYud > y-Wq8BfPkXLWJOa4&s=W5wu37wMH2lS-OF3c4rTe7_ZNJWutAlxru5RWqyeCwQ&e=
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 0f4f1c1..7d5d102 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -1718,7 +1718,9 @@ icmp6 { to change the source IP address of a packet from <var>A</var> to <var>B</var>, a priority-90 flow matches <code>ip && ip4.dst == <var>B</var></code> with an action - <code>ct_snat; </code>. + <code>ct_snat; </code>. If the NAT rule is of type dnat_and_snat + and has <code>is_stateless=true</code> in the options, then the action + would be <code>replace_dst_ip(<var>B</var>)</code>. </p> <p> @@ -1738,7 +1740,10 @@ icmp6 { <var>B</var>, a priority-100 flow matches <code>ip && ip4.dst == <var>B</var> && inport == <var>GW</var></code>, where <var>GW</var> is the logical router gateway port, with an - action <code>ct_snat;</code>. + action <code>ct_snat;</code>. If the NAT rule is of type + dnat_and_snat and has <code>is_stateless=true</code> in the + options, then the action would be <code>replace_dst_ip + (<var>B</var>)</code>. </p> <p> @@ -1858,7 +1863,10 @@ icmp6 { Gateway router is configured to force SNAT any DNATed packet, the above action will be replaced by <code>flags.force_snat_for_dnat = 1; flags.loopback = 1; - ct_dnat(<var>B</var>);</code>. + ct_dnat(<var>B</var>);</code>. If the NAT rule is of type + dnat_and_snat and has <code>is_stateless=true</code> in the + options, then the action would be <code>replace_dst_ip + (<var>B</var>)</code>. </li> <li> @@ -1890,7 +1898,10 @@ icmp6 { <var>B</var>, a priority-100 flow matches <code>ip && ip4.dst == <var>B</var> && inport == <var>GW</var></code>, where <var>GW</var> is the logical router gateway port, with an - action <code>ct_dnat(<var>B</var>);</code>. + action <code>ct_dnat(<var>B</var>);</code>. If the NAT rule is of + type dnat_and_snat and has <code>is_stateless=true</code> in the + options, then the action would be <code>replace_dst_ip + (<var>B</var>)</code>. </p> <p> @@ -2553,7 +2564,10 @@ nd_ns { matches <code>ip && ip4.src == <var>B</var> && outport == <var>GW</var></code>, where <var>GW</var> is the logical router gateway port, with an action - <code>ct_dnat;</code>. + <code>ct_dnat;</code>. If the NAT rule is of type + dnat_and_snat and has <code>is_stateless=true</code> in the + options, then the action would be <code>replace_src_ip + (<var>B</var>)</code>. </p> <p> @@ -2611,7 +2625,10 @@ nd_ns { <code>ip && ip4.src == <var>A</var></code> with an action <code>ct_snat(<var>B</var>);</code>. The priority of the flow is calculated based on the mask of <var>A</var>, with matches - having larger masks getting higher priorities. + having larger masks getting higher priorities. If the NAT rule is + of type dnat_and_snat and has <code>is_stateless=true</code> in the + options, then the action would be <code>replace_src_ip + (<var>B</var>)</code>. </p> <p> A priority-0 logical flow with match <code>1</code> has actions @@ -2634,7 +2651,10 @@ nd_ns { logical router gateway port, with an action <code>ct_snat(<var>B</var>);</code>. The priority of the flow is calculated based on the mask of <var>A</var>, with matches - having larger masks getting higher priorities. + having larger masks getting higher priorities. If the NAT rule + is of type dnat_and_snat and has <code>is_stateless=true</code> + in the options, then the action would be <code>replace_src_ip + (<var>B</var>)</code>. </p> <p> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index f393ceb..4036392 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -6314,6 +6314,18 @@ copy_ra_to_sb(struct ovn_port *op, const char *address_mode) smap_destroy(&options); } +static inline bool +lrouter_nat_is_stateless(const struct nbrec_nat *nat) +{ + const char *is_stateless = smap_get(&nat->options, "is_stateless"); + + if (is_stateless && !strcmp(is_stateless, "true")) { + return true; + } + + return false; +} + static void build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, struct hmap *lflows, struct shash *meter_groups) @@ -7052,6 +7064,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, nat = od->nbr->nat[i]; ovs_be32 ip, mask; + bool is_stateless = lrouter_nat_is_stateless(nat); char *error = ip_parse_masked(nat->external_ip, &ip, &mask); if (error || mask != OVS_BE32_MAX) { @@ -7117,15 +7130,26 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (!od->l3dgw_port) { /* Gateway router. */ ds_clear(&match); + ds_clear(&actions); ds_put_format(&match, "ip && ip4.dst == %s", nat->external_ip); + + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { + ds_put_format(&actions, "ip4.dst=%s; next;", + nat->logical_ip); + } else { + ds_put_cstr(&actions, "ct_snat;"); + } + ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 90, - ds_cstr(&match), "ct_snat;"); + ds_cstr(&match), ds_cstr(&actions)); } else { /* Distributed router. */ /* Traffic received on l3dgw_port is subject to NAT. */ ds_clear(&match); + ds_clear(&actions); + ds_put_format(&match, "ip && ip4.dst == %s" " && inport == %s", nat->external_ip, @@ -7136,8 +7160,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, ds_put_format(&match, " && is_chassis_resident(%s)", od->l3redirect_port->json_key); } + + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { + ds_put_format(&actions, "ip4.dst=%s; next;", + nat->logical_ip); + } else { + ds_put_cstr(&actions, "ct_snat;"); + } + ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 100, - ds_cstr(&match), "ct_snat;"); + ds_cstr(&match), ds_cstr(&actions)); /* Traffic received on other router ports must be * redirected to the central instance of the l3dgw_port @@ -7172,8 +7204,16 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, ds_put_format(&actions, "flags.force_snat_for_dnat = 1; "); } - ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", - nat->logical_ip); + + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { + ds_put_format(&actions, "flags.loopback = 1; " + "ip4.dst=%s; next;", + nat->logical_ip); + } else { + ds_put_format(&actions, "flags.loopback = 1; ct_dnat(%s);", + nat->logical_ip); + } + ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, ds_cstr(&match), ds_cstr(&actions)); } else { @@ -7192,8 +7232,15 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, od->l3redirect_port->json_key); } ds_clear(&actions); - ds_put_format(&actions, "ct_dnat(%s);", - nat->logical_ip); + + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { + ds_put_format(&actions, "ip4.dst=%s; next;", + nat->logical_ip); + } else { + ds_put_format(&actions, "ct_dnat(%s);", + nat->logical_ip); + } + ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 100, ds_cstr(&match), ds_cstr(&actions)); @@ -7235,7 +7282,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", ETH_ADDR_ARGS(mac)); } - ds_put_format(&actions, "ct_dnat;"); + + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { + ds_put_format(&actions, "ip4.src=%s; next;", + nat->external_ip); + } else { + ds_put_format(&actions, "ct_dnat;"); + } + ovn_lflow_add(lflows, od, S_ROUTER_OUT_UNDNAT, 100, ds_cstr(&match), ds_cstr(&actions)); } @@ -7251,7 +7305,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, ds_put_format(&match, "ip && ip4.src == %s", nat->logical_ip); ds_clear(&actions); - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); + + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { + ds_put_format(&actions, "ip4.src=%s; next;", + nat->external_ip); + } else { + ds_put_format(&actions, "ct_snat(%s);", + nat->external_ip); + } /* The priority here is calculated such that the * nat->logical_ip with the longest mask gets a higher @@ -7280,7 +7341,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, ds_put_format(&actions, "eth.src = "ETH_ADDR_FMT"; ", ETH_ADDR_ARGS(mac)); } - ds_put_format(&actions, "ct_snat(%s);", nat->external_ip); + + if (!strcmp(nat->type, "dnat_and_snat") && is_stateless) { + ds_put_format(&actions, "ip4.src=%s; next;", + nat->external_ip); + } else { + ds_put_format(&actions, "ct_snat(%s);", + nat->external_ip); + } /* The priority here is calculated such that the * nat->logical_ip with the longest mask gets a higher diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 42033d5..64511a9 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -966,3 +966,53 @@ OVS_WAIT_UNTIL([ovn-sbctl get Port_Binding ${uuid} options:redirect-type], [0], ]) AT_CLEANUP + +AT_SETUP([ovn -- check stateless dnat_and_snat rule]) +AT_SKIP_IF([test $HAVE_PYTHON = no]) +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 lr-nat-add R1 dnat_and_snat 172.16.1.1 50.0.0.11 +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], [0], [2 +]) + +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [2 +]) + +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [0 +]) + +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [0 +]) + +ovn-nbctl lr-nat-del R1 dnat_and_snat 172.16.1.1 + +ovn-nbctl --stateless lr-nat-add R1 dnat_and_snat 172.16.1.1 50.0.0.11 +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_snat | wc -l], [0], [0 +]) + +AT_CHECK([ovn-sbctl dump-flows R1 | grep ct_dnat | wc -l], [0], [0 +]) + +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.dst=| wc -l], [0], [2 +]) + +AT_CHECK([ovn-sbctl dump-flows R1 | grep ip4.src=| wc -l], [0], [2 +]) + +AT_CLEANUP
For dnat_and_snat rules which are meant to be stateless instead of using ct_snat/dnat OVN actions, we will use ipv4.src/ipv4.dst. This actions will do 1:1 mapping to inner ip to external ip, while recalculating the checksums. Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> --- northd/ovn-northd.8.xml | 34 +++++++++++++++---- northd/ovn-northd.c | 86 +++++++++++++++++++++++++++++++++++++++++++------ tests/ovn-northd.at | 50 ++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 16 deletions(-)