[PATCH 05/14] sysctl: Add a size arg to __register_sysctl_table

[PATCH 05/14] sysctl: Add a size arg to __register_sysctl_table

[Joel Granados]

This is part of the effort to remove the sentinel element in the
ctl_table arrays. We add a table_size argument to
__register_sysctl_table and adjust callers, all of which pass ctl_table
pointers and need an explicit call to ARRAY_SIZE.

The new table_size argument does not yet have any effect in the
init_header call which is still dependent on the sentinel's presence.
table_size *does* however drive the `kzalloc` allocation in
__register_sysctl_table with no adverse effects as the allocated memory
is either one element greater than the calculated ctl_table array (for
the calls in ipc_sysctl.c, mq_sysctl.c and ucount.c) or the exact size
of the calculated ctl_table array (for the call from sysctl_net.c and
register_sysctl). This approach will allows us to "just" remove the
sentinel without further changes to __register_sysctl_table as
table_size will represent the exact size for all the callers at that
point.

Temporarily implement a size calculation in register_net_sysctl, which
is an indirection call for all the network register calls.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 fs/proc/proc_sysctl.c  | 22 +++++++++++-----------
 include/linux/sysctl.h |  2 +-
 ipc/ipc_sysctl.c       |  4 +++-
 ipc/mq_sysctl.c        |  4 +++-
 kernel/ucount.c        |  3 ++-
 net/sysctl_net.c       |  8 +++++++-
 6 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index fa1438f1a355..8d04f01a89c1 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1354,27 +1354,20 @@ static struct ctl_dir *sysctl_mkdir_p(struct ctl_dir *dir, const char *path)
  */
 struct ctl_table_header *__register_sysctl_table(
 	struct ctl_table_set *set,
-	const char *path, struct ctl_table *table)
+	const char *path, struct ctl_table *table, size_t table_size)
 {
 	struct ctl_table_root *root = set->dir.header.root;
 	struct ctl_table_header *header;
-	struct ctl_table_header h_tmp;
 	struct ctl_dir *dir;
-	struct ctl_table *entry;
 	struct ctl_node *node;
-	int nr_entries = 0;
-
-	h_tmp.ctl_table = table;
-	list_for_each_table_entry(entry, (&h_tmp))
-		nr_entries++;
 
 	header = kzalloc(sizeof(struct ctl_table_header) +
-			 sizeof(struct ctl_node)*nr_entries, GFP_KERNEL_ACCOUNT);
+			 sizeof(struct ctl_node)*table_size, GFP_KERNEL_ACCOUNT);
 	if (!header)
 		return NULL;
 
 	node = (struct ctl_node *)(header + 1);
-	init_header(header, root, set, node, table, nr_entries);
+	init_header(header, root, set, node, table, table_size);
 	if (sysctl_check_table(path, header))
 		goto fail;
 
@@ -1423,8 +1416,15 @@ struct ctl_table_header *__register_sysctl_table(
  */
 struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table)
 {
+	int count = 0;
+	struct ctl_table *entry;
+	struct ctl_table_header t_hdr;
+
+	t_hdr.ctl_table = table;
+	list_for_each_table_entry(entry, (&t_hdr))
+		count++;
 	return __register_sysctl_table(&sysctl_table_root.default_set,
-					path, table);
+					path, table, count);
 }
 EXPORT_SYMBOL(register_sysctl);
 
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 33252ad58ebe..0495c858989f 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -226,7 +226,7 @@ extern void retire_sysctl_set(struct ctl_table_set *set);
 
 struct ctl_table_header *__register_sysctl_table(
 	struct ctl_table_set *set,
-	const char *path, struct ctl_table *table);
+	const char *path, struct ctl_table *table, size_t table_size);
 struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table);
 void unregister_sysctl_table(struct ctl_table_header * table);
 
diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index ef313ecfb53a..8c62e443f78b 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -259,7 +259,9 @@ bool setup_ipc_sysctls(struct ipc_namespace *ns)
 				tbl[i].data = NULL;
 		}
 
-		ns->ipc_sysctls = __register_sysctl_table(&ns->ipc_set, "kernel", tbl);
+		ns->ipc_sysctls = __register_sysctl_table(&ns->ipc_set,
+							  "kernel", tbl,
+							  ARRAY_SIZE(ipc_sysctls));
 	}
 	if (!ns->ipc_sysctls) {
 		kfree(tbl);
diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
index fbf6a8b93a26..ebb5ed81c151 100644
--- a/ipc/mq_sysctl.c
+++ b/ipc/mq_sysctl.c
@@ -109,7 +109,9 @@ bool setup_mq_sysctls(struct ipc_namespace *ns)
 				tbl[i].data = NULL;
 		}
 
-		ns->mq_sysctls = __register_sysctl_table(&ns->mq_set, "fs/mqueue", tbl);
+		ns->mq_sysctls = __register_sysctl_table(&ns->mq_set,
+							 "fs/mqueue", tbl,
+							 ARRAY_SIZE(mq_sysctls));
 	}
 	if (!ns->mq_sysctls) {
 		kfree(tbl);
diff --git a/kernel/ucount.c b/kernel/ucount.c
index ee8e57fd6f90..2b80264bb79f 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -104,7 +104,8 @@ bool setup_userns_sysctls(struct user_namespace *ns)
 		for (i = 0; i < UCOUNT_COUNTS; i++) {
 			tbl[i].data = &ns->ucount_max[i];
 		}
-		ns->sysctls = __register_sysctl_table(&ns->set, "user", tbl);
+		ns->sysctls = __register_sysctl_table(&ns->set, "user", tbl,
+						      ARRAY_SIZE(user_table));
 	}
 	if (!ns->sysctls) {
 		kfree(tbl);
diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index 4b45ed631eb8..8ee4b74bc009 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -163,10 +163,16 @@ static void ensure_safe_net_sysctl(struct net *net, const char *path,
 struct ctl_table_header *register_net_sysctl(struct net *net,
 	const char *path, struct ctl_table *table)
 {
+	int count = 0;
+	struct ctl_table *entry;
+
 	if (!net_eq(net, &init_net))
 		ensure_safe_net_sysctl(net, path, table);
 
-	return __register_sysctl_table(&net->sysctls, path, table);
+	for (entry = table; entry->procname; entry++)
+		count++;
+
+	return __register_sysctl_table(&net->sysctls, path, table, count);
 }
 EXPORT_SYMBOL_GPL(register_net_sysctl);
 
-- 
2.30.2

Re: [PATCH 05/14] sysctl: Add a size arg to __register_sysctl_table

[Simon Horman]

On Wed, Jul 26, 2023 at 04:06:25PM +0200, Joel Granados wrote:
> This is part of the effort to remove the sentinel element in the
> ctl_table arrays. We add a table_size argument to
> __register_sysctl_table and adjust callers, all of which pass ctl_table
> pointers and need an explicit call to ARRAY_SIZE.
> 
> The new table_size argument does not yet have any effect in the
> init_header call which is still dependent on the sentinel's presence.
> table_size *does* however drive the `kzalloc` allocation in
> __register_sysctl_table with no adverse effects as the allocated memory
> is either one element greater than the calculated ctl_table array (for
> the calls in ipc_sysctl.c, mq_sysctl.c and ucount.c) or the exact size
> of the calculated ctl_table array (for the call from sysctl_net.c and
> register_sysctl). This approach will allows us to "just" remove the
> sentinel without further changes to __register_sysctl_table as
> table_size will represent the exact size for all the callers at that
> point.
> 
> Temporarily implement a size calculation in register_net_sysctl, which
> is an indirection call for all the network register calls.
> 
> Signed-off-by: Joel Granados <j.granados@samsung.com>
> ---
>  fs/proc/proc_sysctl.c  | 22 +++++++++++-----------
>  include/linux/sysctl.h |  2 +-
>  ipc/ipc_sysctl.c       |  4 +++-
>  ipc/mq_sysctl.c        |  4 +++-
>  kernel/ucount.c        |  3 ++-
>  net/sysctl_net.c       |  8 +++++++-
>  6 files changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index fa1438f1a355..8d04f01a89c1 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -1354,27 +1354,20 @@ static struct ctl_dir *sysctl_mkdir_p(struct ctl_dir *dir, const char *path)
>   */
>  struct ctl_table_header *__register_sysctl_table(
>  	struct ctl_table_set *set,
> -	const char *path, struct ctl_table *table)
> +	const char *path, struct ctl_table *table, size_t table_size)

Hi Joel,

Please consider adding table_size to the kernel doc for this function.

...

Re: [PATCH 05/14] sysctl: Add a size arg to __register_sysctl_table

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2326 bytes --]

On Fri, Jul 28, 2023 at 12:51:22PM +0200, Simon Horman wrote:
> On Wed, Jul 26, 2023 at 04:06:25PM +0200, Joel Granados wrote:
> > This is part of the effort to remove the sentinel element in the
> > ctl_table arrays. We add a table_size argument to
> > __register_sysctl_table and adjust callers, all of which pass ctl_table
> > pointers and need an explicit call to ARRAY_SIZE.
> > 
> > The new table_size argument does not yet have any effect in the
> > init_header call which is still dependent on the sentinel's presence.
> > table_size *does* however drive the `kzalloc` allocation in
> > __register_sysctl_table with no adverse effects as the allocated memory
> > is either one element greater than the calculated ctl_table array (for
> > the calls in ipc_sysctl.c, mq_sysctl.c and ucount.c) or the exact size
> > of the calculated ctl_table array (for the call from sysctl_net.c and
> > register_sysctl). This approach will allows us to "just" remove the
> > sentinel without further changes to __register_sysctl_table as
> > table_size will represent the exact size for all the callers at that
> > point.
> > 
> > Temporarily implement a size calculation in register_net_sysctl, which
> > is an indirection call for all the network register calls.
> > 
> > Signed-off-by: Joel Granados <j.granados@samsung.com>
> > ---
> >  fs/proc/proc_sysctl.c  | 22 +++++++++++-----------
> >  include/linux/sysctl.h |  2 +-
> >  ipc/ipc_sysctl.c       |  4 +++-
> >  ipc/mq_sysctl.c        |  4 +++-
> >  kernel/ucount.c        |  3 ++-
> >  net/sysctl_net.c       |  8 +++++++-
> >  6 files changed, 27 insertions(+), 16 deletions(-)
> > 
> > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> > index fa1438f1a355..8d04f01a89c1 100644
> > --- a/fs/proc/proc_sysctl.c
> > +++ b/fs/proc/proc_sysctl.c
> > @@ -1354,27 +1354,20 @@ static struct ctl_dir *sysctl_mkdir_p(struct ctl_dir *dir, const char *path)
> >   */
> >  struct ctl_table_header *__register_sysctl_table(
> >  	struct ctl_table_set *set,
> > -	const char *path, struct ctl_table *table)
> > +	const char *path, struct ctl_table *table, size_t table_size)
> 
> Hi Joel,
> 
> Please consider adding table_size to the kernel doc for this function.
Good catch. Will do for V2.
> 
> ...

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 06/14] sysctl: Add size to register_sysctl

