diff mbox series

[1/5] test: generator: compose sending packets from reference packet plus differences

Message ID 1486990163-20495-1-git-send-email-bogdan.pricope@linaro.org
State Superseded
Headers show
Series [1/5] test: generator: compose sending packets from reference packet plus differences | expand

Commit Message

Bogdan Pricope Feb. 13, 2017, 12:49 p.m. UTC
Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org>

---
 example/generator/odp_generator.c | 131 +++++++++++++++++++++++++++++++++-----
 1 file changed, 114 insertions(+), 17 deletions(-)

-- 
1.9.1

Comments

Bill Fischofer Feb. 13, 2017, 7:04 p.m. UTC | #1
On Mon, Feb 13, 2017 at 6:49 AM, Bogdan Pricope
<bogdan.pricope@linaro.org> wrote:
> Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org>

> ---

>  example/generator/odp_generator.c | 131 +++++++++++++++++++++++++++++++++-----

>  1 file changed, 114 insertions(+), 17 deletions(-)

>

> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c

> index 8062d87..d1e3ecc 100644

> --- a/example/generator/odp_generator.c

> +++ b/example/generator/odp_generator.c

> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)

>  }

>

>  /**

> - * set up an udp packet

> + * set up an udp packet reference

>   *

>   * @param pool Buffer pool to create packet in

>   *

>   * @return Handle of created packet

>   * @retval ODP_PACKET_INVALID  Packet could not be created

>   */

> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)

> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)

>  {

>         odp_packet_t pkt;

>         char *buf;

>         odph_ethhdr_t *eth;

>         odph_ipv4hdr_t *ip;

>         odph_udphdr_t *udp;

> -       unsigned short seq;

>

>         pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

>                                ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>         memcpy((char *)eth->src.addr, args->appl.srcmac.addr, ODPH_ETHADDR_LEN);

>         memcpy((char *)eth->dst.addr, args->appl.dstmac.addr, ODPH_ETHADDR_LEN);

>         eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);

> +

>         /* ip */

>         odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

> +       odp_packet_has_ipv4_set(pkt, 1);

>         ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>         ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>         ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>         ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN +

>                                        ODPH_IPV4HDR_LEN);

>         ip->proto = ODPH_IPPROTO_UDP;

> -       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

> -       ip->id = odp_cpu_to_be_16(seq);

> +       ip->id = 0;

> +       ip->ttl = 64;

>         ip->chksum = 0;

> -       odph_ipv4_csum_update(pkt);

> +

>         /* udp */

>         odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

> +       odp_packet_has_udp_set(pkt, 1);

>         udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>         udp->src_port = 0;

>         udp->dst_port = 0;

> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>  }

>

>  /**

> - * Set up an icmp packet

> + * set up an udp packet

> + *

> + * @param pool Buffer pool to create packet in

> + * @param pkt_ref Reference UDP packet

> + *

> + * @return Handle of created packet

> + * @retval ODP_PACKET_INVALID  Packet could not be created

> + */

> +static odp_packet_t pack_udp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)

> +{

> +       odp_packet_t pkt;

> +       char *buf;

> +       odph_ipv4hdr_t *ip;

> +       unsigned short seq;

> +

> +       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

> +                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +


From the title of this patch I thought you were going to make use of
packet references and this would probably be a good example to
illustrate such use. For example:

uint32_t hdr_len = ODPH_UDPHDR_LEN + ODPH_IPV4_HDR_LEN + ODPH_ETHHDR_LEN;

odp_packet_t hdr = odp_packet_alloc(pool, hdr_len);

if (hdr == ODP_PACKET_INVALID)
   return hdr;

pkt = odp_packet_ref_pkt(pkt_ref, hdr_len, hdr);

if (pkt == ODP_PACKET_INVALID) {
   odp_free(hdr);
   return pkt;
}

buf = (char *)odp_packet_data(pkt);
odp_memcpy(buf, odp_packet_data(pkt_ref), hdr_len);

/* Update IP ID and checksum */
...

This illustrates the use of references to avoid copying the payload,
which is now shared. This could be made even more efficient by
pre-initializing the hdrs and then skipping the header copy as well
and just create a reference to the payload, update the checksum, and
go.


> +       if (pkt == ODP_PACKET_INVALID)

> +               return pkt;

> +

> +       buf = (char *)odp_packet_data(pkt);

> +       odp_memcpy(buf, odp_packet_data(pkt_ref),

> +               args->appl.payload + ODPH_UDPHDR_LEN +

> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +

> +       /*Update IP ID and checksum*/

> +       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

> +       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

> +       ip->id = odp_cpu_to_be_16(seq);

> +       ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);

> +

> +       return pkt;

> +}

> +

> +/**

> + * Set up an icmp packet reference

>   *

>   * @param pool Buffer pool to create packet in

>   *

>   * @return Handle of created packet

>   * @retval ODP_PACKET_INVALID  Packet could not be created

>   */

> -static odp_packet_t pack_icmp_pkt(odp_pool_t pool)

> +static odp_packet_t setup_icmp_pkt_ref(odp_pool_t pool)

>  {

>         odp_packet_t pkt;

>         char *buf;

>         odph_ethhdr_t *eth;

>         odph_ipv4hdr_t *ip;

>         odph_icmphdr_t *icmp;

> -       struct timeval tval;

> -       uint8_t *tval_d;

> -       unsigned short seq;

>

>         args->appl.payload = 56;

>         pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN +

> -                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>

>         if (pkt == ODP_PACKET_INVALID)

>                 return pkt;

> @@ -265,18 +300,63 @@ static odp_packet_t pack_icmp_pkt(odp_pool_t pool)

>         ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>         ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

>         ip->ver_ihl = ODPH_IPV4 << 4 | ODPH_IPV4HDR_IHL_MIN;

> +       ip->ttl = 64;

>         ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_ICMPHDR_LEN +

>                                        ODPH_IPV4HDR_LEN);

>         ip->proto = ODPH_IPPROTO_ICMP;

> -       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;

> -       ip->id = odp_cpu_to_be_16(seq);

> +       ip->id = 0;

>         ip->chksum = 0;

> -       odph_ipv4_csum_update(pkt);

> +

>         /* icmp */

>         icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>         icmp->type = ICMP_ECHO;

>         icmp->code = 0;

>         icmp->un.echo.id = 0;

> +       icmp->un.echo.sequence = 0;

> +       icmp->chksum = 0;

> +

> +       return pkt;

> +}

> +

> +/**

> + * Set up an icmp packet

> + *

> + * @param pool Buffer pool to create packet in

> + * @param pkt_ref Reference ICMP packet

> + *

> + * @return Handle of created packet

> + * @retval ODP_PACKET_INVALID  Packet could not be created

> + */

> +static odp_packet_t pack_icmp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)

> +{

> +       odp_packet_t pkt;

> +       char *buf;

> +       odph_ipv4hdr_t *ip;

> +       odph_icmphdr_t *icmp;

> +       struct timeval tval;

> +       uint8_t *tval_d;

> +       unsigned short seq;

> +

> +       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN +

> +                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +

> +       if (pkt == ODP_PACKET_INVALID)

> +               return pkt;

> +

> +       buf = (char *)odp_packet_data(pkt);

> +       odp_memcpy(buf, odp_packet_data(pkt_ref),

> +               args->appl.payload + ODPH_ICMPHDR_LEN +

> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +

> +       /* ip */

> +       odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

> +       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

> +       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;

> +       ip->id = odp_cpu_to_be_16(seq);

> +       ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);

> +

> +       /* icmp */

> +       icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>         icmp->un.echo.sequence = ip->id;

>         tval_d = (uint8_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN +

>                                   ODPH_ICMPHDR_LEN);

> @@ -357,6 +437,7 @@ static int gen_send_thread(void *arg)

>         thread_args_t *thr_args;

>         odp_pktout_queue_t pktout;

>         odp_packet_t pkt;

> +       odp_packet_t pkt_ref = ODP_PACKET_INVALID;

>

>         thr = odp_thread_id();

>         thr_args = arg;

> @@ -373,6 +454,21 @@ static int gen_send_thread(void *arg)

>                 return -1;

>         }

