diff mbox

[API-NEXT,PATCHv6,1/5] api: packet: add support for packet splices and references

Message ID 1476157506-11032-2-git-send-email-bill.fischofer@linaro.org
State New
Headers show

Commit Message

Bill Fischofer Oct. 11, 2016, 3:45 a.m. UTC
Introduce four new APIs that support efficient sharing of portions of
packets.

odp_packet_splice() creates a reference to a base packet by splicing a
supplied header packet onto it at a specified offset. If multiple splices
are created then each shares the base packet that was spliced.

odp_packet_ref() creates a reference to a base packet by splicing a
zero-length header onto it at a specified offset. This allows an
application to "hold onto" a pointer to a packet so that it will not be
freed until all references to it are freed.

odp_packet_is_a_splice() allows an application to determine whether a
packet is a splice and if so how many individual packets are contained
within it.

odp_packet_is_spliced() allows an application to determine whether a packet
has a splice on it and if so how many splices are based on it.

Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

---
 include/odp/api/spec/packet.h | 103 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

-- 
2.7.4

Comments

Balasubramanian Manoharan Oct. 21, 2016, 1:21 p.m. UTC | #1
Regards,
Bala


On 11 October 2016 at 09:15, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> Introduce four new APIs that support efficient sharing of portions of

> packets.

>

> odp_packet_splice() creates a reference to a base packet by splicing a

> supplied header packet onto it at a specified offset. If multiple splices

> are created then each shares the base packet that was spliced.

>

> odp_packet_ref() creates a reference to a base packet by splicing a

> zero-length header onto it at a specified offset. This allows an

> application to "hold onto" a pointer to a packet so that it will not be

> freed until all references to it are freed.

>

> odp_packet_is_a_splice() allows an application to determine whether a

> packet is a splice and if so how many individual packets are contained

> within it.

>

> odp_packet_is_spliced() allows an application to determine whether a packet

> has a splice on it and if so how many splices are based on it.

>

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

>  include/odp/api/spec/packet.h | 103 ++++++++++++++++++++++++++++++++++++++++++

>  1 file changed, 103 insertions(+)

>

> diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h

> index 4a14f2d..8e147a3 100644

> --- a/include/odp/api/spec/packet.h

> +++ b/include/odp/api/spec/packet.h

> @@ -844,6 +844,109 @@ int odp_packet_concat(odp_packet_t *dst, odp_packet_t src);

>   */

>  int odp_packet_split(odp_packet_t *pkt, uint32_t len, odp_packet_t *tail);

>

> +/**

> + * Splice a packet

> + *

> + * Create a new packet by splicing a header onto an existing packet. The

> + * spliced packet consists of the header followed by a shared reference to the

> + * base packet receiving the splice starting at a designated offset. If

> + * multiple splices are created on the same base packet, it is the

> + * application's responsibility to coordinate any changes to the shared

> + * segment(s) of the splice, otherwise results are undefined.

> + *

> + * @param hdr    Handle of the header packet to be spliced onto the base packet

> + *

> + * @param pkt    Handle of the base packet that is to receive the splice

> + *

> + * @param offset Byte offset within the base packet at which the splice is

> + *               to be made.

> + *

> + * @return       Handle of the spliced packet. This may or may not be the

> + *               same as the input hdr packet. The caller should use this

> + *               for any future references to the splice. The original hdr

> + *               packet no longer has any independent existence.

> + *

> + * @retval ODP_PACKET_INVALID If the splice failed. In this case both hdr

> + *                            and pkt are unchanged.

> + *

> + * @note The base pkt remains valid after the completion of the splice,

> + *       however changes that affect its length may yield unpredictable

> + *       results when viewed through the returned splice handle. For best

> + *       portability and predictable behavior, applications should regard the

> + *       base packet as read only following a successful splice, with the

> + *       exception of tailroom manipulation (which still requires application

> + *       coordination if multiple splices exist on the same base packet whose

> + *       tail is being pushed or pulled).

> + *

> + * @note Either packet input to this routine may itself be a splice, however

> + *       individual implementations may impose limits on how deeply splices

> + *       may be nested and fail the attempted splice if those limits are

> + *       exceeded.

> + *

> + * @note The packets used to create a splice may reside in different pools,

> + *       however individual implementations may require that both reside in

> + *       the same pool and fail the attempted splice if this restriction is

> + *       not observed. Upon return odp_packet_pool() returns the pool id of

> + *       the header packet.

> + *

> + * @note Once successfully spliced, the base packet may be freed via

> + *       odp_packet_free(), however the storage used to represent it will not

> + *       be released until all splices based on it have themselves been freed.


Do you have a reason why the 'base' packet will be freed by the
application after creating the slice?

> + */

> +odp_packet_t odp_packet_splice(odp_packet_t hdr,

> +                              odp_packet_t pkt, uint32_t offset);


Do we have an use-case for this scenario?
I would prefer that the API take the entire second packet instead of
an 'offset' at-least in the first version and that the 'hdr' handle
becomes invalid once the packet has been sliced and the new handle
returned replaces the 'hdr' handle.

> +