[Joel Granados]

In order to remove the end element from the ctl_table struct arrays, we
replace the register_syctl function with a macro that will add the
ARRAY_SIZE to the new register_sysctl_sz function. In this way the
callers that are already using an array of ctl_table structs do not have
to change. We *do* change the callers that pass the ctl_table array as a
pointer.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 arch/arm64/kernel/armv8_deprecated.c |  2 +-
 arch/s390/appldata/appldata_base.c   |  2 +-
 fs/proc/proc_sysctl.c                | 30 +++++++++++++++-------------
 include/linux/sysctl.h               | 10 ++++++++--
 kernel/ucount.c                      |  2 +-
 net/sysctl_net.c                     |  2 +-
 6 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 1febd412b4d2..e459cfd33711 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -569,7 +569,7 @@ static void __init register_insn_emulation(struct insn_emulation *insn)
 		sysctl->extra2 = &insn->max;
 		sysctl->proc_handler = emulation_proc_handler;
 
-		register_sysctl("abi", sysctl);
+		register_sysctl_sz("abi", sysctl, 1);
 	}
 }
 
diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c
index bbefe5e86bdf..3b0994625652 100644
--- a/arch/s390/appldata/appldata_base.c
+++ b/arch/s390/appldata/appldata_base.c
@@ -365,7 +365,7 @@ int appldata_register_ops(struct appldata_ops *ops)
 	ops->ctl_table[0].proc_handler = appldata_generic_handler;
 	ops->ctl_table[0].data = ops;
 
-	ops->sysctl_header = register_sysctl(appldata_proc_name, ops->ctl_table);
+	ops->sysctl_header = register_sysctl_sz(appldata_proc_name, ops->ctl_table, 1);
 	if (!ops->sysctl_header)
 		goto out;
 	return 0;
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 8d04f01a89c1..c04293911e7e 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -43,7 +43,7 @@ static struct ctl_table sysctl_mount_point[] = {
  */
 struct ctl_table_header *register_sysctl_mount_point(const char *path)
 {
-	return register_sysctl(path, sysctl_mount_point);
+	return register_sysctl_sz(path, sysctl_mount_point, 0);
 }
 EXPORT_SYMBOL(register_sysctl_mount_point);
 
@@ -1398,7 +1398,7 @@ struct ctl_table_header *__register_sysctl_table(
 }
 
 /**
- * register_sysctl - register a sysctl table
+ * register_sysctl_sz - register a sysctl table
  * @path: The path to the directory the sysctl table is in. If the path
  * 	doesn't exist we will create it for you.
  * @table: the table structure. The calller must ensure the life of the @table
@@ -1408,25 +1408,20 @@ struct ctl_table_header *__register_sysctl_table(
  * 	to call unregister_sysctl_table() and can instead use something like
  * 	register_sysctl_init() which does not care for the result of the syctl
  * 	registration.
+ * @table_size: The number of elements in table.
  *
  * Register a sysctl table. @table should be a filled in ctl_table
  * array. A completely 0 filled entry terminates the table.
  *
  * See __register_sysctl_table for more details.
  */
-struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table)
+struct ctl_table_header *register_sysctl_sz(const char *path, struct ctl_table *table,
+					    size_t table_size)
 {
-	int count = 0;
-	struct ctl_table *entry;
-	struct ctl_table_header t_hdr;
-
-	t_hdr.ctl_table = table;
-	list_for_each_table_entry(entry, (&t_hdr))
-		count++;
 	return __register_sysctl_table(&sysctl_table_root.default_set,
-					path, table, count);
+					path, table, table_size);
 }
-EXPORT_SYMBOL(register_sysctl);
+EXPORT_SYMBOL(register_sysctl_sz);
 
 /**
  * __register_sysctl_init() - register sysctl table to path
@@ -1451,10 +1446,17 @@ EXPORT_SYMBOL(register_sysctl);
 void __init __register_sysctl_init(const char *path, struct ctl_table *table,
 				 const char *table_name)
 {
-	struct ctl_table_header *hdr = register_sysctl(path, table);
+	int count = 0;
+	struct ctl_table *entry;
+	struct ctl_table_header t_hdr, *hdr;
+
+	t_hdr.ctl_table = table;
+	list_for_each_table_entry(entry, (&t_hdr))
+		count++;
+	hdr = register_sysctl_sz(path, table, count);
 
 	if (unlikely(!hdr)) {
-		pr_err("failed when register_sysctl %s to %s\n", table_name, path);
+		pr_err("failed when register_sysctl_sz %s to %s\n", table_name, path);
 		return;
 	}
 	kmemleak_not_leak(hdr);
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 0495c858989f..b1168ae281c9 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -215,6 +215,9 @@ struct ctl_path {
 	const char *procname;
 };
 
+#define register_sysctl(path, table)	\
+	register_sysctl_sz(path, table, ARRAY_SIZE(table))
+
 #ifdef CONFIG_SYSCTL
 
 void proc_sys_poll_notify(struct ctl_table_poll *poll);
@@ -227,7 +230,8 @@ extern void retire_sysctl_set(struct ctl_table_set *set);
 struct ctl_table_header *__register_sysctl_table(
 	struct ctl_table_set *set,
 	const char *path, struct ctl_table *table, size_t table_size);
-struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table);
+struct ctl_table_header *register_sysctl_sz(const char *path, struct ctl_table *table,
+					    size_t table_size);
 void unregister_sysctl_table(struct ctl_table_header * table);
 
 extern int sysctl_init_bases(void);
@@ -262,7 +266,9 @@ static inline struct ctl_table_header *register_sysctl_mount_point(const char *p
 	return NULL;
 }
 
-static inline struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table)
+static inline struct ctl_table_header *register_sysctl_sz(const char *path,
+							  struct ctl_table *table,
+							  size_t table_size)
 {
 	return NULL;
 }
diff --git a/kernel/ucount.c b/kernel/ucount.c
index 2b80264bb79f..4aa6166cb856 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -365,7 +365,7 @@ static __init int user_namespace_sysctl_init(void)
 	 * default set so that registrations in the child sets work
 	 * properly.
 	 */
-	user_header = register_sysctl("user", empty);
+	user_header = register_sysctl_sz("user", empty, 0);
 	kmemleak_ignore(user_header);
 	BUG_ON(!user_header);
 	BUG_ON(!setup_userns_sysctls(&init_user_ns));
diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index 8ee4b74bc009..d9cbbb51b143 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -101,7 +101,7 @@ __init int net_sysctl_init(void)
 	 * registering "/proc/sys/net" as an empty directory not in a
 	 * network namespace.
 	 */
-	net_header = register_sysctl("net", empty);
+	net_header = register_sysctl_sz("net", empty, 0);
 	if (!net_header)
 		goto out;
 	ret = register_pernet_subsys(&sysctl_pernet_ops);
-- 
2.30.2

Re: [PATCH 06/14] sysctl: Add size to register_sysctl

[Luis Chamberlain]

On Wed, Jul 26, 2023 at 04:06:26PM +0200, Joel Granados wrote:
> In order to remove the end element from the ctl_table struct arrays, we
> replace the register_syctl function with a macro that will add the
> ARRAY_SIZE to the new register_sysctl_sz function. In this way the
> callers that are already using an array of ctl_table structs do not have
> to change. We *do* change the callers that pass the ctl_table array as a
> pointer.

Thanks for doing this and this series!

> Signed-off-by: Joel Granados <j.granados@samsung.com>
> ---
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index 0495c858989f..b1168ae281c9 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -215,6 +215,9 @@ struct ctl_path {
>  	const char *procname;
>  };
>  
> +#define register_sysctl(path, table)	\
> +	register_sysctl_sz(path, table, ARRAY_SIZE(table))
> +
>  #ifdef CONFIG_SYSCTL

Wasn't it Greg who had suggested this? Maybe add Suggested-by with him
on it.

Also, your cover letter and first few patches are not CC'd to the netdev
list or others. What you want to do is collect all the email addresses
for this small patch series and add them to who you email for your
entire series, otherwise at times they won't be able to properly review
or understand the exact context of the changes. You want folks to do less
work to review, not more.

So please resend and add others to the other patches.

  Luis

Re: [PATCH 06/14] sysctl: Add size to register_sysctl

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2057 bytes --]

On Wed, Jul 26, 2023 at 10:58:30AM -0700, Luis Chamberlain wrote:
> On Wed, Jul 26, 2023 at 04:06:26PM +0200, Joel Granados wrote:
> > In order to remove the end element from the ctl_table struct arrays, we
> > replace the register_syctl function with a macro that will add the
> > ARRAY_SIZE to the new register_sysctl_sz function. In this way the
> > callers that are already using an array of ctl_table structs do not have
> > to change. We *do* change the callers that pass the ctl_table array as a
> > pointer.
> 
> Thanks for doing this and this series!
> 
> > Signed-off-by: Joel Granados <j.granados@samsung.com>
> > ---
> > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> > index 0495c858989f..b1168ae281c9 100644
> > --- a/include/linux/sysctl.h
> > +++ b/include/linux/sysctl.h
> > @@ -215,6 +215,9 @@ struct ctl_path {
> >  	const char *procname;
> >  };
> >  
> > +#define register_sysctl(path, table)	\
> > +	register_sysctl_sz(path, table, ARRAY_SIZE(table))
> > +
> >  #ifdef CONFIG_SYSCTL
> 
> Wasn't it Greg who had suggested this? Maybe add Suggested-by with him
> on it.
Yes. I mentioned him in the cover letter and did not add the tag because
I had not asked for permission to use it. I'll drop him a mail and
include the suggested-by if he agrees.

> 
> Also, your cover letter and first few patches are not CC'd to the netdev
> list or others. What you want to do is collect all the email addresses
> for this small patch series and add them to who you email for your
> entire series, otherwise at times they won't be able to properly review
> or understand the exact context of the changes. You want folks to do less
> work to review, not more.
Here I wanted to avoid very big e-mail headers as I have received
rejections from lists in the past. But I for this set, the number of
e-mails is ok to just include everyone.

I'll do that for V2.
thx for your feedback

best

> 
> So please resend and add others to the other patches.
> 
>   Luis

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 06/14] sysctl: Add size to register_sysctl

[Luis Chamberlain]

