[dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Ayuj Verma]

rte_crypto_sym.h is included prior to rte_crypto_asym.h
in rte_crypto.h, which breaks alphabetical order.

include rte_crypto_sym.h in rte_crypto_asym.h fixes this.

Ayuj Verma (1):
  lib/crypto: fix alphabetical ordering of headers

 lib/librte_cryptodev/rte_crypto.h      | 1 -
 lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Ayuj Verma]

rte_crypto_sym.h is included prior to rte_crypto_asym.h
in rte_crypto.h, which breaks alphabetical order.

include rte_crypto_sym.h in rte_crypto_asym.h fixes this.

Ayuj Verma (1):
  lib/crypto: fix alphabetical ordering of headers

 lib/librte_cryptodev/rte_crypto.h      | 1 -
 lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Ayuj Verma]

include rte_crypto_sym.h in rte_crypto_asym.h in place
of including it in rte_crypto.h.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>

---
 lib/librte_cryptodev/rte_crypto.h      | 1 -
 lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
index fd5ef3a..17dccdf 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -22,7 +22,6 @@
 #include <rte_mempool.h>
 #include <rte_common.h>
 
-#include "rte_crypto_sym.h"
 #include "rte_crypto_asym.h"
 
 /** Crypto operation types */
diff --git a/lib/librte_cryptodev/rte_crypto_asym.h b/lib/librte_cryptodev/rte_crypto_asym.h
index 5e43620..a55923a 100644
--- a/lib/librte_cryptodev/rte_crypto_asym.h
+++ b/lib/librte_cryptodev/rte_crypto_asym.h
@@ -25,6 +25,8 @@
 #include <rte_mempool.h>
 #include <rte_common.h>
 
+#include "rte_crypto_sym.h"
+
 typedef struct rte_crypto_param_t {
 	uint8_t *data;
 	/**< pointer to buffer holding data */
-- 
1.8.3.1

[dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Ayuj Verma]

include rte_crypto_sym.h in rte_crypto_asym.h in place
of including it in rte_crypto.h.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>

---
 lib/librte_cryptodev/rte_crypto.h      | 1 -
 lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
index fd5ef3a..17dccdf 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -22,7 +22,6 @@
 #include <rte_mempool.h>
 #include <rte_common.h>
 
-#include "rte_crypto_sym.h"
 #include "rte_crypto_asym.h"
 
 /** Crypto operation types */
diff --git a/lib/librte_cryptodev/rte_crypto_asym.h b/lib/librte_cryptodev/rte_crypto_asym.h
index 5e43620..a55923a 100644
--- a/lib/librte_cryptodev/rte_crypto_asym.h
+++ b/lib/librte_cryptodev/rte_crypto_asym.h
@@ -25,6 +25,8 @@
 #include <rte_mempool.h>
 #include <rte_common.h>
 
+#include "rte_crypto_sym.h"
+
 typedef struct rte_crypto_param_t {
 	uint8_t *data;
 	/**< pointer to buffer holding data */
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Trahe, Fiona]

Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Friday, April 12, 2019 8:03 AM
> To: akhil.goyal@nxp.com; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>
> Cc: shallyv@marvell.com; ssahu@marvell.com; kkotamarthy@marvell.com; adesai@marvell.com;
> dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> Subject: [PATCH v1] fix alphabetical ordering of headers
> 
> rte_crypto_sym.h is included prior to rte_crypto_asym.h
> in rte_crypto.h, which breaks alphabetical order.
> 
> include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
[Fiona] I presume you tried just swapping the order and it broke?
If something in rte_crypto_asym.h depends on something from rte_crypto_sym.h, it 
probably shouldn't. What's the dependency and can/should it be moved to rte_crypto.h?

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Trahe, Fiona]

Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Friday, April 12, 2019 8:03 AM
> To: akhil.goyal@nxp.com; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>
> Cc: shallyv@marvell.com; ssahu@marvell.com; kkotamarthy@marvell.com; adesai@marvell.com;
> dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> Subject: [PATCH v1] fix alphabetical ordering of headers
> 
> rte_crypto_sym.h is included prior to rte_crypto_asym.h
> in rte_crypto.h, which breaks alphabetical order.
> 
> include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
[Fiona] I presume you tried just swapping the order and it broke?
If something in rte_crypto_asym.h depends on something from rte_crypto_sym.h, it 
probably shouldn't. What's the dependency and can/should it be moved to rte_crypto.h?

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Ayuj Verma]

Hi Fiona,


Sorry for delay in response.

Please see inline.


Thanks and regards

Ayuj Verma


________________________________
From: Trahe, Fiona <fiona.trahe@intel.com>
Sent: 12 April 2019 20:47
To: Ayuj Verma; akhil.goyal@nxp.com; Kusztal, ArkadiuszX
Cc: Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org; Trahe, Fiona
Subject: RE: [PATCH v1] fix alphabetical ordering of headers

Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Friday, April 12, 2019 8:03 AM
> To: akhil.goyal@nxp.com; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>
> Cc: shallyv@marvell.com; ssahu@marvell.com; kkotamarthy@marvell.com; adesai@marvell.com;
> dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> Subject: [PATCH v1] fix alphabetical ordering of headers
>
> rte_crypto_sym.h is included prior to rte_crypto_asym.h
> in rte_crypto.h, which breaks alphabetical order.
>
> include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
[Fiona] I presume you tried just swapping the order and it broke?
If something in rte_crypto_asym.h depends on something from rte_crypto_sym.h, it
probably shouldn't. What's the dependency and can/should it be moved to rte_crypto.h?
[Ayuj] it is enum rte_crypto_auth_algorithm which it import from sym.h. So do you suggest to move it to rte_crypto.h?

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Ayuj Verma]

