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

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),ct_mark(0
> > /0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),packet_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,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(ip
> > v4(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),pack
> > 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,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_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,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(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),ct_mark(0
> > /0),ct_label(0/0),recirc_id(0x14d),dp_hash(0/0),in_port(rep0_0),pack
> > 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),ipv4(src=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),ct_mark(0
> > /0),ct_label(0/0),recirc_id(0x14d),dp_hash(0/0),in_port(rep0_0),pack
> > 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,tos=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),ct_mark(0
> > /0),ct_label(0/0),recirc_id(0x14e),dp_hash(0/0),in_port(rep0_0),pack
> > 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,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]>
> > ---
> > 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/bn6pr13mb1202629b22d0676432213f29ed...@bn6pr13mb1202.namprd13.prod.outlook.com/

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

Reply via email to