On 12.04.2017 12:44, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of >> Dmitry Eremin-Solenikov >> Sent: Tuesday, April 11, 2017 2:02 AM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [PATCH] api: ipsec: change semantics of >> odp_ipsec_result function >> >> - Move packets from the event instead of copying them. This simplifies >> event handling/freeing code, which now does not have to track, which >> packets were copied from the event and which packets should be freed. > > Current spec is stateless. If array is too small in the first call, > application must call again with larger array. Application must not > proceeding to access packets, before large enough array is used and result > event is "freed".
I think this can be troublesome for the application. Because if the array is too small, it has to dynamically allocate new array (be it a call to malloc, alloca or just dynamic on-stack arrays with variable size). If API allows the application to do 'partial processing', there would be no dynamic allocation inside the app at this point. Are we OK with dynamic allocation at this point? >> - Do not require to free the event before processing packets. This >> allows one to copy packets from the event in small batches and >> process them accordingly. > > This *disallows* implementation to use the packet as the results event. I remember we talked about ipsec-result-event-as-packet on the previous arch call. I gave it a though. IIRC, your idea was that odp_event_free in this case can just change the type of the event (from IPSEC_RESULT to PACKET). And I had some troubles with this idea. Because now odp_event_free() combines two different code paths for IPSEC_RESULT: - If the event was successfully passed through odp_ipsec_result(), just change the event type. - If it did not pass through odp_ipsec_result() or if the call did not succeed, actually free the event/packet. This introduces extra state into the IPSEC_RESULT context data. > Current spec allows implementation (e.g HW) to enqueue a packet with an IPSEC > flag set, and use that as the result event. The conversion function + free > can modify the event type from IPSEC result to packet. This is how crypto > works today, although the spec is not explicit enough about it. I see your point, I dislike this 'don't actually free' behaviour of odp_event_free() if the odp_ipsec_result() was successful. What if we change the requirements in the following way: If the event was fully processed by odp_ipsec_result(), application may not call odp_event_free() on it. Successful odp_ipsec_result() consumes and frees the event on its own. What do you think? -- With best wishes Dmitry
