[v2,8/8] crypto/ipsec_mb: set and use session ID

Message ID 20230516152422.606617-9-ciara.power@intel.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series add AESNI_MB optimisations |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS

Commit Message

Power, Ciara May 16, 2023, 3:24 p.m. UTC
  From: Pablo de Lara <pablo.de.lara.guarch@intel.com>

When creating a session, get the session ID that
defines the fixed session parameters and store it in the private data.
When retrieving IMB_JOB's, if their internal session ID matches
the one in the private session data, these fixed session parameters
do not need to be filled again.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 drivers/crypto/ipsec_mb/pmd_aesni_mb.c      | 22 ++++++++++++++++++++-
 drivers/crypto/ipsec_mb/pmd_aesni_mb_priv.h |  2 ++
 2 files changed, 23 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon June 10, 2023, 8:15 p.m. UTC | #1
16/05/2023 17:24, Ciara Power:
> From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> When creating a session, get the session ID that
> defines the fixed session parameters and store it in the private data.
> When retrieving IMB_JOB's, if their internal session ID matches
> the one in the private session data, these fixed session parameters
> do not need to be filled again.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
[...]
> +#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
> +	sess->session_id = imb_set_session(mb_mgr, &sess->template_job);
> +#endif

For info, this does not compile with
https://git.gitlab.arm.com/arm-reference-solutions/ipsec-mb.git
because Arm did not merge Intel's code correctly,
and imb_set_session() is missing while version is 1.4.0-dev.

Anyway I hate this situation having 2 repos for the same thing.
Please merge Arm code in the original repository from Intel.
  
Akhil Goyal June 14, 2023, 5:35 a.m. UTC | #2
> 16/05/2023 17:24, Ciara Power:
> > From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> >
> > When creating a session, get the session ID that
> > defines the fixed session parameters and store it in the private data.
> > When retrieving IMB_JOB's, if their internal session ID matches
> > the one in the private session data, these fixed session parameters
> > do not need to be filled again.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
> [...]
> > +#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
> > +	sess->session_id = imb_set_session(mb_mgr, &sess->template_job);
> > +#endif
> 
> For info, this does not compile with
> https://git.gitlab.arm.com/arm-reference-solutions/ipsec-mb
> because Arm did not merge Intel's code correctly,
> and imb_set_session() is missing while version is 1.4.0-dev.
> 
> Anyway I hate this situation having 2 repos for the same thing.
> Please merge Arm code in the original repository from Intel.
> 
Is it possible to make Arm repo as main repo and modify DPDK documentation,
if Intel not agreeing to include Arm code?
Currently the Arm repo use case is broken.
If it is not resolved soon, we can submit a patch to revert the patch
which is breaking compilation. This need to be fixed by RC2.
  
Fangming Fang June 15, 2023, 2:46 a.m. UTC | #3
> From: Akhil Goyal <gakhil@marvell.com>
> > 16/05/2023 17:24, Ciara Power:
> > > From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > >
> > > When creating a session, get the session ID that defines the fixed
> > > session parameters and store it in the private data.
> > > When retrieving IMB_JOB's, if their internal session ID matches the
> > > one in the private session data, these fixed session parameters do
> > > not need to be filled again.
> > >
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > [...]
> > > +#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
> > > + sess->session_id = imb_set_session(mb_mgr, &sess->template_job);
> > > +#endif
> >
> > For info, this does not compile with
> > https://git.gitlab.arm.com/arm-reference-solutions/ipsec-mb
> > because Arm did not merge Intel's code correctly, and
> > imb_set_session() is missing while version is 1.4.0-dev.
> >
> > Anyway I hate this situation having 2 repos for the same thing.
> > Please merge Arm code in the original repository from Intel.
> >
> Is it possible to make Arm repo as main repo and modify DPDK documentation,
> if Intel not agreeing to include Arm code?
> Currently the Arm repo use case is broken.
> If it is not resolved soon, we can submit a patch to revert the patch which is
> breaking compilation. This need to be fixed by RC2.

It would be better if the original repo can merge Arm changes. We will keep regular sync with original repo in the current situation, the next sync will be carried out earlier in Q3.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  
Akhil Goyal June 15, 2023, 4:47 a.m. UTC | #4
> -----Original Message-----
> From: Fangming Fang <Fangming.Fang@arm.com>
> Sent: Thursday, June 15, 2023 8:16 AM
> To: Akhil Goyal <gakhil@marvell.com>; thomas@monjalon.net; Ciara Power
> <ciara.power@intel.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; kai.ji@intel.com; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>;
> john.mcnamara@intel.com
> Cc: dev@dpdk.org; david.marchand@redhat.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Ashwin Sekhar T K <asekhar@marvell.com>
> Subject: RE: [EXT] Re: [PATCH v2 8/8] crypto/ipsec_mb: set and use session ID
> 
> > From: Akhil Goyal <gakhil@marvell.com>
> > > 16/05/2023 17:24, Ciara Power:
> > > > From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > >
> > > > When creating a session, get the session ID that defines the fixed
> > > > session parameters and store it in the private data.
> > > > When retrieving IMB_JOB's, if their internal session ID matches the
> > > > one in the private session data, these fixed session parameters do
> > > > not need to be filled again.
> > > >
> > > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > > [...]
> > > > +#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
> > > > + sess->session_id = imb_set_session(mb_mgr, &sess->template_job);
> > > > +#endif
> > >
> > > For info, this does not compile with
> > > https://git.gitlab.arm.com/arm-reference-solutions/ipsec-mb
> > > because Arm did not merge Intel's code correctly, and
> > > imb_set_session() is missing while version is 1.4.0-dev.
> > >
> > > Anyway I hate this situation having 2 repos for the same thing.
> > > Please merge Arm code in the original repository from Intel.
> > >
> > Is it possible to make Arm repo as main repo and modify DPDK documentation,
> > if Intel not agreeing to include Arm code?
> > Currently the Arm repo use case is broken.
> > If it is not resolved soon, we can submit a patch to revert the patch which is
> > breaking compilation. This need to be fixed by RC2.
> 
> It would be better if the original repo can merge Arm changes. We will keep
> regular sync with original repo in the current situation, the next sync will be
> carried out earlier in Q3.

