physical: Prevent wrong FDB to be learned with multichassis port.

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]> Acked-by: Mark Michelson <[email protected]> Signed-off-by: Numan Siddique <[email protected]>
almusil · Sep 19, 2024 · 5bf9cb9 · 5bf9cb9
1 parent f958cc7
commit 5bf9cb9
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 3 deletions.
diff --git 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
@@ -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() {