On Thu, Jul 27, 2023 at 02:22:00PM +0200, Joel Granados wrote:
> On Wed, Jul 26, 2023 at 10:58:30AM -0700, Luis Chamberlain wrote:
> > On Wed, Jul 26, 2023 at 04:06:26PM +0200, Joel Granados wrote:
> > > In order to remove the end element from the ctl_table struct arrays, we
> > > replace the register_syctl function with a macro that will add the
> > > ARRAY_SIZE to the new register_sysctl_sz function. In this way the
> > > callers that are already using an array of ctl_table structs do not have
> > > to change. We *do* change the callers that pass the ctl_table array as a
> > > pointer.
> > 
> > Thanks for doing this and this series!
> > 
> > > Signed-off-by: Joel Granados <j.granados@samsung.com>
> > > ---
> > > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> > > index 0495c858989f..b1168ae281c9 100644
> > > --- a/include/linux/sysctl.h
> > > +++ b/include/linux/sysctl.h
> > > @@ -215,6 +215,9 @@ struct ctl_path {
> > >  	const char *procname;
> > >  };
> > >  
> > > +#define register_sysctl(path, table)	\
> > > +	register_sysctl_sz(path, table, ARRAY_SIZE(table))
> > > +
> > >  #ifdef CONFIG_SYSCTL
> > 
> > Wasn't it Greg who had suggested this? Maybe add Suggested-by with him
> > on it.
> Yes. I mentioned him in the cover letter and did not add the tag because
> I had not asked for permission to use it. I'll drop him a mail and
> include the suggested-by if he agrees.

FWIW, I never ask, if they ask for it, clearly they suggested it.

> > Also, your cover letter and first few patches are not CC'd to the netdev
> > list or others. What you want to do is collect all the email addresses
> > for this small patch series and add them to who you email for your
> > entire series, otherwise at times they won't be able to properly review
> > or understand the exact context of the changes. You want folks to do less
> > work to review, not more.
> Here I wanted to avoid very big e-mail headers as I have received
> rejections from lists in the past. But I for this set, the number of
> e-mails is ok to just include everyone.

I hear that from time to time, if you have issues with adding folks on
the To address it may be an SMTP server issue, ie, corp email SMTP
server issues. To fix that I avoid corp email SMTP servers.

  Luis

Re: [PATCH 06/14] sysctl: Add size to register_sysctl

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2886 bytes --]

On Thu, Jul 27, 2023 at 08:42:02AM -0700, Luis Chamberlain wrote:
> On Thu, Jul 27, 2023 at 02:22:00PM +0200, Joel Granados wrote:
> > On Wed, Jul 26, 2023 at 10:58:30AM -0700, Luis Chamberlain wrote:
> > > On Wed, Jul 26, 2023 at 04:06:26PM +0200, Joel Granados wrote:
> > > > In order to remove the end element from the ctl_table struct arrays, we
> > > > replace the register_syctl function with a macro that will add the
> > > > ARRAY_SIZE to the new register_sysctl_sz function. In this way the
> > > > callers that are already using an array of ctl_table structs do not have
> > > > to change. We *do* change the callers that pass the ctl_table array as a
> > > > pointer.
> > > 
> > > Thanks for doing this and this series!
> > > 
> > > > Signed-off-by: Joel Granados <j.granados@samsung.com>
> > > > ---
> > > > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> > > > index 0495c858989f..b1168ae281c9 100644
> > > > --- a/include/linux/sysctl.h
> > > > +++ b/include/linux/sysctl.h
> > > > @@ -215,6 +215,9 @@ struct ctl_path {
> > > >  	const char *procname;
> > > >  };
> > > >  
> > > > +#define register_sysctl(path, table)	\
> > > > +	register_sysctl_sz(path, table, ARRAY_SIZE(table))
> > > > +
> > > >  #ifdef CONFIG_SYSCTL
> > > 
> > > Wasn't it Greg who had suggested this? Maybe add Suggested-by with him
> > > on it.
> > Yes. I mentioned him in the cover letter and did not add the tag because
> > I had not asked for permission to use it. I'll drop him a mail and
> > include the suggested-by if he agrees.
> 
> FWIW, I never ask, if they ask for it, clearly they suggested it.
I was following Documentation/process/submitting-patches.rst:
"... Please note that this tag should not be added without the
reporter's permission... ".
In any case, Greg has already said yes :)

> 
> > > Also, your cover letter and first few patches are not CC'd to the netdev
> > > list or others. What you want to do is collect all the email addresses
> > > for this small patch series and add them to who you email for your
> > > entire series, otherwise at times they won't be able to properly review
> > > or understand the exact context of the changes. You want folks to do less
> > > work to review, not more.
> > Here I wanted to avoid very big e-mail headers as I have received
> > rejections from lists in the past. But I for this set, the number of
> > e-mails is ok to just include everyone.
> 
> I hear that from time to time, if you have issues with adding folks on
> the To address it may be an SMTP server issue, ie, corp email SMTP
> server issues. To fix that I avoid corp email SMTP servers.
My experience was more from the lists rejecting the e-mail because the
header was too big. With that said, I'll look into SMTP alternatives to
reduce possible errors

> 
>   Luis

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 08/14] sysctl: Add size to register_net_sysctl function

[Joel Granados]

This is part of the effort to remove the sentinel (last empty) element
from the ctl_table arrays.

We create a new function register_net_sysctl_sz with an extra size
argument. A macro replaces the existing register_net_sysctl. For now,
the size passed by the macro is SIZE_MAX (will eventually be ARRAY_SIZE)
to avoid compilation warnings from pointers to ctl_table arrays instead
of the arrays themselves.

Care is also taken to add table_size to the stopping criteria in such a
way that when we remove the empty sentinel element, it will continue
stopping in the last element of the ctl_table array

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 include/net/net_namespace.h | 10 ++++++----
 net/sysctl_net.c            | 22 +++++++++++++---------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 78beaa765c73..e4e5fe75a281 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -469,15 +469,17 @@ void unregister_pernet_device(struct pernet_operations *);
 
 struct ctl_table;
 
+#define register_net_sysctl(net, path, table)	\
+	register_net_sysctl_sz(net, path, table, SIZE_MAX)
 #ifdef CONFIG_SYSCTL
 int net_sysctl_init(void);
-struct ctl_table_header *register_net_sysctl(struct net *net, const char *path,
-					     struct ctl_table *table);
+struct ctl_table_header *register_net_sysctl_sz(struct net *net, const char *path,
+					     struct ctl_table *table, size_t table_size);
 void unregister_net_sysctl_table(struct ctl_table_header *header);
 #else
 static inline int net_sysctl_init(void) { return 0; }
-static inline struct ctl_table_header *register_net_sysctl(struct net *net,
-	const char *path, struct ctl_table *table)
+static inline struct ctl_table_header *register_net_sysctl_sz(struct net *net,
+	const char *path, struct ctl_table *table, size_t table_size)
 {
 	return NULL;
 }
diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index d9cbbb51b143..051ed5f6fc93 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -122,12 +122,13 @@ __init int net_sysctl_init(void)
  *    allocated.
  */
 static void ensure_safe_net_sysctl(struct net *net, const char *path,
-				   struct ctl_table *table)
+				   struct ctl_table *table, size_t table_size)
 {
 	struct ctl_table *ent;
 
 	pr_debug("Registering net sysctl (net %p): %s\n", net, path);
-	for (ent = table; ent->procname; ent++) {
+	ent = table;
+	for (size_t i = 0; i < table_size && ent->procname; ent++, i++) {
 		unsigned long addr;
 		const char *where;
 
@@ -160,21 +161,24 @@ static void ensure_safe_net_sysctl(struct net *net, const char *path,
 	}
 }
 
-struct ctl_table_header *register_net_sysctl(struct net *net,
-	const char *path, struct ctl_table *table)
+struct ctl_table_header *register_net_sysctl_sz(struct net *net,
+						const char *path,
+						struct ctl_table *table,
+						size_t table_size)
 {
-	int count = 0;
+	int count;
 	struct ctl_table *entry;
 
 	if (!net_eq(net, &init_net))
-		ensure_safe_net_sysctl(net, path, table);
+		ensure_safe_net_sysctl(net, path, table, table_size);
 
-	for (entry = table; entry->procname; entry++)
-		count++;
+	entry = table;
+	for (count = 0 ; count < table_size && entry->procname; entry++, count++)
+		;
 
 	return __register_sysctl_table(&net->sysctls, path, table, count);
 }
-EXPORT_SYMBOL_GPL(register_net_sysctl);
+EXPORT_SYMBOL_GPL(register_net_sysctl_sz);
 
 void unregister_net_sysctl_table(struct ctl_table_header *header)
 {
-- 
2.30.2

[PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Joel Granados]

This is part of the effort to remove the sentinel (last empty) element
from the ctl_table arrays. We update to the new function and pass it the
array size.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 net/ax25/sysctl_net_ax25.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ax25/sysctl_net_ax25.c b/net/ax25/sysctl_net_ax25.c
index 2154d004d3dc..db66e11e7fe8 100644
--- a/net/ax25/sysctl_net_ax25.c
+++ b/net/ax25/sysctl_net_ax25.c
@@ -159,7 +159,8 @@ int ax25_register_dev_sysctl(ax25_dev *ax25_dev)
 		table[k].data = &ax25_dev->values[k];
 
 	snprintf(path, sizeof(path), "net/ax25/%s", ax25_dev->dev->name);
-	ax25_dev->sysheader = register_net_sysctl(&init_net, path, table);
+	ax25_dev->sysheader = register_net_sysctl_sz(&init_net, path, table,
+						     ARRAY_SIZE(ax25_param_table));
 	if (!ax25_dev->sysheader) {
 		kfree(table);
 		return -ENOMEM;
-- 
2.30.2

Re: [PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Luis Chamberlain]

On Wed, Jul 26, 2023 at 04:06:29PM +0200, Joel Granados wrote:
> This is part of the effort to remove the sentinel (last empty) element
> from the ctl_table arrays. We update to the new function and pass it the
> array size.

The commit log does not explain why. It also does not explain if there
is any delta on size at compile or runtime.

  Luis

Re: [PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

On Wed, Jul 26, 2023 at 11:00:37AM -0700, Luis Chamberlain wrote:
> On Wed, Jul 26, 2023 at 04:06:29PM +0200, Joel Granados wrote:
> > This is part of the effort to remove the sentinel (last empty) element
> > from the ctl_table arrays. We update to the new function and pass it the
> > array size.
> 
> The commit log does not explain why. It also does not explain if there
I'll add the "why".

> is any delta on size at compile or runtime.
There are no deltas in this patch set. We start seeing the deltas when
we start removing with the next 6 chunks. I'll try to make that more
clear in the commit message.

> 
>   Luis

thx for the feedback

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Luis Chamberlain]

On Thu, Jul 27, 2023 at 02:31:12PM +0200, Joel Granados wrote:
> There are no deltas in this patch set. We start seeing the deltas when
> we start removing with the next 6 chunks. I'll try to make that more
> clear in the commit message.

Indeed, even if no deltas are created it is importan then to say that.
If there are no deltas the "why" becomes more important. If the why is
to make it easier to apply subsequent patches, that must be said. When
you iterate your new series try to review the patches as if you were not
the person submitting them, and try to think of ways to make it easier
for the patch reviewer to do less work. The less work and easier patch
review is the better for them.

  Luis

Re: [PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

On Thu, Jul 27, 2023 at 08:44:24AM -0700, Luis Chamberlain wrote:
> On Thu, Jul 27, 2023 at 02:31:12PM +0200, Joel Granados wrote:
> > There are no deltas in this patch set. We start seeing the deltas when
> > we start removing with the next 6 chunks. I'll try to make that more
> > clear in the commit message.
> 
> Indeed, even if no deltas are created it is importan then to say that.
> If there are no deltas the "why" becomes more important. If the why is
> to make it easier to apply subsequent patches, that must be said. When
yes. The why for this patch set in particular is to make it easier to
apply the sentinel removal patches.

I think the difficulty for me comes from having two whys: 1. The one for
this patch set which is to make it easier to apply sentinel removal patches. And 2.
The one for the "big" patch (that actually removes the sentinels) which is to
reduce build time size and run time memory bloat.

> you iterate your new series try to review the patches as if you were not
> the person submitting them, and try to think of ways to make it easier
> for the patch reviewer to do less work. The less work and easier patch
> review is the better for them.
Ack. For all these commits I'll try to weave in the two Whys to make the
review process a bit easier.

> 
>   Luis

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Luis Chamberlain]

On Fri, Jul 28, 2023 at 09:35:36AM +0200, Joel Granados wrote:
> On Thu, Jul 27, 2023 at 08:44:24AM -0700, Luis Chamberlain wrote:
> > On Thu, Jul 27, 2023 at 02:31:12PM +0200, Joel Granados wrote:
> > > There are no deltas in this patch set. We start seeing the deltas when
> > > we start removing with the next 6 chunks. I'll try to make that more
> > > clear in the commit message.
> > 
> > Indeed, even if no deltas are created it is importan then to say that.
> > If there are no deltas the "why" becomes more important. If the why is
> > to make it easier to apply subsequent patches, that must be said. When
> yes. The why for this patch set in particular is to make it easier to
> apply the sentinel removal patches.
> 
> I think the difficulty for me comes from having two whys: 1. The one for
> this patch set which is to make it easier to apply sentinel removal patches. And 2.
> The one for the "big" patch (that actually removes the sentinels) which is to
> reduce build time size and run time memory bloat.

The 2) is part of the real why, 1) is more of how to do 2) cleanly. But
the real why is the savings in memory because we are moving arrays out
of kernel/sysctl.c so we don't want to incur a size penalty. The
collateral to avoid increasing size in the moves also proves to save us
more memory overall, on the ballpark of about 64 bytes per array in the
kernel both at runtime and build time. The build time gain is mostly
on the __init stuff and so gets freed right away, but since sysctl
code always kzallocs the arrays passed we also save 64 bytes per array
in the end at runtime.

  Luis

Re: [PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

On Fri, Jul 28, 2023 at 11:16:06AM -0700, Luis Chamberlain wrote:
> On Fri, Jul 28, 2023 at 09:35:36AM +0200, Joel Granados wrote:
> > On Thu, Jul 27, 2023 at 08:44:24AM -0700, Luis Chamberlain wrote:
> > > On Thu, Jul 27, 2023 at 02:31:12PM +0200, Joel Granados wrote:
> > > > There are no deltas in this patch set. We start seeing the deltas when
> > > > we start removing with the next 6 chunks. I'll try to make that more
> > > > clear in the commit message.
> > > 
> > > Indeed, even if no deltas are created it is importan then to say that.
> > > If there are no deltas the "why" becomes more important. If the why is
> > > to make it easier to apply subsequent patches, that must be said. When
> > yes. The why for this patch set in particular is to make it easier to
> > apply the sentinel removal patches.
> > 
> > I think the difficulty for me comes from having two whys: 1. The one for
> > this patch set which is to make it easier to apply sentinel removal patches. And 2.
> > The one for the "big" patch (that actually removes the sentinels) which is to
> > reduce build time size and run time memory bloat.
> 
> The 2) is part of the real why, 1) is more of how to do 2) cleanly. But
> the real why is the savings in memory because we are moving arrays out
> of kernel/sysctl.c so we don't want to incur a size penalty. The
> collateral to avoid increasing size in the moves also proves to save us
> more memory overall, on the ballpark of about 64 bytes per array in the
> kernel both at runtime and build time. The build time gain is mostly
> on the __init stuff and so gets freed right away, but since sysctl
> code always kzallocs the arrays passed we also save 64 bytes per array
> in the end at runtime.
Yes. In my new version I have tried to mention both 1 and 2 and
differentiate between them. I stuck with the "why" for this patch set is
to make it easier to reach 2.

