netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block
@ 2013-10-05 20:09 Fabio Estevam
  2013-10-05 20:26 ` Fabio Estevam
  2013-10-05 20:27 ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Fabio Estevam @ 2013-10-05 20:09 UTC (permalink / raw)
  To: davem; +Cc: edumazet, hannes, netdev, olof, Fabio Estevam

From: Fabio Estevam <fabio.estevam@freescale.com>

Commit 9a3bab6b05 (net: net_secret should not depend on TCP) introduced
the following build warning when CONFIG_IPV6 is not selected:

net/core/secure_seq.c:17:13: warning: 'net_secret_init' defined but not used [-Wunused-function]

Fix it by moving net_secret_init(void) inside the '#if IS_ENABLED(CONFIG_IPV6)'
block.

Reported-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 net/core/secure_seq.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 3f1ec15..ee70541 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -10,6 +10,24 @@
 
 #include <net/secure_seq.h>
 
+#ifdef CONFIG_INET
+static u32 seq_scale(u32 seq)
+{
+	/*
+	 *	As close as possible to RFC 793, which
+	 *	suggests using a 250 kHz clock.
+	 *	Further reading shows this assumes 2 Mb/s networks.
+	 *	For 10 Mb/s Ethernet, a 1 MHz clock is appropriate.
+	 *	For 10 Gb/s Ethernet, a 1 GHz clock should be ok, but
+	 *	we also need to limit the resolution so that the u32 seq
+	 *	overlaps less than one time per MSL (2 minutes).
+	 *	Choosing a clock of 64 ns period is OK. (period of 274 s)
+	 */
+	return seq + (ktime_to_ns(ktime_get_real()) >> 6);
+}
+#endif
+
+#if IS_ENABLED(CONFIG_IPV6)
 #define NET_SECRET_SIZE (MD5_MESSAGE_BYTES / 4)
 
 static u32 net_secret[NET_SECRET_SIZE] ____cacheline_aligned;
@@ -30,24 +48,6 @@ static void net_secret_init(void)
 	}
 }
 
-#ifdef CONFIG_INET
-static u32 seq_scale(u32 seq)
-{
-	/*
-	 *	As close as possible to RFC 793, which
-	 *	suggests using a 250 kHz clock.
-	 *	Further reading shows this assumes 2 Mb/s networks.
-	 *	For 10 Mb/s Ethernet, a 1 MHz clock is appropriate.
-	 *	For 10 Gb/s Ethernet, a 1 GHz clock should be ok, but
-	 *	we also need to limit the resolution so that the u32 seq
-	 *	overlaps less than one time per MSL (2 minutes).
-	 *	Choosing a clock of 64 ns period is OK. (period of 274 s)
-	 */
-	return seq + (ktime_to_ns(ktime_get_real()) >> 6);
-}
-#endif
-
-#if IS_ENABLED(CONFIG_IPV6)
 __u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
 				   __be16 sport, __be16 dport)
 {
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block
  2013-10-05 20:09 [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block Fabio Estevam
@ 2013-10-05 20:26 ` Fabio Estevam
  2013-10-05 20:27 ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2013-10-05 20:26 UTC (permalink / raw)
  To: David S. Miller
  Cc: edumazet, netdev, Olof Johansson, Fabio Estevam, Hannes Frederic Sowa

On Sat, Oct 5, 2013 at 5:09 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Commit 9a3bab6b05 (net: net_secret should not depend on TCP) introduced
> the following build warning when CONFIG_IPV6 is not selected:
>
> net/core/secure_seq.c:17:13: warning: 'net_secret_init' defined but not used [-Wunused-function]
>
> Fix it by moving net_secret_init(void) inside the '#if IS_ENABLED(CONFIG_IPV6)'
> block.
>
> Reported-by: Olof Johansson <olof@lixom.net>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Another solution would be:

--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -10,6 +10,7 @@

 #include <net/secure_seq.h>

