DPDK patches and discussions
 help / color / mirror / Atom feed
From: Akhil Goyal <gakhil@marvell.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
	Anoob Joseph <anoobj@marvell.com>,
	"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>,
	"Trahe, Fiona" <fiona.trahe@intel.com>,
	"Doherty, Declan" <declan.doherty@intel.com>,
	"matan@nvidia.com" <matan@nvidia.com>,
	"g.singh@nxp.com" <g.singh@nxp.com>,
	"Zhang, Roy Fan" <roy.fan.zhang@intel.com>,
	"jianjay.zhou@huawei.com" <jianjay.zhou@huawei.com>,
	"asomalap@amd.com" <asomalap@amd.com>,
	"ruifeng.wang@arm.com" <ruifeng.wang@arm.com>,
	"Nicolau, Radu" <radu.nicolau@intel.com>,
	"ajit.khaparde@broadcom.com" <ajit.khaparde@broadcom.com>,
	Nagadheeraj Rottela <rnagadheeraj@marvell.com>,
	Ankur Dwivedi <adwivedi@marvell.com>,
	"Power, Ciara" <ciara.power@intel.com>,
	"Wang, Haiyue" <haiyue.wang@intel.com>,
	"jiawenwu@trustnetic.com" <jiawenwu@trustnetic.com>,
	"jianwang@trustnetic.com" <jianwang@trustnetic.com>