> +/**

> + * Create a reference to a packet

> + *

> + * Create a (shared) reference to a base packet starting at a specified

> + * byte offset. A reference is simply a splice with a zero-length header.

> + *

> + * @param pkt    Handle of the base packet for which a reference is to be

> + *               created.

> + * @param offset Byte offset in the base packet at which shared reference is

> + *               to begin.

> + *

> + * @return                    Handle of the reference packet

> + * @retval ODP_PACKET_INVALID Operation failed. Base packet remains unchanged.

> + *

> + * @note The zero-length header used to create a packet reference is always

> + *       drawn from the same pool as the base packet.

> + *

> + * @note Because references are a type of splice, any bytes pushed onto the

> + *       head of a reference (via odp_packet_push_head() or

> + *       odp_packet_extend_head() are unique to this reference and are not

> + *       seen by any other reference to the same base packet.

> + */

> +odp_packet_t odp_packet_ref(odp_packet_t pkt, uint32_t offset);


In the first version we can drop the offset and create a reference of
the entire packet.

> +

> +/**

> + * Tests if a packet is a splice

> + *

> + * A packet is a splice if it was created by odp_packet_splice() or

> + * odp_packet_ref(). Note that a splice may be a simple splice or a

> + * compound splice (a splice created from another splice).

> + *

> + * @param pkt Packet Handle

> + *

> + * @retval 0 Packet is not a splice

> + * @retval >0 Packet is a splice containing N individual packets

> + */

> +int odp_packet_is_a_splice(odp_packet_t pkt);

> +

> +/**

> + * Tests if a packet is spliced

> + *

> + * A packet is spliced if a splice was created on it by odp_packet_splice()

> + * or odp_packet_ref().

> + *

> + * @param pkt Packet Handle

> + *

> + * @retval 0 Packet is not spliced

> + * @retval >0 Packet has N splices based on it

> + */

> +int odp_packet_is_spliced(odp_packet_t pkt);

> +

>  /*

>   *

>   * Copy

> --

> 2.7.4

>
Bill Fischofer Oct. 21, 2016, 2:11 p.m. UTC | #2
On Fri, Oct 21, 2016 at 8:21 AM, Bala Manoharan <bala.manoharan@linaro.org>
wrote:

> Regards,

> Bala

>

>

> On 11 October 2016 at 09:15, Bill Fischofer <bill.fischofer@linaro.org>

> wrote:

> > Introduce four new APIs that support efficient sharing of portions of

> > packets.

> >

> > odp_packet_splice() creates a reference to a base packet by splicing a

> > supplied header packet onto it at a specified offset. If multiple splices

> > are created then each shares the base packet that was spliced.

> >

> > odp_packet_ref() creates a reference to a base packet by splicing a

> > zero-length header onto it at a specified offset. This allows an

> > application to "hold onto" a pointer to a packet so that it will not be

> > freed until all references to it are freed.

> >

> > odp_packet_is_a_splice() allows an application to determine whether a

> > packet is a splice and if so how many individual packets are contained

> > within it.

> >

> > odp_packet_is_spliced() allows an application to determine whether a

> packet

> > has a splice on it and if so how many splices are based on it.

> >

> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> > ---

> >  include/odp/api/spec/packet.h | 103 ++++++++++++++++++++++++++++++

> ++++++++++++

> >  1 file changed, 103 insertions(+)

> >

> > diff --git a/include/odp/api/spec/packet.h

> b/include/odp/api/spec/packet.h

> > index 4a14f2d..8e147a3 100644

> > --- a/include/odp/api/spec/packet.h

> > +++ b/include/odp/api/spec/packet.h

> > @@ -844,6 +844,109 @@ int odp_packet_concat(odp_packet_t *dst,

> odp_packet_t src);

> >   */

> >  int odp_packet_split(odp_packet_t *pkt, uint32_t len, odp_packet_t

> *tail);

> >

> > +/**

> > + * Splice a packet

> > + *

> > + * Create a new packet by splicing a header onto an existing packet. The

> > + * spliced packet consists of the header followed by a shared reference

> to the

> > + * base packet receiving the splice starting at a designated offset. If

> > + * multiple splices are created on the same base packet, it is the

> > + * application's responsibility to coordinate any changes to the shared

> > + * segment(s) of the splice, otherwise results are undefined.

> > + *

> > + * @param hdr    Handle of the header packet to be spliced onto the

> base packet

> > + *

> > + * @param pkt    Handle of the base packet that is to receive the splice

> > + *

> > + * @param offset Byte offset within the base packet at which the splice

> is

> > + *               to be made.

> > + *

> > + * @return       Handle of the spliced packet. This may or may not be

> the

> > + *               same as the input hdr packet. The caller should use

> this

> > + *               for any future references to the splice. The original

> hdr

> > + *               packet no longer has any independent existence.

> > + *

> > + * @retval ODP_PACKET_INVALID If the splice failed. In this case both

> hdr

> > + *                            and pkt are unchanged.

> > + *

> > + * @note The base pkt remains valid after the completion of the splice,

> > + *       however changes that affect its length may yield unpredictable

> > + *       results when viewed through the returned splice handle. For

> best

> > + *       portability and predictable behavior, applications should

> regard the

> > + *       base packet as read only following a successful splice, with

> the

> > + *       exception of tailroom manipulation (which still requires

> application

> > + *       coordination if multiple splices exist on the same base packet

> whose

> > + *       tail is being pushed or pulled).

> > + *

> > + * @note Either packet input to this routine may itself be a splice,

> however

> > + *       individual implementations may impose limits on how deeply

> splices

> > + *       may be nested and fail the attempted splice if those limits are

> > + *       exceeded.

> > + *

> > + * @note The packets used to create a splice may reside in different

> pools,

> > + *       however individual implementations may require that both

> reside in

> > + *       the same pool and fail the attempted splice if this

> restriction is

> > + *       not observed. Upon return odp_packet_pool() returns the pool

> id of

> > + *       the header packet.

> > + *

> > + * @note Once successfully spliced, the base packet may be freed via

> > + *       odp_packet_free(), however the storage used to represent it

> will not

> > + *       be released until all splices based on it have themselves been

> freed.

>

> Do you have a reason why the 'base' packet will be freed by the

> application after creating the slice?

>


The point being made here is only that the storage behind the base packet
will not be released until all references to it are freed. The application
is under no obligation to issue an odp_packet_free() at any particular
time. One reason why a packet (or one of its references) may be freed is if
it is transmitted since ODP TX processing implicitly calls
odp_packet_free() once the odp_packet_t has been sent. So, for example, if
a reference to a packet is transmitted then the base packet will still be
around since only the reference will be freed by the transmit. Or
conversely if a packet is transmitted but the caller still holds a
reference to it then again while the base packet will be freed by the
transmit the portion of it contained in the reference would still be
retained until the reference was itself freed.


>

> > + */

