[net-next v4 0/3] use standard sysctl macro

[net-next v4 0/3] use standard sysctl macro

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patchset introduce sysctl macro or replace var
with macro.

Tonghao Zhang (3):
  net: sysctl: use shared sysctl macro
  net: sysctl: introduce sysctl SYSCTL_THREE
  selftests/sysctl: add sysctl macro test

 fs/proc/proc_sysctl.c                    |  2 +-
 include/linux/sysctl.h                   |  9 ++---
 lib/test_sysctl.c                        | 21 ++++++++++++
 net/core/sysctl_net_core.c               | 13 +++----
 net/ipv4/sysctl_net_ipv4.c               | 16 ++++-----
 net/ipv6/sysctl_net_ipv6.c               |  6 ++--
 net/netfilter/ipvs/ip_vs_ctl.c           |  4 +--
 tools/testing/selftests/sysctl/sysctl.sh | 43 ++++++++++++++++++++++++
 8 files changed, 84 insertions(+), 30 deletions(-)

-- 
v4: add selftests/sysctl patch
v3: split patch to two.

Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
--
2.27.0

[net-next v4 1/3] net: sysctl: use shared sysctl macro

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patch replace two, four and long_one to SYSCTL_XXX.

Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 net/core/sysctl_net_core.c | 10 ++++------
 net/ipv4/sysctl_net_ipv4.c | 13 +++++--------
 net/ipv6/sysctl_net_ipv6.c |  3 +--
 3 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 8295e5877eb3..3a0ce309ffcd 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -25,13 +25,11 @@
 
 #include "dev.h"
 
-static int two = 2;
 static int three = 3;
 static int int_3600 = 3600;
 static int min_sndbuf = SOCK_MIN_SNDBUF;
 static int min_rcvbuf = SOCK_MIN_RCVBUF;
 static int max_skb_frags = MAX_SKB_FRAGS;
-static long long_one __maybe_unused = 1;
 static long long_max __maybe_unused = LONG_MAX;
 
 static int net_msg_warn;	/* Unused, but still a sysctl */
@@ -390,7 +388,7 @@ static struct ctl_table net_core_table[] = {
 		.extra2		= SYSCTL_ONE,
 # else
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &two,
+		.extra2		= SYSCTL_TWO,
 # endif
 	},
 # ifdef CONFIG_HAVE_EBPF_JIT
@@ -401,7 +399,7 @@ static struct ctl_table net_core_table[] = {
 		.mode		= 0600,
 		.proc_handler	= proc_dointvec_minmax_bpf_restricted,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &two,
+		.extra2		= SYSCTL_TWO,
 	},
 	{
 		.procname	= "bpf_jit_kallsyms",
@@ -419,7 +417,7 @@ static struct ctl_table net_core_table[] = {
 		.maxlen		= sizeof(long),
 		.mode		= 0600,
 		.proc_handler	= proc_dolongvec_minmax_bpf_restricted,
-		.extra1		= &long_one,
+		.extra1		= SYSCTL_LONG_ONE,
 		.extra2		= &bpf_jit_limit_max,
 	},
 #endif
@@ -546,7 +544,7 @@ static struct ctl_table net_core_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &two,
+		.extra2		= SYSCTL_TWO,
 	},
 	{
 		.procname	= "devconf_inherit_init_net",
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index ad80d180b60b..9ff60a389cd0 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -20,10 +20,7 @@
 #include <net/protocol.h>
 #include <net/netevent.h>
 
-static int two = 2;
 static int three __maybe_unused = 3;
-static int four = 4;
-static int thousand = 1000;
 static int tcp_retr1_max = 255;
 static int ip_local_port_range_min[] = { 1, 1 };
 static int ip_local_port_range_max[] = { 65535, 65535 };
@@ -1006,7 +1003,7 @@ static struct ctl_table ipv4_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dou8vec_minmax,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &two,
+		.extra2		= SYSCTL_TWO,
 	},
 	{
 		.procname	= "tcp_max_syn_backlog",
@@ -1117,7 +1114,7 @@ static struct ctl_table ipv4_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dou8vec_minmax,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &four,
+		.extra2		= SYSCTL_FOUR,
 	},
 	{
 		.procname	= "tcp_recovery",
@@ -1310,7 +1307,7 @@ static struct ctl_table ipv4_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &thousand,
+		.extra2		= SYSCTL_ONE_THOUSAND,
 	},
 	{
 		.procname	= "tcp_pacing_ca_ratio",
@@ -1319,7 +1316,7 @@ static struct ctl_table ipv4_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &thousand,
+		.extra2		= SYSCTL_ONE_THOUSAND,
 	},
 	{
 		.procname	= "tcp_wmem",
@@ -1391,7 +1388,7 @@ static struct ctl_table ipv4_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dou8vec_minmax,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &two,
+		.extra2		= SYSCTL_TWO,
 	},
 	{ }
 };
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index d53dd142bf87..560c48d0ddb7 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -23,7 +23,6 @@
 #endif
 #include <linux/ioam6.h>
 
