[net-next RESEND v2] net: core: use shared sysctl macro

[net-next RESEND v2] net: core: use shared sysctl macro

[xiangxia.m.yue]

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

This patch introdues the SYSCTL_THREE, and replace the
two, three and long_one to SYSCTL_XXX accordingly.

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

 ./run_kselftest.sh -c sysctl
 ...
 # Running test: sysctl_test_0006 - run #49
 # Checking bitmap handler... ok
 # Wed Mar 16 14:58:41 UTC 2022
 # Running test: sysctl_test_0007 - run #0
 # Boot param test only possible sysctl_test is built-in, not module:
 # CONFIG_TEST_SYSCTL=m
 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: 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: Dmitry Vyukov <dvyukov@google.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 fs/proc/proc_sysctl.c          |  2 +-
 include/linux/sysctl.h         | 13 +++++++------
 net/core/sysctl_net_core.c     | 14 +++++---------
 net/ipv4/sysctl_net_ipv4.c     | 16 ++++++----------
 net/ipv6/sysctl_net_ipv6.c     |  6 ++----
 net/netfilter/ipvs/ip_vs_ctl.c |  4 +---
 6 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 7d9cfc730bd4..0bdd9249666b 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[] = { -1, 0, 1, 2, 3, 4, 100, 200, 1000, 3000, INT_MAX, 65535 };
 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..b2ac6542455f 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -42,12 +42,13 @@ struct ctl_dir;
 #define SYSCTL_ZERO			((void *)&sysctl_vals[1])
 #define SYSCTL_ONE			((void *)&sysctl_vals[2])
 #define SYSCTL_TWO			((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])
-#define SYSCTL_ONE_THOUSAND		((void *)&sysctl_vals[7])
-#define SYSCTL_THREE_THOUSAND		((void *)&sysctl_vals[8])
-#define SYSCTL_INT_MAX			((void *)&sysctl_vals[9])
+#define SYSCTL_THREE			((void *)&sysctl_vals[4])
+#define SYSCTL_FOUR			((void *)&sysctl_vals[5])
+#define SYSCTL_ONE_HUNDRED		((void *)&sysctl_vals[6])
+#define SYSCTL_TWO_HUNDRED		((void *)&sysctl_vals[7])
+#define SYSCTL_ONE_THOUSAND		((void *)&sysctl_vals[8])
+#define SYSCTL_THREE_THOUSAND		((void *)&sysctl_vals[9])
+#define SYSCTL_INT_MAX			((void *)&sysctl_vals[10])
 
 /* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
 #define SYSCTL_MAXOLDUID		((void *)&sysctl_vals[10])
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 7123fe7feeac..6ea51c155860 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -23,14 +23,10 @@
 #include <net/busy_poll.h>
 #include <net/pkt_sched.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 */
 
@@ -388,7 +384,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
@@ -399,7 +395,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",
@@ -417,7 +413,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
@@ -544,7 +540,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",
@@ -553,7 +549,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 ad80d180b60b..cd448cdd3b38 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -20,10 +20,6 @@
 #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 +1002,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",
@@ -1059,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",
@@ -1117,7 +1113,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 +1306,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 +1315,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 +1387,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..94a0a294c6a1 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -23,8 +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;
 static u32 rt6_multipath_hash_fields_all_mask =
@@ -172,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",
@@ -197,7 +195,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",
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

Re: [net-next RESEND v2] net: core: use shared sysctl macro

[Luis Chamberlain]

On Wed, Apr 06, 2022 at 08:42:08PM +0800, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> This patch introdues the SYSCTL_THREE, and replace the
> two, three and long_one to SYSCTL_XXX accordingly.
> 
>  KUnit:
>  [23:03:58] ================ sysctl_test (10 subtests) =================
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_null_tbl_data
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
>  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_positive
>  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_negative
>  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_positive
>  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_negative
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
>  [23:03:58] =================== [PASSED] sysctl_test ===================
> 
>  ./run_kselftest.sh -c sysctl
>  ...
>  # Running test: sysctl_test_0006 - run #49
>  # Checking bitmap handler... ok
>  # Wed Mar 16 14:58:41 UTC 2022
>  # Running test: sysctl_test_0007 - run #0
>  # Boot param test only possible sysctl_test is built-in, not module:
>  # CONFIG_TEST_SYSCTL=m
>  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: 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: Dmitry Vyukov <dvyukov@google.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Lorenz Bauer <lmb@cloudflare.com>
> Cc: Akhmat Karakotov <hmukos@yandex-team.ru>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