> > +odp_packet_t odp_packet_splice(odp_packet_t hdr,

> > +                              odp_packet_t pkt, uint32_t offset);

>

> Do we have an use-case for this scenario?

> I would prefer that the API take the entire second packet instead of

> an 'offset' at-least in the first version and that the 'hdr' handle

> becomes invalid once the packet has been sliced and the new handle

> returned replaces the 'hdr' handle.

>


The main use case here would be to splice on a preallocated header onto a
portion of a base packet representing the payload.  The alternative would
be to create a reference (e.g., splice a zero-length header onto a packet)
and then call odp_packet_push_head() to build a header onto the reference.
But being able to have these headers preallocated could be more efficient.

As currently defined, the returned handle replaces the hdr packet handle
since the hdr no longer has an independent identity after the splice. The
base pkt, by contrast, is still valid since other references/splices can be
created on it to result in sharing. So the second part of your request is
already handled. I tried to make that clear in the doxygen for @return.

Restricting the valid offset values for a splice is something that may be
needed for some implementations, and that's why I wanted to have this
discussion. I'd expect such limits to be communicated in an
odp_packet_capability() API that we'd add.

Of course, if a given implementation can only support splicing at offset
zero then a non-zero offset splice can be affected by first calling
odp_packet_pull_head() on the base packet before doing the splice. I assume
that you can support that combination of operations?


>

> > +

> > +/**

> > + * Create a reference to a packet

> > + *

> > + * Create a (shared) reference to a base packet starting at a specified

> > + * byte offset. A reference is simply a splice with a zero-length

> header.

> > + *

> > + * @param pkt    Handle of the base packet for which a reference is to

> be

> > + *               created.

> > + * @param offset Byte offset in the base packet at which shared

> reference is

> > + *               to begin.

> > + *

> > + * @return                    Handle of the reference packet

> > + * @retval ODP_PACKET_INVALID Operation failed. Base packet remains

> unchanged.

> > + *

> > + * @note The zero-length header used to create a packet reference is

> always

> > + *       drawn from the same pool as the base packet.

> > + *

> > + * @note Because references are a type of splice, any bytes pushed onto

> the

> > + *       head of a reference (via odp_packet_push_head() or

> > + *       odp_packet_extend_head() are unique to this reference and are

> not

> > + *       seen by any other reference to the same base packet.

> > + */

> > +odp_packet_t odp_packet_ref(odp_packet_t pkt, uint32_t offset);

>

> In the first version we can drop the offset and create a reference of

> the entire packet.

>


Since a reference is just a splice of a zero-length header any restriction
on supported splice offsets would propagate to references as well. Can this
be done via a capability() API? The advantage of keeping the offset (even
if 0 is the only valid value in the first release) is to preserve API/ABI
compatibility if this restriction is relaxed in a later release.


>

> > +

> > +/**

> > + * Tests if a packet is a splice

> > + *

> > + * A packet is a splice if it was created by odp_packet_splice() or

> > + * odp_packet_ref(). Note that a splice may be a simple splice or a

> > + * compound splice (a splice created from another splice).

> > + *

> > + * @param pkt Packet Handle

> > + *

> > + * @retval 0 Packet is not a splice

> > + * @retval >0 Packet is a splice containing N individual packets

> > + */

> > +int odp_packet_is_a_splice(odp_packet_t pkt);

> > +

> > +/**

> > + * Tests if a packet is spliced

> > + *

> > + * A packet is spliced if a splice was created on it by

> odp_packet_splice()

> > + * or odp_packet_ref().

> > + *

> > + * @param pkt Packet Handle

> > + *

> > + * @retval 0 Packet is not spliced

> > + * @retval >0 Packet has N splices based on it

> > + */

> > +int odp_packet_is_spliced(odp_packet_t pkt);

> > +

