From: Tom Herbert <tom@herbertland.com>
To: Alexander Duyck <alexander.duyck@gmail.com>
Cc: Davide Caratti <dcaratti@redhat.com>,
David Laight <David.Laight@aculab.com>,
"David S . Miller" <davem@davemloft.net>,
Linux Kernel Network Developers <netdev@vger.kernel.org>,
"linux-sctp @ vger . kernel . org" <linux-sctp@vger.kernel.org>,
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Subject: Re: [PATCH RFC net-next v2 1/4] skbuff: add stub to help computing crc32c on SCTP packets
Date: Tue, 28 Feb 2017 19:17:55 -0800 [thread overview]
Message-ID: <CALx6S36oXDyrm=4Bi-EdKgCBYYHuHENCoFz8GMBm6RmGCCep6g@mail.gmail.com> (raw)
In-Reply-To: <CAKgT0Udbs_=vanS+oF8ePXTNjEfsq9y==bhtQ-5Cxb-g+iO4bg@mail.gmail.com>
On Tue, Feb 28, 2017 at 2:46 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Tue, Feb 28, 2017 at 2:32 AM, Davide Caratti <dcaratti@redhat.com> wrote:
>> sctp_compute_checksum requires crc32c symbol (provided by libcrc32c), so
>> it can't be used in net core. Like it has been done previously with other
>> symbols (e.g. ipv6_dst_lookup), introduce a stub struct skb_checksum_ops
>> to allow computation of SCTP checksum in net core after sctp.ko (and thus
>> libcrc32c) has been loaded.
>
> At a minimum the name really needs to change. SCTP does not do
> checksums. It does a CRC, and a CRC is a very different thing. The
> fact that somebody decided that offloading a CRC could use the same
> framework is very unfortunate, and your patch descriptions in this
> whole set are calling out a CRC as checksums which it is not.
>
> I don't want to see anything "checksum" or "csum" related in the
> naming when it comes to dealing with SCTP unless we absolutely have to
> have it. So any function names or structures with sctp in the name
> should call out "crc32" or "crc", please don't use checksum.
>
Alexander,
I agree that internal functions to sctp should not refer to checksum,
but I think we need to take care to be consistent with any external
API (even if somebody made a mistake defining it this way :-) ). As
you know the checksum interface must be very precisely defined, there
is no leeway for ambiguity. Many places in the stack use csum and
CHECKSUM_* to refer to the API not the actual algorithm, others don't
(e.g. CHECKSUM_UNNECESSARY can apply to SCTP checksum,
CHECKSUM_COMPLETE must be an Internet checksum).
For instance, in that light skb_sctp_csum_help is appropriately named
I think because this is being called from skb_csum_help and refers to
the interface to resolve a checksum.
Tom
>> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
>> ---
>> include/linux/skbuff.h | 2 ++
>> net/core/skbuff.c | 20 ++++++++++++++++++++
>> net/sctp/offload.c | 7 +++++++
>> 3 files changed, 29 insertions(+)
>>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 69ccd26..cab9a32 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -3125,6 +3125,8 @@ struct skb_checksum_ops {
>> __wsum (*combine)(__wsum csum, __wsum csum2, int offset, int len);
>> };
>>
>> +extern const struct skb_checksum_ops *sctp_csum_stub __read_mostly;
>> +
>> __wsum __skb_checksum(const struct sk_buff *skb, int offset, int len,
>> __wsum csum, const struct skb_checksum_ops *ops);
>> __wsum skb_checksum(const struct sk_buff *skb, int offset, int len,
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index f355795..64fd8fd 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -2242,6 +2242,26 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
>> }
>> EXPORT_SYMBOL(skb_copy_and_csum_bits);
>>
>> +static __wsum warn_sctp_csum_update(const void *buff, int len, __wsum sum)
>> +{
>> + net_warn_ratelimited("attempt to compute crc32c without sctp.ko\n");
>> + return 0;
>> +}
>> +
>> +static __wsum warn_sctp_csum_combine(__wsum csum, __wsum csum2,
>> + int offset, int len)
>> +{
>> + net_warn_ratelimited("attempt to compute crc32c without sctp.ko\n");
>> + return 0;
>> +}
>> +
>> +const struct skb_checksum_ops *sctp_csum_stub __read_mostly =
>> + &(struct skb_checksum_ops) {
>> + .update = warn_sctp_csum_update,
>> + .combine = warn_sctp_csum_combine,
>> +};
>> +EXPORT_SYMBOL(sctp_csum_stub);
>> +
>> /**
>> * skb_zerocopy_headlen - Calculate headroom needed for skb_zerocopy()
>> * @from: source buffer
>> diff --git a/net/sctp/offload.c b/net/sctp/offload.c
>> index 4f5a2b5..e9c3db0 100644
>> --- a/net/sctp/offload.c
>> +++ b/net/sctp/offload.c
>> @@ -98,6 +98,12 @@ static const struct net_offload sctp6_offload = {
>> },
>> };
>>
>> +static const struct skb_checksum_ops *sctp_csum_ops __read_mostly =
>> + &(struct skb_checksum_ops) {
>> + .update = sctp_csum_update,
>> + .combine = sctp_csum_combine,
>> +};
>> +
>> int __init sctp_offload_init(void)
>> {
>> int ret;
>> @@ -110,6 +116,7 @@ int __init sctp_offload_init(void)
>> if (ret)
>> goto ipv4;
>>
>> + sctp_csum_stub = sctp_csum_ops;
>> return ret;
>>
>> ipv4:
>> --
>> 2.7.4
>>
next prev parent reply other threads:[~2017-03-01 3:17 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-23 16:52 [RFC PATCH net-next 0/5] net: improve support for SCTP checksums Davide Caratti
2017-01-23 16:52 ` [RFC PATCH net-next 1/5] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti
2017-01-23 16:52 ` [RFC PATCH net-next 2/5] net: split skb_checksum_help Davide Caratti
2017-01-23 20:59 ` Tom Herbert
2017-01-24 16:35 ` David Laight
2017-02-02 15:07 ` Davide Caratti
2017-02-02 16:55 ` David Laight
2017-02-02 18:08 ` Tom Herbert
2017-02-27 13:39 ` Davide Caratti
2017-02-27 15:11 ` Tom Herbert
2017-02-28 10:31 ` Davide Caratti
2017-02-28 10:32 ` [PATCH RFC net-next v2 1/4] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti
2017-02-28 10:32 ` [PATCH RFC net-next v2 2/4] net: introduce skb_sctp_csum_help Davide Caratti
2017-02-28 10:32 ` [PATCH RFC net-next v2 3/4] net: more accurate checksumming in validate_xmit_skb Davide Caratti
2017-02-28 19:50 ` Tom Herbert
2017-02-28 10:32 ` [PATCH RFC net-next v2 4/4] Documentation: update notes on checksum offloading Davide Caratti
2017-02-28 22:46 ` [PATCH RFC net-next v2 1/4] skbuff: add stub to help computing crc32c on SCTP packets Alexander Duyck
2017-03-01 3:17 ` Tom Herbert [this message]
2017-03-01 10:53 ` David Laight
2017-03-06 21:51 ` Davide Caratti
2017-03-07 18:06 ` Alexander Duyck
2017-03-18 13:17 ` Davide Caratti
2017-03-18 22:35 ` Tom Herbert
2017-04-07 14:16 ` [PATCH RFC net-next v3 0/7] improve CRC32c in the forwarding path Davide Caratti
2017-04-07 14:16 ` [PATCH RFC net-next v3 1/7] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti
2017-04-07 14:16 ` [PATCH RFC net-next v3 2/7] net: introduce skb_crc32c_csum_help Davide Caratti
2017-04-07 14:16 ` [PATCH RFC net-next v3 3/7] sk_buff: remove support for csum_bad in sk_buff Davide Caratti
2017-04-07 14:16 ` [PATCH RFC net-next v3 4/7] net: use skb->csum_algo to identify packets needing crc32c Davide Caratti
2017-04-07 15:43 ` Tom Herbert
2017-04-07 17:29 ` Davide Caratti
2017-04-07 18:11 ` Tom Herbert
2017-04-13 10:36 ` Davide Caratti
2017-04-20 13:38 ` [PATCH RFC net-next v4 0/7] net: improve support for SCTP checksums Davide Caratti
2017-04-20 13:38 ` [PATCH RFC net-next v4 1/7] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti
2017-04-20 13:38 ` [PATCH RFC net-next v4 2/7] net: introduce skb_crc32c_csum_help Davide Caratti
2017-04-27 12:29 ` Marcelo Ricardo Leitner
2017-04-20 13:38 ` [PATCH RFC net-next v4 3/7] sk_buff: remove support for csum_bad in sk_buff Davide Caratti
2017-04-27 1:34 ` [sk_buff] 95510aef27: BUG:Bad_page_state_in_process kernel test robot
2017-04-29 20:21 ` [PATCH RFC net-next v4 3/7] sk_buff: remove support for csum_bad in sk_buff Tom Herbert
2017-04-20 13:38 ` [PATCH RFC net-next v4 4/7] net: use skb->csum_not_inet to identify packets needing crc32c Davide Caratti
2017-04-29 20:18 ` Tom Herbert
2017-04-20 13:38 ` [PATCH RFC net-next v4 5/7] net: more accurate checksumming in validate_xmit_skb() Davide Caratti
2017-04-20 13:38 ` [PATCH RFC net-next v4 6/7] openvswitch: more accurate checksumming in queue_userspace_packet() Davide Caratti
2017-04-20 13:38 ` [PATCH RFC net-next v4 7/7] sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY} Davide Caratti
2017-04-29 20:20 ` Tom Herbert
2017-04-27 12:41 ` [PATCH RFC net-next v4 0/7] net: improve support for SCTP checksums Marcelo Ricardo Leitner
2017-04-07 14:16 ` [PATCH RFC net-next v3 5/7] net: more accurate checksumming in validate_xmit_skb() Davide Caratti
2017-04-07 14:16 ` [PATCH RFC net-next v3 6/7] openvswitch: more accurate checksumming in queue_userspace_packet() Davide Caratti
2017-04-07 14:16 ` [PATCH RFC net-next v3 7/7] sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY} Davide Caratti
2017-01-23 16:52 ` [RFC PATCH net-next 3/5] net: introduce skb_sctp_csum_help Davide Caratti
2017-01-23 16:52 ` [RFC PATCH net-next 4/5] net: more accurate checksumming in validate_xmit_skb Davide Caratti
2017-01-23 16:52 ` [RFC PATCH net-next 5/5] Documentation: add description of skb_sctp_csum_help Davide Caratti
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='CALx6S36oXDyrm=4Bi-EdKgCBYYHuHENCoFz8GMBm6RmGCCep6g@mail.gmail.com' \
--to=tom@herbertland.com \
--cc=David.Laight@aculab.com \
--cc=alexander.duyck@gmail.com \
--cc=davem@davemloft.net \
--cc=dcaratti@redhat.com \
--cc=linux-sctp@vger.kernel.org \
--cc=marcelo.leitner@gmail.com \
--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 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).