diff mbox series

[v4,1/8] net: Remove the now superfluous sentinel elements from ctl_table array

Message ID 20240425-jag-sysctl_remset_net-v4-1-9e82f985777d@samsung.com (mailing list archive)
State Superseded
Headers show
Series [v4,1/8] net: Remove the now superfluous sentinel elements from ctl_table array | expand

Commit Message

Joel Granados via B4 Relay April 25, 2024, 12:02 p.m. UTC 
From: Joel Granados <j.granados@samsung.com>

This commit comes at the tail end of a greater effort to remove the
empty elements at the end of the ctl_table arrays (sentinels) which
will reduce the overall build time size of the kernel and run time
memory bloat by ~64 bytes per sentinel (further information Link :
https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/)

* Remove sentinel element from ctl_table structs.
* Remove the zeroing out of an array element (to make it look like a
  sentinel) in neigh_sysctl_register and lowpan_frags_ns_sysctl_register
  This is not longer needed and is safe after commit c899710fe7f9
  ("networking: Update to register_net_sysctl_sz") added the array size
  to the ctl_table registration.
* Replace the for loop stop condition in sysctl_core_net_init that tests
  for procname == NULL with one that depends on array size
* Removed the "-1" in mpls_net_init that adjusted for having an extra
  empty element when looping over ctl_table arrays
* Use a table_size variable to keep the value of ARRAY_SIZE

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 net/core/neighbour.c                |  5 +----
 net/core/sysctl_net_core.c          | 12 +++++-------
 net/dccp/sysctl.c                   |  2 --
 net/ieee802154/6lowpan/reassembly.c |  6 +-----
 net/mpls/af_mpls.c                  | 12 ++++++------
 net/unix/sysctl_net_unix.c          |  1 -
 6 files changed, 13 insertions(+), 25 deletions(-)

Comments

Jakub Kicinski April 25, 2024, 10:58 p.m. UTC | #1 
On Thu, 25 Apr 2024 14:02:59 +0200 Joel Granados via B4 Relay wrote:
> -	for (i = 0; i < ARRAY_SIZE(mpls_table) - 1; i++)
> +	for (i = 0; i < tabel_size; i++)
>  		table[i].data = (char *)net + (uintptr_t)table[i].data;
>  
>  	net->mpls.ctl = register_net_sysctl_sz(net, "net/mpls", table,
> -					       ARRAY_SIZE(mpls_table));
> +					       tabel_size);

../net/mpls/af_mpls.c: In function ‘mpls_net_init’:
../net/mpls/af_mpls.c:2676:25: error: ‘tabel_size’ undeclared (first use in this function); did you mean ‘table_size’?
 2676 |         for (i = 0; i < tabel_size; i++)
      |                         ^~~~~~~~~~
      |                         table_size
../net/mpls/af_mpls.c:2676:25: note: each undeclared identifier is reported only once for each function it appears in
../net/mpls/af_mpls.c:2660:16: warning: unused variable ‘table_size’ [-Wunused-variable]
 2660 |         size_t table_size = ARRAY_SIZE(mpls_table);
      |                ^~~~~~~~~~
Joel Granados April 26, 2024, 6:59 a.m. UTC | #2 
On Thu, Apr 25, 2024 at 03:58:04PM -0700, Jakub Kicinski wrote:
> On Thu, 25 Apr 2024 14:02:59 +0200 Joel Granados via B4 Relay wrote:
> > -	for (i = 0; i < ARRAY_SIZE(mpls_table) - 1; i++)
> > +	for (i = 0; i < tabel_size; i++)
> >  		table[i].data = (char *)net + (uintptr_t)table[i].data;
> >  
> >  	net->mpls.ctl = register_net_sysctl_sz(net, "net/mpls", table,
> > -					       ARRAY_SIZE(mpls_table));
> > +					       tabel_size);
> 
> ../net/mpls/af_mpls.c: In function ‘mpls_net_init’:
> ../net/mpls/af_mpls.c:2676:25: error: ‘tabel_size’ undeclared (first use in this function); did you mean ‘table_size’?
>  2676 |         for (i = 0; i < tabel_size; i++)
>       |                         ^~~~~~~~~~
>       |                         table_size
> ../net/mpls/af_mpls.c:2676:25: note: each undeclared identifier is reported only once for each function it appears in
> ../net/mpls/af_mpls.c:2660:16: warning: unused variable ‘table_size’ [-Wunused-variable]
>  2660 |         size_t table_size = ARRAY_SIZE(mpls_table);
Sorry about this. I pulled the trigger way too early. This is already
fixed in my v4.
>       |                ^~~~~~~~~~
> -- 
> netdev FAQ tl;dr:
>  - designate your patch to a tree - [PATCH net] or [PATCH net-next]
>  - for fixes the Fixes: tag is required, regardless of the tree
>  - don't post large series (> 15 patches), break them up
>  - don't repost your patches within one 24h period
> 
> pw-bot: cr
Jakub Kicinski April 26, 2024, 2:19 p.m. UTC | #3 
On Fri, 26 Apr 2024 08:59:31 +0200 Joel Granados wrote:
> Sorry about this. I pulled the trigger way too early. This is already
> fixed in my v4.
> >       |                ^~~~~~~~~~
> > -- 
> > netdev FAQ tl;dr:
> >  - designate your patch to a tree - [PATCH net] or [PATCH net-next]
> >  - for fixes the Fixes: tag is required, regardless of the tree
> >  - don't post large series (> 15 patches), break them up
> >  - don't repost your patches within one 24h period

