Looks good to me, Ales.
Acked-by: Mark Michelson <[email protected]>
On 8/20/24 05:01, Ales Musil wrote:
If multichassis VIF is set with "unknown" address it might store
wrong MAC address into the FDB table. The ICMP Need frag is generated
by swapping the MAC src and dst of the original packet, however the
inport remains the same. As a consequence the match on the inport to
learn FDB will store source MAC address which is the original
destination address, that leads to redirection of traffic the VIF
which is wrong.
Swap the inport and outport for the ICMP error to prevent this issue
it also makes more sense as the ICMP is supposed to be generated by
entity along the way and not by the original VIF.
Note that those flows is still needed as userspace datapath is not
capable of PMTUD yet.
Reported-at: https://issues.redhat.com/browse/FDP-620
Signed-off-by: Ales Musil <[email protected]>
---
v2: Rebase on top of latest main.
Fix the typo in the test and add note into the commit message.
---
controller/physical.c | 6 ++++++
tests/ovn.at | 24 +++++++++++++++++++++---
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/controller/physical.c b/controller/physical.c
index 9e04ad5f2..55edbd54e 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -1258,6 +1258,12 @@ reply_imcp_error_if_pkt_too_big(struct
ovn_desired_flow_table *flow_table,
ofpact_put_set_field(
&inner_ofpacts, mf_from_id(MFF_LOG_FLAGS), &value, &mask);
+ /* inport <-> outport */
+ put_stack(MFF_LOG_INPORT, ofpact_put_STACK_PUSH(&inner_ofpacts));
+ put_stack(MFF_LOG_OUTPORT, ofpact_put_STACK_PUSH(&inner_ofpacts));
+ put_stack(MFF_LOG_INPORT, ofpact_put_STACK_POP(&inner_ofpacts));
+ put_stack(MFF_LOG_OUTPORT, ofpact_put_STACK_POP(&inner_ofpacts));
+
/* eth.src <-> eth.dst */
put_stack(MFF_ETH_DST, ofpact_put_STACK_PUSH(&inner_ofpacts));
put_stack(MFF_ETH_SRC, ofpact_put_STACK_PUSH(&inner_ofpacts));
diff --git a/tests/ovn.at b/tests/ovn.at
index 50c9f04da..bbb1bee35 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -15740,14 +15740,17 @@ m4_define([MULTICHASSIS_PATH_MTU_DISCOVERY_TEST],
second_mac=00:00:00:00:00:02
multi1_mac=00:00:00:00:00:f0
multi2_mac=00:00:00:00:00:f1
+ external_mac=00:00:00:00:ee:ff
first_ip=10.0.0.1
second_ip=10.0.0.2
multi1_ip=10.0.0.10
multi2_ip=10.0.0.20
+ external_ip=10.0.0.30
first_ip6=abcd::1
second_ip6=abcd::2
multi1_ip6=abcd::f0
multi2_ip6=abcd::f1
+ external_ip6=abcd::eeff
check ovn-nbctl ls-add ls0
check ovn-nbctl lsp-add ls0 first
@@ -15866,6 +15869,24 @@ m4_define([MULTICHASSIS_PATH_MTU_DISCOVERY_TEST],
reset_env
+ AS_BOX([Multi with "unknown" to external doesn't produce wrong FDB])
+ len=3000
+ check ovn-nbctl --wait=hv lsp-set-addresses multi1 unknown
+
+ packet=$(send_ip_packet multi1 1 $multi1_mac $external_mac $multi1_ip
$external_ip $(payload $len) 1 ${expected_ip_mtu})
+ echo $packet >> hv1/multi1.expected
+
+ packet=$(send_ip6_packet multi1 1 $multi1_mac $external_mac $multi1_ip6
$external_ip6 $(payload $len) 1 ${expected_ip_mtu})
+ echo $packet >> hv1/multi1.expected
+
+ check_pkts
+ reset_env
+
+ check_row_count fdb 0 mac="$external_mac"
+ ovn-sbctl --all destroy fdb
+
+ check ovn-nbctl --wait=hv lsp-set-addresses multi1 "${multi1_mac} ${multi1_ip}
${multi1_ip6}"
+
AS_BOX([Packets of proper size are delivered from multichassis to regular
ports])
len=1000
@@ -15965,9 +15986,6 @@ m4_define([MULTICHASSIS_PATH_MTU_DISCOVERY_TEST],
packet=$(send_ip6_packet multi1 1 $multi1_mac $multi2_mac $multi1_ip6
$multi2_ip6 $(payload $len) 1)
echo $packet >> hv1/multi1.expected
- check_pkts
- reset_env
-
AS_BOX([MTU updates are honored in ICMP Path MTU calculation])
set_mtu() {
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
