From patchwork Sat Dec 16 03:48:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1876862 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4SsXC76sfsz23p3 for ; Sat, 16 Dec 2023 14:49:22 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 7857861B1C; Sat, 16 Dec 2023 03:49:19 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 7857861B1C X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UQNOgAEjfU6Y; Sat, 16 Dec 2023 03:49:18 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 1D03261004; Sat, 16 Dec 2023 03:49:17 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 1D03261004 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id EC6ACC0077; Sat, 16 Dec 2023 03:49:16 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 59EB0C0037 for ; Sat, 16 Dec 2023 03:49:15 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 2085942262 for ; Sat, 16 Dec 2023 03:49:15 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 2085942262 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bGqaSPg3ZX5J for ; Sat, 16 Dec 2023 03:49:13 +0000 (UTC) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::221]) by smtp4.osuosl.org (Postfix) with ESMTPS id 704B742124 for ; Sat, 16 Dec 2023 03:49:13 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 704B742124 Received: by mail.gandi.net (Postfix) with ESMTPSA id 30C45240002; Sat, 16 Dec 2023 03:49:09 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Fri, 15 Dec 2023 22:48:58 -0500 Message-ID: <20231216034858.3814485-1-numans@ovn.org> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-GND-Sasl: numans@ovn.org Subject: [ovs-dev] [PATCH ovn] Revert "ovn: add geneve PMTUD support" X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique This reverts commit 450e41e783bfa69e4f9d6c80f6bcb01147d5cfe1. If a packet has to be tunnelled to another node and if the physical interface used for tunnelling has lower MTU than the packet or if there is a route exception with a lower MTU, then the geneve kernel module generates an ICMP need frag packet. This packet was getting dropped since the metadata had to be swapped. The commit [1] did exactly that and fixed the issue. But it has 2 issues - 1. It introduced a regression for the scenario when an ICMP need frag packet generated outside of OVN has to be tunnelled and delivered to the destination VM/pod. These ICMP need frag packets are now dropped. 2. If the logical switches has ACLs or load balancers configured then these icmp need frag packets are dropped as they are not sent to the correct zone. Its better to revert until we find a proper solution for the original issue. [1] - 450e41e783bf("ovn: add geneve PMTUD support") Signed-off-by: Numan Siddique Acked-by: Dumitru Ceara --- .../workflows/ovn-fake-multinode-tests.yml | 6 +- NEWS | 1 - controller/physical.c | 45 ------------ northd/northd.c | 24 ------- northd/ovn-northd.8.xml | 8 --- tests/multinode.at | 69 ------------------- tests/ovn-northd.at | 6 -- 7 files changed, 3 insertions(+), 156 deletions(-) diff --git a/.github/workflows/ovn-fake-multinode-tests.yml b/.github/workflows/ovn-fake-multinode-tests.yml index b3ba82a30b..25610df534 100644 --- a/.github/workflows/ovn-fake-multinode-tests.yml +++ b/.github/workflows/ovn-fake-multinode-tests.yml @@ -76,8 +76,8 @@ jobs: fail-fast: false matrix: cfg: - - { branch: "main", testsuiteflags: ""} - - { branch: "branch-22.03", testsuiteflags: "-k 'basic test'" } + - { branch: "main" } + - { branch: "branch-22.03" } name: multinode tests ${{ join(matrix.cfg.*, ' ') }} env: RUNC_CMD: podman @@ -176,7 +176,7 @@ jobs: - name: Run fake-multinode system tests run: | - if ! sudo make check-multinode TESTSUITEFLAGS="${{ matrix.cfg.testsuiteflags }}"; then + if ! sudo make check-multinode; then sudo podman exec -it ovn-central ovn-nbctl show || : sudo podman exec -it ovn-central ovn-sbctl show || : sudo podman exec -it ovn-chassis-1 ovs-vsctl show || : diff --git a/NEWS b/NEWS index acb3b854fb..e10fb79dd8 100644 --- a/NEWS +++ b/NEWS @@ -9,7 +9,6 @@ Post v23.09.0 connection method and doesn't require additional probing. external_ids:ovn-openflow-probe-interval configuration option for ovn-controller no longer matters and is ignored. - - Enable PMTU discovery on geneve tunnels for E/W traffic. OVN v23.09.0 - 15 Sep 2023 -------------------------- diff --git a/controller/physical.c b/controller/physical.c index ce3b88d165..ba88e1d8b4 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -2446,51 +2446,6 @@ physical_run(struct physical_ctx *p_ctx, &ofpacts, hc_uuid); } - /* Add specif flows for E/W ICMPv{4,6} packets if tunnelled packets do not - * fit path MTU. - */ - HMAP_FOR_EACH (tun, hmap_node, p_ctx->chassis_tunnels) { - ofpbuf_clear(&ofpacts); - if (tun->type == GENEVE) { - put_move(MFF_TUN_ID, 0, MFF_LOG_DATAPATH, 0, 24, &ofpacts); - put_move(p_ctx->mff_ovn_geneve, 16, MFF_LOG_OUTPORT, 0, 16, - &ofpacts); - put_move(p_ctx->mff_ovn_geneve, 0, MFF_LOG_INPORT, 0, 15, - &ofpacts); - } else if (tun->type == STT) { - put_move(MFF_TUN_ID, 0, MFF_LOG_DATAPATH, 0, 24, &ofpacts); - put_move(MFF_TUN_ID, 40, MFF_LOG_OUTPORT, 0, 16, &ofpacts); - put_move(MFF_TUN_ID, 24, MFF_LOG_INPORT, 0, 15, &ofpacts); - } else if (tun->type == VXLAN) { - put_move(MFF_TUN_ID, 0, MFF_LOG_DATAPATH, 0, 12, &ofpacts); - put_move(MFF_TUN_ID, 12, MFF_LOG_INPORT, 0, 12, &ofpacts); - } else { - OVS_NOT_REACHED(); - } - put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts); - - /* IPv4 */ - struct match match = MATCH_CATCHALL_INITIALIZER; - match_set_in_port(&match, tun->ofport); - match_set_dl_type(&match, htons(ETH_TYPE_IP)); - match_set_nw_proto(&match, IPPROTO_ICMP); - match_set_icmp_type(&match, 3); - match_set_icmp_code(&match, 4); - - ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 120, 0, &match, - &ofpacts, hc_uuid); - /* IPv6 */ - match_init_catchall(&match); - match_set_in_port(&match, tun->ofport); - match_set_dl_type(&match, htons(ETH_TYPE_IPV6)); - match_set_nw_proto(&match, IPPROTO_ICMPV6); - match_set_icmp_type(&match, 2); - match_set_icmp_code(&match, 0); - - ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 120, 0, &match, - &ofpacts, hc_uuid); - } - /* Add VXLAN specific rules to transform port keys * from 12 bits to 16 bits used elsewhere. */ HMAP_FOR_EACH (tun, hmap_node, p_ctx->chassis_tunnels) { diff --git a/northd/northd.c b/northd/northd.c index 792f8ba7fa..fa4f67dc65 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -12763,28 +12763,6 @@ build_lrouter_force_snat_flows(struct hmap *lflows, struct ovn_datapath *od, ds_destroy(&actions); } -/* Following flows are used to manage traffic redirected by the kernel - * (e.g. ICMP errors packets) that enter the cluster from the geneve ports - */ -static void -build_lrouter_redirected_traffic_flows( - struct ovn_port *op, struct hmap *lflows, - struct ds *match, struct ds *actions) -{ - ovs_assert(op->nbrp); - if (!is_l3dgw_port(op)) { - return; - } - - ds_clear(match); - ds_put_format(match, "inport == %s && !is_chassis_resident(%s)", - op->cr_port->json_key, op->cr_port->json_key); - ds_clear(actions); - ds_put_format(actions, "inport = %s; next;", op->json_key); - ovn_lflow_add(lflows, op->od, S_ROUTER_IN_ADMISSION, 120, - ds_cstr(match), ds_cstr(actions)); -} - static void build_lrouter_force_snat_flows_op(struct ovn_port *op, struct hmap *lflows, @@ -16188,8 +16166,6 @@ build_lswitch_and_lrouter_iterate_by_lrp(struct ovn_port *op, &lsi->match, &lsi->actions, lsi->meter_groups); build_lrouter_force_snat_flows_op(op, lsi->lflows, &lsi->match, &lsi->actions); - build_lrouter_redirected_traffic_flows(op, lsi->lflows, &lsi->match, - &lsi->actions); } static void * diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index a77bd719e8..97718821fe 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -2463,14 +2463,6 @@ output; (LBs, NAT).