> >  /*

> >   *

> >   * Copy

> > --

> > 2.7.4

> >

>
Balasubramanian Manoharan Oct. 21, 2016, 5:07 p.m. UTC | #3
Regards,
Bala


On 21 October 2016 at 19:41, Bill Fischofer <bill.fischofer@linaro.org> wrote:
>

>

> On Fri, Oct 21, 2016 at 8:21 AM, Bala Manoharan <bala.manoharan@linaro.org>

> wrote:

>>

>> Regards,

>> Bala

>>

>>

>> On 11 October 2016 at 09:15, Bill Fischofer <bill.fischofer@linaro.org>

>> wrote:

>> > Introduce four new APIs that support efficient sharing of portions of

>> > packets.

>> >

>> > odp_packet_splice() creates a reference to a base packet by splicing a

>> > supplied header packet onto it at a specified offset. If multiple

>> > splices

>> > are created then each shares the base packet that was spliced.

>> >

>> > odp_packet_ref() creates a reference to a base packet by splicing a

>> > zero-length header onto it at a specified offset. This allows an

>> > application to "hold onto" a pointer to a packet so that it will not be

>> > freed until all references to it are freed.

>> >

>> > odp_packet_is_a_splice() allows an application to determine whether a

>> > packet is a splice and if so how many individual packets are contained

>> > within it.

>> >

>> > odp_packet_is_spliced() allows an application to determine whether a

>> > packet

>> > has a splice on it and if so how many splices are based on it.

>> >

>> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>> > ---

>> >  include/odp/api/spec/packet.h | 103

>> > ++++++++++++++++++++++++++++++++++++++++++

>> >  1 file changed, 103 insertions(+)

>> >

>> > diff --git a/include/odp/api/spec/packet.h

>> > b/include/odp/api/spec/packet.h

>> > index 4a14f2d..8e147a3 100644

>> > --- a/include/odp/api/spec/packet.h

>> > +++ b/include/odp/api/spec/packet.h

>> > @@ -844,6 +844,109 @@ int odp_packet_concat(odp_packet_t *dst,

>> > odp_packet_t src);

>> >   */

>> >  int odp_packet_split(odp_packet_t *pkt, uint32_t len, odp_packet_t

>> > *tail);

>> >

>> > +/**

>> > + * Splice a packet

>> > + *

>> > + * Create a new packet by splicing a header onto an existing packet.

>> > The

>> > + * spliced packet consists of the header followed by a shared reference

>> > to the

>> > + * base packet receiving the splice starting at a designated offset. If

>> > + * multiple splices are created on the same base packet, it is the

>> > + * application's responsibility to coordinate any changes to the shared

>> > + * segment(s) of the splice, otherwise results are undefined.

>> > + *

>> > + * @param hdr    Handle of the header packet to be spliced onto the

>> > base packet

>> > + *

>> > + * @param pkt    Handle of the base packet that is to receive the

>> > splice

>> > + *

>> > + * @param offset Byte offset within the base packet at which the splice

>> > is

>> > + *               to be made.

>> > + *

>> > + * @return       Handle of the spliced packet. This may or may not be

>> > the

>> > + *               same as the input hdr packet. The caller should use

>> > this

>> > + *               for any future references to the splice. The original

>> > hdr

>> > + *               packet no longer has any independent existence.

>> > + *

>> > + * @retval ODP_PACKET_INVALID If the splice failed. In this case both

>> > hdr

>> > + *                            and pkt are unchanged.

>> > + *

>> > + * @note The base pkt remains valid after the completion of the splice,

>> > + *       however changes that affect its length may yield unpredictable

>> > + *       results when viewed through the returned splice handle. For

>> > best

>> > + *       portability and predictable behavior, applications should

>> > regard the

>> > + *       base packet as read only following a successful splice, with

>> > the

>> > + *       exception of tailroom manipulation (which still requires

>> > application

>> > + *       coordination if multiple splices exist on the same base packet

>> > whose

>> > + *       tail is being pushed or pulled).

>> > + *

>> > + * @note Either packet input to this routine may itself be a splice,

>> > however

>> > + *       individual implementations may impose limits on how deeply

>> > splices

>> > + *       may be nested and fail the attempted splice if those limits

>> > are

>> > + *       exceeded.

>> > + *

>> > + * @note The packets used to create a splice may reside in different

>> > pools,

>> > + *       however individual implementations may require that both

>> > reside in

>> > + *       the same pool and fail the attempted splice if this

>> > restriction is

>> > + *       not observed. Upon return odp_packet_pool() returns the pool

>> > id of

>> > + *       the header packet.

>> > + *

>> > + * @note Once successfully spliced, the base packet may be freed via

>> > + *       odp_packet_free(), however the storage used to represent it

>> > will not

>> > + *       be released until all splices based on it have themselves been

>> > freed.

>>

>> Do you have a reason why the 'base' packet will be freed by the

>> application after creating the slice?

>

>

> The point being made here is only that the storage behind the base packet

> will not be released until all references to it are freed. The application

> is under no obligation to issue an odp_packet_free() at any particular time.

> One reason why a packet (or one of its references) may be freed is if it is

> transmitted since ODP TX processing implicitly calls odp_packet_free() once

> the odp_packet_t has been sent. So, for example, if a reference to a packet

> is transmitted then the base packet will still be around since only the

> reference will be freed by the transmit. Or conversely if a packet is

> transmitted but the caller still holds a reference to it then again while

> the base packet will be freed by the transmit the portion of it contained in

> the reference would still be retained until the reference was itself freed.


I believe this is a general concept with reference, whenever you do
not want a packet to be freed during transmission you create a
reference for the packet and then transmit the packet so that the
implementation only decrements the reference count but does not free
the packet.
So I would prefer changing the sentence to reflect the general concept
of reference book keeping rather than explaining about storage of the
base packet, since the concept is same for 'base' packet or a whole
packet and the way it gets implemented is implementation specific.

>

>>

>>

>> > + */

>> > +odp_packet_t odp_packet_splice(odp_packet_t hdr,

>> > +                              odp_packet_t pkt, uint32_t offset);