I guess you didn't bother reading the tl;dr I put in here.. SMH.
Joel Granados April 29, 2024, 8:22 a.m. UTC | #4 
On Fri, Apr 26, 2024 at 07:19:44AM -0700, Jakub Kicinski wrote:
> On Fri, 26 Apr 2024 08:59:31 +0200 Joel Granados wrote:
> > Sorry about this. I pulled the trigger way too early. This is already
> > fixed in my v4.
> > >       |                ^~~~~~~~~~
> > > -- 
> > > netdev FAQ tl;dr:
> > >  - designate your patch to a tree - [PATCH net] or [PATCH net-next]
I'll add "net" to my V6. I wont change the my base commit which is
v6.9-rc1.

> > >  - for fixes the Fixes: tag is required, regardless of the tree
There are no actual fixes to mainline commits. So I shouldn't add the
"Fixes" tag on any of these.

> > >  - don't post large series (> 15 patches), break them up
This series is 8 patches, so I'm well bellow the 15 cap value.

> > >  - don't repost your patches within one 24h period
That means that my V5 came around 80 minutes to soon. I'll keep these in
mind for next time.

> 
> I guess you didn't bother reading the tl;dr I put in here.. SMH.
I completely missed it as I thought that it was just another quirky
signature to be ignored:). I put my comments and questions inline.
Joel Granados May 1, 2024, 9:16 a.m. UTC | #5 
On Mon, Apr 29, 2024 at 10:22:19AM +0200, Joel Granados wrote:
> On Fri, Apr 26, 2024 at 07:19:44AM -0700, Jakub Kicinski wrote:
> > On Fri, 26 Apr 2024 08:59:31 +0200 Joel Granados wrote:
> > > Sorry about this. I pulled the trigger way too early. This is already
> > > fixed in my v4.
> > > >       |                ^~~~~~~~~~
> > > > -- 
> > > > netdev FAQ tl;dr:
> > > >  - designate your patch to a tree - [PATCH net] or [PATCH net-next]
> I'll add "net" to my V6. I wont change the my base commit which is
> v6.9-rc1.
Actually, I added net-next and rebased onto net-next/main. Please advise
if I need to rebase to some other branch in
pub/scm/linux/kernel/git/netdev/net-next

Best
diff mbox series

Patch

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 552719c3bbc3..b0327402b3e6 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -3728,7 +3728,7 @@  static int neigh_proc_base_reachable_time(struct ctl_table *ctl, int write,
 
 static struct neigh_sysctl_table {
 	struct ctl_table_header *sysctl_header;
-	struct ctl_table neigh_vars[NEIGH_VAR_MAX + 1];
+	struct ctl_table neigh_vars[NEIGH_VAR_MAX];
 } neigh_sysctl_template __read_mostly = {
 	.neigh_vars = {
 		NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(MCAST_PROBES, "mcast_solicit"),
@@ -3779,7 +3779,6 @@  static struct neigh_sysctl_table {
 			.extra2		= SYSCTL_INT_MAX,
 			.proc_handler	= proc_dointvec_minmax,
 		},
-		{},
 	},
 };
 
@@ -3807,8 +3806,6 @@  int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
 	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 + 1;
 	} else {
 		struct neigh_table *tbl = p->tbl;
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 6973dda3abda..a84690b13bb9 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -660,7 +660,6 @@  static struct ctl_table net_core_table[] = {
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
 	},
-	{ }
 };
 
 static struct ctl_table netns_core_table[] = {
@@ -697,7 +696,6 @@  static struct ctl_table netns_core_table[] = {
 		.extra2		= SYSCTL_ONE,
 		.proc_handler	= proc_dou8vec_minmax,
 	},
-	{ }
 };
 
 static int __init fb_tunnels_only_for_init_net_sysctl_setup(char *str)
