On 23-Jul-24 5:57 PM, Akhil Goyal wrote:
Hi all,

This patch breaks ipsec tests with ipsec-secgw:


./examples/ipsec-secgw/test/run_test.sh -4 trs_aesctr_sha1
...
ERROR: ./examples/ipsec-secgw/test/linux_test.sh failed for dst=192.168.31.14,
sz=1
  test IPv4 trs_aesctr_sha1 finished with status 1
ERROR  test trs_aesctr_sha1 FAILED

The patch seems to be correct.
Please check endianness in the PMD you are testing.

In my opinion salt should not be affected by endianness and it should be used as it is in the key parameter. I think the patch is wrong to make it CPU endianness dependent before being passed to the PMDs, any PMD that needs the endianness swapped should do it in the PMD code. Indeed it's passed around as a 32 bit integer but it's not used as such, and when it's actually used it should be evaluated as a byte array.

https://datatracker.ietf.org/doc/html/rfc4106#section-4
https://datatracker.ietf.org/doc/html/rfc4106#section-8.1





On 03/07/2024 18:58, Akhil Goyal wrote:





                -----Original Message-----
                From: Akhil Goyal <gak...@marvell.com>
<mailto:gak...@marvell.com>
                Sent: Friday, March 15, 2024 12:42 AM
                To: Akhil Goyal <gak...@marvell.com>
<mailto:gak...@marvell.com> ; Chaoyong He
                <chaoyong...@corigine.com>
<mailto:chaoyong...@corigine.com> ; dev@dpdk.org <mailto:dev@dpdk.org>
                Cc: oss-driv...@corigine.com <mailto:oss-
driv...@corigine.com> ; Shihong Wang <shihong.w...@corigine.com>
<mailto:shihong.w...@corigine.com> ;
                sta...@dpdk.org <mailto:sta...@dpdk.org>
                Subject: RE: [EXTERNAL] [PATCH v2] examples/ipsec-secgw: fix
SA salt
                endianness problem


                        Subject: RE: [EXTERNAL] [PATCH v2] examples/ipsec-
secgw: fix SA salt
                        endianness problem


                                From: Shihong Wang
<shihong.w...@corigine.com> <mailto:shihong.w...@corigine.com>

                                The SA salt of struct ipsec_sa is a CPU-endian
u32 variable, but it’s
                                value is stored in an array of encryption or
authentication keys
                                according to big-endian. So it maybe need to
convert the endianness
                                order to ensure that the value assigned to the
SA salt is CPU-endian.

                                Fixes: 50d75cae2a2c ("examples/ipsec-secgw:
initialize SA salt")
                                Fixes: 9413c3901f31 ("examples/ipsec-secgw:
support additional algorithms")
                                Fixes: 501e9c226adf ("examples/ipsec-secgw:
add AEAD parameters")
                                Cc: sta...@dpdk.org <mailto:sta...@dpdk.org>

                                Signed-off-by: Shihong Wang
<shihong.w...@corigine.com> <mailto:shihong.w...@corigine.com>
                                Reviewed-by: Chaoyong He
<chaoyong...@corigine.com> <mailto:chaoyong...@corigine.com>


                        Acked-by: Akhil Goyal <gak...@marvell.com>
<mailto:gak...@marvell.com>

                        Applied to dpdk-next-crypto


                The patch is pulled back from dpdk-next-crypto.
                This change may cause all the PMDs to fail these cases.
                Would need acks from PMDs.


        Applied to dpdk-next-crypto
        No update from PMD owners.
        Applying it before RC2 so that we have time for fixes if needed.


--
Regards,
Vladimir

Reply via email to