[patch for-3.8] fs, dlm: fix build error when EXPERIMENTAL is disabled

[patch for-3.8] fs, dlm: fix build error when EXPERIMENTAL is disabled

[David Rientjes]

CONFIG_IP_SCTP relies on being able to select things like CONFIG_CRC32C to 
build.  Thus, nothing should be selecting CONFIG_IP_SCTP that does not 
meet its requirements.

For example, if CONFIG_EXPERIMENTAL is disabled and CONFIG_DLM is enabled, 
the build fails at link time:

	net/built-in.o: In function `sctp_crc32c':
	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
	net/built-in.o:include/net/sctp/checksum.h:51: more undefined references to `crc32c' follow

Fix this by making CONFIG_DLM depend on CONFIG_EXPERIMENTAL so that 
CONFIG_IP_SCTP properly builds.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 fs/dlm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/dlm/Kconfig b/fs/dlm/Kconfig
--- a/fs/dlm/Kconfig
+++ b/fs/dlm/Kconfig
@@ -2,6 +2,7 @@ menuconfig DLM
 	tristate "Distributed Lock Manager (DLM)"
 	depends on INET
 	depends on SYSFS && CONFIGFS_FS && (IPV6 || IPV6=n)
+	depends on EXPERIMENTAL
 	select IP_SCTP
 	help
 	A general purpose distributed lock manager for kernel or userspace

Re: [patch for-3.8] fs, dlm: fix build error when EXPERIMENTAL is disabled

[Steven Whitehouse]

Hi,

On Mon, 2013-02-11 at 13:48 -0800, David Rientjes wrote:
> CONFIG_IP_SCTP relies on being able to select things like CONFIG_CRC32C to 
> build.  Thus, nothing should be selecting CONFIG_IP_SCTP that does not 
> meet its requirements.
> 
> For example, if CONFIG_EXPERIMENTAL is disabled and CONFIG_DLM is enabled, 
> the build fails at link time:
> 
> 	net/built-in.o: In function `sctp_crc32c':
> 	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
> 	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
> 	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
> 	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
> 	include/net/sctp/checksum.h:51: undefined reference to `crc32c'
> 	net/built-in.o:include/net/sctp/checksum.h:51: more undefined references to `crc32c' follow
> 
> Fix this by making CONFIG_DLM depend on CONFIG_EXPERIMENTAL so that 
> CONFIG_IP_SCTP properly builds.
> 
That doesn't seem right to me... DLM has not been experimental for a
long time now. Why not just select CRC32 in addition to IP_SCTP ?

Steve.

> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  fs/dlm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/dlm/Kconfig b/fs/dlm/Kconfig
> --- a/fs/dlm/Kconfig
> +++ b/fs/dlm/Kconfig
> @@ -2,6 +2,7 @@ menuconfig DLM
>  	tristate "Distributed Lock Manager (DLM)"
>  	depends on INET
>  	depends on SYSFS && CONFIGFS_FS && (IPV6 || IPV6=n)
> +	depends on EXPERIMENTAL
>  	select IP_SCTP
>  	help
>  	A general purpose distributed lock manager for kernel or userspace
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch for-3.8] fs, dlm: fix build error when EXPERIMENTAL is disabled

[Linus Torvalds]

On Tue, Feb 12, 2013 at 1:50 AM, Steven Whitehouse <swhiteho@redhat.com> wrote:
>
> That doesn't seem right to me... DLM has not been experimental for a
> long time now. Why not just select CRC32 in addition to IP_SCTP ?

Hmm. IP_SCTP already does a "select libcrc32c". So why doesn't that
end up working?

                    Linus

Re: [patch for-3.8] fs, dlm: fix build error when EXPERIMENTAL is disabled

[David Rientjes]

On Tue, 12 Feb 2013, Linus Torvalds wrote:

> On Tue, Feb 12, 2013 at 1:50 AM, Steven Whitehouse <swhiteho@redhat.com> wrote:
> >
> > That doesn't seem right to me... DLM has not been experimental for a
> > long time now. Why not just select CRC32 in addition to IP_SCTP ?
> 
> Hmm. IP_SCTP already does a "select libcrc32c". So why doesn't that
> end up working?
> 

Kconfig won't select things that CONFIG_IP_SCTP select unless its "depends 
on" are satisfied, which is why I made CONFIG_DLM depend on 
CONFIG_EXPERIMENTAL.

Steven says DLM hasn't been experimental for a long time; sorry, but if an 
option you select is experimental then you're experimental as well.

If you'd like to push a patch that removes EXPERIMENTAL from IP_SCTP to 
David Miller, that works too.  But that is a completely separate topic 
from dlm.  For 3.8, I suggest respecting CONFIG_IP_SCTP's requirements and 
then removing EXPERIMENTAL from both if the networking guys agree.

[patch for-3.8] net, sctp: remove CONFIG_EXPERIMENTAL

[David Rientjes]

From: Kees Cook <keescook@chromium.org>

This config item has not carried much meaning for a while now and is
almost always enabled by default. As agreed during the Linux kernel
summit, remove it.

Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 Ah, look what I found in my mailbox from October 23.  The patch for dlm
 was merged, but this wasn't for some reason.  It's acked by the 
 maintainer so it should be good to go and fixes the reported build error:

        net/built-in.o: In function `sctp_crc32c':
        include/net/sctp/checksum.h:51: undefined reference to `crc32c'
        include/net/sctp/checksum.h:51: undefined reference to `crc32c'
        include/net/sctp/checksum.h:51: undefined reference to `crc32c'
        include/net/sctp/checksum.h:51: undefined reference to `crc32c'
        include/net/sctp/checksum.h:51: undefined reference to `crc32c'
        net/built-in.o:include/net/sctp/checksum.h:51: more undefined references to `crc32c' follow

 And I guess "dlm not being marked experimental for a long time" meant
 three months.

 net/sctp/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
