diff mbox

[net-next,1/2] net: sctp: convert sctp_checksum_disable module param into sctp sysctl

Message ID 1375816693-7697-1-git-send-email-dborkman@redhat.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Daniel Borkmann Aug. 6, 2013, 7:18 p.m. UTC
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.

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(-)

Comments

Vladislav Yasevich Aug. 6, 2013, 7:31 p.m. UTC | #1
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 */ }
>   };
>
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Tuexen Aug. 6, 2013, 7:56 p.m. UTC | #2
On Aug 6, 2013, at 9:31 PM, Vlad Yasevich <vyasevich@gmail.com> wrote:

> 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.
I agree with Vlad. In FreeBSD this is only a compile time flag
used at the time where no checksum offloading was available
and during research work people wanted to know how the
performance would be when it is available.

Since we have now these nics, not sure if it makes sense
to keep it configurable at all...

Best regards
Michael
> 
> -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 */ }
>>  };
>> 
>> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Borkmann Aug. 6, 2013, 8:19 p.m. UTC | #3
On 08/06/2013 09:31 PM, Vlad Yasevich wrote:
> 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.

Ah, okay. I didn't follow that discussion. Actually both can easily be
set or unset as it's just /sys/module/sctp/parameters/no_checksums, but
I understand your reasoning that sysctl is something more "official".

> -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 */ }
>>   };
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Aug. 9, 2013, 6:33 p.m. UTC | #4
From: Daniel Borkmann <dborkman@redhat.com>
Date: Tue,  6 Aug 2013 21:18:12 +0200

> 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.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vladislav Yasevich Aug. 9, 2013, 6:50 p.m. UTC | #5
On 08/09/2013 02:33 PM, David Miller wrote:
> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Tue,  6 Aug 2013 21:18:12 +0200
>
>> 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.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>
> Applied.

David

Please revert this.  This isn't a knob that that should be
so freely exposed.  I thought my response to Daniel made
that clear.

Thanks
-vlad

> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Aug. 9, 2013, 8:10 p.m. UTC | #6
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Fri, 09 Aug 2013 14:50:40 -0400

> Please revert this.  This isn't a knob that that should be
> so freely exposed.  I thought my response to Daniel made
> that clear.

Done, sorry about that.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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 */ }
 };