Dear netdev developers,

The call to xfrm_find_acq_byseq() by the pfkey_getspi() and 
xfrm_alloc_userspi() functions is quite costly and proves to entail 
scalability issues when performing thousands of IKE negotiations with 
racoon (from ipsec-tools distribution) or charon (from strongswan 
distribution).

Removing this call in the kernel drastically accelerates the processing 
and does not seem to entail functional problems.

For now, I don't see the point of this call. I need to understand its 
purpose, because I'm highly tempted to simply remove it.

Regards,
--

[ message continues ]

From: Christophe Gouault <christophe.gouault@6wind.com>

First of all, removing a function because you don't understand
why it's there is rarely a good idea :-)

I think the semantics require that we check for existing ACQUIRE
state entries before we allocate an SPI.

The likelyhood of breaking something if you remove the call is very
high.

--

[ message continues ]

Hi David,

First of all, thank you for your answer.

Yes, don't worry, I never act that way. I was deliberately a little 
Well, I still don't see in which case this can happen. The only 
situations in which an ACQUIRE state entry is created is when an acquire 
message is raised by the kernel (this creates a temporary outbound 
state) or when a getspi is issued from the userland (this creates a 
temporary state, as far as I know, this is the future inbound state 
negotiated by IKE).

In my humble opinion, issuing a getspi for the output state does not 
make sense since the SPI is chosen by the destination host.
So the possibly existing ACQUIRE state entry would be the result of a 
former getspi with the same value of the seq field. So this call would 
aim at cancelling a former call to getspi, maybe to cope with an 
application that would retry a failed negotiation before the former 
getspi expired?... I'm not really convinced by this explanation.

But there are maybe other uses of the getspi call than assigning a SPI 
Probably. I'm still interested in a concrete example ;-)

Christophe
--

[ message continues ]

Correction: it does not cancel the former call but returns the existing 
ACQUIRE state entry.

By the way, is it possible that the first call (to xfrm_find_acq_byseq) 
--

[ message continues ]

From: Christophe Gouault <christophe.gouault@6wind.com>

Christophe, digging deep into the tree history I found a
commit that should answer your question:

commit 4d9f62e953567482223b7110c5e8961c4d494c1f
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Fri Sep 10 00:36:11 2004 -0700

    [IPSEC]: Find larval SAs by sequence number
    
    When larval states are generated along with ACQUIRE messages, we should
    use the sequence to find the corresponding larval state when creating
    states with ADD_SA or ALLOC_SPI.
    
    If we don't do that, then it may take down an unrelated larval state
    with the same parameters (think different TCP sessions).  This not only
    leaves behind a larval state that shouldn't be there, it may also cause
    another ACQUIRE message to be sent unnecessarily.
    
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 40c65db..b5c9b10 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -896,4 +896,17 @@ typedef void (icv_update_fn_t)(struct crypto_tfm *, struct scatterlist *, unsign
 extern void skb_icv_walk(const struct sk_buff *skb, struct crypto_tfm *tfm,
 			 int offset, int len, icv_update_fn_t icv_update);
 
+static inline int xfrm_addr_cmp(xfrm_address_t *a, xfrm_address_t *b,
+				int family)
+{
+	switch (family) {
+	default:
+	case AF_INET:
+		return a->a4 - b->a4;
+	case AF_INET6:
+		return ipv6_addr_cmp((struct in6_addr *)a,
+				     (struct in6_addr *)b);
+	}
+}
+
 #endif	/* _NET_XFRM_H */
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 8ca25fd..b059fa2 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1156,7 +1156,16 @@ static int pfkey_getspi(struct sock *sk, struct sk_buff *skb, struct sadb_msg *h
 		break;
 #endif
 	}
-	if (xdaddr)
+
+	if (hdr->sadb_msg_seq) {
+		x = xfrm_find_acq_byseq(hdr->sadb_msg_seq);
+		if (x && xfrm_addr_cmp(&x->id.daddr, ...
[ message continues ]