Yes, it is better to keep original repo. But if Intel is not agreeing to it,
We may need to find some alternate way to fix it.
We need a sync as soon as possible, as with current Arm repo,
DPDK compilation is broken.

For now, we cannot merge anything in ipsec_mb driver till we fix the compilation
With Arm repo - either by Arm repo sync or by reverting the patches.

Regards,
Akhil
  
De Lara Guarch, Pablo June 16, 2023, 9:25 a.m. UTC | #5
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, June 15, 2023 5:47 AM
> To: Fangming Fang <Fangming.Fang@arm.com>; thomas@monjalon.net;
> Power, Ciara <ciara.power@intel.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ji, Kai <kai.ji@intel.com>; De Lara Guarch,
> Pablo <pablo.de.lara.guarch@intel.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; david.marchand@redhat.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Ashwin Sekhar T K <asekhar@marvell.com>
> Subject: RE: [EXT] Re: [PATCH v2 8/8] crypto/ipsec_mb: set and use session ID
> 
> 
> 
> > -----Original Message-----
> > From: Fangming Fang <Fangming.Fang@arm.com>
> > Sent: Thursday, June 15, 2023 8:16 AM
> > To: Akhil Goyal <gakhil@marvell.com>; thomas@monjalon.net; Ciara Power
> > <ciara.power@intel.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; kai.ji@intel.com; Pablo de Lara
> > <pablo.de.lara.guarch@intel.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>;
> > john.mcnamara@intel.com
> > Cc: dev@dpdk.org; david.marchand@redhat.com; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; Ashwin Sekhar T K <asekhar@marvell.com>
> > Subject: RE: [EXT] Re: [PATCH v2 8/8] crypto/ipsec_mb: set and use
> > session ID
> >
> > > From: Akhil Goyal <gakhil@marvell.com>
> > > > 16/05/2023 17:24, Ciara Power:
> > > > > From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > > >
> > > > > When creating a session, get the session ID that defines the
> > > > > fixed session parameters and store it in the private data.
> > > > > When retrieving IMB_JOB's, if their internal session ID matches
> > > > > the one in the private session data, these fixed session
> > > > > parameters do not need to be filled again.
> > > > >
> > > > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > > > [...]
> > > > > +#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
> > > > > + sess->session_id = imb_set_session(mb_mgr,
> > > > > + sess->&sess->template_job);
> > > > > +#endif
> > > >
> > > > For info, this does not compile with
> > > > https://git.gitlab.arm.com/arm-reference-solutions/ipsec-mb
> > > > because Arm did not merge Intel's code correctly, and
> > > > imb_set_session() is missing while version is 1.4.0-dev.
> > > >
> > > > Anyway I hate this situation having 2 repos for the same thing.
> > > > Please merge Arm code in the original repository from Intel.
> > > >
> > > Is it possible to make Arm repo as main repo and modify DPDK
> > > documentation, if Intel not agreeing to include Arm code?
> > > Currently the Arm repo use case is broken.
> > > If it is not resolved soon, we can submit a patch to revert the
> > > patch which is breaking compilation. This need to be fixed by RC2.
> >
> > It would be better if the original repo can merge Arm changes. We will
> > keep regular sync with original repo in the current situation, the
> > next sync will be carried out earlier in Q3.
> 
> Yes, it is better to keep original repo. But if Intel is not agreeing to it, We may
> need to find some alternate way to fix it.
> We need a sync as soon as possible, as with current Arm repo, DPDK
> compilation is broken.
> 
> For now, we cannot merge anything in ipsec_mb driver till we fix the
> compilation With Arm repo - either by Arm repo sync or by reverting the
> patches.

DPDK is supposed to use stable versions of the library (e.g. releases), not top of main branch.
As described in README https://github.com/intel/intel-ipsec-mb#4-package-content, tip of the main is NOT a release software version.
In case of ARM library clone, it is not clear what the latest stable version is.
As the Intel library version is bumped in the first stages of development for the next release, it is advisable to avoid updating this version in ARM library, until a new Intel intel-ipsec-mb version is released.
For now, the simplest solution is to revert the ARM library version to 1.3 (changing IMB_VERSION_NUM and IMB_VERSION_STR), until the ARM repo is synced to Intel 1.4.0 (which has just been released).

