On Mon, Jan 9, 2017 at 10:08 AM, Bala Manoharan
<bala.manoha...@linaro.org> wrote:
> Except for the Minor documentation comment below
>
> Reviewed-by: Balasubramanian Manoharan <bala.manoha...@linaro.org>
>
> On 5 January 2017 at 20:25, Bill Fischofer <bill.fischo...@linaro.org> wrote:
>> Introduce three new APIs that support efficient sharing of portions of
>> packets.
>>
>> odp_packet_ref_static() creates an alias for a base packet
>>
>> odp_packet_ref() creates a reference to a base packet
>>
>> odp_packet_ref_pkt() creates a reference to a base packet from a supplied
>> header packet
>>
>> In addition, two other APIs simplify working with references
>>
>> odp_packet_is_ref() says whether a packet is a reference
>>
>> odp_packet_unshared_len() gives the sum of data lengths over all unshared
>> packet segments. These are the only bytes of the packet that may be
>> modified safely.
>>
>> Signed-off-by: Bill Fischofer <bill.fischo...@linaro.org>
>> ---
>> Changes in v6:
>> - Miscellaneous formatting corrections
>> - Minor documentation improvements
>>
>> Changes in v5:
>> - Clarified that both input packets to odp_packet_ref_pkt() must be from the
>>   same pool and results are undefined if they are not.
>> - Clarified that input(s) to reference create APIs may be references and that
>>   implementations are free to perform partial or full packet copies if needed
>>   to support such compound references.
>> - Removed odp_packet_has_ref() and redefined odp_packet_is_ref() to indicate
>>   simply whether other handles exist that share bytes with this packet.
>> - Modified validation tests to reflect the above changes.
>>
>> Changes in v4:
>> - Bug fixes
>> - Expand validation testing to cover extensions on packets with references
>>
>> Changes in v3:
>> - Rebased to latest API-NEXT
>> - Bug fixes
>> - Eliminate concept of base packets, both references and referenced packets 
>> may
>>   have head extensions as needed
>>  include/odp/api/spec/packet.h | 172 
>> +++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 171 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
>> index 4a86eba..9b6681c 100644
>> --- a/include/odp/api/spec/packet.h
>> +++ b/include/odp/api/spec/packet.h
>> @@ -256,6 +256,20 @@ uint32_t odp_packet_seg_len(odp_packet_t pkt);
>>  uint32_t odp_packet_len(odp_packet_t pkt);
>>
>>  /**
>> + * Packet unshared data len
>> + *
>> + * Returns the sum of data lengths over all unshared packet segments. These
>> + * are the only bytes that should be modified by the caller. The rest of the
>> + * packet should be treated as read only. odp_packet_unshared_len() will be
>> + * equal to odp_packet_len() if odp_packet_is_ref() is 0.
>> + *
>> + * @param pkt Packet handle
>> + *
>> + * @return Packet unshared data length
>> + */
>> +uint32_t odp_packet_unshared_len(odp_packet_t pkt);
>> +
>> +/**
>>   * Packet headroom length
>>   *
>>   * Returns the current packet level headroom length.
>> @@ -795,7 +809,7 @@ uint32_t odp_packet_seg_data_len(odp_packet_t pkt, 
>> odp_packet_seg_t seg);
>>   * data pointers. Otherwise, all old pointers remain valid.
>>   *
>>   * The resulting packet is always allocated from the same pool as
>> - * the destination packet. The source packet may have been allocate from
>> + * the destination packet. The source packet may have been allocated from
>>   * any pool.
>>   *
>>   * On failure, both handles remain valid and packets are not modified.
>> @@ -848,6 +862,162 @@ int odp_packet_split(odp_packet_t *pkt, uint32_t len, 
>> odp_packet_t *tail);
>>
>>  /*
>>   *
>> + * References
>> + * ********************************************************
>> + *
>> + */
>> +
>> +/**
>> + * Create a static reference to a packet
>> + *
>> + * A static reference is used to obtain an additional handle for referring 
>> to
>> + * a packet so that the storage behind it is not freed until all references 
>> to
>> + * the packet are freed. This can be used, for example, to support efficient
>> + * retransmission processing.
>> + *
>> + * The intent of a static reference is that both the packet and the returned
>> + * reference to it will be treated as read only after this call. Results are
>> + * undefined if this restriction is not observed.
>> + *
>> + * Static references have restrictions but may have performance advantages 
>> on
>> + * some platforms if the caller does not intend to modify the reference
>> + * packet. If modification is needed (e.g., to prefix a unique header onto 
>> the
>> + * packet) then odp_packet_ref() or odp_packet_ref_pkt() should be used.
>> + *
>> + * @param pkt    Handle of the packet for which a static reference is
>> + *               to be created.
>> + *
>> + * @return                    Handle of the static reference packet
>> + * @retval ODP_PACKET_INVALID Operation failed. pkt remains unchanged.
>> + */
>> +odp_packet_t odp_packet_ref_static(odp_packet_t pkt);
>> +
>> +/**
>> + * Create a reference to a packet
>> + *
>> + * Create a dynamic reference to a packet starting at a specified byte
>> + * offset. This reference may be used as an argument to header manipulation
>> + * APIs to prefix a unique header onto the shared payload. The storage
>> + * associated with the referenced packet is not freed until all references 
>> to
>> + * it are freed, which permits easy multicasting or retransmission 
>> processing
>> + * to be performed. Following a successful call, the shared portions of the
>> + * referenced packet packet should be treated as read only. Results are
>> + * undefined if this restriction is not observed.
>> + *
>> + * This operation logically prepends a zero-length header onto the 
>> referenced
>> + * packet beginning at the specified offset. This header is always drawn 
>> from
>> + * the same pool as the referenced packet.
>> + *
>> + * Because references are unique packets, 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 the referenced packet or by any 
>> other
>> + * reference to the same packet. Similarly, any bytes pushed onto the
>> + * head of the referenced packet are not visible to any reference created
>> + * prior to the push operation.
>> + *
>> + * Note that the existence of references does not in any way change the
>> + * requirement that a single packet handle may only be manipulated by one
>> + * thread at a time. The odp_packet_unshared_len() API may be used to
>> + * determine the number of bytes starting at offset zero that are unique to
>> + * this packet handle. Only these bytes should be considered modifiable. All
>> + * other bytes accessible from this handle should be treated as read only.
>> + *
>> + * The packet used as input to this routine may itself by a reference to 
>> some
>> + * other packet. Implementations that are unable to support such compound
>> + * references without copies MAY perform a partial or full packet copy
>> + * operation to return a valid reference in response to this call.
>> + *
>> + * If the caller does not indend to push a header onto either the returned
>> + * reference or the packet being referenced, or otherwise modify either
>> + * packet, the odp_packet_ref_static() API may be used. This may be a more
>> + * efficient means of obtaining another reference to a packet if both will 
>> be
>> + * treated as read only.
>> + *
>> + * @param pkt    Handle of the packet for which a reference is to be
>> + *               created.
>> + *
>> + * @param offset Byte offset in pkt at which the shared reference is to
>> + *               begin. This must be in the range 0..odp_packet_len(pkt)-1.
>> + *
>> + * @return                    Handle of the reference packet
>> + * @retval ODP_PACKET_INVALID Operation failed. pkt remains unchanged.
>> + */
>> +odp_packet_t odp_packet_ref(odp_packet_t pkt, uint32_t offset);
>> +
>> +/**
>> + * Create a reference to another packet from a header packet
>> + *
>> + * Create a dynamic reference to a packet starting at a specified byte 
>> offset
>> + * by prepending a supplied header packet. The resulting reference consists 
>> of
>> + * the unshared header followed by the shared referenced packet starting at
>> + * the specified offset. This shared portion should be regarded as read
>> + * only. The storage associated with the shared portion of the reference is
>> + * not freed until all references to it are freed, which permits easy
>> + * multicasting or retransmission processing to be performed.
>> + *
>> + * Because references are unique packets, 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 the referenced packet or by any 
>> other
>> + * reference to the same packet. Similarly, any bytes pushed onto the
>> + * head of the referenced packet are not visible to any reference created
>> + * prior to the push operation.
>> + *
>> + * Note that the existence of references does not in any way change the
>> + * requirement that a single packet handle may only be manipulated by one
>> + * thread at a time. The odp_packet_unshared_len() API may be used to
>> + * determine the number of bytes starting at offset zero that are unique to
>> + * this packet handle. Only these bytes should be considered modifiable. All
>> + * other bytes accessible from this handle should be treated as read only.
>> + *
>> + * Either packet used as input to this routine may itself by a reference to
>> + * some other packet. Implementations that are unable to support such 
>> compound
>> + * references without copies MAY perform a partial or full packet copy
>> + * operation to return a valid reference in response to this call.
>> + *
>> + * The packets used as input to this routine should be drawn from the same
>> + * pool. Results are undefined if this restriction is not observed.
>> + *
>> + * odp_packet_pool() on the returned reference returns the pool id of the
>> + * header packet.
>> + *
>> + * @param pkt    Handle of the packet for which a reference is to be
>> + *               created.
>> + *
>> + * @param offset Byte offset in pkt at which the shared reference is to
>> + *               begin. Must be in the range 0..odp_packet_len(pkt)-1.
>> + *
>> + * @param hdr    Handle of the header packet to be prefixed onto pkt to 
>> create
>> + *               the reference. If this is specified as ODP_PACKET_INVALID,
>> + *               this call is equivalent to odp_packet_ref(). The supplied 
>> hdr
>> + *               must be distinct from pkt and results are undefined if an
>> + *               attempt is made to create circular references.
>
> We can add documentation that if 'hdr' or 'pkt' is ODP_PACKET_INVALID
> then then api will fail.

The intent here is that hdr can be specified as ODP_PACKET_INVALID, in
which case this call is equivalent to odp_packet_ref(). I believe this
was Petri's suggestion, but I have no problem with removing that
feature and requiring that both pkt and hdr be valid packet handles.

>
>> + *
>> + * @return       Handle of the reference packet. This may or may not be the
>> + *               same as the input hdr packet. The caller should only use 
>> this
>> + *               handle since the original hdr packet no longer has any
>> + *               independent existence.
>> + *
>> + * @retval ODP_PACKET_INVALID Operation failed. Both pkt and hdr are 
>> unchanged.
>> + */
>> +odp_packet_t odp_packet_ref_pkt(odp_packet_t pkt, uint32_t offset,
>> +                               odp_packet_t hdr);
>> +
>> +/**
>> + * Test if a packet is a reference
>> + *
>> + * A packet is a reference if any other packet handles exist that share any
>> + * data bytes with this packet.
>> + *
>> + * @param pkt Packet handle
>> + *
>> + * @retval 0  Packet is not a reference. All bytes in this packet are 
>> unshared.
>> + * @retval 1  Packet is a reference. One or more other packet handles exist
>> + *            that share bytes with this packet.
>> + */
>> +int odp_packet_is_ref(odp_packet_t pkt);
>> +
>> +/*
>> + *
>>   * Copy
>>   * ********************************************************
>>   *
>> --
>> 2.7.4
>>

Reply via email to