+#if IS_ENABLED(CONFIG_IPV6)
 #define NET_SECRET_SIZE (MD5_MESSAGE_BYTES / 4)

 static u32 net_secret[NET_SECRET_SIZE] ____cacheline_aligned;
@@ -29,6 +30,7 @@ static void net_secret_init(void)
                cmpxchg(&net_secret[--i], 0, tmp);
        }
 }
+#endif

If this is preferred, I can send a v2 like that.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block
  2013-10-05 20:09 [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block Fabio Estevam
  2013-10-05 20:26 ` Fabio Estevam
@ 2013-10-05 20:27 ` David Miller
  2013-10-06  5:25   ` Olof Johansson
  1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2013-10-05 20:27 UTC (permalink / raw)
  To: festevam; +Cc: edumazet, hannes, netdev, olof, fabio.estevam

From: Fabio Estevam <festevam@gmail.com>
Date: Sat,  5 Oct 2013 17:09:50 -0300

> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Commit 9a3bab6b05 (net: net_secret should not depend on TCP) introduced
> the following build warning when CONFIG_IPV6 is not selected:
> 
> net/core/secure_seq.c:17:13: warning: 'net_secret_init' defined but not used [-Wunused-function]
> 
> Fix it by moving net_secret_init(void) inside the '#if IS_ENABLED(CONFIG_IPV6)'
> block.
> 
> Reported-by: Olof Johansson <olof@lixom.net>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

seq_scale is used by secure_tcp_sequence_number, which is only protected
by CONFIG_INET.  I have no idea how you can get this build problem.

And I cannot reproduce it here:

[davem@drr net]$ egrep CONFIG_IPV6 .config
# CONFIG_IPV6 is not set
[davem@drr net]$ make net/core/secure_seq.o
scripts/kconfig/conf --silentoldconfig Kconfig
make[1]: Nothing to be done for `all'.
make[1]: Nothing to be done for `relocs'.
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  CC      net/core/secure_seq.o
[davem@drr net]$ 

Furthermore your commit message is talking about net_secret_init but
your patch does things with seq_scale.

I'm not applying this patch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block
  2013-10-05 20:27 ` David Miller
@ 2013-10-06  5:25   ` Olof Johansson
  2013-10-06 16:29     ` Fabio Estevam
  0 siblings, 1 reply; 6+ messages in thread
From: Olof Johansson @ 2013-10-06  5:25 UTC (permalink / raw)
  To: David Miller
  Cc: Fabio Estevam, edumazet, hannes, Network Development, Fabio Estevam

On Sat, Oct 5, 2013 at 1:27 PM, David Miller <davem@davemloft.net> wrote:
> From: Fabio Estevam <festevam@gmail.com>
> Date: Sat,  5 Oct 2013 17:09:50 -0300
>
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Commit 9a3bab6b05 (net: net_secret should not depend on TCP) introduced
>> the following build warning when CONFIG_IPV6 is not selected:
>>
>> net/core/secure_seq.c:17:13: warning: 'net_secret_init' defined but not used [-Wunused-function]
>>
>> Fix it by moving net_secret_init(void) inside the '#if IS_ENABLED(CONFIG_IPV6)'
>> block.
>>
>> Reported-by: Olof Johansson <olof@lixom.net>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>
> seq_scale is used by secure_tcp_sequence_number, which is only protected
> by CONFIG_INET.  I have no idea how you can get this build problem.
>
> And I cannot reproduce it here:

You get it if you have CONFIG_NET enabled, but CONFIG_INET off. We
seem to have a few defconfigs on arm that has that setting, likely
because they lack native network interfaces but need local unix
sockets. Or whatever. But that's how you hit it.

Steps to reproduce, even with ARCH=sparc:

make allnoconfig
edit .config, set CONFIG_NET=y
yes "" | make oldconfig
make


-Olof

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block
  2013-10-06  5:25   ` Olof Johansson
@ 2013-10-06 16:29     ` Fabio Estevam
  0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2013-10-06 16:29 UTC (permalink / raw)
  To: Olof Johansson
  Cc: David Miller, edumazet, hannes, Network Development, Fabio Estevam

Hi Olof,

On Sun, Oct 6, 2013 at 2:25 AM, Olof Johansson <olof@lixom.net> wrote:

> You get it if you have CONFIG_NET enabled, but CONFIG_INET off. We
> seem to have a few defconfigs on arm that has that setting, likely
> because they lack native network interfaces but need local unix
> sockets. Or whatever. But that's how you hit it.
>
> Steps to reproduce, even with ARCH=sparc:
>
> make allnoconfig
> edit .config, set CONFIG_NET=y
> yes "" | make oldconfig
> make

Yes, I fixed the commit log and patch in v2.

Regards,

Fabio Estevam

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block
@ 2013-10-05 20:56 Fabio Estevam
  0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2013-10-05 20:56 UTC (permalink / raw)
  To: davem; +Cc: edumazet, hannes, netdev, olof, Fabio Estevam

From: Fabio Estevam <fabio.estevam@freescale.com>

Commit 9a3bab6b05 (net: net_secret should not depend on TCP) introduced
the following build warning when CONFIG_IPV6 is not selected:

net/core/secure_seq.c:17:13: warning: 'net_secret_init' defined but not used [-Wunused-function]

Fix it by moving net_secret_init(void) inside the '#if IS_ENABLED(CONFIG_IPV6)'
block.

Reported-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 net/core/secure_seq.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 3f1ec15..ee70541 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -10,6 +10,24 @@
 
 #include <net/secure_seq.h>
 
+#ifdef CONFIG_INET
+static u32 seq_scale(u32 seq)
+{
+	/*
+	 *	As close as possible to RFC 793, which
+	 *	suggests using a 250 kHz clock.
+	 *	Further reading shows this assumes 2 Mb/s networks.
+	 *	For 10 Mb/s Ethernet, a 1 MHz clock is appropriate.
+	 *	For 10 Gb/s Ethernet, a 1 GHz clock should be ok, but
+	 *	we also need to limit the resolution so that the u32 seq
+	 *	overlaps less than one time per MSL (2 minutes).
+	 *	Choosing a clock of 64 ns period is OK. (period of 274 s)
+	 */
+	return seq + (ktime_to_ns(ktime_get_real()) >> 6);
+}
+#endif
+
+#if IS_ENABLED(CONFIG_IPV6)
 #define NET_SECRET_SIZE (MD5_MESSAGE_BYTES / 4)
 
 static u32 net_secret[NET_SECRET_SIZE] ____cacheline_aligned;
@@ -30,24 +48,6 @@ static void net_secret_init(void)
 	}
 }
 
