| Message ID | 20170609105143.11350-1-petri.savolainen@linaro.org |
|---|---|
| Headers | show |
| Series | IPSEC packet event | expand |
Petri, On 09.06.2017 13:51, Petri Savolainen wrote: > Requires "[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. Two generic issues: - I thought that basic idea was to generate ODP_EVENT_PACKET and then use API call to check if there is IPsec metadata associated with it? - First patch contains several 'extra' changes. Can we work towards splitting it into more manageable pieces? -- With best wishes Dmitry
> -----Original Message----- > From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsolenikov@linaro.org] > Sent: Friday, June 09, 2017 2:49 PM > To: Petri Savolainen <petri.savolainen@linaro.org>; lng- > odp@lists.linaro.org > Subject: Re: [lng-odp] [API-NEXT PATCH 0/2] IPSEC packet event > > Petri, > > On 09.06.2017 13:51, Petri Savolainen wrote: > > Requires "[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. > > Two generic issues: > > - I thought that basic idea was to generate ODP_EVENT_PACKET and then > use API call to check if there is IPsec metadata associated with it? > We ended up with defining odp_event_type() to be the call to check if there's extra metadata. The reason is that when ipsec decrypt fails, data is garbage, so application needs to check ipsec status always for those packets. With single event type, application would need to do the ipsec check also for non-ipsec packets. This API keeps normal packets as is. > - First patch contains several 'extra' changes. Can we work towards > splitting it into more manageable pieces? Which ones you feel disturbing? Some of those are consequence of editing the corresponding comment block anyway (e.g. for the event change). I would not want to edit the same comment block two times: first in the minimal way possible, and then rewrite it for better quality in a next patch. Just comment on those places you disagree with the new text. -Petri
On Fri, Jun 9, 2017 at 7:06 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> wrote: > > >> -----Original Message----- >> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsolenikov@linaro.org] >> Sent: Friday, June 09, 2017 2:49 PM >> To: Petri Savolainen <petri.savolainen@linaro.org>; lng- >> odp@lists.linaro.org >> Subject: Re: [lng-odp] [API-NEXT PATCH 0/2] IPSEC packet event >> >> Petri, >> >> On 09.06.2017 13:51, Petri Savolainen wrote: >> > Requires "[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. >> >> Two generic issues: >> >> - I thought that basic idea was to generate ODP_EVENT_PACKET and then >> use API call to check if there is IPsec metadata associated with it? >> > > We ended up with defining odp_event_type() to be the call to check if there's extra metadata. The reason is that when ipsec decrypt fails, data is garbage, so application needs to check ipsec status always for those packets. With single event type, application would need to do the ipsec check also for non-ipsec packets. This API keeps normal packets as is. It's sufficient for this to be reflected in the general packet error bits so application code that doesn't care about IPsec details simply knows that this packet has an error. Please see my earlier post about odp_packet_type(). > > >> - First patch contains several 'extra' changes. Can we work towards >> splitting it into more manageable pieces? > > Which ones you feel disturbing? Some of those are consequence of editing the corresponding comment block anyway (e.g. for the event change). I would not want to edit the same comment block two times: first in the minimal way possible, and then rewrite it for better quality in a next patch. Just comment on those places you disagree with the new text. > > -Petri > >
> -----Original Message----- > From: Bill Fischofer [mailto:bill.fischofer@linaro.org] > Sent: Friday, June 09, 2017 6:27 PM > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> > Cc: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>; lng- > odp@lists.linaro.org > Subject: Re: [lng-odp] [API-NEXT PATCH 0/2] IPSEC packet event > > On Fri, Jun 9, 2017 at 7:06 AM, Savolainen, Petri (Nokia - FI/Espoo) > <petri.savolainen@nokia.com> wrote: > > > > > >> -----Original Message----- > >> From: Dmitry Eremin-Solenikov > [mailto:dmitry.ereminsolenikov@linaro.org] > >> Sent: Friday, June 09, 2017 2:49 PM > >> To: Petri Savolainen <petri.savolainen@linaro.org>; lng- > >> odp@lists.linaro.org > >> Subject: Re: [lng-odp] [API-NEXT PATCH 0/2] IPSEC packet event > >> > >> Petri, > >> > >> On 09.06.2017 13:51, Petri Savolainen wrote: > >> > Requires "[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. > >> > >> Two generic issues: > >> > >> - I thought that basic idea was to generate ODP_EVENT_PACKET and then > >> use API call to check if there is IPsec metadata associated with it? > >> > > > > We ended up with defining odp_event_type() to be the call to check if > there's extra metadata. The reason is that when ipsec decrypt fails, data > is garbage, so application needs to check ipsec status always for those > packets. With single event type, application would need to do the ipsec > check also for non-ipsec packets. This API keeps normal packets as is. > > It's sufficient for this to be reflected in the general packet error > bits so application code that doesn't care about IPsec details simply > knows that this packet has an error. Please see my earlier post about > odp_packet_type(). > Packet type would result two step check for every packet. At least for every ipsec/crypto/comp packet: type = event_type(ev) if (type == packet) pkt = packet_from_event(ev) // generic subtype = packet_type(pkt) if (subtype == packet_ipsec) // 2nd branch foo(); else bar(); Direct event type is more efficient for both application and implementation. Also implementation can e.g. finalize IPSEC processing during the IPSEC specific conversion function: type = event_type(ev) if (type == packet_ipsec) pkt = packet_ipsec_from_event(ev) // ipsec speficic foo(); else (type == packet_ipsec) pkt = packet_from_event(ev) // basic pkt specific bar(); Class is a backward compatible addition later on: class = event_class(ev) if (class == packet) pkt = packet_from_class(ev) // generic bar(); else -Petri