>>

>> Do we have an use-case for this scenario?

>> I would prefer that the API take the entire second packet instead of

>> an 'offset' at-least in the first version and that the 'hdr' handle

>> becomes invalid once the packet has been sliced and the new handle

>> returned replaces the 'hdr' handle.

>

>

> The main use case here would be to splice on a preallocated header onto a

> portion of a base packet representing the payload.  The alternative would be

> to create a reference (e.g., splice a zero-length header onto a packet) and

> then call odp_packet_push_head() to build a header onto the reference. But

> being able to have these headers preallocated could be more efficient.

>

> As currently defined, the returned handle replaces the hdr packet handle

> since the hdr no longer has an independent identity after the splice. The

> base pkt, by contrast, is still valid since other references/splices can be

> created on it to result in sharing. So the second part of your request is

> already handled. I tried to make that clear in the doxygen for @return.


This could be achieved by first creating reference for the hdr packet
and then using the handle returned by the reference API and create a
splice.

>

> Restricting the valid offset values for a splice is something that may be

> needed for some implementations, and that's why I wanted to have this

> discussion. I'd expect such limits to be communicated in an

> odp_packet_capability() API that we'd add.


I believe offset is something which could be implemented by all HWs
using software interaction but since these APIs are executed in the
fast-path having a separate API for creating reference at 0 offset
could be beneficial as the implementation could be more efficient in
that case.

>

> Of course, if a given implementation can only support splicing at offset

> zero then a non-zero offset splice can be affected by first calling

> odp_packet_pull_head() on the base packet before doing the splice. I assume

> that you can support that combination of operations?

>

>>

>>

>> > +

>> > +/**

>> > + * Create a reference to a packet

>> > + *

>> > + * Create a (shared) reference to a base packet starting at a specified

>> > + * byte offset. A reference is simply a splice with a zero-length

>> > header.

>> > + *

>> > + * @param pkt    Handle of the base packet for which a reference is to

>> > be

>> > + *               created.

>> > + * @param offset Byte offset in the base packet at which shared

>> > reference is

>> > + *               to begin.

>> > + *

>> > + * @return                    Handle of the reference packet

>> > + * @retval ODP_PACKET_INVALID Operation failed. Base packet remains

>> > unchanged.

>> > + *

>> > + * @note The zero-length header used to create a packet reference is

>> > always

>> > + *       drawn from the same pool as the base packet.

>> > + *

>> > + * @note Because references are a type of splice, any bytes pushed onto

>> > the

>> > + *       head of a reference (via odp_packet_push_head() or

>> > + *       odp_packet_extend_head() are unique to this reference and are

>> > not

>> > + *       seen by any other reference to the same base packet.

>> > + */

>> > +odp_packet_t odp_packet_ref(odp_packet_t pkt, uint32_t offset);

>>

>> In the first version we can drop the offset and create a reference of

>> the entire packet.

>

>

> Since a reference is just a splice of a zero-length header any restriction

> on supported splice offsets would propagate to references as well. Can this

> be done via a capability() API? The advantage of keeping the offset (even if

> 0 is the only valid value in the first release) is to preserve API/ABI

> compatibility if this restriction is relaxed in a later release.


I believe the offset 0 will be the general use-case hence it is better
to keep a separate API for creating reference of entire packet.
Creating multiple references at multiple offset of a packet
complicates the implementation logic and since these codes execute in
the fast path the logic should be as efficient as possible.

I would prefer adding the simple use-case in this version since I
believe it could be easily agreed upon and implemented by all HWs.
>

>>

>>

>> > +

>> > +/**

>> > + * Tests if a packet is a splice

>> > + *

>> > + * A packet is a splice if it was created by odp_packet_splice() or

>> > + * odp_packet_ref(). Note that a splice may be a simple splice or a

>> > + * compound splice (a splice created from another splice).

>> > + *

>> > + * @param pkt Packet Handle

>> > + *

>> > + * @retval 0 Packet is not a splice

>> > + * @retval >0 Packet is a splice containing N individual packets

>> > + */

>> > +int odp_packet_is_a_splice(odp_packet_t pkt);

>> > +

>> > +/**

>> > + * Tests if a packet is spliced

>> > + *

>> > + * A packet is spliced if a splice was created on it by

>> > odp_packet_splice()

>> > + * or odp_packet_ref().

>> > + *

>> > + * @param pkt Packet Handle

>> > + *

>> > + * @retval 0 Packet is not spliced

>> > + * @retval >0 Packet has N splices based on it

>> > + */

>> > +int odp_packet_is_spliced(odp_packet_t pkt);

>> > +

