On Mon, Oct 17, 2016 at 8:54 AM, Ola Liljedahl <ola.liljed...@linaro.org>

> 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.

> 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

> 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. 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