On 12/19/16 19:41, Bill Fischofer wrote: > On Mon, Dec 19, 2016 at 9:57 AM, Maxim Uvarov <maxim.uva...@linaro.org> wrote: >> how about group all packet reference apis to odp_packet_ref_ naming? >> >> In that case >> >> odp_packet_ref() -> odp_packet_ref_create() (same as odp_pool_create, >> odp_queue, create). > > I think the _create() suffix is unnecessary but have no objection to > that if that's a consensus view. Note that we also have the other > variants to deal with and finding a way to add _create() to them > without it looking awkward might be a challenge. > >> >> both >> odp_packet_is_ref() and odp_packet_has_ref() -> odp_packet_ref() >> (function which returns ref counter) >> >> It will be more easy if application could not see difference between >> reference and original packet. At least if we do not have special use >> case for that. > > The reason applications see a difference (and why there are two calls > rather than one) is that there is a difference, as previously > discussed. I can modify a reference (indeed that's one of the main > uses for it) by pushing a unique header onto it. I cannot modify a > referenced packet as the entire packet must be treated as read-only, > per our convention. If you want to modify both independently the way > you do that is create two references to the same base packet, and then > both of these can have their own unique prefixes while the base packet > remains read-only. >
Maybe I understood previously something wrong. My understanding before what that read-only for packet which has references can be implementation specific. I.e. when you set up reference you defend original memory with mprotect(READ) and set up flag in buffer header that memory is protected. Then if somebody needs to modify that memory first he needs to get access to packet with something like *odp_packet_data(pkt) and here we have 2 options: 1) if uses asks for data then alloc new packet and gave him writeable pointer 2) add call *odp_packet_data_ro(pkt) which will keep packet read only and *odp_packet_data(pkt) will assume writes. Other functions with packets work with handles and can take care about rw internally. That looks more easy to use from application side. Maxim. >> >> Maxim. >> >> >> On 11/15/16 17:44, Bill Fischofer 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, three other APIs simplify working with references >>> >>> odp_packet_is_ref() says whether a packet is a reference >>> >>> odp_packet_has_ref() says whether a packet has had a reference made to it >>> >>> odp_packet_unshared_len() gives the length of the prefix bytes that are >>> unique to this reference. These are the only bytes of the packet that may >>> be modified safely. >>> >>> Signed-off-by: Bill Fischofer <bill.fischo...@linaro.org> >>> --- >>> include/odp/api/spec/packet.h | 168 >>> ++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 168 insertions(+) >>> >>> diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h >>> index faf62e2..137024f 100644 >>> --- a/include/odp/api/spec/packet.h >>> +++ b/include/odp/api/spec/packet.h >>> @@ -256,6 +256,23 @@ 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. >>> + * >>> + * This routine will return 0 if odp_packet_has_ref() != 0 and will be the >>> + * same as odp_packet_len() if odp_packet_has_ref() == 0 and >>> + * odp_packet_is_ref() == 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. >>> @@ -847,6 +864,157 @@ 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 base packet and the >>> + * returned reference will be treated as read-only after this call. Results >>> + * are unpredictable 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 base packet for which a static reference is >>> + * to be created. >>> + * >>> + * @return Handle of the static reference packet >>> + * @retval ODP_PACKET_INVALID Operation failed. Base packet 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 base 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 base. The storage >>> associated >>> + * with the base 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 base packet should be >>> treated >>> + * as read-only. Results are unpredictable if this restriction is not >>> + * observed. >>> + * >>> + * This operation prepends a zero-length header onto the base packet >>> beginning >>> + * at the specified offset. This header is always drawn from the same pool >>> as >>> + * the base 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 base packet or by any other >>> + * reference to the same base packet. >>> + * >>> + * The base packet used as input to this routine may itself by a reference >>> to >>> + * some other base packet. Implementations MAY restrict the ability to >>> create >>> + * such compound references. Attempts to exceed any implementation limits >>> in >>> + * this regard will result in this call failing and returning >>> + * ODP_PACKET_INVALID. >>> + * >>> + * If the caller does not indend to push a header onto the returned >>> reference, >>> + * the odp_packet_ref_static() API may be used. This may be a more >>> efficient >>> + * means of obtaining another reference to a base packet that will be >>> treated >>> + * as read-only. >>> + * >>> + * @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 the shared >>> reference >>> + * is to begin. Must be in the range >>> 0..odp_packet_len(pkt)-1. >>> + * >>> + * @return Handle of the reference packet >>> + * @retval ODP_PACKET_INVALID Operation failed. Base packet 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 base 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 base 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. >>> + * >>> + * Either packet input to this routine may itself be a reference, however >>> + * individual implementations MAY impose limits on how deeply splices may >>> be >>> + * nested and fail the call if those limits are exceeded. >>> + * >>> + * The packets used as input to this routine may reside in different pools, >>> + * however individual implementations MAY require that both reside in the >>> same >>> + * pool and fail the call 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 base packet for which a reference is to be >>> + * created. >>> + * >>> + * @param offset Byte offset in the base packet at which the shared >>> reference >>> + * to begin. Must be in the range 0..odp_packet_len(pkt)-1. >>> + * >>> + * @param hdr Handle of the header packet to be prefixed onto the base >>> + * packet 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 >>> + * the base pkt and results are undefined if an attempt is >>> made >>> + * to create circular references. >>> + * >>> + * @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 If the reference failed. In this case 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 it was created by odp_packet_ref() or >>> + * odp_packet_ref_pkt(). Note that a reference created from another >>> + * reference is considered a compound reference. Calls on such packets will >>> + * result in return values greater than 1. >>> + * >>> + * @param pkt Packet handle >>> + * >>> + * @retval 0 Packet is not a reference >>> + * @retval >0 Packet is a reference consisting of N individual packets. >>> + */ >>> +int odp_packet_is_ref(odp_packet_t pkt); >>> + >>> +/** >>> + * Test if a packet has a reference >>> + * >>> + * A packet has a reference if a reference was created on it by >>> + * odp_packet_ref_static(), odp_packet_ref(), or odp_packet_ref_pkt(). >>> + * >>> + * @param pkt Packet handle >>> + * >>> + * @retval 0 Packet does not have any references >>> + * @retval >0 Packet has N references based on it >>> + */ >>> +int odp_packet_has_ref(odp_packet_t pkt); >>> + >>> +/* >>> + * >>> * Copy >>> * ******************************************************** >>> * >>> >>