>

> +       if (args->appl.mode == APPL_MODE_UDP)

> +               pkt_ref = setup_udp_pkt_ref(thr_args->pool);

> +       else if (args->appl.mode == APPL_MODE_PING)

> +               pkt_ref = setup_icmp_pkt_ref(thr_args->pool);

> +       else {

> +               EXAMPLE_ERR("  [%02i] Error: invalid processing mode %d\n",

> +                       thr, args->appl.mode);

> +               return -1;

> +       }

> +       if (pkt_ref == ODP_PACKET_INVALID) {

> +               EXAMPLE_ERR("  [%2i] Error: reference packet creation failed\n",

> +                       thr);

> +               return -1;

> +       }

> +

>         printf("  [%02i] created mode: SEND\n", thr);

>

>         odp_barrier_wait(&barrier);

> @@ -386,9 +482,9 @@ static int gen_send_thread(void *arg)

>                 pkt = ODP_PACKET_INVALID;

>

>                 if (args->appl.mode == APPL_MODE_UDP)

> -                       pkt = pack_udp_pkt(thr_args->pool);

> +                       pkt = pack_udp_pkt(thr_args->pool, pkt_ref);

>                 else if (args->appl.mode == APPL_MODE_PING)

> -                       pkt = pack_icmp_pkt(thr_args->pool);

> +                       pkt = pack_icmp_pkt(thr_args->pool, pkt_ref);

>

>                 if (pkt == ODP_PACKET_INVALID) {

>                         /* Thread gives up as soon as it sees the pool empty.

> @@ -440,6 +536,7 @@ static int gen_send_thread(void *arg)

>                         args->appl.timeout--;

>                 }

>         }

> +       odp_packet_free(pkt_ref);

>

>         return 0;

>  }

> --

> 1.9.1

>
Bogdan Pricope Feb. 14, 2017, 12:09 p.m. UTC | #2
Hi,

Any mechanism that will prevent from copying data is a good thing.
This change can be done after the new API is merged in master branch.


Br,
Bogdan

On 13 February 2017 at 21:04, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> On Mon, Feb 13, 2017 at 6:49 AM, Bogdan Pricope

> <bogdan.pricope@linaro.org> wrote:

>> Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org>

>> ---

>>  example/generator/odp_generator.c | 131 +++++++++++++++++++++++++++++++++-----

>>  1 file changed, 114 insertions(+), 17 deletions(-)

>>

>> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c

>> index 8062d87..d1e3ecc 100644

>> --- a/example/generator/odp_generator.c

>> +++ b/example/generator/odp_generator.c

>> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)

>>  }

>>

>>  /**

>> - * set up an udp packet

>> + * set up an udp packet reference

>>   *

>>   * @param pool Buffer pool to create packet in

>>   *

>>   * @return Handle of created packet

>>   * @retval ODP_PACKET_INVALID  Packet could not be created

>>   */

>> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)

>>  {

>>         odp_packet_t pkt;

>>         char *buf;

>>         odph_ethhdr_t *eth;

>>         odph_ipv4hdr_t *ip;

>>         odph_udphdr_t *udp;

>> -       unsigned short seq;

>>

>>         pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

>>                                ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>>         memcpy((char *)eth->src.addr, args->appl.srcmac.addr, ODPH_ETHADDR_LEN);

>>         memcpy((char *)eth->dst.addr, args->appl.dstmac.addr, ODPH_ETHADDR_LEN);

>>         eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);

>> +

>>         /* ip */

>>         odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

>> +       odp_packet_has_ipv4_set(pkt, 1);

>>         ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>>         ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>>         ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

>> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>>         ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN +

>>                                        ODPH_IPV4HDR_LEN);

>>         ip->proto = ODPH_IPPROTO_UDP;

>> -       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

>> -       ip->id = odp_cpu_to_be_16(seq);

>> +       ip->id = 0;

>> +       ip->ttl = 64;

>>         ip->chksum = 0;

>> -       odph_ipv4_csum_update(pkt);

>> +

>>         /* udp */

>>         odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>> +       odp_packet_has_udp_set(pkt, 1);

>>         udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>>         udp->src_port = 0;

>>         udp->dst_port = 0;

>> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>>  }

>>

>>  /**

>> - * Set up an icmp packet

>> + * set up an udp packet

>> + *

>> + * @param pool Buffer pool to create packet in

>> + * @param pkt_ref Reference UDP packet

>> + *

>> + * @return Handle of created packet

>> + * @retval ODP_PACKET_INVALID  Packet could not be created

>> + */

>> +static odp_packet_t pack_udp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)

>> +{

>> +       odp_packet_t pkt;

>> +       char *buf;

>> +       odph_ipv4hdr_t *ip;

>> +       unsigned short seq;

>> +

>> +       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

>> +                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>> +

>

> From the title of this patch I thought you were going to make use of

> packet references and this would probably be a good example to

> illustrate such use. For example:

>

> uint32_t hdr_len = ODPH_UDPHDR_LEN + ODPH_IPV4_HDR_LEN + ODPH_ETHHDR_LEN;

>

> odp_packet_t hdr = odp_packet_alloc(pool, hdr_len);

>

> if (hdr == ODP_PACKET_INVALID)

>    return hdr;

>

> pkt = odp_packet_ref_pkt(pkt_ref, hdr_len, hdr);

>

> if (pkt == ODP_PACKET_INVALID) {

>    odp_free(hdr);

>    return pkt;

> }

>

> buf = (char *)odp_packet_data(pkt);

> odp_memcpy(buf, odp_packet_data(pkt_ref), hdr_len);

>

> /* Update IP ID and checksum */

> ...

>

> This illustrates the use of references to avoid copying the payload,

> which is now shared. This could be made even more efficient by

> pre-initializing the hdrs and then skipping the header copy as well

> and just create a reference to the payload, update the checksum, and

> go.

>

>

>> +       if (pkt == ODP_PACKET_INVALID)

>> +               return pkt;

>> +

>> +       buf = (char *)odp_packet_data(pkt);

>> +       odp_memcpy(buf, odp_packet_data(pkt_ref),

>> +               args->appl.payload + ODPH_UDPHDR_LEN +

>> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>> +

>> +       /*Update IP ID and checksum*/

>> +       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>> +       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

>> +       ip->id = odp_cpu_to_be_16(seq);

>> +       ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);

>> +

>> +       return pkt;

>> +}

>> +

>> +/**

>> + * Set up an icmp packet reference

>>   *

>>   * @param pool Buffer pool to create packet in

>>   *

>>   * @return Handle of created packet

>>   * @retval ODP_PACKET_INVALID  Packet could not be created

>>   */

>> -static odp_packet_t pack_icmp_pkt(odp_pool_t pool)

>> +static odp_packet_t setup_icmp_pkt_ref(odp_pool_t pool)

>>  {

>>         odp_packet_t pkt;

>>         char *buf;

>>         odph_ethhdr_t *eth;

>>         odph_ipv4hdr_t *ip;

>>         odph_icmphdr_t *icmp;

>> -       struct timeval tval;

>> -       uint8_t *tval_d;

>> -       unsigned short seq;

>>

>>         args->appl.payload = 56;

>>         pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN +

>> -                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>>

>>         if (pkt == ODP_PACKET_INVALID)

>>                 return pkt;

>> @@ -265,18 +300,63 @@ static odp_packet_t pack_icmp_pkt(odp_pool_t pool)

>>         ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>>         ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

>>         ip->ver_ihl = ODPH_IPV4 << 4 | ODPH_IPV4HDR_IHL_MIN;

>> +       ip->ttl = 64;

>>         ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_ICMPHDR_LEN +

>>                                        ODPH_IPV4HDR_LEN);

>>         ip->proto = ODPH_IPPROTO_ICMP;

>> -       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;

>> -       ip->id = odp_cpu_to_be_16(seq);

>> +       ip->id = 0;

>>         ip->chksum = 0;

>> -       odph_ipv4_csum_update(pkt);

>> +

>>         /* icmp */

>>         icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>>         icmp->type = ICMP_ECHO;

>>         icmp->code = 0;

>>         icmp->un.echo.id = 0;

>> +       icmp->un.echo.sequence = 0;

>> +       icmp->chksum = 0;

>> +

>> +       return pkt;

>> +}

>> +

>> +/**

>> + * Set up an icmp packet

>> + *

>> + * @param pool Buffer pool to create packet in

>> + * @param pkt_ref Reference ICMP packet

>> + *

>> + * @return Handle of created packet

>> + * @retval ODP_PACKET_INVALID  Packet could not be created

>> + */

>> +static odp_packet_t pack_icmp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)

>> +{

>> +       odp_packet_t pkt;

>> +       char *buf;

>> +       odph_ipv4hdr_t *ip;

>> +       odph_icmphdr_t *icmp;

>> +       struct timeval tval;

>> +       uint8_t *tval_d;

>> +       unsigned short seq;

>> +

>> +       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN +

>> +                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>> +

>> +       if (pkt == ODP_PACKET_INVALID)

>> +               return pkt;

>> +

>> +       buf = (char *)odp_packet_data(pkt);

>> +       odp_memcpy(buf, odp_packet_data(pkt_ref),

>> +               args->appl.payload + ODPH_ICMPHDR_LEN +

>> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>> +

>> +       /* ip */

>> +       odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

>> +       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>> +       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;

>> +       ip->id = odp_cpu_to_be_16(seq);

>> +       ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);

>> +

>> +       /* icmp */

>> +       icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>>         icmp->un.echo.sequence = ip->id;

>>         tval_d = (uint8_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN +

>>                                   ODPH_ICMPHDR_LEN);

>> @@ -357,6 +437,7 @@ static int gen_send_thread(void *arg)

>>         thread_args_t *thr_args;

>>         odp_pktout_queue_t pktout;

>>         odp_packet_t pkt;

>> +       odp_packet_t pkt_ref = ODP_PACKET_INVALID;

>>

>>         thr = odp_thread_id();

>>         thr_args = arg;

>> @@ -373,6 +454,21 @@ static int gen_send_thread(void *arg)

>>                 return -1;

>>         }

>>

>> +       if (args->appl.mode == APPL_MODE_UDP)

>> +               pkt_ref = setup_udp_pkt_ref(thr_args->pool);

>> +       else if (args->appl.mode == APPL_MODE_PING)

>> +               pkt_ref = setup_icmp_pkt_ref(thr_args->pool);

>> +       else {

>> +               EXAMPLE_ERR("  [%02i] Error: invalid processing mode %d\n",

>> +                       thr, args->appl.mode);

>> +               return -1;

>> +       }

>> +       if (pkt_ref == ODP_PACKET_INVALID) {

>> +               EXAMPLE_ERR("  [%2i] Error: reference packet creation failed\n",

>> +                       thr);

>> +               return -1;

>> +       }

>> +

>>         printf("  [%02i] created mode: SEND\n", thr);

>>

>>         odp_barrier_wait(&barrier);

>> @@ -386,9 +482,9 @@ static int gen_send_thread(void *arg)

>>                 pkt = ODP_PACKET_INVALID;

>>

>>                 if (args->appl.mode == APPL_MODE_UDP)

>> -                       pkt = pack_udp_pkt(thr_args->pool);

>> +                       pkt = pack_udp_pkt(thr_args->pool, pkt_ref);

>>                 else if (args->appl.mode == APPL_MODE_PING)

>> -                       pkt = pack_icmp_pkt(thr_args->pool);

>> +                       pkt = pack_icmp_pkt(thr_args->pool, pkt_ref);

>>

>>                 if (pkt == ODP_PACKET_INVALID) {

>>                         /* Thread gives up as soon as it sees the pool empty.

>> @@ -440,6 +536,7 @@ static int gen_send_thread(void *arg)

>>                         args->appl.timeout--;

>>                 }

>>         }

>> +       odp_packet_free(pkt_ref);

>>

>>         return 0;

>>  }

>> --

>> 1.9.1

>>
Nicolas Morey-Chaisemartin Feb. 14, 2017, 2:38 p.m. UTC | #3
Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
> Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org>

> ---

>  example/generator/odp_generator.c | 131 +++++++++++++++++++++++++++++++++-----

>  1 file changed, 114 insertions(+), 17 deletions(-)

>

> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c

> index 8062d87..d1e3ecc 100644

> --- a/example/generator/odp_generator.c

> +++ b/example/generator/odp_generator.c

> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)

>  }

>  

>  /**

> - * set up an udp packet

> + * set up an udp packet reference

>   *

>   * @param pool Buffer pool to create packet in

>   *

>   * @return Handle of created packet

>   * @retval ODP_PACKET_INVALID  Packet could not be created

>   */

> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)

> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)

>  {

>  	odp_packet_t pkt;

>  	char *buf;

>  	odph_ethhdr_t *eth;

>  	odph_ipv4hdr_t *ip;

>  	odph_udphdr_t *udp;

> -	unsigned short seq;

>  

>  	pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

>  			       ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>  	memcpy((char *)eth->src.addr, args->appl.srcmac.addr, ODPH_ETHADDR_LEN);

>  	memcpy((char *)eth->dst.addr, args->appl.dstmac.addr, ODPH_ETHADDR_LEN);

>  	eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);

> +

>  	/* ip */

>  	odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

> +	odp_packet_has_ipv4_set(pkt, 1);

>  	ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>  	ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>  	ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>  	ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN +

>  				       ODPH_IPV4HDR_LEN);

>  	ip->proto = ODPH_IPPROTO_UDP;

> -	seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

> -	ip->id = odp_cpu_to_be_16(seq);

> +	ip->id = 0;

> +	ip->ttl = 64;

>  	ip->chksum = 0;

> -	odph_ipv4_csum_update(pkt);

> +

>  	/* udp */

>  	odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

> +	odp_packet_has_udp_set(pkt, 1);

>  	udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>  	udp->src_port = 0;

>  	udp->dst_port = 0;

> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>  }

>  

The calls to odp_packet_has_ipv4_set and odp_packet_has_udp_set are actually a bug fix needed to have the proper checksum computations.
Without these calls, the checksum are set to 0 as all the packet parse flags are 0.

It might be worth splitting this into a specific patch.
I posted one for monarch without any answer to it, but it's something that should be put in all branches as it's a bug fix.

