On Thu, Mar 14, 2024 at 2:13 PM Martin Kalcok <[email protected]>
wrote:
> Hello all,
> I have one more follow-up regarding the comments in v1. @amusil, you were
> concerned about the impact this change would have on the performance so I
> ran some tests to try to gauge it. I used following setup with two physical
> hosts connected over switch:
>
> | Physical ext. | Hypervisor1
> | network |
> alice1-|----- switch -----|-- DLR --- foo1
> | |
>
> * alice1 acts as external device.
> * It doesn't run OVN
> * It's connected to regular physical network.
> * Hypervisor1 runs OVN chassis
> * foo1 is a container behind Distributed LR with SNAT enabled.
>
> Goal of this test was to measure whether the proposed patch affects
> throughput of the traffic that flows from "foo1" to external network. I
> used iperf3 and ran 3x 5 minute measurements with 10 streams ("iperf3 -i 0
> -t 300 -P 10 -c alice1"). I used 5 minute for each measurement to smooth
> out possible jitter and I ran each measurement 3 times to get feel for the
> variance in the network throughput over longer period. I also did some
> trial and error testing that showed that 10 parallel streams reached
> highest throughput.
>
> Following are the three (summary) results with this patch applied:
>
> # run 1 (with patch)
> [SUM] 0.00-300.00 sec 296 GBytes 8.48 Gbits/sec sender
> [SUM] 0.00-300.00 sec 296 GBytes 8.48 Gbits/sec receiver
>
> # run 2 (with patch)
> [SUM] 0.00-300.00 sec 288 GBytes 8.24 Gbits/sec sender
> [SUM] 0.00-300.00 sec 288 GBytes 8.23 Gbits/sec receiver
>
> # run 3 (with patch)
> [SUM] 0.00-300.00 sec 299 GBytes 8.55 Gbits/sec sender
> [SUM] 0.00-300.00 sec 299 GBytes 8.55 Gbits/sec receiver
>
> Next thing I did was to rebuild ovn-controller without these patches,
> replaced the ovn-controller binary on the Hypervisor1 and restarted
> ovn-controller daemon. I verified that the feature flag "ct-commit-to-zone"
> was removed from the chassis (in the SB database), which forced the
> recompute, and then I reran the tests:
>
> # run 1 (clean)
> [SUM] 0.00-300.00 sec 300 GBytes 8.59 Gbits/sec sender
> [SUM] 0.00-300.00 sec 300 GBytes 8.59 Gbits/sec receiver
>
> # run 2 (clean)
> [SUM] 0.00-300.00 sec 285 GBytes 8.15 Gbits/sec sender
> [SUM] 0.00-300.00 sec 285 GBytes 8.15 Gbits/sec receiver
>
> # run 3 (clean)
> [SUM] 0.00-300.00 sec 295 GBytes 8.46 Gbits/sec sender
> [SUM] 0.00-300.00 sec 295 GBytes 8.46 Gbits/sec receiver
>
> These tests show that while there was some fluctuation between each
> individual test, when comparing patched and clean version, they come out
> about the same. I'm happy to run more tests/scenarios if you have something
> else in mind that should be tested.
>
>
>
Hi Martin,
thank you for the performance numbers, it looks fine to me. I have some
comments, see down below.
> On Wed, Mar 13, 2024 at 10:17 AM Martin Kalcok <
> [email protected]> wrote:
>
>> Regarding the failed unstable test in the CI, I suspect that this is not
>> related to the code change, I've had couple successful CI runs in my branch
>> [0].
>>
>> Martin.
>>
>> [0] https://github.com/mkalcok/ovn/actions/runs/8256539983
>>
>> On Tue, Mar 12, 2024 at 8:45 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
>>> 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 | 68 ++++++++++++++++++++++++++++++++--------
>>> northd/ovn-northd.8.xml | 29 +++++++++++++++++
>>> tests/ovn-northd.at | 33 ++++++++++++++++----
>>> tests/system-ovn.at | 69 +++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 180 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/northd/northd.c b/northd/northd.c
>>> index 2c3560ce2..25af52d5a 100644
>>> --- a/northd/northd.c
>>> +++ b/northd/northd.c
>>> @@ -14438,20 +14438,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
>>> @@ -14462,7 +14469,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);
>>> }
>>> }
>>> @@ -14489,7 +14496,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. */
>>> @@ -14536,7 +14544,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;
>>> @@ -14585,12 +14593,14 @@ 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;
>>> }
>>>
>>> + struct ds match_all_from_snat = DS_EMPTY_INITIALIZER;
>>> ds_clear(actions);
>>>
>>> /* The priority here is calculated such that the
>>> @@ -14599,7 +14609,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);
>>> + ds_clone(&match_all_from_snat, match);
>>>
>>
We don't have to clone the match, you can just store the match length e.g.
size_t match_len = match->len;
>
>>> if (!od->is_gw_router) {
>>> /* Distributed router. */
>>> @@ -14624,6 +14636,36 @@ 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) {
>>>
>>
Here you can just truncate the ds to the state before the previous if:
ds_truncate(match, match_len);
> + /* 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_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(snat); next; ",
>>> + lflow_ref);
>>> + }
>>> +
>>> + ds_destroy(&match_all_from_snat);
>>> }
>>>
>>> static void
>>> @@ -15167,7 +15209,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 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..d4a1212d0 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
>>> @@ -1135,6 +1136,7 @@ echo "CR-LRP UUID is: " $uuid
>>> check ovn-nbctl set Logical_Router $cr_uuid options:chassis=gw1
>>> check ovn-nbctl --wait=sb sync
>>>
>>> +
>>>
>>
nit: Unrelated
ovn-nbctl create Address_Set name=allowed_range addresses=\"1.1.1.1\"
>>> ovn-nbctl create Address_Set name=disallowed_range addresses=\"2.2.2.2\"
>>>
>>> @@ -1155,6 +1157,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 +1188,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 +1283,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
>>>
>>> @@ -5489,7 +5493,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
>>> @@ -5590,12 +5595,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 +5747,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);)
>>> ])
>>>
>> @@ -7576,9 +7589,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
>>> @@ -7643,6 +7661,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);)
>>>
>>
I would actually expect one of the tests in ovn-northd.at to also check the
flow that does the ct_commit(snat).
> 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])
>>>
>>
Do we actually need to send data? I think -z is also enough for TCP to
prove that the connection works.
> +}
>>> +
>>> +# 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])
>>>
>>
Same as above.
+}
>>> +
>>> +# 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
>>>
>>>
>>
>> --
>> Best Regards,
>> Martin Kalcok.
>>
>
>
> --
> Best Regards,
> Martin Kalcok.
>
Thanks,
Ales
--
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
