Sounds fine to me. Are you planning to add this, or you want me to do it?
On 20/07/2017, 12:09 PM, "Olivier Matz" <olivier.m...@6wind.com> wrote: >Hi Ido, > >On Tue, 18 Jul 2017 12:18:20 +0000, "Ido Barnea (ibarnea)" <ibar...@cisco.com> >wrote: >> Hi, >> Is it intentional that rte_ipv4_phdr_cksum and others in this file assume >> that ipv4 header is sizeof(struct ipv4_hdr), actually assuming that there >> are no IPv4 options? >> If not, I would like to submit a patch changing that. >> Something in the line of: >> sizeof(struct ipv4_hdr) changes to ipv4_hdr->version_ihl & >> IPV4_HDR_IHL_MASK) * IPV4_IHL_MULTIPLIER) >> > >I agree we should have other functions that take the real ip header >len in account. What about the following: > >static inline uint16_t >rte_ipv4ext_cksum(const struct ipv4_hdr *ipv4_hdr, size_t hdrlen) > >static inline uint16_t >rte_ipv4ext_phdr_cksum(const struct ipv4_hdr *ipv4_hdr, size_t hdrlen, > uint64_t ol_flags) >... > >The current function rte_ipv4_cksum() can call rte_ipv4ext_cksum(.., 5) >without additional cost, since 5 will be a constant, resolved at >compilation time. > >I also noticed that some functions are not properly documented (it should >be announced that options are not supported). Also, the api comment >of rte_ipv6_udptcp_cksum() talks about ipv4... > > >Thanks, >Olivier