diff mbox series

[ovs-dev] 回复: [OVN PATCH] northd: add unsnat/undnat lflow for established connections

Message ID BN6PR13MB1202629B22D0676432213F29ED659@BN6PR13MB1202.namprd13.prod.outlook.com
State Not Applicable
Headers show
Series [ovs-dev] 回复: [OVN PATCH] northd: add unsnat/undnat lflow for established connections | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning

Commit Message

Wentao Jia Aug. 10, 2022, 8:26 a.m. UTC 
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 <amusil@redhat.com>
发送时间: 2022年8月9日 19:45
收件人: Wentao Jia <wentao.jia@corigine.com>
抄送: dev@openvswitch.org
主题: Re: [ovs-dev] [OVN PATCH] northd: add unsnat/undnat lflow for established connections


On Tue, Aug 2, 2022 at 9:05 PM Wentao Jia <wentao.jia@corigine.com<mailto:wentao.jia@corigine.com>> 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 <wentao.jia@corigine.com<mailto:wentao.jia@corigine.com>>
---
northd/northd.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

--
2.31.1
diff mbox series

Patch

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;
     }