[PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[Lucian Adrian Grijincu]

Some members of net_device are used only by some protocols.

If those protocols are not compiled (as modules or linked in) they
should not take up space in the structure.

These members are also used in inline functions defined in headers not
protected by the protocol-specific CONFIG_ guards. Adding #ifdef CONFIG_*
guards header wide is against the current style and more invasive than
guarding only the definitions.

This patch includes a fix for an error spotted by Eric Dumazet
<eric.dumazet@gmail.com> regarding mishandlement of modules in a
previous version of the patch.

Signed-off-by: Lucian Adrian Grijincu <lgrijincu@ixiacom.com>
---
 include/linux/inetdevice.h |    2 ++
 include/linux/netdevice.h  |   14 ++++++++++++++
 include/net/ax25.h         |    2 ++
 net/core/dev.c             |    6 ++++++
 4 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index acef2a7..88132ff 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -166,6 +166,7 @@ static __inline__ int bad_mask(__be32 mask, __be32 addr)

 #define endfor_ifa(in_dev) }

+#ifdef CONFIG_INET
 static inline struct in_device *__in_dev_get_rcu(const struct
net_device *dev)
 {
 	struct in_device *in_dev = dev->ip_ptr;
@@ -192,6 +193,7 @@ __in_dev_get_rtnl(const struct net_device *dev)
 {
 	return (struct in_device*)dev->ip_ptr;
 }
+#endif /* CONFIG_INET */

 extern void in_dev_finish_destroy(struct in_device *idev);

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8a17b6c..465c3de 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -762,14 +762,28 @@ struct net_device
 #ifdef CONFIG_NET_DSA
 	void			*dsa_ptr;	/* dsa specific data */
 #endif
+#if defined(CONFIG_ATALK) || defined(CONFIG_ATALK_MODULE)
 	void 			*atalk_ptr;	/* AppleTalk link 	*/
+#endif
+#ifdef CONFIG_INET
 	void			*ip_ptr;	/* IPv4 specific data	*/
+#endif
+#if defined(CONFIG_DECNET) || defined(CONFIG_DECNET_MODULE)
 	void                    *dn_ptr;        /* DECnet specific data */
+#endif
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 	void                    *ip6_ptr;       /* IPv6 specific data */
+#endif
+#if defined(CONFIG_ECONET) || defined(CONFIG_ECONET_MODULE)
 	void			*ec_ptr;	/* Econet specific data	*/
+#endif
+#if defined(CONFIG_AX25) || defined(CONFIG_AX25_MODULE)
 	void			*ax25_ptr;	/* AX.25 specific data */
+#endif
+#ifdef CONFIG_WIRELESS
 	struct wireless_dev	*ieee80211_ptr;	/* IEEE 802.11 specific data,
 						   assign before registering */
+#endif

 /*
  * Cache line mostly used on receive path (including eth_type_trans())
diff --git a/include/net/ax25.h b/include/net/ax25.h
index 717e219..37f86ac 100644
--- a/include/net/ax25.h
+++ b/include/net/ax25.h
@@ -300,10 +300,12 @@ extern void ax25_digi_invert(const ax25_digi *,
ax25_digi *);
 extern ax25_dev *ax25_dev_list;
 extern spinlock_t ax25_dev_lock;

+#if defined(CONFIG_AX25) || defined(CONFIG_AX25_MODULE)
 static inline ax25_dev *ax25_dev_ax25dev(struct net_device *dev)
 {
 	return dev->ax25_ptr;
 }
+#endif

 extern ax25_dev *ax25_addr_ax25dev(ax25_address *);
 extern void ax25_dev_device_up(struct net_device *);
diff --git a/net/core/dev.c b/net/core/dev.c
index e2e9e4a..d8e85ba 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4686,9 +4686,15 @@ void netdev_run_todo(void)

 		/* paranoia */
 		BUG_ON(atomic_read(&dev->refcnt));
+#ifdef CONFIG_INET
 		WARN_ON(dev->ip_ptr);
+#endif
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 		WARN_ON(dev->ip6_ptr);
+#endif
+#if defined(CONFIG_DECNET) || defined(CONFIG_DECNET_MODULE)
 		WARN_ON(dev->dn_ptr);
+#endif

 		if (dev->destructor)
 			dev->destructor(dev);
-- 
1.6.3.3

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[David Miller]

From: Lucian Adrian Grijincu <lgrijincu@ixiacom.com>
Date: Mon, 27 Jul 2009 18:09:08 +0300

> Some members of net_device are used only by some protocols.
> 
> If those protocols are not compiled (as modules or linked in) they
> should not take up space in the structure.

This benefits, at best, %0.000000001 of users of the Linux kernel,
because every distribution is going to turn on every single option.

If you want to shrink structures, find ways to eliminate or
shrink structure members in all cases.

I'm not applying this.

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[Joe Perches]

On Mon, 2009-07-27 at 08:26 -0700, David Miller wrote:
> From: Lucian Adrian Grijincu <lgrijincu@ixiacom.com>
> Date: Mon, 27 Jul 2009 18:09:08 +0300
> > Some members of net_device are used only by some protocols.
> > If those protocols are not compiled (as modules or linked in) they
> > should not take up space in the structure.
> This benefits, at best, %0.000000001 of users of the Linux kernel,
> because every distribution is going to turn on every single option.

I think the cost of maintaining this is small and the
percentage of users that benefit underestimated.

It does help handsets a trivial amount, which are unlikely
to ever enable decnet or econet.

> If you want to shrink structures, find ways to eliminate or
> shrink structure members in all cases.

Good advice.

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Mon, 27 Jul 2009 09:23:01 -0700

> On Mon, 2009-07-27 at 08:26 -0700, David Miller wrote:
>> From: Lucian Adrian Grijincu <lgrijincu@ixiacom.com>
>> Date: Mon, 27 Jul 2009 18:09:08 +0300
>> > Some members of net_device are used only by some protocols.
>> > If those protocols are not compiled (as modules or linked in) they
>> > should not take up space in the structure.
>> This benefits, at best, %0.000000001 of users of the Linux kernel,
>> because every distribution is going to turn on every single option.
> 
> I think the cost of maintaining this is small and the
> percentage of users that benefit underestimated.

Prove it.

Prove that it helps enough to maintain this ifdef abortion in our
header files.

It only gives the false sense of satisfation that we're making our
datastructures less bloated.  We're not, and in fact they keep getting
larger, and if we add these ifdefs the new excuse for bloat will be
"but it gets compiled out on embedded et al.  builds that don't use
this or that feature."

Putting this crap in is just a big smoke screen which gives a
disincentive for making real improvements in this area.

If I thought this was worthwhile I would have done it 10 years ago.

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[Joe Perches]

On Mon, 2009-07-27 at 09:27 -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Mon, 27 Jul 2009 09:23:01 -0700
> > I think the cost of maintaining this is small and the
> > percentage of users that benefit underestimated.
> Prove it.

Cost/bit/knot valuations are arbitrary.

cheers, Joe

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[Octavian Purdila]

On Monday 27 July 2009 18:26:44 David Miller wrote:

> From: Lucian Adrian Grijincu <lgrijincu@ixiacom.com>
> Date: Mon, 27 Jul 2009 18:09:08 +0300
>
> > Some members of net_device are used only by some protocols.
> >
> > If those protocols are not compiled (as modules or linked in) they
> > should not take up space in the structure.
>
> This benefits, at best, %0.000000001 of users of the Linux kernel,
> because every distribution is going to turn on every single option.
>

Not all Linux users are using regular (desktop/server) distributions. Linux is 
used in embedded systems as well and in these case it makes sense to turn off 
ax25/econet/decnet and in some cases even wireless/ipv6.

> If you want to shrink structures, find ways to eliminate or
> shrink structure members in all cases.

We are looking into that as well since we have a pretty aggressive goal (get 
net_device to 450 bytes or so), but we thought of starting with the low 
hanging fruits. 

Any suggestions in this area?

tavi

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[Joe Perches]

On Tue, 2009-07-28 at 17:43 +0300, Octavian Purdila wrote:
> On Monday 27 July 2009 18:26:44 David Miller wrote:
> > From: Lucian Adrian Grijincu <lgrijincu@ixiacom.com>
> > > Some members of net_device are used only by some protocols.
> > > If those protocols are not compiled (as modules or linked in) they
> > > should not take up space in the structure.
> > This benefits, at best, %0.000000001 of users of the Linux kernel,
> > because every distribution is going to turn on every single option.
> Not all Linux users are using regular (desktop/server) distributions. Linux is 
> used in embedded systems as well and in these case it makes sense to turn off 
> ax25/econet/decnet and in some cases even wireless/ipv6.
> > If you want to shrink structures, find ways to eliminate or
> > shrink structure members in all cases.
> We are looking into that as well since we have a pretty aggressive goal (get 
> net_device to 450 bytes or so), but we thought of starting with the low 
> hanging fruits. 
> Any suggestions in this area?

Some maybe not so good ones:

Perhaps:
	struct timer_list	watchdog_timer;
might become a
	struct timer_list	*watchdog_timer;

Maybe all the protocol/mac/bridge/vlan/garp/wireless specific
pointers might become a pointer to a list or array of pointers
allocated on demand.

Maybe combine perm_addr/broadcast/mc_list/uc addresses into a
single list with type information.  Maybe include dev_addrs,
but it's iterated with an rcu lock held.

Gaining less than 0, except maybe for weird network testing devices,
use a pointer to struct net_device_stats.

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[Octavian Purdila]

On Tuesday 28 July 2009 19:49:15 Joe Perches wrote:

> > We are looking into that as well since we have a pretty aggressive goal
> > (get net_device to 450 bytes or so), but we thought of starting with the
> > low hanging fruits.
> > Any suggestions in this area?
>
> Some maybe not so good ones:
>
> Perhaps:
> 	struct timer_list	watchdog_timer;
> might become a
> 	struct timer_list	*watchdog_timer;
>
> Maybe all the protocol/mac/bridge/vlan/garp/wireless specific
> pointers might become a pointer to a list or array of pointers
> allocated on demand.
>
> Maybe combine perm_addr/broadcast/mc_list/uc addresses into a
> single list with type information.  Maybe include dev_addrs,
> but it's iterated with an rcu lock held.
>
> Gaining less than 0, except maybe for weird network testing devices,
> use a pointer to struct net_device_stats.

Thanks for the suggestions.

BTW, we've noticed that the biggest net_device consumer is struct device with 
176 bytes consumed on our arch (powerpc) and the only things that are used 
from it seems to be the numa node id and the driver name. Some drivers also 
use it but most of the usage seems to be related to sysfs.

Do you think its worth trying to make dev dependent on CONFIG_SYSFS?

tavi

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[David Miller]

From: Octavian Purdila <opurdila@ixiacom.com>
Date: Tue, 28 Jul 2009 17:43:27 +0300

> On Monday 27 July 2009 18:26:44 David Miller wrote:
> 
>> This benefits, at best, %0.000000001 of users of the Linux kernel,
>> because every distribution is going to turn on every single option.
>>
> 
> Not all Linux users are using regular (desktop/server) distributions.

I didn't say all, I said "nearly all" which for many considerations
is roughly equivalent.

That %0.0000000001 was meant to represent non-distribution users.

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[Octavian Purdila]

On Tuesday 28 July 2009 20:28:08 David Miller wrote:

> From: Octavian Purdila <opurdila@ixiacom.com>
> Date: Tue, 28 Jul 2009 17:43:27 +0300
>
> > On Monday 27 July 2009 18:26:44 David Miller wrote:
> >> This benefits, at best, %0.000000001 of users of the Linux kernel,
> >> because every distribution is going to turn on every single option.
> >
> > Not all Linux users are using regular (desktop/server) distributions.
>
> I didn't say all, I said "nearly all" which for many considerations
> is roughly equivalent.
>
> That %0.0000000001 was meant to represent non-distribution users.

There are many embedded / dedicated devices out there which use a customized 
Linux kernel (wireless routers and other network appliances, set-top boxes, 
TVs, etc.). 

I have no idea about what the actual percent of the Linux users these account 
for, but to dismiss it as  %0.0000000001 seems like a rush statement to me.

tavi

Re: [PATCH] net: shrink net_device by #ifdef-ing protocol-specific members

[Rémi Denis-Courmont]

On Monday 27 July 2009 18:09:08 ext Lucian Adrian Grijincu wrote:
> +#if defined(CONFIG_AX25) || defined(CONFIG_AX25_MODULE)
>  	void			*ax25_ptr;	/* AX.25 specific data */
> +#endif
> +#ifdef CONFIG_WIRELESS
>  	struct wireless_dev	*ieee80211_ptr;	/* IEEE 802.11 specific data,
>  						   assign before registering */
> +#endif

If I'm not mistaken, AX.25 can only be used with ARPHRD_AX25 devices, no? 
Should there be a single opaque pointer for all non-generic address families 
do, that would only be used by the one matching the header type?

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki