subject:"Re\: \[lng\-odp\] \[PATCH 1\/5\] test\: generator\: compose sending packets from reference packet plus differences"

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

2017-03-02 Thread Bogdan Pricope

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  > wrote:
>
> Signed-off-by: Bogdan Pricope  >
> ---
>  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(, 1) % 0x;
> -   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(, 1) % 0x;
> +   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;
>

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

2017-02-28 Thread Yi He

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 
wrote:

> Signed-off-by: Bogdan Pricope 
> ---
>  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(, 1) % 0x;
> -   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(, 1) % 0x;
> +   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 =

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

2017-02-15 Thread Bogdan Pricope

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  wrote:
> So this is a request to add this to Monarch LTS then ?
>
> On 15 February 2017 at 03:58, Bogdan Pricope 
> 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
>>  wrote:
>> >
>> >
>> > Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
>> >> Signed-off-by: Bogdan Pricope 
>> >> ---
>> >>  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(, 1) % 0x;
>> >> - 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"
>
>

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

2017-02-15 Thread Bogdan Pricope

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
 wrote:
>
>
> Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
>> Signed-off-by: Bogdan Pricope 
>> ---
>>  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(, 1) % 0x;
>> - 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

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

2017-02-14 Thread Nicolas Morey-Chaisemartin



Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
> Signed-off-by: Bogdan Pricope 
> ---
>  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(, 1) % 0x;
> - 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

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

2017-02-14 Thread Bogdan Pricope

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  wrote:
> On Mon, Feb 13, 2017 at 6:49 AM, Bogdan Pricope
>  wrote:
>> Signed-off-by: Bogdan Pricope 
>> ---
>>  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(, 1) % 0x;
>> -   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(, 1) % 0x;

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

2017-02-13 Thread Bill Fischofer

On Mon, Feb 13, 2017 at 6:49 AM, Bogdan Pricope
 wrote:
> Signed-off-by: Bogdan Pricope 
> ---
>  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(, 1) % 0x;
> -   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(, 1) % 0x;
> +   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

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

7 matches

Site Navigation

Mail list logo

Footer information