I'll send it out today.
> 
>   Luis

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

On Wed, Jul 26, 2023 at 11:00:37AM -0700, Luis Chamberlain wrote:
> On Wed, Jul 26, 2023 at 04:06:29PM +0200, Joel Granados wrote:
> > This is part of the effort to remove the sentinel (last empty) element
> > from the ctl_table arrays. We update to the new function and pass it the
> > array size.
> 
> The commit log does not explain why. It also does not explain if there
> is any delta on size at compile or runtime.
Just to be on the same page:
You mean the specific why for this commit. like for example:
"
We update to register_net_sysctl_sz to prepare for when the ctl_table
array sentinels are removed.
"

right?


> 
>   Luis

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/14] ax.25: Update to register_net_sysctl_sz

[Luis Chamberlain]

On Thu, Jul 27, 2023 at 05:38:04PM +0200, Joel Granados wrote:
> On Wed, Jul 26, 2023 at 11:00:37AM -0700, Luis Chamberlain wrote:
> > On Wed, Jul 26, 2023 at 04:06:29PM +0200, Joel Granados wrote:
> > > This is part of the effort to remove the sentinel (last empty) element
> > > from the ctl_table arrays. We update to the new function and pass it the
> > > array size.
> > 
> > The commit log does not explain why. It also does not explain if there
> > is any delta on size at compile or runtime.
> Just to be on the same page:
> You mean the specific why for this commit. like for example:
> "
> We update  to register_net_sysctl_sz

I think it is clearer to just say:

Move from register_net_sysctl() to register_net_sysctl_sz()

> to prepare for when the ctl_table
> array sentinels are removed.

That is not as clear. I think you should just spell it out.

We want to use the syctl ARRAY_SIZE() when possible, and subsequent
patches ... now the register_net_sysctl() <does something> while
register_net_sysctl_sz() <does something else> and <in this case>
this user <does something> and so we must use register_net_sysctl_sz().

> "
> 
> right?

  Luis

[PATCH 10/14] netfilter: Update to register_net_sysctl_sz

[Joel Granados]

This is part of the effort to remove the sentinel (last empty) element
from the ctl_table arrays. We update to the new function and pass it the
array size. Care is taken to mirror the NULL assignments with a size of
zero (for the unprivileged users)

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 net/bridge/br_netfilter_hooks.c         |  3 ++-
 net/ipv6/netfilter/nf_conntrack_reasm.c |  3 ++-
 net/netfilter/ipvs/ip_vs_ctl.c          |  8 ++++++--
 net/netfilter/ipvs/ip_vs_lblc.c         | 10 +++++++---
 net/netfilter/ipvs/ip_vs_lblcr.c        | 10 +++++++---
 net/netfilter/nf_conntrack_standalone.c |  4 +++-
 net/netfilter/nf_log.c                  |  7 ++++---
 7 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 1a801fab9543..15186247b59a 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -1135,7 +1135,8 @@ static int br_netfilter_sysctl_init_net(struct net *net)
 
 	br_netfilter_sysctl_default(brnet);
 
-	brnet->ctl_hdr = register_net_sysctl(net, "net/bridge", table);
+	brnet->ctl_hdr = register_net_sysctl_sz(net, "net/bridge", table,
+						ARRAY_SIZE(brnf_table));
 	if (!brnet->ctl_hdr) {
 		if (!net_eq(net, &init_net))
 			kfree(table);
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index d13240f13607..b2dd48911c8d 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -87,7 +87,8 @@ static int nf_ct_frag6_sysctl_register(struct net *net)
 	table[2].data	= &nf_frag->fqdir->high_thresh;
 	table[2].extra1	= &nf_frag->fqdir->low_thresh;
 
-	hdr = register_net_sysctl(net, "net/netfilter", table);
+	hdr = register_net_sysctl_sz(net, "net/netfilter", table,
+				     ARRAY_SIZE(nf_ct_frag6_sysctl_table));
 	if (hdr == NULL)
 		goto err_reg;
 
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 62606fb44d02..8d69e4c2d822 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -4266,6 +4266,7 @@ static int __net_init ip_vs_control_net_init_sysctl(struct netns_ipvs *ipvs)
 	struct net *net = ipvs->net;
 	struct ctl_table *tbl;
 	int idx, ret;
+	size_t ctl_table_size = ARRAY_SIZE(vs_vars);
 
 	atomic_set(&ipvs->dropentry, 0);
 	spin_lock_init(&ipvs->dropentry_lock);
@@ -4282,8 +4283,10 @@ static int __net_init ip_vs_control_net_init_sysctl(struct netns_ipvs *ipvs)
 			return -ENOMEM;
 
 		/* Don't export sysctls to unprivileged users */
-		if (net->user_ns != &init_user_ns)
+		if (net->user_ns != &init_user_ns) {
 			tbl[0].procname = NULL;
+			ctl_table_size = 0;
+		}
 	} else
 		tbl = vs_vars;
 	/* Initialize sysctl defaults */
@@ -4353,7 +4356,8 @@ static int __net_init ip_vs_control_net_init_sysctl(struct netns_ipvs *ipvs)
 #endif
 
 	ret = -ENOMEM;
-	ipvs->sysctl_hdr = register_net_sysctl(net, "net/ipv4/vs", tbl);
+	ipvs->sysctl_hdr = register_net_sysctl_sz(net, "net/ipv4/vs", tbl,
+						  ctl_table_size);
 	if (!ipvs->sysctl_hdr)
 		goto err;
 	ipvs->sysctl_tbl = tbl;
diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index 1b87214d385e..cf78ba4ce5ff 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -550,6 +550,7 @@ static struct ip_vs_scheduler ip_vs_lblc_scheduler = {
 static int __net_init __ip_vs_lblc_init(struct net *net)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
+	size_t vars_table_size = ARRAY_SIZE(vs_vars_table);
 
 	if (!ipvs)
 		return -ENOENT;
@@ -562,16 +563,19 @@ static int __net_init __ip_vs_lblc_init(struct net *net)
 			return -ENOMEM;
 
 		/* Don't export sysctls to unprivileged users */
-		if (net->user_ns != &init_user_ns)
+		if (net->user_ns != &init_user_ns) {
 			ipvs->lblc_ctl_table[0].procname = NULL;
+			vars_table_size = 0;
+		}
 
 	} else
 		ipvs->lblc_ctl_table = vs_vars_table;
 	ipvs->sysctl_lblc_expiration = DEFAULT_EXPIRATION;
 	ipvs->lblc_ctl_table[0].data = &ipvs->sysctl_lblc_expiration;
 
