Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/include/odp_chksum_internal.h
line 24
@@ -0,0 +1,59 @@
+/* Copyright (c) 2013, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP checksum - implementation internal
+ */
+
+#ifndef ODP_CHKSUM_INTERNAL_H_
+#define ODP_CHKSUM_INTERNAL_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Simple implementation of ones complement sum.
+ * Based on RFC1071 and its errata.
+ */
+static uint32_t _odp_chksum_ones_comp16_32(const void *p, uint32_t len,
+ odp_bool_t odd_offset)
Comment:
The calculation starts from the address passed in as `p`. Whether that address
is odd or even doesn't need a separate parameter to determine. The various ODP
packet routines that return packet addresses, with the exception of
`odp_packet_align()`, make no guarantees as to the alignment of any address
they return.
> Dmitry Eremin-Solenikov(lumag) wrote:
> This is true. I'll fix this issue.
>> Dmitry Eremin-Solenikov(lumag) wrote:
>> Yes. However depending on the endianness of the host, it should be converted
>> to a different host u16 value. It is a pair `{0, proto}`. On BE it becomes
>> just `proto` u16, on LE it is equal to `proto << 8`.
>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>> sorry. odd_offset references to an offset inside the packet. If you start
>>> calculating checksum from odd offset, you have to insert additional 0.
>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> The protocol field in the pseudo-header is simply the 8-bit IP protocol
>>>> field considered as a 16-bit value. It's also in network byte order so
>>>> these two have the same bit value.
>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>> I don't understand that response. Can you elaborate?
>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>> 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_r162243873
updated_at 2018-01-18 03:50:17
