Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo
@ 2020-02-14 17:14 Stefano Brivio
  2020-02-14 17:14 ` [PATCH nf-next 1/2] netfilter: nft_set_pipapo: Fix mapping table example in comments Stefano Brivio
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Stefano Brivio @ 2020-02-14 17:14 UTC (permalink / raw)
  To: Pablo Neira Ayuso, netfilter-devel

Patch 1/2 fixes examples of mapping table values in comments,
patch 2/2 drops an abuse of unlikely(), both reported by Pablo.

No functional changes are intended here. I'm not entirely sure
these should be for nf-next, but I guess so as they don't carry
any functional fix.

Stefano Brivio (2):
  netfilter: nft_set_pipapo: Fix mapping table example in comments
  netfilter: nft_set_pipapo: Don't abuse unlikely() in pipapo_refill()

 net/netfilter/nft_set_pipapo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.25.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH nf-next 1/2] netfilter: nft_set_pipapo: Fix mapping table example in comments
  2020-02-14 17:14 [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo Stefano Brivio
@ 2020-02-14 17:14 ` Stefano Brivio
  2020-02-14 17:14 ` [PATCH nf-next 2/2] netfilter: nft_set_pipapo: Don't abuse unlikely() in pipapo_refill() Stefano Brivio
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Stefano Brivio @ 2020-02-14 17:14 UTC (permalink / raw)
  To: Pablo Neira Ayuso, netfilter-devel

In both insertion and lookup examples, the two element pointers
of rule mapping tables were swapped. Fix that.

Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 net/netfilter/nft_set_pipapo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index f0cb1e13af50..579600b39f39 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -203,7 +203,7 @@
  * ::
  *
  *       rule indices in last field:    0    1
- *       map to elements:             0x42  0x66
+ *       map to elements:             0x66  0x42
  *
  *
  * Matching
@@ -298,7 +298,7 @@
  * ::
  *
  *       rule indices in last field:    0    1
- *       map to elements:             0x42  0x66
+ *       map to elements:             0x66  0x42
  *
  *      the matching element is at 0x42.
  *
-- 
2.25.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH nf-next 2/2] netfilter: nft_set_pipapo: Don't abuse unlikely() in pipapo_refill()
  2020-02-14 17:14 [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo Stefano Brivio
  2020-02-14 17:14 ` [PATCH nf-next 1/2] netfilter: nft_set_pipapo: Fix mapping table example in comments Stefano Brivio
@ 2020-02-14 17:14 ` Stefano Brivio
  2020-02-14 17:33 ` [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo Pablo Neira Ayuso
  2020-02-18 21:07 ` Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Stefano Brivio @ 2020-02-14 17:14 UTC (permalink / raw)
  To: Pablo Neira Ayuso, netfilter-devel

I originally used unlikely() in the if (match_only) clause, which
we hit on the mapping table for the last field in a set, to ensure
we avoid branching to the rest of for loop body, which is executed
more frequently.

However, Pablo reports, this is confusing as it gives the impression
that this is not a common case, and it's actually not the intended
usage of unlikely().

I couldn't observe any statistical difference in matching rates on
x864_64 and aarch64 without it, so just drop it.

Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 net/netfilter/nft_set_pipapo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index 579600b39f39..feac8553f6d9 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -503,7 +503,7 @@ static int pipapo_refill(unsigned long *map, int len, int rules,
 				return -1;
 			}
 
-			if (unlikely(match_only)) {
+			if (match_only) {
 				bitmap_clear(map, i, 1);
 				return i;
 			}
-- 
2.25.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo
  2020-02-14 17:14 [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo Stefano Brivio
  2020-02-14 17:14 ` [PATCH nf-next 1/2] netfilter: nft_set_pipapo: Fix mapping table example in comments Stefano Brivio
  2020-02-14 17:14 ` [PATCH nf-next 2/2] netfilter: nft_set_pipapo: Don't abuse unlikely() in pipapo_refill() Stefano Brivio
@ 2020-02-14 17:33 ` Pablo Neira Ayuso
  2020-02-18 21:07 ` Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2020-02-14 17:33 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: netfilter-devel

On Fri, Feb 14, 2020 at 06:14:12PM +0100, Stefano Brivio wrote:
> Patch 1/2 fixes examples of mapping table values in comments,
> patch 2/2 drops an abuse of unlikely(), both reported by Pablo.
> 
> No functional changes are intended here. I'm not entirely sure
> these should be for nf-next, but I guess so as they don't carry
> any functional fix.

These are small, they can be placed into nf.git and I think they
qualify as fixes.

Thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo
  2020-02-14 17:14 [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo Stefano Brivio
                   ` (2 preceding siblings ...)
  2020-02-14 17:33 ` [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo Pablo Neira Ayuso
@ 2020-02-18 21:07 ` Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2020-02-18 21:07 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: netfilter-devel

On Fri, Feb 14, 2020 at 06:14:12PM +0100, Stefano Brivio wrote:
> Patch 1/2 fixes examples of mapping table values in comments,
> patch 2/2 drops an abuse of unlikely(), both reported by Pablo.
> 
> No functional changes are intended here. I'm not entirely sure
> these should be for nf-next, but I guess so as they don't carry
> any functional fix.

Applied this fixes to nf.git. Thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 17:14 [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo Stefano Brivio
2020-02-14 17:14 ` [PATCH nf-next 1/2] netfilter: nft_set_pipapo: Fix mapping table example in comments Stefano Brivio
2020-02-14 17:14 ` [PATCH nf-next 2/2] netfilter: nft_set_pipapo: Don't abuse unlikely() in pipapo_refill() Stefano Brivio
2020-02-14 17:33 ` [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo Pablo Neira Ayuso
2020-02-18 21:07 ` Pablo Neira Ayuso

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git