-	ipvs->lblc_ctl_header =
-		register_net_sysctl(net, "net/ipv4/vs", ipvs->lblc_ctl_table);
+	ipvs->lblc_ctl_header = register_net_sysctl_sz(net, "net/ipv4/vs",
+						       ipvs->lblc_ctl_table,
+						       vars_table_size);
 	if (!ipvs->lblc_ctl_header) {
 		if (!net_eq(net, &init_net))
 			kfree(ipvs->lblc_ctl_table);
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index ad8f5fea6d3a..9eddf118b40e 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -736,6 +736,7 @@ static struct ip_vs_scheduler ip_vs_lblcr_scheduler =
 static int __net_init __ip_vs_lblcr_init(struct net *net)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
+	size_t vars_table_size = ARRAY_SIZE(vs_vars_table);
 
 	if (!ipvs)
 		return -ENOENT;
@@ -748,15 +749,18 @@ static int __net_init __ip_vs_lblcr_init(struct net *net)
 			return -ENOMEM;
 
 		/* Don't export sysctls to unprivileged users */
-		if (net->user_ns != &init_user_ns)
+		if (net->user_ns != &init_user_ns) {
 			ipvs->lblcr_ctl_table[0].procname = NULL;
+			vars_table_size = 0;
+		}
 	} else
 		ipvs->lblcr_ctl_table = vs_vars_table;
 	ipvs->sysctl_lblcr_expiration = DEFAULT_EXPIRATION;
 	ipvs->lblcr_ctl_table[0].data = &ipvs->sysctl_lblcr_expiration;
 
-	ipvs->lblcr_ctl_header =
-		register_net_sysctl(net, "net/ipv4/vs", ipvs->lblcr_ctl_table);
+	ipvs->lblcr_ctl_header = register_net_sysctl_sz(net, "net/ipv4/vs",
+							ipvs->lblcr_ctl_table,
+							vars_table_size);
 	if (!ipvs->lblcr_ctl_header) {
 		if (!net_eq(net, &init_net))
 			kfree(ipvs->lblcr_ctl_table);
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 169e16fc2bce..0ee98ce5b816 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -1106,7 +1106,9 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
 		table[NF_SYSCTL_CT_BUCKETS].mode = 0444;
 	}
 
-	cnet->sysctl_header = register_net_sysctl(net, "net/netfilter", table);
+	cnet->sysctl_header = register_net_sysctl_sz(net, "net/netfilter",
+						     table,
+						     ARRAY_SIZE(nf_ct_sysctl_table));
 	if (!cnet->sysctl_header)
 		goto out_unregister_netfilter;
 
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 8a29290149bd..8cc52d2bd31b 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -487,9 +487,10 @@ static int netfilter_log_sysctl_init(struct net *net)
 	for (i = NFPROTO_UNSPEC; i < NFPROTO_NUMPROTO; i++)
 		table[i].extra2 = net;
 
-	net->nf.nf_log_dir_header = register_net_sysctl(net,
-						"net/netfilter/nf_log",
-						table);
+	net->nf.nf_log_dir_header = register_net_sysctl_sz(net,
+							   "net/netfilter/nf_log",
+							   table,
+							   ARRAY_SIZE(nf_log_sysctl_table));
 	if (!net->nf.nf_log_dir_header)
 		goto err_reg;
 
-- 
2.30.2

Re: [PATCH 10/14] netfilter: Update to register_net_sysctl_sz

[Luis Chamberlain]

On Wed, Jul 26, 2023 at 04:06:30PM +0200, Joel Granados wrote:
> This is part of the effort to remove the sentinel (last empty) element
> from the ctl_table arrays. We update to the new function and pass it the
> array size. Care is taken to mirror the NULL assignments with a size of
> zero (for the unprivileged users)

Same in this commit, the "why" is missing and size impact, if any.

  Luis

[PATCH 11/14] networking: Update to register_net_sysctl_sz

[Joel Granados]

This is part of the effort to remove the sentinel (last empty) element
from the ctl_table arrays. We update to the new function and pass it the
array size. Care is taken to mirror the NULL assignments with a size of
zero (for the unprivileged users). An additional size function was added
to the following files in order to calculate the size of an array that
is defined in another file:
    include/net/ipv6.h
    net/ipv6/icmp.c
    net/ipv6/route.c
    net/ipv6/sysctl_net_ipv6.c

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 include/net/ipv6.h                  |  2 +
 net/core/neighbour.c                |  8 +++-
 net/core/sysctl_net_core.c          |  3 +-
 net/ieee802154/6lowpan/reassembly.c |  8 +++-
 net/ipv4/devinet.c                  |  3 +-
 net/ipv4/ip_fragment.c              |  3 +-
 net/ipv4/route.c                    |  8 +++-
 net/ipv4/sysctl_net_ipv4.c          |  3 +-
 net/ipv4/xfrm4_policy.c             |  3 +-
 net/ipv6/addrconf.c                 |  3 +-
 net/ipv6/icmp.c                     |  5 ++
 net/ipv6/reassembly.c               |  3 +-
 net/ipv6/route.c                    | 13 ++++--
 net/ipv6/sysctl_net_ipv6.c          | 16 +++++--
 net/ipv6/xfrm6_policy.c             |  3 +-
 net/mpls/af_mpls.c                  | 72 +++++++++++++++--------------
 net/mptcp/ctrl.c                    |  3 +-
 net/rds/tcp.c                       |  3 +-
 net/sctp/sysctl.c                   |  4 +-
 net/smc/smc_sysctl.c                |  3 +-
 net/unix/sysctl_net_unix.c          |  3 +-
 net/xfrm/xfrm_sysctl.c              |  8 +++-
 22 files changed, 116 insertions(+), 64 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 7332296eca44..63ba68536a20 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1274,7 +1274,9 @@ static inline int snmp6_unregister_dev(struct inet6_dev *idev) { return 0; }
 
 #ifdef CONFIG_SYSCTL
 struct ctl_table *ipv6_icmp_sysctl_init(struct net *net);
+size_t ipv6_icmp_sysctl_table_size(void);
 struct ctl_table *ipv6_route_sysctl_init(struct net *net);
+size_t ipv6_route_sysctl_table_size(struct net *net);
 int ipv6_sysctl_register(void);
 void ipv6_sysctl_unregister(void);
 #endif
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index ddd0f32de20e..adc7fc4ff9bf 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -3779,6 +3779,7 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
 	const char *dev_name_source;
 	char neigh_path[ sizeof("net//neigh/") + IFNAMSIZ + IFNAMSIZ ];
 	char *p_name;
+	size_t neigh_vars_size;
 
 	t = kmemdup(&neigh_sysctl_template, sizeof(*t), GFP_KERNEL_ACCOUNT);
 	if (!t)
@@ -3790,11 +3791,13 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
 		t->neigh_vars[i].extra2 = p;
 	}
 
+	neigh_vars_size = ARRAY_SIZE(t->neigh_vars);
 	if (dev) {
 		dev_name_source = dev->name;
 		/* Terminate the table early */
 		memset(&t->neigh_vars[NEIGH_VAR_GC_INTERVAL], 0,
 		       sizeof(t->neigh_vars[NEIGH_VAR_GC_INTERVAL]));
+		neigh_vars_size = NEIGH_VAR_BASE_REACHABLE_TIME_MS;
 	} else {
 		struct neigh_table *tbl = p->tbl;
 		dev_name_source = "default";
@@ -3841,8 +3844,9 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
 
 	snprintf(neigh_path, sizeof(neigh_path), "net/%s/neigh/%s",
 		p_name, dev_name_source);
-	t->sysctl_header =
-		register_net_sysctl(neigh_parms_net(p), neigh_path, t->neigh_vars);
+	t->sysctl_header = register_net_sysctl_sz(neigh_parms_net(p),
+						  neigh_path, t->neigh_vars,
+						  neigh_vars_size);
 	if (!t->sysctl_header)
 		goto free;
 
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 782273bb93c2..03f1edb948d7 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -712,7 +712,8 @@ static __net_init int sysctl_core_net_init(struct net *net)
 			tmp->data += (char *)net - (char *)&init_net;
 	}
 
-	net->core.sysctl_hdr = register_net_sysctl(net, "net/core", tbl);
+	net->core.sysctl_hdr = register_net_sysctl_sz(net, "net/core", tbl,
+						      ARRAY_SIZE(netns_core_table));
 	if (net->core.sysctl_hdr == NULL)
 		goto err_reg;
 
diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
index a91283d1e5bf..6dd960ec558c 100644
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -360,6 +360,7 @@ static int __net_init lowpan_frags_ns_sysctl_register(struct net *net)
 	struct ctl_table_header *hdr;
 	struct netns_ieee802154_lowpan *ieee802154_lowpan =
 		net_ieee802154_lowpan(net);
+	size_t table_size = ARRAY_SIZE(lowpan_frags_ns_ctl_table);
 
 	table = lowpan_frags_ns_ctl_table;
 	if (!net_eq(net, &init_net)) {
@@ -369,8 +370,10 @@ static int __net_init lowpan_frags_ns_sysctl_register(struct net *net)
 			goto err_alloc;
 
 		/* Don't export sysctls to unprivileged users */
-		if (net->user_ns != &init_user_ns)
+		if (net->user_ns != &init_user_ns) {
 			table[0].procname = NULL;
+			table_size = 0;
+		}
 	}
 
 	table[0].data	= &ieee802154_lowpan->fqdir->high_thresh;
@@ -379,7 +382,8 @@ static int __net_init lowpan_frags_ns_sysctl_register(struct net *net)
 	table[1].extra2	= &ieee802154_lowpan->fqdir->high_thresh;
 	table[2].data	= &ieee802154_lowpan->fqdir->timeout;
 
-	hdr = register_net_sysctl(net, "net/ieee802154/6lowpan", table);
+	hdr = register_net_sysctl_sz(net, "net/ieee802154/6lowpan", table,
+				     table_size);
 	if (hdr == NULL)
 		goto err_reg;
 
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 5deac0517ef7..89087844ea6e 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2720,7 +2720,8 @@ static __net_init int devinet_init_net(struct net *net)
 		goto err_reg_dflt;
 
 	err = -ENOMEM;
-	forw_hdr = register_net_sysctl(net, "net/ipv4", tbl);
+	forw_hdr = register_net_sysctl_sz(net, "net/ipv4", tbl,
+					  ARRAY_SIZE(ctl_forward_entry));
 	if (!forw_hdr)
 		goto err_reg_ctl;
 	net->ipv4.forw_hdr = forw_hdr;
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 69c00ffdcf3e..a4941f53b523 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -615,7 +615,8 @@ static int __net_init ip4_frags_ns_ctl_register(struct net *net)
 	table[2].data	= &net->ipv4.fqdir->timeout;
 	table[3].data	= &net->ipv4.fqdir->max_dist;
 