-#ifdef CONFIG_INET
-static u32 seq_scale(u32 seq)
-{
-	/*
-	 *	As close as possible to RFC 793, which
-	 *	suggests using a 250 kHz clock.
-	 *	Further reading shows this assumes 2 Mb/s networks.
-	 *	For 10 Mb/s Ethernet, a 1 MHz clock is appropriate.
-	 *	For 10 Gb/s Ethernet, a 1 GHz clock should be ok, but
-	 *	we also need to limit the resolution so that the u32 seq
-	 *	overlaps less than one time per MSL (2 minutes).
-	 *	Choosing a clock of 64 ns period is OK. (period of 274 s)
-	 */
-	return seq + (ktime_to_ns(ktime_get_real()) >> 6);
-}
-#endif
-
-#if IS_ENABLED(CONFIG_IPV6)
 __u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
 				   __be16 sport, __be16 dport)
 {
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-06 16:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-05 20:09 [PATCH] net: secure_seq: Move net_secret_init() definition into CONFIG_IPV6 if block Fabio Estevam
2013-10-05 20:26 ` Fabio Estevam
2013-10-05 20:27 ` David Miller
2013-10-06  5:25   ` Olof Johansson
2013-10-06 16:29     ` Fabio Estevam
2013-10-05 20:56 Fabio Estevam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).