Sounds like we should hold v1.0.2 for this bug fix?  Maxim: I think this
review is properly yours as you did the jumbo frame add for v1.0.1.

On Thu, Mar 26, 2015 at 12:52 PM, Stuart Haslam <[email protected]>
wrote:

> A magic number, used to ensure jumbo frames are transmitted and received
> in full, is written to the end of a packet buffer at a fixed offset of
> 9170 bytes. However for non-jumbo tests this is way beyond the end of
> the allocated buffer.
>
> Signed-off-by: Stuart Haslam <[email protected]>
> ---
>  test/validation/odp_pktio.c | 84
> ++++++++++++++++++++++++++++-----------------
>  1 file changed, 52 insertions(+), 32 deletions(-)
>
> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
> index e022c33..d653da4 100644
> --- a/test/validation/odp_pktio.c
> +++ b/test/validation/odp_pktio.c
> @@ -44,7 +44,6 @@ typedef struct ODP_PACKED {
>
>  /** structure of test packet UDP payload */
>  typedef struct ODP_PACKED {
> -       pkt_head_t head;
>         char data[PKT_BUF_JUMBO_MAX_PAYLOAD - sizeof(pkt_head_t) -
>                   sizeof(uint32be_t)];
>         uint32be_t magic2;
> @@ -74,33 +73,47 @@ static void pktio_pkt_set_macs(odp_packet_t pkt,
>
>  static uint32_t pkt_payload_len(void)
>  {
> -       return test_jumbo ? sizeof(pkt_test_data_t) : sizeof(pkt_head_t);
> +       uint32_t len;
> +
> +       len = sizeof(pkt_head_t);
> +
> +       if (test_jumbo)
> +               len += sizeof(pkt_test_data_t);
> +
> +       return len;
>  }
>
>  static int pktio_pkt_set_seq(odp_packet_t pkt)
>  {
>         static uint32_t tstseq;
> -       size_t l4_off;
> -       pkt_test_data_t *data;
> -       uint32_t len = pkt_payload_len();
> -
> +       size_t off;
> +       pkt_head_t head;
>
> -       l4_off = odp_packet_l4_offset(pkt);
> -       if (!l4_off) {
> +       off = odp_packet_l4_offset(pkt);
> +       if (!off) {
>                 CU_FAIL("packet L4 offset not set");
>                 return -1;
>         }
>
> -       data = calloc(1, len);
> -       CU_ASSERT_FATAL(data != NULL);
> +       head.magic = TEST_SEQ_MAGIC;
> +       head.seq   = tstseq;
> +
> +       off += ODPH_UDPHDR_LEN;
> +       odp_packet_copydata_in(pkt, off, sizeof(head), &head);
> +
> +       if (test_jumbo) {
> +               pkt_test_data_t *data;
> +
> +               data = calloc(1, sizeof(*data));
> +               CU_ASSERT_FATAL(data != NULL);
>
> -       data->head.magic = TEST_SEQ_MAGIC;
> -       data->magic2 = TEST_SEQ_MAGIC;
> -       data->head.seq   = tstseq;
> +               data->magic2 = TEST_SEQ_MAGIC;
> +
> +               off += sizeof(head);
> +               odp_packet_copydata_in(pkt, off, sizeof(*data), data);
> +               free(data);
> +       }
>
> -       odp_packet_copydata_in(pkt, l4_off+ODPH_UDPHDR_LEN,
> -                              len, data);
> -       free(data);
>         tstseq++;
>
>         return 0;
> @@ -108,30 +121,37 @@ static int pktio_pkt_set_seq(odp_packet_t pkt)
>
>  static uint32_t pktio_pkt_seq(odp_packet_t pkt)
>  {
> -       size_t l4_off;
> +       size_t off;
>         uint32_t seq = TEST_SEQ_INVALID;
> -       pkt_test_data_t *data;
> -       uint32_t len = pkt_payload_len();
> +       pkt_head_t head;
>
> -       l4_off = odp_packet_l4_offset(pkt);
> -       if (l4_off ==  ODP_PACKET_OFFSET_INVALID)
> +       off = odp_packet_l4_offset(pkt);
> +       if (off ==  ODP_PACKET_OFFSET_INVALID)
>                 return TEST_SEQ_INVALID;
>
> -       data = calloc(1, len);
> -       CU_ASSERT_FATAL(data != NULL);
> +       off += ODPH_UDPHDR_LEN;
> +       odp_packet_copydata_out(pkt, off, sizeof(head), &head);
>
> -       odp_packet_copydata_out(pkt, l4_off+ODPH_UDPHDR_LEN,
> -                               len, data);
> +       if (head.magic != TEST_SEQ_MAGIC)
> +               return TEST_SEQ_INVALID;
>
> -       if (data->head.magic == TEST_SEQ_MAGIC) {
> -               if (test_jumbo && data->magic2 != TEST_SEQ_MAGIC) {
> -                       free(data);
> -                       return TEST_SEQ_INVALID;
> -               }
> -               seq = data->head.seq;
> +       seq = head.seq;
> +
> +       if (test_jumbo) {
> +               pkt_test_data_t *data;
> +
> +               data = calloc(1, sizeof(*data));
> +               CU_ASSERT_FATAL(data != NULL);
> +
> +               off += sizeof(head);
> +               odp_packet_copydata_out(pkt, off, sizeof(*data), data);
> +
> +               if (data->magic2 != TEST_SEQ_MAGIC)
> +                       seq = TEST_SEQ_INVALID;
> +
> +               free(data);
>         }
>
> -       free(data);
>         return seq;
>  }
>
> --
> 2.1.1
>
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to