Nicolas
Bogdan Pricope Feb. 15, 2017, 8:58 a.m. UTC | #4
Hi,

There were multiple small “fixes” required to make the packet valid.
They should go in together and in all branches.

Br,
Bogdan

On 14 February 2017 at 16:38, Nicolas Morey-Chaisemartin
<nmorey@kalray.eu> wrote:
>

>

> Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :

>> Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org>

>> ---

>>  example/generator/odp_generator.c | 131 +++++++++++++++++++++++++++++++++-----

>>  1 file changed, 114 insertions(+), 17 deletions(-)

>>

>> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c

>> index 8062d87..d1e3ecc 100644

>> --- a/example/generator/odp_generator.c

>> +++ b/example/generator/odp_generator.c

>> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)

>>  }

>>

>>  /**

>> - * set up an udp packet

>> + * set up an udp packet reference

>>   *

>>   * @param pool Buffer pool to create packet in

>>   *

>>   * @return Handle of created packet

>>   * @retval ODP_PACKET_INVALID  Packet could not be created

>>   */

>> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)

>>  {

>>       odp_packet_t pkt;

>>       char *buf;

>>       odph_ethhdr_t *eth;

>>       odph_ipv4hdr_t *ip;

>>       odph_udphdr_t *udp;

>> -     unsigned short seq;

>>

>>       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

>>                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>>       memcpy((char *)eth->src.addr, args->appl.srcmac.addr, ODPH_ETHADDR_LEN);

>>       memcpy((char *)eth->dst.addr, args->appl.dstmac.addr, ODPH_ETHADDR_LEN);

>>       eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);

>> +

>>       /* ip */

>>       odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

>> +     odp_packet_has_ipv4_set(pkt, 1);

>>       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>>       ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>>       ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

>> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>>       ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN +

>>                                      ODPH_IPV4HDR_LEN);

>>       ip->proto = ODPH_IPPROTO_UDP;

>> -     seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

>> -     ip->id = odp_cpu_to_be_16(seq);

>> +     ip->id = 0;

>> +     ip->ttl = 64;

>>       ip->chksum = 0;

>> -     odph_ipv4_csum_update(pkt);

>> +

>>       /* udp */

>>       odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>> +     odp_packet_has_udp_set(pkt, 1);

>>       udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>>       udp->src_port = 0;

>>       udp->dst_port = 0;

>> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>>  }

>>

> The calls to odp_packet_has_ipv4_set and odp_packet_has_udp_set are actually a bug fix needed to have the proper checksum computations.

> Without these calls, the checksum are set to 0 as all the packet parse flags are 0.

>

> It might be worth splitting this into a specific patch.

> I posted one for monarch without any answer to it, but it's something that should be put in all branches as it's a bug fix.

>

> Nicolas
Mike Holmes Feb. 15, 2017, 4:55 p.m. UTC | #5
So this is a request to add this to Monarch LTS then ?

On 15 February 2017 at 03:58, Bogdan Pricope <bogdan.pricope@linaro.org>
wrote:

> Hi,

>

> There were multiple small “fixes” required to make the packet valid.

> They should go in together and in all branches.

>

> Br,

> Bogdan

>

> On 14 February 2017 at 16:38, Nicolas Morey-Chaisemartin

> <nmorey@kalray.eu> wrote:

> >

> >

> > Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :

> >> Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org>

> >> ---

> >>  example/generator/odp_generator.c | 131 ++++++++++++++++++++++++++++++

> +++-----

> >>  1 file changed, 114 insertions(+), 17 deletions(-)

> >>

> >> diff --git a/example/generator/odp_generator.c b/example/generator/odp_

> generator.c

> >> index 8062d87..d1e3ecc 100644

> >> --- a/example/generator/odp_generator.c

> >> +++ b/example/generator/odp_generator.c

> >> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)

> >>  }

> >>

> >>  /**

> >> - * set up an udp packet

> >> + * set up an udp packet reference

> >>   *

> >>   * @param pool Buffer pool to create packet in

> >>   *

> >>   * @return Handle of created packet

> >>   * @retval ODP_PACKET_INVALID  Packet could not be created

> >>   */

> >> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)

> >> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)

> >>  {

> >>       odp_packet_t pkt;

> >>       char *buf;

> >>       odph_ethhdr_t *eth;

> >>       odph_ipv4hdr_t *ip;

> >>       odph_udphdr_t *udp;

> >> -     unsigned short seq;

> >>

> >>       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN

> +

> >>                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> >> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

> >>       memcpy((char *)eth->src.addr, args->appl.srcmac.addr,

> ODPH_ETHADDR_LEN);

> >>       memcpy((char *)eth->dst.addr, args->appl.dstmac.addr,

> ODPH_ETHADDR_LEN);

> >>       eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);

> >> +

> >>       /* ip */

> >>       odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

> >> +     odp_packet_has_ipv4_set(pkt, 1);

> >>       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

> >>       ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

> >>       ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

> >> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

> >>       ip->tot_len = odp_cpu_to_be_16(args->appl.payload +

> ODPH_UDPHDR_LEN +

> >>                                      ODPH_IPV4HDR_LEN);

> >>       ip->proto = ODPH_IPPROTO_UDP;

> >> -     seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

> >> -     ip->id = odp_cpu_to_be_16(seq);

> >> +     ip->id = 0;

> >> +     ip->ttl = 64;

> >>       ip->chksum = 0;

> >> -     odph_ipv4_csum_update(pkt);

> >> +

> >>       /* udp */

> >>       odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

> >> +     odp_packet_has_udp_set(pkt, 1);

> >>       udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

> >>       udp->src_port = 0;

> >>       udp->dst_port = 0;

> >> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

> >>  }

> >>

> > The calls to odp_packet_has_ipv4_set and odp_packet_has_udp_set are

> actually a bug fix needed to have the proper checksum computations.

> > Without these calls, the checksum are set to 0 as all the packet parse

> flags are 0.

> >

> > It might be worth splitting this into a specific patch.

> > I posted one for monarch without any answer to it, but it's something

> that should be put in all branches as it's a bug fix.

> >

> > Nicolas

>




-- 
Mike Holmes
Program Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"
Bogdan Pricope Feb. 16, 2017, 6:34 a.m. UTC | #6
Hi,

From my point of view, at the end of review process this series (+ the
patch related to stats logging message) should be added at least in
odp-dpdk.

Bogdan

On 15 February 2017 at 18:55, Mike Holmes <mike.holmes@linaro.org> wrote:
> So this is a request to add this to Monarch LTS then ?

>

> On 15 February 2017 at 03:58, Bogdan Pricope <bogdan.pricope@linaro.org>

> wrote:

>>

>> Hi,

>>

>> There were multiple small “fixes” required to make the packet valid.

>> They should go in together and in all branches.

>>

>> Br,

>> Bogdan

>>

>> On 14 February 2017 at 16:38, Nicolas Morey-Chaisemartin

>> <nmorey@kalray.eu> wrote:

>> >

>> >

>> > Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :

>> >> Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org>

>> >> ---

>> >>  example/generator/odp_generator.c | 131

>> >> +++++++++++++++++++++++++++++++++-----

>> >>  1 file changed, 114 insertions(+), 17 deletions(-)

>> >>

>> >> diff --git a/example/generator/odp_generator.c

>> >> b/example/generator/odp_generator.c

>> >> index 8062d87..d1e3ecc 100644

>> >> --- a/example/generator/odp_generator.c

>> >> +++ b/example/generator/odp_generator.c

>> >> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int

