diff mbox series

RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Message ID MW4PR11MB59115297140DB034BD39047187222@MW4PR11MB5911.namprd11.prod.outlook.com (mailing list archive)
State New
Delegated to: akhil goyal
Headers
Series RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific |

Checks

Context Check Description
ci/loongarch-compilation warning apply patch failure
ci/Intel-compilation warning apply issues
ci/iol-testing warning apply patch failure

Commit Message

Ganapati Kundapura March 5, 2024, 7:13 a.m. UTC 
  Hi dpdk-dev,
   Can 'uint8_t reserved[1]' of 'struct rte_crypto_op' be renamed
to 'uint8_t impl_opaque' for implementation specific?

An implementation may use this field to hold implementation specific
value to share value between dequeue and enqueue operation and crypto library/driver
can also use this field to share implementation specfic value to event crypto adapter/application.

'struct rte_event' has 'uint8_t impl_opaque' member
struct rte_event {
                ...
                uint8_t impl_opaque;
                /**< Implementation specific opaque value.
                * An implementation may use this field to hold
                * implementation specific value to share between
                * dequeue and enqueue operation.
                * The application should not modify this field.
                */
                ...
};

Event crypto adapter, on dequeuing the event, enqueues rte_event::event_ptr
to cryptodev as rte_crypto_op and converts the dequeued crypto op to rte_event
without restoring the implementation specific opaque value.

By having the 'uint8_t impl_opaque' member in 'struct rte_crypto_op' as

Also crypto library/driver can use rte_crypto_op::impl_opaque field to
share implementation specific opaque value to the event crypto adapter/application.

I look forward to feedback on this proposal. Patch will be submitted
for review once the initial feedback is received.

Thank you,
Ganapati

Comments

Akhil Goyal March 5, 2024, 4:47 p.m. UTC | #1 
Hi Ganapati,

Can you please explain the flow with a sequence of APIs to be used.

Regards,
Akhil

From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
Sent: Tuesday, March 5, 2024 12:44 PM
To: dpdk-dev <dev@dpdk.org>; Akhil Goyal <gakhil@marvell.com>; fanzhang.oss@gmail.com; Ji, Kai <kai.ji@intel.com>; Power, Ciara <ciara.power@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Jerin Jacob <jerinjacobk@gmail.com>
Subject: [EXTERNAL] RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Prioritize security for external emails: Confirm sender and content safety before clicking links or opening attachments
Ganapati Kundapura March 6, 2024, 4:57 a.m. UTC | #2 
Hi Akhil,
    No changes in sequence of API's by adding 'uint8_t impl_opaque' to 'struct rte_crypto_op'.
It's required in case application/event dispatcher passes some implementation specific value in rte_event::impl_opaque, to restore the value
back on to rte_event::impl_opaque after enqueue to and dequeue from cryptodev.

Here is the pseudocode for one of the use case
Application/event dispatcher passes implementation specific value in rte_event::impl_opaque.
struct rte_event ev;
rte_event_dequeue_burst(..., &ev, ...)
struct rte_crypto_op *crypto_op = ev.event_ptr;   // ev.impl_opaque some implementation specific value
rte_cryptodev_enqueue_burst(..., crypto_op, ...) ; // ev.impl_opaque is not passed to crypto_op

With rte_crypto_op::impl_opaque field which is unchanged in library/driver
crypto_op->impl_opaque = ev.impl_opaque;
rte_cryptodev_enqueue_burst(..., crypto_op, ...) ;

...
rte_crypto_dequeue_burst(..., crypto_op, ...)
ev.event_ptr = crypto_op;
...
rte_event_enqueue_burst(..., &ev, ...);  // ev::impl_opaque value is lost

with rte_crypto_op::impl_opaque field
ev.event_ptr = crypto_op;
ev.impl_opaque = crypto_op->impl_opaque; // implementation specific value in rte_event::impl_opaque restored back
rte_event_enqueue_burst(..., &ev, ...);

Thanks,
Ganapati


From: Akhil Goyal <gakhil@marvell.com>
Sent: Tuesday, March 5, 2024 10:18 PM
To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; dpdk-dev <dev@dpdk.org>; fanzhang.oss@gmail.com; Ji, Kai <kai.ji@intel.com>; Power, Ciara <ciara.power@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Jerin Jacob <jerinjacobk@gmail.com>
Subject: RE: RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Hi Ganapati,

Can you please explain the flow with a sequence of APIs to be used.

Regards,
Akhil