-static int two = 2;
 static int three = 3;
 static int flowlabel_reflect_max = 0x7;
 static int auto_flowlabels_max = IP6_AUTO_FLOW_LABEL_MAX;
@@ -197,7 +196,7 @@ static struct ctl_table ipv6_table_template[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dou8vec_minmax,
 		.extra1         = SYSCTL_ZERO,
-		.extra2         = &two,
+		.extra2         = SYSCTL_TWO,
 	},
 	{
 		.procname	= "ioam6_id",
-- 
2.27.0

[net-next v4 2/3] net: sysctl: introduce sysctl SYSCTL_THREE

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patch introdues the SYSCTL_THREE.

KUnit:
[00:10:14] ================ sysctl_test (10 subtests) =================
[00:10:14] [PASSED] sysctl_test_api_dointvec_null_tbl_data
[00:10:14] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
[00:10:14] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
[00:10:14] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
[00:10:14] [PASSED] sysctl_test_dointvec_read_happy_single_positive
[00:10:14] [PASSED] sysctl_test_dointvec_read_happy_single_negative
[00:10:14] [PASSED] sysctl_test_dointvec_write_happy_single_positive
[00:10:14] [PASSED] sysctl_test_dointvec_write_happy_single_negative
[00:10:14] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
[00:10:14] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
[00:10:14] =================== [PASSED] sysctl_test ===================

./run_kselftest.sh -c sysctl
...
ok 1 selftests: sysctl: sysctl.sh

Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Simon Horman <horms@verge.net.au>
---
 fs/proc/proc_sysctl.c          | 2 +-
 include/linux/sysctl.h         | 9 +++++----
 net/core/sysctl_net_core.c     | 3 +--
 net/ipv4/sysctl_net_ipv4.c     | 3 +--
 net/ipv6/sysctl_net_ipv6.c     | 3 +--
 net/netfilter/ipvs/ip_vs_ctl.c | 4 +---
 6 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 7d9cfc730bd4..5851c2a92c0d 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -26,7 +26,7 @@ static const struct file_operations proc_sys_dir_file_operations;
 static const struct inode_operations proc_sys_dir_operations;
 
 /* shared constants to be used in various sysctls */
-const int sysctl_vals[] = { -1, 0, 1, 2, 4, 100, 200, 1000, 3000, INT_MAX, 65535 };
+const int sysctl_vals[] = { 0, 1, 2, 3, 4, 100, 200, 1000, 3000, INT_MAX, 65535, -1 };
 EXPORT_SYMBOL(sysctl_vals);
 
 const unsigned long sysctl_long_vals[] = { 0, 1, LONG_MAX };
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 6353d6db69b2..80263f7cdb77 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -38,10 +38,10 @@ struct ctl_table_header;
 struct ctl_dir;
 
 /* Keep the same order as in fs/proc/proc_sysctl.c */
-#define SYSCTL_NEG_ONE			((void *)&sysctl_vals[0])
-#define SYSCTL_ZERO			((void *)&sysctl_vals[1])
-#define SYSCTL_ONE			((void *)&sysctl_vals[2])
-#define SYSCTL_TWO			((void *)&sysctl_vals[3])
+#define SYSCTL_ZERO			((void *)&sysctl_vals[0])
+#define SYSCTL_ONE			((void *)&sysctl_vals[1])
+#define SYSCTL_TWO			((void *)&sysctl_vals[2])
+#define SYSCTL_THREE			((void *)&sysctl_vals[3])
 #define SYSCTL_FOUR			((void *)&sysctl_vals[4])
 #define SYSCTL_ONE_HUNDRED		((void *)&sysctl_vals[5])
 #define SYSCTL_TWO_HUNDRED		((void *)&sysctl_vals[6])
@@ -51,6 +51,7 @@ struct ctl_dir;
 
 /* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
 #define SYSCTL_MAXOLDUID		((void *)&sysctl_vals[10])
+#define SYSCTL_NEG_ONE			((void *)&sysctl_vals[11])
 
 extern const int sysctl_vals[];
 
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 3a0ce309ffcd..195ca5c28771 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -25,7 +25,6 @@
 
 #include "dev.h"
 
-static int three = 3;
 static int int_3600 = 3600;
 static int min_sndbuf = SOCK_MIN_SNDBUF;
 static int min_rcvbuf = SOCK_MIN_RCVBUF;
@@ -553,7 +552,7 @@ static struct ctl_table net_core_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &three,
+		.extra2		= SYSCTL_THREE,
 	},
 	{
 		.procname	= "high_order_alloc_disable",
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 9ff60a389cd0..cd448cdd3b38 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -20,7 +20,6 @@
 #include <net/protocol.h>
 #include <net/netevent.h>
 
-static int three __maybe_unused = 3;
 static int tcp_retr1_max = 255;
 static int ip_local_port_range_min[] = { 1, 1 };
 static int ip_local_port_range_max[] = { 65535, 65535 };
@@ -1056,7 +1055,7 @@ static struct ctl_table ipv4_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_fib_multipath_hash_policy,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &three,
+		.extra2		= SYSCTL_THREE,
 	},
 	{
 		.procname	= "fib_multipath_hash_fields",
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 560c48d0ddb7..94a0a294c6a1 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -23,7 +23,6 @@
 #endif
 #include <linux/ioam6.h>
 
-static int three = 3;
 static int flowlabel_reflect_max = 0x7;
 static int auto_flowlabels_max = IP6_AUTO_FLOW_LABEL_MAX;
 static u32 rt6_multipath_hash_fields_all_mask =
@@ -171,7 +170,7 @@ static struct ctl_table ipv6_table_template[] = {
 		.mode		= 0644,
 		.proc_handler   = proc_rt6_multipath_hash_policy,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &three,
+		.extra2		= SYSCTL_THREE,
 	},
 	{
 		.procname	= "fib_multipath_hash_fields",
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 7f645328b47f..efab2b06d373 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1767,8 +1767,6 @@ static int ip_vs_zero_all(struct netns_ipvs *ipvs)
 
 #ifdef CONFIG_SYSCTL
 
-static int three = 3;
-
 static int
 proc_do_defense_mode(struct ctl_table *table, int write,
 		     void *buffer, size_t *lenp, loff_t *ppos)
@@ -1977,7 +1975,7 @@ static struct ctl_table vs_vars[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
-		.extra2		= &three,
+		.extra2		= SYSCTL_THREE,
 	},
 	{
 		.procname	= "nat_icmp_send",
-- 
2.27.0

[net-next v4 3/3] selftests/sysctl: add sysctl macro test

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/test_sysctl.c                        | 21 ++++++++++++
 tools/testing/selftests/sysctl/sysctl.sh | 43 ++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
index a5a3d6c27e1f..43b8d502f4c7 100644
--- a/lib/test_sysctl.c
+++ b/lib/test_sysctl.c
@@ -43,6 +43,7 @@ struct test_sysctl_data {
 	int int_0001;
 	int int_0002;
 	int int_0003[4];
+	int match_int[12];
 
 	int boot_int;
 
@@ -95,6 +96,13 @@ static struct ctl_table test_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "match_int",
+		.data		= &test_data.match_int,
+		.maxlen		= sizeof(test_data.match_int),
+		.mode		= 0444,
+		.proc_handler	= proc_dointvec,
+	},
 	{
 		.procname	= "boot_int",
 		.data		= &test_data.boot_int,
@@ -132,6 +140,19 @@ static struct ctl_table_header *test_sysctl_header;
 
 static int __init test_sysctl_init(void)
 {
+	test_data.match_int[0] = *(int *)SYSCTL_ZERO,
+	test_data.match_int[1] = *(int *)SYSCTL_ONE,
+	test_data.match_int[2] = *(int *)SYSCTL_TWO,
+	test_data.match_int[3] = *(int *)SYSCTL_THREE,
+	test_data.match_int[4] = *(int *)SYSCTL_FOUR,
+	test_data.match_int[5] = *(int *)SYSCTL_ONE_HUNDRED,
+	test_data.match_int[6] = *(int *)SYSCTL_TWO_HUNDRED,
+	test_data.match_int[7] = *(int *)SYSCTL_ONE_THOUSAND,
+	test_data.match_int[8] = *(int *)SYSCTL_THREE_THOUSAND,
+	test_data.match_int[9] = *(int *)SYSCTL_INT_MAX,
+	test_data.match_int[10] = *(int *)SYSCTL_MAXOLDUID,
+	test_data.match_int[11] = *(int *)SYSCTL_NEG_ONE,
+
 	test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL);
 	if (!test_data.bitmap_0001)
 		return -ENOMEM;
diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 19515dcb7d04..cd74f4749748 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -40,6 +40,7 @@ ALL_TESTS="$ALL_TESTS 0004:1:1:uint_0001"
 ALL_TESTS="$ALL_TESTS 0005:3:1:int_0003"
 ALL_TESTS="$ALL_TESTS 0006:50:1:bitmap_0001"
 ALL_TESTS="$ALL_TESTS 0007:1:1:boot_int"
+ALL_TESTS="$ALL_TESTS 0008:1:1:match_int"
 
 function allow_user_defaults()
 {
@@ -785,6 +786,47 @@ sysctl_test_0007()
 	return $ksft_skip
 }
 
+sysctl_test_0008()
+{
+	TARGET="${SYSCTL}/match_int"
+	if [ ! -f $TARGET ]; then
+		echo "Skipping test for $TARGET as it is not present ..."
+		return $ksft_skip
+	fi
+
+	echo -n "Testing if $TARGET is matched with kernel ..."
+	ORIG_VALUES=$(cat "${TARGET}")
+
+	# SYSCTL_ZERO		0
+	# SYSCTL_ONE		1
+	# SYSCTL_TWO		2
+	# SYSCTL_THREE		3
+	# SYSCTL_FOUR		4
+	# SYSCTL_ONE_HUNDRED	100
+	# SYSCTL_TWO_HUNDRED	200
+	# SYSCTL_ONE_THOUSAND	1000
+	# SYSCTL_THREE_THOUSAND 3000
+	# SYSCTL_INT_MAX	INT_MAX
+	# SYSCTL_MAXOLDUID	65535
+	# SYSCTL_NEG_ONE	-1
+	local VALUES=(0 1 2 3 4 100 200 1000 3000 $INT_MAX 65535 -1)
+	idx=0
+
+	for ori in $ORIG_VALUES; do
+		val=${VALUES[$idx]}
+		if [ $ori -ne $val ]; then
+			echo "Expected $val, got $ori, TEST FAILED"
+			rc=1
+			test_rc
+		fi
+
+		let idx=$idx+1
+	done
+
+	echo "ok"
+	return 0
+}
+
 list_tests()
 {
 	echo "Test ID list:"
@@ -800,6 +842,7 @@ list_tests()
 	echo "0005 x $(get_test_count 0005) - tests proc_douintvec() array"
 	echo "0006 x $(get_test_count 0006) - tests proc_do_large_bitmap()"
 	echo "0007 x $(get_test_count 0007) - tests setting sysctl from kernel boot param"
+	echo "0008 x $(get_test_count 0008) - tests sysctl macro values match"
 }
 
 usage()
-- 
2.27.0

Re: [net-next v4 0/3] use standard sysctl macro

[Luis Chamberlain]

On Fri, Apr 22, 2022 at 03:01:38PM +0800, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> This patchset introduce sysctl macro or replace var
> with macro.
> 
> Tonghao Zhang (3):
>   net: sysctl: use shared sysctl macro
>   net: sysctl: introduce sysctl SYSCTL_THREE
>   selftests/sysctl: add sysctl macro test

I see these are based on net-next, to avoid conflicts with
sysctl development this may be best based on sysctl-next
though. Jakub?

  Luis

Re: [net-next v4 0/3] use standard sysctl macro

[Jakub Kicinski]

On Fri, 22 Apr 2022 07:44:12 -0700 Luis Chamberlain wrote:
> On Fri, Apr 22, 2022 at 03:01:38PM +0800, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > 
> > This patchset introduce sysctl macro or replace var
> > with macro.
> > 
> > Tonghao Zhang (3):
> >   net: sysctl: use shared sysctl macro
> >   net: sysctl: introduce sysctl SYSCTL_THREE
> >   selftests/sysctl: add sysctl macro test  
> 
> I see these are based on net-next, to avoid conflicts with
> sysctl development this may be best based on sysctl-next
> though. Jakub?

I guess the base should be whatever we are going to use as
a base for a branch, the branch we can both pull in?

How many patches like that do you see flying around, tho?
I feel like I've seen at least 3 - netfilter, net core and bpf.
It's starting to feel like we should have one patch that adds all 
the constants and self test, put that in a branch anyone can pull in,
and then do the conversions in separate patches..

Option number two - rename the statics in the subsystems to SYSCTL_x,
and we can do a much smaller cleanup in the next cycle which would
replace those with a centralized instances? That should have minimal
chance of conflicts so no need to do special branches.

Option number three defer all this until the merge window.

Re: [net-next v4 0/3] use standard sysctl macro

[Luis Chamberlain]

On Fri, Apr 22, 2022 at 12:43:40PM -0700, Jakub Kicinski wrote:
> On Fri, 22 Apr 2022 07:44:12 -0700 Luis Chamberlain wrote:
> > On Fri, Apr 22, 2022 at 03:01:38PM +0800, xiangxia.m.yue@gmail.com wrote:
> > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > > 
> > > This patchset introduce sysctl macro or replace var
> > > with macro.
> > > 
> > > Tonghao Zhang (3):
> > >   net: sysctl: use shared sysctl macro
> > >   net: sysctl: introduce sysctl SYSCTL_THREE
> > >   selftests/sysctl: add sysctl macro test  
> > 
> > I see these are based on net-next, to avoid conflicts with
> > sysctl development this may be best based on sysctl-next
> > though. Jakub?
> 
> I guess the base should be whatever we are going to use as
> a base for a branch, the branch we can both pull in?
> 
> How many patches like that do you see flying around, tho?
> I feel like I've seen at least 3 - netfilter, net core and bpf.
> It's starting to feel like we should have one patch that adds all 
> the constants and self test, put that in a branch anyone can pull in,
> and then do the conversions in separate patches..
> 
> Option number two - rename the statics in the subsystems to SYSCTL_x,
> and we can do a much smaller cleanup in the next cycle which would
> replace those with a centralized instances? That should have minimal
> chance of conflicts so no need to do special branches.
> 
> Option number three defer all this until the merge window.

I have a better option. I checked to see the diff stat between
the proposed patch to see what the chances of a conflict are
and so far I don't see any conflict so I think this patchset
should just go through your tree.

So feel free to take it in! Let me know if that's OK!

The proposed pathset diffstat:

  fs/proc/proc_sysctl.c                    |  2 +-
  include/linux/sysctl.h                   |  9 ++---
  lib/test_sysctl.c                        | 21 ++++++++++++
  net/core/sysctl_net_core.c               | 13 +++----
  net/ipv4/sysctl_net_ipv4.c               | 16 ++++-----
  net/ipv6/sysctl_net_ipv6.c               |  6 ++--
  net/netfilter/ipvs/ip_vs_ctl.c           |  4 +--
  tools/testing/selftests/sysctl/sysctl.sh | 43 ++++++++++++++++++++++++
  8 files changed, 84 insertions(+), 30 deletions(-)

The sysctl-next diff stat:

 fs/proc/proc_sysctl.c        |  88 ++++++-----
 include/linux/acct.h         |   1 -
 include/linux/delayacct.h    |   3 -
 include/linux/ftrace.h       |   3 -
 include/linux/initrd.h       |   2 -
 include/linux/latencytop.h   |   3 -
 include/linux/lockdep.h      |   4 -
 include/linux/oom.h          |   4 -
 include/linux/panic.h        |   6 -
 include/linux/reboot.h       |   4 -
 include/linux/sched/sysctl.h |  41 -----
 include/linux/writeback.h    |  15 --
 init/do_mounts_initrd.c      |  22 ++-
 kernel/acct.c                |  22 ++-
 kernel/bpf/syscall.c         |  87 ++++++++++
 kernel/delayacct.c           |  22 ++-
 kernel/latencytop.c          |  41 +++--
 kernel/locking/lockdep.c     |  35 ++++-
 kernel/panic.c               |  26 ++-
 kernel/rcu/rcu.h             |   2 +
 kernel/reboot.c              |  34 +++-
 kernel/sched/core.c          |  69 +++++---
 kernel/sched/deadline.c      |  42 ++++-
 kernel/sched/fair.c          |  32 +++-
 kernel/sched/rt.c            |  63 +++++++-
 kernel/sched/sched.h         |   7 +
 kernel/sched/topology.c      |  25 ++-
 kernel/sysctl.c              | 366 -------------------------------------------
 kernel/trace/ftrace.c        | 101 +++++++-----
 mm/oom_kill.c                |  38 ++++-
 mm/page-writeback.c          | 104 ++++++++++--
 31 files changed, 718 insertions(+), 594 deletions(-)

   Luis

Re: [net-next v4 0/3] use standard sysctl macro

[Jakub Kicinski]

On Mon, 25 Apr 2022 12:47:06 -0700 Luis Chamberlain wrote:
> I have a better option. I checked to see the diff stat between
> the proposed patch to see what the chances of a conflict are
> and so far I don't see any conflict so I think this patchset
> should just go through your tree.
> 
> So feel free to take it in! Let me know if that's OK!

Ok, assuming the netfilter and bpf patches I saw were the only other
conversions we can resolve the conflicts before code reaches Linus...

Re: [net-next v4 3/3] selftests/sysctl: add sysctl macro test

[Jakub Kicinski]

On Fri, 22 Apr 2022 15:01:41 +0800 xiangxia.m.yue@gmail.com wrote:
>  static int __init test_sysctl_init(void)
>  {
> +	test_data.match_int[0] = *(int *)SYSCTL_ZERO,
> +	test_data.match_int[1] = *(int *)SYSCTL_ONE,
> +	test_data.match_int[2] = *(int *)SYSCTL_TWO,
> +	test_data.match_int[3] = *(int *)SYSCTL_THREE,
> +	test_data.match_int[4] = *(int *)SYSCTL_FOUR,
> +	test_data.match_int[5] = *(int *)SYSCTL_ONE_HUNDRED,
> +	test_data.match_int[6] = *(int *)SYSCTL_TWO_HUNDRED,
> +	test_data.match_int[7] = *(int *)SYSCTL_ONE_THOUSAND,
> +	test_data.match_int[8] = *(int *)SYSCTL_THREE_THOUSAND,
> +	test_data.match_int[9] = *(int *)SYSCTL_INT_MAX,
> +	test_data.match_int[10] = *(int *)SYSCTL_MAXOLDUID,
> +	test_data.match_int[11] = *(int *)SYSCTL_NEG_ONE,

> +	local VALUES=(0 1 2 3 4 100 200 1000 3000 $INT_MAX 65535 -1)

How does this test work? Am I reading it right that it checks if this
bash array is in sync with the kernel code?

I'd be better if we were checking the values of the constants against
literals / defines.

Re: [net-next v4 0/3] use standard sysctl macro

[Luis Chamberlain]

On Mon, Apr 25, 2022 at 12:56:44PM -0700, Jakub Kicinski wrote:
> On Mon, 25 Apr 2022 12:47:06 -0700 Luis Chamberlain wrote:
> > I have a better option. I checked to see the diff stat between
> > the proposed patch to see what the chances of a conflict are
> > and so far I don't see any conflict so I think this patchset
> > should just go through your tree.
> > 
> > So feel free to take it in! Let me know if that's OK!
> 
> Ok, assuming the netfilter and bpf patches I saw were the only other
> conversions we can resolve the conflicts before code reaches Linus...

Sure thing.

  Luis

Re: [net-next v4 3/3] selftests/sysctl: add sysctl macro test

[Tonghao Zhang]

On Tue, Apr 26, 2022 at 3:58 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 22 Apr 2022 15:01:41 +0800 xiangxia.m.yue@gmail.com wrote:
> >  static int __init test_sysctl_init(void)
> >  {
> > +     test_data.match_int[0] = *(int *)SYSCTL_ZERO,
> > +     test_data.match_int[1] = *(int *)SYSCTL_ONE,
> > +     test_data.match_int[2] = *(int *)SYSCTL_TWO,
> > +     test_data.match_int[3] = *(int *)SYSCTL_THREE,
> > +     test_data.match_int[4] = *(int *)SYSCTL_FOUR,
> > +     test_data.match_int[5] = *(int *)SYSCTL_ONE_HUNDRED,
> > +     test_data.match_int[6] = *(int *)SYSCTL_TWO_HUNDRED,
> > +     test_data.match_int[7] = *(int *)SYSCTL_ONE_THOUSAND,
> > +     test_data.match_int[8] = *(int *)SYSCTL_THREE_THOUSAND,
> > +     test_data.match_int[9] = *(int *)SYSCTL_INT_MAX,
> > +     test_data.match_int[10] = *(int *)SYSCTL_MAXOLDUID,
> > +     test_data.match_int[11] = *(int *)SYSCTL_NEG_ONE,
>
> > +     local VALUES=(0 1 2 3 4 100 200 1000 3000 $INT_MAX 65535 -1)
>
> How does this test work? Am I reading it right that it checks if this
> bash array is in sync with the kernel code?
This patch tries to avoid SYSCTL_XXX not mapping the values we hoped,
when introducing the new SYSCTL_YYY.
for example:
SYSCTL_TWO, we hope it is 2, so we check it in userspace.
> I'd be better if we were checking the values of the constants against
> literals / defines.
Hi Jakub, I think this patch checks the values of the constants
against defines. But I should make the codes more readable

 static int __init test_sysctl_init(void)
 {
+       int i;
+
+       struct {
+               int defined;
+               int wanted;
+       } match_int[] = {
+               {.defined = *(int *)SYSCTL_ZERO,        .wanted = 0},
+               {.defined = *(int *)SYSCTL_ONE,         .wanted = 1},
+               {.defined = *(int *)SYSCTL_TWO,         .wanted = 2},
+               {.defined = *(int *)SYSCTL_THREE,       .wanted = 3},
+               {.defined = *(int *)SYSCTL_FOUR,        .wanted = 4},
+               {.defined = *(int *)SYSCTL_ONE_HUNDRED, .wanted = 100},
+               {.defined = *(int *)SYSCTL_TWO_HUNDRED, .wanted = 200},
+               {.defined = *(int *)SYSCTL_ONE_THOUSAND, .wanted = 1000},
+               {.defined = *(int *)SYSCTL_THREE_THOUSAND, .wanted = 3000},
+               {.defined = *(int *)SYSCTL_INT_MAX,     .wanted = INT_MAX},
+               {.defined = *(int *)SYSCTL_MAXOLDUID,   .wanted = 65535},
+               {.defined = *(int *)SYSCTL_NEG_ONE,     .wanted = -1},
+       };
+
+       for (i = 0; i < ARRAY_SIZE(match_int); i++)
+               if (match_int[i].defined != match_int[i].wanted)
+                       match_int_ok = 0;
+




--
Best regards, Tonghao

Re: [net-next v4 3/3] selftests/sysctl: add sysctl macro test

[Jakub Kicinski]

On Sun, 1 May 2022 11:31:47 +0800 Tonghao Zhang wrote:
>  static int __init test_sysctl_init(void)
>  {
> +       int i;
> +

nit: No empty line needed here.

> +       struct {
> +               int defined;
> +               int wanted;
> +       } match_int[] = {
> +               {.defined = *(int *)SYSCTL_ZERO,        .wanted = 0},
> +               {.defined = *(int *)SYSCTL_ONE,         .wanted = 1},
> +               {.defined = *(int *)SYSCTL_TWO,         .wanted = 2},
> +               {.defined = *(int *)SYSCTL_THREE,       .wanted = 3},
> +               {.defined = *(int *)SYSCTL_FOUR,        .wanted = 4},
> +               {.defined = *(int *)SYSCTL_ONE_HUNDRED, .wanted = 100},
> +               {.defined = *(int *)SYSCTL_TWO_HUNDRED, .wanted = 200},
> +               {.defined = *(int *)SYSCTL_ONE_THOUSAND, .wanted = 1000},
> +               {.defined = *(int *)SYSCTL_THREE_THOUSAND, .wanted = 3000},
> +               {.defined = *(int *)SYSCTL_INT_MAX,     .wanted = INT_MAX},
> +               {.defined = *(int *)SYSCTL_MAXOLDUID,   .wanted = 65535},
> +               {.defined = *(int *)SYSCTL_NEG_ONE,     .wanted = -1},
> +       };
> +
> +       for (i = 0; i < ARRAY_SIZE(match_int); i++)
> +               if (match_int[i].defined != match_int[i].wanted)
> +                       match_int_ok = 0;

That's better, thank you!