Subject: Re: [dpdk-dev] [PATCH v3 6/8] cryptodev: rework session framework
Date: Thu, 21 Oct 2021 06:53:50 +0000	[thread overview]
Message-ID: <CO6PR18MB44843EC3B92B90A43E4FEA09D8BF9@CO6PR18MB4484.namprd18.prod.outlook.com> (raw)
In-Reply-To: <DM6PR11MB44918A9A57A620A6792FA9039ABE9@DM6PR11MB4491.namprd11.prod.outlook.com>

 
> > 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.
> 
Agreed, that there is no guarantee that the sess_priv will be aligned.
I believe this is requirement from the PMD side for a particular alignment.
Is it possible for the PMD to use __rte_aligned for the fields which are required to
Be aligned. For aesni_gcm it is 16B aligned requirement, for some other PMD it may be
64B alignment. 

  reply	other threads:[~2021-10-21  6:54 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30 14:50 [dpdk-dev] [PATCH 0/3] crypto/security session framework rework Akhil Goyal
2021-09-30 14:50 ` [dpdk-dev] [PATCH 1/3] security: rework session framework Akhil Goyal
2021-09-30 14:50 ` [dpdk-dev] [PATCH 2/3] drivers/net: temporary disable ixgbe and txgbe Akhil Goyal
2021-10-12 12:26   ` Zhang, Roy Fan
2021-10-12 12:29     ` Akhil Goyal
2021-10-12 13:32       ` Zhang, Roy Fan
2021-09-30 14:50 ` [dpdk-dev] [PATCH 3/3] cryptodev: rework session framework Akhil Goyal
2021-10-01 15:53   ` Zhang, Roy Fan
2021-10-04 19:07     ` Akhil Goyal
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 0/7] crypto/security session framework rework Akhil Goyal
2021-10-13 19:22   ` [dpdk-dev] [PATCH v2 1/7] security: rework session framework Akhil Goyal
2021-10-18 21:34     ` [dpdk-dev] [PATCH v3 0/8] crypto/security session framework rework Akhil Goyal
2021-10-18 21:34       ` [dpdk-dev] [PATCH v3 1/8] security: rework session framework Akhil Goyal
2021-10-18 21:34       ` [dpdk-dev] [PATCH v3 2/8] security: hide security session struct Akhil Goyal
2021-10-18 21:34       ` [dpdk-dev] [PATCH v3 3/8] net/cnxk: rework security session framework Akhil Goyal
2021-10-18 21:34       ` [dpdk-dev] [PATCH v3 4/8] security: pass session iova in PMD sess create Akhil Goyal
2021-10-18 21:34       ` [dpdk-dev] [PATCH v3 5/8] drivers/crypto: support security session get size op Akhil Goyal
2021-10-18 21:34       ` [dpdk-dev] [PATCH v3 6/8] cryptodev: rework session framework Akhil Goyal
2021-10-20 19:27         ` Ananyev, Konstantin
2021-10-21  6:53           ` Akhil Goyal [this message]
2021-10-21 10:38             ` Ananyev, Konstantin
2021-10-21 12:30               ` Akhil Goyal
2021-10-21 13:11                 ` Ananyev, Konstantin
2021-10-18 21:34       ` [dpdk-dev] [PATCH v3 7/8] cryptodev: hide sym session structure Akhil Goyal
2021-10-18 21:34       ` [dpdk-dev] [PATCH v3 8/8] cryptodev: pass session iova in configure session Akhil Goyal
2021-10-20 14:36       ` [dpdk-dev] [PATCH v3 0/8] crypto/security session framework rework Hemant Agrawal
2021-10-20 15:45       ` Power, Ciara
2021-10-20 16:41         ` Akhil Goyal
2021-10-20 16:48           ` Akhil Goyal
2021-10-20 18:04             ` Akhil Goyal
2021-10-21  8:43               ` Zhang, Roy Fan
2021-10-13 19:22   ` [dpdk-dev] [PATCH v2 2/7] security: hide security session struct Akhil Goyal
2021-10-13 19:22   ` [dpdk-dev] [PATCH v2 3/7] net/cnxk: rework security session framework Akhil Goyal
2021-10-13 19:22   ` [dpdk-dev] [PATCH v2 4/7] security: pass session iova in PMD sess create Akhil Goyal
2021-10-13 19:22   ` [dpdk-dev] [PATCH v2 5/7] cryptodev: rework session framework Akhil Goyal
2021-10-13 19:22   ` [dpdk-dev] [PATCH v2 6/7] cryptodev: hide sym session structure Akhil Goyal
2021-10-13 19:22   ` [dpdk-dev] [PATCH v2 7/7] cryptodev: pass session iova in configure session Akhil Goyal
2021-10-14 11:47   ` [dpdk-dev] [PATCH v2 0/7] crypto/security session framework rework Akhil Goyal
2021-10-14 12:30     ` Zhang, Roy Fan
2021-10-14 12:34       ` Akhil Goyal
2021-10-14 17:07     ` Zhang, Roy Fan
2021-10-14 18:23       ` Akhil Goyal
2021-10-14 18:57         ` Akhil Goyal
2021-10-15 15:33           ` Zhang, Roy Fan
2021-10-15 17:42             ` Akhil Goyal
2021-10-15 18:47               ` Akhil Goyal
2021-10-16 13:31                 ` Zhang, Roy Fan
2021-10-16 13:21               ` Zhang, Roy Fan
2021-10-15  8:12         ` Zhang, Roy Fan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CO6PR18MB44843EC3B92B90A43E4FEA09D8BF9@CO6PR18MB4484.namprd18.prod.outlook.com \
    --to=gakhil@marvell.com \
    --cc=adwivedi@marvell.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=anoobj@marvell.com \
    --cc=asomalap@amd.com \
    --cc=ciara.power@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=fiona.trahe@intel.com \
    --cc=g.singh@nxp.com \
    --cc=haiyue.wang@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jianjay.zhou@huawei.com \
    --cc=jianwang@trustnetic.com \
    --cc=jiawenwu@trustnetic.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=matan@nvidia.com \
    --cc=pablo.de.lara.guarch@intel.com \
    --cc=radu.nicolau@intel.com \
    --cc=rnagadheeraj@marvell.com \
    --cc=roy.fan.zhang@intel.com \
    --cc=ruifeng.wang@arm.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).