>> >  /*

>> >   *

>> >   * Copy

>> > --

>> > 2.7.4

>> >

>

>
Bill Fischofer Oct. 21, 2016, 5:25 p.m. UTC | #4
On Fri, Oct 21, 2016 at 12:07 PM, Bala Manoharan <bala.manoharan@linaro.org>
wrote:

> Regards,

> Bala

>

>

> On 21 October 2016 at 19:41, Bill Fischofer <bill.fischofer@linaro.org>

> wrote:

> >

> >

> > On Fri, Oct 21, 2016 at 8:21 AM, Bala Manoharan <

> bala.manoharan@linaro.org>

> > wrote:

> >>

> >> Regards,

> >> Bala

> >>

> >>

> >> On 11 October 2016 at 09:15, Bill Fischofer <bill.fischofer@linaro.org>

> >> wrote:

> >> > Introduce four new APIs that support efficient sharing of portions of

> >> > packets.

> >> >

> >> > odp_packet_splice() creates a reference to a base packet by splicing a

> >> > supplied header packet onto it at a specified offset. If multiple

> >> > splices

> >> > are created then each shares the base packet that was spliced.

> >> >

> >> > odp_packet_ref() creates a reference to a base packet by splicing a

> >> > zero-length header onto it at a specified offset. This allows an

> >> > application to "hold onto" a pointer to a packet so that it will not

> be

> >> > freed until all references to it are freed.

> >> >

> >> > odp_packet_is_a_splice() allows an application to determine whether a

> >> > packet is a splice and if so how many individual packets are contained

> >> > within it.

> >> >

> >> > odp_packet_is_spliced() allows an application to determine whether a

> >> > packet

> >> > has a splice on it and if so how many splices are based on it.

> >> >

> >> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> >> > ---

> >> >  include/odp/api/spec/packet.h | 103

> >> > ++++++++++++++++++++++++++++++++++++++++++

> >> >  1 file changed, 103 insertions(+)

> >> >

> >> > diff --git a/include/odp/api/spec/packet.h

> >> > b/include/odp/api/spec/packet.h

> >> > index 4a14f2d..8e147a3 100644

> >> > --- a/include/odp/api/spec/packet.h

> >> > +++ b/include/odp/api/spec/packet.h

> >> > @@ -844,6 +844,109 @@ int odp_packet_concat(odp_packet_t *dst,

> >> > odp_packet_t src);

> >> >   */

> >> >  int odp_packet_split(odp_packet_t *pkt, uint32_t len, odp_packet_t

> >> > *tail);

> >> >

> >> > +/**

> >> > + * Splice a packet

> >> > + *

> >> > + * Create a new packet by splicing a header onto an existing packet.

> >> > The

> >> > + * spliced packet consists of the header followed by a shared

> reference

> >> > to the

> >> > + * base packet receiving the splice starting at a designated offset.

> If

> >> > + * multiple splices are created on the same base packet, it is the

> >> > + * application's responsibility to coordinate any changes to the

> shared

> >> > + * segment(s) of the splice, otherwise results are undefined.

> >> > + *

> >> > + * @param hdr    Handle of the header packet to be spliced onto the

> >> > base packet

> >> > + *

> >> > + * @param pkt    Handle of the base packet that is to receive the

> >> > splice

> >> > + *

> >> > + * @param offset Byte offset within the base packet at which the

> splice

> >> > is

> >> > + *               to be made.

> >> > + *

> >> > + * @return       Handle of the spliced packet. This may or may not be

> >> > the

> >> > + *               same as the input hdr packet. The caller should use

> >> > this

> >> > + *               for any future references to the splice. The

> original

> >> > hdr

> >> > + *               packet no longer has any independent existence.

> >> > + *

> >> > + * @retval ODP_PACKET_INVALID If the splice failed. In this case both

> >> > hdr

> >> > + *                            and pkt are unchanged.

> >> > + *

> >> > + * @note The base pkt remains valid after the completion of the

> splice,

> >> > + *       however changes that affect its length may yield

> unpredictable

> >> > + *       results when viewed through the returned splice handle. For

> >> > best

> >> > + *       portability and predictable behavior, applications should

> >> > regard the

> >> > + *       base packet as read only following a successful splice, with

> >> > the

> >> > + *       exception of tailroom manipulation (which still requires

> >> > application

> >> > + *       coordination if multiple splices exist on the same base

> packet

> >> > whose

> >> > + *       tail is being pushed or pulled).

> >> > + *

> >> > + * @note Either packet input to this routine may itself be a splice,

> >> > however

> >> > + *       individual implementations may impose limits on how deeply

> >> > splices

> >> > + *       may be nested and fail the attempted splice if those limits

> >> > are

> >> > + *       exceeded.

> >> > + *

> >> > + * @note The packets used to create a splice may reside in different

> >> > pools,

> >> > + *       however individual implementations may require that both

> >> > reside in

> >> > + *       the same pool and fail the attempted splice if this

> >> > restriction is

> >> > + *       not observed. Upon return odp_packet_pool() returns the pool

> >> > id of

> >> > + *       the header packet.

> >> > + *

> >> > + * @note Once successfully spliced, the base packet may be freed via

> >> > + *       odp_packet_free(), however the storage used to represent it

> >> > will not

> >> > + *       be released until all splices based on it have themselves

> been

> >> > freed.

> >>

> >> Do you have a reason why the 'base' packet will be freed by the

> >> application after creating the slice?

> >

> >

> > The point being made here is only that the storage behind the base packet

> > will not be released until all references to it are freed. The

> application

> > is under no obligation to issue an odp_packet_free() at any particular

> time.

> > One reason why a packet (or one of its references) may be freed is if it

> is

> > transmitted since ODP TX processing implicitly calls odp_packet_free()

> once

> > the odp_packet_t has been sent. So, for example, if a reference to a

> packet

> > is transmitted then the base packet will still be around since only the

> > reference will be freed by the transmit. Or conversely if a packet is

> > transmitted but the caller still holds a reference to it then again while

> > the base packet will be freed by the transmit the portion of it

> contained in

> > the reference would still be retained until the reference was itself

> freed.

>

> I believe this is a general concept with reference, whenever you do

> not want a packet to be freed during transmission you create a

> reference for the packet and then transmit the packet so that the

> implementation only decrements the reference count but does not free

> the packet.

> So I would prefer changing the sentence to reflect the general concept

> of reference book keeping rather than explaining about storage of the

> base packet, since the concept is same for 'base' packet or a whole

> packet and the way it gets implemented is implementation specific.

>


I have no problem rewording this as the concept remains the same. We don't
mention reference counts explicitly, though that's the obvious way for
implementations to do the necessary bookkeeping.


>

> >

> >>

> >>

> >> > + */