>> >> *paddr)

>> >>  }

>> >>

>> >>  /**

>> >> - * set up an udp packet

>> >> + * set up an udp packet reference

>> >>   *

>> >>   * @param pool Buffer pool to create packet in

>> >>   *

>> >>   * @return Handle of created packet

>> >>   * @retval ODP_PACKET_INVALID  Packet could not be created

>> >>   */

>> >> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>> >> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)

>> >>  {

>> >>       odp_packet_t pkt;

>> >>       char *buf;

>> >>       odph_ethhdr_t *eth;

>> >>       odph_ipv4hdr_t *ip;

>> >>       odph_udphdr_t *udp;

>> >> -     unsigned short seq;

>> >>

>> >>       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN

>> >> +

>> >>                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>> >> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>> >>       memcpy((char *)eth->src.addr, args->appl.srcmac.addr,

>> >> ODPH_ETHADDR_LEN);

>> >>       memcpy((char *)eth->dst.addr, args->appl.dstmac.addr,

>> >> ODPH_ETHADDR_LEN);

>> >>       eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);

>> >> +

>> >>       /* ip */

>> >>       odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

>> >> +     odp_packet_has_ipv4_set(pkt, 1);

>> >>       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>> >>       ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>> >>       ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

>> >> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>> >>       ip->tot_len = odp_cpu_to_be_16(args->appl.payload +

>> >> ODPH_UDPHDR_LEN +

>> >>                                      ODPH_IPV4HDR_LEN);

>> >>       ip->proto = ODPH_IPPROTO_UDP;

>> >> -     seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

>> >> -     ip->id = odp_cpu_to_be_16(seq);

>> >> +     ip->id = 0;

>> >> +     ip->ttl = 64;

>> >>       ip->chksum = 0;

>> >> -     odph_ipv4_csum_update(pkt);

>> >> +

>> >>       /* udp */

>> >>       odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN +

>> >> ODPH_IPV4HDR_LEN);

>> >> +     odp_packet_has_udp_set(pkt, 1);

>> >>       udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN +

>> >> ODPH_IPV4HDR_LEN);

>> >>       udp->src_port = 0;

>> >>       udp->dst_port = 0;

>> >> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>> >>  }

>> >>

>> > The calls to odp_packet_has_ipv4_set and odp_packet_has_udp_set are

>> > actually a bug fix needed to have the proper checksum computations.

>> > Without these calls, the checksum are set to 0 as all the packet parse

>> > flags are 0.

>> >

>> > It might be worth splitting this into a specific patch.

>> > I posted one for monarch without any answer to it, but it's something

>> > that should be put in all branches as it's a bug fix.

>> >

>> > Nicolas

>

>

>

>

> --

> Mike Holmes

> Program Manager - Linaro Networking Group

> Linaro.org │ Open source software for ARM SoCs

> "Work should be fun and collaborative, the rest follows"

>

>
Yi He March 1, 2017, 6:59 a.m. UTC | #7
Comments on 1/5 and 2/5, other patches in this series look OK to me.

On 13 February 2017 at 20:49, Bogdan Pricope <bogdan.pricope@linaro.org>
wrote:

> Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org>

> ---

>  example/generator/odp_generator.c | 131 ++++++++++++++++++++++++++++++

> +++-----

>  1 file changed, 114 insertions(+), 17 deletions(-)

>

> diff --git a/example/generator/odp_generator.c b/example/generator/odp_

> generator.c

> index 8062d87..d1e3ecc 100644

> --- a/example/generator/odp_generator.c

> +++ b/example/generator/odp_generator.c

> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)

>  }

>

>  /**

> - * set up an udp packet

> + * set up an udp packet reference

>   *

>   * @param pool Buffer pool to create packet in

>   *

>   * @return Handle of created packet

>   * @retval ODP_PACKET_INVALID  Packet could not be created

>   */

> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)

> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)

>  {

>         odp_packet_t pkt;

>         char *buf;

>         odph_ethhdr_t *eth;

>         odph_ipv4hdr_t *ip;

>         odph_udphdr_t *udp;

> -       unsigned short seq;

>

>         pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

>                                ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>         memcpy((char *)eth->src.addr, args->appl.srcmac.addr,

> ODPH_ETHADDR_LEN);

>         memcpy((char *)eth->dst.addr, args->appl.dstmac.addr,

> ODPH_ETHADDR_LEN);

>         eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);

> +

>         /* ip */

>         odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

> +       odp_packet_has_ipv4_set(pkt, 1);

>         ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>         ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>         ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>         ip->tot_len = odp_cpu_to_be_16(args->appl.payload +

> ODPH_UDPHDR_LEN +

>                                        ODPH_IPV4HDR_LEN);

>         ip->proto = ODPH_IPPROTO_UDP;

> -       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

> -       ip->id = odp_cpu_to_be_16(seq);

> +       ip->id = 0;

> +       ip->ttl = 64;

>         ip->chksum = 0;

> -       odph_ipv4_csum_update(pkt);

> +

>         /* udp */

>         odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

> +       odp_packet_has_udp_set(pkt, 1);

>         udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>         udp->src_port = 0;

>         udp->dst_port = 0;

> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>  }

>

>  /**

> - * Set up an icmp packet

> + * set up an udp packet

> + *

> + * @param pool Buffer pool to create packet in

> + * @param pkt_ref Reference UDP packet

> + *

> + * @return Handle of created packet

> + * @retval ODP_PACKET_INVALID  Packet could not be created

> + */

> +static odp_packet_t pack_udp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)

> +{

> +       odp_packet_t pkt;

> +       char *buf;

> +       odph_ipv4hdr_t *ip;

> +       unsigned short seq;

> +

> +       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

> +                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +

> +       if (pkt == ODP_PACKET_INVALID)

> +               return pkt;

> +

> +       buf = (char *)odp_packet_data(pkt);

> +       odp_memcpy(buf, odp_packet_data(pkt_ref),

> +               args->appl.payload + ODPH_UDPHDR_LEN +

> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +

> +       /*Update IP ID and checksum*/

> +       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

> +       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

> +       ip->id = odp_cpu_to_be_16(seq);

> +       ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);

> +

> +       return pkt;

> +}

> +

> +/**

> + * Set up an icmp packet reference

>   *

>   * @param pool Buffer pool to create packet in

>   *

>   * @return Handle of created packet

>   * @retval ODP_PACKET_INVALID  Packet could not be created

>   */

> -static odp_packet_t pack_icmp_pkt(odp_pool_t pool)

> +static odp_packet_t setup_icmp_pkt_ref(odp_pool_t pool)

>  {

>         odp_packet_t pkt;

>         char *buf;

>         odph_ethhdr_t *eth;

>         odph_ipv4hdr_t *ip;

>         odph_icmphdr_t *icmp;

> -       struct timeval tval;

> -       uint8_t *tval_d;

> -       unsigned short seq;

>

>         args->appl.payload = 56;

>         pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN

> +

> -                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>

>         if (pkt == ODP_PACKET_INVALID)

>                 return pkt;

> @@ -265,18 +300,63 @@ static odp_packet_t pack_icmp_pkt(odp_pool_t pool)

>         ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>         ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

>         ip->ver_ihl = ODPH_IPV4 << 4 | ODPH_IPV4HDR_IHL_MIN;

> +       ip->ttl = 64;

>         ip->tot_len = odp_cpu_to_be_16(args->appl.payload +

> ODPH_ICMPHDR_LEN +

>                                        ODPH_IPV4HDR_LEN);

>         ip->proto = ODPH_IPPROTO_ICMP;

> -       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;

> -       ip->id = odp_cpu_to_be_16(seq);

> +       ip->id = 0;

>         ip->chksum = 0;

> -       odph_ipv4_csum_update(pkt);

> +

>         /* icmp */

>         icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN +

> ODPH_IPV4HDR_LEN);

