From patchwork Tue Nov 15 13:19:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eelco Chaudron X-Patchwork-Id: 1704086 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=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=g2UFKsQ0; dkim-atps=neutral Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4NBRcL56dKz23mY for ; Wed, 16 Nov 2022 00:20:02 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 06C9A409F3; Tue, 15 Nov 2022 13:20:00 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 06C9A409F3 Authentication-Results: smtp4.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=g2UFKsQ0 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 aUHYp6_s3XBL; Tue, 15 Nov 2022 13:19:58 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 8A2484047D; Tue, 15 Nov 2022 13:19:57 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 8A2484047D Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5AC91C0033; Tue, 15 Nov 2022 13:19:57 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id C5711C002D for ; Tue, 15 Nov 2022 13:19:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 8F1418139B for ; Tue, 15 Nov 2022 13:19:55 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 8F1418139B Authentication-Results: smtp1.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=g2UFKsQ0 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id zoCYGrL_x5sO for ; Tue, 15 Nov 2022 13:19:54 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 8FADF81349 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 8FADF81349 for ; Tue, 15 Nov 2022 13:19:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668518393; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=+L9ju0kxLKAo63gWJEgQCfNLrgW45hSNxQzT6pA7C44=; b=g2UFKsQ047bvsIT8pF4LjvZOpugfGtSkUp58Ol0zPIk99KOTZmUAnYPepQFdVLhAEVbXEU aFr1eNVgVPjZunkay7DHNMQ8XuyB1VMlsj63AI6SKzOIG11ZK6Ojx+wn9icJf0t4Pnq9nO /xNAe99xC+MrlXa4GozinvigxJPVmxE= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-197-Wkfe9kYeOTWIsXGs0u_HPQ-1; Tue, 15 Nov 2022 08:19:50 -0500 X-MC-Unique: Wkfe9kYeOTWIsXGs0u_HPQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 10989185A78F; Tue, 15 Nov 2022 13:19:50 +0000 (UTC) Received: from ebuild.redhat.com (unknown [10.39.193.124]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5A7222166B2B; Tue, 15 Nov 2022 13:19:49 +0000 (UTC) From: Eelco Chaudron To: dev@openvswitch.org Date: Tue, 15 Nov 2022 14:19:45 +0100 Message-Id: <166851836075.873171.3801658873892477358.stgit@ebuild> User-Agent: StGit/1.1 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: i.maximets@ovn.org Subject: [ovs-dev] [PATCH v2] dpif-netdev: Use unmasked key when adding datapath flows. 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" The datapath supports installing wider flows, and OVS relies on this behavior. For example if ipv4(src=1.1.1.1/192.0.0.0, dst=1.1.1.2/192.0.0.0) exists, a wider flow (smaller mask) of ipv4(src=192.1.1.1/128.0.0.0,dst=192.1.1.2/128.0.0.0) is allowed to be added. However, if we try to add a wildcard rule, the installation fails: # ovs-appctl dpctl/add-flow system@myDP "in_port(1),eth_type(0x0800), \ ipv4(src=1.1.1.1/192.0.0.0,dst=1.1.1.2/192.0.0.0,frag=no)" 2 # ovs-appctl dpctl/add-flow system@myDP "in_port(1),eth_type(0x0800), \ ipv4(src=192.1.1.1/0.0.0.0,dst=49.1.1.2/0.0.0.0,frag=no)" 2 ovs-vswitchd: updating flow table (File exists) The reason is that the key used to determine if the flow is already present in the system uses the original key ANDed with the mask. This results in the IP address not being part of the (miniflow) key, i.e., being substituted with an all-zero value. When doing the actual lookup, this results in the key wrongfully matching the first flow, and therefore the flow does not get installed. The solution is to use the unmasked key for the existence check, the same way this is handled in the "slow" dpif_flow_put() case. OVS relies on the fact that overlapping flows can exist if one is a superset of the other. Note that this is only true when the same set of actions is applied. This is due to how the revalidator process works. During revalidation, OVS removes too generic flows from the datapath to avoid incorrect matches but allows too narrow flows to stay in the datapath to avoid the data plane disruption and also to avoid constant flow deletions if the datapath ignores wildcards on certain fields/bits. See flow_wildcards_has_extra() check in the revalidate_ukey__() function. The problem here is that we have a too narrow flow installed, and now OpenFlow rules got changed, so the actual flow should be more generic. Revalidators will not remove the narrow flow, and we will eventually get an upcall on the packet that doesn't match the narrow flow, but we will not be able to install a more generic flow because after masking with the new wider mask, the key matches on the narrow flow, so we get EEXIST. Fixes: beb75a40fdc2 ("userspace: Switching of L3 packets in L2 pipeline") Signed-off-by: Eelco Chaudron --- v2: - Updated commit message and added fixes tag. lib/dpif-netdev.c | 33 +++++++++++++++++++++++++++++---- tests/dpif-netdev.at | 14 ++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index a45b46014..daa00aa2f 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3321,6 +3321,28 @@ netdev_flow_key_init_masked(struct netdev_flow_key *dst, (dst_u64 - miniflow_get_values(&dst->mf)) * 8); } +/* Initializes 'dst' as a copy of 'flow'. */ +static inline void +netdev_flow_key_init(struct netdev_flow_key *key, + const struct flow *flow) +{ + uint64_t *dst = miniflow_values(&key->mf); + uint32_t hash = 0; + uint64_t value; + + miniflow_map_init(&key->mf, flow); + miniflow_init(&key->mf, flow); + + size_t n = dst - miniflow_get_values(&key->mf); + + FLOW_FOR_EACH_IN_MAPS (value, flow, key->mf.map) { + hash = hash_add64(hash, value); + } + + key->hash = hash_finish(hash, n * 8); + key->len = netdev_flow_key_size(n); +} + static inline void emc_change_entry(struct emc_entry *ce, struct dp_netdev_flow *flow, const struct netdev_flow_key *key) @@ -4195,7 +4217,7 @@ static int dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put) { struct dp_netdev *dp = get_dp_netdev(dpif); - struct netdev_flow_key key, mask; + struct netdev_flow_key key; struct dp_netdev_pmd_thread *pmd; struct match match; ovs_u128 ufid; @@ -4244,9 +4266,12 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put) /* Must produce a netdev_flow_key for lookup. * Use the same method as employed to create the key when adding - * the flow to the dplcs to make sure they match. */ - netdev_flow_mask_init(&mask, &match); - netdev_flow_key_init_masked(&key, &match.flow, &mask); + * the flow to the dplcs to make sure they match. + * We need to put in the unmasked key as flow_put_on_pmd() will first try + * to see if an entry exists doing a packet type lookup. As masked-out + * fields are interpreted as zeros, they could falsely match a wider IP + * address mask. Installation of the flow will use the match variable. */ + netdev_flow_key_init(&key, &match.flow); if (put->pmd_id == PMD_ID_NULL) { if (cmap_count(&dp->poll_threads) == 0) { diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at index 3179e1645..32054c52e 100644 --- a/tests/dpif-netdev.at +++ b/tests/dpif-netdev.at @@ -636,6 +636,20 @@ OVS_VSWITCHD_STOP(["/flow: in_port is not an exact match/d /failed to put/d"]) AT_CLEANUP +AT_SETUP([dpif-netdev - check dpctl/add-flow wider ip match]) +OVS_VSWITCHD_START( + [add-port br0 p1 \ + -- set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p0.sock \ + -- set bridge br0 datapath-type=dummy]) + +AT_CHECK([ovs-appctl revalidator/pause]) +AT_CHECK([ovs-appctl dpctl/add-flow "in_port(1),eth_type(0x0800),ipv4(src=0.0.0.0/192.0.0.0,dst=0.0.0.0/192.0.0.0,frag=no)" "3"]) +AT_CHECK([ovs-appctl dpctl/add-flow "in_port(1),eth_type(0x0800),ipv4(src=192.1.1.1/0.0.0.0,dst=49.1.1.1/0.0.0.0,frag=no)" "3"]) +AT_CHECK([ovs-appctl revalidator/resume]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + # SEND_UDP_PKTS([p_name], [p_ofport]) # # Sends 128 packets to port 'p_name' with different UDP destination ports.