> >> > +odp_packet_t odp_packet_splice(odp_packet_t hdr,

> >> > +                              odp_packet_t pkt, uint32_t offset);

> >>

> >> Do we have an use-case for this scenario?

> >> I would prefer that the API take the entire second packet instead of

> >> an 'offset' at-least in the first version and that the 'hdr' handle

> >> becomes invalid once the packet has been sliced and the new handle

> >> returned replaces the 'hdr' handle.

> >

> >

> > The main use case here would be to splice on a preallocated header onto a

> > portion of a base packet representing the payload.  The alternative

> would be

> > to create a reference (e.g., splice a zero-length header onto a packet)

> and

> > then call odp_packet_push_head() to build a header onto the reference.

> But

> > being able to have these headers preallocated could be more efficient.

> >

> > As currently defined, the returned handle replaces the hdr packet handle

> > since the hdr no longer has an independent identity after the splice. The

> > base pkt, by contrast, is still valid since other references/splices can

> be

> > created on it to result in sharing. So the second part of your request is

> > already handled. I tried to make that clear in the doxygen for @return.

>

> This could be achieved by first creating reference for the hdr packet

> and then using the handle returned by the reference API and create a

> splice.

>


That's effectively what happens as a reference is just a splice of a
zero-length header packet. Creating a reference to the header would mean
that the header would then have to be regarded as read-only, however.
Splicing that onto a base would then create a compound reference (you're
splicing a reference/splice onto another packet). odp-linux can do that but
the question is whether every implementation can do arbitrary compound
splices? That may need to be another capability indication.


>

> >

> > Restricting the valid offset values for a splice is something that may be

> > needed for some implementations, and that's why I wanted to have this

> > discussion. I'd expect such limits to be communicated in an

> > odp_packet_capability() API that we'd add.

>

> I believe offset is something which could be implemented by all HWs

> using software interaction but since these APIs are executed in the

> fast-path having a separate API for creating reference at 0 offset

> could be beneficial as the implementation could be more efficient in

> that case.

>


Petri originally proposed one API and I proposed to split that into two so
that references didn't have to specify an explicit null header. Having a
third that doesn't use the offset parameter is a possibility, but
specifying an offset of 0 seems simple to optimize. But I'm certainly open
to having that as a separate API if that makes life easier.


>

> >

> > Of course, if a given implementation can only support splicing at offset

> > zero then a non-zero offset splice can be affected by first calling

> > odp_packet_pull_head() on the base packet before doing the splice. I

> assume

> > that you can support that combination of operations?

> >

> >>

> >>

> >> > +

> >> > +/**

> >> > + * Create a reference to a packet

> >> > + *

> >> > + * Create a (shared) reference to a base packet starting at a

> specified

> >> > + * byte offset. A reference is simply a splice with a zero-length

> >> > header.

> >> > + *

> >> > + * @param pkt    Handle of the base packet for which a reference is

> to

> >> > be

> >> > + *               created.

> >> > + * @param offset Byte offset in the base packet at which shared

> >> > reference is

> >> > + *               to begin.

> >> > + *

> >> > + * @return                    Handle of the reference packet

> >> > + * @retval ODP_PACKET_INVALID Operation failed. Base packet remains

> >> > unchanged.

> >> > + *

> >> > + * @note The zero-length header used to create a packet reference is

> >> > always

> >> > + *       drawn from the same pool as the base packet.

> >> > + *

> >> > + * @note Because references are a type of splice, any bytes pushed

> onto

> >> > the

> >> > + *       head of a reference (via odp_packet_push_head() or

> >> > + *       odp_packet_extend_head() are unique to this reference and

> are

> >> > not

> >> > + *       seen by any other reference to the same base packet.

> >> > + */

> >> > +odp_packet_t odp_packet_ref(odp_packet_t pkt, uint32_t offset);

> >>

> >> In the first version we can drop the offset and create a reference of

> >> the entire packet.

> >

> >

> > Since a reference is just a splice of a zero-length header any

> restriction

> > on supported splice offsets would propagate to references as well. Can

> this

> > be done via a capability() API? The advantage of keeping the offset

> (even if

> > 0 is the only valid value in the first release) is to preserve API/ABI

> > compatibility if this restriction is relaxed in a later release.

>

> I believe the offset 0 will be the general use-case hence it is better

> to keep a separate API for creating reference of entire packet.

> Creating multiple references at multiple offset of a packet

> complicates the implementation logic and since these codes execute in

> the fast path the logic should be as efficient as possible.

>

> I would prefer adding the simple use-case in this version since I

> believe it could be easily agreed upon and implemented by all HWs.

>


I'd like to get confirmation of that from other HW platforms. Good to have
your input on this.


> >

> >>

> >>

> >> > +