> 
> Regards,
> Akhil
  
Akhil Goyal June 16, 2023, 9:38 a.m. UTC | #6
> > > > >
> > > > > For info, this does not compile with
> > > > > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__git.gitlab.arm.com_arm-2Dreference-2Dsolutions_ipsec-
> 2Dmb&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey3e
> u68gBzn7DkPwuqhd6WNyo&m=g7xU0Pucoh3ZqU7RUlw8mhb-
> zlRr9t6XI0UCQi5vEjEfcovdH8kkXIJ_O-_c5zeg&s=J-l3-
> qBNnT5HJ7OGeu5iECPxB1jrpQ5iA01_AOC2Bac&e=
> > > > > because Arm did not merge Intel's code correctly, and
> > > > > imb_set_session() is missing while version is 1.4.0-dev.
> > > > >
> > > > > Anyway I hate this situation having 2 repos for the same thing.
> > > > > Please merge Arm code in the original repository from Intel.
> > > > >
> > > > Is it possible to make Arm repo as main repo and modify DPDK
> > > > documentation, if Intel not agreeing to include Arm code?
> > > > Currently the Arm repo use case is broken.
> > > > If it is not resolved soon, we can submit a patch to revert the
> > > > patch which is breaking compilation. This need to be fixed by RC2.
> > >
> > > It would be better if the original repo can merge Arm changes. We will
> > > keep regular sync with original repo in the current situation, the
> > > next sync will be carried out earlier in Q3.
> >
> > Yes, it is better to keep original repo. But if Intel is not agreeing to it, We may
> > need to find some alternate way to fix it.
> > We need a sync as soon as possible, as with current Arm repo, DPDK
> > compilation is broken.
> >
> > For now, we cannot merge anything in ipsec_mb driver till we fix the
> > compilation With Arm repo - either by Arm repo sync or by reverting the
> > patches.
> 
> DPDK is supposed to use stable versions of the library (e.g. releases), not top of
> main branch.
> As described in README https://github.com/intel/intel-ipsec-mb#4-package-content , tip of
> the main is NOT a release software version.
> In case of ARM library clone, it is not clear what the latest stable version is.
> As the Intel library version is bumped in the first stages of development for the
> next release, it is advisable to avoid updating this version in ARM library, until a
> new Intel intel-ipsec-mb version is released.
> For now, the simplest solution is to revert the ARM library version to 1.3
> (changing IMB_VERSION_NUM and IMB_VERSION_STR), until the ARM repo is
> synced to Intel 1.4.0 (which has just been released).
> 
Hi Pablo,

The point of discussion here is to have a common repo for both Intel and Arm.
We would have less control if there are 2 repos for same stuff.
The point is if Intel is not willing to take Arm changes in the repo (preferred solution),
Why not update documentation to make Arm one as default which is a
super set of Intel and Arm repo? This will avoid problems going forward.

Regards,
Akhil
  
Fangming Fang June 20, 2023, 6:32 a.m. UTC | #7
> From: Akhil Goyal <gakhil@marvell.com>
> Subject: RE: [EXT] Re: [PATCH v2 8/8] crypto/ipsec_mb: set and use session ID
>
> > > > > >
> > > > > > For info, this does not compile with
> > > > > > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__git.gitlab.arm.com_arm-2Dreference-2Dsolutions_ipsec-
> >
> 2Dmb&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey
> 3e
> > u68gBzn7DkPwuqhd6WNyo&m=g7xU0Pucoh3ZqU7RUlw8mhb-
> > zlRr9t6XI0UCQi5vEjEfcovdH8kkXIJ_O-_c5zeg&s=J-l3-
> > qBNnT5HJ7OGeu5iECPxB1jrpQ5iA01_AOC2Bac&e=
> > > > > > because Arm did not merge Intel's code correctly, and
> > > > > > imb_set_session() is missing while version is 1.4.0-dev.
> > > > > >
> > > > > > Anyway I hate this situation having 2 repos for the same thing.
> > > > > > Please merge Arm code in the original repository from Intel.
> > > > > >
> > > > > Is it possible to make Arm repo as main repo and modify DPDK
> > > > > documentation, if Intel not agreeing to include Arm code?
> > > > > Currently the Arm repo use case is broken.
> > > > > If it is not resolved soon, we can submit a patch to revert the
> > > > > patch which is breaking compilation. This need to be fixed by RC2.
> > > >
> > > > It would be better if the original repo can merge Arm changes. We
> > > > will keep regular sync with original repo in the current
> > > > situation, the next sync will be carried out earlier in Q3.
> > >
> > > Yes, it is better to keep original repo. But if Intel is not
> > > agreeing to it, We may need to find some alternate way to fix it.
> > > We need a sync as soon as possible, as with current Arm repo, DPDK
> > > compilation is broken.
> > >
> > > For now, we cannot merge anything in ipsec_mb driver till we fix the
> > > compilation With Arm repo - either by Arm repo sync or by reverting
> > > the patches.
> >
> > DPDK is supposed to use stable versions of the library (e.g.
> > releases), not top of main branch.
> > As described in README
> > https://github.com/intel/intel-ipsec-mb#4-package-content , tip of the main
> is NOT a release software version.
> > In case of ARM library clone, it is not clear what the latest stable version is.
> > As the Intel library version is bumped in the first stages of
> > development for the next release, it is advisable to avoid updating
> > this version in ARM library, until a new Intel intel-ipsec-mb version is
> released.
> > For now, the simplest solution is to revert the ARM library version to
> > 1.3 (changing IMB_VERSION_NUM and IMB_VERSION_STR), until the ARM
> repo
> > is synced to Intel 1.4.0 (which has just been released).
> >
> Hi Pablo,
>
> The point of discussion here is to have a common repo for both Intel and Arm.
> We would have less control if there are 2 repos for same stuff.
> The point is if Intel is not willing to take Arm changes in the repo (preferred
> solution), Why not update documentation to make Arm one as default which
> is a super set of Intel and Arm repo? This will avoid problems going forward.
>
> Regards,
> Akhil