-	hdr = register_net_sysctl(net, "net/ipv4", table);
+	hdr = register_net_sysctl_sz(net, "net/ipv4", table,
+				     ARRAY_SIZE(ip4_frags_ns_ctl_table));
 	if (!hdr)
 		goto err_reg;
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 98d7e6ba7493..e7e9fba0357a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -3592,6 +3592,7 @@ static struct ctl_table ipv4_route_netns_table[] = {
 static __net_init int sysctl_route_net_init(struct net *net)
 {
 	struct ctl_table *tbl;
+	size_t table_size = ARRAY_SIZE(ipv4_route_netns_table);
 
 	tbl = ipv4_route_netns_table;
 	if (!net_eq(net, &init_net)) {
@@ -3603,8 +3604,10 @@ static __net_init int sysctl_route_net_init(struct net *net)
 
 		/* Don't export non-whitelisted sysctls to unprivileged users */
 		if (net->user_ns != &init_user_ns) {
-			if (tbl[0].procname != ipv4_route_flush_procname)
+			if (tbl[0].procname != ipv4_route_flush_procname) {
 				tbl[0].procname = NULL;
+				table_size = 0;
+			}
 		}
 
 		/* Update the variables to point into the current struct net
@@ -3615,7 +3618,8 @@ static __net_init int sysctl_route_net_init(struct net *net)
 	}
 	tbl[0].extra1 = net;
 
-	net->ipv4.route_hdr = register_net_sysctl(net, "net/ipv4/route", tbl);
+	net->ipv4.route_hdr = register_net_sysctl_sz(net, "net/ipv4/route",
+						     tbl, table_size);
 	if (!net->ipv4.route_hdr)
 		goto err_reg;
 	return 0;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 2afb0870648b..6ac890b4073f 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -1519,7 +1519,8 @@ static __net_init int ipv4_sysctl_init_net(struct net *net)
 		}
 	}
 
-	net->ipv4.ipv4_hdr = register_net_sysctl(net, "net/ipv4", table);
+	net->ipv4.ipv4_hdr = register_net_sysctl_sz(net, "net/ipv4", table,
+						    ARRAY_SIZE(ipv4_net_table));
 	if (!net->ipv4.ipv4_hdr)
 		goto err_reg;
 
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 9403bbaf1b61..57ea394ffa8c 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -178,7 +178,8 @@ static __net_init int xfrm4_net_sysctl_init(struct net *net)
 		table[0].data = &net->xfrm.xfrm4_dst_ops.gc_thresh;
 	}
 
-	hdr = register_net_sysctl(net, "net/ipv4", table);
+	hdr = register_net_sysctl_sz(net, "net/ipv4", table,
+				     ARRAY_SIZE(xfrm4_policy_table));
 	if (!hdr)
 		goto err_reg;
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index e5213e598a04..d615a84965c2 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -7085,7 +7085,8 @@ static int __addrconf_sysctl_register(struct net *net, char *dev_name,
 
 	snprintf(path, sizeof(path), "net/ipv6/conf/%s", dev_name);
 
-	p->sysctl_header = register_net_sysctl(net, path, table);
+	p->sysctl_header = register_net_sysctl_sz(net, path, table,
+						  ARRAY_SIZE(addrconf_sysctl));
 	if (!p->sysctl_header)
 		goto free;
 
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 65fa5014bc85..a76b01b41b57 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -1229,4 +1229,9 @@ struct ctl_table * __net_init ipv6_icmp_sysctl_init(struct net *net)
 	}
 	return table;
 }
+
+size_t ipv6_icmp_sysctl_table_size(void)
+{
+	return ARRAY_SIZE(ipv6_icmp_table_template);
+}
 #endif
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 5bc8a28e67f9..5ebc47da1000 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -470,7 +470,8 @@ static int __net_init ip6_frags_ns_sysctl_register(struct net *net)
 	table[1].extra2	= &net->ipv6.fqdir->high_thresh;
 	table[2].data	= &net->ipv6.fqdir->timeout;
 
-	hdr = register_net_sysctl(net, "net/ipv6", table);
+	hdr = register_net_sysctl_sz(net, "net/ipv6", table,
+				     ARRAY_SIZE(ip6_frags_ns_ctl_table));
 	if (!hdr)
 		goto err_reg;
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 64e873f5895f..51c6cdae8723 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -6447,14 +6447,19 @@ struct ctl_table * __net_init ipv6_route_sysctl_init(struct net *net)
 		table[8].data = &net->ipv6.sysctl.ip6_rt_min_advmss;
 		table[9].data = &net->ipv6.sysctl.ip6_rt_gc_min_interval;
 		table[10].data = &net->ipv6.sysctl.skip_notify_on_dev_down;
-
-		/* Don't export sysctls to unprivileged users */
-		if (net->user_ns != &init_user_ns)
-			table[1].procname = NULL;
 	}
 
 	return table;
 }
+
+size_t ipv6_route_sysctl_table_size(struct net *net)
+{
+	/* Don't export sysctls to unprivileged users */
+	if (net->user_ns != &init_user_ns)
+		return 0;
+
+	return ARRAY_SIZE(ipv6_route_table_template);
+}
 #endif
 
 static int __net_init ip6_route_net_init(struct net *net)
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 94a0a294c6a1..888676163e90 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -275,17 +275,23 @@ static int __net_init ipv6_sysctl_net_init(struct net *net)
 	if (!ipv6_icmp_table)
 		goto out_ipv6_route_table;
 
-	net->ipv6.sysctl.hdr = register_net_sysctl(net, "net/ipv6", ipv6_table);
+	net->ipv6.sysctl.hdr = register_net_sysctl_sz(net, "net/ipv6",
+						      ipv6_table,
+						      ARRAY_SIZE(ipv6_table_template));
 	if (!net->ipv6.sysctl.hdr)
 		goto out_ipv6_icmp_table;
 
-	net->ipv6.sysctl.route_hdr =
-		register_net_sysctl(net, "net/ipv6/route", ipv6_route_table);
+	net->ipv6.sysctl.route_hdr = register_net_sysctl_sz(net,
+							    "net/ipv6/route",
+							    ipv6_route_table,
+							    ipv6_route_sysctl_table_size(net));
 	if (!net->ipv6.sysctl.route_hdr)
 		goto out_unregister_ipv6_table;
 
-	net->ipv6.sysctl.icmp_hdr =
-		register_net_sysctl(net, "net/ipv6/icmp", ipv6_icmp_table);
+	net->ipv6.sysctl.icmp_hdr = register_net_sysctl_sz(net,
+							   "net/ipv6/icmp",
+							   ipv6_icmp_table,
+							   ipv6_icmp_sysctl_table_size());
 	if (!net->ipv6.sysctl.icmp_hdr)
 		goto out_unregister_route_table;
 
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index eecc5e59da17..8f931e46b460 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -205,7 +205,8 @@ static int __net_init xfrm6_net_sysctl_init(struct net *net)
 		table[0].data = &net->xfrm.xfrm6_dst_ops.gc_thresh;
 	}
 
-	hdr = register_net_sysctl(net, "net/ipv6", table);
+	hdr = register_net_sysctl_sz(net, "net/ipv6", table,
+				     ARRAY_SIZE(xfrm6_policy_table));
 	if (!hdr)
 		goto err_reg;
 
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index bf6e81d56263..5bad14b3c71e 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -1396,6 +1396,40 @@ static const struct ctl_table mpls_dev_table[] = {
 	{ }
 };
 