> >> > +/**

> >> > + * Tests if a packet is a splice

> >> > + *

> >> > + * A packet is a splice if it was created by odp_packet_splice() or

> >> > + * odp_packet_ref(). Note that a splice may be a simple splice or a

> >> > + * compound splice (a splice created from another splice).

> >> > + *

> >> > + * @param pkt Packet Handle

> >> > + *

> >> > + * @retval 0 Packet is not a splice

> >> > + * @retval >0 Packet is a splice containing N individual packets

> >> > + */

> >> > +int odp_packet_is_a_splice(odp_packet_t pkt);

> >> > +

> >> > +/**

> >> > + * Tests if a packet is spliced

> >> > + *

> >> > + * A packet is spliced if a splice was created on it by

> >> > odp_packet_splice()

> >> > + * or odp_packet_ref().

> >> > + *

> >> > + * @param pkt Packet Handle

> >> > + *

> >> > + * @retval 0 Packet is not spliced

> >> > + * @retval >0 Packet has N splices based on it

> >> > + */

> >> > +int odp_packet_is_spliced(odp_packet_t pkt);

> >> > +

> >> >  /*

> >> >   *

> >> >   * Copy

> >> > --

> >> > 2.7.4

> >> >

> >

> >

>
diff mbox

Patch

diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
index 4a14f2d..8e147a3 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -844,6 +844,109 @@  int odp_packet_concat(odp_packet_t *dst, odp_packet_t src);
  */
 int odp_packet_split(odp_packet_t *pkt, uint32_t len, odp_packet_t *tail);
 
+/**
+ * Splice a packet
+ *
+ * Create a new packet by splicing a header onto an existing packet. The
+ * spliced packet consists of the header followed by a shared reference to the
+ * base packet receiving the splice starting at a designated offset. If
+ * multiple splices are created on the same base packet, it is the
+ * application's responsibility to coordinate any changes to the shared
+ * segment(s) of the splice, otherwise results are undefined.
+ *
+ * @param hdr    Handle of the header packet to be spliced onto the base packet
+ *
+ * @param pkt    Handle of the base packet that is to receive the splice
+ *
+ * @param offset Byte offset within the base packet at which the splice is
+ *               to be made.
+ *
+ * @return       Handle of the spliced packet. This may or may not be the
+ *               same as the input hdr packet. The caller should use this
+ *               for any future references to the splice. The original hdr
+ *               packet no longer has any independent existence.
+ *
+ * @retval ODP_PACKET_INVALID If the splice failed. In this case both hdr
+ *                            and pkt are unchanged.
+ *
+ * @note The base pkt remains valid after the completion of the splice,
+ *       however changes that affect its length may yield unpredictable
+ *       results when viewed through the returned splice handle. For best
+ *       portability and predictable behavior, applications should regard the
+ *       base packet as read only following a successful splice, with the
+ *       exception of tailroom manipulation (which still requires application
+ *       coordination if multiple splices exist on the same base packet whose
+ *       tail is being pushed or pulled).
+ *
+ * @note Either packet input to this routine may itself be a splice, however
+ *       individual implementations may impose limits on how deeply splices
+ *       may be nested and fail the attempted splice if those limits are
+ *       exceeded.
+ *
+ * @note The packets used to create a splice may reside in different pools,
+ *       however individual implementations may require that both reside in
+ *       the same pool and fail the attempted splice if this restriction is
+ *       not observed. Upon return odp_packet_pool() returns the pool id of
+ *       the header packet.
+ *
+ * @note Once successfully spliced, the base packet may be freed via
+ *       odp_packet_free(), however the storage used to represent it will not
+ *       be released until all splices based on it have themselves been freed.
+ */
+odp_packet_t odp_packet_splice(odp_packet_t hdr,
+			       odp_packet_t pkt, uint32_t offset);
+
+/**
+ * Create a reference to a packet
+ *
+ * Create a (shared) reference to a base packet starting at a specified
+ * byte offset. A reference is simply a splice with a zero-length header.
+ *
+ * @param pkt    Handle of the base packet for which a reference is to be
+ *               created.
+ * @param offset Byte offset in the base packet at which shared reference is
+ *               to begin.
+ *
+ * @return                    Handle of the reference packet
+ * @retval ODP_PACKET_INVALID Operation failed. Base packet remains unchanged.
+ *
+ * @note The zero-length header used to create a packet reference is always
+ *       drawn from the same pool as the base packet.
+ *
+ * @note Because references are a type of splice, any bytes pushed onto the
+ *       head of a reference (via odp_packet_push_head() or
+ *       odp_packet_extend_head() are unique to this reference and are not
+ *       seen by any other reference to the same base packet.
+ */
+odp_packet_t odp_packet_ref(odp_packet_t pkt, uint32_t offset);
+
+/**
+ * Tests if a packet is a splice
+ *
+ * A packet is a splice if it was created by odp_packet_splice() or
+ * odp_packet_ref(). Note that a splice may be a simple splice or a
+ * compound splice (a splice created from another splice).
+ *
+ * @param pkt Packet Handle
+ *
+ * @retval 0 Packet is not a splice
+ * @retval >0 Packet is a splice containing N individual packets
+ */
+int odp_packet_is_a_splice(odp_packet_t pkt);
+
+/**
+ * Tests if a packet is spliced
+ *
+ * A packet is spliced if a splice was created on it by odp_packet_splice()
+ * or odp_packet_ref().
+ *
+ * @param pkt Packet Handle
+ *
+ * @retval 0 Packet is not spliced
+ * @retval >0 Packet has N splices based on it
+ */
+int odp_packet_is_spliced(odp_packet_t pkt);
+
 /*
  *
  * Copy