From: Vlad Yasevich <vladislav.yasevich@hp.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org, davem@davemloft.net
Subject: Re: [PATCH 1/4] sctp: Fix crc32c calculations on big-endian arhes.
Date: Fri, 23 Jan 2009 14:52:44 +0000 [thread overview]
Message-ID: <4979D9BC.5000609@hp.com> (raw)
In-Reply-To: <20090123092449.GA27399@gondor.apana.org.au>
Herbert Xu wrote:
> On Fri, Jan 23, 2009 at 04:28:49PM +1100, Herbert Xu wrote:
>>>> static inline __be32 sctp_end_cksum(__be32 crc32)
>>>> {
>>>> - return ~crc32;
>>>> + return (__force __be32)~cpu_to_le32((__force u32)crc32);
>>>> }
>>> Ouch, surely there is a better way to do this?
>> In fact this looks wrong. Has this code actually been tested
>> on big-endian?
Yes, the code was tested by me and the person who reported it
as listed in the commit log. :)
>
> Reading this again it does seem to do the right thing as it's
> using the raw crc32c interface as opposed to crypto crc32c.
>
> However, I suggest that we change it as follows:
>
> 1) Make sh->csum __le32 since we're using crc32c_le.
> 2) Change all intermediate values in sctp/checksum.h to u32.
> 3) Make sctp_end_cksum return __le32 and have it do
>
> return cpu_to_le32(~crc);
>
I'll give it some thought. It would clean up all the __force
casts, but it will be a little misleading to define a packet
checksum as little endian. :)
-vlad
> Cheers,
WARNING: multiple messages have this Message-ID (diff)
From: Vlad Yasevich <vladislav.yasevich@hp.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org, davem@davemloft.net
Subject: Re: [PATCH 1/4] sctp: Fix crc32c calculations on big-endian arhes.
Date: Fri, 23 Jan 2009 09:52:44 -0500 [thread overview]
Message-ID: <4979D9BC.5000609@hp.com> (raw)
In-Reply-To: <20090123092449.GA27399@gondor.apana.org.au>
Herbert Xu wrote:
> On Fri, Jan 23, 2009 at 04:28:49PM +1100, Herbert Xu wrote:
>>>> static inline __be32 sctp_end_cksum(__be32 crc32)
>>>> {
>>>> - return ~crc32;
>>>> + return (__force __be32)~cpu_to_le32((__force u32)crc32);
>>>> }
>>> Ouch, surely there is a better way to do this?
>> In fact this looks wrong. Has this code actually been tested
>> on big-endian?
Yes, the code was tested by me and the person who reported it
as listed in the commit log. :)
>
> Reading this again it does seem to do the right thing as it's
> using the raw crc32c interface as opposed to crypto crc32c.
>
> However, I suggest that we change it as follows:
>
> 1) Make sh->csum __le32 since we're using crc32c_le.
> 2) Change all intermediate values in sctp/checksum.h to u32.
> 3) Make sctp_end_cksum return __le32 and have it do
>
> return cpu_to_le32(~crc);
>
I'll give it some thought. It would clean up all the __force
casts, but it will be a little misleading to define a packet
checksum as little endian. :)
-vlad
> Cheers,
next prev parent reply other threads:[~2009-01-23 14:52 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-22 22:36 [PATCH] sctp: regression bug fixes Vlad Yasevich
2009-01-22 22:36 ` Vlad Yasevich
2009-01-22 22:36 ` [PATCH 1/4] sctp: Fix crc32c calculations on big-endian arhes Vlad Yasevich
2009-01-22 22:36 ` Vlad Yasevich
2009-01-23 5:19 ` Herbert Xu
2009-01-23 5:19 ` Herbert Xu
2009-01-23 5:28 ` Herbert Xu
2009-01-23 5:28 ` Herbert Xu
2009-01-23 9:24 ` Herbert Xu
2009-01-23 9:24 ` Herbert Xu
2009-01-23 14:52 ` Vlad Yasevich [this message]
2009-01-23 14:52 ` Vlad Yasevich
2009-01-23 22:17 ` Herbert Xu
2009-01-23 22:17 ` Herbert Xu
2009-01-22 22:36 ` [PATCH 2/4] sctp: Correctly start rtx timer on new packet transmissions Vlad Yasevich
2009-01-22 22:36 ` Vlad Yasevich
2009-01-22 22:36 ` [PATCH 3/4] sctp: Properly timestamp outgoing data chunks for rtx purposes Vlad Yasevich
2009-01-22 22:36 ` Vlad Yasevich
2009-01-22 22:36 ` [PATCH 4/4] sctp: Fix another socket race during accept/peeloff Vlad Yasevich
2009-01-22 22:36 ` Vlad Yasevich
2009-01-22 22:53 ` [PATCH] sctp: regression bug fixes David Miller
2009-01-22 22:53 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4979D9BC.5000609@hp.com \
--to=vladislav.yasevich@hp.com \
--cc=davem@davemloft.net \
--cc=herbert@gondor.apana.org.au \
--cc=linux-sctp@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.