Re: [dpdk-dev] [PATCH v3 6/8] cryptodev: rework session framework

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Akhil,


> As per current design, rte_cryptodev_sym_session_create() and
> rte_cryptodev_sym_session_init() use separate mempool objects
> for a single session.
> And structure rte_cryptodev_sym_session is not directly used
> by the application, it may cause ABI breakage if the structure
> is modified in future.
> 
> To address these two issues, the rte_cryptodev_sym_session_create
> will take one mempool object for both the session and session
> private data. The API rte_cryptodev_sym_session_init will now not
> take mempool object.
> rte_cryptodev_sym_session_create will now return an opaque session
> pointer which will be used by the app in rte_cryptodev_sym_session_init
> and other APIs.
> 
> With this change, rte_cryptodev_sym_session_init will send
> pointer to session private data of corresponding driver to the PMD
> based on the driver_id for filling the PMD data.
> 
> In data path, opaque session pointer is attached to rte_crypto_op
> and the PMD can call an internal library API to get the session
> private data pointer based on the driver id.
> 
> Note: currently nb_drivers are getting updated in RTE_INIT which
> result in increasing the memory requirements for session.
> User can compile off drivers which are not in use to reduce the
> memory consumption of a session.
> 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> ---

With that patch ipsec-secgw functional tests crashes for AES_GCM test-cases.
To be more specific:
examples/ipsec-secgw/test/run_test.sh -4 tun_aesgcm

[24126592.561071] traps: dpdk-ipsec-secg[3254860] general protection fault ip:7f3ac2397027 sp:7ffeaade8848 error:0 in libIPSec_MB.so.1.0.0[7f3ac238f000+2a20000]

Looking a bit deeper, it fails at:
#0  0x00007ff9274f4027 in aes_keyexp_128_enc_avx512 ()
   from /lib/libIPSec_MB.so.1
#1  0x00007ff929f0ac97 in aes_gcm_pre_128_avx_gen4 ()
   from /lib/libIPSec_MB.so.1
#2  0x0000561757073753 in aesni_gcm_session_configure (mb_mgr=0x56175c5fe400,
    session=0x17e3b72d8, xform=0x17e05d7c0)
    at ../drivers/crypto/ipsec_mb/pmd_aesni_gcm.c:132
#3  0x00005617570592af in ipsec_mb_sym_session_configure (
    dev=0x56175be0c940 <rte_crypto_devices>, xform=0x17e05d7c0,
    sess=0x17e3b72d8) at ../drivers/crypto/ipsec_mb/ipsec_mb_ops.c:330
#4  0x0000561753b4d6ae in rte_cryptodev_sym_session_init (dev_id=0 '\000',
    sess_opaque=0x17e3b4940, xforms=0x17e05d7c0)
    at ../lib/cryptodev/rte_cryptodev.c:1736
#5  0x0000561752ef99b7 in create_lookaside_session (
    ipsec_ctx=0x56175aa6a210 <lcore_conf+1105232>, sa=0x17e05d140,
    ips=0x17e05d140) at ../examples/ipsec-secgw/ipsec.c:145
#6  0x0000561752f0cf98 in fill_ipsec_session (ss=0x17e05d140,
    ctx=0x56175aa6a210 <lcore_conf+1105232>, sa=0x17e05d140)
    at ../examples/ipsec-secgw/ipsec_process.c:89
#7  0x0000561752f0d7dd in ipsec_process (
    ctx=0x56175aa6a210 <lcore_conf+1105232>, trf=0x7ffd192326a0)
    at ../examples/ipsec-secgw/ipsec_process.c:300
#8  0x0000561752f21027 in process_pkts_outbound (
--Type <RET> for more, q to quit, c to continue without paging--
    ipsec_ctx=0x56175aa6a210 <lcore_conf+1105232>, traffic=0x7ffd192326a0)
    at ../examples/ipsec-secgw/ipsec-secgw.c:839
#9  0x0000561752f21b2e in process_pkts (
    qconf=0x56175aa57340 <lcore_conf+1027712>, pkts=0x7ffd19233c20,
    nb_pkts=1 '\001', portid=1) at ../examples/ipsec-secgw/ipsec-secgw.c:1072
#10 0x0000561752f224db in ipsec_poll_mode_worker ()
    at ../examples/ipsec-secgw/ipsec-secgw.c:1262
#11 0x0000561752f38adc in ipsec_launch_one_lcore (args=0x56175c549700)
    at ../examples/ipsec-secgw/ipsec_worker.c:654
#12 0x0000561753cbc523 in rte_eal_mp_remote_launch (
    f=0x561752f38ab5 <ipsec_launch_one_lcore>, arg=0x56175c549700,
    call_main=CALL_MAIN) at ../lib/eal/common/eal_common_launch.c:64
#13 0x0000561752f265ed in main (argc=12, argv=0x7ffd19234168)
    at ../examples/ipsec-secgw/ipsec-secgw.c:2978
(gdb) frame 2
#2  0x0000561757073753 in aesni_gcm_session_configure (mb_mgr=0x56175c5fe400,
    session=0x17e3b72d8, xform=0x17e05d7c0)
    at ../drivers/crypto/ipsec_mb/pmd_aesni_gcm.c:132
132                     mb_mgr->gcm128_pre(key, &sess->gdata_key);

Because of un-expected unaligned memory access:
(gdb) disas
Dump of assembler code for function aes_keyexp_128_enc_avx512:
   0x00007ff9274f400b <+0>:     endbr64
   0x00007ff9274f400f <+4>:     cmp    $0x0,%rdi
   0x00007ff9274f4013 <+8>:     je     0x7ff9274f41b4 <aes_keyexp_128_enc_avx512+425>
   0x00007ff9274f4019 <+14>:    cmp    $0x0,%rsi
   0x00007ff9274f401d <+18>:    je     0x7ff9274f41b4 <aes_keyexp_128_enc_avx512+425>
   0x00007ff9274f4023 <+24>:    vmovdqu (%rdi),%xmm1
=> 0x00007ff9274f4027 <+28>:    vmovdqa %xmm1,(%rsi)

(gdb) print/x $rsi
$12 = 0x17e3b72e8

And this is caused because now AES_GCM session private data is not 16B-bits
aligned anymore:
(gdb) print ((struct aesni_gcm_session *)sess->sess_data[index].data)
$29 = (struct aesni_gcm_session *) 0x17e3b72d8

print &((struct aesni_gcm_session *)sess->sess_data[index].data)->gdata_key
$31 = (struct gcm_key_data *) 0x17e3b72e8

As I understand the reason for that is that we changed the way how sess_data[index].data
is populated. Now it is just:
sess->sess_data[index].data = (void *)((uint8_t *)sess +
                                rte_cryptodev_sym_get_header_session_size() +
                                (index * sess->priv_sz));

So, as I can see, there is no guarantee that PMD's private sess data will be aligned on 16B
as expected.