> -----Original Message----- > From: Bill Fischofer [mailto:[email protected]] > Sent: Thursday, December 22, 2016 6:59 PM > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- > labs.com> > Cc: lng-odp-forward <[email protected]> > Subject: Re: [lng-odp] [API-NEXT PATCH 1/7] api: packet: src and dst > packet must not be the same > > On Thu, Dec 22, 2016 at 8:33 AM, Petri Savolainen > <[email protected]> wrote: > > Concat and copy_from_pkt operations must be called with src and > > dst packet handles which refer to the same packet. > > > > Signed-off-by: Petri Savolainen <[email protected]> > > --- > > include/odp/api/spec/packet.h | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/include/odp/api/spec/packet.h > b/include/odp/api/spec/packet.h > > index faf62e2..4a86eba 100644 > > --- a/include/odp/api/spec/packet.h > > +++ b/include/odp/api/spec/packet.h > > @@ -781,7 +781,8 @@ uint32_t odp_packet_seg_data_len(odp_packet_t pkt, > odp_packet_seg_t seg); > > * Concatenate all packet data from 'src' packet into tail of 'dst' > packet. > > * Operation preserves 'dst' packet metadata in the resulting packet, > > * while 'src' packet handle, metadata and old segment handles for both > packets > > - * become invalid. > > + * become invalid. Source and destination packet handles must not refer > to > > + * the same packet. > > Why introduce this restriction? Concatenating a packet to itself is a > well-defined operation and is supported by other languages that > provide similar concatenation operations.
Concat is not the same as copy. Application forms a new packet from two old packets. If application wants to duplicate data in one packet it must do a copy. If you concat a packet into itself, you form a loop which does not make any sense. > > > * > > * A successful operation overwrites 'dst' packet handle with a new > handle, > > * which application must use as the reference to the resulting packet > > @@ -928,6 +929,9 @@ int odp_packet_copy_from_mem(odp_packet_t pkt, > uint32_t offset, > > * Copy 'len' bytes of data from 'src' packet to 'dst' packet. Copy > starts from > > * the specified source and destination packet offsets. Copied areas > > * (offset ... offset + len) must not exceed their packet data lengths. > > + * Source and destination packet handles must not refer to the same > packet (use > > + * odp_packet_copy_data() or odp_packet_move_data() for a single > packet). > > Same question here. It's simple enough for the implementation to check > this and behave appropriately (as the current implementation does). > Why introduce this change now? As the documentation change highlights: we have odp_packet_copy_data() and odp_packet_move_data() defined exactly for the case of copying or moving data within one packet. odp_packet_copy_from_pkt() is defined for copying data between two different packets. Application must use the right tool for the problem, instead of implementation needing to check every time if application happens to use a wrong tool. So, it's not change in behavior, but more explicit definition of the purpose of the existing calls. -Petri
