[Swan-dev] suspicious code in linux/net/ipsec/ipsec_ocf.c

[David McCullough]

D. Hugh Redelmeier wrote the following:
>         crp->crp_flags =
>                 CRYPTO_F_SKBUF |
>                 (ipsec_ocf_cbimm ? CRYPTO_F_BATCH : 0) |
>                 (ipsec_ocf_batch ? CRYPTO_F_BATCH : 0) |
>                 0;
> 
> First of all, "| 0;" is pointless.

Its a left over from when the code used ifdef's,  makes sense if you see
the history I guess.

> Second of all, along with CRYPTO_F_BATCH, cryptodev.h defines 
> CRYPTO_F_CBIMM.
> 
> So: should the code be:
> 
>         crp->crp_flags =
>                 CRYPTO_F_SKBUF |
>                 (ipsec_ocf_cbimm ? CRYPTO_F_CBIMM : 0) |
>                 (ipsec_ocf_batch ? CRYPTO_F_BATCH : 0);

Yes,  the code should be as you have shown above.  They are seperate flags
that can be set independently.

My only concern is that this typo has been there since Dec 2010 and
effectively means that no one has been using CBIMM.  So I wonder if we
should default ipsec_ocf_cbimm to 0 instead of 1.

The default for this was previously 1 and the overall operation has not
changed,  so I am ok to leave it at 1,  just keep it in mind I guess in
case we start seeing reports of OCF issues on libreswan :-)

Cheers,
Davidm

> or should it be
> 
>         crp->crp_flags =
>                 CRYPTO_F_SKBUF |
>                 (ipsec_ocf_cbimm | ipsec_ocf_batch ? CRYPTO_F_BATCH : 0);

-- 
David McCullough,  davidm at spottygum.com,   Ph: 0410 560 763