Message ID | 1465474361-8019-1-git-send-email-bill.fischofer@linaro.org |
---|---|
State | Accepted |
Commit | f9baafc4a5644973410ef6c5a1d233671bec1d17 |
Headers | show |
Ping. This has Zoltan's review and needs a merge. On Thu, Jun 9, 2016 at 7:12 AM, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2310 by copying the > user area as part of odp_packet_copy(). The copy fails if the user area > size of the destination pool is not large enough to hold the source packet > user area. > > Reported-by: Zoltan Kiss <zoltan.kiss@linaro.org> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > Reviewed-by: Zoltan Kiss <zoltan.kiss@linaro.org> > --- > v3: Resplit changes (oversight caught by Zoltan during review) > > v2: Zoltan review comments. Include additional metadata like ts, color, > that > were added more recently. > > .../linux-generic/include/odp_packet_internal.h | 5 ++++- > platform/linux-generic/odp_packet.c | 23 > +++++++++------------- > 2 files changed, 13 insertions(+), 15 deletions(-) > > diff --git a/platform/linux-generic/include/odp_packet_internal.h > b/platform/linux-generic/include/odp_packet_internal.h > index d5ace12..ab54227 100644 > --- a/platform/linux-generic/include/odp_packet_internal.h > +++ b/platform/linux-generic/include/odp_packet_internal.h > @@ -192,6 +192,9 @@ static inline void > copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr, > dst_hdr->l4_len = src_hdr->l4_len; > > dst_hdr->dst_queue = src_hdr->dst_queue; > + dst_hdr->flow_hash = src_hdr->flow_hash; > + dst_hdr->timestamp = src_hdr->timestamp; > + dst_hdr->op_result = src_hdr->op_result; > } > > static inline void *packet_map(odp_packet_hdr_t *pkt_hdr, > @@ -294,7 +297,7 @@ static inline int > packet_parse_not_complete(odp_packet_hdr_t *pkt_hdr) > } > > /* Forward declarations */ > -void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t > dstpkt); > +int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t > dstpkt); > > odp_packet_t packet_alloc(odp_pool_t pool_hdl, uint32_t len, int parse); > > diff --git a/platform/linux-generic/odp_packet.c > b/platform/linux-generic/odp_packet.c > index 2868736..40549a8 100644 > --- a/platform/linux-generic/odp_packet.c > +++ b/platform/linux-generic/odp_packet.c > @@ -756,22 +756,11 @@ odp_packet_t odp_packet_copy(odp_packet_t pkt, > odp_pool_t pool) > { > odp_packet_hdr_t *srchdr = odp_packet_hdr(pkt); > uint32_t pktlen = srchdr->frame_len; > - uint32_t meta_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t, buf_hdr); > odp_packet_t newpkt = odp_packet_alloc(pool, pktlen); > > if (newpkt != ODP_PACKET_INVALID) { > - odp_packet_hdr_t *newhdr = odp_packet_hdr(newpkt); > - uint8_t *newstart, *srcstart; > - > - /* Must copy metadata first, followed by packet data */ > - newstart = (uint8_t *)newhdr + meta_offset; > - srcstart = (uint8_t *)srchdr + meta_offset; > - > - memcpy(newstart, srcstart, > - sizeof(odp_packet_hdr_t) - meta_offset); > - > - if (odp_packet_copy_from_pkt(newpkt, 0, pkt, 0, > - pktlen) != 0) { > + if (_odp_packet_copy_md_to_packet(pkt, newpkt) || > + odp_packet_copy_from_pkt(newpkt, 0, pkt, 0, pktlen)) { > odp_packet_free(newpkt); > newpkt = ODP_PACKET_INVALID; > } > @@ -966,7 +955,7 @@ int odp_packet_is_valid(odp_packet_t pkt) > * > */ > > -void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t > dstpkt) > +int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t > dstpkt) > { > odp_packet_hdr_t *srchdr = odp_packet_hdr(srcpkt); > odp_packet_hdr_t *dsthdr = odp_packet_hdr(dstpkt); > @@ -986,6 +975,12 @@ void _odp_packet_copy_md_to_packet(odp_packet_t > srcpkt, odp_packet_t dstpkt) > odp_atomic_load_u32( > &srchdr->buf_hdr.ref_count)); > copy_packet_parser_metadata(srchdr, dsthdr); > + > + /* Metadata copied, but return indication of whether the packet > + * user area was truncated in the process. Note this can only > + * happen when copying between different pools. > + */ > + return dsthdr->buf_hdr.uarea_size < srchdr->buf_hdr.uarea_size; > } > > /** > -- > 2.7.4 > >
diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h index d5ace12..ab54227 100644 --- a/platform/linux-generic/include/odp_packet_internal.h +++ b/platform/linux-generic/include/odp_packet_internal.h @@ -192,6 +192,9 @@ static inline void copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr, dst_hdr->l4_len = src_hdr->l4_len; dst_hdr->dst_queue = src_hdr->dst_queue; + dst_hdr->flow_hash = src_hdr->flow_hash; + dst_hdr->timestamp = src_hdr->timestamp; + dst_hdr->op_result = src_hdr->op_result; } static inline void *packet_map(odp_packet_hdr_t *pkt_hdr, @@ -294,7 +297,7 @@ static inline int packet_parse_not_complete(odp_packet_hdr_t *pkt_hdr) } /* Forward declarations */ -void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt); +int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt); odp_packet_t packet_alloc(odp_pool_t pool_hdl, uint32_t len, int parse); diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c index 2868736..40549a8 100644 --- a/platform/linux-generic/odp_packet.c +++ b/platform/linux-generic/odp_packet.c @@ -756,22 +756,11 @@ odp_packet_t odp_packet_copy(odp_packet_t pkt, odp_pool_t pool) { odp_packet_hdr_t *srchdr = odp_packet_hdr(pkt); uint32_t pktlen = srchdr->frame_len; - uint32_t meta_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t, buf_hdr); odp_packet_t newpkt = odp_packet_alloc(pool, pktlen); if (newpkt != ODP_PACKET_INVALID) { - odp_packet_hdr_t *newhdr = odp_packet_hdr(newpkt); - uint8_t *newstart, *srcstart; - - /* Must copy metadata first, followed by packet data */ - newstart = (uint8_t *)newhdr + meta_offset; - srcstart = (uint8_t *)srchdr + meta_offset; - - memcpy(newstart, srcstart, - sizeof(odp_packet_hdr_t) - meta_offset); - - if (odp_packet_copy_from_pkt(newpkt, 0, pkt, 0, - pktlen) != 0) { + if (_odp_packet_copy_md_to_packet(pkt, newpkt) || + odp_packet_copy_from_pkt(newpkt, 0, pkt, 0, pktlen)) { odp_packet_free(newpkt); newpkt = ODP_PACKET_INVALID; } @@ -966,7 +955,7 @@ int odp_packet_is_valid(odp_packet_t pkt) * */ -void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt) +int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt) { odp_packet_hdr_t *srchdr = odp_packet_hdr(srcpkt); odp_packet_hdr_t *dsthdr = odp_packet_hdr(dstpkt); @@ -986,6 +975,12 @@ void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt) odp_atomic_load_u32( &srchdr->buf_hdr.ref_count)); copy_packet_parser_metadata(srchdr, dsthdr); + + /* Metadata copied, but return indication of whether the packet + * user area was truncated in the process. Note this can only + * happen when copying between different pools. + */ + return dsthdr->buf_hdr.uarea_size < srchdr->buf_hdr.uarea_size; } /**