Dmitry Eremin-Solenikov(lumag) replied on github web page:

platform/linux-generic/odp_packet.c
line 85
@@ -2052,6 +2083,18 @@ static inline uint8_t parse_ipv4(packet_parser_t *prs, 
const uint8_t **parseptr,
        *offset   += ihl * 4;
        *parseptr += ihl * 4;
 
+       if (chksums.chksum.udp || chksums.chksum.tcp) {
+               l4_part_sum = _odp_chksum_ones_comp16_32(
+                               (const uint16_t *)&ipv4->src_addr,
+                               2 * _ODP_IPV4ADDR_LEN, false);
+#if ODP_BYTE_ORDER == ODP_BIG_ENDIAN
+               l4_part_sum += ipv4->proto;
+#else
+               l4_part_sum += ((uint16_t)ipv4->proto) << 8;
+#endif


Comment:
Yes, but the adding word is not (proto is the lat byte in pseudo header).

> Dmitry Eremin-Solenikov(lumag) wrote:
> It is an offset insite packet.


>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>> Drop last parameter (`odd_offset`)


>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>> Again `ipv6->next_hdr` is a `uint8_t` field, so is endian-neutral.


>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> `ipv4->proto` is a `uint8_t` field here, so why is this needed? These are 
>>>> the same independent of endianness.


>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>> Drop last parameter (`odd_offset`)


>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> Drop last parameter (`odd_offset`)


>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>> This erroneously increments `data` by 2 when you want to increment it 
>>>>>>> by 1 here so that it points to `p + 1`, not `p + 2`. Correct is `data = 
>>>>>>> (const uint16_t *)((uintptr_t)p + 1);`


>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>> The `odd_offset` parameter is unnecessary here. What matters is 
>>>>>>>> whether `p` starts on an even or odd byte boundary since that 
>>>>>>>> determines whether you can fetch `data` as `uint16_t` quantities 
>>>>>>>> without making unaligned storage references. Each call to 
>>>>>>>> `_odp_chksum_ones_comp16_32()` is independent in this regard, so it 
>>>>>>>> must always be determined anew for each call.


>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> `if (len > 1 && (uintptr_t)p % 2) ...`


>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>> Drop use of `odd_offset` as noted earlier.


>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>> Checkpatch flag:
>>>>>>>>>>> ```
>>>>>>>>>>> WARNING: line over 80 characters
>>>>>>>>>>> #102: FILE: platform/linux-generic/odp_packet.c:2204:
>>>>>>>>>>> +           ip_proto = parse_ipv4(prs, &parseptr, &offset, 
>>>>>>>>>>> frame_len, chksums);
>>>>>>>>>>> total: 0 errors, 1 warnings, 0 checks, 163 lines checked
>>>>>>>>>>> ```


>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>> 2018


https://github.com/Linaro/odp/pull/389#discussion_r161940706
updated_at 2018-01-17 02:11:01

Reply via email to