diff mbox

sctp: Change defaults on cookie hmac selection

Message ID 1355511060-27320-1-git-send-email-nhorman@tuxdriver.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Neil Horman Dec. 14, 2012, 6:51 p.m. UTC
Recently I posted commit 3c68198e75 which made selection of the cookie hmac
algorithm selectable.  This is all well and good, but Linus noted that it
changes the default config:
http://marc.info/?l=linux-netdev&m=135536629004808&w=2

I've modified the sctp Kconfig file to reflect the recommended way of making
this choice, using the thermal driver example specified, and brought the
defaults back into line with the way they were prior to my origional patch

Tested by myself (allbeit fairly quickly).  All configuration combinations seems
to work soundly.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: David Miller <davem@davemloft.net>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: linux-sctp@vger.kernel.org
---
 net/sctp/Kconfig | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Vladislav Yasevich Dec. 14, 2012, 8:01 p.m. UTC | #1
On 12/14/2012 01:51 PM, Neil Horman wrote:
> Recently I posted commit 3c68198e75 which made selection of the cookie hmac
> algorithm selectable.  This is all well and good, but Linus noted that it
> changes the default config:
> http://marc.info/?l=linux-netdev&m=135536629004808&w=2
>
> I've modified the sctp Kconfig file to reflect the recommended way of making
> this choice, using the thermal driver example specified, and brought the
> defaults back into line with the way they were prior to my origional patch
>
> Tested by myself (allbeit fairly quickly).  All configuration combinations seems
> to work soundly.
>

Just tried it and like how it looks.

Acked-by: Vlad Yasevich <vyasevich@gmail.com>

> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: David Miller <davem@davemloft.net>
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Vlad Yasevich <vyasevich@gmail.com>
> CC: linux-sctp@vger.kernel.org
> ---
>   net/sctp/Kconfig | 22 ++++++++++++++++++++--
>   1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
> index a9edd2e..7cd47be 100644
> --- a/net/sctp/Kconfig
> +++ b/net/sctp/Kconfig
> @@ -66,12 +66,31 @@ config SCTP_DBG_OBJCNT
>   	  'cat /proc/net/sctp/sctp_dbg_objcnt'
>
>   	  If unsure, say N
> +choice
> +	prompt "Default SCTP cookie HMAC encoding"
> +	default SCTP_COOKIE_HMAC_MD5
> +	help
> +	  This option sets the default sctp cookie hmac algorithm
> +	  when in doubt select 'md5'
> +
> +config SCTP_DEFAULT_COOKIE_HMAC_MD5
> +	bool "Enable optional MD5 hmac cookie generation"
> +	help
> +	  Enable optional MD5 hmac based SCTP cookie generation
> +	select SCTP_COOKIE_HMAC_MD5
> +
> +config SCTP_DEFAULT_COOKIE_HMAC_SHA1
> +	bool "Enable optional SHA1 hmac cookie generation"
> +	help
> +	  Enable optional SHA1 hmac based SCTP cookie generation
> +	select SCTP_COOKIE_HMAC_SHA1
> +
> +endchoice
>
>   config SCTP_COOKIE_HMAC_MD5
>   	bool "Enable optional MD5 hmac cookie generation"
>   	help
>   	  Enable optional MD5 hmac based SCTP cookie generation
> -	default y
>   	select CRYPTO_HMAC if SCTP_COOKIE_HMAC_MD5
>   	select CRYPTO_MD5 if SCTP_COOKIE_HMAC_MD5
>
> @@ -79,7 +98,6 @@ config SCTP_COOKIE_HMAC_SHA1
>   	bool "Enable optional SHA1 hmac cookie generation"
>   	help
>   	  Enable optional SHA1 hmac based SCTP cookie generation
> -	default y
>   	select CRYPTO_HMAC if SCTP_COOKIE_HMAC_SHA1
>   	select CRYPTO_SHA1 if SCTP_COOKIE_HMAC_SHA1
>
>

--
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
Linus Torvalds Dec. 14, 2012, 9:56 p.m. UTC | #2
On Fri, Dec 14, 2012 at 10:51 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
>
> Tested by myself (allbeit fairly quickly).  All configuration combinations seems
> to work soundly.

Ok, looks good, but I suspect we should also re-introduce the "none"
choice we used to have.

No?

          Linus
