From patchwork Thu Feb 29 17:31:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Hemminger X-Patchwork-Id: 137513 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 20A4543C3F; Thu, 29 Feb 2024 18:32:25 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5E9CE42F91; Thu, 29 Feb 2024 18:32:10 +0100 (CET) Received: from mail-pg1-f174.google.com (mail-pg1-f174.google.com [209.85.215.174]) by mails.dpdk.org (Postfix) with ESMTP id C711542F85 for ; Thu, 29 Feb 2024 18:32:08 +0100 (CET) Received: by mail-pg1-f174.google.com with SMTP id 41be03b00d2f7-5d8b887bb0cso962794a12.2 for ; Thu, 29 Feb 2024 09:32:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1709227928; x=1709832728; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=+AeF2SCiSz//fc0HRFr/2OIbFohjhgN6psQREKMfvKU=; b=de/EO9/YC6N0oSHTJ9QEtsMErV4tPQrdZqg/XyFYJIYY/AO7AlRNlSTu5dkO2ewn/r AiNBZznAFzT9EMEAC+8/GRlzALMtAIkuZ9jXnnjk3fUr6Keyre61djswbIwopB/HbL+q RYqKrC2v4kcxySJV4AzOlKJM+X79/yxdZ2QZvQSyrrQJKIjaDzWjy20CYe2L3RuwOAK4 FAKL6xjPBPjUU/3gIN+zs/os28w+HN1iVvryJS2rWg7dC2avd3MsqB/gcVWwQVUYNmxq ZsLxliFb4lQsmv7FS8tZb0zmhsnOQBLzQtdm8s37wMJ3Id60bxnXpz4T2L7x22akEV11 lTiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709227928; x=1709832728; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+AeF2SCiSz//fc0HRFr/2OIbFohjhgN6psQREKMfvKU=; b=P0iSLkjVAc72pjjiRz5Pmy8g4+BrH4I3MMkUXKzRCOdW0HgDRa7eksClEEMLiki48T KL3IEUSx43WZtELNFJKR5UOb5qFTJkGZnWByylaFyoXGxhp23fsLkHhuaefsNci9jhJc kjBLbfLntfCLTra/ubowz8AmPRTKagm8bdtXEXoGG53lLZiSWpoE2axHQ1md2zhFVTDJ Tt1BnlUaNSE85GJkF54uIJqy2C1I5/aPG26wPnh0fs2u3naibc398iqJ4wQ/Y3f6Qtq9 Xmo3E70s985hTzwbmSvLLSHevAxLAzGg014ipgy/9NhT4SNfFTqEl06oDdPBugWMEbVN +JFg== X-Gm-Message-State: AOJu0YxZv3wnTgcNZnAfoeHBhh9/a6Xl0DCSDegeTC4aQZrIyLKkn1Wa UcVShMGUdUeBUFP51KtxpAB2ztFvIfKDChAscoaQLA/Ih3+VZye5lLOxoAIXjknUm+7/6ef0vWn 0 X-Google-Smtp-Source: AGHT+IHTEbBrLI6NPYPA+Oj+PVTXSjwOfpK43mw+8LWX4o+5DOigr9dpq9ILfE6G8nXHTlJWpmfAFQ== X-Received: by 2002:a05:6a00:3d43:b0:6e5:3abd:33bd with SMTP id lp3-20020a056a003d4300b006e53abd33bdmr3616230pfb.34.1709227927900; Thu, 29 Feb 2024 09:32:07 -0800 (PST) Received: from hermes.local (204-195-123-141.wavecable.com. [204.195.123.141]) by smtp.gmail.com with ESMTPSA id r31-20020a63fc5f000000b005dc3fc53f19sm1584846pgk.7.2024.02.29.09.32.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Feb 2024 09:32:07 -0800 (PST) From: Stephen Hemminger To: dev@dpdk.org Cc: Stephen Hemminger , Olga Shern , Keith Wiles , Pascal Mazon , Kevin Traynor , Ferruh Yigit Subject: [PATCH 3/3] net/tap: compute TC handle correctly Date: Thu, 29 Feb 2024 09:31:08 -0800 Message-ID: <20240229173154.121491-4-stephen@networkplumber.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240229173154.121491-1-stephen@networkplumber.org> References: <20240229173154.121491-1-stephen@networkplumber.org> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The code to take a flow pointer and make a TC handle was incorrect and would always generate the same handle. This is because it was hashing the address of the union on the stack (which is invariant) rather than the contents of the union. The following testpmd case would cause an error: testpmd> flow create 0 ingress pattern eth src is 06:05:04:03:02:01 \ / end actions queue index 2 / end Flow rule #0 created testpmd> flow create 0 ingress pattern eth src is 06:05:04:03:02:02 \ / end actions queue index 3 / end tap_nl_dump_ext_ack(): Filter already exists tap_flow_create(): Kernel refused TC filter rule creation (17): File exists port_flow_complain(): Caught PMD error type 2 (flow rule (handle)): overlapping rules or Kernel too old for flower support: File exists This fix does it in a more robust manner using size independent code. It also initializes the hash seed so the same hash won't show up every time and risk potential leakage of address to other places. Bugzilla ID: 1382 Fixes: de96fe68ae95 ("net/tap: add basic flow API patterns and actions") Fixes: a625ab89df11 ("net/tap: fix build with GCC 11") Signed-off-by: Stephen Hemminger --- drivers/net/tap/tap_flow.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c index 5b0fee906493..94f9b99cdaa6 100644 --- a/drivers/net/tap/tap_flow.c +++ b/drivers/net/tap/tap_flow.c @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -1297,9 +1298,7 @@ tap_flow_validate(struct rte_eth_dev *dev, * In those rules, the handle (uint32_t) is the part that would identify * specifically each rule. * - * On 32-bit architectures, the handle can simply be the flow's pointer address. - * On 64-bit architectures, we rely on jhash(flow) to find a (sufficiently) - * unique handle. + * Use jhash of the flow poitner to make a unique handle. * * @param[in, out] flow * The flow that needs its handle set. @@ -1309,16 +1308,18 @@ tap_flow_set_handle(struct rte_flow *flow) { union { struct rte_flow *flow; - const void *key; - } tmp; - uint32_t handle = 0; + uint32_t words[sizeof(flow) / sizeof(uint32_t)]; + } tmp = { + .flow = flow, + }; + uint32_t handle; + static uint64_t hash_seed; - tmp.flow = flow; + if (hash_seed == 0) + hash_seed = rte_rand(); + + handle = rte_jhash_32b(tmp.words, sizeof(flow) / sizeof(uint32_t), hash_seed); - if (sizeof(flow) > 4) - handle = rte_jhash(tmp.key, sizeof(flow), 1); - else - handle = (uintptr_t)flow; /* must be at least 1 to avoid letting the kernel choose one for us */ if (!handle) handle = 1;