From: Vlad Yasevich <vyasevich@gmail•com>
To: Daniel Borkmann <dborkman@redhat•com>
Cc: davem@davemloft•net, netdev@vger•kernel.org, linux-sctp@vger•kernel.org
Subject: Re: [PATCH net-next 1/2] net: sctp: convert sctp_checksum_disable module param into sctp sysctl
Date: Tue, 06 Aug 2013 15:31:26 -0400 [thread overview]
Message-ID: <52014F0E.6040101@gmail.com> (raw)
In-Reply-To: <1375816693-7697-1-git-send-email-dborkman@redhat.com>
On 08/06/2013 03:18 PM, Daniel Borkmann wrote:
> Get rid of the last module parameter for SCTP and make this
> configurable via sysctl for SCTP like all the rest of SCTP's
> configuration knobs.
>
But this isn't like the rest of the sctp knobs. Disabling this violates
the base protocol and we don't really want to encourage
people to do this.
There was a long discussion about it back in 2009 when the original
patch was submitted that proposed a sysctl. Nothing has changed since
then to convince me that this sysctl would be a good idea.
-vlad
> Signed-off-by: Daniel Borkmann <dborkman@redhat•com>
> ---
> Documentation/networking/ip-sysctl.txt | 8 ++++++++
> include/net/netns/sctp.h | 3 +++
> include/net/sctp/structs.h | 5 -----
> net/sctp/input.c | 4 ++--
> net/sctp/output.c | 5 ++++-
> net/sctp/protocol.c | 5 +++--
> net/sctp/sysctl.c | 10 +++++++++-
> 7 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 36be26b..6f5b813 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -1507,6 +1507,14 @@ sack_timeout - INTEGER
>
> Default: 200
>
> +checksum_disable - BOOLEAN
> + Disable SCTP checksum computing and verification for debugging purpose.
> +
> + 1: Disable checksumming
> + 0: Enable checksumming
> +
> + Default: 0
> +
> valid_cookie_life - INTEGER
> The default lifetime of the SCTP cookie (in milliseconds). The cookie
> is used during association establishment.
> diff --git a/include/net/netns/sctp.h b/include/net/netns/sctp.h
> index 3573a81..ebfdf1e 100644
> --- a/include/net/netns/sctp.h
> +++ b/include/net/netns/sctp.h
> @@ -129,6 +129,9 @@ struct netns_sctp {
>
> /* Threshold for autoclose timeout, in seconds. */
> unsigned long max_autoclose;
> +
> + /* Flag to disable SCTP checksumming. */
> + int checksum_disable;
> };
>
> #endif /* __NETNS_SCTP_H__ */
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index d9c93a7..06ebeaa 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -141,10 +141,6 @@ extern struct sctp_globals {
> /* This is the sctp port control hash. */
> int port_hashsize;
> struct sctp_bind_hashbucket *port_hashtable;
> -
> - /* Flag to indicate whether computing and verifying checksum
> - * is disabled. */
> - bool checksum_disable;
> } sctp_globals;
>
> #define sctp_max_instreams (sctp_globals.max_instreams)
> @@ -156,7 +152,6 @@ extern struct sctp_globals {
> #define sctp_assoc_hashtable (sctp_globals.assoc_hashtable)
> #define sctp_port_hashsize (sctp_globals.port_hashsize)
> #define sctp_port_hashtable (sctp_globals.port_hashtable)
> -#define sctp_checksum_disable (sctp_globals.checksum_disable)
>
> /* SCTP Socket type: UDP or TCP style. */
> typedef enum {
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index fa91aff..b9a25e1 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -140,8 +140,8 @@ int sctp_rcv(struct sk_buff *skb)
> __skb_pull(skb, skb_transport_offset(skb));
> if (skb->len < sizeof(struct sctphdr))
> goto discard_it;
> - if (!sctp_checksum_disable && !skb_csum_unnecessary(skb) &&
> - sctp_rcv_checksum(net, skb) < 0)
> + if (!net->sctp.checksum_disable && !skb_csum_unnecessary(skb) &&
> + sctp_rcv_checksum(net, skb) < 0)
> goto discard_it;
>
> skb_pull(skb, sizeof(struct sctphdr));
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 5a55c55d..cdb5f49 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -395,6 +395,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)
> int padding; /* How much padding do we need? */
> __u8 has_data = 0;
> struct dst_entry *dst = tp->dst;
> + struct net *net;
> unsigned char *auth = NULL; /* pointer to auth in skb data */
> __u32 cksum_buf_len = sizeof(struct sctphdr);
>
> @@ -541,7 +542,9 @@ int sctp_packet_transmit(struct sctp_packet *packet)
> * Note: Adler-32 is no longer applicable, as has been replaced
> * by CRC32-C as described in <draft-ietf-tsvwg-sctpcsum-02.txt>.
> */
> - if (!sctp_checksum_disable) {
> + net = dev_net(dst->dev);
> +
> + if (!net->sctp.checksum_disable) {
> if (!(dst->dev->features & NETIF_F_SCTP_CSUM)) {
> __u32 crc32 = sctp_start_cksum((__u8 *)sh, cksum_buf_len);
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index b52ec25..a570a63 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1193,6 +1193,9 @@ static int __net_init sctp_net_init(struct net *net)
> /* Whether Cookie Preservative is enabled(1) or not(0) */
> net->sctp.cookie_preserve_enable = 1;
>
> + /* Whether SCTP checksumming is disabled(1) or not(0) */
> + net->sctp.checksum_disable = 0;
> +
> /* Default sctp sockets to use md5 as their hmac alg */
> #if defined (CONFIG_SCTP_DEFAULT_COOKIE_HMAC_MD5)
> net->sctp.sctp_hmac_alg = "md5";
> @@ -1549,6 +1552,4 @@ MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-132");
> MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-132");
> MODULE_AUTHOR("Linux Kernel SCTP developers <linux-sctp@vger•kernel.org>");
> MODULE_DESCRIPTION("Support for the SCTP protocol (RFC2960)");
> -module_param_named(no_checksums, sctp_checksum_disable, bool, 0644);
> -MODULE_PARM_DESC(no_checksums, "Disable checksums computing and verification");
> MODULE_LICENSE("GPL");
> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
> index 1906747..754809a 100644
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@ -296,7 +296,15 @@ static struct ctl_table sctp_net_table[] = {
> .extra1 = &max_autoclose_min,
> .extra2 = &max_autoclose_max,
> },
> -
> + {
> + .procname = "checksum_disable",
> + .data = &init_net.sctp.checksum_disable,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> { /* sentinel */ }
> };
>
>
next prev parent reply other threads:[~2013-08-06 19:31 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-06 19:18 [PATCH net-next 1/2] net: sctp: convert sctp_checksum_disable module param into sctp sysctl Daniel Borkmann
2013-08-06 19:18 ` [PATCH net-next 2/2] net: sctp: trivial: update bug report in header comment Daniel Borkmann
2013-08-06 19:33 ` Vlad Yasevich
2013-08-09 18:33 ` David Miller
2013-08-06 19:31 ` Vlad Yasevich [this message]
2013-08-06 19:56 ` [PATCH net-next 1/2] net: sctp: convert sctp_checksum_disable module param into sctp sysctl Michael Tuexen
2013-08-06 20:19 ` Daniel Borkmann
2013-08-09 18:33 ` David Miller
2013-08-09 18:50 ` Vlad Yasevich
2013-08-09 20:10 ` 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=52014F0E.6040101@gmail.com \
--to=vyasevich@gmail$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=dborkman@redhat$(echo .)com \
--cc=linux-sctp@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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