diff mbox

[PATCHv3,1/2] linux-generic: packet: copy user area as part of odp_packet_copy()

Message ID 1465474361-8019-1-git-send-email-bill.fischofer@linaro.org
State Accepted
Commit f9baafc4a5644973410ef6c5a1d233671bec1d17
Headers show

Commit Message

Bill Fischofer June 9, 2016, 12:12 p.m. UTC
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(-)

Comments

Bill Fischofer June 29, 2016, 9:57 p.m. UTC | #1
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 mbox

Patch

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;
 }
 
 /**