+static int mpls_platform_labels(struct ctl_table *table, int write,
+				void *buffer, size_t *lenp, loff_t *ppos);
+#define MPLS_NS_SYSCTL_OFFSET(field)		\
+	(&((struct net *)0)->field)
+
+static const struct ctl_table mpls_table[] = {
+	{
+		.procname	= "platform_labels",
+		.data		= NULL,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= mpls_platform_labels,
+	},
+	{
+		.procname	= "ip_ttl_propagate",
+		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.ip_ttl_propagate),
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_ONE,
+	},
+	{
+		.procname	= "default_ttl",
+		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.default_ttl),
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ONE,
+		.extra2		= &ttl_max,
+	},
+	{ }
+};
+
 static int mpls_dev_sysctl_register(struct net_device *dev,
 				    struct mpls_dev *mdev)
 {
@@ -1419,7 +1453,8 @@ static int mpls_dev_sysctl_register(struct net_device *dev,
 
 	snprintf(path, sizeof(path), "net/mpls/conf/%s", dev->name);
 
-	mdev->sysctl = register_net_sysctl(net, path, table);
+	mdev->sysctl = register_net_sysctl_sz(net, path, table,
+					      ARRAY_SIZE(mpls_dev_table));
 	if (!mdev->sysctl)
 		goto free;
 
@@ -2637,38 +2672,6 @@ static int mpls_platform_labels(struct ctl_table *table, int write,
 	return ret;
 }
 
-#define MPLS_NS_SYSCTL_OFFSET(field)		\
-	(&((struct net *)0)->field)
-
-static const struct ctl_table mpls_table[] = {
-	{
-		.procname	= "platform_labels",
-		.data		= NULL,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= mpls_platform_labels,
-	},
-	{
-		.procname	= "ip_ttl_propagate",
-		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.ip_ttl_propagate),
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= SYSCTL_ZERO,
-		.extra2		= SYSCTL_ONE,
-	},
-	{
-		.procname	= "default_ttl",
-		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.default_ttl),
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= SYSCTL_ONE,
-		.extra2		= &ttl_max,
-	},
-	{ }
-};
-
 static int mpls_net_init(struct net *net)
 {
 	struct ctl_table *table;
@@ -2689,7 +2692,8 @@ static int mpls_net_init(struct net *net)
 	for (i = 0; i < ARRAY_SIZE(mpls_table) - 1; i++)
 		table[i].data = (char *)net + (uintptr_t)table[i].data;
 
-	net->mpls.ctl = register_net_sysctl(net, "net/mpls", table);
+	net->mpls.ctl = register_net_sysctl_sz(net, "net/mpls", table,
+					       ARRAY_SIZE(mpls_table));
 	if (net->mpls.ctl == NULL) {
 		kfree(table);
 		return -ENOMEM;
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index ae20b7d92e28..43e540328a52 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -150,7 +150,8 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
 	table[4].data = &pernet->stale_loss_cnt;
 	table[5].data = &pernet->pm_type;
 
-	hdr = register_net_sysctl(net, MPTCP_SYSCTL_PATH, table);
+	hdr = register_net_sysctl_sz(net, MPTCP_SYSCTL_PATH, table,
+				     ARRAY_SIZE(mptcp_sysctl_table));
 	if (!hdr)
 		goto err_reg;
 
diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index c5b86066ff66..2dba7505b414 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -565,7 +565,8 @@ static __net_init int rds_tcp_init_net(struct net *net)
 	}
 	tbl[RDS_TCP_SNDBUF].data = &rtn->sndbuf_size;
 	tbl[RDS_TCP_RCVBUF].data = &rtn->rcvbuf_size;
-	rtn->rds_tcp_sysctl = register_net_sysctl(net, "net/rds/tcp", tbl);
+	rtn->rds_tcp_sysctl = register_net_sysctl_sz(net, "net/rds/tcp", tbl,
+						     ARRAY_SIZE(rds_tcp_sysctl_table));
 	if (!rtn->rds_tcp_sysctl) {
 		pr_warn("could not register sysctl\n");
 		err = -ENOMEM;
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index a7a9136198fd..f65d6f92afcb 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -612,7 +612,9 @@ int sctp_sysctl_net_register(struct net *net)
 	table[SCTP_PF_RETRANS_IDX].extra2 = &net->sctp.ps_retrans;
 	table[SCTP_PS_RETRANS_IDX].extra1 = &net->sctp.pf_retrans;
 
-	net->sctp.sysctl_header = register_net_sysctl(net, "net/sctp", table);
+	net->sctp.sysctl_header = register_net_sysctl_sz(net, "net/sctp",
+							 table,
+							 ARRAY_SIZE(sctp_net_table));
 	if (net->sctp.sysctl_header == NULL) {
 		kfree(table);
 		return -ENOMEM;
diff --git a/net/smc/smc_sysctl.c b/net/smc/smc_sysctl.c
index b6f79fabb9d3..3ab2d8eefc55 100644
--- a/net/smc/smc_sysctl.c
+++ b/net/smc/smc_sysctl.c
@@ -81,7 +81,8 @@ int __net_init smc_sysctl_net_init(struct net *net)
 			table[i].data += (void *)net - (void *)&init_net;
 	}
 
-	net->smc.smc_hdr = register_net_sysctl(net, "net/smc", table);
+	net->smc.smc_hdr = register_net_sysctl_sz(net, "net/smc", table,
+						  ARRAY_SIZE(smc_table));
 	if (!net->smc.smc_hdr)
 		goto err_reg;
 
diff --git a/net/unix/sysctl_net_unix.c b/net/unix/sysctl_net_unix.c
index 500129aa710c..3e84b31c355a 100644
--- a/net/unix/sysctl_net_unix.c
+++ b/net/unix/sysctl_net_unix.c
@@ -36,7 +36,8 @@ int __net_init unix_sysctl_register(struct net *net)
 		table[0].data = &net->unx.sysctl_max_dgram_qlen;
 	}
 
-	net->unx.ctl = register_net_sysctl(net, "net/unix", table);
+	net->unx.ctl = register_net_sysctl_sz(net, "net/unix", table,
+					      ARRAY_SIZE(unix_table));
 	if (net->unx.ctl == NULL)
 		goto err_reg;
 
diff --git a/net/xfrm/xfrm_sysctl.c b/net/xfrm/xfrm_sysctl.c
index 0c6c5ef65f9d..7fdeafc838a7 100644
--- a/net/xfrm/xfrm_sysctl.c
+++ b/net/xfrm/xfrm_sysctl.c
@@ -44,6 +44,7 @@ static struct ctl_table xfrm_table[] = {
 int __net_init xfrm_sysctl_init(struct net *net)
 {
 	struct ctl_table *table;
+	size_t table_size = ARRAY_SIZE(xfrm_table);
 
 	__xfrm_sysctl_init(net);
 
@@ -56,10 +57,13 @@ int __net_init xfrm_sysctl_init(struct net *net)
 	table[3].data = &net->xfrm.sysctl_acq_expires;
 
 	/* Don't export sysctls to unprivileged users */
-	if (net->user_ns != &init_user_ns)
+	if (net->user_ns != &init_user_ns) {
 		table[0].procname = NULL;
+		table_size = 0;
+	}
 
-	net->xfrm.sysctl_hdr = register_net_sysctl(net, "net/core", table);
+	net->xfrm.sysctl_hdr = register_net_sysctl_sz(net, "net/core", table,
+						      table_size);
 	if (!net->xfrm.sysctl_hdr)
 		goto out_register;
 	return 0;
-- 
2.30.2

Re: [PATCH 11/14] networking: Update to register_net_sysctl_sz

[Luis Chamberlain]

On Wed, Jul 26, 2023 at 04:06:31PM +0200, Joel Granados wrote:
> This is part of the effort to remove the sentinel (last empty) element
> from the ctl_table arrays. We update to the new function and pass it the
> array size. Care is taken to mirror the NULL assignments with a size of
> zero (for the unprivileged users). An additional size function was added
> to the following files in order to calculate the size of an array that
> is defined in another file:
>     include/net/ipv6.h
>     net/ipv6/icmp.c
>     net/ipv6/route.c
>     net/ipv6/sysctl_net_ipv6.c
> 

Same here as with the other patches, the "why" and size impact should go here.
I'll skip mentioning that in the other patches.

> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index bf6e81d56263..5bad14b3c71e 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -1396,6 +1396,40 @@ static const struct ctl_table mpls_dev_table[] = {
>  	{ }
>  };
>  
> +static int mpls_platform_labels(struct ctl_table *table, int write,
> +				void *buffer, size_t *lenp, loff_t *ppos);
> +#define MPLS_NS_SYSCTL_OFFSET(field)		\
> +	(&((struct net *)0)->field)
> +
> +static const struct ctl_table mpls_table[] = {
> +	{
> +		.procname	= "platform_labels",
> +		.data		= NULL,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= mpls_platform_labels,
> +	},
> +	{
> +		.procname	= "ip_ttl_propagate",
> +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.ip_ttl_propagate),
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= SYSCTL_ZERO,
> +		.extra2		= SYSCTL_ONE,
> +	},
> +	{
> +		.procname	= "default_ttl",
> +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.default_ttl),
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= SYSCTL_ONE,
> +		.extra2		= &ttl_max,
> +	},
> +	{ }
> +};

Unless we hear otherwise from networking folks, I think this move alone
should probably go as a separate patch with no functional changes to
make the changes easier to review / bisect.

  Luis

Re: [PATCH 11/14] networking: Update to register_net_sysctl_sz

[David Ahern]

On 7/26/23 12:05 PM, Luis Chamberlain wrote:
> On Wed, Jul 26, 2023 at 04:06:31PM +0200, Joel Granados wrote:
>> This is part of the effort to remove the sentinel (last empty) element
>> from the ctl_table arrays. We update to the new function and pass it the
>> array size. Care is taken to mirror the NULL assignments with a size of
>> zero (for the unprivileged users). An additional size function was added
>> to the following files in order to calculate the size of an array that
>> is defined in another file:
>>     include/net/ipv6.h
>>     net/ipv6/icmp.c
>>     net/ipv6/route.c
>>     net/ipv6/sysctl_net_ipv6.c
>>
> 
> Same here as with the other patches, the "why" and size impact should go here.
> I'll skip mentioning that in the other patches.
> 
>> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
>> index bf6e81d56263..5bad14b3c71e 100644
>> --- a/net/mpls/af_mpls.c
>> +++ b/net/mpls/af_mpls.c
>> @@ -1396,6 +1396,40 @@ static const struct ctl_table mpls_dev_table[] = {
>>  	{ }
>>  };
>>  
>> +static int mpls_platform_labels(struct ctl_table *table, int write,
>> +				void *buffer, size_t *lenp, loff_t *ppos);
>> +#define MPLS_NS_SYSCTL_OFFSET(field)		\
>> +	(&((struct net *)0)->field)
>> +
>> +static const struct ctl_table mpls_table[] = {
>> +	{
>> +		.procname	= "platform_labels",
>> +		.data		= NULL,
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0644,
>> +		.proc_handler	= mpls_platform_labels,
>> +	},
>> +	{
>> +		.procname	= "ip_ttl_propagate",
>> +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.ip_ttl_propagate),
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0644,
>> +		.proc_handler	= proc_dointvec_minmax,
>> +		.extra1		= SYSCTL_ZERO,
>> +		.extra2		= SYSCTL_ONE,
>> +	},
>> +	{
>> +		.procname	= "default_ttl",
>> +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.default_ttl),
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0644,
>> +		.proc_handler	= proc_dointvec_minmax,
>> +		.extra1		= SYSCTL_ONE,
>> +		.extra2		= &ttl_max,
>> +	},
>> +	{ }
>> +};
> 
> Unless we hear otherwise from networking folks, I think this move alone
> should probably go as a separate patch with no functional changes to
> make the changes easier to review / bisect.
> 

+1

Re: [PATCH 11/14] networking: Update to register_net_sysctl_sz

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2492 bytes --]

On Wed, Jul 26, 2023 at 11:05:24AM -0700, Luis Chamberlain wrote:
> On Wed, Jul 26, 2023 at 04:06:31PM +0200, Joel Granados wrote:
> > This is part of the effort to remove the sentinel (last empty) element
> > from the ctl_table arrays. We update to the new function and pass it the
> > array size. Care is taken to mirror the NULL assignments with a size of
> > zero (for the unprivileged users). An additional size function was added
> > to the following files in order to calculate the size of an array that
> > is defined in another file:
> >     include/net/ipv6.h
> >     net/ipv6/icmp.c
> >     net/ipv6/route.c
> >     net/ipv6/sysctl_net_ipv6.c
> > 
> 
> Same here as with the other patches, the "why" and size impact should go here.
> I'll skip mentioning that in the other patches.
Indeed. FYI: I answered in your original feedback.

> 
> > diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> > index bf6e81d56263..5bad14b3c71e 100644
> > --- a/net/mpls/af_mpls.c
> > +++ b/net/mpls/af_mpls.c
> > @@ -1396,6 +1396,40 @@ static const struct ctl_table mpls_dev_table[] = {
> >  	{ }
> >  };
> >  
> > +static int mpls_platform_labels(struct ctl_table *table, int write,
> > +				void *buffer, size_t *lenp, loff_t *ppos);
> > +#define MPLS_NS_SYSCTL_OFFSET(field)		\
> > +	(&((struct net *)0)->field)
> > +
> > +static const struct ctl_table mpls_table[] = {
> > +	{
> > +		.procname	= "platform_labels",
> > +		.data		= NULL,
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= mpls_platform_labels,
> > +	},
> > +	{
> > +		.procname	= "ip_ttl_propagate",
> > +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.ip_ttl_propagate),
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1		= SYSCTL_ZERO,
> > +		.extra2		= SYSCTL_ONE,
> > +	},
> > +	{
> > +		.procname	= "default_ttl",
> > +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.default_ttl),
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1		= SYSCTL_ONE,
> > +		.extra2		= &ttl_max,
> > +	},
> > +	{ }
> > +};
> 
> Unless we hear otherwise from networking folks, I think this move alone
> should probably go as a separate patch with no functional changes to
> make the changes easier to review / bisect.

Ok. I'll take it out in its own commit and prepend it as a preparation
patch.

thx for the feedback
Best

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 11/14] networking: Update to register_net_sysctl_sz

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]

