Thanks for reviewing this as well, Ales. On Wed, Mar 6, 2024 at 8:08 AM Ales Musil <[email protected]> wrote:
> > > On Wed, Mar 6, 2024 at 8:07 AM Ales Musil <[email protected]> wrote: > >> >> >> On Mon, Mar 4, 2024 at 11:56 AM 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 >>> 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. >>> >>> If an additional security is needed, ACLs can be used to filter >>> out incomming traffic from external networks. >>> >>> 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]> >>> --- >>> >> >> Hi Martin, >> >> I have a few comments down below. >> >> >>> northd/northd.c | 82 ++++++++++++++++++++++++++++++++++++----- >>> northd/ovn-northd.8.xml | 29 +++++++++++++++ >>> tests/ovn-northd.at | 15 +++++++- >>> tests/system-ovn.at | 69 ++++++++++++++++++++++++++++++++++ >>> 4 files changed, 185 insertions(+), 10 deletions(-) >>> >>> diff --git a/northd/northd.c b/northd/northd.c >>> index 2c3560ce2..4b79b357c 100644 >>> --- a/northd/northd.c >>> +++ b/northd/northd.c >>> @@ -14437,21 +14437,28 @@ 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) >>> +build_lrouter_out_snat_direction_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, >>> + 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 >>> @@ -14462,11 +14469,37 @@ 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); >>> } >>> } >>> >>> +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) >>> +{ >>> + build_lrouter_out_snat_direction_match__(lflows, od, nat, match, >>> + distributed_nat, >>> cidr_bits, is_v6, >>> + l3dgw_port, lflow_ref, >>> false); >>> +} >>> + >>> +static void >>> +build_lrouter_out_snat_reverse_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) >>> +{ >>> + build_lrouter_out_snat_direction_match__(lflows, od, nat, match, >>> + distributed_nat, >>> cidr_bits, is_v6, >>> + l3dgw_port, lflow_ref, >>> true); >>> +} >>> + >>> >> >> nit: I don't think we need two new functions when only one argument was >> added. Considering the old function was used at 3 places only anyway. >> > Ack. I went with this approach because I deemed it more cautious/less intrusive, although bit more verbose. However If you are OK with changing the function's signature, I can go with that as well. > >> >>> static void >>> build_lrouter_out_snat_stateless_flow(struct lflow_table *lflows, >>> const struct ovn_datapath *od, >>> @@ -14591,6 +14624,7 @@ build_lrouter_out_snat_flow(struct lflow_table >>> *lflows, >>> return; >>> } >>> >>> + struct ds match_all_from_snat = DS_EMPTY_INITIALIZER; >>> ds_clear(actions); >>> >>> /* The priority here is calculated such that the >>> @@ -14600,6 +14634,7 @@ build_lrouter_out_snat_flow(struct lflow_table >>> *lflows, >>> >>> build_lrouter_out_snat_match(lflows, od, nat, match, >>> distributed_nat, >>> cidr_bits, is_v6, l3dgw_port, >>> lflow_ref); >>> + ds_clone(&match_all_from_snat, match); >>> >>> if (!od->is_gw_router) { >>> /* Distributed router. */ >>> @@ -14624,6 +14659,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 (!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. >>> + */ >>> + ovn_lflow_add(lflows, od, S_ROUTER_OUT_POST_UNDNAT, 70, >>> + ds_cstr(&match_all_from_snat), "ct_snat; ", >>> + lflow_ref); >>> >> + >>> + build_lrouter_out_snat_reverse_match(lflows, od, nat, match, >>> + distributed_nat, >>> cidr_bits, is_v6, >>> + l3dgw_port, lflow_ref); >>> + >>> + /* 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(snat); next; ", >>> + lflow_ref); >>> + } >>> + >>> + ds_destroy(&match_all_from_snat); >>> >> >> I suppose that the gw router works because we have ct_commit for new >> traffic [0]. I wonder if we could extend that with a check for the DGP case. >> > Yes. I think so too. I tested removing the `is_gw_router` condition and it fixed our use-case on the Distributed router. However it also broke tests fro "stateless" NAT because there were calls to "ct_*" in the flows even if stateless NAT was requested. I ultimately decided to go with this, more targeted, approach rather than sending everything to conntrack. > >> >>> } >>> >>> static void >>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml >>> index 17b414144..ce6fd1270 100644 >>> --- a/northd/ovn-northd.8.xml >>> +++ b/northd/ovn-northd.8.xml >>> @@ -4869,6 +4869,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 >>> @@ -5061,6 +5068,18 @@ 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>. >>> @@ -5072,6 +5091,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 0732486f3..a7597a975 100644 >>> --- a/tests/ovn-northd.at >>> +++ b/tests/ovn-northd.at >>> @@ -1155,6 +1155,7 @@ 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);) >>> ]) >>> >>> @@ -1185,6 +1186,7 @@ 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;) >>> ]) >>> @@ -1279,7 +1281,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 >>> >>> @@ -5590,12 +5592,16 @@ 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);) >>> ]) >>> @@ -5738,12 +5744,16 @@ 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);) >>> ]) >>> @@ -7643,6 +7653,9 @@ 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);) >>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >>> index c22c7882f..80e9b7d0b 100644 >>> --- a/tests/system-ovn.at >>> +++ b/tests/system-ovn.at >>> @@ -3572,6 +3572,40 @@ 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.pcap | wc -l) >>> test "${total_pkts}" = "3" >>> @@ -3738,6 +3772,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 >>> @@ -3918,6 +3985,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> >>> ]) >>> >>> @@ -4086,6 +4154,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 >>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >>> >> One thing that slightly worries me is the potential impact on traffic >> performance. Basically anything going towards the SNAT would go through CT >> at least once. >> > Am I correct to assume that the "expensive" operation here is only the `ct_commit`? If so, then only "new" traffic towards SNAT would be something "additional" that goes through CT and "reply" traffic should be unaffected (performance-wise), correct? > >> Thanks, >> Ales >> >> -- >> >> Ales Musil >> >> Senior Software Engineer - OVN Core >> >> Red Hat EMEA <https://www.redhat.com> >> >> [email protected] >> <https://red.ht/sig> >> > > Forgot to add the link. > > [0] > https://github.com/ovn-org/ovn/blob/b92ad9e0b408a202273d69ba573f2538e53c6e48/northd/northd.c#L14998-L15009 > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > [email protected] > <https://red.ht/sig> > -- Best Regards, Martin Kalcok. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
