OK. For this series:
Reviewed-and-tested-by: Bill Fischofer <bill.fischo...@linaro.org> On Fri, Dec 23, 2016 at 1:56 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia-bell-labs.com> wrote: > > >> -----Original Message----- >> From: Bill Fischofer [mailto:bill.fischo...@linaro.org] >> Sent: Thursday, December 22, 2016 6:59 PM >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- >> labs.com> >> Cc: lng-odp-forward <lng-odp@lists.linaro.org> >> 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 >> <petri.savolai...@nokia.com> 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 <petri.savolai...@nokia.com> >> > --- >> > 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 > >
