[dpdk-dev,RFC,1/4] rte_security: API definitions
Checks
Commit Message
On 8/15/2017 4:34 PM, Radu Nicolau wrote:
>
> On 8/15/2017 7:35 AM, Akhil Goyal wrote:
>> Detailed description is added in the coverletter
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>> ---
>> lib/librte_cryptodev/rte_security.c | 171 +++++++++++++++
>> lib/librte_cryptodev/rte_security.h | 409
>> ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 580 insertions(+)
>> create mode 100644 lib/librte_cryptodev/rte_security.c
>> create mode 100644 lib/librte_cryptodev/rte_security.h
>>
>> +int
>> +rte_security_session_init(uint16_t dev_id,
>> + struct rte_security_session *sess,
>> + struct rte_security_sess_conf *conf,
>> + struct rte_mempool *mp)
>> +{
>> + struct rte_cryptodev *cdev = NULL;
>> + struct rte_eth_dev *dev = NULL;
>> + uint8_t index;
>> + int ret;
>> +
>> + if (sess == NULL || conf == NULL)
>> + return -EINVAL;
>> +
>> + switch (conf->action_type) {
>> + case RTE_SECURITY_SESS_CRYPTO_PROTO_OFFLOAD:
>> + if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
>> + return -EINVAL;
>> + cdev = rte_cryptodev_pmd_get_dev(dev_id);
>> + index = cdev->driver_id;
>> + if (sess->sess_private_data[index] == NULL) {
>> + ret = cdev->sec_ops->session_configure(cdev, conf, sess,
>> mp);
>> + if (ret < 0) {
>> + CDEV_LOG_ERR(
>> + "cdev_id %d failed to configure session details",
>> + dev_id);
>> + return ret;
>> + }
>> + }
>> + break;
>> + case RTE_SECURITY_SESS_ETH_INLINE_CRYPTO:
>> + case RTE_SECURITY_SESS_ETH_PROTO_OFFLOAD:
>> + dev = &rte_eth_devices[dev_id];
>> + index = dev->data->port_id;
>> + if (sess->sess_private_data[index] == NULL) {
>> +// ret = dev->sec_ops->session_configure(dev, conf, sess,
>> mp);
>> +// if (ret < 0) {
>> +// CDEV_LOG_ERR(
>> +// "dev_id %d failed to configure session details",
>> +// dev_id);
>> +// return ret;
>> +// }
> The commented lines above suggests that also eth devices will have a
> sec_ops field, (which makes sense). Is this correct?
> Also, if the above is correct, session_configure and session_clear
> should accept both crypto and eth devices as first parameter.
Yes you are correct both these ops should accept void *dev and
internally in the driver should typecast to respective device.
Please consider the following diff over this patch
"cdev_id %d failed to configure
session details",
@@ -101,7 +102,8 @@ rte_security_session_init(uint16_t dev_id,
dev = &rte_eth_devices[dev_id];
index = dev->data->port_id;
if (sess->sess_private_data[index] == NULL) {
-// ret = dev->sec_ops->session_configure(dev, conf,
sess, mp);
+// ret = dev->sec_ops->session_configure((void *)dev,
+// conf, sess, mp);
Thanks,
Akhil
Comments
Hi Thomas,
Can we get a next-security tree to do development around this proposal?
Also, we can discuss about this proposal in general in next techboard meeting.
Regards,
Hemant
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Wednesday, August 16, 2017 1:10 PM
> To: Radu Nicolau <radu.nicolau@intel.com>; dev@dpdk.org;
> declan.doherty@intel.com; thomas@monjalon.net;
> aviadye@mellanox.com; borisp@mellanox.com;
> pablo.de.lara.guarch@intel.com; sergio.gonzalez.monroy@intel.com
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; Sandeep Malik
> <sandeep.malik@nxp.com>
> Subject: Re: [RFC PATCH 1/4] rte_security: API definitions
>
> On 8/15/2017 4:34 PM, Radu Nicolau wrote:
> >
> > On 8/15/2017 7:35 AM, Akhil Goyal wrote:
> >> Detailed description is added in the coverletter
> >>
> >> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> >> ---
> >> lib/librte_cryptodev/rte_security.c | 171 +++++++++++++++
> >> lib/librte_cryptodev/rte_security.h | 409
> >> ++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 580 insertions(+)
> >> create mode 100644 lib/librte_cryptodev/rte_security.c
> >> create mode 100644 lib/librte_cryptodev/rte_security.h
> >>
>
> >> +int
> >> +rte_security_session_init(uint16_t dev_id,
> >> + struct rte_security_session *sess,
> >> + struct rte_security_sess_conf *conf,
> >> + struct rte_mempool *mp) {
> >> + struct rte_cryptodev *cdev = NULL;
> >> + struct rte_eth_dev *dev = NULL;
> >> + uint8_t index;
> >> + int ret;
> >> +
> >> + if (sess == NULL || conf == NULL)
> >> + return -EINVAL;
> >> +
> >> + switch (conf->action_type) {
> >> + case RTE_SECURITY_SESS_CRYPTO_PROTO_OFFLOAD:
> >> + if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
> >> + return -EINVAL;
> >> + cdev = rte_cryptodev_pmd_get_dev(dev_id);
> >> + index = cdev->driver_id;
> >> + if (sess->sess_private_data[index] == NULL) {
> >> + ret = cdev->sec_ops->session_configure(cdev, conf, sess,
> >> mp);
> >> + if (ret < 0) {
> >> + CDEV_LOG_ERR(
> >> + "cdev_id %d failed to configure session details",
> >> + dev_id);
> >> + return ret;
> >> + }
> >> + }
> >> + break;
> >> + case RTE_SECURITY_SESS_ETH_INLINE_CRYPTO:
> >> + case RTE_SECURITY_SESS_ETH_PROTO_OFFLOAD:
> >> + dev = &rte_eth_devices[dev_id];
> >> + index = dev->data->port_id;
> >> + if (sess->sess_private_data[index] == NULL) {
> >> +// ret = dev->sec_ops->session_configure(dev, conf, sess,
> >> mp);
> >> +// if (ret < 0) {
> >> +// CDEV_LOG_ERR(
> >> +// "dev_id %d failed to configure session details",
> >> +// dev_id);
> >> +// return ret;
> >> +// }
> > The commented lines above suggests that also eth devices will have a
> > sec_ops field, (which makes sense). Is this correct?
> > Also, if the above is correct, session_configure and session_clear
> > should accept both crypto and eth devices as first parameter.
>
> Yes you are correct both these ops should accept void *dev and internally in
> the driver should typecast to respective device.
> Please consider the following diff over this patch
>
>
> diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> index 219fba6..ab3ecf7 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> +++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> @@ -371,7 +371,7 @@ struct rte_cryptodev_ops {
> * - Returns -ENOTSUP if crypto device does not support the crypto
> transform.
> * - Returns -ENOMEM if the private session could not be allocated.
> */
> -typedef int (*security_configure_session_t)(struct rte_cryptodev *dev,
> +typedef int (*security_configure_session_t)(void *dev,
> struct rte_security_sess_conf *conf,
> struct rte_security_session *sess,
> struct rte_mempool *mp);
> @@ -382,7 +382,7 @@ typedef int (*security_configure_session_t)(struct
> rte_cryptodev *dev,
> * @param dev Crypto device pointer
> * @param sess Security session structure
> */
> -typedef void (*security_free_session_t)(struct rte_cryptodev *dev,
> +typedef void (*security_free_session_t)(void *dev,
> struct rte_security_session *sess);
>
> /** Security operations function pointer table */
> diff --git a/lib/librte_cryptodev/rte_security.c
> b/lib/librte_cryptodev/rte_security.c
> index 7c73c93..a7558bb 100644
> --- a/lib/librte_cryptodev/rte_security.c
> +++ b/lib/librte_cryptodev/rte_security.c
> @@ -87,7 +87,8 @@ rte_security_session_init(uint16_t dev_id,
> cdev = rte_cryptodev_pmd_get_dev(dev_id);
> index = cdev->driver_id;
> if (sess->sess_private_data[index] == NULL) {
> - ret = cdev->sec_ops->session_configure(cdev,
> conf, sess, mp);
> + ret = cdev->sec_ops->session_configure((void *)cdev,
> + conf, sess, mp);
> if (ret < 0) {
> CDEV_LOG_ERR(
> "cdev_id %d failed to configure
> session details",
> @@ -101,7 +102,8 @@ rte_security_session_init(uint16_t dev_id,
> dev = &rte_eth_devices[dev_id];
> index = dev->data->port_id;
> if (sess->sess_private_data[index] == NULL) {
> -// ret = dev->sec_ops->session_configure(dev, conf,
> sess, mp);
> +// ret = dev->sec_ops->session_configure((void *)dev,
> +// conf, sess, mp);
>
> Thanks,
> Akhil
Hi,
16/08/2017 17:40, Hemant Agrawal:
> Hi Thomas,
> Can we get a next-security tree to do development around this proposal?
>
> Also, we can discuss about this proposal in general in next techboard meeting.
First question to ask:
Why not create a repository elsewhere for your trials?
The benefit of creating a dpdk.org repo is to show it as an official feature.
So the idea behind this new library must be accepted by the technical board first.
The other use of official repos is prepare pull request for subsequent releases.
Do we want to have a -next tree for IPsec development and keep it for next releases?
I think it makes sense to have a -next tree for IPsec offloading in general.
Before the techboard approves it, we need to define the name (and the scope)
of the tree, and who will be the maintainer of the tree.
On 8/18/2017 2:46 PM, Thomas Monjalon wrote:
> Hi,
>
> 16/08/2017 17:40, Hemant Agrawal:
>> Hi Thomas,
>> Can we get a next-security tree to do development around this proposal?
>>
>> Also, we can discuss about this proposal in general in next techboard meeting.
>
> First question to ask:
> Why not create a repository elsewhere for your trials?
>
github is a good place, but I heard some companies (e.g. Intel) may have
concerns over posting the work to github.
> The benefit of creating a dpdk.org repo is to show it as an official feature.
> So the idea behind this new library must be accepted by the technical board first.
>
agree. we will discuss it in next tech board meeting.
> The other use of official repos is prepare pull request for subsequent releases.
> Do we want to have a -next tree for IPsec development and keep it for next releases?
>
I believe it will be required for few release. In future, this work can
merge and maintained in next-crypto. However this may change as we develop.
> I think it makes sense to have a -next tree for IPsec offloading in general.
> Before the techboard approves it, we need to define the name (and the scope)
> of the tree, and who will be the maintainer of the tree.
>
I see following individual maintaining it :
1. Akhil Goyal (akhil.goyal@nxp.com)
2. Boris (borisp@mellanox.com)
3. Declan Doherty (declan.doherty@intel.com)
Regards,
Hemant
For drafting, we have opened this github repository:
https://github.com/Mellanox/dpdk-next-crypto
Akhil/Hemant could you please push your rte_security patches there?
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, August 18, 2017 12:17
> To: Hemant Agrawal <hemant.agrawal@nxp.com>
> Cc: Akhil Goyal <akhil.goyal@nxp.com>; Radu Nicolau
> <radu.nicolau@intel.com>; dev@dpdk.org; declan.doherty@intel.com; Aviad
> Yehezkel <aviadye@mellanox.com>; Boris Pismenny <borisp@mellanox.com>;
> pablo.de.lara.guarch@intel.com; sergio.gonzalez.monroy@intel.com; Sandeep
> Malik <sandeep.malik@nxp.com>; techboard@dpdk.org
> Subject: Re: [RFC PATCH 1/4] rte_security: API definitions
>
> Hi,
>
> 16/08/2017 17:40, Hemant Agrawal:
> > Hi Thomas,
> > Can we get a next-security tree to do development around this
> proposal?
> >
> > Also, we can discuss about this proposal in general in next techboard meeting.
>
> First question to ask:
> Why not create a repository elsewhere for your trials?
>
> The benefit of creating a dpdk.org repo is to show it as an official feature.
> So the idea behind this new library must be accepted by the technical board
> first.
>
> The other use of official repos is prepare pull request for subsequent releases.
> Do we want to have a -next tree for IPsec development and keep it for next
> releases?
>
> I think it makes sense to have a -next tree for IPsec offloading in general.
> Before the techboard approves it, we need to define the name (and the scope)
> of the tree, and who will be the maintainer of the tree.
Hi Boris,
On 8/21/2017 4:02 PM, Boris Pismenny wrote:
> For drafting, we have opened this github repository:
> https://github.com/Mellanox/dpdk-next-crypto
>
> Akhil/Hemant could you please push your rte_security patches there?
>
I have pushed the patches on this tree.
Regards,
Akhil
b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -371,7 +371,7 @@ struct rte_cryptodev_ops {
* - Returns -ENOTSUP if crypto device does not support the crypto
transform.
* - Returns -ENOMEM if the private session could not be allocated.
*/
-typedef int (*security_configure_session_t)(struct rte_cryptodev *dev,
+typedef int (*security_configure_session_t)(void *dev,
struct rte_security_sess_conf *conf,
struct rte_security_session *sess,
struct rte_mempool *mp);
@@ -382,7 +382,7 @@ typedef int (*security_configure_session_t)(struct
rte_cryptodev *dev,
* @param dev Crypto device pointer
* @param sess Security session structure
*/
-typedef void (*security_free_session_t)(struct rte_cryptodev *dev,
+typedef void (*security_free_session_t)(void *dev,
struct rte_security_session *sess);
/** Security operations function pointer table */
b/lib/librte_cryptodev/rte_security.c
@@ -87,7 +87,8 @@ rte_security_session_init(uint16_t dev_id,
cdev = rte_cryptodev_pmd_get_dev(dev_id);
index = cdev->driver_id;
if (sess->sess_private_data[index] == NULL) {
- ret = cdev->sec_ops->session_configure(cdev,
conf, sess, mp);
+ ret = cdev->sec_ops->session_configure((void *)cdev,
+ conf, sess, mp);
if (ret < 0) {
CDEV_LOG_ERR(