From: Kundapura, Ganapati <ganapati.kundapura@intel.com<mailto:ganapati.kundapura@intel.com>>
Sent: Tuesday, March 5, 2024 12:44 PM
To: dpdk-dev <dev@dpdk.org<mailto:dev@dpdk.org>>; Akhil Goyal <gakhil@marvell.com<mailto:gakhil@marvell.com>>; fanzhang.oss@gmail.com<mailto:fanzhang.oss@gmail.com>; Ji, Kai <kai.ji@intel.com<mailto:kai.ji@intel.com>>; Power, Ciara <ciara.power@intel.com<mailto:ciara.power@intel.com>>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com<mailto:abhinandan.gujjar@intel.com>>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com<mailto:jay.jayatheerthan@intel.com>>; Jerin Jacob <jerinjacobk@gmail.com<mailto:jerinjacobk@gmail.com>>
Subject: [EXTERNAL] RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Prioritize security for external emails: Confirm sender and content safety before clicking links or opening attachments
Ganapati Kundapura March 12, 2024, 7:52 a.m. UTC | #3 
Hi DPDK,
   Any comments on this proposal?

Thanks,
Ganapati

From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
Sent: Wednesday, March 6, 2024 10:27 AM
To: Akhil Goyal <gakhil@marvell.com>; dpdk-dev <dev@dpdk.org>; fanzhang.oss@gmail.com; Ji, Kai <kai.ji@intel.com>; Power, Ciara <ciara.power@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Jerin Jacob <jerinjacobk@gmail.com>
Subject: RE: RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Hi Akhil,
    No changes in sequence of API's by adding 'uint8_t impl_opaque' to 'struct rte_crypto_op'.
It's required in case application/event dispatcher passes some implementation specific value in rte_event::impl_opaque, to restore the value
back on to rte_event::impl_opaque after enqueue to and dequeue from cryptodev.

Here is the pseudocode for one of the use case
Application/event dispatcher passes implementation specific value in rte_event::impl_opaque.
struct rte_event ev;
rte_event_dequeue_burst(..., &ev, ...)
struct rte_crypto_op *crypto_op = ev.event_ptr;   // ev.impl_opaque some implementation specific value
rte_cryptodev_enqueue_burst(..., crypto_op, ...) ; // ev.impl_opaque is not passed to crypto_op

With rte_crypto_op::impl_opaque field which is unchanged in library/driver
crypto_op->impl_opaque = ev.impl_opaque;
rte_cryptodev_enqueue_burst(..., crypto_op, ...) ;

...
rte_crypto_dequeue_burst(..., crypto_op, ...)
ev.event_ptr = crypto_op;
...
rte_event_enqueue_burst(..., &ev, ...);  // ev::impl_opaque value is lost

with rte_crypto_op::impl_opaque field
ev.event_ptr = crypto_op;
ev.impl_opaque = crypto_op->impl_opaque; // implementation specific value in rte_event::impl_opaque restored back
rte_event_enqueue_burst(..., &ev, ...);

Thanks,
Ganapati


From: Akhil Goyal <gakhil@marvell.com<mailto:gakhil@marvell.com>>
Sent: Tuesday, March 5, 2024 10:18 PM
To: Kundapura, Ganapati <ganapati.kundapura@intel.com<mailto:ganapati.kundapura@intel.com>>; dpdk-dev <dev@dpdk.org<mailto:dev@dpdk.org>>; fanzhang.oss@gmail.com<mailto:fanzhang.oss@gmail.com>; Ji, Kai <kai.ji@intel.com<mailto:kai.ji@intel.com>>; Power, Ciara <ciara.power@intel.com<mailto:ciara.power@intel.com>>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com<mailto:abhinandan.gujjar@intel.com>>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com<mailto:jay.jayatheerthan@intel.com>>; Jerin Jacob <jerinjacobk@gmail.com<mailto:jerinjacobk@gmail.com>>
Subject: RE: RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Hi Ganapati,

Can you please explain the flow with a sequence of APIs to be used.

Regards,
Akhil

From: Kundapura, Ganapati <ganapati.kundapura@intel.com<mailto:ganapati.kundapura@intel.com>>
Sent: Tuesday, March 5, 2024 12:44 PM
To: dpdk-dev <dev@dpdk.org<mailto:dev@dpdk.org>>; Akhil Goyal <gakhil@marvell.com<mailto:gakhil@marvell.com>>; fanzhang.oss@gmail.com<mailto:fanzhang.oss@gmail.com>; Ji, Kai <kai.ji@intel.com<mailto:kai.ji@intel.com>>; Power, Ciara <ciara.power@intel.com<mailto:ciara.power@intel.com>>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com<mailto:abhinandan.gujjar@intel.com>>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com<mailto:jay.jayatheerthan@intel.com>>; Jerin Jacob <jerinjacobk@gmail.com<mailto:jerinjacobk@gmail.com>>
Subject: [EXTERNAL] RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Prioritize security for external emails: Confirm sender and content safety before clicking links or opening attachments
Akhil Goyal March 12, 2024, 8:10 a.m. UTC | #4 
Hi Ganapati,

Is it not possible to use rte_event_crypto_adapter_enqueue
if you want to send the event context to cryptodev?
While using rte_cryptodev_enqueue() all previous stage event context is meant to be lost and
It would send a new crypto request to cryptodev and is not supposed to be aware of event context.

P.S. Please fix your mail client to reply in plain text on mailing list.

Regards,
Akhil

