| Message ID | 20170616104940.20166-1-petri.savolainen@linaro.org |
|---|---|
| Headers | show |
| Series | IPSEC packet event | expand |
This should still be reviewed by Bala and Nikhil with respect to their HW, but these changes resolve all of the concerns I had with the original version. For his series: Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> On Fri, Jun 16, 2017 at 5:49 AM, Petri Savolainen <petri.savolainen@linaro.org> wrote: > Applies on top of: "[API-NEXT PATCH v2 0/2] IPsec API update" > > Input and output of IPSEC operations are packets. Parameter and > result structures are cleaner when packet arrays are direct > parameters to functions. Also API is more flexible for > application or API pipelining when output is packets with > additional metadata. Application or API pipeline stages which > do not care about IPSEC results may work on basic packet metadata. > > v3: > * typo corrected (Bill) > > v2: > * Introduce event subtype > * event type "packet" is the same for all packets > * event subtype "packet_ipsec" indicates that a packet contain IPSEC results > * Added output packet format description, which was accidentally left out > from previous version (moved text from deleted result type to > odp_ipsec_in() / odp_ipsec_out() documentation) > > > Petri Savolainen (3): > api: event: add subtype to expand event type > api: ipsec: change IPSEC result to packet > api: ipsec: disable event is the last event > > include/odp/api/spec/event.h | 80 +++- > include/odp/api/spec/ipsec.h | 417 ++++++++++++--------- > include/odp/arch/default/api/abi/event.h | 9 +- > .../include/odp/api/plat/event_types.h | 8 +- > platform/linux-generic/odp_ipsec.c | 67 +++- > 5 files changed, 372 insertions(+), 209 deletions(-) > > -- > 2.13.0 >
Some queries: - Now since IPSEC, and simple IPSEC packets are delivered through same ODP_EVENT_TYPE_PACKET, how does application knows whether L2_HEADER was valid?? Does application always have to check on subtype? - Also, do we need separate odp_ipsec_packet_to_event APIs(and vice versa) now? If yes, it will defeat the purpose of subtypes. - Does error in IPSEC operation affects, packet error flags?? - Since IN/OUT params for IPSEC operations are different, does NUM_SA=0 is a valid input for out_sa_params? - ODP_EVENT_IPSEC_STATUS events are delivered on SA queues or default queue? - Will per packet checksum override work in conjunction with IPSEC_SA settings as well? Regards Nikhil -----Original Message----- From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Bill Fischofer Sent: Saturday, June 17, 2017 3:46 AM To: Petri Savolainen <petri.savolainen@linaro.org> Cc: lng-odp-forward <lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [API-NEXT PATCH v3 0/3] IPSEC packet event This should still be reviewed by Bala and Nikhil with respect to their HW, but these changes resolve all of the concerns I had with the original version. For his series: Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> On Fri, Jun 16, 2017 at 5:49 AM, Petri Savolainen <petri.savolainen@linaro.org> wrote: > Applies on top of: "[API-NEXT PATCH v2 0/2] IPsec API update" > > Input and output of IPSEC operations are packets. Parameter and result > structures are cleaner when packet arrays are direct parameters to > functions. Also API is more flexible for application or API pipelining > when output is packets with additional metadata. Application or API > pipeline stages which do not care about IPSEC results may work on > basic packet metadata. > > v3: > * typo corrected (Bill) > > v2: > * Introduce event subtype > * event type "packet" is the same for all packets > * event subtype "packet_ipsec" indicates that a packet contain IPSEC results > * Added output packet format description, which was accidentally left out > from previous version (moved text from deleted result type to > odp_ipsec_in() / odp_ipsec_out() documentation) > > > Petri Savolainen (3): > api: event: add subtype to expand event type > api: ipsec: change IPSEC result to packet > api: ipsec: disable event is the last event > > include/odp/api/spec/event.h | 80 +++- > include/odp/api/spec/ipsec.h | 417 ++++++++++++--------- > include/odp/arch/default/api/abi/event.h | 9 +- > .../include/odp/api/plat/event_types.h | 8 +- > platform/linux-generic/odp_ipsec.c | 67 +++- > 5 files changed, 372 insertions(+), 209 deletions(-) > > -- > 2.13.0 >
> -----Original Message----- > From: Nikhil Agarwal [mailto:nikhil.agarwal@nxp.com] > Sent: Monday, June 19, 2017 2:07 PM > To: Bill Fischofer <bill.fischofer@linaro.org>; Petri Savolainen > <petri.savolainen@linaro.org> > Cc: lng-odp-forward <lng-odp@lists.linaro.org> > Subject: RE: [lng-odp] [API-NEXT PATCH v3 0/3] IPSEC packet event > > Some queries: > > - Now since IPSEC, and simple IPSEC packets are delivered through same > ODP_EVENT_TYPE_PACKET, how does application knows whether L2_HEADER was > valid?? Does application always have to check on subtype? There are many options. 1) Application may know that certain queue will deliver only basic or only ipsec packets, thus no check needed. 2) Application can directly check subtype (basic vs ipsec packet). 3) Application can check both types in series. 4) If an application pipeline stage does not care about L2, but only those things that are common to all packet types, it does not need to subtype... > - Also, do we need separate odp_ipsec_packet_to_event APIs(and vice versa) > now? If yes, it will defeat the purpose of subtypes. " Each event subtype may define specific functions (e.g. odp_ipsec_packet_from_event()) to convert between the generic event handle (odp_event_t) and event type specific handle (e.g. odp_packet_t). When subtype is known, these subtype specific functions should be preferred over the event type general function (e.g. odp_packet_from_event())." Subtype specific functions are optional but preferred. Those may have better performance as implementation does not need to check types. > - Does error in IPSEC operation affects, packet error flags?? Currently, not specified to do so. It's possible to do that if it's considered necessary. Although, I'd do it in another patch since after this the spec is clear about metadata: ipsec_packet has ipsec related metadata, basic packet does not. > - Since IN/OUT params for IPSEC operations are different, does NUM_SA=0 is > a valid input for out_sa_params? The patch says: " * Valid values are: * - 1: Single SA for all packets * - N: A SA per packet. N must match the number of packets. */ int num_sa; " So, zero SAs is not a valid value for output. > - ODP_EVENT_IPSEC_STATUS events are delivered on SA queues or default > queue? Nothing changed here: events are sent to SA queue when the SA is known and to the default queue when the SA is not known. > - Will per packet checksum override work in conjunction with IPSEC_SA > settings as well? Nothing changed here. Current text in the repo is still valid: /** * Configuration options for IPSEC outbound processing */ typedef struct odp_ipsec_outbound_config_t { /** Flags to control L3/L4 checksum insertion as part of outbound * packet processing. Packet must have set with valid L3/L4 offsets. * Checksum configuration is ignored for packets that checksum cannot * be computed for (e.g. IPv4 fragments). Application may use a packet * metadata flag to disable checksum insertion per packet bases. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ */ union { -Petri
For the series: Reviewed-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> On 16 June 2017 at 16:19, Petri Savolainen <petri.savolainen@linaro.org> wrote: > Applies on top of: "[API-NEXT PATCH v2 0/2] IPsec API update" > > Input and output of IPSEC operations are packets. Parameter and > result structures are cleaner when packet arrays are direct > parameters to functions. Also API is more flexible for > application or API pipelining when output is packets with > additional metadata. Application or API pipeline stages which > do not care about IPSEC results may work on basic packet metadata. > > v3: > * typo corrected (Bill) > > v2: > * Introduce event subtype > * event type "packet" is the same for all packets > * event subtype "packet_ipsec" indicates that a packet contain IPSEC > results > * Added output packet format description, which was accidentally left out > from previous version (moved text from deleted result type to > odp_ipsec_in() / odp_ipsec_out() documentation) > > > Petri Savolainen (3): > api: event: add subtype to expand event type > api: ipsec: change IPSEC result to packet > api: ipsec: disable event is the last event > > include/odp/api/spec/event.h | 80 +++- > include/odp/api/spec/ipsec.h | 417 > ++++++++++++--------- > include/odp/arch/default/api/abi/event.h | 9 +- > .../include/odp/api/plat/event_types.h | 8 +- > platform/linux-generic/odp_ipsec.c | 67 +++- > 5 files changed, 372 insertions(+), 209 deletions(-) > > -- > 2.13.0 > >
For the series: Reviewed-by: Nikhil Agarwal <Nikhil.agarwal@linaro.org> -----Original Message----- From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Bala Manoharan Sent: Wednesday, June 21, 2017 6:58 PM To: Petri Savolainen <petri.savolainen@linaro.org> Cc: lng-odp-forward <lng-odp@lists.linaro.org> Subject: Re: [lng-odp] [API-NEXT PATCH v3 0/3] IPSEC packet event For the series: Reviewed-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> On 16 June 2017 at 16:19, Petri Savolainen <petri.savolainen@linaro.org> wrote: > Applies on top of: "[API-NEXT PATCH v2 0/2] IPsec API update" > > Input and output of IPSEC operations are packets. Parameter and result > structures are cleaner when packet arrays are direct parameters to > functions. Also API is more flexible for application or API pipelining > when output is packets with additional metadata. Application or API > pipeline stages which do not care about IPSEC results may work on > basic packet metadata. > > v3: > * typo corrected (Bill) > > v2: > * Introduce event subtype > * event type "packet" is the same for all packets > * event subtype "packet_ipsec" indicates that a packet contain > IPSEC results > * Added output packet format description, which was accidentally left out > from previous version (moved text from deleted result type to > odp_ipsec_in() / odp_ipsec_out() documentation) > > > Petri Savolainen (3): > api: event: add subtype to expand event type > api: ipsec: change IPSEC result to packet > api: ipsec: disable event is the last event > > include/odp/api/spec/event.h | 80 +++- > include/odp/api/spec/ipsec.h | 417 > ++++++++++++--------- > include/odp/arch/default/api/abi/event.h | 9 +- > .../include/odp/api/plat/event_types.h | 8 +- > platform/linux-generic/odp_ipsec.c | 67 +++- > 5 files changed, 372 insertions(+), 209 deletions(-) > > -- > 2.13.0 > >
On 16 June 2017 at 13:49, Petri Savolainen <petri.savolainen@linaro.org> wrote: > Applies on top of: "[API-NEXT PATCH v2 0/2] IPsec API update" > > Input and output of IPSEC operations are packets. Parameter and > result structures are cleaner when packet arrays are direct > parameters to functions. Also API is more flexible for > application or API pipelining when output is packets with > additional metadata. Application or API pipeline stages which > do not care about IPSEC results may work on basic packet metadata. For the whole series: Reviewed-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> -- With best wishes Dmitry
Applies on top of: "[API-NEXT PATCH v2 0/2] IPsec API update" Input and output of IPSEC operations are packets. Parameter and result structures are cleaner when packet arrays are direct parameters to functions. Also API is more flexible for application or API pipelining when output is packets with additional metadata. Application or API pipeline stages which do not care about IPSEC results may work on basic packet metadata. v3: * typo corrected (Bill) v2: * Introduce event subtype * event type "packet" is the same for all packets * event subtype "packet_ipsec" indicates that a packet contain IPSEC results * Added output packet format description, which was accidentally left out from previous version (moved text from deleted result type to odp_ipsec_in() / odp_ipsec_out() documentation) Petri Savolainen (3): api: event: add subtype to expand event type api: ipsec: change IPSEC result to packet api: ipsec: disable event is the last event include/odp/api/spec/event.h | 80 +++- include/odp/api/spec/ipsec.h | 417 ++++++++++++--------- include/odp/arch/default/api/abi/event.h | 9 +- .../include/odp/api/plat/event_types.h | 8 +- platform/linux-generic/odp_ipsec.c | 67 +++- 5 files changed, 372 insertions(+), 209 deletions(-) -- 2.13.0