On Mon, Apr 22, 2024 at 9:45 AM Martin Kalcok <[email protected]> wrote:
> wrt the failed ovn-kubernetes tests, they seemed to have passed > successfully in my branch [0]. Is it possible that the tests are unstable? > > [0] https://github.com/mkalcok/ovn/actions/runs/8752915096 > Hi Martin, yeah unfortunately the test was broken and AFAIK it should be fixed now [0]. > > On Fri, Apr 19, 2024 at 2:33 PM Martin Kalcok <[email protected]> > wrote: > >> This change fixes bug that breaks ability of machines from external >> networks, to communicate with machines in SNATed networks (specifically >> when using a Distributed router). >> >> Currently when a machine (S1) on an external network tries to talk >> over TCP with a machine (A1) in a network that has enabled SNAT, the >> connection is established successfully. However after the three-way >> handshake, any packets that come from the A1 machine will have their >> source address translated by the Distributed router, breaking the >> communication. >> >> Existing rule in `build_lrouter_out_snat_flow` that decides which >> packets should be SNATed already tries to avoid SNATing packets in >> reply direction with `(!ct.trk || !ct.rpl)`. However, previous stages >> in the distributed LR egress pipeline do not initiate the CT state. >> >> Additionally we need to commit new connections that originate from >> external networks into CT, so that the packets in the reply direction >> (back to the external network) can be properly identified. >> >> Rationale: >> >> In my original RFC [0], there were questions about the motivation for >> fixing this issue. I'll try to summarize why I think this is a bug >> that should be fixed. >> >> 1. Current implementation for Distributed router already tries to >> avoid SNATing packets in the reply direction, it's just missing >> initialized CT states to make proper decisions. >> >> 2. This same scenario works with Gateway Router. I tested with >> following setup: >> >> foo -- R1 -- join - R3 -- alice >> | >> bar ----------R2 >> >> R1 is a Distributed router with SNAT for foo. R2 is a Gateway >> router with SNAT for bar. R3 is a Gateway router with no SNAT. >> Using 'alice1' as a client I was able to talk over TCP with >> 'bar1' but connection with 'foo1' failed. >> >> 3. Regarding security and "leaking" of internal IPs. Reading through >> RFC 4787 [1], 5382 [2] and their update in 7857 [3], the >> specifications do not seem to mandate that SNAT implementations >> must filter incoming traffic destined directly to the internal >> network. Sections like "5. Filtering Behavior" in 4787 and >> "4.3 Externally Initiated Connections" in 5382 describe only >> behavior for traffic destined to external IP/Port associated >> with NAT on the device that performs NAT. >> >> Besides, with the current implementation, it's already possible >> to scan the internal network with pings and TCP syn scanning. >> >> 4. We do have customers/clouds that depend on this functionality. >> This is a scenario that used to work in Openstack with ML2/OVS >> and migrating those clouds to ML2/OVN would break it. >> >> [0] >> https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411670.html >> [1]https://datatracker.ietf.org/doc/html/rfc4787 >> [2]https://datatracker.ietf.org/doc/html/rfc5382 >> [3]https://datatracker.ietf.org/doc/html/rfc7857 >> >> Signed-off-by: Martin Kalcok <[email protected]> >> --- >> northd/northd.c | 66 ++++++++++++++++++++++++++++------- >> northd/ovn-northd.8.xml | 28 +++++++++++++++ >> tests/ovn-northd.at | 76 +++++++++++++++++++++++++++++++++++++---- >> tests/system-ovn.at | 68 ++++++++++++++++++++++++++++++++++++ >> 4 files changed, 219 insertions(+), 19 deletions(-) >> >> diff --git a/northd/northd.c b/northd/northd.c >> index 37f443e70..2c2eee445 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -14425,20 +14425,27 @@ build_lrouter_out_is_dnat_local(struct >> lflow_table *lflows, >> >> static void >> build_lrouter_out_snat_match(struct lflow_table *lflows, >> - const struct ovn_datapath *od, >> - const struct nbrec_nat *nat, struct ds >> *match, >> - bool distributed_nat, int cidr_bits, bool >> is_v6, >> - struct ovn_port *l3dgw_port, >> - struct lflow_ref *lflow_ref) >> + const struct ovn_datapath *od, >> + const struct nbrec_nat *nat, >> + struct ds *match, >> + bool distributed_nat, int >> cidr_bits, >> + bool is_v6, >> + struct ovn_port *l3dgw_port, >> + struct lflow_ref *lflow_ref, >> + bool is_reverse) >> { >> ds_clear(match); >> >> - ds_put_format(match, "ip && ip%c.src == %s", is_v6 ? '6' : '4', >> + ds_put_format(match, "ip && ip%c.%s == %s", >> + is_v6 ? '6' : '4', >> + is_reverse ? "dst" : "src", >> nat->logical_ip); >> >> if (!od->is_gw_router) { >> /* Distributed router. */ >> - ds_put_format(match, " && outport == %s", l3dgw_port->json_key); >> + ds_put_format(match, " && %s == %s", >> + is_reverse ? "inport" : "outport", >> + l3dgw_port->json_key); >> if (od->n_l3dgw_ports) { >> ds_put_format(match, " && is_chassis_resident(\"%s\")", >> distributed_nat >> @@ -14449,7 +14456,7 @@ build_lrouter_out_snat_match(struct lflow_table >> *lflows, >> >> if (nat->allowed_ext_ips || nat->exempted_ext_ips) { >> lrouter_nat_add_ext_ip_match(od, lflows, match, nat, >> - is_v6, false, cidr_bits, >> + is_v6, is_reverse, cidr_bits, >> lflow_ref); >> } >> } >> @@ -14476,7 +14483,8 @@ build_lrouter_out_snat_stateless_flow(struct >> lflow_table *lflows, >> uint16_t priority = cidr_bits + 1; >> >> build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat, >> - cidr_bits, is_v6, l3dgw_port, >> lflow_ref); >> + cidr_bits, is_v6, l3dgw_port, lflow_ref, >> + false); >> >> if (!od->is_gw_router) { >> /* Distributed router. */ >> @@ -14523,7 +14531,7 @@ build_lrouter_out_snat_in_czone_flow(struct >> lflow_table *lflows, >> >> build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat, >> cidr_bits, is_v6, l3dgw_port, >> - lflow_ref); >> + lflow_ref, false); >> >> if (od->n_l3dgw_ports) { >> priority += 128; >> @@ -14572,7 +14580,8 @@ build_lrouter_out_snat_flow(struct lflow_table >> *lflows, >> struct ds *actions, bool distributed_nat, >> struct eth_addr mac, int cidr_bits, bool >> is_v6, >> struct ovn_port *l3dgw_port, >> - struct lflow_ref *lflow_ref) >> + struct lflow_ref *lflow_ref, >> + const struct chassis_features *features) >> { >> if (strcmp(nat->type, "snat") && strcmp(nat->type, "dnat_and_snat")) >> { >> return; >> @@ -14586,7 +14595,9 @@ build_lrouter_out_snat_flow(struct lflow_table >> *lflows, >> uint16_t priority = cidr_bits + 1; >> >> build_lrouter_out_snat_match(lflows, od, nat, match, distributed_nat, >> - cidr_bits, is_v6, l3dgw_port, >> lflow_ref); >> + cidr_bits, is_v6, l3dgw_port, lflow_ref, >> + false); >> + size_t original_match_len = match->length; >> >> if (!od->is_gw_router) { >> /* Distributed router. */ >> @@ -14611,6 +14622,35 @@ build_lrouter_out_snat_flow(struct lflow_table >> *lflows, >> priority, ds_cstr(match), >> ds_cstr(actions), &nat->header_, >> lflow_ref); >> + >> + /* For the SNAT networks, we need to make sure that connections are >> + * properly tracked so we can decide whether to perform SNAT on >> traffic >> + * exiting the network. */ >> + if (features->ct_commit_to_zone && !strcmp(nat->type, "snat") && >> + !od->is_gw_router) { >> + /* For traffic that comes from SNAT network, initiate CT state >> before >> + * entering S_ROUTER_OUT_SNAT to allow matching on various CT >> states. >> + */ >> + ds_truncate(match, original_match_len); >> + ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_UNDNAT, 70, >> + ds_cstr(match), "ct_snat;", >> + lflow_ref); >> + >> + build_lrouter_out_snat_match(lflows, od, nat, match, >> + distributed_nat, cidr_bits, is_v6, >> + l3dgw_port, lflow_ref, true); >> + >> + /* New traffic that goes into SNAT network is committed to CT to >> avoid >> + * SNAT-ing replies.*/ >> + ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, priority, >> + ds_cstr(match), "ct_snat;", >> + lflow_ref); >> + >> + ds_put_cstr(match, " && ct.new"); >> + ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_SNAT, priority, >> + ds_cstr(match), "ct_commit_to_zone(snat);", >> + lflow_ref); >> + } >> } >> >> static void >> @@ -15149,7 +15189,7 @@ build_lrouter_nat_defrag_and_lb( >> } else { >> build_lrouter_out_snat_flow(lflows, od, nat, match, actions, >> distributed_nat, mac, cidr_bits, >> is_v6, >> - l3dgw_port, lflow_ref); >> + l3dgw_port, lflow_ref, features); >> } >> >> /* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */ >> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml >> index b14a30285..b8e542fcf 100644 >> --- a/northd/ovn-northd.8.xml >> +++ b/northd/ovn-northd.8.xml >> @@ -4877,6 +4877,13 @@ nd_ns { >> >> <p> >> <ul> >> + <li> >> + A priority-70 logical flow is added that initiates CT state for >> + traffic that is configured to be SNATed on Distributed routers. >> + This allows the next table, <code>lr_out_snat</code>, to >> + effectively match on various CT states. >> + </li> >> + >> <li> >> A priority-50 logical flow is added that commits any untracked >> flows >> from the previous table <code>lr_out_undnat</code> for Gateway >> @@ -5069,6 +5076,17 @@ nd_ns { >> >> </li> >> >> + <li> >> + An additional flow is added for traffic that goes in opposite >> + direction (i.e. it enters a network with configured SNAT). Where >> the >> + flow above matched on <code>ip4.src == <var>A</var> && >> outport >> + == <var>GW</var></code>, this flow matches on <code> ip4.dst == >> + <var>A</var> && inport == <var>GW</var></code>. A CT >> state is >> + initiated for this traffic so that the following table, <code> >> + lr_out_post_snat</code>, can identify whether the traffic flow >> was >> + initiated from the internal or external network. >> + </li> >> + >> <li> >> A priority-0 logical flow with match <code>1</code> has actions >> <code>next;</code>. >> @@ -5080,6 +5098,16 @@ nd_ns { >> <p> >> Packets reaching this table are processed according to the flows >> below: >> <ul> >> + <li> >> + Traffic that goes directly into a network configured with SNAT >> on >> + Distributed routers, and was initiated from an external >> network (i.e. >> + it matches <code>ct.new</code>), is committed to the SNAT CT >> zone. >> + This ensures that replies returning from the SNATed network do >> not >> + have their source address translated. For details about match >> rules >> + and priority see section "Egress Table 3: SNAT on Distributed >> + Routers". >> + </li> >> + >> <li> >> A priority-0 logical flow that matches all packets not already >> handled (match <code>1</code>) and action <code>next;</code>. >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index be006fb32..e1fa23682 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -1107,7 +1107,8 @@ ovn_start >> # >> # DR is connected to S1 and CR is connected to S2 >> >> -check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 >> +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ >> + -- set chassis gw1 other_config:ct-commit-to-zone="true" >> >> check ovn-nbctl lr-add DR >> check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24 >> @@ -1155,15 +1156,24 @@ AT_CAPTURE_FILE([crflows]) >> AT_CHECK([grep -e "lr_out_snat" drflows | ovn_strip_lflows], [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.dst == >> 50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && >> ip4.src == $allowed_range), action=(ct_snat;) >> 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_post_snat" drflows | ovn_strip_lflows], [0], >> [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> + table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst == >> 50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && >> ip4.src == $allowed_range && ct.new), action=(ct_commit_to_zone(snat);) >> +]) >> + >> AT_CHECK([grep -e "lr_out_snat" crflows | ovn_strip_lflows], [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=33 , match=(ip && ip4.src == >> 50.0.0.11 && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), >> action=(ct_snat(172.16.1.1);) >> ]) >> >> +AT_CHECK([grep -e "lr_out_post_snat" crflows | ovn_strip_lflows], [0], >> [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> +]) >> >> # SNAT with DISALLOWED_IPs >> check ovn-nbctl lr-nat-del DR snat 50.0.0.11 >> @@ -1185,10 +1195,16 @@ AT_CAPTURE_FILE([crflows2]) >> AT_CHECK([grep -e "lr_out_snat" drflows2 | ovn_strip_lflows], [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.dst == >> 50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1")), >> action=(ct_snat;) >> 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;) >> ]) >> >> +AT_CHECK([grep -e "lr_out_post_snat" drflows2 | ovn_strip_lflows], [0], >> [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> + table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst == >> 50.0.0.11 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && >> ct.new), action=(ct_commit_to_zone(snat);) >> +]) >> + >> AT_CHECK([grep -e "lr_out_snat" crflows2 | ovn_strip_lflows], [0], [dnl >> table=??(lr_out_snat ), priority=0 , match=(1), >> action=(next;) >> table=??(lr_out_snat ), priority=120 , match=(nd_ns), >> action=(next;) >> @@ -1196,6 +1212,10 @@ AT_CHECK([grep -e "lr_out_snat" crflows2 | >> ovn_strip_lflows], [0], [dnl >> table=??(lr_out_snat ), priority=35 , match=(ip && ip4.src == >> 50.0.0.11 && ip4.dst == $disallowed_range), action=(next;) >> ]) >> >> +AT_CHECK([grep -e "lr_out_post_snat" crflows2 | ovn_strip_lflows], [0], >> [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> +]) >> + >> # Stateful FIP with ALLOWED_IPs >> check ovn-nbctl lr-nat-del DR snat 50.0.0.11 >> check ovn-nbctl lr-nat-del CR snat 50.0.0.11 >> @@ -1217,12 +1237,20 @@ AT_CHECK([grep -e "lr_out_snat" drflows3 | >> ovn_strip_lflows], [0], [dnl >> 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_post_snat" drflows3 | ovn_strip_lflows], [0], >> [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> +]) >> + >> AT_CHECK([grep -e "lr_out_snat" crflows3 | ovn_strip_lflows], [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=33 , match=(ip && ip4.src == >> 50.0.0.11 && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), >> action=(ct_snat(172.16.1.2);) >> ]) >> >> +AT_CHECK([grep -e "lr_out_post_snat" crflows3 | ovn_strip_lflows], [0], >> [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> +]) >> + >> # Stateful FIP with DISALLOWED_IPs >> ovn-nbctl lr-nat-del DR dnat_and_snat 172.16.1.2 >> ovn-nbctl lr-nat-del CR dnat_and_snat 172.16.1.2 >> @@ -1279,7 +1307,7 @@ AT_CHECK([grep -e "lr_out_snat" crflows5 | >> ovn_strip_lflows], [0], [dnl >> table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == >> 50.0.0.11 && ip4.dst == $allowed_range), action=(ip4.src=172.16.1.2; next;) >> ]) >> >> -# Stateful FIP with DISALLOWED_IPs >> +# Stateless FIP with DISALLOWED_IPs >> ovn-nbctl lr-nat-del DR dnat_and_snat 172.16.1.2 >> ovn-nbctl lr-nat-del CR dnat_and_snat 172.16.1.2 >> >> @@ -5521,7 +5549,8 @@ AT_CHECK([grep "lr_out_snat" lr0flows | >> ovn_strip_lflows], [0], [dnl >> >> check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ >> -- set chassis gw1 other_config:ct-no-masked-label="true" \ >> - -- set chassis gw1 other_config:ovn-ct-lb-related="true" >> + -- set chassis gw1 other_config:ovn-ct-lb-related="true" \ >> + -- set chassis gw1 other_config:ct-commit-to-zone="true" >> >> # Create a distributed gw port on lr0 >> check ovn-nbctl ls-add public >> @@ -5622,16 +5651,26 @@ AT_CHECK([grep "lr_out_undnat" lr0flows | >> ovn_strip_lflows], [0], [dnl >> >> AT_CHECK([grep "lr_out_post_undnat" lr0flows | ovn_strip_lflows], [0], >> [dnl >> table=??(lr_out_post_undnat ), priority=0 , match=(1), >> action=(next;) >> + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == >> 10.0.0.0/24 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_snat;) >> + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == >> 10.0.0.10 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_snat;) >> ]) >> >> AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [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.dst == >> 10.0.0.0/24 && inport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_snat;) >> 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.dst == >> 10.0.0.10 && inport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_snat;) >> 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);) >> ]) >> >> +AT_CHECK([grep "lr_out_post_snat" lr0flows | ovn_strip_lflows], [0], [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> + table=??(lr_out_post_snat ), priority=153 , match=(ip && ip4.dst == >> 10.0.0.0/24 && inport == "lr0-public" && >> is_chassis_resident("cr-lr0-public") && ct.new), >> action=(ct_commit_to_zone(snat);) >> + table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst == >> 10.0.0.10 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") >> && ct.new), action=(ct_commit_to_zone(snat);) >> +]) >> + >> # Associate load balancer to lr0 >> >> check ovn-nbctl lb-add lb0 172.168.0.100:8082 "10.0.0.50:82,10.0.0.60:82 >> " >> @@ -5770,16 +5809,26 @@ AT_CHECK([grep "lr_out_undnat" lr0flows | >> ovn_strip_lflows], [0], [dnl >> >> AT_CHECK([grep "lr_out_post_undnat" lr0flows | ovn_strip_lflows], [0], >> [dnl >> table=??(lr_out_post_undnat ), priority=0 , match=(1), >> action=(next;) >> + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == >> 10.0.0.0/24 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_snat;) >> + table=??(lr_out_post_undnat ), priority=70 , match=(ip && ip4.src == >> 10.0.0.10 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_snat;) >> ]) >> >> AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [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.dst == >> 10.0.0.0/24 && inport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_snat;) >> 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.dst == >> 10.0.0.10 && inport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_snat;) >> 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);) >> ]) >> >> +AT_CHECK([grep "lr_out_post_snat" lr0flows | ovn_strip_lflows], [0], [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> + table=??(lr_out_post_snat ), priority=153 , match=(ip && ip4.dst == >> 10.0.0.0/24 && inport == "lr0-public" && >> is_chassis_resident("cr-lr0-public") && ct.new), >> action=(ct_commit_to_zone(snat);) >> + table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst == >> 10.0.0.10 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public") >> && ct.new), action=(ct_commit_to_zone(snat);) >> +]) >> + >> # Make the logical router as Gateway router >> check ovn-nbctl clear logical_router_port lr0-public gateway_chassis >> check ovn-nbctl set logical_router lr0 options:chassis=gw1 >> @@ -7606,9 +7655,14 @@ ovn_start >> # Validate SNAT, DNAT and DNAT_AND_SNAT behavior with multiple >> # distributed gateway LRPs. >> >> -check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 >> -check ovn-sbctl chassis-add gw2 geneve 128.0.0.1 >> -check ovn-sbctl chassis-add gw3 geneve 129.0.0.1 >> +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ >> + -- set chassis gw1 other_config:ct-commit-to-zone="true" >> + >> +check ovn-sbctl chassis-add gw2 geneve 128.0.0.1 \ >> + -- set chassis gw2 other_config:ct-commit-to-zone="true" >> + >> +check ovn-sbctl chassis-add gw3 geneve 129.0.0.1 \ >> + -- set chassis gw3 other_config:ct-commit-to-zone="true" >> >> check ovn-nbctl lr-add DR >> check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24 >> @@ -7673,11 +7727,21 @@ AT_CHECK([grep lr_in_unsnat lrflows | grep >> ct_snat | ovn_strip_lflows], [0], [dn >> ]) >> >> AT_CHECK([grep lr_out_snat lrflows | grep ct_snat | ovn_strip_lflows], >> [0], [dnl >> + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst == >> 20.0.0.10 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1")), >> action=(ct_snat;) >> + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst == >> 20.0.0.10 && inport == "DR-S2" && is_chassis_resident("cr-DR-S2")), >> action=(ct_snat;) >> + table=??(lr_out_snat ), priority=161 , match=(ip && ip4.dst == >> 20.0.0.10 && inport == "DR-S3" && is_chassis_resident("cr-DR-S3")), >> action=(ct_snat;) >> 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_out_post_snat lrflows | ovn_strip_lflows], [0], [dnl >> + table=??(lr_out_post_snat ), priority=0 , match=(1), >> action=(next;) >> + table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst == >> 20.0.0.10 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1") && >> ct.new), action=(ct_commit_to_zone(snat);) >> + table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst == >> 20.0.0.10 && inport == "DR-S2" && is_chassis_resident("cr-DR-S2") && >> ct.new), action=(ct_commit_to_zone(snat);) >> + table=??(lr_out_post_snat ), priority=161 , match=(ip && ip4.dst == >> 20.0.0.10 && inport == "DR-S3" && is_chassis_resident("cr-DR-S3") && >> ct.new), action=(ct_commit_to_zone(snat);) >> +]) >> + >> check ovn-nbctl --wait=sb lr-nat-del DR snat 20.0.0.10 >> AT_CHECK([ovn-sbctl dump-flows DR | grep -e lr_in_unsnat -e lr_out_snat >> | grep ct_snat | wc -l], [0], [0 >> ]) >> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >> index 516fb4d99..0cd121981 100644 >> --- a/tests/system-ovn.at >> +++ b/tests/system-ovn.at >> @@ -3574,6 +3574,39 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 >> 10.0.0.1 | FORMAT_PING], \ >> 3 packets transmitted, 3 received, 0% packet loss, time 0ms >> ]) >> >> +# test_connectivity_from_ext takes parameters 'vm' and 'ip'. It tests >> +# icmp, udp and tcp connectivity from external network to the 'vm' on >> +# the specified 'ip'. >> +test_connectivity_from_ext() { >> + local vm=$1; shift >> + local ip=$1; shift >> + >> + # Start listening daemons for UDP and TCP connections >> + NETNS_DAEMONIZE($vm, [nc -l -u 1234], [nc-$vm-$ip-udp.pid]) >> + NETNS_DAEMONIZE($vm, [nc -l -k 1235], [nc-$vm-$ip-tcp.pid]) >> + >> + # Ensure that vm can be pinged on the specified IP >> + NS_CHECK_EXEC([alice1], [ping -q -c 3 -i 0.3 -w 2 $ip | >> FORMAT_PING], \ >> + [0], [dnl >> +3 packets transmitted, 3 received, 0% packet loss, time 0ms >> +]) >> + >> + # Perform two consecutive UDP connections to the specified IP >> + NS_CHECK_EXEC([alice1], [nc -u $ip 1234 -p 2000 -z]) >> + NS_CHECK_EXEC([alice1], [nc -u $ip 1234 -p 2000 -z]) >> + >> + # Send data over TCP connection to the specified IP >> + NS_CHECK_EXEC([alice1], [echo "TCP test" | nc --send-only $ip 1235]) >> +} >> + >> +# Test access from external network to the internal IP of a VM that >> +# has also configured DNAT >> +test_connectivity_from_ext foo1 192.168.1.2 >> + >> +# Test access from external network to the internal IP of a VM that >> +# does not have DNAT >> +test_connectivity_from_ext bar1 192.168.2.2 >> + >> OVS_WAIT_UNTIL([ >> total_pkts=$(cat ext-net.tcpdump | wc -l) >> test "${total_pkts}" = "3" >> @@ -3740,6 +3773,39 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], >> [dnl >> >> >> icmpv6,orig=(src=fd12::2,dst=fd20::2,id=<cleared>,type=128,code=0),reply=(src=fd20::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> >> ]) >> >> +# test_connectivity_from_ext takes parameters 'vm' and 'ip'. It tests >> +# icmp, udp and tcp connectivity from external network to the 'vm' on >> +# the specified 'ip'. >> +test_connectivity_from_ext() { >> + local vm=$1; shift >> + local ip=$1; shift >> + >> + # Start listening daemons for UDP and TCP connections >> + NETNS_DAEMONIZE($vm, [nc -l -u 1234], [nc-$vm-$ip-udp.pid]) >> + NETNS_DAEMONIZE($vm, [nc -l -k 1235], [nc-$vm-$ip-tcp.pid]) >> + >> + # Ensure that vm can be pinged on the specified IP >> + NS_CHECK_EXEC([alice1], [ping -q -c 3 -i 0.3 -w 2 $ip | >> FORMAT_PING], \ >> + [0], [dnl >> +3 packets transmitted, 3 received, 0% packet loss, time 0ms >> +]) >> + >> + # Perform two consecutive UDP connections to the specified IP >> + NS_CHECK_EXEC([alice1], [nc -u $ip 1234 -p 2000 -z]) >> + NS_CHECK_EXEC([alice1], [nc -u $ip 1234 -p 2000 -z]) >> + >> + # Send data over TCP connection to the specified IP >> + NS_CHECK_EXEC([alice1], [echo "TCP test" | nc --send-only $ip 1235]) >> +} >> + >> +# Test access from external network to the internal IP of a VM that >> +# has also configured DNAT >> +test_connectivity_from_ext foo1 fd11::2 >> + >> +# Test access from external network to the internal IP of a VM that >> +# does not have DNAT >> +test_connectivity_from_ext bar1 fd12::2 >> + >> OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> >> as ovn-sb >> @@ -3920,6 +3986,7 @@ NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2 >> 172.16.1.4 | FORMAT_PING], \ >> AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep icmp | >> FORMAT_CT(172.16.1.1) | \ >> sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl >> >> >> icmp,orig=(src=172.16.1.1,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> >> >> +icmp,orig=(src=172.16.1.1,dst=192.168.2.2,id=<cleared>,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> >> >> >> icmp,orig=(src=192.168.1.3,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=172.16.1.4,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> >> ]) >> >> @@ -4088,6 +4155,7 @@ NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2 >> fd20::4 | FORMAT_PING], \ >> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd20::1) | \ >> sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl >> >> >> icmpv6,orig=(src=fd11::3,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd20::4,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> >> >> +icmpv6,orig=(src=fd20::1,dst=fd12::2,id=<cleared>,type=128,code=0),reply=(src=fd12::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> >> >> >> icmpv6,orig=(src=fd20::1,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd12::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> >> ]) >> >> -- >> 2.40.1 >> >> > > -- > Best Regards, > Martin Kalcok. > The change looks good to me, thanks! Acked-by: Ales Musil <[email protected]> [0] https://github.com/ovn-org/ovn-kubernetes/commit/236e63c665bb60dd48dd5cc7f8ef2324a061d229 -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