>         icmp->type = ICMP_ECHO;

>         icmp->code = 0;

>         icmp->un.echo.id = 0;

> +       icmp->un.echo.sequence = 0;

> +       icmp->chksum = 0;

> +

> +       return pkt;

> +}

> +

> +/**

> + * Set up an icmp packet

> + *

> + * @param pool Buffer pool to create packet in

> + * @param pkt_ref Reference ICMP packet

> + *

> + * @return Handle of created packet

> + * @retval ODP_PACKET_INVALID  Packet could not be created

> + */

> +static odp_packet_t pack_icmp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)

> +{

> +       odp_packet_t pkt;

> +       char *buf;

> +       odph_ipv4hdr_t *ip;

> +       odph_icmphdr_t *icmp;

> +       struct timeval tval;

> +       uint8_t *tval_d;

> +       unsigned short seq;

> +

> +       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN

> +

> +                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +

> +       if (pkt == ODP_PACKET_INVALID)

> +               return pkt;

> +

> +       buf = (char *)odp_packet_data(pkt);

> +       odp_memcpy(buf, odp_packet_data(pkt_ref),

> +               args->appl.payload + ODPH_ICMPHDR_LEN +

> +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

> +

> +       /* ip */

>


This one odp_packet_l3_offset_set seems not needed?

+       odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);
> +       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

> +       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;

> +       ip->id = odp_cpu_to_be_16(seq);

> +       ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);

> +

> +       /* icmp */

> +       icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN +

> ODPH_IPV4HDR_LEN);

>         icmp->un.echo.sequence = ip->id;

>         tval_d = (uint8_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN +

>                                   ODPH_ICMPHDR_LEN);

> @@ -357,6 +437,7 @@ static int gen_send_thread(void *arg)

>         thread_args_t *thr_args;

>         odp_pktout_queue_t pktout;

>         odp_packet_t pkt;

> +       odp_packet_t pkt_ref = ODP_PACKET_INVALID;

>

>         thr = odp_thread_id();

>         thr_args = arg;

> @@ -373,6 +454,21 @@ static int gen_send_thread(void *arg)

>                 return -1;

>         }

>

> +       if (args->appl.mode == APPL_MODE_UDP)

> +               pkt_ref = setup_udp_pkt_ref(thr_args->pool);

> +       else if (args->appl.mode == APPL_MODE_PING)

> +               pkt_ref = setup_icmp_pkt_ref(thr_args->pool);

> +       else {

> +               EXAMPLE_ERR("  [%02i] Error: invalid processing mode %d\n",

> +                       thr, args->appl.mode);

> +               return -1;

> +       }

> +       if (pkt_ref == ODP_PACKET_INVALID) {

> +               EXAMPLE_ERR("  [%2i] Error: reference packet creation

> failed\n",

> +                       thr);

> +               return -1;

> +       }

> +

>         printf("  [%02i] created mode: SEND\n", thr);

>

>         odp_barrier_wait(&barrier);

> @@ -386,9 +482,9 @@ static int gen_send_thread(void *arg)

>                 pkt = ODP_PACKET_INVALID;

>

>                 if (args->appl.mode == APPL_MODE_UDP)

> -                       pkt = pack_udp_pkt(thr_args->pool);

> +                       pkt = pack_udp_pkt(thr_args->pool, pkt_ref);

>


The pkts[] allocated by pack_*_pkt() were free-ed only in error condition
on line 555:

EXAMPLE_ERR("  [%02i] packet send failed\n", thr);
for (i = 0; i < burst_size; i++)
odp_packet_free(pkt_array[burst_start + i]);
break;


>                 else if (args->appl.mode == APPL_MODE_PING)

> -                       pkt = pack_icmp_pkt(thr_args->pool);

> +                       pkt = pack_icmp_pkt(thr_args->pool, pkt_ref);

>

>                 if (pkt == ODP_PACKET_INVALID) {

>                         /* Thread gives up as soon as it sees the pool

> empty.

> @@ -440,6 +536,7 @@ static int gen_send_thread(void *arg)

>                         args->appl.timeout--;

>                 }

>         }

> +       odp_packet_free(pkt_ref);

>

>         return 0;

>  }

> --

> 1.9.1

>

>
Bogdan Pricope March 2, 2017, 11:28 a.m. UTC | #8
My comments, inline.


On 03/01/2017 08:59 AM, Yi He wrote:
> Comments on 1/5 and 2/5, other patches in this series look OK to me.

>

> On 13 February 2017 at 20:49, Bogdan Pricope <bogdan.pricope@linaro.org <mailto:bogdan.pricope@linaro.org>> wrote:

>

>     Signed-off-by: Bogdan Pricope <bogdan.pricope@linaro.org <mailto:bogdan.pricope@linaro.org>>

>     ---

>      example/generator/odp_generator.c | 131 +++++++++++++++++++++++++++++++++-----

>      1 file changed, 114 insertions(+), 17 deletions(-)

>

>     diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c

>     index 8062d87..d1e3ecc 100644

>     --- a/example/generator/odp_generator.c

>     +++ b/example/generator/odp_generator.c

>     @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)

>      }

>

>      /**

>     - * set up an udp packet

>     + * set up an udp packet reference

>       *

>       * @param pool Buffer pool to create packet in

>       *

>       * @return Handle of created packet

>       * @retval ODP_PACKET_INVALID  Packet could not be created

>       */

>     -static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>     +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)

>      {

>             odp_packet_t pkt;

>             char *buf;

>             odph_ethhdr_t *eth;

>             odph_ipv4hdr_t *ip;

>             odph_udphdr_t *udp;

>     -       unsigned short seq;

>

>             pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

>                                    ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>     @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>             memcpy((char *)eth->src.addr, args->appl.srcmac.addr, ODPH_ETHADDR_LEN);

>             memcpy((char *)eth->dst.addr, args->appl.dstmac.addr, ODPH_ETHADDR_LEN);

>             eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);

>     +

>             /* ip */

>             odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

>     +       odp_packet_has_ipv4_set(pkt, 1);

>             ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>             ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>             ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

>     @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>             ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN +

>                                            ODPH_IPV4HDR_LEN);

>             ip->proto = ODPH_IPPROTO_UDP;

>     -       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

>     -       ip->id = odp_cpu_to_be_16(seq);

>     +       ip->id = 0;

>     +       ip->ttl = 64;

>             ip->chksum = 0;

>     -       odph_ipv4_csum_update(pkt);

>     +

>             /* udp */

>             odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>     +       odp_packet_has_udp_set(pkt, 1);

>             udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>             udp->src_port = 0;

>             udp->dst_port = 0;

>     @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)

>      }

>

>      /**

>     - * Set up an icmp packet

>     + * set up an udp packet

>     + *

>     + * @param pool Buffer pool to create packet in

>     + * @param pkt_ref Reference UDP packet

>     + *

>     + * @return Handle of created packet

>     + * @retval ODP_PACKET_INVALID  Packet could not be created

>     + */

>     +static odp_packet_t pack_udp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)

