| Message ID | 20240501-jag-sysctl_remset_net-v6-8-370b702b6b4a@samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 78a7b5dbc0609b8c395f754953a45db687cb05e6 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v6,1/8] net: Remove the now superfluous sentinel elements from ctl_table array | expand |
2024-05-01, 11:29:32 +0200, Joel Granados via B4 Relay wrote: > 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/) > > Avoid a buffer overflow when traversing the ctl_table by ensuring that > AX25_MAX_VALUES is the same as the size of ax25_param_table. This is > done with a BUILD_BUG_ON where ax25_param_table is defined and a > CONFIG_AX25_DAMA_SLAVE guard in the unnamed enum definition as well as > in the ax25_dev_device_up and ax25_ds_set_timer functions. ^^ nit: not anymore ;) (but not worth a repost IMO) > diff --git a/net/ax25/ax25_ds_timer.c b/net/ax25/ax25_ds_timer.c > index c4f8adbf8144..c50a58d9e368 100644 > --- a/net/ax25/ax25_ds_timer.c > +++ b/net/ax25/ax25_ds_timer.c > @@ -55,6 +55,7 @@ void ax25_ds_set_timer(ax25_dev *ax25_dev) > ax25_dev->dama.slave_timeout = > msecs_to_jiffies(ax25_dev->values[AX25_VALUES_DS_TIMEOUT]) / 10; > mod_timer(&ax25_dev->dama.slave_timer, jiffies + HZ); > + return; nit: return not needed here since we're already at the bottom of the function, but probably not worth a repost of the series. > }
On Wed, May 01, 2024 at 03:15:54PM +0200, Sabrina Dubroca wrote: > 2024-05-01, 11:29:32 +0200, Joel Granados via B4 Relay wrote: > > 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/) > > > > Avoid a buffer overflow when traversing the ctl_table by ensuring that > > AX25_MAX_VALUES is the same as the size of ax25_param_table. This is > > done with a BUILD_BUG_ON where ax25_param_table is defined and a > > CONFIG_AX25_DAMA_SLAVE guard in the unnamed enum definition as well as > > in the ax25_dev_device_up and ax25_ds_set_timer functions. > ^^ > nit: not anymore ;) > (but not worth a repost IMO) > > > > diff --git a/net/ax25/ax25_ds_timer.c b/net/ax25/ax25_ds_timer.c > > index c4f8adbf8144..c50a58d9e368 100644 > > --- a/net/ax25/ax25_ds_timer.c > > +++ b/net/ax25/ax25_ds_timer.c > > @@ -55,6 +55,7 @@ void ax25_ds_set_timer(ax25_dev *ax25_dev) > > ax25_dev->dama.slave_timeout = > > msecs_to_jiffies(ax25_dev->values[AX25_VALUES_DS_TIMEOUT]) / 10; > > mod_timer(&ax25_dev->dama.slave_timer, jiffies + HZ); > > + return; > > nit: return not needed here since we're already at the bottom of the > function, but probably not worth a repost of the series. > Thx. I will not repost, but I have changed them locally so they are there in case a V7 is required. Best
On Fri, May 03, 2024 at 02:18:11PM +0200, Joel Granados wrote: > On Wed, May 01, 2024 at 03:15:54PM +0200, Sabrina Dubroca wrote: > > 2024-05-01, 11:29:32 +0200, Joel Granados via B4 Relay wrote: > > > From: Joel Granados <j.granados@samsung.com> > > > diff --git a/net/ax25/ax25_ds_timer.c b/net/ax25/ax25_ds_timer.c > > > index c4f8adbf8144..c50a58d9e368 100644 > > > --- a/net/ax25/ax25_ds_timer.c > > > +++ b/net/ax25/ax25_ds_timer.c > > > @@ -55,6 +55,7 @@ void ax25_ds_set_timer(ax25_dev *ax25_dev) > > > ax25_dev->dama.slave_timeout = > > > msecs_to_jiffies(ax25_dev->values[AX25_VALUES_DS_TIMEOUT]) / 10; > > > mod_timer(&ax25_dev->dama.slave_timer, jiffies + HZ); > > > + return; > > > > nit: return not needed here since we're already at the bottom of the > > function, but probably not worth a repost of the series. > > > Thx. I will not repost, but I have changed them locally so they are > there in case a V7 is required. > It's a checkpatch.pl -f warning so we probably will want to fix it eventually. regards, dan carpenter
On Fri, May 03, 2024 at 06:23:14PM +0300, Dan Carpenter wrote: > On Fri, May 03, 2024 at 02:18:11PM +0200, Joel Granados wrote: > > On Wed, May 01, 2024 at 03:15:54PM +0200, Sabrina Dubroca wrote: > > > 2024-05-01, 11:29:32 +0200, Joel Granados via B4 Relay wrote: > > > > From: Joel Granados <j.granados@samsung.com> > > > > diff --git a/net/ax25/ax25_ds_timer.c b/net/ax25/ax25_ds_timer.c > > > > index c4f8adbf8144..c50a58d9e368 100644 > > > > --- a/net/ax25/ax25_ds_timer.c > > > > +++ b/net/ax25/ax25_ds_timer.c > > > > @@ -55,6 +55,7 @@ void ax25_ds_set_timer(ax25_dev *ax25_dev) > > > > ax25_dev->dama.slave_timeout = > > > > msecs_to_jiffies(ax25_dev->values[AX25_VALUES_DS_TIMEOUT]) / 10; > > > > mod_timer(&ax25_dev->dama.slave_timer, jiffies + HZ); > > > > + return; > > > > > > nit: return not needed here since we're already at the bottom of the > > > function, but probably not worth a repost of the series. > > > > > Thx. I will not repost, but I have changed them locally so they are > > there in case a V7 is required. > > > > It's a checkpatch.pl -f warning so we probably will want to fix it > eventually. According to [1] the patchset has already been applied. So I'll just send another patch for it to be applied on top. Thx for pointing this out. [1] https://patchwork.kernel.org/project/netdevbpf/patch/20240501-jag-sysctl_remset_net-v6-1-370b702b6b4a@samsung.com/
diff --git a/include/net/ax25.h b/include/net/ax25.h index 0d939e5aee4e..eb9cee8252c8 100644 --- a/include/net/ax25.h +++ b/include/net/ax25.h @@ -139,7 +139,9 @@ enum { AX25_VALUES_N2, /* Default N2 value */ AX25_VALUES_PACLEN, /* AX.25 MTU */ AX25_VALUES_PROTOCOL, /* Std AX.25, DAMA Slave, DAMA Master */ +#ifdef CONFIG_AX25_DAMA_SLAVE AX25_VALUES_DS_TIMEOUT, /* DAMA Slave timeout */ +#endif AX25_MAX_VALUES /* THIS MUST REMAIN THE LAST ENTRY OF THIS LIST */ }; diff --git a/net/ax25/ax25_dev.c b/net/ax25/ax25_dev.c index 282ec581c072..0bc682ffae9c 100644 --- a/net/ax25/ax25_dev.c +++ b/net/ax25/ax25_dev.c @@ -78,7 +78,10 @@ void ax25_dev_device_up(struct net_device *dev) ax25_dev->values[AX25_VALUES_N2] = AX25_DEF_N2; ax25_dev->values[AX25_VALUES_PACLEN] = AX25_DEF_PACLEN; ax25_dev->values[AX25_VALUES_PROTOCOL] = AX25_DEF_PROTOCOL; + +#ifdef CONFIG_AX25_DAMA_SLAVE ax25_dev->values[AX25_VALUES_DS_TIMEOUT]= AX25_DEF_DS_TIMEOUT; +#endif #if defined(CONFIG_AX25_DAMA_SLAVE) || defined(CONFIG_AX25_DAMA_MASTER) ax25_ds_setup_timer(ax25_dev); diff --git a/net/ax25/ax25_ds_timer.c b/net/ax25/ax25_ds_timer.c index c4f8adbf8144..c50a58d9e368 100644 --- a/net/ax25/ax25_ds_timer.c +++ b/net/ax25/ax25_ds_timer.c @@ -55,6 +55,7 @@ void ax25_ds_set_timer(ax25_dev *ax25_dev) ax25_dev->dama.slave_timeout = msecs_to_jiffies(ax25_dev->values[AX25_VALUES_DS_TIMEOUT]) / 10; mod_timer(&ax25_dev->dama.slave_timer, jiffies + HZ); + return; } /* diff --git a/net/ax25/sysctl_net_ax25.c b/net/ax25/sysctl_net_ax25.c index e0128dc9def3..68753aa30334 100644 --- a/net/ax25/sysctl_net_ax25.c +++ b/net/ax25/sysctl_net_ax25.c @@ -141,8 +141,6 @@ static const struct ctl_table ax25_param_table[] = { .extra2 = &max_ds_timeout }, #endif - - { } /* that's all, folks! */ }; int ax25_register_dev_sysctl(ax25_dev *ax25_dev) @@ -155,6 +153,7 @@ int ax25_register_dev_sysctl(ax25_dev *ax25_dev) if (!table) return -ENOMEM; + BUILD_BUG_ON(ARRAY_SIZE(ax25_param_table) != AX25_MAX_VALUES); for (k = 0; k < AX25_MAX_VALUES; k++) table[k].data = &ax25_dev->values[k]; diff --git a/net/x25/sysctl_net_x25.c b/net/x25/sysctl_net_x25.c index e9802afa43d0..643f50874dfe 100644 --- a/net/x25/sysctl_net_x25.c +++ b/net/x25/sysctl_net_x25.c @@ -71,7 +71,6 @@ static struct ctl_table x25_table[] = { .mode = 0644, .proc_handler = proc_dointvec, }, - { }, }; int __init x25_register_sysctl(void)