DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] crypto/armv8: enable meson build
@ 2019-10-03 22:57 Dharmik Thakkar
  2019-10-05 15:28 ` Jerin Jacob
  2019-11-18  7:49 ` Akhil Goyal
  0 siblings, 2 replies; 13+ messages in thread
From: Dharmik Thakkar @ 2019-10-03 22:57 UTC (permalink / raw)
  To: Jerin Jacob, Bruce Richardson; +Cc: dev, honnappa.nagarahalli, Dharmik Thakkar

Add new meson.build file for crypto/armv8

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
---
 drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
 drivers/crypto/meson.build       |  6 +++---
 meson_options.txt                |  2 ++
 3 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 drivers/crypto/armv8/meson.build

diff --git a/drivers/crypto/armv8/meson.build b/drivers/crypto/armv8/meson.build
new file mode 100644
index 000000000000..1ef78fa5d8c7
--- /dev/null
+++ b/drivers/crypto/armv8/meson.build
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2019 Arm Limited
+
+path = get_option('armv8_crypto_dir')
+if path == ''
+	build = false
+	reason = 'missing dependency, "armv8_crypto"'
+	subdir_done()
+endif
+
+inc_dir = path + '/asm/include'
+
+lib = cc.find_library('libarmv8_crypto', dirs: [path], required: false)
+if not lib.found()
+	build = false
+	reason = 'missing dependency, "armv8_crypto"'
+	subdir_done()
+else
+	ext_deps += lib
+	includes += include_directories(inc_dir)
+endif
+
+deps += ['bus_vdev']
+sources = files('rte_armv8_pmd.c', 'rte_armv8_pmd_ops.c')
+allow_experimental_apis = true
diff --git a/drivers/crypto/meson.build b/drivers/crypto/meson.build
index 83e78860ebee..605dcdd5f4d6 100644
--- a/drivers/crypto/meson.build
+++ b/drivers/crypto/meson.build
@@ -1,9 +1,9 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-drivers = ['aesni_gcm', 'aesni_mb', 'caam_jr', 'ccp', 'dpaa_sec', 'dpaa2_sec',
-	'kasumi', 'mvsam', 'null', 'octeontx', 'openssl', 'qat', 'scheduler',
-	'snow3g', 'virtio', 'zuc']
+drivers = ['aesni_gcm', 'aesni_mb', 'armv8', 'caam_jr', 'ccp', 'dpaa_sec',
+	'dpaa2_sec', 'kasumi', 'mvsam', 'null', 'octeontx', 'openssl', 'qat',
+	'scheduler', 'snow3g', 'virtio', 'zuc']
 
 std_deps = ['cryptodev'] # cryptodev pulls in all other needed deps
 config_flag_fmt = 'RTE_LIBRTE_@0@_PMD'