I can take this through sysctl-next [0] if folks are OK with that. There are
quite a bit of changes already queued there for sysctl.

Jakub?

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=sysctl-next

  Luis

Re: [net-next RESEND v2] net: core: use shared sysctl macro

[Jakub Kicinski]

On Wed, 6 Apr 2022 09:20:24 -0700 Luis Chamberlain wrote:
> On Wed, Apr 06, 2022 at 08:42:08PM +0800, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > 
> > This patch introdues the SYSCTL_THREE, and replace the
> > two, three and long_one to SYSCTL_XXX accordingly.
> > 
> >  KUnit:
> >  [23:03:58] ================ sysctl_test (10 subtests) =================
> >  [23:03:58] [PASSED] sysctl_test_api_dointvec_null_tbl_data
> >  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
> >  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
> >  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
> >  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_positive
> >  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_negative
> >  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_positive
> >  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_negative
> >  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
> >  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
> >  [23:03:58] =================== [PASSED] sysctl_test ===================
> > 
> >  ./run_kselftest.sh -c sysctl
> >  ...
> >  # Running test: sysctl_test_0006 - run #49
> >  # Checking bitmap handler... ok
> >  # Wed Mar 16 14:58:41 UTC 2022
> >  # Running test: sysctl_test_0007 - run #0
> >  # Boot param test only possible sysctl_test is built-in, not module:
> >  # CONFIG_TEST_SYSCTL=m
> >  ok 1 selftests: sysctl: sysctl.sh
>
> I can take this through sysctl-next [0] if folks are OK with that. There are
> quite a bit of changes already queued there for sysctl.
> 
> Jakub?
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=sysctl-next

sysctl-next makes a lot of sense, but I'm worried about conflicts.
Would you be able to spin up a stable branch based on -rc1 so we
can pull it into net-next as well?

Let me take a look at the patch as well...

Re: [net-next RESEND v2] net: core: use shared sysctl macro

[Jakub Kicinski]

On Wed,  6 Apr 2022 20:42:08 +0800 xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> This patch introdues the SYSCTL_THREE, and replace the
> two, three and long_one to SYSCTL_XXX accordingly.
> 
>  KUnit:
>  [23:03:58] ================ sysctl_test (10 subtests) =================
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_null_tbl_data
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
>  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_positive
>  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_negative
>  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_positive
>  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_negative
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
>  [23:03:58] =================== [PASSED] sysctl_test ===================
> 
>  ./run_kselftest.sh -c sysctl
>  ...
>  # Running test: sysctl_test_0006 - run #49
>  # Checking bitmap handler... ok
>  # Wed Mar 16 14:58:41 UTC 2022
>  # Running test: sysctl_test_0007 - run #0
>  # Boot param test only possible sysctl_test is built-in, not module:
>  # CONFIG_TEST_SYSCTL=m
>  ok 1 selftests: sysctl: sysctl.sh

> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 7d9cfc730bd4..0bdd9249666b 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[] = { -1, 0, 1, 2, 3, 4, 100, 200, 1000, 3000, INT_MAX, 65535 };
>  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..b2ac6542455f 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -42,12 +42,13 @@ struct ctl_dir;
>  #define SYSCTL_ZERO			((void *)&sysctl_vals[1])
>  #define SYSCTL_ONE			((void *)&sysctl_vals[2])
>  #define SYSCTL_TWO			((void *)&sysctl_vals[3])
> -#define SYSCTL_FOUR			((void *)&sysctl_vals[4])

nit: I vote we move the -1 later in the array, so that for the first
5 "natural" numbers the index matches the value.

> -#define SYSCTL_ONE_HUNDRED		((void *)&sysctl_vals[5])
> -#define SYSCTL_TWO_HUNDRED		((void *)&sysctl_vals[6])
> -#define SYSCTL_ONE_THOUSAND		((void *)&sysctl_vals[7])
> -#define SYSCTL_THREE_THOUSAND		((void *)&sysctl_vals[8])
> -#define SYSCTL_INT_MAX			((void *)&sysctl_vals[9])
> +#define SYSCTL_THREE			((void *)&sysctl_vals[4])
> +#define SYSCTL_FOUR			((void *)&sysctl_vals[5])
> +#define SYSCTL_ONE_HUNDRED		((void *)&sysctl_vals[6])
> +#define SYSCTL_TWO_HUNDRED		((void *)&sysctl_vals[7])
> +#define SYSCTL_ONE_THOUSAND		((void *)&sysctl_vals[8])
> +#define SYSCTL_THREE_THOUSAND		((void *)&sysctl_vals[9])
> +#define SYSCTL_INT_MAX			((void *)&sysctl_vals[10])

