[PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo

[PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo

[Stefano Brivio]

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

[PATCH nf-next 1/2] netfilter: nft_set_pipapo: Fix mapping table example in comments

[Stefano Brivio]

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

[PATCH nf-next 2/2] netfilter: nft_set_pipapo: Don't abuse unlikely() in pipapo_refill()

[Stefano Brivio]

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

Re: [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo

[Pablo Neira Ayuso]

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.

Re: [PATCH nf-next 0/2] Two non-functional fixes for nft_set_pipapo

[Pablo Neira Ayuso]

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.