>     +{

>     +       odp_packet_t pkt;

>     +       char *buf;

>     +       odph_ipv4hdr_t *ip;

>     +       unsigned short seq;

>     +

>     +       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +

>     +                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>     +

>     +       if (pkt == ODP_PACKET_INVALID)

>     +               return pkt;

>     +

>     +       buf = (char *)odp_packet_data(pkt);

>     +       odp_memcpy(buf, odp_packet_data(pkt_ref),

>     +               args->appl.payload + ODPH_UDPHDR_LEN +

>     +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>     +

>     +       /*Update IP ID and checksum*/

>     +       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>     +       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;

>     +       ip->id = odp_cpu_to_be_16(seq);

>     +       ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);

>     +

>     +       return pkt;

>     +}

>     +

>     +/**

>     + * Set up an icmp packet reference

>       *

>       * @param pool Buffer pool to create packet in

>       *

>       * @return Handle of created packet

>       * @retval ODP_PACKET_INVALID  Packet could not be created

>       */

>     -static odp_packet_t pack_icmp_pkt(odp_pool_t pool)

>     +static odp_packet_t setup_icmp_pkt_ref(odp_pool_t pool)

>      {

>             odp_packet_t pkt;

>             char *buf;

>             odph_ethhdr_t *eth;

>             odph_ipv4hdr_t *ip;

>             odph_icmphdr_t *icmp;

>     -       struct timeval tval;

>     -       uint8_t *tval_d;

>     -       unsigned short seq;

>

>             args->appl.payload = 56;

>             pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN +

>     -                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>     +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>

>             if (pkt == ODP_PACKET_INVALID)

>                     return pkt;

>     @@ -265,18 +300,63 @@ static odp_packet_t pack_icmp_pkt(odp_pool_t pool)

>             ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);

>             ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);

>             ip->ver_ihl = ODPH_IPV4 << 4 | ODPH_IPV4HDR_IHL_MIN;

>     +       ip->ttl = 64;

>             ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_ICMPHDR_LEN +

>                                            ODPH_IPV4HDR_LEN);

>             ip->proto = ODPH_IPPROTO_ICMP;

>     -       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;

>     -       ip->id = odp_cpu_to_be_16(seq);

>     +       ip->id = 0;

>             ip->chksum = 0;

>     -       odph_ipv4_csum_update(pkt);

>     +

>             /* icmp */

>             icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>             icmp->type = ICMP_ECHO;

>             icmp->code = 0;

>             icmp->un.echo.id <http://un.echo.id> = 0;

>     +       icmp->un.echo.sequence = 0;

>     +       icmp->chksum = 0;

>     +

>     +       return pkt;

>     +}

>     +

>     +/**

>     + * Set up an icmp packet

>     + *

>     + * @param pool Buffer pool to create packet in

>     + * @param pkt_ref Reference ICMP packet

>     + *

>     + * @return Handle of created packet

>     + * @retval ODP_PACKET_INVALID  Packet could not be created

>     + */

>     +static odp_packet_t pack_icmp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)

>     +{

>     +       odp_packet_t pkt;

>     +       char *buf;

>     +       odph_ipv4hdr_t *ip;

>     +       odph_icmphdr_t *icmp;

>     +       struct timeval tval;

>     +       uint8_t *tval_d;

>     +       unsigned short seq;

>     +

>     +       pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN +

>     +                              ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>     +

>     +       if (pkt == ODP_PACKET_INVALID)

>     +               return pkt;

>     +

>     +       buf = (char *)odp_packet_data(pkt);

>     +       odp_memcpy(buf, odp_packet_data(pkt_ref),

>     +               args->appl.payload + ODPH_ICMPHDR_LEN +

>     +               ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);

>     +

>     +       /* ip */

>

>

> This one odp_packet_l3_offset_set seems not needed?

True.
>

>     +       odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);

>     +       ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);

>     +       seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;

>     +       ip->id = odp_cpu_to_be_16(seq);

>     +       ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);

>     +

>     +       /* icmp */

>     +       icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);

>             icmp->un.echo.sequence = ip->id;

>             tval_d = (uint8_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN +

>                                       ODPH_ICMPHDR_LEN);

>     @@ -357,6 +437,7 @@ static int gen_send_thread(void *arg)

>             thread_args_t *thr_args;

>             odp_pktout_queue_t pktout;

>             odp_packet_t pkt;

>     +       odp_packet_t pkt_ref = ODP_PACKET_INVALID;

>

>             thr = odp_thread_id();

>             thr_args = arg;

>     @@ -373,6 +454,21 @@ static int gen_send_thread(void *arg)

>                     return -1;

>             }

>

>     +       if (args->appl.mode == APPL_MODE_UDP)

>     +               pkt_ref = setup_udp_pkt_ref(thr_args->pool);

>     +       else if (args->appl.mode == APPL_MODE_PING)

>     +               pkt_ref = setup_icmp_pkt_ref(thr_args->pool);

>     +       else {

>     +               EXAMPLE_ERR("  [%02i] Error: invalid processing mode %d\n",

>     +                       thr, args->appl.mode);

>     +               return -1;

>     +       }

>     +       if (pkt_ref == ODP_PACKET_INVALID) {

>     +               EXAMPLE_ERR("  [%2i] Error: reference packet creation failed\n",

>     +                       thr);

>     +               return -1;

>     +       }

>     +

>             printf("  [%02i] created mode: SEND\n", thr);

>

>             odp_barrier_wait(&barrier);

>     @@ -386,9 +482,9 @@ static int gen_send_thread(void *arg)

>                     pkt = ODP_PACKET_INVALID;

>

>                     if (args->appl.mode == APPL_MODE_UDP)

>     -                       pkt = pack_udp_pkt(thr_args->pool);

>     +                       pkt = pack_udp_pkt(thr_args->pool, pkt_ref);

>

>

> The pkts[] allocated by pack_*_pkt() were free-ed only in error condition on line 555:

I am not get it: packets are either sent with success on the interface (and are not under our control any more) or free-ed on API failure.
Retry mechanism guards against partial sent. I don't see the problem.
>

> EXAMPLE_ERR("  [%02i] packet send failed\n", thr);

> for (i = 0; i < burst_size; i++)

> odp_packet_free(pkt_array[burst_start + i]);

> break;

>  

>

>                     else if (args->appl.mode == APPL_MODE_PING)

>     -                       pkt = pack_icmp_pkt(thr_args->pool);

>     +                       pkt = pack_icmp_pkt(thr_args->pool, pkt_ref);

>

>                     if (pkt == ODP_PACKET_INVALID) {

>                             /* Thread gives up as soon as it sees the pool empty.

>     @@ -440,6 +536,7 @@ static int gen_send_thread(void *arg)

>                             args->appl.timeout--;

>                     }

>             }

>     +       odp_packet_free(pkt_ref);

>

>             return 0;

>      }

>     --

>     1.9.1

>

>
diff mbox series

Patch

diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
index 8062d87..d1e3ecc 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -170,21 +170,20 @@  static int scan_ip(char *buf, unsigned int *paddr)
 }
 
 /**
- * set up an udp packet
+ * set up an udp packet reference
  *
  * @param pool Buffer pool to create packet in
  *
  * @return Handle of created packet
  * @retval ODP_PACKET_INVALID  Packet could not be created
  */
-static odp_packet_t pack_udp_pkt(odp_pool_t pool)
+static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)
 {
 	odp_packet_t pkt;
 	char *buf;
 	odph_ethhdr_t *eth;
 	odph_ipv4hdr_t *ip;
 	odph_udphdr_t *udp;
-	unsigned short seq;
 
 	pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +
 			       ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
@@ -200,8 +199,10 @@  static odp_packet_t pack_udp_pkt(odp_pool_t pool)
 	memcpy((char *)eth->src.addr, args->appl.srcmac.addr, ODPH_ETHADDR_LEN);
 	memcpy((char *)eth->dst.addr, args->appl.dstmac.addr, ODPH_ETHADDR_LEN);
 	eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
+
 	/* ip */
 	odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);
