* [dpdk-users] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs @ 2018-10-20 3:32 lidejun 2018-10-20 5:12 ` [dpdk-users] [dpdk-dev] " Shyam Shrivastav 0 siblings, 1 reply; 9+ messages in thread From: lidejun @ 2018-10-20 3:32 UTC (permalink / raw) To: users, dev; +Cc: Lichunhe (Cloud Networking), Wangliefeng Has anybody used the following two APIs calculating ipv4&ipv6 tcp/udp pseudo header checksum? 1. rte_ipv4_phdr_cksum 2. rte_ipv6_phdr_cksum The ipv4 version does not exclude ip options and ipv6 version does not exclude extersion headers. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-users] [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs 2018-10-20 3:32 [dpdk-users] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs lidejun @ 2018-10-20 5:12 ` Shyam Shrivastav 2018-10-20 5:22 ` Shyam Shrivastav 0 siblings, 1 reply; 9+ messages in thread From: Shyam Shrivastav @ 2018-10-20 5:12 UTC (permalink / raw) To: lidejun1; +Cc: users, dev, lichunhe, wangliefeng that is correct , pseudo header doesn't include ipv4 options or ipv6 extension headers .. On Sat, Oct 20, 2018 at 9:02 AM lidejun <lidejun1@huawei.com> wrote: > Has anybody used the following two APIs calculating ipv4&ipv6 tcp/udp > pseudo header checksum? > > 1. rte_ipv4_phdr_cksum > > 2. rte_ipv6_phdr_cksum > The ipv4 version does not exclude ip options and ipv6 version does not > exclude extersion headers. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-users] [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs 2018-10-20 5:12 ` [dpdk-users] [dpdk-dev] " Shyam Shrivastav @ 2018-10-20 5:22 ` Shyam Shrivastav 2018-10-20 6:14 ` [dpdk-users] 答复: " lidejun 0 siblings, 1 reply; 9+ messages in thread From: Shyam Shrivastav @ 2018-10-20 5:22 UTC (permalink / raw) To: lidejun1; +Cc: users, dev, lichunhe, wangliefeng Realized my answer is confusing, I meant to say that code is correct as pseudo ipv4/ipv6 headers for the purpose of checksum calculations doesn't include options or extension headers, see udp wiki or corresponding rfcs https://en.wikipedia.org/wiki/User_Datagram_Protocol On Sat, Oct 20, 2018 at 10:42 AM Shyam Shrivastav < shrivastav.shyam@gmail.com> wrote: > that is correct , pseudo header doesn't include ipv4 options or ipv6 > extension headers .. > > On Sat, Oct 20, 2018 at 9:02 AM lidejun <lidejun1@huawei.com> wrote: > >> Has anybody used the following two APIs calculating ipv4&ipv6 tcp/udp >> pseudo header checksum? >> >> 1. rte_ipv4_phdr_cksum >> >> 2. rte_ipv6_phdr_cksum >> The ipv4 version does not exclude ip options and ipv6 version does not >> exclude extersion headers. >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-users] 答复: [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs 2018-10-20 5:22 ` Shyam Shrivastav @ 2018-10-20 6:14 ` lidejun 2018-10-20 6:30 ` [dpdk-users] " Shyam Shrivastav 0 siblings, 1 reply; 9+ messages in thread From: lidejun @ 2018-10-20 6:14 UTC (permalink / raw) To: Shyam Shrivastav; +Cc: users, dev, Lichunhe (Cloud Networking), zhangxufeng (C) I mean DPDK APIs do not exclude ipv4 options or ipv6 extension headers, it is bug? 发件人: Shyam Shrivastav [mailto:shrivastav.shyam@gmail.com] 发送时间: 2018年10月20日 13:23 收件人: lidejun <lidejun1@huawei.com> 抄送: users <users@dpdk.org>; dev@dpdk.org; Lichunhe (Cloud Networking) <lichunhe@huawei.com>; Wangliefeng <wangliefeng@huawei.com> 主题: Re: [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs Realized my answer is confusing, I meant to say that code is correct as pseudo ipv4/ipv6 headers for the purpose of checksum calculations doesn't include options or extension headers, see udp wiki or corresponding rfcs https://en.wikipedia.org/wiki/User_Datagram_Protocol On Sat, Oct 20, 2018 at 10:42 AM Shyam Shrivastav <shrivastav.shyam@gmail.com<mailto:shrivastav.shyam@gmail.com>> wrote: that is correct , pseudo header doesn't include ipv4 options or ipv6 extension headers .. On Sat, Oct 20, 2018 at 9:02 AM lidejun <lidejun1@huawei.com<mailto:lidejun1@huawei.com>> wrote: Has anybody used the following two APIs calculating ipv4&ipv6 tcp/udp pseudo header checksum? 1. rte_ipv4_phdr_cksum 2. rte_ipv6_phdr_cksum The ipv4 version does not exclude ip options and ipv6 version does not exclude extersion headers. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-users] [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs 2018-10-20 6:14 ` [dpdk-users] 答复: " lidejun @ 2018-10-20 6:30 ` Shyam Shrivastav 2018-10-22 8:03 ` Ferruh Yigit 0 siblings, 1 reply; 9+ messages in thread From: Shyam Shrivastav @ 2018-10-20 6:30 UTC (permalink / raw) To: lidejun1; +Cc: users, dev, lichunhe, zhangxufeng4 Yes you are right, I misread, following code (ipv4 case) assumes no ip options while calculating pseudo hdr length field psd_hdr.len = rte_cpu_to_be_16( (uint16_t)(rte_be_to_cpu_16(ipv4_hdr->total_length) - sizeof(struct ipv4_hdr))); should be psd_hdr.len = rte_cpu_to_be_16( (uint16_t)(rte_be_to_cpu_16(ipv4_hdr->total_length) - (ipv4_hdr->version_ihl & 0x0f)*4))); On Sat, Oct 20, 2018 at 11:44 AM lidejun <lidejun1@huawei.com> wrote: > I mean DPDK APIs do not exclude ipv4 options or ipv6 extension headers, it > is bug? > > > > *发件人:* Shyam Shrivastav [mailto:shrivastav.shyam@gmail.com] > *发送时间:* 2018年10月20日 13:23 > *收件人:* lidejun <lidejun1@huawei.com> > *抄送:* users <users@dpdk.org>; dev@dpdk.org; Lichunhe (Cloud Networking) < > lichunhe@huawei.com>; Wangliefeng <wangliefeng@huawei.com> > *主题:* Re: [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs > > > > Realized my answer is confusing, I meant to say that code is correct as > pseudo ipv4/ipv6 headers for the purpose of checksum calculations doesn't > include options or extension headers, see udp wiki or corresponding rfcs > > > > https://en.wikipedia.org/wiki/User_Datagram_Protocol > > > > On Sat, Oct 20, 2018 at 10:42 AM Shyam Shrivastav < > shrivastav.shyam@gmail.com> wrote: > > that is correct , pseudo header doesn't include ipv4 options or ipv6 > extension headers .. > > > > On Sat, Oct 20, 2018 at 9:02 AM lidejun <lidejun1@huawei.com> wrote: > > Has anybody used the following two APIs calculating ipv4&ipv6 tcp/udp > pseudo header checksum? > > 1. rte_ipv4_phdr_cksum > > 2. rte_ipv6_phdr_cksum > The ipv4 version does not exclude ip options and ipv6 version does not > exclude extersion headers. > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-users] [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs 2018-10-20 6:30 ` [dpdk-users] " Shyam Shrivastav @ 2018-10-22 8:03 ` Ferruh Yigit 2018-10-23 9:01 ` Olivier Matz 0 siblings, 1 reply; 9+ messages in thread From: Ferruh Yigit @ 2018-10-22 8:03 UTC (permalink / raw) To: Shyam Shrivastav, lidejun1 Cc: users, dev, lichunhe, zhangxufeng4, Olivier MATZ On 10/20/2018 7:30 AM, Shyam Shrivastav wrote: > Yes you are right, I misread, following code (ipv4 case) assumes no ip > options while calculating pseudo hdr length field > > psd_hdr.len = rte_cpu_to_be_16( > (uint16_t)(rte_be_to_cpu_16(ipv4_hdr->total_length) > - sizeof(struct ipv4_hdr))); > > should be > > psd_hdr.len = rte_cpu_to_be_16( > (uint16_t)(rte_be_to_cpu_16(ipv4_hdr->total_length) > - (ipv4_hdr->version_ihl & 0x0f)*4))); cc'ed maintainer of the that part, Olivier. > > On Sat, Oct 20, 2018 at 11:44 AM lidejun <lidejun1@huawei.com> wrote: > >> I mean DPDK APIs do not exclude ipv4 options or ipv6 extension headers, it >> is bug? >> >> >> >> *发件人:* Shyam Shrivastav [mailto:shrivastav.shyam@gmail.com] >> *发送时间:* 2018年10月20日 13:23 >> *收件人:* lidejun <lidejun1@huawei.com> >> *抄送:* users <users@dpdk.org>; dev@dpdk.org; Lichunhe (Cloud Networking) < >> lichunhe@huawei.com>; Wangliefeng <wangliefeng@huawei.com> >> *主题:* Re: [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs >> >> >> >> Realized my answer is confusing, I meant to say that code is correct as >> pseudo ipv4/ipv6 headers for the purpose of checksum calculations doesn't >> include options or extension headers, see udp wiki or corresponding rfcs >> >> >> >> https://en.wikipedia.org/wiki/User_Datagram_Protocol >> >> >> >> On Sat, Oct 20, 2018 at 10:42 AM Shyam Shrivastav < >> shrivastav.shyam@gmail.com> wrote: >> >> that is correct , pseudo header doesn't include ipv4 options or ipv6 >> extension headers .. >> >> >> >> On Sat, Oct 20, 2018 at 9:02 AM lidejun <lidejun1@huawei.com> wrote: >> >> Has anybody used the following two APIs calculating ipv4&ipv6 tcp/udp >> pseudo header checksum? >> >> 1. rte_ipv4_phdr_cksum >> >> 2. rte_ipv6_phdr_cksum >> The ipv4 version does not exclude ip options and ipv6 version does not >> exclude extersion headers. >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-users] [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs 2018-10-22 8:03 ` Ferruh Yigit @ 2018-10-23 9:01 ` Olivier Matz 2018-10-23 10:53 ` Shyam Shrivastav 2018-11-28 15:07 ` Hyong Youb Kim 0 siblings, 2 replies; 9+ messages in thread From: Olivier Matz @ 2018-10-23 9:01 UTC (permalink / raw) To: Ferruh Yigit Cc: Shyam Shrivastav, lidejun1, users, dev, lichunhe, zhangxufeng4 Hi, You are right, the current code does not take IP or IPv6 options in account. I think this should be considered as a bug. The fix for IPv4 is not complicated, I did a quick draft here: http://git.droids-corp.org/?p=dpdk.git;a=commitdiff;h=96a6978ef6814e1450e1bd65fbce91c3d85b3121 For IPv6, it is more complex than expected: https://tools.ietf.org/html/rfc2460.html#section-8.1 - we need to skip extension headers - we need to parse routing headers and use the proper destination address in the pseudo header checksum This makes me think that the API is not adequate. Asking the user to provide the headers in a contiguous memory without specifying the length is quite dangerous, especially if the header comes from outside, as it can trigger out of bound accesses. I wonder if we shouldn't switch to a mbuf based API instead, and deprecate the old one. Thoughts? Olivier On Mon, Oct 22, 2018 at 09:03:14AM +0100, Ferruh Yigit wrote: > On 10/20/2018 7:30 AM, Shyam Shrivastav wrote: > > Yes you are right, I misread, following code (ipv4 case) assumes no ip > > options while calculating pseudo hdr length field > > > > psd_hdr.len = rte_cpu_to_be_16( > > (uint16_t)(rte_be_to_cpu_16(ipv4_hdr->total_length) > > - sizeof(struct ipv4_hdr))); > > > > should be > > > > psd_hdr.len = rte_cpu_to_be_16( > > (uint16_t)(rte_be_to_cpu_16(ipv4_hdr->total_length) > > - (ipv4_hdr->version_ihl & 0x0f)*4))); > > cc'ed maintainer of the that part, Olivier. > > > > > On Sat, Oct 20, 2018 at 11:44 AM lidejun <lidejun1@huawei.com> wrote: > > > >> I mean DPDK APIs do not exclude ipv4 options or ipv6 extension headers, it > >> is bug? > >> > >> > >> > >> *发件人:* Shyam Shrivastav [mailto:shrivastav.shyam@gmail.com] > >> *发送时间:* 2018年10月20日 13:23 > >> *收件人:* lidejun <lidejun1@huawei.com> > >> *抄送:* users <users@dpdk.org>; dev@dpdk.org; Lichunhe (Cloud Networking) < > >> lichunhe@huawei.com>; Wangliefeng <wangliefeng@huawei.com> > >> *主题:* Re: [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs > >> > >> > >> > >> Realized my answer is confusing, I meant to say that code is correct as > >> pseudo ipv4/ipv6 headers for the purpose of checksum calculations doesn't > >> include options or extension headers, see udp wiki or corresponding rfcs > >> > >> > >> > >> https://en.wikipedia.org/wiki/User_Datagram_Protocol > >> > >> > >> > >> On Sat, Oct 20, 2018 at 10:42 AM Shyam Shrivastav < > >> shrivastav.shyam@gmail.com> wrote: > >> > >> that is correct , pseudo header doesn't include ipv4 options or ipv6 > >> extension headers .. > >> > >> > >> > >> On Sat, Oct 20, 2018 at 9:02 AM lidejun <lidejun1@huawei.com> wrote: > >> > >> Has anybody used the following two APIs calculating ipv4&ipv6 tcp/udp > >> pseudo header checksum? > >> > >> 1. rte_ipv4_phdr_cksum > >> > >> 2. rte_ipv6_phdr_cksum > >> The ipv4 version does not exclude ip options and ipv6 version does not > >> exclude extersion headers. > >> > >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-users] [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs 2018-10-23 9:01 ` Olivier Matz @ 2018-10-23 10:53 ` Shyam Shrivastav 2018-11-28 15:07 ` Hyong Youb Kim 1 sibling, 0 replies; 9+ messages in thread From: Shyam Shrivastav @ 2018-10-23 10:53 UTC (permalink / raw) To: olivier.matz; +Cc: Ferruh Yigit, lidejun1, users, dev, lichunhe, zhangxufeng4 These fixes/modifications should include the upper level APIs, rte_ipv4_udptcp_cksum and rte_ipv6_udptcp_cksum. Even for ipv4 following API is more/really useful if changed to take mbufs rte_ipv4_udptcp_cksum(const struct ipv4_hdr *ipv4_hdr, const void *l4_hdr) I can not use it in present form as it requires single contiguous l4 buffer, instead using rte_raw_cksum_mbuf. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-users] [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs 2018-10-23 9:01 ` Olivier Matz 2018-10-23 10:53 ` Shyam Shrivastav @ 2018-11-28 15:07 ` Hyong Youb Kim 1 sibling, 0 replies; 9+ messages in thread From: Hyong Youb Kim @ 2018-11-28 15:07 UTC (permalink / raw) To: Olivier Matz Cc: Ferruh Yigit, Shyam Shrivastav, lidejun1, users, dev, lichunhe, zhangxufeng4 On Tue, Oct 23, 2018 at 11:01:58AM +0200, Olivier Matz wrote: > Hi, > > You are right, the current code does not take IP or IPv6 options > in account. I think this should be considered as a bug. > > The fix for IPv4 is not complicated, I did a quick draft here: > http://git.droids-corp.org/?p=dpdk.git;a=commitdiff;h=96a6978ef6814e1450e1bd65fbce91c3d85b3121 > > For IPv6, it is more complex than expected: > https://tools.ietf.org/html/rfc2460.html#section-8.1 > > - we need to skip extension headers > - we need to parse routing headers and use the proper destination > address in the pseudo header checksum > > This makes me think that the API is not adequate. Asking the user > to provide the headers in a contiguous memory without specifying > the length is quite dangerous, especially if the header comes from > outside, as it can trigger out of bound accesses. > > I wonder if we shouldn't switch to a mbuf based API instead, and > deprecate the old one. > > Thoughts? > Olivier > I have been looking into a similar issue because rte_net_intel_cksum_prepare(), which is used by most tx_pkt_prepare handlers, does not work when ipv6 extensions are present. That function is using rte_ipv6_phdr_cksum(). And this makes rte_eth_tx_prepare() kinda useless for any workloads that encounter ipv6 extensions. There are 2 routing header types now (2 and 3). https://www.iana.org/assignments/ipv6-parameters/ipv6-parameters.xhtml#ipv6-parameters-3 In addition to these routing headers, there is also ipv6 mobility. Pseudo header's source address is supposed to be the address in the Home Address option. https://tools.ietf.org/html/rfc6275#page-36 Who knows there may be future extensions that affect pseudo header.. We can probably make rte_ipv6_phdr_cksum() to understand all existing headers that affect pseudo header, but it will still not be future proof. Should at least document the limitations for rte_ipv6_phdr_cksum().. -Hyong ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-11-28 15:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-20 3:32 [dpdk-users] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs lidejun 2018-10-20 5:12 ` [dpdk-users] [dpdk-dev] " Shyam Shrivastav 2018-10-20 5:22 ` Shyam Shrivastav 2018-10-20 6:14 ` [dpdk-users] 答复: " lidejun 2018-10-20 6:30 ` [dpdk-users] " Shyam Shrivastav 2018-10-22 8:03 ` Ferruh Yigit 2018-10-23 9:01 ` Olivier Matz 2018-10-23 10:53 ` Shyam Shrivastav 2018-11-28 15:07 ` Hyong Youb Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).