On 17 October 2016 at 16:34, Bill Fischofer <bill.fischo...@linaro.org>
wrote:

>
>
> On Mon, Oct 17, 2016 at 8:54 AM, Ola Liljedahl <ola.liljed...@linaro.org>
> wrote:
>
>> On 10 October 2016 at 17:50, Bill Fischofer <bill.fischo...@linaro.org>
>> wrote:
>>
>>> This patch adds support for packet references and splices following
>>> discussions at LAS16 on this subject.
>>>
>>> I've changed things around from Petri's original proposal by splitting
>>> this
>>> into two separate APIs: odp_packet_splice() and odp_packet_ref(), where
>>> the
>>> latter is just a splice of a zero-length header on to a base packet. The
>>> various odp packet manipulation APIs have also been enhanced to behave
>>> sensibly when presented with a spliced packet as input. Reference counts
>>> are
>>> used to enable odp_packet_free() to not free a packet until all splices
>>> based
>>> on it are also freed.
>>>
>> Seems OK.
>>
>>
>>>
>>> Also added are two new APIs for working with spliced packets as these
>>> seem
>>> necessary for completeness:
>>>
>>> - odp_packet_is_a_splice() tells whether an input packet is a splice,
>>> and if
>>> so how many spliced packets it contains
>>>
>>> - odp_packet_is_spliced() tells whether any splices have been created on
>>> this
>>> packet, and if so how many.
>>>
>> Is there any conceptual difference between the base packet and any
>> splices?
>>
>> Can you add a splice to a splice?
>>
>
> Architecturally, yes. Compound splices of arbitrary depth can be
> accommodated , however not every implementation may be able to support
> this. I'd expect us to need to add some odp_packet_capability() info to
> allow implementation limits to be communicated to applications, but we need
> to specify what sort of things we want to be able to limit this way.
>
>
>>
>>
>>>
>>> Note that there is no odp_packet_unsplice() API. To remove a splice from
>>> a
>>> base packet currently requres that the splice be freed via an
>>> odp_packet_free() call. We should discuss and decide if such an API is
>>> warranted for symmetry.
>>>
>> As long as odp_packet_free() respects any additional references (to
>> segments)
>> caused by splicing, I don't think we need any free splice call. How would
>> it be
>> different from odp_packet_free()?
>>
>
> After splicing the header packet becomes the handle for the resulting
> splice, while the handle for the base packet is still valid. An unsplice
> operation would separate these so that what was the splice handle becomes
> just a handle to the header, which can then still be used. Not sure if
> there is a use case for this. It would basically be an "undo" on the splice.
>
Aha.


>
>
>>
>> What happens when you truncate the tail? E.g. of the base packet?
>> Does this affect the splices? (I prefer not).
>>
>
> Yes. After a successful splice, all offsets beyond the splice point are
> shared with all other splices so any change to that part of the splice is
> visible to all other splices on the same base packet. For this reason it's
> suggested that the shared portion be treated as read only. However, if the
> application changes the base packet or an offset in a splice that is
> visible to other splices, then it's the application responsibility to
> ensure that proper synchronization is performed to avoid unpredictable
> behavior.
>
Is it really unpredictable?
If you change packet data through one handle, all other handles referring
to the same segment should also see this change.

And implementation that implements splicing through copying will not see
such changes. So perhaps implementation dependent behaviour.



>
>
>>
>> To use this splice API for fragmentation, you want to be able to add
>> splices with new
>> L2/IP headers at suitable places (at each MTU point) and then truncate
>> the base packet
>> so that only the just added splice refers the the data beyond the new end
>> of the base
>> packet.
>>
>
> That's not a use case that's currently supported as a splice is created
> with an offset only rather than an offset and length, so splices go from
> the specified offset through the end of the base packet.
>
That's OK. That's what I want. What I don't want it that if I truncate the
length of the packet through one handle, that should affect the other
handles. Is that behaviour really intrinsic to the splice API definition?


> If we wanted we could expand the API to accommodate a length. We should
> discuss further. The concern here is whether such extensions would be
> efficiently implementable on all platforms.
>
>
>>
>> It would be useful to use the API for reassembly. Splice fragment N onto
>> fragment N+1
>> (after the IP header of frag N+1) and free fragment N+1. Repeat for all
>> fragments from
>> last to first. The splice for the first fragment will then refer to the
>> complete reassembled
>> datagram.
>>
>> It seems like these use cases should be supported by this API if the
>> specification is
>> written with this in mind.
>>
>
> That would be a reasonable use case with the ability to specify a length
> as well as an offset on odp_packet_splice(), however this would also
> require explicit support for nested splices since ipfrag reassembly can
> involve many such operations.
>
>
>>
>> I will check the later patches as well. We have a bunch of use cases
>> define here:
>> https://docs.google.com/document/d/1r4olxr39fHvgFACQp_1RL_
>> mh9736no3TFaVM7_XkUtM/edit
>>
>>
> I'll look at that doc and add comments there. I notice it's viewable but
> not commentable, please enable comments on that doc.
>
>
>>
>>
>>>
>>> Changes for v4:
>>> - Add negative tests to validation test suite
>>> - Fix implementation bugs relating to negative tests
>>>
>>> Changes for v3:
>>> - Bug fixes (detected by the validation tests)
>>> - Addition of validation tests for these new APIs
>>> - Diagrams and User Guide documentation for these new APIs
>>>
>>> Changes for v2:
>>> - Bug fixes
>>> - Enhance ODP packet segment APIs to behave properly with spliced packets
>>>
>>> Bill Fischofer (5):
>>>   api: packet: add support for packet splices and references
>>>   linux-generic: packet: implement splice/reference apis
>>>   validation: packet: add packet splice/reference tests
>>>   doc: images: add images for packet splice/reference documentation
>>>   doc: userguide: add user documentation for packet splice/reference
>>>     APIs
>>>
>>>  doc/images/doublesplice.svg                        |  67 ++++++
>>>  doc/images/pktref.svg                              |  49 +++++
>>>  doc/images/splice.svg                              |  64 ++++++
>>>  doc/users-guide/users-guide-packet.adoc            | 118 ++++++++++
>>>  include/odp/api/spec/packet.h                      | 103 +++++++++
>>>  .../linux-generic/include/odp_packet_internal.h    |  54 ++++-
>>>  platform/linux-generic/odp_packet.c                | 241
>>> ++++++++++++++++++---
>>>  test/common_plat/validation/api/packet/packet.c    | 176
>>> +++++++++++++++
>>>  test/common_plat/validation/api/packet/packet.h    |   1 +
>>>  9 files changed, 836 insertions(+), 37 deletions(-)
>>>  create mode 100644 doc/images/doublesplice.svg
>>>  create mode 100644 doc/images/pktref.svg
>>>  create mode 100644 doc/images/splice.svg
>>>
>>> --
>>> 2.7.4
>>>
>>>
>>
>

Reply via email to