Hi, Numan
The previous patch is wrong, because the snat lflow is added to the
lr_out_undnat stage and action is ct_dnat_in_czone.(table=1 lflow)
table=3 (lr_out_snat ), priority=153 , match=(ip && ip4.src ==
192.168.200.0/24 && outport == "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
action=(ct_snat_in_czone(1.1.1.124);)
table=1 (lr_out_undnat ), priority=100 , match=(ip && ip4.src ==
192.168.200.0/24 && outport == "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
action=(ct_dnat_in_czone;)
The snat flow is split into 2 lflow
Function build_lrouter_out_snat_flow is build ct.new snat lflow
Another function build_lrouter_out_snat_flow2 is build ct.est snat lflow, the
lflow priority is higher than ct.new snat lflow
Does my fix make sense?
B R
Wentao Jia
diff --git a/northd/northd.c b/northd/northd.c
index 6771ccce5..77f160770 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -13298,7 +13298,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct
ovn_datapath *od,
bool stateless = lrouter_nat_is_stateless(nat);
if (od->is_gw_router) {
ds_clear(match);
- ds_put_format(match, "ip && ip%s.src == %s",
+ ds_put_format(match, "ct.new && ip && ip%s.src == %s",
is_v6 ? "6" : "4", nat->logical_ip);
ds_clear(actions);
@@ -13332,7 +13332,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct
ovn_datapath *od,
/* Distributed router. */
ds_clear(match);
- ds_put_format(match, "ip && ip%s.src == %s && outport == %s",
+ ds_put_format(match, "ct.new && ip && ip%s.src == %s && outport == %s",
is_v6 ? "6" : "4", nat->logical_ip,
l3dgw_port->json_key);
if (od->n_l3dgw_ports) {
@@ -13400,6 +13400,128 @@ build_lrouter_out_snat_flow(struct hmap *lflows,
struct ovn_datapath *od,
}
}
+static void
+build_lrouter_out_snat_flow2(struct hmap *lflows, struct ovn_datapath *od,
+ const struct nbrec_nat *nat, struct ds *match,
+ struct ds *actions, bool distributed,
+ struct eth_addr mac, int cidr_bits, bool is_v6,
+ struct ovn_port *l3dgw_port)
+{
+ /* Egress SNAT table: Packets enter the egress pipeline with
+ * source ip address that needs to be SNATted to a external ip
+ * address. */
+ if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) {
+ return;
+ }
+
+ bool stateless = lrouter_nat_is_stateless(nat);
+ if (od->is_gw_router) {
+ ds_clear(match);
+ ds_put_format(match, "ct.est && ip && ip%s.src == %s",
+ is_v6 ? "6" : "4", nat->logical_ip);
+ ds_clear(actions);
+
+ if (nat->allowed_ext_ips || nat->exempted_ext_ips) {
+ lrouter_nat_add_ext_ip_match(od, lflows, match, nat,
+ is_v6, false, cidr_bits);
+ }
+
+ if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
+ ds_put_format(actions, "ip%s.src=%s; next;",
+ is_v6 ? "6" : "4", nat->external_ip);
+ } else {
+ ds_put_format(match, " && (!ct.trk || !ct.rpl)");
+ ds_put_format(actions, "ct_snat");
+// ds_put_format(actions, "ct_snat(%s", nat->external_ip);
+//
+// if (nat->external_port_range[0]) {
+// ds_put_format(actions, ",%s",
+// nat->external_port_range);
+// }
+ ds_put_format(actions, ");");
+ }
+
+ /* The priority here is calculated such that the
+ * nat->logical_ip with the longest mask gets a higher
+ * priority. */
+ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_SNAT,
+ cidr_bits + 1, ds_cstr(match),
+ ds_cstr(actions), &nat->header_);
+ } else {
+ uint16_t priority = cidr_bits + 5;
+
+ /* Distributed router. */
+ ds_clear(match);
+ ds_put_format(match, "ct.est && ip && ip%s.src == %s && outport == %s",
+ is_v6 ? "6" : "4", nat->logical_ip,
+ l3dgw_port->json_key);
+ if (od->n_l3dgw_ports) {
+ if (distributed) {
+ ovs_assert(nat->logical_port);
+ priority += 128;
+ ds_put_format(match, " && is_chassis_resident(\"%s\")",
+ nat->logical_port);
+ } else {
+ /* Flows for NAT rules that are centralized are only
+ * programmed on the gateway chassis. */
+ priority += 128;
+ ds_put_format(match, " && is_chassis_resident(%s)",
+ l3dgw_port->cr_port->json_key);
+ }
+ }
+ ds_clear(actions);
+
+ if (nat->allowed_ext_ips || nat->exempted_ext_ips) {
+ lrouter_nat_add_ext_ip_match(od, lflows, match, nat,
+ is_v6, false, cidr_bits);
+ }
+
+ if (distributed) {
+ ds_put_format(actions, "eth.src = "ETH_ADDR_FMT"; ",
+ ETH_ADDR_ARGS(mac));
+ }
+
+ if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
+ ds_put_format(actions, "ip%s.src=%s; next;",
+ is_v6 ? "6" : "4", nat->external_ip);
+ } else {
+ ds_put_format(actions, "ct_snat_in_czone");
+// ds_put_format(actions, "ct_snat_in_czone(%s",
+// nat->external_ip);
+// if (nat->external_port_range[0]) {
+// ds_put_format(actions, ",%s", nat->external_port_range);
+// }
+ ds_put_format(actions, ");");
+ }
+
+ /* The priority here is calculated such that the
+ * nat->logical_ip with the longest mask gets a higher
+ * priority. */
+ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_SNAT,
+ priority, ds_cstr(match),
+ ds_cstr(actions), &nat->header_);
+
+ if (!stateless) {
+ ds_put_cstr(match, " && "REGBIT_DST_NAT_IP_LOCAL" == 1");
+ ds_clear(actions);
+ if (distributed) {
+ ds_put_format(actions, "eth.src = "ETH_ADDR_FMT"; ",
+ ETH_ADDR_ARGS(mac));
+ }
+ ds_put_format(actions, REGBIT_DST_NAT_IP_LOCAL" = 0;
ct_snat");
+// ds_put_format(actions, REGBIT_DST_NAT_IP_LOCAL" = 0;
ct_snat(%s",
+// nat->external_ip);
+// if (nat->external_port_range[0]) {
+// ds_put_format(actions, ",%s", nat->external_port_range);
+// }
+ ds_put_format(actions, ");");
+ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_SNAT,
+ priority + 1, ds_cstr(match),
+ ds_cstr(actions), &nat->header_);
+ }
+ }
+}
+
static void
build_lrouter_ingress_nat_check_pkt_len(struct hmap *lflows,
const struct nbrec_nat *nat,
@@ -13788,6 +13910,8 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
*od, struct hmap *lflows,
/* S_ROUTER_OUT_SNAT */
build_lrouter_out_snat_flow(lflows, od, nat, match, actions,
distributed,
mac, cidr_bits, is_v6, l3dgw_port);
+ build_lrouter_out_snat_flow2(lflows, od, nat, match, actions,
distributed,
+ mac, cidr_bits, is_v6, l3dgw_port);
/* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */
build_lrouter_ingress_flow(lflows, od, nat, match, actions, mac,
-----Original Message-----
From: Numan Siddique <[email protected]>
Sent: Tuesday, October 11, 2022 9:39 AM
To: Wentao Jia <[email protected]>
Cc: [email protected]
Subject: Re: [ovs-dev] [OVN PATCH] northd: add unsnat/undnat lflow for
established connections
On Sun, Oct 9, 2022 at 5:03 AM Wentao Jia <[email protected]> wrote:
>
> Hi, Numan
>
> Sorry for long delay
>
> Base on your suggestions, lflow ct_snat_in_czone is split into 2 lflows，one
> lflow for ct.new, and other for ct.est.
> table=3 (lr_out_snat ), priority=154 , match=(ct.est && ip &&
> ip4.src == 192.168.200.0/24 && outport ==
> "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> action=(ct_snat_in_czone);)
> table=3 (lr_out_snat ), priority=153 , match=(ct.new && ip &&
> ip4.src == 192.168.200.0/24 && outport ==
> "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> action=(ct_snat_in_czone(1.1.1.124);)
My suggest is to have logical flows like below
------
table=1 (lr_out_undnat ), priority=100 , match=(ip && ip4.src
== 192.168.200.0/24 && outport == "lr0-public" &&
is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone;)
table=1 (lr_out_undnat ), priority=100 , match=(ip && ip4.src
== 192.168.200.0/24 && outport == "lr0-public" &&
is_chassis_resident("cr-lr0-public") && reg9[4] == 1),
action=(ct_snat;)
table=3 (lr_out_snat ), priority=154 , match=(ip && ip4.src
== 192.168.200.0/24 && outport == "lr0-public" &&
is_chassis_resident("cr-lr0-public") && reg9[4] == 1 && ct.new),
action=(reg9[4] = 0; ct_snat(1.1.1.124);)
table=3 (lr_out_snat ), priority=153 , match=(ip && ip4.src
==192.168.200.0/24 && outport == "lr0-public" &&
is_chassis_resident("cr-lr0-public") && ct.new),
action=(ct_snat_in_czone(1.1.1.124);)
-----
If you could share your patch or share it in your github repo, I could
probably try it out and comment.
Thanks
Numan
>
> traffic is failed, the nat action is skipped
> ufid:0e021319-00ce-4869-bb5a-aebfb6172eb9,
> skb_priority(0/0),skb_mark(0/0),ct_state(0/0x3f),ct_zone(0/0),ct_mark(
> 0/0x3),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_2),packet_
> type(ns=0/0,id=0/0),eth(src=fa:16:3e:4c:e6:1f,dst=fa:16:3e:c9:a7:6b),e
> th_type(0x0800),ipv4(src=192.168.210.103,dst=1.1.1.254,proto=6,tos=0/0
> ,ttl=64,frag=no),tcp(src=0/0,dst=0/0), packets:2, bytes:148,
> used:3.230s, offloaded:yes, dp:tc,
> actions:set(eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:43)),set(ipv4
> (ttl=63)),enp1s0np1
>
> there maybe some wrong with ct_snat_in_czone, but I can not find it
> out do you have any idea about this ?
>
> B R
> Wentao Jia
>
> -----Original Message-----
> From: Numan Siddique <[email protected]>
> Sent: Thursday, August 18, 2022 11:56 AM
> To: Wentao Jia <[email protected]>
> Cc: [email protected]
> Subject: Re: [ovs-dev] [OVN PATCH] northd: add unsnat/undnat lflow for
> established connections
>
> On Thu, Aug 18, 2022 at 1:16 PM Wentao Jia <[email protected]> wrote:
> >
> >
> > Hi, Numan
> >
> > It is not difficult to reproduce，this is about snat/dnat rules
> > create router and create different type of nat rules of this router
> > no other issues to be considered。
> >
>
> Right. I now see the issue locally. I do see that the packet from the VM to
> outside is always committed to the conntrack (along with snat) and that's
> causing the HWOL issues.
>
> But I don't think the proposed solution is the right fix. Looks like CI
> didn't trigger for your patch here [1].
> And I pushed your patch to my local github repo and you can see lots of test
> failures - [2].
>
> In order to address this HWOL limitation, we should first send the packet to
> conntrack with the action - ct_snat_in_czone (in router egress pipeline).
> If the connection was already committed, this would snat it. If it's a new
> connection we should commit to it.
>
> Seems to me we should have flows like below (assuming there is an
> SNAT entry for 192.168.200.0/24 <-> 1.1.1.124) (please note I may not
> be accurate with the below flows)
>
> table=1 (lr_out_undnat ), priority=100 , match=(ip && ip4.src
> == 192.168.200.0/24 && outport == "lr0-public" &&
> is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone;)
> table=1 (lr_out_undnat ), priority=100 , match=(ip && ip4.src
> == 192.168.200.0/24 && outport == "lr0-public" &&
> is_chassis_resident("cr-lr0-public") && reg9[4] == 1),
> action=(ct_snat;)
> table=3 (lr_out_snat ), priority=154 , match=(ip && ip4.src
> == 192.168.200.0/24 && outport == "lr0-public" &&
> is_chassis_resident("cr-lr0-public") && reg9[4] == 1 && ct.new),
> action=(reg9[4] = 0; ct_snat(1.1.1.124);)
> table=3 (lr_out_snat ), priority=153 , match=(ip && ip4.src
> ==192.168.200.0/24 && outport == "lr0-public" &&
> is_chassis_resident("cr-lr0-public") && ct.new),
> action=(ct_snat_in_czone(1.1.1.124);)
>
> Notice the "ct.new" in table 3 flows.
>
> [1] -
> https://patchwork.ozlabs.org/project/ovn/patch/BN6PR13MB120270F798BE46
> [email protected]/
> [2] - https://github.com/numansiddique/ovn/tree/ct_commit_snat_issue
>
> https://github.com/numansiddique/ovn/runs/7891302586?check_suite_focus
> =true
>
> Please also add documentation in ovn-northd.8.xml about the new logical flows
> you'd be adding, fix any existing test case failures and also add a system
> test if possible.
>
> Thanks
> Numan
>
> > Wentao Jia
> >
> > -----Original Message-----
> > From: Numan Siddique <[email protected]>
> > Sent: Thursday, August 18, 2022 10:30 AM
> > To: Wentao Jia <[email protected]>
> > Cc: [email protected]
> > Subject: Re: [ovs-dev] [OVN PATCH] northd: add unsnat/undnat lflow
> > for established connections
> >
> > On Thu, Aug 18, 2022 at 12:16 PM Wentao Jia <[email protected]> wrote:
> > >
> > >
> > > Hi, Numan
> > >
> > > This is snat rule bug，when a snat rule set，this issue is
> > > reproduced you can find the different between snat rule and
> > > dnat_and_snat rule Logical flow, ovs openflow and ovs datapath
> > > flow are all different
> > >
> > > network topology and nat rules in last mail is not enough to reproduce
> > > this?
> >
> > Please understand that for me to reproduce locally, I need to create all
> > the switches and routers manually or script it from your shared topology
> > and I may miss out on a few details.
> > Having the OVN Northbound database from you would definitely make it easier
> > for me.
> >
> > Numan
> >
> > >
> > > The switch and router table in northbound switch
> > > 62b0ba88-554e-46dd-9c45-cff840af9626
> > > (neutron-27eb266a-de05-4db6-a142-8f8dda7ed0aa) (aka priv-net)
> > > port 484688a3-79dd-40e7-a075-66441b34e1c7
> > > type: localport
> > > addresses: ["fa:16:3e:cd:8d:1b 192.168.200.2"]
> > > port 478ccbbd-680c-4f70-a87b-178462695926
> > > type: router
> > > router-port: lrp-478ccbbd-680c-4f70-a87b-178462695926
> > > port e1d7711b-1d29-4f53-9527-4740ff8ee653
> > > type: router
> > > router-port: lrp-e1d7711b-1d29-4f53-9527-4740ff8ee653
> > > port babced4e-f5ba-4130-8d12-61f48d58592b (aka port100)
> > > addresses: ["fa:16:3e:42:31:df 192.168.200.238", "unknown"]
> > > port 940d7a3d-5262-49fa-b2f0-0055f9f70fd3 (aka port103)
> > > addresses: ["fa:16:3e:4c:e6:1f 192.168.210.103", "unknown"]
> > > port 139a94d3-1eac-49e9-b4f3-052d20c4001e (aka port101)
> > > addresses: ["fa:16:3e:a1:50:79 192.168.200.165", "unknown"]
> > > port 47ca6c1e-5045-4177-9da3-1535ee9cbed7 (aka port102)
> > > addresses: ["fa:16:3e:a5:44:f0 192.168.210.102",
> > > "unknown"]
> > >
> > > router 2a180a00-3ccc-4537-8978-05f4fcf1b0ee
> > > (neutron-35b1a8c5-82dd-4fc4-83f9-8dfd2e4fb43c) (aka route2)
> > > port lrp-0e701852-85a0-4c57-8014-77a8607a9406
> > > mac: "fa:16:3e:ae:b5:e5"
> > > networks: ["1.1.1.124/24"]
> > > gateway chassis: [c7bcd287-db8b-4ad8-b38e-bbd69079a2cb]
> > > port lrp-e1d7711b-1d29-4f53-9527-4740ff8ee653
> > > mac: "fa:16:3e:39:69:0a"
> > > networks: ["192.168.200.1/24"]
> > > port lrp-478ccbbd-680c-4f70-a87b-178462695926
> > > mac: "fa:16:3e:c9:a7:6b"
> > > networks: ["192.168.210.1/24"]
> > > nat 1ba5d885-91ef-4c3b-a40e-ef0633c33b02
> > > external ip: "1.1.1.45"
> > > logical ip: "192.168.210.102"
> > > type: "dnat_and_snat"
> > > nat 88b9371b-a058-46b5-91f9-6741edf3391a
> > > external ip: "1.1.1.124"
> > > logical ip: "192.168.210.0/24"
> > > type: "snat"
> > > nat f9843d73-341a-4557-b996-ced76b4fa8e9
> > > external ip: "1.1.1.124"
> > > logical ip: "192.168.200.0/24"
> > > type: "snat"
> > >
> > > B R
> > > Wentao Jia
> > >
> > > -----Original Message-----
> > > From: Numan Siddique <[email protected]>
> > > Sent: Thursday, August 18, 2022 9:44 AM
> > > To: Wentao Jia <[email protected]>
> > > Cc: Ales Musil <[email protected]>; [email protected]
> > > Subject: Re: [ovs-dev] [OVN PATCH] northd: add unsnat/undnat lflow
> > > for established connections
> > >
> > > On Thu, Aug 11, 2022 at 4:57 PM Wentao Jia <[email protected]>
> > > wrote:
> > > >
> > > > Hi, Numan
> > > >
> > > > Thanks for you reply.
> > > >
> > > > The network topology as follows:
> > > >
> > > > vm1 -----> logical switch 1
> > > > (192.168.200.165) \
> > > > -----------> router ---------> extenel-net
> > > > /
> > > > vm2 -----> logical switch 2
> > > > (192.168.210.102)
> > > >
> > > > Nat rules of the rotuer:
> > > > TYPE EXTERNAL_IP LOGICAL_IP
> > > > dnat_and_snat 1.1.1.166 192.168.210.102
> > > > snat 1.1.1.124 192.168.200.0/24
> > > > snat 1.1.1.124 192.168.210.0/24
> > > >
> > > > there is only lr_out_snat logical flows for snat rules, no
> > > > lr_out_undnat logical flow.
> > > > table=3 (lr_out_snat ), priority=153 , match=(ip && ip4.src
> > > > == 192.168.200.0/24 && outport ==
> > > > "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_snat_in_czone(1.1.1.124);)
> > > > table=3 (lr_out_snat ), priority=153 , match=(ip && ip4.src
> > > > == 192.168.210.0/24 && outport ==
> > > > "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_snat_in_czone(1.1.1.124);)
> > > >
> > > > but there are lr_out_snat and lr_out_undnat logical flows for
> > > > dnat_and_snat rule
> > > > table=1 (lr_out_undnat ), priority=100 , match=(ip && ip4.src
> > > > == 192.168.210.102 && outport ==
> > > > "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_dnat_in_czone;)
> > > > table=3 (lr_out_snat ), priority=161 , match=(ip && ip4.src
> > > > == 192.168.210.102 && outport ==
> > > > "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_snat_in_czone(1.1.1.166);)
> > > >
> > > > and there are lr_in_dnat and lr_in_unsnat logical flows for
> > > > dnat_and_snat rule too
> > > > table=4 (lr_in_unsnat ), priority=100 , match=(ip && ip4.dst
> > > > == 1.1.1.166 && inport == "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > flags.loopback == 0 &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_snat_in_czone;)
> > > > table=4 (lr_in_unsnat ), priority=100 , match=(ip && ip4.dst
> > > > == 1.1.1.166 && inport == "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > flags.loopback == 1 && flags.use_snat_zone == 1 &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_snat;)
> > > > table=6 (lr_in_dnat ), priority=100 , match=(ip && ip4.dst
> > > > == 1.1.1.166 && inport == "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_dnat_in_czone(192.168.210.102);)
> > > >
> > > > after patched, another lr_out_undnat logical flow for snat rule and
> > > > another lr_in_unsnat logical flow for dnat rule, lr_out_undnat logical
> > > > flows as follows:
> > > > table=1 (lr_out_undnat ), priority=100 , match=(ip && ip4.src
> > > > == 192.168.200.0/24 && outport ==
> > > > "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_dnat_in_czone;)
> > > > table=1 (lr_out_undnat ), priority=100 , match=(ip && ip4.src
> > > > == 192.168.210.0/24 && outport ==
> > > > "lrp-0e701852-85a0-4c57-8014-77a8607a9406" &&
> > > > is_chassis_resident("cr-lrp-0e701852-85a0-4c57-8014-77a8607a9406")),
> > > > action=(ct_dnat_in_czone;)
> > >
> > >
> > > Can you share the ovn-nbctl commands to reproduce this ? or even better
> > > if you can share your OVN Northbound database so that I can locally try ?
> > >
> > > Thanks
> > > Numan
> > >
> > > >
> > > > B R
> > > > Wentao Jia
> > > >
> > > > -----Original Message-----
> > > > From: Numan Siddique <[email protected]>
> > > > Sent: Thursday, August 11, 2022 12:11 PM
> > > > To: Ales Musil <[email protected]>
> > > > Cc: Wentao Jia <[email protected]>; [email protected]
> > > > Subject: Re: [ovs-dev] [OVN PATCH] northd: add unsnat/undnat
> > > > lflow for established connections
> > > >
> > > > On Tue, Aug 9, 2022 at 9:46 PM Ales Musil <[email protected]> wrote:
> > > > >
> > > > > On Tue, Aug 2, 2022 at 9:05 PM Wentao Jia <[email protected]>
> > > > > wrote:
> > > > >
> > > > > > snat/dnat rules of logical router, no logical flows for
> > > > > > established connection, all natted packets deliver to kernel
> > > > > > conntrack module by ct commit, this is low performance and
> > > > > > difficult to offload.
> > > > > > add another logical flows without ct commit forestablished
> > > > > > on pipeline stage of unsnat/undnat for logical router
> > > > > >
> > > > > > before patched, datapath flows for nat with ct commit
> > > > > > ufid:db1fbd1b-8f16-4681-81b0-3796d60332a8,
> > > > > > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),c
> > > > > > t_
> > > > > > ma
> > > > > > rk
> > > > > > (0
> > > > > > /0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),
> > > > > > pa
> > > > > > ck
> > > > > > et
> > > > > > _t
> > > > > > ype(ns=0/0,id=0/0),eth(src=fa:16:3e:a1:50:79,dst=fa:16:3e:39:69:
> > > > > > 0a
> > > > > > ),
> > > > > > eth_type(0x0800),ipv4(src=
> > > > > > 192.168.200.128/255.255.255.192,dst=1.1.1.254,proto=6,tos=0/
> > > > > > 0,
> > > > > > tt
> > > > > > l=
> > > > > > 64 ,frag=no),tcp(src=0/0,dst=0/0), packets:2969075,
> > > > > > bytes:4071176800, used:0.000s, dp:tc,
> > > > > > actions:set(eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:43)
> > > > > > ),
> > > > > > se
> > > > > > t(
> > > > > > ip
> > > > > > v4(ttl=63)),ct(commit,zone=22,nat(src=1.1.1.124)),recirc(0x1
> > > > > > 4b
> > > > > > ) ufid:e9c5df95-02df-4629-b399-ddeb5581e997,
> > > > > > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),c
> > > > > > t_
> > > > > > ma
> > > > > > rk
> > > > > > (0
> > > > > > /0),ct_label(0/0),recirc_id(0x14b),dp_hash(0/0),in_port(rep0
> > > > > > _0
> > > > > > ),
> > > > > > pa
> > > > > > ck
> > > > > > et_type(ns=0/0,id=0/0),eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:
> > > > > > 43),eth_type(0x0800),ipv4(src=
> > > > > > 0.0.0.0/0.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,tos=
> > > > > > 0/ 0, tt l= 0/0,frag=no), packets:2969075, bytes:4071176800,
> > > > > > used:0.001s, offloaded:yes, dp:tc,
> > > > > > actions:ct_clear,enp1s0np1
> > > > > >
> > > > > > after patched, there is two flows for nat, the flow with ct
> > > > > > commit will be timeout and deleted after connection established.
> > > > > > another flow without ct commit for established connection
> > > > > > ufid:f6a591d6-de32-49cc-bf03-7a00a7601ad0,
> > > > > > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),c
> > > > > > t_
> > > > > > ma
> > > > > > rk
> > > > > > (0
> > > > > > /0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),
> > > > > > pa
> > > > > > ck
> > > > > > et
> > > > > > _t
> > > > > > ype(ns=0/0,id=0/0),eth(src=fa:16:3e:a1:50:79,dst=fa:16:3e:39:69:
> > > > > > 0a
> > > > > > ),
> > > > > > eth_type(0x0800),ipv4(src=192.168.200.165,dst=1.1.1.254,prot
> > > > > > o= 6, to s= 0/0,ttl=64,frag=no),tcp(src=0/0,dst=0/0),
> > > > > > packets:5518542, bytes:7924612730, used:0.040s,
> > > > > > offloaded:yes, dp:tc,
> > > > > > actions:set(eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:43)
> > > > > > ),
> > > > > > se
> > > > > > t(
> > > > > > ip
> > > > > > v4(ttl=63)),ct(zone=22,nat),recirc(0x14d)
> > > > > > ufid:6af5a3ed-5920-4f5b-923e-7e2cb5fc3d6c,
> > > > > > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),c
> > > > > > t_
> > > > > > ma
> > > > > > rk
> > > > > > (0
> > > > > > /0),ct_label(0/0),recirc_id(0x14d),dp_hash(0/0),in_port(rep0
> > > > > > _0
> > > > > > ),
> > > > > > pa
> > > > > > ck
> > > > > > et_type(ns=0/0,id=0/0),eth(src=00:00:00:00:00:00/00:00:00:00:00:
> > > > > > 00
> > > > > > ,d
> > > > > > st=00:00:00:00:00:00/00:00:00:00:00:00),eth_type(0x0800),ipv
> > > > > > 4(
> > > > > > sr
> > > > > > c=
> > > > > > 19
> > > > > > 2.168.200.165,dst=
> > > > > > 0.0.0.0/0.0.0.0,proto=0/0,tos=0/0,ttl=0/0,frag=no),
> > > > > > packets:1, bytes:60, used:6.530s, dp:tc,
> > > > > > actions:ct(commit,zone=22,nat(src=1.1.1.166)),recirc(0x14e)
> > > > > > ufid:b3505ba7-9367-4533-a5df-f1f897376c54,
> > > > > > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),c
> > > > > > t_
> > > > > > ma
> > > > > > rk
> > > > > > (0
> > > > > > /0),ct_label(0/0),recirc_id(0x14d),dp_hash(0/0),in_port(rep0
> > > > > > _0
> > > > > > ),
> > > > > > pa
> > > > > > ck
> > > > > > et_type(ns=0/0,id=0/0),eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:
> > > > > > 43),eth_type(0x0800),ipv4(src=
> > > > > > 0.0.0.0/128.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,to
> > > > > > s= 0/ 0, tt l=0/0,frag=no), packets:5518539,
> > > > > > bytes:7924612529, used:0.040s, offloaded:yes, dp:tc,
> > > > > > actions:ct_clear,enp1s0np1
> > > > > > ufid:ac4c188c-5320-4376-a53e-b1561e5ca209,
> > > > > > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),c
> > > > > > t_
> > > > > > ma
> > > > > > rk
> > > > > > (0
> > > > > > /0),ct_label(0/0),recirc_id(0x14e),dp_hash(0/0),in_port(rep0
> > > > > > _0
> > > > > > ),
> > > > > > pa
> > > > > > ck
> > > > > > et_type(ns=0/0,id=0/0),eth(src=fa:16:3e:ae:b5:e5,dst=8c:1f:64:30:61:
> > > > > > 43),eth_type(0x0800),ipv4(src=
> > > > > > 0.0.0.0/0.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,tos=
> > > > > > 0/ 0, tt l= 0/0,frag=no), packets:1, bytes:60, used:6.531s,
> > > > > > offloaded:yes, dp:tc,
> > > > > > actions:ct_clear,enp1s0np1
> > > > > >
> > > > > > Signed-off-by: Wentao Jia <[email protected]>
> > > > > > ---
> > > > > > northd/northd.c | 10 +++-------
> > > > > > 1 file changed, 3 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/northd/northd.c b/northd/northd.c index
> > > > > > d31cb1688..1e7406a72 100644
> > > > > > --- a/northd/northd.c
> > > > > > +++ b/northd/northd.c
> > > > > > @@ -12867,11 +12867,8 @@ build_lrouter_in_unsnat_flow(struct
> > > > > > hmap *lflows, struct ovn_datapath *od,
> > > > > > * Undoing SNAT has to happen before DNAT processing. This is
> > > > > > * because when the packet was DNATed in ingress pipeline, it
> > > > > > did
> > > > > > * not know about the possibility of eventual additional SNAT in
> > > > > > - * egress pipeline. */
> > > > > > - if (strcmp(nat->type, "snat") && strcmp(nat->type,
> > > > > > "dnat_and_snat")) {
> > > > > > - return;
> > > > > > - }
> > > > > > -
> > > > > > + * egress pipeline.
> > > > > > + */
> > > > > > bool stateless = lrouter_nat_is_stateless(nat);
> > > > > > if (od->is_gw_router) {
> > > > > > ds_clear(match);
> > > > > > @@ -13036,8 +13033,7 @@ build_lrouter_out_undnat_flow(struct
> > > > > > hmap *lflows, struct ovn_datapath *od,
> > > > > > *
> > > > > > * Note that this only applies for NAT on a distributed router.
> > > > > > */
> > > > > > - if (!od->n_l3dgw_ports ||
> > > > > > - (strcmp(nat->type, "dnat") && strcmp(nat->type,
> > > > > > "dnat_and_snat"))) {
> > > > > > + if (!od->n_l3dgw_ports) {
> > > > > > return;
> > > > > > }
> > > > > >
> > > > > > --
> > > > > > 2.31.1
> > > > > >
> > > > > > _______________________________________________
> > > > > > dev mailing list
> > > > > > [email protected]
> > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > > > >
> > > > > >
> > > > > Hi,
> > > > >
> > > > > thank you for the patch. I am afraid that in current form the
> > > > > patch is not right. The changes presented here are adding
> > > > > additional undnat for snat and vice versa. I am not entirely
> > > > > sure how this helps with established connections. Could you
> > > > > please elaborate more on what are you trying to achieve?
> > > >
> > > >
> > > > Hi Wentao Jia,
> > > >
> > > > I see your reply to Ales in the patchworks here -
> > > > https://patchwork.ozlabs.org/project/ovn/patch/BN6PR13MB1202629B
> > > > 22
> > > > D0
> > > > 67 [email protected]/
> > > >
> > > > But it's weird that I don't see it in my mail.
> > > >
> > > > I am really confused with this patch.
> > > >
> > > > Can you please add a test case for this ? And also share the scenario
> > > > with ovn-nbctl commands so that we can test it out locally ?
> > > >
> > > > Thanks
> > > > Numan
> > > >
> > > > >
> > > > > Thanks,
> > > > > Ales
> > > > >
> > > > > --
> > > > >
> > > > > Ales Musil
> > > > >
> > > > > Senior Software Engineer - OVN Core
> > > > >
> > > > > Red Hat EMEA <https://www.redhat.com>
> > > > >
> > > > > [email protected] IM: amusil
> > > > > <https://red.ht/sig>
> > > > > _______________________________________________
> > > > > dev mailing list
> > > > > [email protected]
> > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > > >
> > > > _______________________________________________
> > > > dev mailing list
> > > > [email protected]
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > >
> > > _______________________________________________
> > > dev mailing list
> > > [email protected]
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
