Hi, all:
I encountered a IPsec packets reordering issue, following is the setup
and scenario
There is a IPSec IKEv1 tunnel between B & C
The traffic is UDP from C to A @ 40 mbps
Packets are coming in order at B but leaving out of order towards A
If IPSec is disabled between B & C, there is no packet reordering.
The input and output of B is same physical interface but separated by
two VLANs, and we have directed all our network interrupts to one core.
As per our analysis we are suspecting below is the root cause of the
problem.
All the packets which are out of order have got -EINPROGRESS error in
below part of the code.
File: net/ipv4/esp4.c: function esp_input
.....
aead_request_set_callback(req, 0, esp_input_done, skb);
aead_request_set_crypt(req, sg, sg, elen, iv);
aead_request_set_assoc(req, asg, sizeof(*esph));
err = crypto_aead_decrypt(req);
if (err == -EINPROGRESS)
goto out;
err = esp_input_done2(skb, err);
.....
Below is the place where the packets are either decrypted immediately or
queue for later decryption.
static int ablk_decrypt(struct ablkcipher_request *req)
{
struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
struct async_aes_ctx *ctx = crypto_ablkcipher_ctx(tfm);
if (!irq_fpu_usable()) {
struct ablkcipher_request *cryptd_req =
ablkcipher_request_ctx(req);
memcpy(cryptd_req, req, sizeof(*req));
ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
return crypto_ablkcipher_decrypt(cryptd_req);
} else {
struct blkcipher_desc desc;
desc.tfm = cryptd_ablkcipher_child(ctx->cryptd_tfm);
desc.info = req->info;
desc.flags = 0;
return crypto_blkcipher_crt(desc.tfm)->decrypt(
&desc, req->dst, req->src, req->nbytes);
}
}
Now the problem scenario is, if a packet came for decryption and
"irq_fpu_usable()" is not usable, then it will be queued for later
decryption and crypto_aead_decrypt will be reuturned with "-EINPROGRESS"
error. When the packets are in queue, if some more packets came and
"irq_fpu_usable()" is usable then those packets will be decrypted before
the queued packets and queued packets will get out of order.
And we've figure out a patch as the attached, the basic idea is just
queue the packets if "irq_fpu_usable()" is not usable or if there are
already few packets queued for decryption. Else decrypt the packets.
Could anybody tell if this is a appropriate fix? Or is this reordering
thing a real probelm? 'cause I know the IPsec doesn't guarantee order at
all. Appreciate it very much!
the best,
thank you
On Thu, Jul 31, 2014 at 11:07:26AM +0800, Ming Liu wrote:
>
> And we've figure out a patch as the attached, the basic idea is just
> queue the packets if "irq_fpu_usable()" is not usable or if there
> are already few packets queued for decryption. Else decrypt the
> packets.
Yes your description makes sense and I will review your patch
for inclusion.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 07/31/2014 02:23 PM, Herbert Xu wrote:
> On Thu, Jul 31, 2014 at 11:07:26AM +0800, Ming Liu wrote:
>> And we've figure out a patch as the attached, the basic idea is just
>> queue the packets if "irq_fpu_usable()" is not usable or if there
>> are already few packets queued for decryption. Else decrypt the
>> packets.
> Yes your description makes sense and I will review your patch
> for inclusion.
Thanks, Herbert!
//Ming Liu
>
> Thanks,
On 07/31/2014 02:23 PM, Herbert Xu wrote:
> On Thu, Jul 31, 2014 at 11:07:26AM +0800, Ming Liu wrote:
>> And we've figure out a patch as the attached, the basic idea is just
>> queue the packets if "irq_fpu_usable()" is not usable or if there
>> are already few packets queued for decryption. Else decrypt the
>> packets.
> Yes your description makes sense and I will review your patch
> for inclusion.
Hi, Herbert:
Please review this attached patch instead, the original one has a
problem causing the kernel crash.
the best,
thank you
>
> Thanks,
On Sun, Aug 03, 2014 at 05:57:06PM +0800, Ming Liu wrote:
>
> Please review this attached patch instead, the original one has a
> problem causing the kernel crash.
Thanks for the patch. I think it would better to enforce ordering
for all operations rather than treat encryptions separately from
decryptions. We could conceivably have more complex operations made
up from both encryptions and decryptions that could then get
out-of-order.
It would also make the code simpler.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Hi, Herbert:
I've figured out a new patch for this issue reported by me previously,
the basic idea is adding a cryptd_flush_queue function fixing it by
being called from softirq to flush all previous queued elements before
processing a new one, and it works very well so far per my test, would
you please review it?
the best,
thank you
On 08/14/2014 04:47 PM, Herbert Xu wrote:
> On Sun, Aug 03, 2014 at 05:57:06PM +0800, Ming Liu wrote:
>> Please review this attached patch instead, the original one has a
>> problem causing the kernel crash.
> Thanks for the patch. I think it would better to enforce ordering
> for all operations rather than treat encryptions separately from
> decryptions. We could conceivably have more complex operations made
> up from both encryptions and decryptions that could then get
> out-of-order.
>
> It would also make the code simpler.
>
> Cheers,