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.hol...@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.pric...@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
>> <nmo...@kalray.eu> wrote:
>> >
>> >
>> > Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
>> >> Signed-off-by: Bogdan Pricope <bogdan.pric...@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"
>
>

Reply via email to