On Wed, Jul 26, 2023 at 11:05:24AM -0700, Luis Chamberlain wrote:
> On Wed, Jul 26, 2023 at 04:06:31PM +0200, Joel Granados wrote:
> > This is part of the effort to remove the sentinel (last empty) element
> > from the ctl_table arrays. We update to the new function and pass it the
> > array size. Care is taken to mirror the NULL assignments with a size of
> > zero (for the unprivileged users). An additional size function was added
> > to the following files in order to calculate the size of an array that
> > is defined in another file:
> >     include/net/ipv6.h
> >     net/ipv6/icmp.c
> >     net/ipv6/route.c
> >     net/ipv6/sysctl_net_ipv6.c
> > 
> 
> Same here as with the other patches, the "why" and size impact should go here.
> I'll skip mentioning that in the other patches.
> 
> > diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> > index bf6e81d56263..5bad14b3c71e 100644
> > --- a/net/mpls/af_mpls.c
> > +++ b/net/mpls/af_mpls.c
> > @@ -1396,6 +1396,40 @@ static const struct ctl_table mpls_dev_table[] = {
> >  	{ }
> >  };
> >  
> > +static int mpls_platform_labels(struct ctl_table *table, int write,
> > +				void *buffer, size_t *lenp, loff_t *ppos);
> > +#define MPLS_NS_SYSCTL_OFFSET(field)		\
> > +	(&((struct net *)0)->field)
> > +
> > +static const struct ctl_table mpls_table[] = {
> > +	{
> > +		.procname	= "platform_labels",
> > +		.data		= NULL,
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= mpls_platform_labels,
> > +	},
> > +	{
> > +		.procname	= "ip_ttl_propagate",
> > +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.ip_ttl_propagate),
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1		= SYSCTL_ZERO,
> > +		.extra2		= SYSCTL_ONE,
> > +	},
> > +	{
> > +		.procname	= "default_ttl",
> > +		.data		= MPLS_NS_SYSCTL_OFFSET(mpls.default_ttl),
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1		= SYSCTL_ONE,
> > +		.extra2		= &ttl_max,
> > +	},
> > +	{ }
> > +};
> 
> Unless we hear otherwise from networking folks, I think this move alone
> should probably go as a separate patch with no functional changes to
> make the changes easier to review / bisect.
On further inspection, I have dropped this part of the patch as there is
no real reason to move the mpls_table up the file. I'll comment this in
the new cover letter
> 
>   Luis

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 12/14] vrf: Update to register_net_sysctl_sz

[Joel Granados]

This is part of the effort to remove the sentinel (last empty) element
from the ctl_table arrays. We update to the new function and pass it the
array size.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 drivers/net/vrf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index bdb3a76a352e..f4c3df15a0e5 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1979,7 +1979,8 @@ static int vrf_netns_init_sysctl(struct net *net, struct netns_vrf *nn_vrf)
 	/* init the extra1 parameter with the reference to current netns */
 	table[0].extra1 = net;
 
-	nn_vrf->ctl_hdr = register_net_sysctl(net, "net/vrf", table);
+	nn_vrf->ctl_hdr = register_net_sysctl_sz(net, "net/vrf", table,
+						 ARRAY_SIZE(vrf_table));
 	if (!nn_vrf->ctl_hdr) {
 		kfree(table);
 		return -ENOMEM;
-- 
2.30.2

[PATCH 13/14] sysctl: SIZE_MAX->ARRAY_SIZE in register_net_sysctl

[Joel Granados]

This is part of the effort to remove the end element (sentinel) from the
ctl_table arrays. Now that all the callers to register_net_sysctl are
actual arrays, we replace SIZE_MAX with ARRAY_SIZE of the table.

Stopping condition continues to be based on both size and procname null
test. This sets things up for when the sentinel element is actually
removed: Before removing sentinel the stopping criteria will be the last
null element. When the sentinel is removed then the (correct) size will
take over.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 include/net/net_namespace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index e4e5fe75a281..75dba309e043 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -470,7 +470,7 @@ void unregister_pernet_device(struct pernet_operations *);
 struct ctl_table;
 
 #define register_net_sysctl(net, path, table)	\
-	register_net_sysctl_sz(net, path, table, SIZE_MAX)
+	register_net_sysctl_sz(net, path, table, ARRAY_SIZE(table))
 #ifdef CONFIG_SYSCTL
 int net_sysctl_init(void);
 struct ctl_table_header *register_net_sysctl_sz(struct net *net, const char *path,
-- 
2.30.2

Re: [PATCH 00/14] sysctl: Add a size argument to register functions in sysctl

[Luis Chamberlain]

On Wed, Jul 26, 2023 at 04:06:20PM +0200, Joel Granados wrote:
> What?
> These commits set things up so we can start removing the sentinel elements.

Yes but the why must explained right away.

> Why?
> This is part of the push to trim down kernel/sysctl.c by moving the large array
> that was causing merge conflicts. 

Let me elaborate on that:

While the move moving over time of array elements out of kernel/sysctl.c
to their own place helps merge conflicts this patch set does not help
with that in and of itself, what it does is help make sure the move of
sysctls to their own files does not bloat the kernel more, and in fact
helps reduce the overall build time size of the kernel and run time
memory consumed by the kernel by about ~64 bytes per array.

Without this patch set each time we moved a set of sysctls out of
kernel/sysctl.c to its own subsystem we'd have to add a new sentinel
element (an empty sysctl entry), and while that helps clean up
kernel/sysctl.c to avoid merge conflicts, it also bloats the kernel
by about 64 bytes on average each time.

We can do better. We can make those moves *not* have a size penalty, and
all around also reduce the build / run time of the kernel.

*This* is the why, that if we don't do this the cleanup of
kernel/sysctl.c ends up slowly bloating the kernel. Willy had
suggested we instead remove the sentinel so that each move does not
incur a size penalty, but also that in turn reduces the size of the
kernel at build time / run time by a ballpark about ~64 bytes per
array.

Then the following is more details about estimates of overall size
savings, it's not miscellaneous information at all, it's very relevant
information to this patch set.

> Misc:
> A consequence of eventually removing all the sentinels (64 bytes per sentinel)
> is the bytes we save. Here I include numbers for when all sentinels are removed
> to contextualize this chunk
>   * bloat-o-meter:
>     The "yesall" configuration results save 9158 bytes (you can see the output here
>     https://lore.kernel.org/all/20230621091000.424843-1-j.granados@samsung.com/.
>     The "tiny" configuration + CONFIG_SYSCTL save 1215 bytes (you can see the
>     output here [2])
>   * memory usage:
>     As we no longer need the sentinel element within proc_sysctl.c, we save some
>     bytes in main memory as well. In my testing kernel I measured a difference of
>     6720 bytes. I include the way to measure this in [1]

  Luis

Re: [PATCH 00/14] sysctl: Add a size argument to register functions in sysctl

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 3312 bytes --]

On Wed, Jul 26, 2023 at 11:15:40AM -0700, Luis Chamberlain wrote:
> On Wed, Jul 26, 2023 at 04:06:20PM +0200, Joel Granados wrote:
> > What?
> > These commits set things up so we can start removing the sentinel elements.
> 
> Yes but the why must explained right away.
My intention of putting the "what" first was to explain the chunking and
clarify right away that what was contained in the "why" was not for this
chunk but for the *whole* patchset.

I have swapped them in my cover letter as I can see that the ambiguity
is gone once you start reading the "what"

> 
> > Why?
> > This is part of the push to trim down kernel/sysctl.c by moving the large array
> > that was causing merge conflicts. 
> 
> Let me elaborate on that:
> 
> While the move moving over time of array elements out of kernel/sysctl.c
> to their own place helps merge conflicts this patch set does not help
> with that in and of itself, what it does is help make sure the move of
> sysctls to their own files does not bloat the kernel more, and in fact
> helps reduce the overall build time size of the kernel and run time
> memory consumed by the kernel by about ~64 bytes per array.
> 
> Without this patch set each time we moved a set of sysctls out of
> kernel/sysctl.c to its own subsystem we'd have to add a new sentinel
> element (an empty sysctl entry), and while that helps clean up
> kernel/sysctl.c to avoid merge conflicts, it also bloats the kernel
> by about 64 bytes on average each time.
> 
> We can do better. We can make those moves *not* have a size penalty, and
> all around also reduce the build / run time of the kernel.
> 
> *This* is the why, that if we don't do this the cleanup of
> kernel/sysctl.c ends up slowly bloating the kernel. Willy had
> suggested we instead remove the sentinel so that each move does not
> incur a size penalty, but also that in turn reduces the size of the
> kernel at build time / run time by a ballpark about ~64 bytes per
> array.
Thx for this.
This is a more clear wording for the "Why". Do you mind if I copy/paste
it (with some changes to make it flow) into my next cover letter?

> 
> Then the following is more details about estimates of overall size
> savings, it's not miscellaneous information at all, it's very relevant
> information to this patch set.
Did not mean to downplay the importance here. Just did not have a good
title for the section. I'll change it to "Size saving estimates".
> 
> > Misc:
> > A consequence of eventually removing all the sentinels (64 bytes per sentinel)
> > is the bytes we save. Here I include numbers for when all sentinels are removed
> > to contextualize this chunk
> >   * bloat-o-meter:
> >     The "yesall" configuration results save 9158 bytes (you can see the output here
> >     https://lore.kernel.org/all/20230621091000.424843-1-j.granados@samsung.com/.
> >     The "tiny" configuration + CONFIG_SYSCTL save 1215 bytes (you can see the
> >     output here [2])
> >   * memory usage:
> >     As we no longer need the sentinel element within proc_sysctl.c, we save some
> >     bytes in main memory as well. In my testing kernel I measured a difference of
> >     6720 bytes. I include the way to measure this in [1]
> 
>   Luis

-- 

best

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/14] sysctl: Add a size argument to register functions in sysctl

[Luis Chamberlain]

On Thu, Jul 27, 2023 at 01:43:18PM +0200, Joel Granados wrote:
> On Wed, Jul 26, 2023 at 11:15:40AM -0700, Luis Chamberlain wrote:
> > On Wed, Jul 26, 2023 at 04:06:20PM +0200, Joel Granados wrote:
> > > What?
> > > These commits set things up so we can start removing the sentinel elements.
> > 
> > Yes but the why must explained right away.
> My intention of putting the "what" first was to explain the chunking and

It may help also just to clarify:

   sentinel, the extra empty struct ctl_table at the end of each
   sysctl array.

> Thx for this.
> This is a more clear wording for the "Why". Do you mind if I copy/paste
> it (with some changes to make it flow) into my next cover letter?

I don't mind at all.

  Luis

Re: [PATCH 00/14] sysctl: Add a size argument to register functions in sysctl

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

On Thu, Jul 27, 2023 at 08:39:59AM -0700, Luis Chamberlain wrote:
> On Thu, Jul 27, 2023 at 01:43:18PM +0200, Joel Granados wrote:
> > On Wed, Jul 26, 2023 at 11:15:40AM -0700, Luis Chamberlain wrote:
> > > On Wed, Jul 26, 2023 at 04:06:20PM +0200, Joel Granados wrote:
> > > > What?
> > > > These commits set things up so we can start removing the sentinel elements.
> > > 
> > > Yes but the why must explained right away.
> > My intention of putting the "what" first was to explain the chunking and
> 
> It may help also just to clarify:
> 
>    sentinel, the extra empty struct ctl_table at the end of each
>    sysctl array.
This clarification is already there: "... sentinel element (the extra
empty element in the ctl_table arrays ...".

> 
> > Thx for this.
> > This is a more clear wording for the "Why". Do you mind if I copy/paste
> > it (with some changes to make it flow) into my next cover letter?
> 
> I don't mind at all.
> 
>   Luis

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]