Message ID | 20231010123614.591-1-liuxie_11@163.com |
---|---|
State | Accepted |
Delegated to: | Dumitru Ceara |
Headers | show |
Series | [ovs-dev,v2] northd: Avoid snat on reply packets for dgw | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | warning | apply and check: warning |
ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 10/10/23 14:35, liuxie_11@163.com wrote: > From: shylou <liuxie_11@163.com> > > OVN had fix the issue[1] that avoid snat on reply packets for > gateway router. It is also needed to be dealt with for dgw. > > [1]https://github.com/ovn-org/ovn/commit/8b3e1afc30 > > Signed-off-by: Xie Liu <liushyshy@gmail.com> > Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks for this new revision, Xie Liu! I didn't get a chance to review it thoroughly yet and I haven't acked the previous version either. Next time please wait for an explicit "Acked-by: .." reply before adding it to the commit message of the new version. I'll try to review this patch as soon as possible, for now let's wait for ovsrobot to run CI on it: https://github.com/ovsrobot/ovn/actions/runs/6469624517 https://github.com/ovsrobot/ovn/actions/runs/6469624513 Thanks, Dumitru > --- > northd/northd.c | 8 ++----- > tests/ovn-northd.at | 56 ++++++++++++++++++++++----------------------- > tests/ovn.at | 4 ++-- > 3 files changed, 32 insertions(+), 36 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 3eaa43f07..3da91fde8 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -14649,9 +14649,8 @@ build_lrouter_out_snat_in_czone_flow(struct hmap *lflows, > ds_put_format(&zone_actions, "eth.src = "ETH_ADDR_FMT"; ", > ETH_ADDR_ARGS(mac)); > } > - > + ds_put_format(match, " && (!ct.trk || !ct.rpl)"); > ds_put_cstr(&zone_actions, REGBIT_DST_NAT_IP_LOCAL" = 0; "); > - > ds_put_format(actions, "ct_snat_in_czone(%s", nat->external_ip); > ds_put_format(&zone_actions, "ct_snat(%s", nat->external_ip); > > @@ -14707,11 +14706,8 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, > ds_put_format(actions, "eth.src = "ETH_ADDR_FMT"; ", > ETH_ADDR_ARGS(mac)); > } > - } else { > - /* Gateway router. */ > - ds_put_cstr(match, " && (!ct.trk || !ct.rpl)"); > } > - > + ds_put_cstr(match, " && (!ct.trk || !ct.rpl)"); > 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); > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 23dbe111f..ce3de2048 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -1100,7 +1100,7 @@ AT_CAPTURE_FILE([crflows]) > AT_CHECK([grep -e "lr_out_snat" drflows | sed 's/table=../table=??/' | sort], [0], [dnl > table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) > table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range), action=(ct_snat(172.16.1.1);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);) > ]) > > AT_CHECK([grep -e "lr_out_snat" crflows | sed 's/table=../table=??/' | sort], [0], [dnl > @@ -1130,7 +1130,7 @@ AT_CAPTURE_FILE([crflows2]) > AT_CHECK([grep -e "lr_out_snat" drflows2 | sed 's/table=../table=??/' | sort], [0], [dnl > table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) > table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ct_snat(172.16.1.1);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);) > table=??(lr_out_snat ), priority=163 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $disallowed_range), action=(next;) > ]) > > @@ -1159,7 +1159,7 @@ AT_CAPTURE_FILE([crflows2]) > AT_CHECK([grep -e "lr_out_snat" drflows3 | sed 's/table=../table=??/' | sort], [0], [dnl > table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) > table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range), action=(ct_snat(172.16.1.2);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);) > ]) > > AT_CHECK([grep -e "lr_out_snat" crflows3 | sed 's/table=../table=??/' | sort], [0], [dnl > @@ -1186,7 +1186,7 @@ AT_CAPTURE_FILE([crflows2]) > AT_CHECK([grep -e "lr_out_snat" drflows4 | sed 's/table=../table=??/' | sort], [0], [dnl > table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) > table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ct_snat(172.16.1.2);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);) > table=??(lr_out_snat ), priority=163 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $disallowed_range), action=(next;) > ]) > > @@ -5352,12 +5352,12 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], > AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl > table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) > table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.10);) > - table=? (lr_out_snat ), priority=154 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.10);) > - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.30);) > - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.20);) > - table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.30);) > - table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.20);) > + table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.10);) > + table=? (lr_out_snat ), priority=154 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.10);) > + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.30);) > + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.20);) > + table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.30);) > + table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.20);) > ]) > > # Separate zones for DGP > @@ -5400,9 +5400,9 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], > AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl > table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) > table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.10);) > - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.30);) > - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.20);) > + table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) > + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) > + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) > ]) > > # Associate load balancer to lr0 > @@ -5482,12 +5482,12 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], > AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl > table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) > table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.10);) > - table=? (lr_out_snat ), priority=154 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.10);) > - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.30);) > - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.20);) > - table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.30);) > - table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.20);) > + table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.10);) > + table=? (lr_out_snat ), priority=154 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.10);) > + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.30);) > + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.20);) > + table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.30);) > + table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.20);) > ]) > > # Separate zones for DGP > @@ -5548,9 +5548,9 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], > AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl > table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) > table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.10);) > - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.30);) > - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.20);) > + table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) > + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) > + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) > ]) > > # Make the logical router as Gateway router > @@ -7340,9 +7340,9 @@ AT_CHECK([grep lr_in_unsnat lrflows | grep ct_snat | sed 's/table=../table=??/' > ]) > > AT_CHECK([grep lr_out_snat lrflows | grep ct_snat | sed 's/table=../table=??/' | sort], [0], [dnl > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ct_snat(172.16.1.10);) > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2")), action=(ct_snat(10.0.0.10);) > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3")), action=(ct_snat(192.168.0.10);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.10);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2") && (!ct.trk || !ct.rpl)), action=(ct_snat(10.0.0.10);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3") && (!ct.trk || !ct.rpl)), action=(ct_snat(192.168.0.10);) > ]) > > check ovn-nbctl --wait=sb lr-nat-del DR snat 20.0.0.10 > @@ -7416,9 +7416,9 @@ AT_CHECK([grep lr_in_unsnat lrflows | grep ct_snat | sed 's/table=../table=??/' > ]) > > AT_CHECK([grep lr_out_snat lrflows | grep ct_snat | sed 's/table=../table=??/' | sort], [0], [dnl > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ct_snat(172.16.1.10);) > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2")), action=(ct_snat(10.0.0.10);) > - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3")), action=(ct_snat(192.168.0.10);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.10);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2") && (!ct.trk || !ct.rpl)), action=(ct_snat(10.0.0.10);) > + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3") && (!ct.trk || !ct.rpl)), action=(ct_snat(192.168.0.10);) > ]) > > AT_CHECK([grep lr_in_dnat lrflows | grep ct_dnat | sed 's/table=../table=??/' | sort], [0], [dnl > diff --git a/tests/ovn.at b/tests/ovn.at > index 2576a659b..79c729bc6 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -34453,7 +34453,7 @@ dnat_zone=$(ovs-ofctl dump-flows br-int table=$DNAT_TABLE,metadata=0x${lr0_dp_ke > if test -n "$dnat_zone"; then > dnat_zone=${dnat_zone::-1} > fi > -snat_zone=$(ovs-ofctl dump-flows br-int table=$SNAT_TABLE,metadata=0x${lr0_dp_key} | grep priority=153 | grep -o zone=.*, | cut -d '=' -f 2) > +snat_zone=$(ovs-ofctl dump-flows br-int table=$SNAT_TABLE,metadata=0x${lr0_dp_key} | grep priority=153 | grep ct_state=-trk | grep -o zone=.*, | cut -d '=' -f 2) > if test -n "$snat_zone"; then > snat_zone=${snat_zone::-1} > fi > @@ -34470,7 +34470,7 @@ dnat_zone=$(ovs-ofctl dump-flows br-int table=$DNAT_TABLE,metadata=0x${lr0_dp_ke > if test -n "$dnat_zone"; then > dnat_zone=${dnat_zone::-1} > fi > -snat_zone=$(ovs-ofctl dump-flows br-int table=$SNAT_TABLE,metadata=0x${lr0_dp_key} | grep priority=153 | grep -o zone=.*, | cut -d '=' -f 2) > +snat_zone=$(ovs-ofctl dump-flows br-int table=$SNAT_TABLE,metadata=0x${lr0_dp_key} | grep priority=153 | grep ct_state=-trk | grep -o zone=.*, | cut -d '=' -f 2) > if test -n "$snat_zone"; then > snat_zone=${snat_zone::-1} > fi
On 10/10/23 14:35, liuxie_11@163.com wrote: > From: shylou <liuxie_11@163.com> > > OVN had fix the issue[1] that avoid snat on reply packets for > gateway router. It is also needed to be dealt with for dgw. > > [1]https://github.com/ovn-org/ovn/commit/8b3e1afc30 > > Signed-off-by: Xie Liu <liushyshy@gmail.com> I applied this patch to main and backported it to 23.09 and 23.06. I didn't backport it further because there quite quite a few conflicts. Please let me know if you need it on older branches. Thanks, Dumitru
diff --git a/northd/northd.c b/northd/northd.c index 3eaa43f07..3da91fde8 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -14649,9 +14649,8 @@ build_lrouter_out_snat_in_czone_flow(struct hmap *lflows, ds_put_format(&zone_actions, "eth.src = "ETH_ADDR_FMT"; ", ETH_ADDR_ARGS(mac)); } - + ds_put_format(match, " && (!ct.trk || !ct.rpl)"); ds_put_cstr(&zone_actions, REGBIT_DST_NAT_IP_LOCAL" = 0; "); - ds_put_format(actions, "ct_snat_in_czone(%s", nat->external_ip); ds_put_format(&zone_actions, "ct_snat(%s", nat->external_ip); @@ -14707,11 +14706,8 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, ds_put_format(actions, "eth.src = "ETH_ADDR_FMT"; ", ETH_ADDR_ARGS(mac)); } - } else { - /* Gateway router. */ - ds_put_cstr(match, " && (!ct.trk || !ct.rpl)"); } - + ds_put_cstr(match, " && (!ct.trk || !ct.rpl)"); 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); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 23dbe111f..ce3de2048 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1100,7 +1100,7 @@ AT_CAPTURE_FILE([crflows]) AT_CHECK([grep -e "lr_out_snat" drflows | sed 's/table=../table=??/' | sort], [0], [dnl table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range), action=(ct_snat(172.16.1.1);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);) ]) AT_CHECK([grep -e "lr_out_snat" crflows | sed 's/table=../table=??/' | sort], [0], [dnl @@ -1130,7 +1130,7 @@ AT_CAPTURE_FILE([crflows2]) AT_CHECK([grep -e "lr_out_snat" drflows2 | sed 's/table=../table=??/' | sort], [0], [dnl table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ct_snat(172.16.1.1);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);) table=??(lr_out_snat ), priority=163 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $disallowed_range), action=(next;) ]) @@ -1159,7 +1159,7 @@ AT_CAPTURE_FILE([crflows2]) AT_CHECK([grep -e "lr_out_snat" drflows3 | sed 's/table=../table=??/' | sort], [0], [dnl table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range), action=(ct_snat(172.16.1.2);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);) ]) AT_CHECK([grep -e "lr_out_snat" crflows3 | sed 's/table=../table=??/' | sort], [0], [dnl @@ -1186,7 +1186,7 @@ AT_CAPTURE_FILE([crflows2]) AT_CHECK([grep -e "lr_out_snat" drflows4 | sed 's/table=../table=??/' | sort], [0], [dnl table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ct_snat(172.16.1.2);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);) table=??(lr_out_snat ), priority=163 , match=(ip && ip4.src == 50.0.0.11 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && ip4.dst == $disallowed_range), action=(next;) ]) @@ -5352,12 +5352,12 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.10);) - table=? (lr_out_snat ), priority=154 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.10);) - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.30);) - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.20);) - table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.30);) - table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.20);) + table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.10);) + table=? (lr_out_snat ), priority=154 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.10);) + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.30);) + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.20);) + table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.30);) + table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.20);) ]) # Separate zones for DGP @@ -5400,9 +5400,9 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.10);) - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.30);) - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.20);) + table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) ]) # Associate load balancer to lr0 @@ -5482,12 +5482,12 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.10);) - table=? (lr_out_snat ), priority=154 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.10);) - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.30);) - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat_in_czone(172.168.0.20);) - table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.30);) - table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.20);) + table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.10);) + table=? (lr_out_snat ), priority=154 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.10);) + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.30);) + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat_in_czone(172.168.0.20);) + table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.30);) + table=? (lr_out_snat ), priority=162 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl) && reg9[[4]] == 1), action=(reg9[[4]] = 0; ct_snat(172.168.0.20);) ]) # Separate zones for DGP @@ -5548,9 +5548,9 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.10);) - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.30);) - table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat(172.168.0.20);) + table=? (lr_out_snat ), priority=153 , match=(ip && ip4.src == 10.0.0.0/24 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) + table=? (lr_out_snat ), priority=161 , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) ]) # Make the logical router as Gateway router @@ -7340,9 +7340,9 @@ AT_CHECK([grep lr_in_unsnat lrflows | grep ct_snat | sed 's/table=../table=??/' ]) AT_CHECK([grep lr_out_snat lrflows | grep ct_snat | sed 's/table=../table=??/' | sort], [0], [dnl - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ct_snat(172.16.1.10);) - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2")), action=(ct_snat(10.0.0.10);) - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3")), action=(ct_snat(192.168.0.10);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.10);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2") && (!ct.trk || !ct.rpl)), action=(ct_snat(10.0.0.10);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3") && (!ct.trk || !ct.rpl)), action=(ct_snat(192.168.0.10);) ]) check ovn-nbctl --wait=sb lr-nat-del DR snat 20.0.0.10 @@ -7416,9 +7416,9 @@ AT_CHECK([grep lr_in_unsnat lrflows | grep ct_snat | sed 's/table=../table=??/' ]) AT_CHECK([grep lr_out_snat lrflows | grep ct_snat | sed 's/table=../table=??/' | sort], [0], [dnl - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(ct_snat(172.16.1.10);) - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2")), action=(ct_snat(10.0.0.10);) - table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3")), action=(ct_snat(192.168.0.10);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S1" && is_chassis_resident("cr-DR-S1") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.10);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S2" && is_chassis_resident("cr-DR-S2") && (!ct.trk || !ct.rpl)), action=(ct_snat(10.0.0.10);) + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.src == 20.0.0.10 && outport == "DR-S3" && is_chassis_resident("cr-DR-S3") && (!ct.trk || !ct.rpl)), action=(ct_snat(192.168.0.10);) ]) AT_CHECK([grep lr_in_dnat lrflows | grep ct_dnat | sed 's/table=../table=??/' | sort], [0], [dnl diff --git a/tests/ovn.at b/tests/ovn.at index 2576a659b..79c729bc6 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -34453,7 +34453,7 @@ dnat_zone=$(ovs-ofctl dump-flows br-int table=$DNAT_TABLE,metadata=0x${lr0_dp_ke if test -n "$dnat_zone"; then dnat_zone=${dnat_zone::-1} fi -snat_zone=$(ovs-ofctl dump-flows br-int table=$SNAT_TABLE,metadata=0x${lr0_dp_key} | grep priority=153 | grep -o zone=.*, | cut -d '=' -f 2) +snat_zone=$(ovs-ofctl dump-flows br-int table=$SNAT_TABLE,metadata=0x${lr0_dp_key} | grep priority=153 | grep ct_state=-trk | grep -o zone=.*, | cut -d '=' -f 2) if test -n "$snat_zone"; then snat_zone=${snat_zone::-1} fi @@ -34470,7 +34470,7 @@ dnat_zone=$(ovs-ofctl dump-flows br-int table=$DNAT_TABLE,metadata=0x${lr0_dp_ke if test -n "$dnat_zone"; then dnat_zone=${dnat_zone::-1} fi -snat_zone=$(ovs-ofctl dump-flows br-int table=$SNAT_TABLE,metadata=0x${lr0_dp_key} | grep priority=153 | grep -o zone=.*, | cut -d '=' -f 2) +snat_zone=$(ovs-ofctl dump-flows br-int table=$SNAT_TABLE,metadata=0x${lr0_dp_key} | grep priority=153 | grep ct_state=-trk | grep -o zone=.*, | cut -d '=' -f 2) if test -n "$snat_zone"; then snat_zone=${snat_zone::-1} fi