--- a/net/sctp/Kconfig
+++ b/net/sctp/Kconfig
@@ -3,8 +3,8 @@
 #
 
 menuconfig IP_SCTP
-	tristate "The SCTP Protocol (EXPERIMENTAL)"
-	depends on INET && EXPERIMENTAL
+	tristate "The SCTP Protocol"
+	depends on INET
 	depends on IPV6 || IPV6=n
 	select CRYPTO
 	select CRYPTO_HMAC

Re: [patch for-3.8] net, sctp: remove CONFIG_EXPERIMENTAL

[Steven Whitehouse]

Hi,

On Tue, 2013-02-12 at 16:24 -0800, David Rientjes wrote:
> From: Kees Cook <keescook@chromium.org>
> 
> This config item has not carried much meaning for a while now and is
> almost always enabled by default. As agreed during the Linux kernel
> summit, remove it.
> 
> Acked-by: David S. Miller <davem@davemloft.net>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  Ah, look what I found in my mailbox from October 23.  The patch for dlm
>  was merged, but this wasn't for some reason.  It's acked by the 
>  maintainer so it should be good to go and fixes the reported build error:
> 
>         net/built-in.o: In function `sctp_crc32c':
>         include/net/sctp/checksum.h:51: undefined reference to `crc32c'
>         include/net/sctp/checksum.h:51: undefined reference to `crc32c'
>         include/net/sctp/checksum.h:51: undefined reference to `crc32c'
>         include/net/sctp/checksum.h:51: undefined reference to `crc32c'
>         include/net/sctp/checksum.h:51: undefined reference to `crc32c'
>         net/built-in.o:include/net/sctp/checksum.h:51: more undefined references to `crc32c' follow
> 
>  And I guess "dlm not being marked experimental for a long time" meant
>  three months.
> 
Yes, I was surprised about that. We removed that tag from GFS2 back in
2010 it seems and I'm surprised that it didn't get removed from DLM at
around the same time. Either way though, it is a long time since either
have been experimental in reality, as both are stable and have many
users.

Also SCTP is only a (run time) option for DLM, and few users will
actually use it, since most people will use the TCP transport instead.

Anyway, this patch looks ok to me, so:
Acked-by: Steven Whitehouse <swhiteho@redhat.com>

Steve.


>  net/sctp/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
> --- a/net/sctp/Kconfig
> +++ b/net/sctp/Kconfig
> @@ -3,8 +3,8 @@
>  #
>  
>  menuconfig IP_SCTP
> -	tristate "The SCTP Protocol (EXPERIMENTAL)"
> -	depends on INET && EXPERIMENTAL
> +	tristate "The SCTP Protocol"
> +	depends on INET
>  	depends on IPV6 || IPV6=n
>  	select CRYPTO
>  	select CRYPTO_HMAC

Re: [patch for-3.8] net, sctp: remove CONFIG_EXPERIMENTAL

[David Miller]

From: David Rientjes <rientjes@google.com>
Date: Tue, 12 Feb 2013 16:24:56 -0800 (PST)

> From: Kees Cook <keescook@chromium.org>
> 
> This config item has not carried much meaning for a while now and is
> almost always enabled by default. As agreed during the Linux kernel
> summit, remove it.
> 
> Acked-by: David S. Miller <davem@davemloft.net>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Rientjes <rientjes@google.com>

Applied, thanks.