+	odp_packet_has_ipv4_set(pkt, 1);
 	ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);
 	ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);
 	ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);
@@ -209,12 +210,13 @@  static odp_packet_t pack_udp_pkt(odp_pool_t pool)
 	ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN +
 				       ODPH_IPV4HDR_LEN);
 	ip->proto = ODPH_IPPROTO_UDP;
-	seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;
-	ip->id = odp_cpu_to_be_16(seq);
+	ip->id = 0;
+	ip->ttl = 64;
 	ip->chksum = 0;
-	odph_ipv4_csum_update(pkt);
+
 	/* udp */
 	odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
+	odp_packet_has_udp_set(pkt, 1);
 	udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
 	udp->src_port = 0;
 	udp->dst_port = 0;
@@ -226,27 +228,60 @@  static odp_packet_t pack_udp_pkt(odp_pool_t pool)
 }
 
 /**
- * Set up an icmp packet
+ * set up an udp packet
+ *
+ * @param pool Buffer pool to create packet in
+ * @param pkt_ref Reference UDP packet
+ *
+ * @return Handle of created packet
+ * @retval ODP_PACKET_INVALID  Packet could not be created
+ */
+static odp_packet_t pack_udp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)
+{
+	odp_packet_t pkt;
+	char *buf;
+	odph_ipv4hdr_t *ip;
+	unsigned short seq;
+
+	pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +
+			       ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
+
+	if (pkt == ODP_PACKET_INVALID)
+		return pkt;
+
+	buf = (char *)odp_packet_data(pkt);
+	odp_memcpy(buf, odp_packet_data(pkt_ref),
+		args->appl.payload + ODPH_UDPHDR_LEN +
+		ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
+
+	/*Update IP ID and checksum*/
+	ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);
+	seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xFFFF;
+	ip->id = odp_cpu_to_be_16(seq);
+	ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);
+
+	return pkt;
+}
+
+/**
+ * Set up an icmp packet reference
  *
  * @param pool Buffer pool to create packet in
  *
  * @return Handle of created packet
  * @retval ODP_PACKET_INVALID  Packet could not be created
  */
-static odp_packet_t pack_icmp_pkt(odp_pool_t pool)
+static odp_packet_t setup_icmp_pkt_ref(odp_pool_t pool)
 {
 	odp_packet_t pkt;
 	char *buf;
 	odph_ethhdr_t *eth;
 	odph_ipv4hdr_t *ip;
 	odph_icmphdr_t *icmp;
-	struct timeval tval;
-	uint8_t *tval_d;
-	unsigned short seq;
 
 	args->appl.payload = 56;
 	pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN +
-			       ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
+		ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
 
 	if (pkt == ODP_PACKET_INVALID)
 		return pkt;
@@ -265,18 +300,63 @@  static odp_packet_t pack_icmp_pkt(odp_pool_t pool)
 	ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);
 	ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);
 	ip->ver_ihl = ODPH_IPV4 << 4 | ODPH_IPV4HDR_IHL_MIN;
+	ip->ttl = 64;
 	ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_ICMPHDR_LEN +
 				       ODPH_IPV4HDR_LEN);
 	ip->proto = ODPH_IPPROTO_ICMP;
-	seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;
-	ip->id = odp_cpu_to_be_16(seq);
+	ip->id = 0;
 	ip->chksum = 0;
-	odph_ipv4_csum_update(pkt);
+
 	/* icmp */
 	icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
 	icmp->type = ICMP_ECHO;
 	icmp->code = 0;
 	icmp->un.echo.id = 0;
+	icmp->un.echo.sequence = 0;
+	icmp->chksum = 0;
+
+	return pkt;
+}
+
+/**
+ * Set up an icmp packet
+ *
+ * @param pool Buffer pool to create packet in
+ * @param pkt_ref Reference ICMP packet
+ *
+ * @return Handle of created packet
+ * @retval ODP_PACKET_INVALID  Packet could not be created
+ */
+static odp_packet_t pack_icmp_pkt(odp_pool_t pool, odp_packet_t pkt_ref)
+{
+	odp_packet_t pkt;
+	char *buf;
+	odph_ipv4hdr_t *ip;
+	odph_icmphdr_t *icmp;
+	struct timeval tval;
+	uint8_t *tval_d;
+	unsigned short seq;
+
+	pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_ICMPHDR_LEN +
+			       ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
+
+	if (pkt == ODP_PACKET_INVALID)
+		return pkt;
+
+	buf = (char *)odp_packet_data(pkt);
+	odp_memcpy(buf, odp_packet_data(pkt_ref),
+		args->appl.payload + ODPH_ICMPHDR_LEN +
+		ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
+
+	/* ip */
+	odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);
+	ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);
+	seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0xffff;
+	ip->id = odp_cpu_to_be_16(seq);
+	ip->chksum = odph_chksum(ip, ODPH_IPV4HDR_LEN);
+
+	/* icmp */
+	icmp = (odph_icmphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
 	icmp->un.echo.sequence = ip->id;
 	tval_d = (uint8_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN +
 				  ODPH_ICMPHDR_LEN);
@@ -357,6 +437,7 @@  static int gen_send_thread(void *arg)
 	thread_args_t *thr_args;
 	odp_pktout_queue_t pktout;
 	odp_packet_t pkt;
+	odp_packet_t pkt_ref = ODP_PACKET_INVALID;
 
 	thr = odp_thread_id();
 	thr_args = arg;
@@ -373,6 +454,21 @@  static int gen_send_thread(void *arg)
 		return -1;
 	}
 
+	if (args->appl.mode == APPL_MODE_UDP)
+		pkt_ref = setup_udp_pkt_ref(thr_args->pool);
+	else if (args->appl.mode == APPL_MODE_PING)
+		pkt_ref = setup_icmp_pkt_ref(thr_args->pool);
+	else {
+		EXAMPLE_ERR("  [%02i] Error: invalid processing mode %d\n",
+			thr, args->appl.mode);
+		return -1;
+	}
+	if (pkt_ref == ODP_PACKET_INVALID) {
+		EXAMPLE_ERR("  [%2i] Error: reference packet creation failed\n",
+			thr);
+		return -1;
+	}
+
 	printf("  [%02i] created mode: SEND\n", thr);
 
 	odp_barrier_wait(&barrier);
@@ -386,9 +482,9 @@  static int gen_send_thread(void *arg)
 		pkt = ODP_PACKET_INVALID;
 
 		if (args->appl.mode == APPL_MODE_UDP)
-			pkt = pack_udp_pkt(thr_args->pool);
+			pkt = pack_udp_pkt(thr_args->pool, pkt_ref);
 		else if (args->appl.mode == APPL_MODE_PING)
-			pkt = pack_icmp_pkt(thr_args->pool);
+			pkt = pack_icmp_pkt(thr_args->pool, pkt_ref);
 
 		if (pkt == ODP_PACKET_INVALID) {
 			/* Thread gives up as soon as it sees the pool empty.
@@ -440,6 +536,7 @@  static int gen_send_thread(void *arg)
 			args->appl.timeout--;
 		}
 	}
+	odp_packet_free(pkt_ref);
 
 	return 0;
 }