We have reverted the Arm library version to 1.3.0 to work around this issue according to Pablo's suggestion. The library fixing this issue is tagged as SECLIB-IPSEC-2023.06.20.

Thanks,
Fangming

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  
Thomas Monjalon June 20, 2023, 2:41 p.m. UTC | #8
16/06/2023 11:38, Akhil Goyal:
> > > > > >
> > > > > > For info, this does not compile with
> > > > > > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__git.gitlab.arm.com_arm-2Dreference-2Dsolutions_ipsec-
> > 2Dmb&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey3e
> > u68gBzn7DkPwuqhd6WNyo&m=g7xU0Pucoh3ZqU7RUlw8mhb-
> > zlRr9t6XI0UCQi5vEjEfcovdH8kkXIJ_O-_c5zeg&s=J-l3-
> > qBNnT5HJ7OGeu5iECPxB1jrpQ5iA01_AOC2Bac&e=
> > > > > > because Arm did not merge Intel's code correctly, and
> > > > > > imb_set_session() is missing while version is 1.4.0-dev.
> > > > > >
> > > > > > Anyway I hate this situation having 2 repos for the same thing.
> > > > > > Please merge Arm code in the original repository from Intel.
> > > > > >
> > > > > Is it possible to make Arm repo as main repo and modify DPDK
> > > > > documentation, if Intel not agreeing to include Arm code?
> > > > > Currently the Arm repo use case is broken.
> > > > > If it is not resolved soon, we can submit a patch to revert the
> > > > > patch which is breaking compilation. This need to be fixed by RC2.
> > > >
> > > > It would be better if the original repo can merge Arm changes. We will
> > > > keep regular sync with original repo in the current situation, the
> > > > next sync will be carried out earlier in Q3.
> > >
> > > Yes, it is better to keep original repo. But if Intel is not agreeing to it, We may
> > > need to find some alternate way to fix it.
> > > We need a sync as soon as possible, as with current Arm repo, DPDK
> > > compilation is broken.
> > >
> > > For now, we cannot merge anything in ipsec_mb driver till we fix the
> > > compilation With Arm repo - either by Arm repo sync or by reverting the
> > > patches.
> > 
> > DPDK is supposed to use stable versions of the library (e.g. releases), not top of
> > main branch.
> > As described in README https://github.com/intel/intel-ipsec-mb#4-package-content , tip of
> > the main is NOT a release software version.
> > In case of ARM library clone, it is not clear what the latest stable version is.
> > As the Intel library version is bumped in the first stages of development for the
> > next release, it is advisable to avoid updating this version in ARM library, until a
> > new Intel intel-ipsec-mb version is released.
> > For now, the simplest solution is to revert the ARM library version to 1.3
> > (changing IMB_VERSION_NUM and IMB_VERSION_STR), until the ARM repo is
> > synced to Intel 1.4.0 (which has just been released).
> > 
> Hi Pablo,
> 
> The point of discussion here is to have a common repo for both Intel and Arm.
> We would have less control if there are 2 repos for same stuff.
> The point is if Intel is not willing to take Arm changes in the repo (preferred solution),
> Why not update documentation to make Arm one as default which is a
> super set of Intel and Arm repo? This will avoid problems going forward.

Pablo, how can we reconcile both projects in one repository please?
  
De Lara Guarch, Pablo June 21, 2023, 7:11 p.m. UTC | #9
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, June 20, 2023 3:42 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Fangming Fang
> <Fangming.Fang@arm.com>; Power, Ciara <ciara.power@intel.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ji, Kai
> <kai.ji@intel.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>; Mcnamara,
> John <john.mcnamara@intel.com>; Akhil Goyal <gakhil@marvell.com>
> Cc: dev@dpdk.org; david.marchand@redhat.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Ashwin Sekhar T K <asekhar@marvell.com>
> Subject: Re: [EXT] Re: [PATCH v2 8/8] crypto/ipsec_mb: set and use session ID
> 
> 16/06/2023 11:38, Akhil Goyal:
> > > > > > >
> > > > > > > For info, this does not compile with
> > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__git.gitlab.arm.com_arm-2Dreference-2Dsolutions_ipsec-
> > >
> 2Dmb&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey
> 3e
> > > u68gBzn7DkPwuqhd6WNyo&m=g7xU0Pucoh3ZqU7RUlw8mhb-
> > > zlRr9t6XI0UCQi5vEjEfcovdH8kkXIJ_O-_c5zeg&s=J-l3-
> > > qBNnT5HJ7OGeu5iECPxB1jrpQ5iA01_AOC2Bac&e=
> > > > > > > because Arm did not merge Intel's code correctly, and
> > > > > > > imb_set_session() is missing while version is 1.4.0-dev.
> > > > > > >
> > > > > > > Anyway I hate this situation having 2 repos for the same thing.
> > > > > > > Please merge Arm code in the original repository from Intel.
> > > > > > >
> > > > > > Is it possible to make Arm repo as main repo and modify DPDK
> > > > > > documentation, if Intel not agreeing to include Arm code?
> > > > > > Currently the Arm repo use case is broken.
> > > > > > If it is not resolved soon, we can submit a patch to revert
> > > > > > the patch which is breaking compilation. This need to be fixed by
> RC2.
> > > > >
> > > > > It would be better if the original repo can merge Arm changes.
> > > > > We will keep regular sync with original repo in the current
> > > > > situation, the next sync will be carried out earlier in Q3.
> > > >
> > > > Yes, it is better to keep original repo. But if Intel is not
> > > > agreeing to it, We may need to find some alternate way to fix it.
> > > > We need a sync as soon as possible, as with current Arm repo, DPDK
> > > > compilation is broken.
> > > >
> > > > For now, we cannot merge anything in ipsec_mb driver till we fix
> > > > the compilation With Arm repo - either by Arm repo sync or by
> > > > reverting the patches.
> > >
> > > DPDK is supposed to use stable versions of the library (e.g.
> > > releases), not top of main branch.
> > > As described in README
> > > https://github.com/intel/intel-ipsec-mb#4-package-content , tip of the
> main is NOT a release software version.
> > > In case of ARM library clone, it is not clear what the latest stable version is.
> > > As the Intel library version is bumped in the first stages of
> > > development for the next release, it is advisable to avoid updating
> > > this version in ARM library, until a new Intel intel-ipsec-mb version is
> released.
> > > For now, the simplest solution is to revert the ARM library version
> > > to 1.3 (changing IMB_VERSION_NUM and IMB_VERSION_STR), until the
> ARM
> > > repo is synced to Intel 1.4.0 (which has just been released).
> > >
> > Hi Pablo,
> >
> > The point of discussion here is to have a common repo for both Intel and
> Arm.
> > We would have less control if there are 2 repos for same stuff.
> > The point is if Intel is not willing to take Arm changes in the repo
> > (preferred solution), Why not update documentation to make Arm one as
> > default which is a super set of Intel and Arm repo? This will avoid problems
> going forward.
> 
> Pablo, how can we reconcile both projects in one repository please?
> 

Unfortunately, it is not straight forward to just reconcile both projects.
 
The main issue is that we currently only have 1-2 developers working on the library so we don't have enough bandwidth to review, test and merge patches. Also we FIPS certify each release, which is required by some of our customers. This is expensive and time consuming and we want to minimize the test surface covered by the certification.
 
We think the current method can work, as long as ARM's library gets synced with Intel's library *just after* each release, so that the version number corresponds to the available API.
I understand this is probably not ideal, but this is the best we can manage with our current resources and FIPS constraints.
  
Patrick Robb Feb. 21, 2024, 5:01 a.m. UTC | #10
On Tue, Jun 20, 2023 at 5:44 AM Fangming Fang <Fangming.Fang@arm.com> wrote:

> v1.4 release on the arm repo breaks DPDK
> We have reverted the Arm library version to 1.3.0 to work around this
> issue according to Pablo's suggestion. The library fixing this issue is
> tagged as SECLIB-IPSEC-2023.06.20.
>
> Thanks,
> Fangming
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>

Does anyone know if there are plans to bring the arm ipsec-mb repo to v1.4,
or plans to advance any of the other ideas which were put forth in this
conversation? DPDK 24.03 will require ipsec 1.4 or greater, and there is a
patch pending to that effect.

We do run some testing at the Community Lab on ARM which installs the
ipsec-mb library, and runs ZUC and Snow3G autotests with their respective
crypto vdev pmds. As noted here the the build fails from the v1.4 tag on
arm, and the SECLIB-IPSEC-2023.06.20 cannot be used for 24.03. If it won't
be possible to install ipsec-mb on ARM for use with 24.03 I will have to
take that test coverage offline. I just want to check to see if there is a
workaround I've missed or some planned work on the horizon which will
unblock this. Thanks.
  