From: Kundapura, Ganapati <ganapati.kundapura@intel.com>
Sent: Tuesday, March 12, 2024 1:22 PM
To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Akhil Goyal <gakhil@marvell.com>; dpdk-dev <dev@dpdk.org>; fanzhang.oss@gmail.com; Ji, Kai <kai.ji@intel.com>; Power, Ciara <ciara.power@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Jerin Jacob <jerinjacobk@gmail.com>
Subject: [EXTERNAL] RE: RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Prioritize security for external emails: Confirm sender and content safety before clicking links or opening attachments
Ganapati Kundapura March 12, 2024, 12:02 p.m. UTC | #5 
Hi Akhil,
   Please find my response in lined.

Thanks,
Ganapati

From: Akhil Goyal <gakhil@marvell.com>
Sent: Tuesday, March 12, 2024 1:41 PM
To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; dpdk-dev <dev@dpdk.org>; fanzhang.oss@gmail.com; Ji, Kai <kai.ji@intel.com>; Power, Ciara <ciara.power@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Jerin Jacob <jerinjacobk@gmail.com>
Subject: RE: RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Hi Ganapati,

Is it not possible to use rte_event_crypto_adapter_enqueue
if you want to send the event context to cryptodev?
[Ganapati] No, event crypto adapter sends only ev::event_ptr as rte_crypto_op to cryptodev and not event context.

While using rte_cryptodev_enqueue() all previous stage event context is meant to be lost and
It would send a new crypto request to cryptodev and is not supposed to be aware of event context.
[Ganapati] Yes, proposal is for sending implementation specific value from eventdev to crypodev and vice versa

P.S. Please fix your mail client to reply in plain text on mailing list.
[Ganapati] Done

Regards,
Akhil

From: Kundapura, Ganapati <ganapati.kundapura@intel.com<mailto:ganapati.kundapura@intel.com>>
Sent: Tuesday, March 12, 2024 1:22 PM
To: Kundapura, Ganapati <ganapati.kundapura@intel.com<mailto:ganapati.kundapura@intel.com>>; Akhil Goyal <gakhil@marvell.com<mailto:gakhil@marvell.com>>; dpdk-dev <dev@dpdk.org<mailto:dev@dpdk.org>>; fanzhang.oss@gmail.com<mailto:fanzhang.oss@gmail.com>; Ji, Kai <kai.ji@intel.com<mailto:kai.ji@intel.com>>; Power, Ciara <ciara.power@intel.com<mailto:ciara.power@intel.com>>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com<mailto:arkadiuszx.kusztal@intel.com>>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com<mailto:abhinandan.gujjar@intel.com>>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com<mailto:jay.jayatheerthan@intel.com>>; Jerin Jacob <jerinjacobk@gmail.com<mailto:jerinjacobk@gmail.com>>
Subject: [EXTERNAL] RE: RFC: Using and renaming 8-bit reserved field of rte_crypto_op for implementation specific

Prioritize security for external emails: Confirm sender and content safety before clicking links or opening attachments
Akhil Goyal March 13, 2024, 6:07 p.m. UTC | #6 
Hi Ganapati,

>> Is it not possible to use rte_event_crypto_adapter_enqueue
>> if you want to send the event context to cryptodev?
> [Ganapati] No, event crypto adapter sends only ev::event_ptr as rte_crypto_op to cryptodev and not event context.

>> While using rte_cryptodev_enqueue() all previous stage event context is meant to be lost and 
>> It would send a new crypto request to cryptodev and is not supposed to be aware of event context.
> [Ganapati] Yes, proposal is for sending implementation specific value from eventdev to crypodev and vice versa

As discussed in a separate mail thread, 
impl_opaque in rte_crypto_op is not generic and is specific to your use case.
The crypto dev wont be able to make difference whether to alter it (for any other usecase) or not(for event case).
As per definition of impl_opaque is meant to be consumed by driver. Hence this will contradict the usage.
Since impl_opaque in rte_crypto_op is exposed to driver. Drivers are free to use it.

You may consider using mbuf dynamic fields for setting some userdata for your use case for each packet.
See rte_security_dynfield and add a fastpath cryptodev API to set pkt userdata which is opaque to driver
(similar to rte_security_set_pkt_metadata).
Or else add a similar schema to introduce dynamic fields in rte_crypto_op.

Regards,
Akhil
diff mbox series

Patch

diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
index dbc2700..af46ec9 100644
--- a/lib/cryptodev/rte_crypto.h
+++ b/lib/cryptodev/rte_crypto.h
@@ -146,10 +146,13 @@  struct rte_crypto_op {
                                /**< TLS record */
                        } param1;
                        /**< Additional per operation parameter 1. */
-                       uint8_t reserved[1];
-                       /**< Reserved bytes to fill 64 bits for
-                        * future additions
+                       uint8_t impl_opaque;
+                       /**< Implementation specific opaque value.
+                        * An implementation may use this field to hold
+                        * implementation specific value to share between
+                        * dequeue and enqueue operation.
                         */
+

which is untouched in library/driver and rte_event::impl_opaque field can be restored
while enqueuing the event back to eventdev.