-

- For each gateway port GW on a distributed logical router - a priority-120 flow that matches inport == cr-GW - && !is_chassis_resident(cr-GW) where - cr-GW is the chassis resident port of GW, - stores GW as inport and advances to the next table. -

-

For a distributed logical router or for gateway router where the port is configured with options:gateway_mtu diff --git a/tests/multinode.at b/tests/multinode.at index a051ea8c2a..2b199b4bce 100644 --- a/tests/multinode.at +++ b/tests/multinode.at @@ -72,72 +72,3 @@ M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ping -q -c 3 -i 0.3 -w 2 20.0.0.3 | F ]) AT_CLEANUP - -AT_SETUP([ovn multinode geneve pmtu]) - -# Check that ovn-fake-multinode setup is up and running -check_fake_multinode_setup - -# Delete the multinode NB and OVS resources before starting the test. -cleanup_multinode_resources - -# Test East-West switching -check multinode_nbctl ls-add sw0 -check multinode_nbctl lsp-add sw0 sw0-port1 -check multinode_nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:03 10.0.0.3 1000::3" -check multinode_nbctl lsp-add sw0 sw0-port2 -check multinode_nbctl lsp-set-addresses sw0-port2 "50:54:00:00:00:04 10.0.0.4 1000::4" - -m_as ovn-chassis-1 /data/create_fake_vm.sh sw0-port1 sw0p1 50:54:00:00:00:03 10.0.0.3 24 10.0.0.1 1000::3/64 1000::a -m_as ovn-chassis-2 /data/create_fake_vm.sh sw0-port2 sw0p2 50:54:00:00:00:04 10.0.0.4 24 10.0.0.1 1000::4/64 1000::a - -m_wait_for_ports_up - -M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.4 | FORMAT_PING], \ -[0], [dnl -3 packets transmitted, 3 received, 0% packet loss, time 0ms -]) - -# Create the second logical switch with one port -check multinode_nbctl ls-add sw1 -check multinode_nbctl lsp-add sw1 sw1-port1 -check multinode_nbctl lsp-set-addresses sw1-port1 "40:54:00:00:00:03 20.0.0.3 2000::3" - -# Create a logical router and attach both logical switches -check multinode_nbctl lr-add lr0 -check multinode_nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::a/64 -check multinode_nbctl lsp-add sw0 sw0-lr0 -check multinode_nbctl lsp-set-type sw0-lr0 router -check multinode_nbctl lsp-set-addresses sw0-lr0 router -check multinode_nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 - -check multinode_nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24 2000::a/64 -check multinode_nbctl lsp-add sw1 sw1-lr0 -check multinode_nbctl lsp-set-type sw1-lr0 router -check multinode_nbctl lsp-set-addresses sw1-lr0 router -check multinode_nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1 - -m_as ovn-chassis-2 /data/create_fake_vm.sh sw1-port1 sw1p1 40:54:00:00:00:03 20.0.0.3 24 20.0.0.1 2000::4/64 1000::a - -m_wait_for_ports_up sw1-port1 - -M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ping -q -c 3 -i 0.3 -w 2 20.0.0.3 | FORMAT_PING], \ -[0], [dnl -3 packets transmitted, 3 received, 0% packet loss, time 0ms -]) - -# Change ptmu for the geneve tunnel -m_as ovn-chassis-1 ip route change 170.168.0.0/16 mtu 1200 dev eth1 -M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ping -c 5 -s 1300 -M do 20.0.0.3 2>&1 |grep -q "message too long, mtu=1142"]) - -check multinode_nbctl lrp-set-gateway-chassis lr0-sw0 ovn-chassis-1 10 -check multinode_nbctl lrp-set-gateway-chassis lr0-sw1 ovn-chassis-2 10 - -M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ip route flush dev sw0p1]) -M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ip route add 10.0.0.0/24 dev sw0p1]) -M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ip route add default via 10.0.0.1 dev sw0p1]) - -m_as ovn-chassis-1 ip route change 170.168.0.0/16 mtu 1200 dev eth1 -M_NS_CHECK_EXEC([ovn-chassis-1], [sw0p1], [ping -c 5 -s 1300 -M do 20.0.0.3 2>&1 |grep -q "message too long, mtu=1142"]) - -AT_CLEANUP diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 19e4f1263e..9df459f7bf 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -6480,9 +6480,6 @@ AT_CAPTURE_FILE([lrflows]) # Check the flows in lr_in_admission stage AT_CHECK([grep lr_in_admission lrflows | grep cr-DR | sort], [0], [dnl - table=0 (lr_in_admission ), priority=120 , match=(inport == "cr-DR-S1" && !is_chassis_resident("cr-DR-S1")), action=(inport = "DR-S1"; next;) - table=0 (lr_in_admission ), priority=120 , match=(inport == "cr-DR-S2" && !is_chassis_resident("cr-DR-S2")), action=(inport = "DR-S2"; next;) - table=0 (lr_in_admission ), priority=120 , match=(inport == "cr-DR-S3" && !is_chassis_resident("cr-DR-S3")), action=(inport = "DR-S3"; next;) table=0 (lr_in_admission ), priority=50 , match=(eth.dst == 02:ac:10:01:00:01 && inport == "DR-S1" && is_chassis_resident("cr-DR-S1")), action=(xreg0[[0..47]] = 02:ac:10:01:00:01; next;) table=0 (lr_in_admission ), priority=50 , match=(eth.dst == 03:ac:10:01:00:01 && inport == "DR-S2" && is_chassis_resident("cr-DR-S2")), action=(xreg0[[0..47]] = 03:ac:10:01:00:01; next;) table=0 (lr_in_admission ), priority=50 , match=(eth.dst == 04:ac:10:01:00:01 && inport == "DR-S3" && is_chassis_resident("cr-DR-S3")), action=(xreg0[[0..47]] = 04:ac:10:01:00:01; next;) @@ -6542,7 +6539,6 @@ AT_CAPTURE_FILE([lrflows]) # Check the flows in lr_in_admission stage AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed 's/table=../table=??/' | sort], [0], [dnl - table=??(lr_in_admission ), priority=120 , match=(inport == "cr-lrp1" && !is_chassis_resident("cr-lrp1")), action=(inport = "lrp1"; next;) table=??(lr_in_admission ), priority=50 , match=(eth.dst == 00:00:00:00:00:01 && inport == "lrp1" && is_chassis_resident("cr-lrp1")), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) table=??(lr_in_admission ), priority=50 , match=(eth.mcast && inport == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) ]) @@ -6564,7 +6560,6 @@ AT_CAPTURE_FILE([lrflows]) # Check the flows in lr_in_admission stage AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed 's/table=../table=??/' | sort], [0], [dnl - table=??(lr_in_admission ), priority=120 , match=(inport == "cr-lrp1" && !is_chassis_resident("cr-lrp1")), action=(inport = "lrp1"; next;) table=??(lr_in_admission ), priority=50 , match=(eth.dst == 00:00:00:00:00:01 && inport == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) table=??(lr_in_admission ), priority=50 , match=(eth.mcast && inport == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) ]) @@ -6583,7 +6578,6 @@ AT_CAPTURE_FILE([lrflows]) # Check the flows in lr_in_admission stage AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed 's/table=../table=??/' | sort], [0], [dnl - table=??(lr_in_admission ), priority=120 , match=(inport == "cr-lrp1" && !is_chassis_resident("cr-lrp1")), action=(inport = "lrp1"; next;) table=??(lr_in_admission ), priority=50 , match=(eth.dst == 00:00:00:00:00:01 && inport == "lrp1" && is_chassis_resident("cr-lrp1")), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) table=??(lr_in_admission ), priority=50 , match=(eth.mcast && inport == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) ])