[PATCH net-next] calipso: hash the cache key, not the option start, in calipso_cache_add()

[PATCH net-next] calipso: hash the cache key, not the option start, in calipso_cache_add()

[Joas Antonio dos Santos]

calipso_cache_add() stores calipso_ptr + 2 as the cache key, but
computes entry->hash over calipso_ptr, i.e. starting two bytes earlier
at the option type and length.  calipso_opt_getattr() looks entries up
with calipso_cache_check(calipso + 2, calipso[1], ...), which hashes the
key itself.

The two hashes only match on a jhash collision, so the CALIPSO label
cache never hits and every labelled packet takes the full DOI lookup and
category conversion path.  The memcmp() on the key keeps lookups
correct, so this is a performance bug only.

Hash the stored key, as cipso_v4_cache_add() does for CIPSO.

Fixes: 4fee5242bf41 ("calipso: Add a label cache.")
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Claude:claude-opus-5-5
---
Found by code review.  Compile-tested only (arm64, W=1); not runtime-tested.

 net/ipv6/calipso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
index c072eca50..8c447d860 100644
--- a/net/ipv6/calipso.c
+++ b/net/ipv6/calipso.c
@@ -283,7 +283,7 @@ static int calipso_cache_add(const unsigned char *calipso_ptr,
 		goto cache_add_failure;
 	}
 	entry->key_len = calipso_ptr_len;
-	entry->hash = calipso_map_cache_hash(calipso_ptr, calipso_ptr_len);
+	entry->hash = calipso_map_cache_hash(entry->key, calipso_ptr_len);
 	refcount_inc(&secattr->cache->refcount);
 	entry->lsm_data = secattr->cache;
 

Re: [PATCH net-next] calipso: hash the cache key, not the option start, in calipso_cache_add()

[sashiko-bot]

> calipso_cache_add() stores calipso_ptr + 2 as the cache key, but
> computes entry->hash over calipso_ptr, i.e. starting two bytes earlier
> at the option type and length.  calipso_opt_getattr() looks entries up
> with calipso_cache_check(calipso + 2, calipso[1], ...), which hashes the
> key itself.
> 
> The two hashes only match on a jhash collision, so the CALIPSO label
> cache never hits and every labelled packet takes the full DOI lookup and
> category conversion path.  The memcmp() on the key keeps lookups
> correct, so this is a performance bug only.
> 
> Hash the stored key, as cipso_v4_cache_add() does for CIPSO.
> 
> Fixes: 4fee5242bf41 ("calipso: Add a label cache.")
> Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179138670045.80227.16155454353416856947@gmail.com?part=1

Re: [PATCH net-next] calipso: hash the cache key, not the option start, in calipso_cache_add()

[Daniel Machon]

> calipso_cache_add() stores calipso_ptr + 2 as the cache key, but
> computes entry->hash over calipso_ptr, i.e. starting two bytes earlier
> at the option type and length.  calipso_opt_getattr() looks entries up
> with calipso_cache_check(calipso + 2, calipso[1], ...), which hashes the
> key itself.
> 
> The two hashes only match on a jhash collision, so the CALIPSO label
> cache never hits and every labelled packet takes the full DOI lookup and
> category conversion path.  The memcmp() on the key keeps lookups
> correct, so this is a performance bug only.
> 
> Hash the stored key, as cipso_v4_cache_add() does for CIPSO.

FWIW:

I tested this with a small throwaway KUnit test: put 16 labels into the cache,
then look each one up again the same way the kernel does when a packet arrives.
Without the patch none of them were found (0/16), with it all of them were
(16/16).

Reviewed-by: Daniel Machon <daniel.machon@microchip.com>

Re: [PATCH net-next] calipso: hash the cache key, not the option start, in calipso_cache_add()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 07 Oct 2026 12:25:00 -0300 you wrote:
> calipso_cache_add() stores calipso_ptr + 2 as the cache key, but
> computes entry->hash over calipso_ptr, i.e. starting two bytes earlier
> at the option type and length.  calipso_opt_getattr() looks entries up
> with calipso_cache_check(calipso + 2, calipso[1], ...), which hashes the
> key itself.
> 
> The two hashes only match on a jhash collision, so the CALIPSO label
> cache never hits and every labelled packet takes the full DOI lookup and
> category conversion path.  The memcmp() on the key keeps lookups
> correct, so this is a performance bug only.
> 
> [...]

Here is the summary with links:
  - [net-next] calipso: hash the cache key, not the option start, in calipso_cache_add()
    https://git.kernel.org/netdev/net-next/c/d538cc3ec0bc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next] calipso: hash the cache key, not the option start, in calipso_cache_add()

[Paul Moore]

On Tue, Oct 13, 2026 at 6:10 AM <patchwork-bot+netdevbpf@kernel.org> wrote:
>
> Hello:
>
> This patch was applied to netdev/net-next.git (main)
> by Paolo Abeni <pabeni@redhat.com>:
>
> On Wed, 07 Oct 2026 12:25:00 -0300 you wrote:
> > calipso_cache_add() stores calipso_ptr + 2 as the cache key, but
> > computes entry->hash over calipso_ptr, i.e. starting two bytes earlier
> > at the option type and length.  calipso_opt_getattr() looks entries up
> > with calipso_cache_check(calipso + 2, calipso[1], ...), which hashes the
> > key itself.
> >
> > The two hashes only match on a jhash collision, so the CALIPSO label
> > cache never hits and every labelled packet takes the full DOI lookup and
> > category conversion path.  The memcmp() on the key keeps lookups
> > correct, so this is a performance bug only.
> >
> > [...]
>
> Here is the summary with links:
>   - [net-next] calipso: hash the cache key, not the option start, in calipso_cache_add()
>     https://git.kernel.org/netdev/net-next/c/d538cc3ec0bc
>
> You are awesome, thank you!

My apologies, travel last week for LPC and LSS-EU has me a bit behind,
but this patch looks good to me.  Thanks for identifying the problem
and providing a fix!

Likely too late, but for the sake of posterity ...

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com