> @@ -388,7 +384,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
> @@ -399,7 +395,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",
> @@ -417,7 +413,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,

Hm, looks like most of the conversions are not to the newly added value
of three. Feels like those should be a separate patch.

Re: [net-next RESEND v2] net: core: use shared sysctl macro

[Luis Chamberlain]

On Wed, Apr 06, 2022 at 12:16:11PM -0700, Jakub Kicinski wrote:
> On Wed, 6 Apr 2022 09:20:24 -0700 Luis Chamberlain wrote:
> > On Wed, Apr 06, 2022 at 08:42:08PM +0800, xiangxia.m.yue@gmail.com wrote:
> > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > > 
> > > This patch introdues the SYSCTL_THREE, and replace the
> > > two, three and long_one to SYSCTL_XXX accordingly.
> > > 
> > >  KUnit:
> > >  [23:03:58] ================ sysctl_test (10 subtests) =================
> > >  [23:03:58] [PASSED] sysctl_test_api_dointvec_null_tbl_data
> > >  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
> > >  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
> > >  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
> > >  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_positive
> > >  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_negative
> > >  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_positive
> > >  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_negative
> > >  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
> > >  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
> > >  [23:03:58] =================== [PASSED] sysctl_test ===================
> > > 
> > >  ./run_kselftest.sh -c sysctl
> > >  ...
> > >  # Running test: sysctl_test_0006 - run #49
> > >  # Checking bitmap handler... ok
> > >  # Wed Mar 16 14:58:41 UTC 2022
> > >  # Running test: sysctl_test_0007 - run #0
> > >  # Boot param test only possible sysctl_test is built-in, not module:
> > >  # CONFIG_TEST_SYSCTL=m
> > >  ok 1 selftests: sysctl: sysctl.sh
> >
> > I can take this through sysctl-next [0] if folks are OK with that. There are
> > quite a bit of changes already queued there for sysctl.
> > 
> > Jakub?
> > 
> > [0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=sysctl-next
> 
> sysctl-next makes a lot of sense, but I'm worried about conflicts.

I can try to deal with them as I can send the pull request to Linus towards
the end of the merge window.

> Would you be able to spin up a stable branch based on -rc1 so we
> can pull it into net-next as well?

Yes, absolutely. Just pushed, but I should note that linux-next already
takes in sysctl-next. And there are non-networking changes that are
in sysctl-next. Does net-next go to Linus or is it just to help with
developers so they get something more close to linux-next but not as
insane?

  Luis

Re: [net-next RESEND v2] net: core: use shared sysctl macro

[Jakub Kicinski]

On Wed, 6 Apr 2022 13:46:02 -0700 Luis Chamberlain wrote:
> > sysctl-next makes a lot of sense, but I'm worried about conflicts.  
> 
> I can try to deal with them as I can send the pull request to Linus towards
> the end of the merge window.

Do you mean that your -next branch is unstable and can be rebased?
Often people keep their -next branches stable, and then only Linus 
can deal with the conflict (with linux-next's help).

> > Would you be able to spin up a stable branch based on -rc1 so we
> > can pull it into net-next as well?  
> 
> Yes, absolutely. Just pushed, but I should note that linux-next already
> takes in sysctl-next. And there are non-networking changes that are
> in sysctl-next. 

What I mean by a stable branch is a separate branch on top of -rc1 with
just this patch/series, which we can pull into net-next and you can
pull into sysctl-next. That way this change will appear with the same
commit id in both trees and git will deal with it smoothly.

> Does net-next go to Linus or is it just to help with
> developers so they get something more close to linux-next but not as
> insane?

net-next goes to Linus and it's "stable" by which I mean no rebasing 
or hard pushing.

Re: [net-next RESEND v2] net: core: use shared sysctl macro

[Luis Chamberlain]

On Wed, Apr 06, 2022 at 02:06:29PM -0700, Jakub Kicinski wrote:
> On Wed, 6 Apr 2022 13:46:02 -0700 Luis Chamberlain wrote:
> > > sysctl-next makes a lot of sense, but I'm worried about conflicts.  
> > 
> > I can try to deal with them as I can send the pull request to Linus towards
> > the end of the merge window.
> 
> Do you mean that your -next branch is unstable and can be rebased?

Yes as I had no need / many users other than to get testing done before I
send a pull requst to Linus. The other users are developers sending
random updates with the latest efforts to clean up kernel/sysctl.c and
these sorts of changes. But it would seem that should stop and I should
make it stable.

> Often people keep their -next branches stable, and then only Linus 
> can deal with the conflict (with linux-next's help).

I missed the v5.18 merge window and I hadn't rebased after Linus
released v5.18-rc1 and so I had to rebase now either way too.

> > > Would you be able to spin up a stable branch based on -rc1 so we
> > > can pull it into net-next as well?  
> > 
> > Yes, absolutely. Just pushed, but I should note that linux-next already
> > takes in sysctl-next. And there are non-networking changes that are
> > in sysctl-next. 
> 
> What I mean by a stable branch is a separate branch on top of -rc1 with
> just this patch/series, which we can pull into net-next and you can
> pull into sysctl-next. That way this change will appear with the same
> commit id in both trees and git will deal with it smoothly.

Ah yes, I hadn't needed to make sysctl-next stable but indeed it would
make sense given what you are indicating.
> 
> > Does net-next go to Linus or is it just to help with
> > developers so they get something more close to linux-next but not as
> > insane?
> 
> net-next goes to Linus and it's "stable" by which I mean no rebasing 
> or hard pushing.

I can commit to making sysctl-next to make this coordination easier. It
make sense.

  Luis

Re: [net-next RESEND v2] net: core: use shared sysctl macro

[Alexander Lobakin]

From: xiangxia.m.yue@gmail.com
Date: Wed,  6 Apr 2022 20:42:08 +0800

> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> This patch introdues the SYSCTL_THREE, and replace the
> two, three and long_one to SYSCTL_XXX accordingly.
> 
>  KUnit:
>  [23:03:58] ================ sysctl_test (10 subtests) =================
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_null_tbl_data
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
>  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_positive
>  [23:03:58] [PASSED] sysctl_test_dointvec_read_happy_single_negative
>  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_positive
>  [23:03:58] [PASSED] sysctl_test_dointvec_write_happy_single_negative
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
>  [23:03:58] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
>  [23:03:58] =================== [PASSED] sysctl_test ===================
> 
>  ./run_kselftest.sh -c sysctl
>  ...

--- 8< ---

> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 7d9cfc730bd4..0bdd9249666b 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[] = { -1, 0, 1, 2, 3, 4, 100, 200, 1000, 3000, INT_MAX, 65535 };
>  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..b2ac6542455f 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -42,12 +42,13 @@ struct ctl_dir;
>  #define SYSCTL_ZERO			((void *)&sysctl_vals[1])
>  #define SYSCTL_ONE			((void *)&sysctl_vals[2])
>  #define SYSCTL_TWO			((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])
> -#define SYSCTL_ONE_THOUSAND		((void *)&sysctl_vals[7])
> -#define SYSCTL_THREE_THOUSAND		((void *)&sysctl_vals[8])
> -#define SYSCTL_INT_MAX			((void *)&sysctl_vals[9])
> +#define SYSCTL_THREE			((void *)&sysctl_vals[4])
> +#define SYSCTL_FOUR			((void *)&sysctl_vals[5])
> +#define SYSCTL_ONE_HUNDRED		((void *)&sysctl_vals[6])
> +#define SYSCTL_TWO_HUNDRED		((void *)&sysctl_vals[7])
> +#define SYSCTL_ONE_THOUSAND		((void *)&sysctl_vals[8])
> +#define SYSCTL_THREE_THOUSAND		((void *)&sysctl_vals[9])
> +#define SYSCTL_INT_MAX			((void *)&sysctl_vals[10])
>  
>  /* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
>  #define SYSCTL_MAXOLDUID		((void *)&sysctl_vals[10])

You forgot to change this one. It should point to 65535, i.e.
&sysctl_vals[11] from your adjusted array.

Maybe it's better to add new constants simply to the tail of the
array? To not adjust it each time and give a room for mistakes.

> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 7123fe7feeac..6ea51c155860 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c

--- 8< ---

> -- 
> 2.27.0

Thanks,
Al