--
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
Neil Horman Dec. 15, 2012, 12:38 a.m. UTC | #3
On Fri, Dec 14, 2012 at 01:56:15PM -0800, Linus Torvalds wrote:
> On Fri, Dec 14, 2012 at 10:51 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > Tested by myself (allbeit fairly quickly).  All configuration combinations seems
> > to work soundly.
> 
> Ok, looks good, but I suspect we should also re-introduce the "none"
> choice we used to have.
> 
> No?
> 
No need, I think.  Since its a user configurable choice now, the user can just
set the selector to "None" in proc, and none will be used.

Neil

--
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
Linus Torvalds Dec. 15, 2012, 12:44 a.m. UTC | #4
On Fri, Dec 14, 2012 at 4:38 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
>
> No need, I think.  Since its a user configurable choice now, the user can just
> set the selector to "None" in proc, and none will be used.

Well, by the same logic, there's no point to defaulting to MD5 either,
since the user could just put MD5 in the there.

The point is, if you're asking for a default, you should include the
actual choices, and not limit it to some random subset of choices. No?

             Linus
--
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
Neil Horman Dec. 15, 2012, 1:12 a.m. UTC | #5
On Fri, Dec 14, 2012 at 04:44:56PM -0800, Linus Torvalds wrote:
> On Fri, Dec 14, 2012 at 4:38 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > No need, I think.  Since its a user configurable choice now, the user can just
> > set the selector to "None" in proc, and none will be used.
> 
> Well, by the same logic, there's no point to defaulting to MD5 either,
> since the user could just put MD5 in the there.
> 
> The point is, if you're asking for a default, you should include the
> actual choices, and not limit it to some random subset of choices. No?
> 
>              Linus
> 
Ok, Thats a fair point, I can add a option to choose the "None" option as the
deafult.  Dave, do you want a delta patch, or a new version?
Neil

--
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 Dec. 15, 2012, 1:14 a.m. UTC | #6
From: Neil Horman <nhorman@tuxdriver.com>
Date: Fri, 14 Dec 2012 20:12:29 -0500

> On Fri, Dec 14, 2012 at 04:44:56PM -0800, Linus Torvalds wrote:
>> On Fri, Dec 14, 2012 at 4:38 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
>> >
>> > No need, I think.  Since its a user configurable choice now, the user can just
>> > set the selector to "None" in proc, and none will be used.
>> 
>> Well, by the same logic, there's no point to defaulting to MD5 either,
>> since the user could just put MD5 in the there.
>> 
>> The point is, if you're asking for a default, you should include the
>> actual choices, and not limit it to some random subset of choices. No?
>> 
>>              Linus
>> 
> Ok, Thats a fair point, I can add a option to choose the "None" option as the
> deafult.  Dave, do you want a delta patch, or a new version?

New version please.
--
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/net/sctp/Kconfig b/net/sctp/Kconfig
index a9edd2e..7cd47be 100644
--- a/net/sctp/Kconfig
+++ b/net/sctp/Kconfig
@@ -66,12 +66,31 @@  config SCTP_DBG_OBJCNT
 	  'cat /proc/net/sctp/sctp_dbg_objcnt'
 
 	  If unsure, say N
+choice
+	prompt "Default SCTP cookie HMAC encoding"
+	default SCTP_COOKIE_HMAC_MD5
+	help
+	  This option sets the default sctp cookie hmac algorithm
+	  when in doubt select 'md5'
+
+config SCTP_DEFAULT_COOKIE_HMAC_MD5
+	bool "Enable optional MD5 hmac cookie generation"
+	help
+	  Enable optional MD5 hmac based SCTP cookie generation
+	select SCTP_COOKIE_HMAC_MD5
+
+config SCTP_DEFAULT_COOKIE_HMAC_SHA1
+	bool "Enable optional SHA1 hmac cookie generation"
+	help
+	  Enable optional SHA1 hmac based SCTP cookie generation
+	select SCTP_COOKIE_HMAC_SHA1
+
+endchoice
 
 config SCTP_COOKIE_HMAC_MD5
 	bool "Enable optional MD5 hmac cookie generation"
 	help
 	  Enable optional MD5 hmac based SCTP cookie generation
-	default y
 	select CRYPTO_HMAC if SCTP_COOKIE_HMAC_MD5
 	select CRYPTO_MD5 if SCTP_COOKIE_HMAC_MD5
 
@@ -79,7 +98,6 @@  config SCTP_COOKIE_HMAC_SHA1
 	bool "Enable optional SHA1 hmac cookie generation"
 	help
 	  Enable optional SHA1 hmac based SCTP cookie generation
-	default y
 	select CRYPTO_HMAC if SCTP_COOKIE_HMAC_SHA1
 	select CRYPTO_SHA1 if SCTP_COOKIE_HMAC_SHA1