diff --git a/meson_options.txt b/meson_options.txt
index 448f3e63dcf2..4c0413918a34 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -2,6 +2,8 @@
 
 option('allow_invalid_socket_id', type: 'boolean', value: false,
 	description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
+option('armv8_crypto_dir', type: 'string', value: '',
+	description: 'path to the armv8_crypto library installation directory')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
 	description: 'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.')
 option('enable_docs', type: 'boolean', value: false,
-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-03 22:57 [dpdk-dev] [PATCH] crypto/armv8: enable meson build Dharmik Thakkar
@ 2019-10-05 15:28 ` Jerin Jacob
  2019-10-06 18:06   ` Thomas Monjalon
  2019-11-18  7:49 ` Akhil Goyal
  1 sibling, 1 reply; 13+ messages in thread
From: Jerin Jacob @ 2019-10-05 15:28 UTC (permalink / raw)
  To: Dharmik Thakkar, Akhil Goyal, Hemant Agrawal, Thomas Monjalon,
	anoobj, pathreya
  Cc: Jerin Jacob, Bruce Richardson, dpdk-dev, Honnappa Nagarahalli

On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com> wrote:
>
> Add new meson.build file for crypto/armv8
>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> ---
>  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
>  drivers/crypto/meson.build       |  6 +++---
>  meson_options.txt                |  2 ++
>  3 files changed, 30 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/crypto/armv8/meson.build

>
>  option('allow_invalid_socket_id', type: 'boolean', value: false,
>         description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
> +option('armv8_crypto_dir', type: 'string', value: '',
> +       description: 'path to the armv8_crypto library installation directory')

It is not specific to this patch but it is connected to this patch.

Three years back when Cavium contributed to this driver the situation
was different where only Cavium was contributing to DPDK and now we
have multiple vendors from
ARMv8 platform and ARM itself is contributing it.

When it is submitted, I was not in favor of the external library. But
various reasons it happened to be the external library where 90% meat
in this library and shim PMD
the driver moved to DPDK.

Now, I look back, It does not make sense to the external library. Reasons are
- It won't allow another ARMv8 player to contribute to this library as
Marvell owns this repo and there is no upstreaming path to this
library.
- That made this library to not have 'any' change for the last three
year and everyone have there owned copy of this driver. In fact the
library was not compiling for last 2.5 years.
- AES-NI case it makes sense to have an external library as it is a
single vendor and it is not specific to DPDK. But in this, It is
another way around
- If it an external library, we might as well add the PMD code as well
there and that only 10% of the real stuff.
We are not able able to improve anything in this library due to this situation.

Does anyone care about this PMD? If not, we might as well remove this
DPDK and every vendor can manage the external library and external
PMD(Situation won't change much)

Thoughts from ARM, other ARMv8 vendors or community?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-05 15:28 ` Jerin Jacob
@ 2019-10-06 18:06   ` Thomas Monjalon
  2019-10-07 10:19     ` Jerin Jacob
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Monjalon @ 2019-10-06 18:06 UTC (permalink / raw)
  To: Jerin Jacob, Jerin Jacob
  Cc: Dharmik Thakkar, Akhil Goyal, Hemant Agrawal, anoobj, pathreya,
	Bruce Richardson, dpdk-dev, Honnappa Nagarahalli

05/10/2019 17:28, Jerin Jacob:
> On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com> wrote:
> >
> > Add new meson.build file for crypto/armv8
> >
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > ---
> >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> >  drivers/crypto/meson.build       |  6 +++---
> >  meson_options.txt                |  2 ++
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/crypto/armv8/meson.build
> 
> >
> >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> >         description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
> > +option('armv8_crypto_dir', type: 'string', value: '',
> > +       description: 'path to the armv8_crypto library installation directory')

You should not need such option if you provide a pkg-config file
in your library.


> It is not specific to this patch but it is connected to this patch.
> 
> Three years back when Cavium contributed to this driver the situation
> was different where only Cavium was contributing to DPDK and now we
> have multiple vendors from
> ARMv8 platform and ARM itself is contributing it.
> 
> When it is submitted, I was not in favor of the external library. But
> various reasons it happened to be the external library where 90% meat
> in this library and shim PMD
> the driver moved to DPDK.
> 
> Now, I look back, It does not make sense to the external library. Reasons are
> - It won't allow another ARMv8 player to contribute to this library as
> Marvell owns this repo and there is no upstreaming path to this
> library.

This is a real issue and you are able to fix it.


> - That made this library to not have 'any' change for the last three
> year and everyone have there owned copy of this driver. In fact the
> library was not compiling for last 2.5 years.
> - AES-NI case it makes sense to have an external library as it is a
> single vendor and it is not specific to DPDK. But in this, It is
> another way around

I don't see how it is different, except it is badly maintained.


> - If it an external library, we might as well add the PMD code as well
> there and that only 10% of the real stuff.
> We are not able able to improve anything in this library due to this situation.
> 
> Does anyone care about this PMD? If not, we might as well remove this
> DPDK and every vendor can manage the external library and external
> PMD(Situation won't change much)

External PMD is bad.
I think this library should not be specific to DPDK,
so it would make sense as an external library.


> Thoughts from ARM, other ARMv8 vendors or community?




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-06 18:06   ` Thomas Monjalon
@ 2019-10-07 10:19     ` Jerin Jacob
  2019-10-08  7:18       ` Jerin Jacob
  0 siblings, 1 reply; 13+ messages in thread
From: Jerin Jacob @ 2019-10-07 10:19 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Jerin Jacob, Dharmik Thakkar, Akhil Goyal, Hemant Agrawal,
	anoobj, pathreya, Bruce Richardson, dpdk-dev,
	Honnappa Nagarahalli

On Sun, 6 Oct, 2019, 11:36 PM Thomas Monjalon, <thomas@monjalon.net> wrote:

> 05/10/2019 17:28, Jerin Jacob:
> > On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com>
> wrote:
> > >
> > > Add new meson.build file for crypto/armv8
> > >
> > > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > > ---
> > >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> > >  drivers/crypto/meson.build       |  6 +++---
> > >  meson_options.txt                |  2 ++
> > >  3 files changed, 30 insertions(+), 3 deletions(-)
> > >  create mode 100644 drivers/crypto/armv8/meson.build
> >
> > >
> > >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> > >         description: 'allow out-of-range NUMA socket id\'s for
> platforms that don\'t report the value correctly')
> > > +option('armv8_crypto_dir', type: 'string', value: '',
> > > +       description: 'path to the armv8_crypto library installation
> directory')
>
> You should not need such option if you provide a pkg-config file
> in your library.
>
>
> > It is not specific to this patch but it is connected to this patch.
> >
> > Three years back when Cavium contributed to this driver the situation
> > was different where only Cavium was contributing to DPDK and now we
> > have multiple vendors from
> > ARMv8 platform and ARM itself is contributing it.
> >
> > When it is submitted, I was not in favor of the external library. But
> > various reasons it happened to be the external library where 90% meat
> > in this library and shim PMD
> > the driver moved to DPDK.
> >
> > Now, I look back, It does not make sense to the external library.
> Reasons are
> > - It won't allow another ARMv8 player to contribute to this library as
> > Marvell owns this repo and there is no upstreaming path to this
> > library.
>
> This is a real issue and you are able to fix it.
>

Note sure how I can fix it and why I need to fix it. I just dont want to
start a parallel collaborating infrastructure for DPDK armv8.


>
> > - That made this library to not have 'any' change for the last three
> > year and everyone have there owned copy of this driver. In fact the
> > library was not compiling for last 2.5 years.
> > - AES-NI case it makes sense to have an external library as it is a
> > single vendor and it is not specific to DPDK. But in this, It is
> > another way around
>
> I don't see how it is different, except it is badly maintained.
>

It is different because only one company contributing to it. In this case,
multiple companies needs to contribute.

The library badly maintained in upstream as there is no incentives to
upstream  to external library. I believe each vendor has it own copy of
that. At least Some teams in Marvell internally has copy of it.
What is their incentive to upstream? They ask me the same thing.


>
> > - If it an external library, we might as well add the PMD code as well
> > there and that only 10% of the real stuff.
> > We are not able able to improve anything in this library due to this
> situation.
> >
> > Does anyone care about this PMD? If not, we might as well remove this
> > DPDK and every vendor can manage the external library and external
> > PMD(Situation won't change much)
>
> External PMD is bad.
>

It is SHIM layer. I would say external library also bad if it is specific
to DPDK.

I think this library should not be specific to DPDK,
>

Sadly it is VERY specific to DPDK for doing authentication and encryption
in one shot to improve the performance. Openssl has already has armv8
instructions support for doing it as two pass just that performance is not
good. For use cae such as  IPsec it make sense do authentication and
encryption in one shot for performance improvement.

so it would make sense as an external library


If it an external library, it does NOT make  much sense for Marvell to
maintain it(No incentive and it is pain due lack of collaboration)

Either someone need to step up and maintain it if we NOT choose to make it
as external else we can remove the PMD from dpdk(Makes life easy for
everyone). I don't want to maintain something not upsteamble nor
collaboration friendly aka less quality.

.
>
>
>
>
> > Thoughts from ARM, other ARMv8 vendors or community?
>
>
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-07 10:19     ` Jerin Jacob
@ 2019-10-08  7:18       ` Jerin Jacob
  2019-10-10  4:46         ` Honnappa Nagarahalli
  0 siblings, 1 reply; 13+ messages in thread
From: Jerin Jacob @ 2019-10-08  7:18 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Jerin Jacob, Dharmik Thakkar, Akhil Goyal, Hemant Agrawal,
	anoobj, pathreya, Bruce Richardson, dpdk-dev,
	Honnappa Nagarahalli

On Mon, 7 Oct, 2019, 3:49 PM Jerin Jacob, <jerinjacobk@gmail.com> wrote:

>
>
> On Sun, 6 Oct, 2019, 11:36 PM Thomas Monjalon, <thomas@monjalon.net>
> wrote:
>
>> 05/10/2019 17:28, Jerin Jacob:
>> > On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com>
>> wrote:
>> > >
>> > > Add new meson.build file for crypto/armv8
>> > >
>> > > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> > > ---
>> > >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
>> > >  drivers/crypto/meson.build       |  6 +++---
>> > >  meson_options.txt                |  2 ++
>> > >  3 files changed, 30 insertions(+), 3 deletions(-)
>> > >  create mode 100644 drivers/crypto/armv8/meson.build
>> >
>> > >
>> > >  option('allow_invalid_socket_id', type: 'boolean', value: false,
>> > >         description: 'allow out-of-range NUMA socket id\'s for
>> platforms that don\'t report the value correctly')
>> > > +option('armv8_crypto_dir', type: 'string', value: '',
>> > > +       description: 'path to the armv8_crypto library installation
>> directory')
>>
>> You should not need such option if you provide a pkg-config file
>> in your library.
>>
>>
>> > It is not specific to this patch but it is connected to this patch.
>> >
>> > Three years back when Cavium contributed to this driver the situation
>> > was different where only Cavium was contributing to DPDK and now we
>> > have multiple vendors from
>> > ARMv8 platform and ARM itself is contributing it.
>> >
>> > When it is submitted, I was not in favor of the external library. But
>> > various reasons it happened to be the external library where 90% meat
>> > in this library and shim PMD
>> > the driver moved to DPDK.
>> >
>> > Now, I look back, It does not make sense to the external library.
>> Reasons are
>> > - It won't allow another ARMv8 player to contribute to this library as
>> > Marvell owns this repo and there is no upstreaming path to this
>> > library.
>>
>> This is a real issue and you are able to fix it.
>>
>
> Note sure how I can fix it and why I need to fix it. I just dont want to
> start a parallel collaborating infrastructure for DPDK armv8.
>
>
>>
>> > - That made this library to not have 'any' change for the last three
>> > year and everyone have there owned copy of this driver. In fact the
>> > library was not compiling for last 2.5 years.
>> > - AES-NI case it makes sense to have an external library as it is a
>> > single vendor and it is not specific to DPDK. But in this, It is
>> > another way around
>>
>> I don't see how it is different, except it is badly maintained.
>>
>
> It is different because only one company contributing to it. In this case,
> multiple companies needs to contribute.
>
> The library badly maintained in upstream as there is no incentives to
> upstream  to external library. I believe each vendor has it own copy of
> that. At least Some teams in Marvell internally has copy of it.
> What is their incentive to upstream? They ask me the same thing.
>
>
>>
>> > - If it an external library, we might as well add the PMD code as well
>> > there and that only 10% of the real stuff.
>> > We are not able able to improve anything in this library due to this
>> situation.
>> >
>> > Does anyone care about this PMD? If not, we might as well remove this
>> > DPDK and every vendor can manage the external library and external
>> > PMD(Situation won't change much)
>>
>> External PMD is bad.
>>
>
> It is SHIM layer. I would say external library also bad if it is specific
> to DPDK.
>
> I think this library should not be specific to DPDK,
>>
>
> Sadly it is VERY specific to DPDK for doing authentication and encryption
> in one shot to improve the performance. Openssl has already has armv8
> instructions support for doing it as two pass just that performance is not
> good. For use cae such as  IPsec it make sense do authentication and
> encryption in one shot for performance improvement.
>
> so it would make sense as an external library
>
>
> If it an external library, it does NOT make  much sense for Marvell to
> maintain it(No incentive and it is pain due lack of collaboration)
>
> Either someone need to step up and maintain it if we NOT choose to make it
> as external else we can remove the PMD from dpdk(Makes life easy for
> everyone). I don't want to maintain something not upsteamble nor
> collaboration friendly aka less quality.
>
> .
>>
>>
>>
>>
>> > Thoughts from ARM, other ARMv8 vendors or community?
>>
>
I have expressed my concerns. If there is no constructive feedback to fix
the concern. I will plan for submitting a patch to remove the shim crypto
Armv8 PMD from dpdk by next week.




>>
>>
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-08  7:18       ` Jerin Jacob
@ 2019-10-10  4:46         ` Honnappa Nagarahalli
  2019-10-10  5:24           ` Jerin Jacob
  0 siblings, 1 reply; 13+ messages in thread
From: Honnappa Nagarahalli @ 2019-10-10  4:46 UTC (permalink / raw)
  To: Jerin Jacob, thomas
  Cc: jerinj, Dharmik Thakkar, Akhil.goyal@nxp.com, hemant.agrawal,
	anoobj, pathreya, Bruce Richardson, dpdk-dev,
	Honnappa Nagarahalli, nd, nd

<snip>

On Mon, 7 Oct, 2019, 3:49 PM Jerin Jacob, <jerinjacobk@gmail.com<mailto:jerinjacobk@gmail.com>> wrote:

On Sun, 6 Oct, 2019, 11:36 PM Thomas Monjalon, <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:
05/10/2019 17:28, Jerin Jacob:
> On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com<mailto:dharmik.thakkar@arm.com>> wrote:
> >
> > Add new meson.build file for crypto/armv8
> >
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com<mailto:dharmik.thakkar@arm.com>>
> > ---
> >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> >  drivers/crypto/meson.build       |  6 +++---
> >  meson_options.txt                |  2 ++
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/crypto/armv8/meson.build
>
> >
> >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> >         description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
> > +option('armv8_crypto_dir', type: 'string', value: '',
> > +       description: 'path to the armv8_crypto library installation directory')

You should not need such option if you provide a pkg-config file
in your library.


> It is not specific to this patch but it is connected to this patch.
>
> Three years back when Cavium contributed to this driver the situation
> was different where only Cavium was contributing to DPDK and now we
> have multiple vendors from
> ARMv8 platform and ARM itself is contributing it.
>
> When it is submitted, I was not in favor of the external library. But
> various reasons it happened to be the external library where 90% meat
> in this library and shim PMD
> the driver moved to DPDK.
>
> Now, I look back, It does not make sense to the external library. Reasons are
> - It won't allow another ARMv8 player to contribute to this library as
> Marvell owns this repo and there is no upstreaming path to this
> library.

This is a real issue and you are able to fix it.

Note sure how I can fix it and why I need to fix it. I just dont want to start a parallel collaborating infrastructure for DPDK armv8.



> - That made this library to not have 'any' change for the last three
> year and everyone have there owned copy of this driver. In fact the
> library was not compiling for last 2.5 years.
> - AES-NI case it makes sense to have an external library as it is a
> single vendor and it is not specific to DPDK. But in this, It is
> another way around

I don't see how it is different, except it is badly maintained.

It is different because only one company contributing to it. In this case, multiple companies needs to contribute.

The library badly maintained in upstream as there is no incentives to upstream  to external library. I believe each vendor has it own copy of that. At least Some teams in Marvell internally has copy of it.
What is their incentive to upstream? They ask me the same thing.



> - If it an external library, we might as well add the PMD code as well
> there and that only 10% of the real stuff.
> We are not able able to improve anything in this library due to this situation.
>
> Does anyone care about this PMD? If not, we might as well remove this
> DPDK and every vendor can manage the external library and external
> PMD(Situation won't change much)

External PMD is bad.

It is SHIM layer. I would say external library also bad if it is specific to DPDK.

I think this library should not be specific to DPDK,

Sadly it is VERY specific to DPDK for doing authentication and encryption in one shot to improve the performance. Openssl has already has armv8 instructions support for doing it as two pass just that performance is not good. For use cae such as  IPsec it make sense do authentication and encryption in one shot for performance improvement.
[Honnappa] I think there is a need for such a library not just for DPDK. It would be good if it could do UDP checksum validation for the inner packet as well.

so it would make sense as an external library

If it an external library, it does NOT make  much sense for Marvell to maintain it(No incentive and it is pain due lack of collaboration)

Either someone need to step up and maintain it if we NOT choose to make it as external else we can remove the PMD from dpdk(Makes life easy for everyone). I don't want to maintain something not upsteamble nor collaboration friendly aka less quality.

.




> Thoughts from ARM, other ARMv8 vendors or community?

I have expressed my concerns. If there is no constructive feedback to fix the concern. I will plan for submitting a patch to remove the shim crypto Armv8 PMD from dpdk by next week.
[Honnappa] I do not think there is a need to remove the PMD. As you have mentioned, many might have developed their own libraries and may be dependent on DPDK Armv8 PMD.
From Arm side, there have been efforts to fix the situation. Some have not gone far and some have shown promise, but fell flat. I can say that this is still a priority but I am not sure when we will have something.
My suggestion, we should go ahead with adding the meson build for this PMD.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-10  4:46         ` Honnappa Nagarahalli
@ 2019-10-10  5:24           ` Jerin Jacob
  2019-10-11 19:13             ` Honnappa Nagarahalli
  0 siblings, 1 reply; 13+ messages in thread
From: Jerin Jacob @ 2019-10-10  5:24 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: Thomas Monjalon, Jerin Jacob, Dharmik Thakkar,
	Akhil.goyal@nxp.com, hemant.agrawal, anoobj, pathreya,
	Bruce Richardson, dpdk-dev, nd

On Thu, 10 Oct, 2019, 10:17 AM Honnappa Nagarahalli, <
Honnappa.Nagarahalli@arm.com> wrote:

> <snip>
>
>
>
> On Mon, 7 Oct, 2019, 3:49 PM Jerin Jacob, <jerinjacobk@gmail.com> wrote:
>
>
>
> On Sun, 6 Oct, 2019, 11:36 PM Thomas Monjalon, <thomas@monjalon.net>
> wrote:
>
> 05/10/2019 17:28, Jerin Jacob:
> > On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com>
> wrote:
> > >
> > > Add new meson.build file for crypto/armv8
> > >
> > > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > > ---
> > >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> > >  drivers/crypto/meson.build       |  6 +++---
> > >  meson_options.txt                |  2 ++
> > >  3 files changed, 30 insertions(+), 3 deletions(-)
> > >  create mode 100644 drivers/crypto/armv8/meson.build
> >
> > >
> > >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> > >         description: 'allow out-of-range NUMA socket id\'s for
> platforms that don\'t report the value correctly')
> > > +option('armv8_crypto_dir', type: 'string', value: '',
> > > +       description: 'path to the armv8_crypto library installation
> directory')
>
> You should not need such option if you provide a pkg-config file
> in your library.
>
>
> > It is not specific to this patch but it is connected to this patch.
> >
> > Three years back when Cavium contributed to this driver the situation
> > was different where only Cavium was contributing to DPDK and now we
> > have multiple vendors from
> > ARMv8 platform and ARM itself is contributing it.
> >
> > When it is submitted, I was not in favor of the external library. But
> > various reasons it happened to be the external library where 90% meat
> > in this library and shim PMD
> > the driver moved to DPDK.
> >
> > Now, I look back, It does not make sense to the external library.
> Reasons are
> > - It won't allow another ARMv8 player to contribute to this library as
> > Marvell owns this repo and there is no upstreaming path to this
> > library.
>
> This is a real issue and you are able to fix it.
>
>
>
> Note sure how I can fix it and why I need to fix it. I just dont want to
> start a parallel collaborating infrastructure for DPDK armv8.
>
>
>
>
>
> > - That made this library to not have 'any' change for the last three
> > year and everyone have there owned copy of this driver. In fact the
> > library was not compiling for last 2.5 years.
> > - AES-NI case it makes sense to have an external library as it is a
> > single vendor and it is not specific to DPDK. But in this, It is
> > another way around
>
> I don't see how it is different, except it is badly maintained.
>
>
>
> It is different because only one company contributing to it. In this case,
> multiple companies needs to contribute.
>
>
>
> The library badly maintained in upstream as there is no incentives to
> upstream  to external library. I believe each vendor has it own copy of
> that. At least Some teams in Marvell internally has copy of it.
>
> What is their incentive to upstream? They ask me the same thing.
>
>
>
>
>
> > - If it an external library, we might as well add the PMD code as well
> > there and that only 10% of the real stuff.
> > We are not able able to improve anything in this library due to this
> situation.
> >
> > Does anyone care about this PMD? If not, we might as well remove this
> > DPDK and every vendor can manage the external library and external
> > PMD(Situation won't change much)
>
> External PMD is bad.
>
>
>
> It is SHIM layer. I would say external library also bad if it is specific
> to DPDK.
>
>
>
> I think this library should not be specific to DPDK,
>
>
>
> Sadly it is VERY specific to DPDK for doing authentication and encryption
> in one shot to improve the performance. Openssl has already has armv8
> instructions support for doing it as two pass just that performance is not
> good. For use cae such as  IPsec it make sense do authentication and
> encryption in one shot for performance improvement.
>
> *[Honnappa] *I think there is a need for such a library not just for
> DPDK. It would be good if it could do UDP checksum validation for the inner
> packet as well.
>
>
>
> so it would make sense as an external library
>
>
>
> If it an external library, it does NOT make  much sense for Marvell to
> maintain it(No incentive and it is pain due lack of collaboration)
>
>
>
> Either someone need to step up and maintain it if we NOT choose to make it
> as external else we can remove the PMD from dpdk(Makes life easy for
> everyone). I don't want to maintain something not upsteamble nor
> collaboration friendly aka less quality.
>
>
>
> .
>
>
>
>
> > Thoughts from ARM, other ARMv8 vendors or community?
>
>
>
> I have expressed my concerns. If there is no constructive feedback to fix
> the concern. I will plan for submitting a patch to remove the shim crypto
> Armv8 PMD from dpdk by next week.
>
> *[Honnappa] *I do not think there is a need to remove the PMD. As you
> have mentioned, many might have developed their own libraries and may be
> dependent on DPDK Armv8 PMD.
>

Problem with that approach is that, No convergence/collaboration on this
PMD aka no improvement and less quality.

From Arm side, there have been efforts to fix the situation. Some have not
> gone far and some have shown promise, but fell flat. I can say that this is
> still a priority but I am not sure when we will have something.
>

If ARM is ready to take over the maintenance on PMD and external library
then I am fine with any decision.
Let us know. Personally, I don't like to maintain something not upsteamble
friendly.


My suggestion, we should go ahead with adding the meson build for this PMD.
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-10  5:24           ` Jerin Jacob
@ 2019-10-11 19:13             ` Honnappa Nagarahalli
  2019-10-11 20:02               ` Jerin Jacob
  0 siblings, 1 reply; 13+ messages in thread
From: Honnappa Nagarahalli @ 2019-10-11 19:13 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: thomas, jerinj, Dharmik Thakkar, Akhil.goyal@nxp.com,
	hemant.agrawal, anoobj, pathreya, Bruce Richardson, dpdk-dev,
	Honnappa Nagarahalli, nd, nd

<snip>

On Thu, 10 Oct, 2019, 10:17 AM Honnappa Nagarahalli, <Honnappa.Nagarahalli@arm.com<mailto:Honnappa.Nagarahalli@arm.com>> wrote:
<snip>

On Mon, 7 Oct, 2019, 3:49 PM Jerin Jacob, <jerinjacobk@gmail.com<mailto:jerinjacobk@gmail.com>> wrote:

On Sun, 6 Oct, 2019, 11:36 PM Thomas Monjalon, <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:
05/10/2019 17:28, Jerin Jacob:
> On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com<mailto:dharmik.thakkar@arm.com>> wrote:
> >
> > Add new meson.build file for crypto/armv8
> >
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com<mailto:dharmik.thakkar@arm.com>>
> > ---
> >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> >  drivers/crypto/meson.build       |  6 +++---
> >  meson_options.txt                |  2 ++
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/crypto/armv8/meson.build
>
> >
> >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> >         description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
> > +option('armv8_crypto_dir', type: 'string', value: '',
> > +       description: 'path to the armv8_crypto library installation directory')

You should not need such option if you provide a pkg-config file
in your library.


> It is not specific to this patch but it is connected to this patch.
>
> Three years back when Cavium contributed to this driver the situation
> was different where only Cavium was contributing to DPDK and now we
> have multiple vendors from
> ARMv8 platform and ARM itself is contributing it.
>
> When it is submitted, I was not in favor of the external library. But
> various reasons it happened to be the external library where 90% meat
> in this library and shim PMD
> the driver moved to DPDK.
>
> Now, I look back, It does not make sense to the external library. Reasons are
> - It won't allow another ARMv8 player to contribute to this library as
> Marvell owns this repo and there is no upstreaming path to this
> library.

This is a real issue and you are able to fix it.

Note sure how I can fix it and why I need to fix it. I just dont want to start a parallel collaborating infrastructure for DPDK armv8.



> - That made this library to not have 'any' change for the last three
> year and everyone have there owned copy of this driver. In fact the
> library was not compiling for last 2.5 years.
> - AES-NI case it makes sense to have an external library as it is a
> single vendor and it is not specific to DPDK. But in this, It is
> another way around

I don't see how it is different, except it is badly maintained.

It is different because only one company contributing to it. In this case, multiple companies needs to contribute.

The library badly maintained in upstream as there is no incentives to upstream  to external library. I believe each vendor has it own copy of that. At least Some teams in Marvell internally has copy of it.
What is their incentive to upstream? They ask me the same thing.



> - If it an external library, we might as well add the PMD code as well
> there and that only 10% of the real stuff.
> We are not able able to improve anything in this library due to this situation.
>
> Does anyone care about this PMD? If not, we might as well remove this
> DPDK and every vendor can manage the external library and external
> PMD(Situation won't change much)

External PMD is bad.

It is SHIM layer. I would say external library also bad if it is specific to DPDK.

I think this library should not be specific to DPDK,

Sadly it is VERY specific to DPDK for doing authentication and encryption in one shot to improve the performance. Openssl has already has armv8 instructions support for doing it as two pass just that performance is not good. For use cae such as  IPsec it make sense do authentication and encryption in one shot for performance improvement.
[Honnappa] I think there is a need for such a library not just for DPDK. It would be good if it could do UDP checksum validation for the inner packet as well.

so it would make sense as an external library

If it an external library, it does NOT make  much sense for Marvell to maintain it(No incentive and it is pain due lack of collaboration)

Either someone need to step up and maintain it if we NOT choose to make it as external else we can remove the PMD from dpdk(Makes life easy for everyone). I don't want to maintain something not upsteamble nor collaboration friendly aka less quality.

.




> Thoughts from ARM, other ARMv8 vendors or community?

I have expressed my concerns. If there is no constructive feedback to fix the concern. I will plan for submitting a patch to remove the shim crypto Armv8 PMD from dpdk by next week.
[Honnappa] I do not think there is a need to remove the PMD. As you have mentioned, many might have developed their own libraries and may be dependent on DPDK Armv8 PMD.

Problem with that approach is that, No convergence/collaboration on this PMD aka no improvement and less quality.
[Honnappa] Would not removing this fall under ABI/API compatibility? Essentially, DPDK defines how an external Armv8 Crypto library can work with DPDK. Is it possible to remove it considering that there might be users dependent on this?
I agree with you on the improvements (features?), but not sure on quality. For the features that are supported, the quality should be good.

From Arm side, there have been efforts to fix the situation. Some have not gone far and some have shown promise, but fell flat. I can say that this is still a priority but I am not sure when we will have something.

If ARM is ready to take over the maintenance on PMD and external library then I am fine with any decision.
Let us know. Personally, I don't like to maintain something not upsteamble friendly.
[Honnappa] What is the maintenance burden on the PMD? Can you elaborate?
On the external library, I do not think this is the right forum to make a decision. There are channels provided to all our partners to discuss these kind of topics and I think those should be made use of.

My suggestion, we should go ahead with adding the meson build for this PMD.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-11 19:13             ` Honnappa Nagarahalli
@ 2019-10-11 20:02               ` Jerin Jacob
  2019-10-11 20:14                 ` Honnappa Nagarahalli
  0 siblings, 1 reply; 13+ messages in thread
From: Jerin Jacob @ 2019-10-11 20:02 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: Thomas Monjalon, Jerin Jacob, Dharmik Thakkar, Akhil Goyal,
	Hemant Agrawal, anoobj, pathreya, Richardson, Bruce, dpdk-dev,
	nd, prasun.kapoor

On Sat, 12 Oct, 2019, 12:44 AM Honnappa Nagarahalli, <
Honnappa.Nagarahalli@arm.com> wrote:

> <snip>
>
>
>
> On Thu, 10 Oct, 2019, 10:17 AM Honnappa Nagarahalli, <
> Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
>
>
> On Mon, 7 Oct, 2019, 3:49 PM Jerin Jacob, <jerinjacobk@gmail.com> wrote:
>
>
>
> On Sun, 6 Oct, 2019, 11:36 PM Thomas Monjalon, <thomas@monjalon.net>
> wrote:
>
> 05/10/2019 17:28, Jerin Jacob:
> > On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com>
> wrote:
> > >
> > > Add new meson.build file for crypto/armv8
> > >
> > > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > > ---
> > >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> > >  drivers/crypto/meson.build       |  6 +++---
> > >  meson_options.txt                |  2 ++
> > >  3 files changed, 30 insertions(+), 3 deletions(-)
> > >  create mode 100644 drivers/crypto/armv8/meson.build
> >
> > >
> > >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> > >         description: 'allow out-of-range NUMA socket id\'s for
> platforms that don\'t report the value correctly')
> > > +option('armv8_crypto_dir', type: 'string', value: '',
> > > +       description: 'path to the armv8_crypto library installation
> directory')
>
> You should not need such option if you provide a pkg-config file
> in your library.
>
>
> > It is not specific to this patch but it is connected to this patch.
> >
> > Three years back when Cavium contributed to this driver the situation
> > was different where only Cavium was contributing to DPDK and now we
> > have multiple vendors from
> > ARMv8 platform and ARM itself is contributing it.
> >
> > When it is submitted, I was not in favor of the external library. But
> > various reasons it happened to be the external library where 90% meat
> > in this library and shim PMD
> > the driver moved to DPDK.
> >
> > Now, I look back, It does not make sense to the external library.
> Reasons are
> > - It won't allow another ARMv8 player to contribute to this library as
> > Marvell owns this repo and there is no upstreaming path to this
> > library.
>
> This is a real issue and you are able to fix it.
>
>
>
> Note sure how I can fix it and why I need to fix it. I just dont want to
> start a parallel collaborating infrastructure for DPDK armv8.
>
>
>
>
>
> > - That made this library to not have 'any' change for the last three
> > year and everyone have there owned copy of this driver. In fact the
> > library was not compiling for last 2.5 years.
> > - AES-NI case it makes sense to have an external library as it is a
> > single vendor and it is not specific to DPDK. But in this, It is
> > another way around
>
> I don't see how it is different, except it is badly maintained.
>
>
>
> It is different because only one company contributing to it. In this case,
> multiple companies needs to contribute.
>
>
>
> The library badly maintained in upstream as there is no incentives to
> upstream  to external library. I believe each vendor has it own copy of
> that. At least Some teams in Marvell internally has copy of it.
>
> What is their incentive to upstream? They ask me the same thing.
>
>
>
>
>
> > - If it an external library, we might as well add the PMD code as well
> > there and that only 10% of the real stuff.
> > We are not able able to improve anything in this library due to this
> situation.
> >
> > Does anyone care about this PMD? If not, we might as well remove this
> > DPDK and every vendor can manage the external library and external
> > PMD(Situation won't change much)
>
> External PMD is bad.
>
>
>
> It is SHIM layer. I would say external library also bad if it is specific
> to DPDK.
>
>
>
> I think this library should not be specific to DPDK,
>
>
>
> Sadly it is VERY specific to DPDK for doing authentication and encryption
> in one shot to improve the performance. Openssl has already has armv8
> instructions support for doing it as two pass just that performance is not
> good. For use cae such as  IPsec it make sense do authentication and
> encryption in one shot for performance improvement.
>
> *[Honnappa] *I think there is a need for such a library not just for
> DPDK. It would be good if it could do UDP checksum validation for the inner
> packet as well.
>
>
>
> so it would make sense as an external library
>
>
>
> If it an external library, it does NOT make  much sense for Marvell to
> maintain it(No incentive and it is pain due lack of collaboration)
>
>
>
> Either someone need to step up and maintain it if we NOT choose to make it
> as external else we can remove the PMD from dpdk(Makes life easy for
> everyone). I don't want to maintain something not upsteamble nor
> collaboration friendly aka less quality.
>
>
>
> .
>
>
>
>
> > Thoughts from ARM, other ARMv8 vendors or community?
>
>
>
> I have expressed my concerns. If there is no constructive feedback to fix
> the concern. I will plan for submitting a patch to remove the shim crypto
> Armv8 PMD from dpdk by next week.
>
> *[Honnappa] *I do not think there is a need to remove the PMD. As you
> have mentioned, many might have developed their own libraries and may be
> dependent on DPDK Armv8 PMD.
>
>
>
> Problem with that approach is that, No convergence/collaboration on this
> PMD aka no improvement and less quality.
>
> *[Honnappa] *Would not removing this fall under ABI/API compatibility?
> Essentially, DPDK defines how an external Armv8 Crypto library can work
> with DPDK. Is it possible to remove it considering that there might be
> users dependent on this?
>
> I agree with you on the improvements (features?), but not sure on quality.
> For the features that are supported, the quality should be good.
>

The library was broken for last 2.5 years. Is that the high quality and no
improvement for last 3 year and no single contribution otherthan Marvell in
external library.


>
> From Arm side, there have been efforts to fix the situation. Some have not
> gone far and some have shown promise, but fell flat. I can say that this is
> still a priority but I am not sure when we will have something.
>
>
>
> If ARM is ready to take over the maintenance on PMD and external library
> then I am fine with any decision.
>
> Let us know. Personally, I don't like to maintain something not upsteamble
> friendly.
>
> *[Honnappa] *What is the maintenance burden on the PMD? Can you elaborate?
>

Marvell open-source policy is bit different than cavium policy. We can not
contribute to GitHub repository with out approval. The existing external
library, not belongs to Marvell GitHub domain. I need to create a case to
add new GitHub repo under Marvell to contribute to all armv8 partners. I
don't have justification for that to legal. We have approvals to contribute
to dpdk.org

On the external library, I do not think this is the right forum to make a
> decision. There are channels provided to all our partners to discuss these
> kind of topics and I think those should be made use of.
>

It is cavium created library for dpdk. Why we need to discuss in some other
channel. I believe this is the correct forum for dpdk discussions.

For example, Dharmik got comment to update the external library to support
autoconfig for meson. What is the path for Dharmik to do that?


Don't you think, you need have access to the complete code base to
contribute. That's the reason why I am saying remove the external library
and have it in DPDK so that everyone can contribute and improve.

If you think, otherwise please take over the maintenance keeping initial
author credit. If you need time to take decision that makes sense. You can
share the ETA. Otherwise, this discussion going in circles.




>
> My suggestion, we should go ahead with adding the meson build for this PMD.
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-11 20:02               ` Jerin Jacob
@ 2019-10-11 20:14                 ` Honnappa Nagarahalli
  2019-10-11 20:33                   ` Jerin Jacob
  0 siblings, 1 reply; 13+ messages in thread
From: Honnappa Nagarahalli @ 2019-10-11 20:14 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: thomas, jerinj, Dharmik Thakkar, Akhil.goyal@nxp.com,
	hemant.agrawal, anoobj, pathreya, Richardson, Bruce, dpdk-dev,
	Honnappa Nagarahalli, nd, prasun.kapoor, nd

On Sat, 12 Oct, 2019, 12:44 AM Honnappa Nagarahalli, <Honnappa.Nagarahalli@arm.com<mailto:Honnappa.Nagarahalli@arm.com>> wrote:
<snip>

On Thu, 10 Oct, 2019, 10:17 AM Honnappa Nagarahalli, <Honnappa.Nagarahalli@arm.com<mailto:Honnappa.Nagarahalli@arm.com>> wrote:
<snip>

On Mon, 7 Oct, 2019, 3:49 PM Jerin Jacob, <jerinjacobk@gmail.com<mailto:jerinjacobk@gmail.com>> wrote:

On Sun, 6 Oct, 2019, 11:36 PM Thomas Monjalon, <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:
05/10/2019 17:28, Jerin Jacob:
> On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com<mailto:dharmik.thakkar@arm.com>> wrote:
> >
> > Add new meson.build file for crypto/armv8
> >
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com<mailto:dharmik.thakkar@arm.com>>
> > ---
> >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> >  drivers/crypto/meson.build       |  6 +++---
> >  meson_options.txt                |  2 ++
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/crypto/armv8/meson.build
>
> >
> >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> >         description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
> > +option('armv8_crypto_dir', type: 'string', value: '',
> > +       description: 'path to the armv8_crypto library installation directory')

You should not need such option if you provide a pkg-config file
in your library.


> It is not specific to this patch but it is connected to this patch.
>
> Three years back when Cavium contributed to this driver the situation
> was different where only Cavium was contributing to DPDK and now we
> have multiple vendors from
> ARMv8 platform and ARM itself is contributing it.
>
> When it is submitted, I was not in favor of the external library. But
> various reasons it happened to be the external library where 90% meat
> in this library and shim PMD
> the driver moved to DPDK.
>
> Now, I look back, It does not make sense to the external library. Reasons are
> - It won't allow another ARMv8 player to contribute to this library as
> Marvell owns this repo and there is no upstreaming path to this
> library.

This is a real issue and you are able to fix it.

Note sure how I can fix it and why I need to fix it. I just dont want to start a parallel collaborating infrastructure for DPDK armv8.



> - That made this library to not have 'any' change for the last three
> year and everyone have there owned copy of this driver. In fact the
> library was not compiling for last 2.5 years.
> - AES-NI case it makes sense to have an external library as it is a
> single vendor and it is not specific to DPDK. But in this, It is
> another way around

I don't see how it is different, except it is badly maintained.

It is different because only one company contributing to it. In this case, multiple companies needs to contribute.

The library badly maintained in upstream as there is no incentives to upstream  to external library. I believe each vendor has it own copy of that. At least Some teams in Marvell internally has copy of it.
What is their incentive to upstream? They ask me the same thing.



> - If it an external library, we might as well add the PMD code as well
> there and that only 10% of the real stuff.
> We are not able able to improve anything in this library due to this situation.
>
> Does anyone care about this PMD? If not, we might as well remove this
> DPDK and every vendor can manage the external library and external
> PMD(Situation won't change much)

External PMD is bad.

It is SHIM layer. I would say external library also bad if it is specific to DPDK.

I think this library should not be specific to DPDK,

Sadly it is VERY specific to DPDK for doing authentication and encryption in one shot to improve the performance. Openssl has already has armv8 instructions support for doing it as two pass just that performance is not good. For use cae such as  IPsec it make sense do authentication and encryption in one shot for performance improvement.
[Honnappa] I think there is a need for such a library not just for DPDK. It would be good if it could do UDP checksum validation for the inner packet as well.

so it would make sense as an external library

If it an external library, it does NOT make  much sense for Marvell to maintain it(No incentive and it is pain due lack of collaboration)

Either someone need to step up and maintain it if we NOT choose to make it as external else we can remove the PMD from dpdk(Makes life easy for everyone). I don't want to maintain something not upsteamble nor collaboration friendly aka less quality.

.




> Thoughts from ARM, other ARMv8 vendors or community?

I have expressed my concerns. If there is no constructive feedback to fix the concern. I will plan for submitting a patch to remove the shim crypto Armv8 PMD from dpdk by next week.
[Honnappa] I do not think there is a need to remove the PMD. As you have mentioned, many might have developed their own libraries and may be dependent on DPDK Armv8 PMD.

Problem with that approach is that, No convergence/collaboration on this PMD aka no improvement and less quality.
[Honnappa] Would not removing this fall under ABI/API compatibility? Essentially, DPDK defines how an external Armv8 Crypto library can work with DPDK. Is it possible to remove it considering that there might be users dependent on this?
I agree with you on the improvements (features?), but not sure on quality. For the features that are supported, the quality should be good.

The library was broken for last 2.5 years. Is that the high quality and no improvement for last 3 year and no single contribution otherthan Marvell in external library.
[Honnappa] We need to separate the discussion about PMD and the external library. IMO, PMD cannot be removed as some might be using the interfaces with their own crypto library.

From Arm side, there have been efforts to fix the situation. Some have not gone far and some have shown promise, but fell flat. I can say that this is still a priority but I am not sure when we will have something.

If ARM is ready to take over the maintenance on PMD and external library then I am fine with any decision.
Let us know. Personally, I don't like to maintain something not upsteamble friendly.
[Honnappa] What is the maintenance burden on the PMD? Can you elaborate?

Marvell open-source policy is bit different than cavium policy. We can not contribute to GitHub repository with out approval. The existing external library, not belongs to Marvell GitHub domain. I need to create a case to add new GitHub repo under Marvell to contribute to all armv8 partners. I don't have justification for that to legal. We have approvals to contribute to dpdk.org<http://dpdk.org>

On the external library, I do not think this is the right forum to make a decision. There are channels provided to all our partners to discuss these kind of topics and I think those should be made use of.
It is cavium created library for dpdk. Why we need to discuss in some other channel. I believe this is the correct forum for dpdk discussions.
[Honnappa] May be I was not clear, please see my comment below.

For example, Dharmik got comment to update the external library to support autoconfig for meson. What is the path for Dharmik to do that?
[Honnappa] Is this mainly from testing purposes?

Don't you think, you need have access to the complete code base to contribute. That's the reason why I am saying remove the external library and have it in DPDK so that everyone can contribute and improve.
[Honnappa] I don’t have any issues

If you think, otherwise please take over the maintenance keeping initial author credit. If you need time to take decision that makes sense. You can share the ETA. Otherwise, this discussion going in circles.
[Honnappa] This cannot be decided in this forum.


My suggestion, we should go ahead with adding the meson build for this PMD.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-11 20:14                 ` Honnappa Nagarahalli
@ 2019-10-11 20:33                   ` Jerin Jacob
  0 siblings, 0 replies; 13+ messages in thread
From: Jerin Jacob @ 2019-10-11 20:33 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: Thomas Monjalon, Jerin Jacob, Dharmik Thakkar,
	Akhil.goyal@nxp.com, hemant.agrawal, anoobj, pathreya,
	Richardson, Bruce, dpdk-dev, nd, prasun.kapoor

On Sat, 12 Oct, 2019, 1:44 AM Honnappa Nagarahalli, <
Honnappa.Nagarahalli@arm.com> wrote:

> On Sat, 12 Oct, 2019, 12:44 AM Honnappa Nagarahalli, <
> Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
>
>
> On Thu, 10 Oct, 2019, 10:17 AM Honnappa Nagarahalli, <
> Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
>
>
> On Mon, 7 Oct, 2019, 3:49 PM Jerin Jacob, <jerinjacobk@gmail.com> wrote:
>
>
>
> On Sun, 6 Oct, 2019, 11:36 PM Thomas Monjalon, <thomas@monjalon.net>
> wrote:
>
> 05/10/2019 17:28, Jerin Jacob:
> > On Fri, Oct 4, 2019 at 4:27 AM Dharmik Thakkar <dharmik.thakkar@arm.com>
> wrote:
> > >
> > > Add new meson.build file for crypto/armv8
> > >
> > > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > > ---
> > >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> > >  drivers/crypto/meson.build       |  6 +++---
> > >  meson_options.txt                |  2 ++
> > >  3 files changed, 30 insertions(+), 3 deletions(-)
> > >  create mode 100644 drivers/crypto/armv8/meson.build
> >
> > >
> > >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> > >         description: 'allow out-of-range NUMA socket id\'s for
> platforms that don\'t report the value correctly')
> > > +option('armv8_crypto_dir', type: 'string', value: '',
> > > +       description: 'path to the armv8_crypto library installation
> directory')
>
> You should not need such option if you provide a pkg-config file
> in your library.
>
>
> > It is not specific to this patch but it is connected to this patch.
> >
> > Three years back when Cavium contributed to this driver the situation
> > was different where only Cavium was contributing to DPDK and now we
> > have multiple vendors from
> > ARMv8 platform and ARM itself is contributing it.
> >
> > When it is submitted, I was not in favor of the external library. But
> > various reasons it happened to be the external library where 90% meat
> > in this library and shim PMD
> > the driver moved to DPDK.
> >
> > Now, I look back, It does not make sense to the external library.
> Reasons are
> > - It won't allow another ARMv8 player to contribute to this library as
> > Marvell owns this repo and there is no upstreaming path to this
> > library.
>
> This is a real issue and you are able to fix it.
>
>
>
> Note sure how I can fix it and why I need to fix it. I just dont want to
> start a parallel collaborating infrastructure for DPDK armv8.
>
>
>
>
>
> > - That made this library to not have 'any' change for the last three
> > year and everyone have there owned copy of this driver. In fact the
> > library was not compiling for last 2.5 years.
> > - AES-NI case it makes sense to have an external library as it is a
> > single vendor and it is not specific to DPDK. But in this, It is
> > another way around
>
> I don't see how it is different, except it is badly maintained.
>
>
>
> It is different because only one company contributing to it. In this case,
> multiple companies needs to contribute.
>
>
>
> The library badly maintained in upstream as there is no incentives to
> upstream  to external library. I believe each vendor has it own copy of
> that. At least Some teams in Marvell internally has copy of it.
>
> What is their incentive to upstream? They ask me the same thing.
>
>
>
>
>
> > - If it an external library, we might as well add the PMD code as well
> > there and that only 10% of the real stuff.
> > We are not able able to improve anything in this library due to this
> situation.
> >
> > Does anyone care about this PMD? If not, we might as well remove this
> > DPDK and every vendor can manage the external library and external
> > PMD(Situation won't change much)
>
> External PMD is bad.
>
>
>
> It is SHIM layer. I would say external library also bad if it is specific
> to DPDK.
>
>
>
> I think this library should not be specific to DPDK,
>
>
>
> Sadly it is VERY specific to DPDK for doing authentication and encryption
> in one shot to improve the performance. Openssl has already has armv8
> instructions support for doing it as two pass just that performance is not
> good. For use cae such as  IPsec it make sense do authentication and
> encryption in one shot for performance improvement.
>
> *[Honnappa] *I think there is a need for such a library not just for
> DPDK. It would be good if it could do UDP checksum validation for the inner
> packet as well.
>
>
>
> so it would make sense as an external library
>
>
>
> If it an external library, it does NOT make  much sense for Marvell to
> maintain it(No incentive and it is pain due lack of collaboration)
>
>
>
> Either someone need to step up and maintain it if we NOT choose to make it
> as external else we can remove the PMD from dpdk(Makes life easy for
> everyone). I don't want to maintain something not upsteamble nor
> collaboration friendly aka less quality.
>
>
>
> .
>
>
>
>
> > Thoughts from ARM, other ARMv8 vendors or community?
>
>
>
> I have expressed my concerns. If there is no constructive feedback to fix
> the concern. I will plan for submitting a patch to remove the shim crypto
> Armv8 PMD from dpdk by next week.
>
> *[Honnappa] *I do not think there is a need to remove the PMD. As you
> have mentioned, many might have developed their own libraries and may be
> dependent on DPDK Armv8 PMD.
>
>
>
> Problem with that approach is that, No convergence/collaboration on this
> PMD aka no improvement and less quality.
>
> *[Honnappa] *Would not removing this fall under ABI/API compatibility?
> Essentially, DPDK defines how an external Armv8 Crypto library can work
> with DPDK. Is it possible to remove it considering that there might be
> users dependent on this?
>
> I agree with you on the improvements (features?), but not sure on quality.
> For the features that are supported, the quality should be good.
>
>
>
> The library was broken for last 2.5 years. Is that the high quality and no
> improvement for last 3 year and no single contribution otherthan Marvell in
> external library.
>
> *[Honnappa] *We need to separate the discussion about PMD and the
> external library. IMO, PMD cannot be removed as some might be using the
> interfaces with their own crypto library.
>

Multiple libraries for same job. That's same thing I would like to avoid.
And the PMD does not exist without external library. If some has their own
crypto library then please update in the documentation so that others can
use it. It is not open-source way of doing the stuff. Else I need assume no
one is using the PMD.



>
> From Arm side, there have been efforts to fix the situation. Some have not
> gone far and some have shown promise, but fell flat. I can say that this is
> still a priority but I am not sure when we will have something.
>
>
>
> If ARM is ready to take over the maintenance on PMD and external library
> then I am fine with any decision.
>
> Let us know. Personally, I don't like to maintain something not upsteamble
> friendly.
>
> *[Honnappa] *What is the maintenance burden on the PMD? Can you elaborate?
>
>
>
> Marvell open-source policy is bit different than cavium policy. We can not
> contribute to GitHub repository with out approval. The existing external
> library, not belongs to Marvell GitHub domain. I need to create a case to
> add new GitHub repo under Marvell to contribute to all armv8 partners. I
> don't have justification for that to legal. We have approvals to contribute
> to dpdk.org
>
>
>
> On the external library, I do not think this is the right forum to make a
> decision. There are channels provided to all our partners to discuss these
> kind of topics and I think those should be made use of.
>
> It is cavium created library for dpdk. Why we need to discuss in some
> other channel. I believe this is the correct forum for dpdk discussions.
>
> *[Honnappa] *May be I was not clear, please see my comment below.
>
>
>
> For example, Dharmik got comment to update the external library to
> support autoconfig for meson. What is the path for Dharmik to do that?
>
> *[Honnappa] *Is this mainly from testing purposes?
>
>
>

Not for testing. See Thomas comment

Don't you think, you need have access to the complete code base to
> contribute. That's the reason why I am saying remove the external library
> and have it in DPDK so that everyone can contribute and improve.
>
> *[Honnappa] *I don’t have any issues
>

But DPDK does have support for that.



>
> If you think, otherwise please take over the maintenance keeping initial
> author credit. If you need time to take decision that makes sense. You can
> share the ETA. Otherwise, this discussion going in circles.
>
> *[Honnappa] *This cannot be decided in this forum.
>
>
>

Then please start the discussion with the form you think it as appropriate.


>
> My suggestion, we should go ahead with adding the meson build for this PMD.
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-10-03 22:57 [dpdk-dev] [PATCH] crypto/armv8: enable meson build Dharmik Thakkar
  2019-10-05 15:28 ` Jerin Jacob
@ 2019-11-18  7:49 ` Akhil Goyal
  2019-11-20  4:41   ` Honnappa Nagarahalli
  1 sibling, 1 reply; 13+ messages in thread
From: Akhil Goyal @ 2019-11-18  7:49 UTC (permalink / raw)
  To: Dharmik Thakkar, Jerin Jacob, Bruce Richardson
  Cc: dev, honnappa.nagarahalli, thomas, Ruifeng Wang, gavin.hu

Hi Dharmik,

As per the recent communication with Honnappa on a separate mail chain, ARM has agreed to 
Host the armv8_crypto library. If it is happening in 19.11 timeframe, we should rebase this patch
And add documentation for changed location of the repo and if it is not in 19.11 timeframe we should have a patch to disable the PMD.

Could you please send patches as are applicable?

Regards,
Akhil

> Add new meson.build file for crypto/armv8
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> ---
>  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
>  drivers/crypto/meson.build       |  6 +++---
>  meson_options.txt                |  2 ++
>  3 files changed, 30 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/crypto/armv8/meson.build
> 
> diff --git a/drivers/crypto/armv8/meson.build
> b/drivers/crypto/armv8/meson.build
> new file mode 100644
> index 000000000000..1ef78fa5d8c7
> --- /dev/null
> +++ b/drivers/crypto/armv8/meson.build
> @@ -0,0 +1,25 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2019 Arm Limited
> +
> +path = get_option('armv8_crypto_dir')
> +if path == ''
> +	build = false
> +	reason = 'missing dependency, "armv8_crypto"'
> +	subdir_done()
> +endif
> +
> +inc_dir = path + '/asm/include'
> +
> +lib = cc.find_library('libarmv8_crypto', dirs: [path], required: false)
> +if not lib.found()
> +	build = false
> +	reason = 'missing dependency, "armv8_crypto"'
> +	subdir_done()
> +else
> +	ext_deps += lib
> +	includes += include_directories(inc_dir)
> +endif
> +
> +deps += ['bus_vdev']
> +sources = files('rte_armv8_pmd.c', 'rte_armv8_pmd_ops.c')
> +allow_experimental_apis = true
> diff --git a/drivers/crypto/meson.build b/drivers/crypto/meson.build
> index 83e78860ebee..605dcdd5f4d6 100644
> --- a/drivers/crypto/meson.build
> +++ b/drivers/crypto/meson.build
> @@ -1,9 +1,9 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2017 Intel Corporation
> 
> -drivers = ['aesni_gcm', 'aesni_mb', 'caam_jr', 'ccp', 'dpaa_sec', 'dpaa2_sec',
> -	'kasumi', 'mvsam', 'null', 'octeontx', 'openssl', 'qat', 'scheduler',
> -	'snow3g', 'virtio', 'zuc']
> +drivers = ['aesni_gcm', 'aesni_mb', 'armv8', 'caam_jr', 'ccp', 'dpaa_sec',
> +	'dpaa2_sec', 'kasumi', 'mvsam', 'null', 'octeontx', 'openssl', 'qat',
> +	'scheduler', 'snow3g', 'virtio', 'zuc']
> 
>  std_deps = ['cryptodev'] # cryptodev pulls in all other needed deps
>  config_flag_fmt = 'RTE_LIBRTE_@0@_PMD'
> diff --git a/meson_options.txt b/meson_options.txt
> index 448f3e63dcf2..4c0413918a34 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -2,6 +2,8 @@
> 
>  option('allow_invalid_socket_id', type: 'boolean', value: false,
>  	description: 'allow out-of-range NUMA socket id\'s for platforms that
> don\'t report the value correctly')
> +option('armv8_crypto_dir', type: 'string', value: '',
> +	description: 'path to the armv8_crypto library installation directory')
>  option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
>  	description: 'Subdirectory of libdir where to install PMDs. Defaults to
> using a versioned subdirectory.')
>  option('enable_docs', type: 'boolean', value: false,
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] crypto/armv8: enable meson build
  2019-11-18  7:49 ` Akhil Goyal
@ 2019-11-20  4:41   ` Honnappa Nagarahalli
  0 siblings, 0 replies; 13+ messages in thread
From: Honnappa Nagarahalli @ 2019-11-20  4:41 UTC (permalink / raw)
  To: Akhil.goyal@nxp.com, Dharmik Thakkar, jerinj, Bruce Richardson
  Cc: dev, thomas, Ruifeng Wang (Arm Technology China),
	Gavin Hu (Arm Technology China),
	nd, nd

<snip>

> 
> Hi Dharmik,
> 
> As per the recent communication with Honnappa on a separate mail chain,
> ARM has agreed to Host the armv8_crypto library. If it is happening in 19.11
To the wider audience, yes Arm will host a crypto library which is an existing repo on Arm github. We will pull the Marvell's code into this repo. However, Marvell's code needs changes to be integrated into the existing library. It also needs to go through a security review. We will not be able to complete this work for 19.11. We are targeting 20.02 as of now.

> timeframe, we should rebase this patch And add documentation for changed
> location of the repo and if it is not in 19.11 timeframe we should have a
> patch to disable the PMD.
The PMD is already disabled by default.
IMO, we just need the changes to release notes indicating that the support of the Marvel's crypto library is no longer available.

> 
> Could you please send patches as are applicable?
> 
> Regards,
> Akhil
> 
> > Add new meson.build file for crypto/armv8
> >
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > ---
> >  drivers/crypto/armv8/meson.build | 25 +++++++++++++++++++++++++
> >  drivers/crypto/meson.build       |  6 +++---
> >  meson_options.txt                |  2 ++
> >  3 files changed, 30 insertions(+), 3 deletions(-)  create mode 100644
> > drivers/crypto/armv8/meson.build
> >
> > diff --git a/drivers/crypto/armv8/meson.build
> > b/drivers/crypto/armv8/meson.build
> > new file mode 100644
> > index 000000000000..1ef78fa5d8c7
> > --- /dev/null
> > +++ b/drivers/crypto/armv8/meson.build
> > @@ -0,0 +1,25 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm
> > +Limited
> > +
> > +path = get_option('armv8_crypto_dir') if path == ''
> > +	build = false
> > +	reason = 'missing dependency, "armv8_crypto"'
> > +	subdir_done()
> > +endif
> > +
> > +inc_dir = path + '/asm/include'
> > +
> > +lib = cc.find_library('libarmv8_crypto', dirs: [path], required:
> > +false) if not lib.found()
> > +	build = false
> > +	reason = 'missing dependency, "armv8_crypto"'
> > +	subdir_done()
> > +else
> > +	ext_deps += lib
> > +	includes += include_directories(inc_dir) endif
> > +
> > +deps += ['bus_vdev']
> > +sources = files('rte_armv8_pmd.c', 'rte_armv8_pmd_ops.c')
> > +allow_experimental_apis = true
> > diff --git a/drivers/crypto/meson.build b/drivers/crypto/meson.build
> > index 83e78860ebee..605dcdd5f4d6 100644
> > --- a/drivers/crypto/meson.build
> > +++ b/drivers/crypto/meson.build
> > @@ -1,9 +1,9 @@
> >  # SPDX-License-Identifier: BSD-3-Clause  # Copyright(c) 2017 Intel
> > Corporation
> >
> > -drivers = ['aesni_gcm', 'aesni_mb', 'caam_jr', 'ccp', 'dpaa_sec', 'dpaa2_sec',
> > -	'kasumi', 'mvsam', 'null', 'octeontx', 'openssl', 'qat', 'scheduler',
> > -	'snow3g', 'virtio', 'zuc']
> > +drivers = ['aesni_gcm', 'aesni_mb', 'armv8', 'caam_jr', 'ccp', 'dpaa_sec',
> > +	'dpaa2_sec', 'kasumi', 'mvsam', 'null', 'octeontx', 'openssl', 'qat',
> > +	'scheduler', 'snow3g', 'virtio', 'zuc']
> >
> >  std_deps = ['cryptodev'] # cryptodev pulls in all other needed deps
> > config_flag_fmt = 'RTE_LIBRTE_@0@_PMD'
> > diff --git a/meson_options.txt b/meson_options.txt index
> > 448f3e63dcf2..4c0413918a34 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -2,6 +2,8 @@
> >
> >  option('allow_invalid_socket_id', type: 'boolean', value: false,
> >  	description: 'allow out-of-range NUMA socket id\'s for platforms
> > that don\'t report the value correctly')
> > +option('armv8_crypto_dir', type: 'string', value: '',
> > +	description: 'path to the armv8_crypto library installation
> > +directory')
> >  option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
> >  	description: 'Subdirectory of libdir where to install PMDs. Defaults
> > to using a versioned subdirectory.')  option('enable_docs', type:
> > 'boolean', value: false,
> > --
> > 2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-11-20  4:41 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 22:57 [dpdk-dev] [PATCH] crypto/armv8: enable meson build Dharmik Thakkar
2019-10-05 15:28 ` Jerin Jacob
2019-10-06 18:06   ` Thomas Monjalon
2019-10-07 10:19     ` Jerin Jacob
2019-10-08  7:18       ` Jerin Jacob
2019-10-10  4:46         ` Honnappa Nagarahalli
2019-10-10  5:24           ` Jerin Jacob
2019-10-11 19:13             ` Honnappa Nagarahalli
2019-10-11 20:02               ` Jerin Jacob
2019-10-11 20:14                 ` Honnappa Nagarahalli
2019-10-11 20:33                   ` Jerin Jacob
2019-11-18  7:49 ` Akhil Goyal
2019-11-20  4:41   ` Honnappa Nagarahalli

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).