From patchwork Wed Oct 23 17:06:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 2001205 X-Patchwork-Delegate: i.maximets@samsung.com 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::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (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 4XYb7415Bkz1xtp for ; Thu, 24 Oct 2024 04:06:39 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id A21118101B; Wed, 23 Oct 2024 17:06:37 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id 7tUJCjmddcST; Wed, 23 Oct 2024 17:06:36 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 8B83280F52 Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id 8B83280F52; Wed, 23 Oct 2024 17:06:36 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5DE12C08A6; Wed, 23 Oct 2024 17:06:36 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 35657C08A3 for ; Wed, 23 Oct 2024 17:06:35 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 1C5BF6088F for ; Wed, 23 Oct 2024 17:06:35 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id rk9wSVPPe0in for ; Wed, 23 Oct 2024 17:06:34 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=209.85.128.68; helo=mail-wm1-f68.google.com; envelope-from=i.maximets.ovn@gmail.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp3.osuosl.org 2240B6088E Authentication-Results: smtp3.osuosl.org; dmarc=none (p=none dis=none) header.from=ovn.org DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 2240B6088E Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by smtp3.osuosl.org (Postfix) with ESMTPS id 2240B6088E for ; Wed, 23 Oct 2024 17:06:33 +0000 (UTC) Received: by mail-wm1-f68.google.com with SMTP id 5b1f17b1804b1-43167ff0f91so51444595e9.1 for ; Wed, 23 Oct 2024 10:06:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729703192; x=1730307992; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=AEXs26SparryJI1JEhzP7pzN/zhJhm7PKWqloXiwp3g=; b=sNf5oLMFYZGIQpIswbU5tXbxm6T/BAyZOHnB5SzGHmsPGu5Y0PkY+jXVULkHh60C9x 41mtjmZesxt3sIRjRcW8fkuFbOLX4hnGqnkYHT4L9g6HvgxR/S9Ut6ytsHJO+A74xISI NbpeZIIGYTxr7PyaxNb23cZRZseatR5n8+rHfJq2TOjAc5416YNSVceTnjPDSTCoPH92 48RRQBOMJ+VIg5MPwHtAvSNyUOANEzdNDJSQNdOFe/FcBIwL2bFYuDrA4xLhhk/ZJ7WA 7IKy4JwD76HuFNkbijd3vS7sFxs5q1jPFxyj/nrpeWD58fFAG0qUX51dJlfWYC1AocKz +D8w== X-Gm-Message-State: AOJu0YxGx3weNG/3Yr6llk6b2vJ4adc89O92BBl80MucltVd0G3dU5bO jheGyxWelzcTJ0oadIB7+ToA9nZxgfHROjZX6NJsctnlEiu9R/wbpPcPfPZk X-Google-Smtp-Source: AGHT+IEMjqGWJ0mEpfR21uo52vPkJA/XAOISon0baRUrD+iJRp6gV/rJJw1QjgMS7ykusKHZumGogQ== X-Received: by 2002:a05:600c:1c11:b0:42c:a574:6360 with SMTP id 5b1f17b1804b1-431841aff4emr29538605e9.29.1729703191664; Wed, 23 Oct 2024 10:06:31 -0700 (PDT) Received: from im-t490s.redhat.com (ip-86-49-44-151.bb.vodafone.cz. [86.49.44.151]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-37ee0a614d0sm9332592f8f.63.2024.10.23.10.06.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Oct 2024 10:06:31 -0700 (PDT) From: Ilya Maximets To: ovs-dev@openvswitch.org Cc: Ilya Maximets Date: Wed, 23 Oct 2024 19:06:08 +0200 Message-ID: <20241023170625.1802298-1-i.maximets@ovn.org> X-Mailer: git-send-email 2.46.0 MIME-Version: 1.0 Subject: [ovs-dev] [PATCH] meta-flow: Fix nw_frag mask while parsing from string. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" mf_from_frag_string() sets all the upper bits of the nw_frag to make sure the exact match check will pass. This was not taken into account while splitting nw_frag and IP TOS bits into separate fields and the mask clean up was removed from the cls_rule_set_frag_masked() which is now called match_set_nw_frag_masked(). This leads to the case where the match parsed from the string is not considered equal to the match parsed from the OpenFlow, due to difference in masks. And that is causing ovs-ofctl replace-flows to always replace flows that match on nw_frag, even if they are exactly the same triggering unnecessary flow table updates and revalidation. $ cat frag_flows.txt ip,in_port=1,nw_frag=yes actions=drop $ ovs-ofctl dump-flows --no-stat --no-names br0 ip,in_port=1,nw_frag=yes actions=drop $ ovs-ofctl -vvconn replace-flows br0 frag_flows.txt 2>&1 | grep MOD NXT_FLOW_MOD: DEL_STRICT ip,in_port=1,nw_frag=yes actions=drop NXT_FLOW_MOD: ADD ip,in_port=1,nw_frag=yes actions=drop Clear the extra mask bits while setting match/flow structure from the field to avoid the issue. The issue mainly affects non-exact matches 'non_later' and 'yes', since exact matches are special-handled in many places / considered equal to not having a mask at all. Note: ideally we would not use byte-sized is_all_ones() for exact match checking, but use field-specific checks instead. However, this leads to a much larger change throughout OVS code base and would be much harder to backport. For now, fixing the issue in the way the code was originally implemented. Fixes: 9e44d715638a ("Don't overload IP TOS with the frag matching bits.") Signed-off-by: Ilya Maximets Acked-by: Aaron Conole Acked-by: Eelco Chaudron --- lib/meta-flow.c | 3 ++- tests/ovs-ofctl.at | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/lib/meta-flow.c b/lib/meta-flow.c index 499be04b6..7ebdc1d2c 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -2545,7 +2545,8 @@ mf_set(const struct mf_field *mf, break; case MFF_IP_FRAG: - match_set_nw_frag_masked(match, value->u8, mask->u8); + match_set_nw_frag_masked(match, value->u8, + mask->u8 & FLOW_NW_FRAG_MASK); break; case MFF_ARP_SPA: diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index e2f4429ae..2363b72aa 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -3100,6 +3100,51 @@ AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed '/OFPST_FLO OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ovs-ofctl replace-flows with fragments]) +OVS_VSWITCHD_START + +AT_DATA([frag_flows.txt], [dnl + ip,nw_frag=first actions=drop + ip,nw_frag=later actions=drop + ip,nw_frag=no actions=NORMAL + ip,nw_frag=not_later actions=NORMAL + ip,nw_frag=yes actions=LOCAL +]) +AT_DATA([replace_flows.txt], [dnl + ip,nw_frag=first actions=NORMAL + ip,nw_frag=later actions=LOCAL + ip,nw_frag=no actions=drop + ip,nw_frag=not_later actions=drop + ip,nw_frag=yes actions=drop +]) + +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 frag_flows.txt]) +on_exit 'ovs-ofctl -O OpenFlow13 dump-flows br0' + +dnl Check that flow replacement works. +AT_CHECK([ovs-ofctl -vvconn:console:dbg -O OpenFlow13 \ + replace-flows br0 replace_flows.txt 2>&1 | grep FLOW_MOD \ + | sed 's/.*\(OFPT_FLOW_MOD.*\)/\1/' | strip_xids | sort], [0], [dnl +OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=first actions=NORMAL +OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=later actions=LOCAL +OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=no actions=drop +OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=not_later actions=drop +OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=yes actions=drop +]) + +dnl Check that replacement to the same set doesn't cause flow modifications. +AT_CHECK([ovs-ofctl -vvconn:console:dbg -O OpenFlow13 \ + replace-flows br0 replace_flows.txt 2>&1 | grep FLOW_MOD \ + | sed 's/.*\(OFPT_FLOW_MOD.*\)/\1/' | strip_xids | sort], [0], []) + +dnl Compare the flow dump against the expected set. +cat replace_flows.txt > expout +AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 \ + | ofctl_strip | sed '/OFPST_FLOW/d' | sort], [0], [expout]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ovs-ofctl replace-flows with --bundle]) OVS_VSWITCHD_START