Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:

platform/linux-generic/odp_packet.c
line 160
@@ -2155,14 +2212,35 @@ static inline void parse_tcp(packet_parser_t *prs,
 /**
  * Parser helper function for UDP
  */
-static inline void parse_udp(packet_parser_t *prs,
-                            const uint8_t **parseptr, uint32_t *offset)
+static inline void parse_udp(packet_parser_t *prs, const uint8_t **parseptr,
+                            uint32_t *offset, odp_proto_chksums_t chksums)
 {
        const _odp_udphdr_t *udp = (const _odp_udphdr_t *)*parseptr;
        uint32_t udplen = odp_be_to_cpu_16(udp->length);
 
-       if (odp_unlikely(udplen < sizeof(_odp_udphdr_t)))
+       if (odp_unlikely(udplen < sizeof(_odp_udphdr_t))) {
                prs->error_flags.udp_err = 1;
+               return;
+       }
+
+       if (chksums.chksum.udp &&
+           !prs->input_flags.ipfrag) {
+               if (udp->chksum == 0) {
+                       prs->input_flags.l4_chksum_done =
+                               (prs->input_flags.ipv4 != 1);
+                       prs->error_flags.l4_chksum =
+                               (prs->input_flags.ipv4 != 1);
+               } else {
+                       prs->input_flags.l4_chksum_done = 1;
+                       prs->l4_part_sum += udp->length;
+                       /* Do not include checksum into partial sum */
+                       prs->l4_part_sum += _odp_chksum_ones_comp16_32(
+                                       (const void *)udp,
+                                       _ODP_UDPHDR_LEN - 2,
+                                       false);


Comment:
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_r161937032
updated_at 2018-01-17 01:41:46

Reply via email to