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/bn6pr13mb120270f798be46cfa0d421a1ed...@bn6pr13mb1202.namprd13.prod.outlook.com/
[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),ct_ma
> > > > > rk
> > > > > (0
> > > > > /0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),pack
> > > > > 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(0x14b)
> > > > > ufid:e9c5df95-02df-4629-b399-ddeb5581e997,
> > > > > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_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),ct_ma
> > > > > rk
> > > > > (0
> > > > > /0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(rep0_0),pack
> > > > > 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,proto=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),ct_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),ipv4(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),ct_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,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_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/BN6PR13MB1202629B22D0
> > > 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
