Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/1.1/patches/2222407/?format=api
{ "id": 2222407, "url": "http://patchwork.ozlabs.org/api/1.1/patches/2222407/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netfilter-devel/patch/20260411121303.1555333-1-fw@strlen.de/", "project": { "id": 26, "url": "http://patchwork.ozlabs.org/api/1.1/projects/26/?format=api", "name": "Netfilter Development", "link_name": "netfilter-devel", "list_id": "netfilter-devel.vger.kernel.org", "list_email": "netfilter-devel@vger.kernel.org", "web_url": null, "scm_url": null, "webscm_url": null }, "msgid": "<20260411121303.1555333-1-fw@strlen.de>", "date": "2026-04-11T12:12:59", "name": "[v2,nf-next] netfilter: nft_set_pipapo_avx2: restore performance optimization", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "b53b40f4ce5b30d28d09de2278daf90f2b49b3bd", "submitter": { "id": 1025, "url": "http://patchwork.ozlabs.org/api/1.1/people/1025/?format=api", "name": "Florian Westphal", "email": "fw@strlen.de" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/netfilter-devel/patch/20260411121303.1555333-1-fw@strlen.de/mbox/", "series": [ { "id": 499553, "url": "http://patchwork.ozlabs.org/api/1.1/series/499553/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netfilter-devel/list/?series=499553", "date": "2026-04-11T12:12:59", "name": "[v2,nf-next] netfilter: nft_set_pipapo_avx2: restore performance optimization", "version": 2, "mbox": "http://patchwork.ozlabs.org/series/499553/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/2222407/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/2222407/checks/", "tags": {}, "headers": { "Return-Path": "\n <netfilter-devel+bounces-11821-incoming=patchwork.ozlabs.org@vger.kernel.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "netfilter-devel@vger.kernel.org" ], "Delivered-To": "patchwork-incoming@legolas.ozlabs.org", "Authentication-Results": [ "legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.105.105.114; helo=tor.lore.kernel.org;\n envelope-from=netfilter-devel+bounces-11821-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)", "smtp.subspace.kernel.org;\n arc=none smtp.client-ip=91.216.245.30", "smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de", "smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=Chamillionaire.breakpoint.cc" ], "Received": [ "from tor.lore.kernel.org (tor.lore.kernel.org [172.105.105.114])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4ftCQp1tQNz1yCx\n\tfor <incoming@patchwork.ozlabs.org>; Sat, 11 Apr 2026 22:19:30 +1000 (AEST)", "from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 49992307B37C\n\tfor <incoming@patchwork.ozlabs.org>; Sat, 11 Apr 2026 12:13:17 +0000 (UTC)", "from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id DE232344036;\n\tSat, 11 Apr 2026 12:13:15 +0000 (UTC)", "from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc\n [91.216.245.30])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id E0255248886\n\tfor <netfilter-devel@vger.kernel.org>; Sat, 11 Apr 2026 12:13:13 +0000 (UTC)", "by Chamillionaire.breakpoint.cc (Postfix, from userid 1003)\n\tid CC05660425; Sat, 11 Apr 2026 14:13:11 +0200 (CEST)" ], "ARC-Seal": "i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1775909595; cv=none;\n b=b7U8GcYML4tw26JaNcBQB3trqjT2ftoPQhpoLuri4GPCxirOQZAaI6M4hZQEZimuiiSF4iOrWX7WJ9CqVzwhuvBPX9M8JnS26F25NDdq0Vsu/aWyqXhrnq236aoutwT1uoLektab9JOVGW0WM/JoN3WXuRa8EZReAowgMG/ixkk=", "ARC-Message-Signature": "i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1775909595; c=relaxed/simple;\n\tbh=/nh8vxODz1ciwUVzvy/jAVvX6QDSoz9cf09h+olahy8=;\n\th=From:To:Cc:Subject:Date:Message-ID:MIME-Version;\n b=D4Do2q9fEyR+1fYVLI9O8lGEjw+9NNcsq5FQIG0bAOCl8MHvph1OevsdAfSiX++SFpXlfQ//BJjes6QQenOpSDIZAcNBZ0NcHi7xAJgmuglPHfaKnmcl6WtIVdyuIszSb6WtagYzB1FgUWO0Z1840Lckgi5FUsVLcYjADATQm+A=", "ARC-Authentication-Results": "i=1; smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de;\n spf=pass smtp.mailfrom=Chamillionaire.breakpoint.cc;\n arc=none smtp.client-ip=91.216.245.30", "From": "Florian Westphal <fw@strlen.de>", "To": "<netfilter-devel@vger.kernel.org>", "Cc": "Florian Westphal <fw@strlen.de>,\n\tStefano Brivio <sbrivio@redhat.com>", "Subject": "[PATCH v2 nf-next] netfilter: nft_set_pipapo_avx2: restore\n performance optimization", "Date": "Sat, 11 Apr 2026 14:12:59 +0200", "Message-ID": "<20260411121303.1555333-1-fw@strlen.de>", "X-Mailer": "git-send-email 2.53.0", "Precedence": "bulk", "X-Mailing-List": "netfilter-devel@vger.kernel.org", "List-Id": "<netfilter-devel.vger.kernel.org>", "List-Subscribe": "<mailto:netfilter-devel+subscribe@vger.kernel.org>", "List-Unsubscribe": "<mailto:netfilter-devel+unsubscribe@vger.kernel.org>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit" }, "content": "The avx2 lookup routines get the next map index to process passes as a\nfunction argument, but this isn't obvious because it's hidden in the\nlookup macro.\n\nAdditionally, a recent LLM review pointed out following \"bug\":\n -------------------------------------------------------------\n > b = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n > if (last)\n > - return b;\n > + ret = b;\n >\n > if (unlikely(ret == -1))\n > ret = b / XSAVE_YMM_SIZE;\n\n Does this change introduce a logic error when last=true and no match is\n found? [..]\n\n Should this be changed to an else-if structure instead?\n -------------------------------------------------------------\n\nLLM sees a control-flow change, but there is none:\n\nAll call sites invoke nft_pipapo_avx2_refill() only when at least one\nbit in the map is set, i.e. nft_pipapo_avx2_refill() never returns -1.\n\nAdd a runtime debug check that fires if we'd return -1 as additional\ndocumentation and also make the suggested change, code might be easier\nto understand this way.\n\nIn commit 17a20e09f086 (\"netfilter: nft_set: remove one argument from\nlookup and update functions\") I incorrectly moved the \"ret\" scope into\nthe loop.\n\nThis has no effect on the correctness, but it can (depending on map sizes)\ncause a redundant repeat of an earlier processing step.\n\nRestore the intended 'pass map index' instead of always-0. Note that I\ndid not see any change in performance numbers, but Stefano correctly\npoints out that the existing perf test likely lack a sparse intermediate\nbitmap (between fields) with a lot of leading zeroes.\n\nReviewed-by: Stefano Brivio <sbrivio@redhat.com>\nSigned-off-by: Florian Westphal <fw@strlen.de>\n---\n v2: alter commit message, patch is unchanged.\n\n net/netfilter/nft_set_pipapo_avx2.c | 35 ++++++++++++-----------------\n 1 file changed, 14 insertions(+), 21 deletions(-)", "diff": "diff --git a/net/netfilter/nft_set_pipapo_avx2.c b/net/netfilter/nft_set_pipapo_avx2.c\nindex dad265807b8b..b3f105520a85 100644\n--- a/net/netfilter/nft_set_pipapo_avx2.c\n+++ b/net/netfilter/nft_set_pipapo_avx2.c\n@@ -144,6 +144,7 @@ static void nft_pipapo_avx2_fill(unsigned long *data, int start, int len)\n * This is an alternative implementation of pipapo_refill() suitable for usage\n * with AVX2 lookup routines: we know there are four words to be scanned, at\n * a given offset inside the map, for each matching iteration.\n+ * The caller must ensure at least one bit in the four words is set.\n *\n * This function doesn't actually use any AVX2 instruction.\n *\n@@ -179,6 +180,7 @@ static int nft_pipapo_avx2_refill(int offset, unsigned long *map,\n \tNFT_PIPAPO_AVX2_REFILL_ONE_WORD(3);\n #undef NFT_PIPAPO_AVX2_REFILL_ONE_WORD\n \n+\tDEBUG_NET_WARN_ON_ONCE(ret < 0);\n \treturn ret;\n }\n \n@@ -243,8 +245,7 @@ static int nft_pipapo_avx2_lookup_4b_2(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -320,8 +321,7 @@ static int nft_pipapo_avx2_lookup_4b_4(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -415,8 +415,7 @@ static int nft_pipapo_avx2_lookup_4b_8(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -506,8 +505,7 @@ static int nft_pipapo_avx2_lookup_4b_12(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -642,8 +640,7 @@ static int nft_pipapo_avx2_lookup_4b_32(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -700,8 +697,7 @@ static int nft_pipapo_avx2_lookup_8b_1(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -765,8 +761,7 @@ static int nft_pipapo_avx2_lookup_8b_2(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -840,8 +835,7 @@ static int nft_pipapo_avx2_lookup_8b_4(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -926,8 +920,7 @@ static int nft_pipapo_avx2_lookup_8b_6(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -1020,8 +1013,7 @@ static int nft_pipapo_avx2_lookup_8b_16(unsigned long *map, unsigned long *fill,\n \t\tb = nft_pipapo_avx2_refill(i_ul, &map[i_ul], fill, f->mt, last);\n \t\tif (last)\n \t\t\tret = b;\n-\n-\t\tif (unlikely(ret == -1))\n+\t\telse if (unlikely(ret == -1))\n \t\t\tret = b / XSAVE_YMM_SIZE;\n \n \t\tcontinue;\n@@ -1143,6 +1135,7 @@ struct nft_pipapo_elem *pipapo_get_avx2(const struct nft_pipapo_match *m,\n \tconst struct nft_pipapo_field *f;\n \tunsigned long *res, *fill, *map;\n \tbool map_index;\n+\tint ret = 0;\n \tint i;\n \n \tscratch = *raw_cpu_ptr(m->scratch);\n@@ -1167,8 +1160,8 @@ struct nft_pipapo_elem *pipapo_get_avx2(const struct nft_pipapo_match *m,\n \n \tnft_pipapo_for_each_field(f, i, m) {\n \t\tbool last = i == m->field_count - 1, first = !i;\n-\t\tint ret = 0;\n \n+\t\t/* NB: previous round @ret is passed to avx2 lookup fn */\n #define NFT_SET_PIPAPO_AVX2_LOOKUP(b, n)\t\t\t\t\\\n \t\t(ret = nft_pipapo_avx2_lookup_##b##b_##n(res, fill, f,\t\\\n \t\t\t\t\t\t\t ret, data,\t\\\n", "prefixes": [ "v2", "nf-next" ] }