Hi Fiona,


Sorry for delay in response.

Please see inline.


Thanks and regards

Ayuj Verma


________________________________
From: Trahe, Fiona <fiona.trahe@intel.com>
Sent: 12 April 2019 20:47
To: Ayuj Verma; akhil.goyal@nxp.com; Kusztal, ArkadiuszX
Cc: Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org; Trahe, Fiona
Subject: RE: [PATCH v1] fix alphabetical ordering of headers

Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Friday, April 12, 2019 8:03 AM
> To: akhil.goyal@nxp.com; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>
> Cc: shallyv@marvell.com; ssahu@marvell.com; kkotamarthy@marvell.com; adesai@marvell.com;
> dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> Subject: [PATCH v1] fix alphabetical ordering of headers
>
> rte_crypto_sym.h is included prior to rte_crypto_asym.h
> in rte_crypto.h, which breaks alphabetical order.
>
> include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
[Fiona] I presume you tried just swapping the order and it broke?
If something in rte_crypto_asym.h depends on something from rte_crypto_sym.h, it
probably shouldn't. What's the dependency and can/should it be moved to rte_crypto.h?
[Ayuj] it is enum rte_crypto_auth_algorithm which it import from sym.h. So do you suggest to move it to rte_crypto.h?

Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Anoob Joseph]

Hi Ayuj, Akhil, Fiona, Shally,

I think there are couple of issues with this patch. The real issue here is the dependency of rte_crypto_asym.h on rte_crypto_sym.h. If rte_crypto_asym.h doesn't include all the headers it uses, every file which includes rte_crypto_asym.h will have to include rte_crypto_asym.h's dependencies, which is not the right practice. Keeping it alphabetical etc comes later.

So the patch has to be rephrased to better reflect the purpose.

Also please see inline.

Thanks,
Anoob
 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ayuj Verma
> Sent: Friday, April 12, 2019 12:33 PM
> To: akhil.goyal@nxp.com; arkadiuszx.kusztal@intel.com;
> fiona.trahe@intel.com
> Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
> Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Arvind Desai
> <adesai@marvell.com>; dev@dpdk.org; Ayuj Verma
> <ayverma@marvell.com>
> Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of
> headers
> 
> include rte_crypto_sym.h in rte_crypto_asym.h in place of including it in
> rte_crypto.h.
> 
> Signed-off-by: Ayuj Verma <ayverma@marvell.com>
> Signed-off-by: Shally Verma <shallyv@marvell.com>
> 
> ---
>  lib/librte_cryptodev/rte_crypto.h      | 1 -
>  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto.h
> b/lib/librte_cryptodev/rte_crypto.h
> index fd5ef3a..17dccdf 100644
> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -22,7 +22,6 @@
>  #include <rte_mempool.h>
>  #include <rte_common.h>
> 
> -#include "rte_crypto_sym.h"
>  #include "rte_crypto_asym.h"

[Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h, then you cannot remove the include. You can make it alphabetical, but the include has to be retained.

> 
>  /** Crypto operation types */
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 5e43620..a55923a 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -25,6 +25,8 @@
>  #include <rte_mempool.h>
>  #include <rte_common.h>
> 
> +#include "rte_crypto_sym.h"
> +
>  typedef struct rte_crypto_param_t {
>  	uint8_t *data;
>  	/**< pointer to buffer holding data */
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Anoob Joseph]

Hi Ayuj, Akhil, Fiona, Shally,

I think there are couple of issues with this patch. The real issue here is the dependency of rte_crypto_asym.h on rte_crypto_sym.h. If rte_crypto_asym.h doesn't include all the headers it uses, every file which includes rte_crypto_asym.h will have to include rte_crypto_asym.h's dependencies, which is not the right practice. Keeping it alphabetical etc comes later.

So the patch has to be rephrased to better reflect the purpose.

Also please see inline.

Thanks,
Anoob
 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ayuj Verma
> Sent: Friday, April 12, 2019 12:33 PM
> To: akhil.goyal@nxp.com; arkadiuszx.kusztal@intel.com;
> fiona.trahe@intel.com
> Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
> Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Arvind Desai
> <adesai@marvell.com>; dev@dpdk.org; Ayuj Verma
> <ayverma@marvell.com>
> Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of
> headers
> 
> include rte_crypto_sym.h in rte_crypto_asym.h in place of including it in
> rte_crypto.h.
> 
> Signed-off-by: Ayuj Verma <ayverma@marvell.com>
> Signed-off-by: Shally Verma <shallyv@marvell.com>
> 
> ---
>  lib/librte_cryptodev/rte_crypto.h      | 1 -
>  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto.h
> b/lib/librte_cryptodev/rte_crypto.h
> index fd5ef3a..17dccdf 100644
> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -22,7 +22,6 @@
>  #include <rte_mempool.h>
>  #include <rte_common.h>
> 
> -#include "rte_crypto_sym.h"
>  #include "rte_crypto_asym.h"

[Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h, then you cannot remove the include. You can make it alphabetical, but the include has to be retained.

> 
>  /** Crypto operation types */
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 5e43620..a55923a 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -25,6 +25,8 @@
>  #include <rte_mempool.h>
>  #include <rte_common.h>
> 
> +#include "rte_crypto_sym.h"
> +
>  typedef struct rte_crypto_param_t {
>  	uint8_t *data;
>  	/**< pointer to buffer holding data */
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Shally Verma]

Hi Anoob

> -----Original Message-----
> From: Anoob Joseph
> Sent: Thursday, April 18, 2019 9:28 AM
> To: Ayuj Verma <ayverma@marvell.com>; akhil.goyal@nxp.com;
> arkadiuszx.kusztal@intel.com; fiona.trahe@intel.com
> Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
> Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Arvind Desai
> <adesai@marvell.com>; dev@dpdk.org; Ayuj Verma
> <ayverma@marvell.com>
> Subject: RE: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of
> headers
> 
> Hi Ayuj, Akhil, Fiona, Shally,
> 
> I think there are couple of issues with this patch. The real issue here is the
> dependency of rte_crypto_asym.h on rte_crypto_sym.h. If
> rte_crypto_asym.h doesn't include all the headers it uses, every file which
> includes rte_crypto_asym.h will have to include rte_crypto_asym.h's
> dependencies, which is not the right practice. Keeping it alphabetical etc
> comes later.

[Shally] We can change title something to "include dependencies into asym header files" , but then should I assume that
You agree to this patch?

> 
> So the patch has to be rephrased to better reflect the purpose.
> 
> Also please see inline.
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Ayuj Verma
> > Sent: Friday, April 12, 2019 12:33 PM
> > To: akhil.goyal@nxp.com; arkadiuszx.kusztal@intel.com;
> > fiona.trahe@intel.com
> > Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu
> > <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> > <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> > dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> > Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering
> > of headers
> >
> > include rte_crypto_sym.h in rte_crypto_asym.h in place of including it
> > in rte_crypto.h.
> >
> > Signed-off-by: Ayuj Verma <ayverma@marvell.com>
> > Signed-off-by: Shally Verma <shallyv@marvell.com>
> >
> > ---
> >  lib/librte_cryptodev/rte_crypto.h      | 1 -
> >  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > b/lib/librte_cryptodev/rte_crypto.h
> > index fd5ef3a..17dccdf 100644
> > --- a/lib/librte_cryptodev/rte_crypto.h
> > +++ b/lib/librte_cryptodev/rte_crypto.h
> > @@ -22,7 +22,6 @@
> >  #include <rte_mempool.h>
> >  #include <rte_common.h>
> >
> > -#include "rte_crypto_sym.h"
> >  #include "rte_crypto_asym.h"
> 
> [Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h, then you
> cannot remove the include. 

[Shally] so you mean we should do #include of sym.h in both, this and rte_crypto_asym.h file?

>You can make it alphabetical, but the include has to be retained.
[Shally] Sorry didn't get what you exactly mean by this?

Thanks
Shally

> 

> >
> >  /** Crypto operation types */
> > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > b/lib/librte_cryptodev/rte_crypto_asym.h
> > index 5e43620..a55923a 100644
> > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > @@ -25,6 +25,8 @@
> >  #include <rte_mempool.h>
> >  #include <rte_common.h>
> >
> > +#include "rte_crypto_sym.h"
> > +
> >  typedef struct rte_crypto_param_t {
> >  	uint8_t *data;
> >  	/**< pointer to buffer holding data */
> > --
> > 1.8.3.1

Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Shally Verma]

Hi Anoob

> -----Original Message-----
> From: Anoob Joseph
> Sent: Thursday, April 18, 2019 9:28 AM
> To: Ayuj Verma <ayverma@marvell.com>; akhil.goyal@nxp.com;
> arkadiuszx.kusztal@intel.com; fiona.trahe@intel.com
> Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
> Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Arvind Desai
> <adesai@marvell.com>; dev@dpdk.org; Ayuj Verma
> <ayverma@marvell.com>
> Subject: RE: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of
> headers
> 
> Hi Ayuj, Akhil, Fiona, Shally,
> 
> I think there are couple of issues with this patch. The real issue here is the
> dependency of rte_crypto_asym.h on rte_crypto_sym.h. If
> rte_crypto_asym.h doesn't include all the headers it uses, every file which
> includes rte_crypto_asym.h will have to include rte_crypto_asym.h's
> dependencies, which is not the right practice. Keeping it alphabetical etc
> comes later.

[Shally] We can change title something to "include dependencies into asym header files" , but then should I assume that
You agree to this patch?

> 
> So the patch has to be rephrased to better reflect the purpose.
> 
> Also please see inline.
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Ayuj Verma
> > Sent: Friday, April 12, 2019 12:33 PM
> > To: akhil.goyal@nxp.com; arkadiuszx.kusztal@intel.com;
> > fiona.trahe@intel.com
> > Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu
> > <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> > <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> > dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> > Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering
> > of headers
> >
> > include rte_crypto_sym.h in rte_crypto_asym.h in place of including it
> > in rte_crypto.h.
> >
> > Signed-off-by: Ayuj Verma <ayverma@marvell.com>
> > Signed-off-by: Shally Verma <shallyv@marvell.com>
> >
> > ---
> >  lib/librte_cryptodev/rte_crypto.h      | 1 -
> >  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > b/lib/librte_cryptodev/rte_crypto.h
> > index fd5ef3a..17dccdf 100644
> > --- a/lib/librte_cryptodev/rte_crypto.h
> > +++ b/lib/librte_cryptodev/rte_crypto.h
> > @@ -22,7 +22,6 @@
> >  #include <rte_mempool.h>
> >  #include <rte_common.h>
> >
> > -#include "rte_crypto_sym.h"
> >  #include "rte_crypto_asym.h"
> 
> [Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h, then you
> cannot remove the include. 

[Shally] so you mean we should do #include of sym.h in both, this and rte_crypto_asym.h file?

>You can make it alphabetical, but the include has to be retained.
[Shally] Sorry didn't get what you exactly mean by this?

Thanks
Shally

> 

> >
> >  /** Crypto operation types */
> > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > b/lib/librte_cryptodev/rte_crypto_asym.h
> > index 5e43620..a55923a 100644
> > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > @@ -25,6 +25,8 @@
> >  #include <rte_mempool.h>
> >  #include <rte_common.h>
> >
> > +#include "rte_crypto_sym.h"
> > +
> >  typedef struct rte_crypto_param_t {
> >  	uint8_t *data;
> >  	/**< pointer to buffer holding data */
> > --
> > 1.8.3.1

Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Anoob Joseph]

Hi Shally,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Shally Verma
> Sent: Monday, April 22, 2019 6:23 PM
> To: Anoob Joseph <anoobj@marvell.com>; Ayuj Verma
> <ayverma@marvell.com>; akhil.goyal@nxp.com;
> arkadiuszx.kusztal@intel.com; fiona.trahe@intel.com
> Cc: Sunila Sahu <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> Subject: RE: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of
> headers
> 
> Hi Anoob
> 
> > -----Original Message-----
> > From: Anoob Joseph
> > Sent: Thursday, April 18, 2019 9:28 AM
> > To: Ayuj Verma <ayverma@marvell.com>; akhil.goyal@nxp.com;
> > arkadiuszx.kusztal@intel.com; fiona.trahe@intel.com
> > Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu
> > <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> > <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> > dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> > Subject: RE: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical
> > ordering of headers
> >
> > Hi Ayuj, Akhil, Fiona, Shally,
> >
> > I think there are couple of issues with this patch. The real issue
> > here is the dependency of rte_crypto_asym.h on rte_crypto_sym.h. If
> > rte_crypto_asym.h doesn't include all the headers it uses, every file
> > which includes rte_crypto_asym.h will have to include
> > rte_crypto_asym.h's dependencies, which is not the right practice.
> > Keeping it alphabetical etc comes later.
> 
> [Shally] We can change title something to "include dependencies into asym
> header files" , but then should I assume that You agree to this patch?

[Anoob] Yes. The title and description has to be changed to better reflect what we are trying to solve. I'm in agreement with what the patch is trying to achieve.

> 
> >
> > So the patch has to be rephrased to better reflect the purpose.
> >
> > Also please see inline.
> >
> > Thanks,
> > Anoob
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Ayuj Verma
> > > Sent: Friday, April 12, 2019 12:33 PM
> > > To: akhil.goyal@nxp.com; arkadiuszx.kusztal@intel.com;
> > > fiona.trahe@intel.com
> > > Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu
> > > <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> > > <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> > > dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> > > Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering
> > > of headers
> > >
> > > include rte_crypto_sym.h in rte_crypto_asym.h in place of including
> > > it in rte_crypto.h.
> > >
> > > Signed-off-by: Ayuj Verma <ayverma@marvell.com>
> > > Signed-off-by: Shally Verma <shallyv@marvell.com>
> > >
> > > ---
> > >  lib/librte_cryptodev/rte_crypto.h      | 1 -
> > >  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
> > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > > b/lib/librte_cryptodev/rte_crypto.h
> > > index fd5ef3a..17dccdf 100644
> > > --- a/lib/librte_cryptodev/rte_crypto.h
> > > +++ b/lib/librte_cryptodev/rte_crypto.h
> > > @@ -22,7 +22,6 @@
> > >  #include <rte_mempool.h>
> > >  #include <rte_common.h>
> > >
> > > -#include "rte_crypto_sym.h"
> > >  #include "rte_crypto_asym.h"
> >
> > [Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h,
> > then you cannot remove the include.
> 
> [Shally] so you mean we should do #include of sym.h in both, this and
> rte_crypto_asym.h file?

[Anoob] If this file uses anything from rte_crypto_sym.h, then ' #include "rte_crypto_sym.h" ' has to be retained. Here the patch is removing the include.

Instead may be the includes can be rearranged to make it follow alphabetical order. Again, that need not be part of this patch. If Akhil is okay with having includes violating the alphabetical sequence, this change can be deferred.

> 
> >You can make it alphabetical, but the include has to be retained.
> [Shally] Sorry didn't get what you exactly mean by this?
> 
> Thanks
> Shally
> 
> >
> 
> > >
> > >  /** Crypto operation types */

[Anoob] IMO, only the following change is needed. 

> > > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > > b/lib/librte_cryptodev/rte_crypto_asym.h
> > > index 5e43620..a55923a 100644
> > > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > > @@ -25,6 +25,8 @@
> > >  #include <rte_mempool.h>
> > >  #include <rte_common.h>
> > >
> > > +#include "rte_crypto_sym.h"
> > > +
> > >  typedef struct rte_crypto_param_t {
> > >  	uint8_t *data;
> > >  	/**< pointer to buffer holding data */
> > > --
> > > 1.8.3.1

Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Anoob Joseph]

Hi Shally,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Shally Verma
> Sent: Monday, April 22, 2019 6:23 PM
> To: Anoob Joseph <anoobj@marvell.com>; Ayuj Verma
> <ayverma@marvell.com>; akhil.goyal@nxp.com;
> arkadiuszx.kusztal@intel.com; fiona.trahe@intel.com
> Cc: Sunila Sahu <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> Subject: RE: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of
> headers
> 
> Hi Anoob
> 
> > -----Original Message-----
> > From: Anoob Joseph
> > Sent: Thursday, April 18, 2019 9:28 AM
> > To: Ayuj Verma <ayverma@marvell.com>; akhil.goyal@nxp.com;
> > arkadiuszx.kusztal@intel.com; fiona.trahe@intel.com
> > Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu
> > <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> > <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> > dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> > Subject: RE: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical
> > ordering of headers
> >
> > Hi Ayuj, Akhil, Fiona, Shally,
> >
> > I think there are couple of issues with this patch. The real issue
> > here is the dependency of rte_crypto_asym.h on rte_crypto_sym.h. If
> > rte_crypto_asym.h doesn't include all the headers it uses, every file
> > which includes rte_crypto_asym.h will have to include
> > rte_crypto_asym.h's dependencies, which is not the right practice.
> > Keeping it alphabetical etc comes later.
> 
> [Shally] We can change title something to "include dependencies into asym
> header files" , but then should I assume that You agree to this patch?

[Anoob] Yes. The title and description has to be changed to better reflect what we are trying to solve. I'm in agreement with what the patch is trying to achieve.

> 
> >
> > So the patch has to be rephrased to better reflect the purpose.
> >
> > Also please see inline.
> >
> > Thanks,
> > Anoob
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Ayuj Verma
> > > Sent: Friday, April 12, 2019 12:33 PM
> > > To: akhil.goyal@nxp.com; arkadiuszx.kusztal@intel.com;
> > > fiona.trahe@intel.com
> > > Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu
> > > <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> > > <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> > > dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> > > Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering
> > > of headers
> > >
> > > include rte_crypto_sym.h in rte_crypto_asym.h in place of including
> > > it in rte_crypto.h.
> > >
> > > Signed-off-by: Ayuj Verma <ayverma@marvell.com>
> > > Signed-off-by: Shally Verma <shallyv@marvell.com>
> > >
> > > ---
> > >  lib/librte_cryptodev/rte_crypto.h      | 1 -
> > >  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
> > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > > b/lib/librte_cryptodev/rte_crypto.h
> > > index fd5ef3a..17dccdf 100644
> > > --- a/lib/librte_cryptodev/rte_crypto.h
> > > +++ b/lib/librte_cryptodev/rte_crypto.h
> > > @@ -22,7 +22,6 @@
> > >  #include <rte_mempool.h>
> > >  #include <rte_common.h>
> > >
> > > -#include "rte_crypto_sym.h"
> > >  #include "rte_crypto_asym.h"
> >
> > [Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h,
> > then you cannot remove the include.
> 
> [Shally] so you mean we should do #include of sym.h in both, this and
> rte_crypto_asym.h file?

[Anoob] If this file uses anything from rte_crypto_sym.h, then ' #include "rte_crypto_sym.h" ' has to be retained. Here the patch is removing the include.

Instead may be the includes can be rearranged to make it follow alphabetical order. Again, that need not be part of this patch. If Akhil is okay with having includes violating the alphabetical sequence, this change can be deferred.

> 
> >You can make it alphabetical, but the include has to be retained.
> [Shally] Sorry didn't get what you exactly mean by this?
> 
> Thanks
> Shally
> 
> >
> 
> > >
> > >  /** Crypto operation types */

[Anoob] IMO, only the following change is needed. 

> > > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > > b/lib/librte_cryptodev/rte_crypto_asym.h
> > > index 5e43620..a55923a 100644
> > > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > > @@ -25,6 +25,8 @@
> > >  #include <rte_mempool.h>
> > >  #include <rte_common.h>
> > >
> > > +#include "rte_crypto_sym.h"
> > > +
> > >  typedef struct rte_crypto_param_t {
> > >  	uint8_t *data;
> > >  	/**< pointer to buffer holding data */
> > > --
> > > 1.8.3.1

Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Akhil Goyal]

Hi Anoob,

> > >
> > > Hi Ayuj, Akhil, Fiona, Shally,
> > >
> > > I think there are couple of issues with this patch. The real issue
> > > here is the dependency of rte_crypto_asym.h on rte_crypto_sym.h. If
> > > rte_crypto_asym.h doesn't include all the headers it uses, every file
> > > which includes rte_crypto_asym.h will have to include
> > > rte_crypto_asym.h's dependencies, which is not the right practice.
> > > Keeping it alphabetical etc comes later.
> >
> > [Shally] We can change title something to "include dependencies into asym
> > header files" , but then should I assume that You agree to this patch?
> 
> [Anoob] Yes. The title and description has to be changed to better reflect what
> we are trying to solve. I'm in agreement with what the patch is trying to achieve.
> 
Agreed
> >
> > >
> > > So the patch has to be rephrased to better reflect the purpose.
> > >
> > > Also please see inline.
> > >
> > > Thanks,
> > > Anoob
> > >
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Ayuj Verma
> > > > Sent: Friday, April 12, 2019 12:33 PM
> > > > To: akhil.goyal@nxp.com; arkadiuszx.kusztal@intel.com;
> > > > fiona.trahe@intel.com
> > > > Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu
> > > > <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> > > > <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> > > > dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> > > > Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering
> > > > of headers
> > > >
> > > > include rte_crypto_sym.h in rte_crypto_asym.h in place of including
> > > > it in rte_crypto.h.
> > > >
> > > > Signed-off-by: Ayuj Verma <ayverma@marvell.com>
> > > > Signed-off-by: Shally Verma <shallyv@marvell.com>
> > > >
> > > > ---
> > > >  lib/librte_cryptodev/rte_crypto.h      | 1 -
> > > >  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
> > > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > > > b/lib/librte_cryptodev/rte_crypto.h
> > > > index fd5ef3a..17dccdf 100644
> > > > --- a/lib/librte_cryptodev/rte_crypto.h
> > > > +++ b/lib/librte_cryptodev/rte_crypto.h
> > > > @@ -22,7 +22,6 @@
> > > >  #include <rte_mempool.h>
> > > >  #include <rte_common.h>
> > > >
> > > > -#include "rte_crypto_sym.h"
> > > >  #include "rte_crypto_asym.h"
> > >
> > > [Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h,
> > > then you cannot remove the include.
> >
> > [Shally] so you mean we should do #include of sym.h in both, this and
> > rte_crypto_asym.h file?
> 
> [Anoob] If this file uses anything from rte_crypto_sym.h, then ' #include
> "rte_crypto_sym.h" ' has to be retained. Here the patch is removing the include.

Agreed.
> 
> Instead may be the includes can be rearranged to make it follow alphabetical
> order. Again, that need not be part of this patch. If Akhil is okay with having
> includes violating the alphabetical sequence, this change can be deferred.
I am ok with this.

> 
> >
> > >You can make it alphabetical, but the include has to be retained.
> > [Shally] Sorry didn't get what you exactly mean by this?
> >
> > Thanks
> > Shally
> >
> > >
> >
> > > >
> > > >  /** Crypto operation types */
> 
> [Anoob] IMO, only the following change is needed.

Agreed.
> 
> > > > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > > > b/lib/librte_cryptodev/rte_crypto_asym.h
> > > > index 5e43620..a55923a 100644
> > > > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > > > @@ -25,6 +25,8 @@
> > > >  #include <rte_mempool.h>
> > > >  #include <rte_common.h>
> > > >
> > > > +#include "rte_crypto_sym.h"
> > > > +
> > > >  typedef struct rte_crypto_param_t {
> > > >  	uint8_t *data;
> > > >  	/**< pointer to buffer holding data */
> > > > --
> > > > 1.8.3.1

Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers

[Akhil Goyal]

Hi Anoob,

> > >
> > > Hi Ayuj, Akhil, Fiona, Shally,
> > >
> > > I think there are couple of issues with this patch. The real issue
> > > here is the dependency of rte_crypto_asym.h on rte_crypto_sym.h. If
> > > rte_crypto_asym.h doesn't include all the headers it uses, every file
> > > which includes rte_crypto_asym.h will have to include
> > > rte_crypto_asym.h's dependencies, which is not the right practice.
> > > Keeping it alphabetical etc comes later.
> >
> > [Shally] We can change title something to "include dependencies into asym
> > header files" , but then should I assume that You agree to this patch?
> 
> [Anoob] Yes. The title and description has to be changed to better reflect what
> we are trying to solve. I'm in agreement with what the patch is trying to achieve.
> 
Agreed
> >
> > >
> > > So the patch has to be rephrased to better reflect the purpose.
> > >
> > > Also please see inline.
> > >
> > > Thanks,
> > > Anoob
> > >
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Ayuj Verma
> > > > Sent: Friday, April 12, 2019 12:33 PM
> > > > To: akhil.goyal@nxp.com; arkadiuszx.kusztal@intel.com;
> > > > fiona.trahe@intel.com
> > > > Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu
> > > > <ssahu@marvell.com>; Kanaka Durga Kotamarthy
> > > > <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>;
> > > > dev@dpdk.org; Ayuj Verma <ayverma@marvell.com>
> > > > Subject: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering
> > > > of headers
> > > >
> > > > include rte_crypto_sym.h in rte_crypto_asym.h in place of including
> > > > it in rte_crypto.h.
> > > >
> > > > Signed-off-by: Ayuj Verma <ayverma@marvell.com>
> > > > Signed-off-by: Shally Verma <shallyv@marvell.com>
> > > >
> > > > ---
> > > >  lib/librte_cryptodev/rte_crypto.h      | 1 -
> > > >  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
> > > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > > > b/lib/librte_cryptodev/rte_crypto.h
> > > > index fd5ef3a..17dccdf 100644
> > > > --- a/lib/librte_cryptodev/rte_crypto.h
> > > > +++ b/lib/librte_cryptodev/rte_crypto.h
> > > > @@ -22,7 +22,6 @@
> > > >  #include <rte_mempool.h>
> > > >  #include <rte_common.h>
> > > >
> > > > -#include "rte_crypto_sym.h"
> > > >  #include "rte_crypto_asym.h"
> > >
> > > [Anoob] If rte_crypto.h uses anything defined in rte_crypto_sym.h,
> > > then you cannot remove the include.
> >
> > [Shally] so you mean we should do #include of sym.h in both, this and
> > rte_crypto_asym.h file?
> 
> [Anoob] If this file uses anything from rte_crypto_sym.h, then ' #include
> "rte_crypto_sym.h" ' has to be retained. Here the patch is removing the include.

Agreed.
> 
> Instead may be the includes can be rearranged to make it follow alphabetical
> order. Again, that need not be part of this patch. If Akhil is okay with having
> includes violating the alphabetical sequence, this change can be deferred.
I am ok with this.

> 
> >
> > >You can make it alphabetical, but the include has to be retained.
> > [Shally] Sorry didn't get what you exactly mean by this?
> >
> > Thanks
> > Shally
> >
> > >
> >
> > > >
> > > >  /** Crypto operation types */
> 
> [Anoob] IMO, only the following change is needed.

Agreed.
> 
> > > > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > > > b/lib/librte_cryptodev/rte_crypto_asym.h
> > > > index 5e43620..a55923a 100644
> > > > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > > > @@ -25,6 +25,8 @@
> > > >  #include <rte_mempool.h>
> > > >  #include <rte_common.h>
> > > >
> > > > +#include "rte_crypto_sym.h"
> > > > +
> > > >  typedef struct rte_crypto_param_t {
> > > >  	uint8_t *data;
> > > >  	/**< pointer to buffer holding data */
> > > > --
> > > > 1.8.3.1

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Trahe, Fiona]

Hi Ayuj, Akhil,

From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
Sent: Wednesday, April 17, 2019 10:58 AM
To: Ayuj Verma <ayverma@marvell.com>; Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>; Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>; dev@dpdk.org
Subject: Re: [PATCH v1] fix alphabetical ordering of headers


Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Friday, April 12, 2019 8:03 AM
> To: akhil.goyal@nxp.com<mailto:akhil.goyal@nxp.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; Trahe, Fiona
> <fiona.trahe@intel.com<mailto:fiona.trahe@intel.com>>
> Cc: shallyv@marvell.com<mailto:shallyv@marvell.com>; ssahu@marvell.com<mailto:ssahu@marvell.com>; kkotamarthy@marvell.com<mailto:kkotamarthy@marvell.com>; adesai@marvell.com<mailto:adesai@marvell.com>;
> dev@dpdk.org<mailto:dev@dpdk.org>; Ayuj Verma <ayverma@marvell.com<mailto:ayverma@marvell.com>>
> Subject: [PATCH v1] fix alphabetical ordering of headers
>
> rte_crypto_sym.h is included prior to rte_crypto_asym.h
> in rte_crypto.h, which breaks alphabetical order.
>
> include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
[Fiona] I presume you tried just swapping the order and it broke?
If something in rte_crypto_asym.h depends on something from rte_crypto_sym.h, it
probably shouldn't. What's the dependency and can/should it be moved to rte_crypto.h?
[Ayuj] it is enum rte_crypto_auth_algorithm which it import from sym.h. So do you suggest to move it to rte_crypto.h?

[Akhil] moving the enum is not a good idea. I believe we do not need this change at all. Keeping the headers in alphabetical order may be a preferred way,
But it should not be mandatory, there may be some dependencies like this.
[Fiona] I agree with not moving the enum, now that I understand the dependency.
Either no change or the original patch are fine with me.

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Trahe, Fiona]

Hi Ayuj, Akhil,

From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
Sent: Wednesday, April 17, 2019 10:58 AM
To: Ayuj Verma <ayverma@marvell.com>; Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
Cc: Shally Verma <shallyv@marvell.com>; Sunila Sahu <ssahu@marvell.com>; Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Arvind Desai <adesai@marvell.com>; dev@dpdk.org
Subject: Re: [PATCH v1] fix alphabetical ordering of headers


Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Friday, April 12, 2019 8:03 AM
> To: akhil.goyal@nxp.com<mailto:akhil.goyal@nxp.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; Trahe, Fiona
> <fiona.trahe@intel.com<mailto:fiona.trahe@intel.com>>
> Cc: shallyv@marvell.com<mailto:shallyv@marvell.com>; ssahu@marvell.com<mailto:ssahu@marvell.com>; kkotamarthy@marvell.com<mailto:kkotamarthy@marvell.com>; adesai@marvell.com<mailto:adesai@marvell.com>;
> dev@dpdk.org<mailto:dev@dpdk.org>; Ayuj Verma <ayverma@marvell.com<mailto:ayverma@marvell.com>>
> Subject: [PATCH v1] fix alphabetical ordering of headers
>
> rte_crypto_sym.h is included prior to rte_crypto_asym.h
> in rte_crypto.h, which breaks alphabetical order.
>
> include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
[Fiona] I presume you tried just swapping the order and it broke?
If something in rte_crypto_asym.h depends on something from rte_crypto_sym.h, it
probably shouldn't. What's the dependency and can/should it be moved to rte_crypto.h?
[Ayuj] it is enum rte_crypto_auth_algorithm which it import from sym.h. So do you suggest to move it to rte_crypto.h?

[Akhil] moving the enum is not a good idea. I believe we do not need this change at all. Keeping the headers in alphabetical order may be a preferred way,
But it should not be mandatory, there may be some dependencies like this.
[Fiona] I agree with not moving the enum, now that I understand the dependency.
Either no change or the original patch are fine with me.

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Akhil Goyal]

Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Friday, April 12, 2019 8:03 AM
> To: akhil.goyal@nxp.com<mailto:akhil.goyal@nxp.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; Trahe, Fiona
> <fiona.trahe@intel.com<mailto:fiona.trahe@intel.com>>
> Cc: shallyv@marvell.com<mailto:shallyv@marvell.com>; ssahu@marvell.com<mailto:ssahu@marvell.com>; kkotamarthy@marvell.com<mailto:kkotamarthy@marvell.com>; adesai@marvell.com<mailto:adesai@marvell.com>;
> dev@dpdk.org<mailto:dev@dpdk.org>; Ayuj Verma <ayverma@marvell.com<mailto:ayverma@marvell.com>>
> Subject: [PATCH v1] fix alphabetical ordering of headers
>
> rte_crypto_sym.h is included prior to rte_crypto_asym.h
> in rte_crypto.h, which breaks alphabetical order.
>
> include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
[Fiona] I presume you tried just swapping the order and it broke?
If something in rte_crypto_asym.h depends on something from rte_crypto_sym.h, it
probably shouldn't. What's the dependency and can/should it be moved to rte_crypto.h?
[Ayuj] it is enum rte_crypto_auth_algorithm which it import from sym.h. So do you suggest to move it to rte_crypto.h?

[Akhil] moving the enum is not a good idea. I believe we do not need this change at all. Keeping the headers in alphabetical order may be a preferred way,
But it should not be mandatory, there may be some dependencies like this.

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Akhil Goyal]

Hi Ayuj,

> -----Original Message-----
> From: Ayuj Verma [mailto:ayverma@marvell.com]
> Sent: Friday, April 12, 2019 8:03 AM
> To: akhil.goyal@nxp.com<mailto:akhil.goyal@nxp.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; Trahe, Fiona
> <fiona.trahe@intel.com<mailto:fiona.trahe@intel.com>>
> Cc: shallyv@marvell.com<mailto:shallyv@marvell.com>; ssahu@marvell.com<mailto:ssahu@marvell.com>; kkotamarthy@marvell.com<mailto:kkotamarthy@marvell.com>; adesai@marvell.com<mailto:adesai@marvell.com>;
> dev@dpdk.org<mailto:dev@dpdk.org>; Ayuj Verma <ayverma@marvell.com<mailto:ayverma@marvell.com>>
> Subject: [PATCH v1] fix alphabetical ordering of headers
>
> rte_crypto_sym.h is included prior to rte_crypto_asym.h
> in rte_crypto.h, which breaks alphabetical order.
>
> include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
[Fiona] I presume you tried just swapping the order and it broke?
If something in rte_crypto_asym.h depends on something from rte_crypto_sym.h, it
probably shouldn't. What's the dependency and can/should it be moved to rte_crypto.h?
[Ayuj] it is enum rte_crypto_auth_algorithm which it import from sym.h. So do you suggest to move it to rte_crypto.h?

[Akhil] moving the enum is not a good idea. I believe we do not need this change at all. Keeping the headers in alphabetical order may be a preferred way,
But it should not be mandatory, there may be some dependencies like this.

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Akhil Goyal]

Hi Ayuj,

> > rte_crypto_sym.h is included prior to rte_crypto_asym.h
> > in rte_crypto.h, which breaks alphabetical order.
> >
> > include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
> [Fiona] I presume you tried just swapping the order and it broke?
> If something in rte_crypto_asym.h depends on something from
> rte_crypto_sym.h, it
> probably shouldn't. What's the dependency and can/should it be moved to
> rte_crypto.h?

Could you please reply to Fiona's comment?

Thanks,
Akhil

Re: [dpdk-dev] [PATCH v1] fix alphabetical ordering of headers

[Akhil Goyal]

Hi Ayuj,

> > rte_crypto_sym.h is included prior to rte_crypto_asym.h
> > in rte_crypto.h, which breaks alphabetical order.
> >
> > include rte_crypto_sym.h in rte_crypto_asym.h fixes this.
> [Fiona] I presume you tried just swapping the order and it broke?
> If something in rte_crypto_asym.h depends on something from
> rte_crypto_sym.h, it
> probably shouldn't. What's the dependency and can/should it be moved to
> rte_crypto.h?

Could you please reply to Fiona's comment?

Thanks,
Akhil