Re: [PATCH RFC net-next v2 1/4] skbuff: add stub to help computing crc32c on SCTP packets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 28, 2017 at 2:32 AM, Davide Caratti <dcaratti@xxxxxxxxxx> 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.

> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx>
> Signed-off-by: Davide Caratti <dcaratti@xxxxxxxxxx>
> ---
>  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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Networking Development]     [Linux OMAP]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux