All of lore.kernel.org
 help / color / mirror / Atom feed
* IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs
@ 2018-10-20  3:32 lidejun
  2018-10-20  5:12 ` 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: IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs
  2018-10-20  3:32 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: IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs
  2018-10-20  5:12 ` Shyam Shrivastav
@ 2018-10-20  5:22   ` Shyam Shrivastav
  2018-10-20  6:14     ` 答复: " 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

* 答复:  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       ` 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: IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs
  2018-10-20  6:14     ` 答复: " lidejun
@ 2018-10-20  6:30       ` Shyam Shrivastav
  2018-10-22  8:03         ` [dpdk-users] " 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] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs
  2018-10-20  6:30       ` 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] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs
  2018-10-22  8:03         ` [dpdk-users] " 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] 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] 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 IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs lidejun
2018-10-20  5:12 ` Shyam Shrivastav
2018-10-20  5:22   ` Shyam Shrivastav
2018-10-20  6:14     ` 答复: " lidejun
2018-10-20  6:30       ` Shyam Shrivastav
2018-10-22  8:03         ` [dpdk-users] " 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.