From: Anoob Joseph <anoobj@marvell.com>
To: Shally Verma <shallyv@marvell.com>,
Ayuj Verma <ayverma@marvell.com>,
"akhil.goyal@nxp.com" <akhil.goyal@nxp.com>,
"arkadiuszx.kusztal@intel.com" <arkadiuszx.kusztal@intel.com>,
"fiona.trahe@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" <dev@dpdk.org>,
Ayuj Verma <ayverma@marvell.com>
Subject: Re: [dpdk-dev] [PATCH v1] lib/crypto: fix alphabetical ordering of headers
Date: Tue, 23 Apr 2019 05:42:25 +0000 [thread overview]
Message-ID: <MN2PR18MB28772E00CB39AA2A654FEF0FDF230@MN2PR18MB2877.namprd18.prod.outlook.com> (raw)
In-Reply-To: <BN6PR1801MB20522AFA78973D3C1A07E1BDAD220@BN6PR1801MB2052.namprd18.prod.outlook.com>
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
next prev parent reply other threads:[~2019-04-23 5:42 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-12 7:03 [dpdk-dev] [PATCH v1] " Ayuj Verma
2019-04-12 7:03 ` Ayuj Verma
2019-04-12 7:03 ` [dpdk-dev] [PATCH v1] lib/crypto: " Ayuj Verma
2019-04-12 7:03 ` Ayuj Verma
2019-04-18 3:58 ` Anoob Joseph
2019-04-18 3:58 ` Anoob Joseph
2019-04-22 12:52 ` Shally Verma
2019-04-22 12:52 ` Shally Verma
2019-04-23 5:42 ` Anoob Joseph [this message]
2019-04-23 5:42 ` Anoob Joseph
2019-04-23 6:26 ` Akhil Goyal
2019-04-23 6:26 ` Akhil Goyal
2019-04-12 15:17 ` [dpdk-dev] [PATCH v1] " Trahe, Fiona
2019-04-12 15:17 ` Trahe, Fiona
2019-04-16 16:15 ` Ayuj Verma
2019-04-16 16:15 ` Ayuj Verma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN2PR18MB28772E00CB39AA2A654FEF0FDF230@MN2PR18MB2877.namprd18.prod.outlook.com \
--to=anoobj@marvell.com \
--cc=adesai@marvell.com \
--cc=akhil.goyal@nxp.com \
--cc=arkadiuszx.kusztal@intel.com \
--cc=ayverma@marvell.com \
--cc=dev@dpdk.org \
--cc=fiona.trahe@intel.com \
--cc=kkotamarthy@marvell.com \
--cc=shallyv@marvell.com \
--cc=ssahu@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).