On Wed, Mar 6, 2024 at 1:50 PM Martin Kalcok <[email protected]> wrote:
> > 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? > Actually not only that, any CT operation has its own cost, it also splits megaflows into smaller pieces because with every CT we have to recirculate. > > >> >>> 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. > -- 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
