Re: [PATCH][RFC] Shrink ip_vs_*.c includes

[Roberto Nibali <ratz@tac.ch>]

>>> CONFIG_IP_VS=m
>>> # CONFIG_IP_VS_DEBUG is not set
>>> CONFIG_IP_VS_TAB_BITS=12
>>> CONFIG_IP_VS_PROTO_TCP=y
>>> CONFIG_IP_VS_PROTO_UDP=y
>>> CONFIG_IP_VS_PROTO_ESP=y
>>> CONFIG_IP_VS_PROTO_AH=y

These 'y' caused me to think that it wasn't modular.

>>> CONFIG_IP_VS_RR=m
>>> CONFIG_IP_VS_WRR=m
>>> CONFIG_IP_VS_LC=m
>>> CONFIG_IP_VS_WLC=m
>>> CONFIG_IP_VS_LBLC=m
>>> CONFIG_IP_VS_LBLCR=m
>>> CONFIG_IP_VS_DH=m
>>> CONFIG_IP_VS_SH=m
>>> CONFIG_IP_VS_SED=m
>>> CONFIG_IP_VS_NQ=m
>>> CONFIG_IP_VS_FTP=m
>> Ok, this is m/y mixed. Haven't tried it yet.
> 
> Actually, I think that its completely modular.

So why the heck didn't it trigger on my system? Is it because the
include/net/* headers are not always included on ia64? Well, tant pis!

>> I'll reproduce it with your config. I didn't think of trying it with
>> mixed y/m settings. Following include is missing then:
>>
>> #include <linux/seq_file.h>
> 
> Thanks, I found that linux/module.h was also needed.

Ok. I thought this would get included by any of the other headers. Well,
so long as it also compiles fine on your system, I'm good.

> After putting those two back into ip_vs_conn.c the build went 
> find. I also tried a few other combinations, all on ia64, without issue.
> My diff is below. Could you recheck it?

Seems fine to me, thanks for testing this. I would have felt really bad
if I broke IPVS in mainline because of such changes.

>> Or do you think we could put all the needed includes into ip_vs.h and
>> simply be done with it?
> 
> I spoke breifly with Dave about this, and he isn't very keen on it.

Fair enough.

> The problem with that approach, is that while its less work to
> maintain the headers by hand, it will likely result in uneeded
> includes in some cases.

How so? All includes nota bene have the prevailing:

#ifndef _FOOBAR_H
#define _FOOBAR_H
[...]
#endif  /* _FOOBAR_H */

framework. So would this speak slightely against that? Also, from the
includes we take today, I reckon that in the end we half of the
include/{net,linux}/*.h is in our objects :). But you guys decide. I
could maybe run a call-graph.

> So all of LVS will get built when
> a given header is touched, where perhaps only half of it needed
> to be built.

Well, it does not happen so ofter that a core network related header is
touched to my avail.

> So with that in mind, could you continue in the vein of
> your original patch?

Sure thing. Your attached patch is fine; if you feel confident with our
changes, submit it.

Best regards,
Roberto Nibali, ratz
-- 
-------------------------------------------------------------
addr://Kasinostrasse 30, CH-5001 Aarau tel://++41 62 823 9355
http://www.terreactive.com             fax://++41 62 823 9356
-------------------------------------------------------------
10 Jahre Kompetenz in IT-Sicherheit.              1996 - 2006
Wir sichern Ihren Erfolg.                      terreActive AG
-------------------------------------------------------------