@@ -715,7 +713,8 @@  __setup("fb_tunnels=", fb_tunnels_only_for_init_net_sysctl_setup);
 
 static __net_init int sysctl_core_net_init(struct net *net)
 {
-	struct ctl_table *tbl, *tmp;
+	size_t table_size = ARRAY_SIZE(netns_core_table);
+	struct ctl_table *tbl;
 
 	tbl = netns_core_table;
 	if (!net_eq(net, &init_net)) {
@@ -723,12 +722,11 @@  static __net_init int sysctl_core_net_init(struct net *net)
 		if (tbl == NULL)
 			goto err_dup;
 
-		for (tmp = tbl; tmp->procname; tmp++)
-			tmp->data += (char *)net - (char *)&init_net;
+		for (int i = 0; i < table_size; ++i)
+			(tbl + i)->data += (char *)net - (char *)&init_net;
 	}
 
-	net->core.sysctl_hdr = register_net_sysctl_sz(net, "net/core", tbl,
-						      ARRAY_SIZE(netns_core_table));
+	net->core.sysctl_hdr = register_net_sysctl_sz(net, "net/core", tbl, table_size);
 	if (net->core.sysctl_hdr == NULL)
 		goto err_reg;
 
diff --git a/net/dccp/sysctl.c b/net/dccp/sysctl.c
index ee8d4f5afa72..3fc474d6e57d 100644
--- a/net/dccp/sysctl.c
+++ b/net/dccp/sysctl.c
@@ -90,8 +90,6 @@  static struct ctl_table dccp_default_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_ms_jiffies,
 	},
-
-	{ }
 };
 
 static struct ctl_table_header *dccp_table_header;
diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
index 6dd960ec558c..09b18ee6df00 100644
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -338,7 +338,6 @@  static struct ctl_table lowpan_frags_ns_ctl_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_jiffies,
 	},
-	{ }
 };
 
 /* secret interval has been deprecated */
@@ -351,7 +350,6 @@  static struct ctl_table lowpan_frags_ctl_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_jiffies,
 	},
-	{ }
 };
 
 static int __net_init lowpan_frags_ns_sysctl_register(struct net *net)
@@ -370,10 +368,8 @@  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) {
-			table[0].procname = NULL;
+		if (net->user_ns != &init_user_ns)
 			table_size = 0;
-		}
 	}
 
 	table[0].data	= &ieee802154_lowpan->fqdir->high_thresh;
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 6dab883a08dd..1d15081090be 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -1393,13 +1393,13 @@  static const struct ctl_table mpls_dev_table[] = {
 		.proc_handler	= mpls_conf_proc,
 		.data		= MPLS_PERDEV_SYSCTL_OFFSET(input_enabled),
 	},
-	{ }
 };
 
 static int mpls_dev_sysctl_register(struct net_device *dev,
 				    struct mpls_dev *mdev)
 {
 	char path[sizeof("net/mpls/conf/") + IFNAMSIZ];
+	size_t table_size = ARRAY_SIZE(mpls_dev_table);
 	struct net *net = dev_net(dev);
 	struct ctl_table *table;
 	int i;
@@ -1411,7 +1411,7 @@  static int mpls_dev_sysctl_register(struct net_device *dev,
 	/* Table data contains only offsets relative to the base of
 	 * the mdev at this point, so make them absolute.
 	 */
-	for (i = 0; i < ARRAY_SIZE(mpls_dev_table); i++) {
+	for (i = 0; i < table_size; i++) {
 		table[i].data = (char *)mdev + (uintptr_t)table[i].data;
 		table[i].extra1 = mdev;
 		table[i].extra2 = net;
@@ -1419,8 +1419,7 @@  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_sz(net, path, table,
-					      ARRAY_SIZE(mpls_dev_table));
+	mdev->sysctl = register_net_sysctl_sz(net, path, table, table_size);
 	if (!mdev->sysctl)
 		goto free;
 
@@ -2674,6 +2673,7 @@  static const struct ctl_table mpls_table[] = {
 
 static int mpls_net_init(struct net *net)
 {
+	size_t table_size = ARRAY_SIZE(mpls_table);
 	struct ctl_table *table;
 	int i;
 
@@ -2689,11 +2689,11 @@  static int mpls_net_init(struct net *net)
 	/* Table data contains only offsets relative to the base of
 	 * the mdev at this point, so make them absolute.
 	 */
-	for (i = 0; i < ARRAY_SIZE(mpls_table) - 1; i++)
+	for (i = 0; i < tabel_size; i++)
 		table[i].data = (char *)net + (uintptr_t)table[i].data;
 
 	net->mpls.ctl = register_net_sysctl_sz(net, "net/mpls", table,
-					       ARRAY_SIZE(mpls_table));
+					       tabel_size);
 	if (net->mpls.ctl == NULL) {
 		kfree(table);
 		return -ENOMEM;
diff --git a/net/unix/sysctl_net_unix.c b/net/unix/sysctl_net_unix.c
index 3e84b31c355a..ae45d4cfac39 100644
--- a/net/unix/sysctl_net_unix.c
+++ b/net/unix/sysctl_net_unix.c
@@ -19,7 +19,6 @@  static struct ctl_table unix_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
-	{ }
 };
 
 int __net_init unix_sysctl_register(struct net *net)