Honnappa Nagarahalli Feb. 21, 2024, 5:10 a.m. UTC | #11
> On Feb 20, 2024, at 11:01 PM, Patrick Robb <probb@iol.unh.edu> wrote:
>
>
>
> On Tue, Jun 20, 2023 at 5:44 AM Fangming Fang <Fangming.Fang@arm.com> wrote:
> v1.4 release on the arm repo breaks DPDK
> We have reverted the Arm library version to 1.3.0 to work around this issue according to Pablo's suggestion. The library fixing this issue is tagged as SECLIB-IPSEC-2023.06.20.
>
> Thanks,
> Fangming
>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
> Does anyone know if there are plans to bring the arm ipsec-mb repo to v1.4, or plans to advance any of the other ideas which were put forth in this conversation? DPDK 24.03 will require ipsec 1.4 or greater, and there is a patch pending to that effect.
>
> We do run some testing at the Community Lab on ARM which installs the ipsec-mb library, and runs ZUC and Snow3G autotests with their respective crypto vdev pmds. As noted here the the build fails from the v1.4 tag on arm, and the SECLIB-IPSEC-2023.06.20 cannot be used for 24.03. If it won't be possible to install ipsec-mb on ARM for use with 24.03 I will have to take that test coverage offline. I just want to check to see if there is a workaround I've missed or some planned work on the horizon which will unblock this. Thanks.
I am not sure about the current plans, we will get back on this soon.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  
Patrick Robb Feb. 21, 2024, 5:23 a.m. UTC | #12
Okay. In terms of lab operations, I'll disable the ARM ZUC/Snow3G tests for
main and next-*, but leave enabled for LTS. Once we can use v1.4 for ARM,
will re-enable everything. Thanks.

On Wed, Feb 21, 2024 at 12:10 AM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:

>
>
> > On Feb 20, 2024, at 11:01 PM, Patrick Robb <probb@iol.unh.edu> wrote:
> >
> >
> >
> > On Tue, Jun 20, 2023 at 5:44 AM Fangming Fang <Fangming.Fang@arm.com>
> wrote:
> > v1.4 release on the arm repo breaks DPDK
> > We have reverted the Arm library version to 1.3.0 to work around this
> issue according to Pablo's suggestion. The library fixing this issue is
> tagged as SECLIB-IPSEC-2023.06.20.
> >
> > Thanks,
> > Fangming
> >
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
> >
> > Does anyone know if there are plans to bring the arm ipsec-mb repo to
> v1.4, or plans to advance any of the other ideas which were put forth in
> this conversation? DPDK 24.03 will require ipsec 1.4 or greater, and there
> is a patch pending to that effect.
> >
> > We do run some testing at the Community Lab on ARM which installs the
> ipsec-mb library, and runs ZUC and Snow3G autotests with their respective
> crypto vdev pmds. As noted here the the build fails from the v1.4 tag on
> arm, and the SECLIB-IPSEC-2023.06.20 cannot be used for 24.03. If it won't
> be possible to install ipsec-mb on ARM for use with 24.03 I will have to
> take that test coverage offline. I just want to check to see if there is a
> workaround I've missed or some planned work on the horizon which will
> unblock this. Thanks.
> I am not sure about the current plans, we will get back on this soon.
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
  
Power, Ciara Feb. 21, 2024, 9:50 a.m. UTC | #13
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Wednesday, February 21, 2024 5:10 AM
> To: Patrick Robb <probb@iol.unh.edu>
> Cc: Fangming Fang <Fangming.Fang@arm.com>; Akhil Goyal
> <gakhil@marvell.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; thomas@monjalon.net; Power, Ciara
> <ciara.power@intel.com>; Ji, Kai <kai.ji@intel.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Mcnamara, John <john.mcnamara@intel.com>;
> dev@dpdk.org; Marchand, David <david.marchand@redhat.com>;
> jerinj@marvell.com; Ashwin Sekhar T K <asekhar@marvell.com>; Wathsala
> Wathawana Vithanage <wathsala.vithanage@arm.com>; Dhruv Tripathi
> <Dhruv.Tripathi@arm.com>
> Subject: Re: [EXT] [PATCH v2 8/8] crypto/ipsec_mb: set and use session ID
> 
> 
> 
> > On Feb 20, 2024, at 11:01 PM, Patrick Robb <probb@iol.unh.edu> wrote:
> >
> >
> >
> > On Tue, Jun 20, 2023 at 5:44 AM Fangming Fang
> <Fangming.Fang@arm.com> wrote:
> > v1.4 release on the arm repo breaks DPDK We have reverted the Arm
> > library version to 1.3.0 to work around this issue according to Pablo's
> suggestion. The library fixing this issue is tagged as SECLIB-IPSEC-2023.06.20.
> >
> > Thanks,
> > Fangming
> >
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
> >
> > Does anyone know if there are plans to bring the arm ipsec-mb repo to v1.4,
> or plans to advance any of the other ideas which were put forth in this
> conversation? DPDK 24.03 will require ipsec 1.4 or greater, and there is a
> patch pending to that effect.
> >
> > We do run some testing at the Community Lab on ARM which installs the
> ipsec-mb library, and runs ZUC and Snow3G autotests with their respective
> crypto vdev pmds. As noted here the the build fails from the v1.4 tag on arm,
> and the SECLIB-IPSEC-2023.06.20 cannot be used for 24.03. If it won't be
> possible to install ipsec-mb on ARM for use with 24.03 I will have to take that
> test coverage offline. I just want to check to see if there is a workaround I've
> missed or some planned work on the horizon which will unblock this. Thanks.
> I am not sure about the current plans, we will get back on this soon.


Hi folks,

