Hi, Ales Thanks for your reply
Firstly, think about this issue, the type of nat rule is dnat_and_snat (dnat_and_snat rule), there are unsnat and undnat logical flows, but the type of nat rule is snat(snat rule), there is no undnat logical flow, why snat rules no undnat logical flow, is it correct? In the implementation of ovs conntrack, when the first packect arrive, commit the connection to the kernel connection tracking module if necessary, subsequent packets no need commit again. snat/dnat rules in ovn, connections commit to kernel module again and again on all packets Another reason is tc offload only support established connection, ct commit action is considered to be un established connection and cannot be offloaded B R Wentao 发件人: Ales Musil <[email protected]> 发送时间: 2022年8月9日 19:45 收件人: Wentao Jia <[email protected]> 抄送: [email protected] 主题: Re: [ovs-dev] [OVN PATCH] northd: add unsnat/undnat lflow for established connections On Tue, Aug 2, 2022 at 9:05 PM Wentao Jia <[email protected]<mailto:[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),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),packet_type(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,ttl=64,frag=no<http://192.168.200.128/255.255.255.192,dst=1.1.1.254,proto=6,tos=0/0,ttl=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)),set(ipv4(ttl=63)),ct(commit,zone=22,nat(src=1.1.1.124)),recirc(0x14b) ufid:e9c5df95-02df-4629-b399-ddeb5581e997, skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0x14b),dp_hash(0/0),in_port(rep0_0),packet_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,ttl=0/0,frag=no<http://0.0.0.0/0.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,tos=0/0,ttl=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),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),packet_type(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,proto=6,tos=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)),set(ipv4(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),ct_mark(0/0),ct_label(0/0),recirc_id(0x14d),dp_hash(0/0),in_port(rep0_0),packet_type(ns=0/0,id=0/0),eth(src=00:00:00:00:00:00/00:00:00:00:00:00,dst=00:00:00:00:00:00/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.200.165,dst=0.0.0.0/0.0.0.0,proto=0/0,tos=0/0,ttl=0/0,frag=no<http://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),ct_mark(0/0),ct_label(0/0),recirc_id(0x14d),dp_hash(0/0),in_port(rep0_0),packet_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,tos=0/0,ttl=0/0,frag=no<http://0.0.0.0/128.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,tos=0/0,ttl=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),ct_mark(0/0),ct_label(0/0),recirc_id(0x14e),dp_hash(0/0),in_port(rep0_0),packet_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,ttl=0/0,frag=no<http://0.0.0.0/0.0.0.0,dst=1.1.1.240/255.255.255.240,proto=0/0,tos=0/0,ttl=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]<mailto:[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]<mailto:[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? Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA<https://www.redhat.com> [email protected]<mailto:[email protected]> IM: amusil [https://static.redhat.com/libs/redhat/brand-assets/latest/corp/logo.png]<https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