We had based the ipsec-mb version bump to 1.4 on both intel ipsec-mb and arm ipsec-mb supporting that version, so both could still use the Ipsec-mb SW PMDs.
I based the arm support from the repo main branch (https://gitlab.arm.com/arm-reference-solutions/ipsec-mb/-/tree/main)
It's version is 1.4 and looks like some of the code changes from intel-ipsec-mb 1.4 have been merged in (e.g. imb_set_session function which is 1.4+).
I know there was issues before with it being tagged as 1.4 too soon - is that still the case, or is the main branch up to date now with intel-ipsec-mb 1.4?

Thanks,
Ciara
  
Patrick Robb Feb. 21, 2024, 7:09 p.m. UTC | #14
On Wed, Feb 21, 2024 at 4:50 AM Power, Ciara <ciara.power@intel.com> wrote:

>
> Hi folks,
>
> We had based the ipsec-mb version bump to 1.4 on both intel ipsec-mb and
> arm ipsec-mb supporting that version, so both could still use the Ipsec-mb
> SW PMDs.
> I based the arm support from the repo main branch (
> https://gitlab.arm.com/arm-reference-solutions/ipsec-mb/-/tree/main)
> It's version is 1.4 and looks like some of the code changes from
> intel-ipsec-mb 1.4 have been merged in (e.g. imb_set_session function which
> is 1.4+).
> I know there was issues before with it being tagged as 1.4 too soon - is
> that still the case, or is the main branch up to date now with
> intel-ipsec-mb 1.4?
>
> Thanks,
> Ciara
>

Hi Ciara,

I see what you mean and did a test on an arm container. I used tip of main
for ipsec-mb, applied the v1.4 requirement patch to dpdk, built dpdk and
was able to run the ZUC autotest with the zuc vdev pmd.

root@8aad8acde315:/opt/dpdk/build# meson test cryptodev_sw_zuc_autotest
--test-args="--no-huge --no-pci --vdev=crypto_zuc"
ninja: no work to do.
ninja: Entering directory `/opt/dpdk/build'
ninja: no work to do.
1/1 DPDK:driver-tests / cryptodev_sw_zuc_autotest        OK
0.22s

Ok:                 1
Expected Fail:      0
Fail:               0
Unexpected Pass:    0
Skipped:            0
Timeout:            0

Usually when we install dependencies we go from a tag which is stable.
We'll update the dpdk-ci container image template engine to clone the repo
and then checkout to this commit which is known to be
good: cef9b8924e3ff52f2c5a3703860008c27ee723c4. And our automated testing
can remain online in this way.

Arm folks, I'm not sure what your plans are but at some point in the docs I
think pages like:

https://doc.dpdk.org/guides/cryptodevs/zuc.html
https://doc.dpdk.org/guides/cryptodevs/snow3g.html

should be updated to reference this commit, or a newly introduced tag, to
reflect that the SECLIB-IPSEC-2023.06.20 tag will no longer work for 24.03
as it is ipsec 1.3.

Thanks for the help.
  
Wathsala Wathawana Vithanage Feb. 22, 2024, 1:55 a.m. UTC | #15
I tried mainline earlier today it compiles and links fine. However, build failed
on v1.4. We are working on tagging the mainline, until then please continue
working on mainline. 

> -----Original Message-----
> From: Patrick Robb <probb@iol.unh.edu>
> Sent: Tuesday, February 20, 2024 11:01 PM
> To: Fangming Fang <Fangming.Fang@arm.com>
> Cc: Akhil Goyal <gakhil@marvell.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; thomas@monjalon.net; Power, Ciara
> <ciara.power@intel.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ji, Kai <kai.ji@intel.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Mcnamara, John <john.mcnamara@intel.com>;
> dev@dpdk.org; david.marchand@redhat.com; jerinj@marvell.com; Ashwin
> Sekhar T K <asekhar@marvell.com>
> Subject: Re: [EXT] Re: [PATCH v2 8/8] crypto/ipsec_mb: set and use session ID
> 
> 
> 
> On Tue, Jun 20, 2023 at 5:44 AM Fangming Fang <Fangming.Fang@arm.com
> <mailto:Fangming.Fang@arm.com> > wrote:
> 
> 	v1.4 release on the arm repo breaks DPDK
> 	We have reverted the Arm library version to 1.3.0 to work around this
> issue according to Pablo's suggestion. The library fixing this issue is tagged as
> SECLIB-IPSEC-2023.06.20.
> 
> 	Thanks,
> 	Fangming
> 
> 	IMPORTANT NOTICE: The contents of this email and any attachments
> are confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
> 
> 
> 
> Does anyone know if there are plans to bring the arm ipsec-mb repo to v1.4,
> or plans to advance any of the other ideas which were put forth in this
> conversation? DPDK 24.03 will require ipsec 1.4 or greater, and there is a
> patch pending to that effect.
> 
> We do run some testing at the Community Lab on ARM which installs the
> ipsec-mb library, and runs ZUC and Snow3G autotests with their respective
> crypto vdev pmds. As noted here the the build fails from the v1.4 tag on arm,
> and the SECLIB-IPSEC-2023.06.20 cannot be used for 24.03. If it won't be
> possible to install ipsec-mb on ARM for use with 24.03 I will have to take that
> test coverage offline. I just want to check to see if there is a workaround I've
> missed or some planned work on the horizon which will unblock this. Thanks.
  
Wathsala Wathawana Vithanage Feb. 26, 2024, 11:23 p.m. UTC | #16
Hi Patrick,
> I tried mainline earlier today it compiles and links fine. However, build failed on
> v1.4. We are working on tagging the mainline, until then please continue
> working on mainline.
> 
A new tag SECLIB-IPSEC-2023.10.13 has been created from the tip of arm ipsec-mb git repo. 
Please use this tag going forward, it has been tested and works as expected.
  
Patrick Robb Feb. 27, 2024, 1:05 a.m. UTC | #17
On Mon, Feb 26, 2024 at 6:24 PM Wathsala Wathawana Vithanage <
wathsala.vithanage@arm.com> wrote:

> Hi Patrick,
> > I tried mainline earlier today it compiles and links fine. However,
> build failed on
> > v1.4. We are working on tagging the mainline, until then please continue
> > working on mainline.
> >
> A new tag SECLIB-IPSEC-2023.10.13 has been created from the tip of arm
> ipsec-mb git repo.
> Please use this tag going forward, it has been tested and works as
> expected.
>
> Thanks Wathsala - will do!
  
Akhil Goyal Feb. 27, 2024, 6:13 a.m. UTC | #18
> Subject: RE: [EXT] Re: [PATCH v2 8/8] crypto/ipsec_mb: set and use session ID
> 
> Hi Patrick,
> > I tried mainline earlier today it compiles and links fine. However, build failed on
> > v1.4. We are working on tagging the mainline, until then please continue
> > working on mainline.
> >
> A new tag SECLIB-IPSEC-2023.10.13 has been created from the tip of arm ipsec-
> mb git repo.
> Please use this tag going forward, it has been tested and works as expected.

Can you send a patch to update the documentation with the updated tag?
  
Wathsala Wathawana Vithanage March 5, 2024, 5:36 p.m. UTC | #19
> A new tag SECLIB-IPSEC-2023.10.13 has been created from the tip of arm
> ipsec-mb git repo.
> Please use this tag going forward, it has been tested and works as expected.

Please use SECLIB-IPSEC-2024.03.05 that fixes issues caused by above tag.
We will be updating the documentation soon.
  

Patch

diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
index f83738a5eb..f4322d9af4 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
@@ -845,6 +845,10 @@  aesni_mb_session_configure(IMB_MGR *mb_mgr,
 		}
 	}
 
+#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
+	sess->session_id = imb_set_session(mb_mgr, &sess->template_job);
+#endif
+
 	return 0;
 }
 
@@ -977,6 +981,10 @@  aesni_mb_set_docsis_sec_session_parameters(
 		goto error_exit;
 	}
 
+#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
+	ipsec_sess->session_id = imb_set_session(mb_mgr, &ipsec_sess->template_job);
+#endif
+
 error_exit:
 	free_mb_mgr(mb_mgr);
 	return ret;
@@ -1386,6 +1394,9 @@  set_gcm_job(IMB_MGR *mb_mgr, IMB_JOB *job, const uint8_t sgl,
 		job->msg_len_to_hash_in_bytes = 0;
 		job->msg_len_to_cipher_in_bytes = 0;
 		job->cipher_start_src_offset_in_bytes = 0;
+#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
+		imb_set_session(mb_mgr, job);
+#endif
 	} else {
 		job->hash_start_src_offset_in_bytes =
 				op->sym->aead.data.offset;
@@ -1470,7 +1481,10 @@  set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 	const IMB_CIPHER_MODE cipher_mode =
 			session->template_job.cipher_mode;
 
-	memcpy(job, &session->template_job, sizeof(IMB_JOB));
+#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
+	if (job->session_id != session->session_id)
+#endif
+		memcpy(job, &session->template_job, sizeof(IMB_JOB));
 
 	if (!op->sym->m_dst) {
 		/* in-place operation */
@@ -1510,6 +1524,9 @@  set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 			job->u.GCM.ctx = &qp_data->gcm_sgl_ctx;
 			job->cipher_mode = IMB_CIPHER_GCM_SGL;
 			job->hash_alg = IMB_AUTH_GCM_SGL;
+#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
+			imb_set_session(mb_mgr, job);
+#endif
 		}
 		break;
 	case IMB_AUTH_AES_GMAC_128:
@@ -1534,6 +1551,9 @@  set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
 			job->u.CHACHA20_POLY1305.ctx = &qp_data->chacha_sgl_ctx;
 			job->cipher_mode = IMB_CIPHER_CHACHA20_POLY1305_SGL;
 			job->hash_alg = IMB_AUTH_CHACHA20_POLY1305_SGL;
+#if IMB_VERSION(1, 3, 0) < IMB_VERSION_NUM
+			imb_set_session(mb_mgr, job);
+#endif
 		}
 		break;
 	default:
diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_mb_priv.h b/drivers/crypto/ipsec_mb/pmd_aesni_mb_priv.h
index ce9a6e4886..9b7c9edb6d 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_mb_priv.h
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_mb_priv.h
@@ -854,6 +854,8 @@  get_digest_byte_length(IMB_HASH_ALG algo)
 struct aesni_mb_session {
 	IMB_JOB template_job;
 	/*< Template job structure */
+	uint32_t session_id;
+	/*< IPSec MB session ID */
 	struct {
 		uint16_t offset;
 	} iv;