Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages

[Julian Anastasov]

	Hello,

On Fri, 5 Oct 2012, Arnd Bergmann wrote:

> The ip_vs_set_timeout function sets timeouts for TCP and UDP, which
> can be enabled independently at compile time. The debug message
> always prints both timeouts that are passed into the function,
> but if one is disabled, the message will show uninitialized data.
> 
> This splits the debug message into two separte IP_VS_DBG statements
> that are in the same #ifdef section to ensure we only print the
> text about what is actually going on.
> 
> Without this patch, building ARM ixp4xx_defconfig results in:

	Are there any CONFIG_IP_VS_PROTO_xxx options in this
default config? It is a waste of memory if IPVS is compiled
without any protocols.

> net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd':
> net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized]
> net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here
> net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized]
> net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here
> net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized]
> net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here

	There are many __ip_vs_get_timeouts callers but
just one calls memset(&t, 0, sizeof(t)) before that,
problem only for ip_vs_genl_set_config and ip_vs_set_timeout.

	To be safe, can we move this memset into
__ip_vs_get_timeouts instead of playing games with defines?:

	memset(t, 0, sizeof(*t));

	This debug message will be more precise in showing the
changed values if we replace the __ip_vs_get_timeouts 
call in ip_vs_genl_set_config with memset(&t, 0, sizeof(t)).
Then we will see 0 for values that are not changed/supported.

> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: David S. Miller <davem@xxxxxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx
> Cc: Simon Horman <horms@xxxxxxxxxxxx>
> Cc: Julian Anastasov <ja@xxxxxx>
> Cc: netfilter-devel@xxxxxxxxxxxxxxx
> Cc: netfilter@xxxxxxxxxxxxxxx
> Cc: coreteam@xxxxxxxxxxxxx
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index f51013c..f3a66c3 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -2237,12 +2237,11 @@ static int ip_vs_set_timeout(struct net *net, struct ip_vs_timeout_user *u)
>  	struct ip_vs_proto_data *pd;
>  #endif
>  
> -	IP_VS_DBG(2, "Setting timeout tcp:%d tcpfin:%d udp:%d\n",
> +#ifdef CONFIG_IP_VS_PROTO_TCP
> +	IP_VS_DBG(2, "Setting timeout tcp:%d tcpfin:%d\n",
>  		  u->tcp_timeout,
> -		  u->tcp_fin_timeout,
> -		  u->udp_timeout);
> +		  u->tcp_fin_timeout);
>  
> -#ifdef CONFIG_IP_VS_PROTO_TCP
>  	if (u->tcp_timeout) {
>  		pd = ip_vs_proto_data_get(net, IPPROTO_TCP);
>  		pd->timeout_table[IP_VS_TCP_S_ESTABLISHED]
> @@ -2257,6 +2256,9 @@ static int ip_vs_set_timeout(struct net *net, struct ip_vs_timeout_user *u)
>  #endif
>  
>  #ifdef CONFIG_IP_VS_PROTO_UDP
> +	IP_VS_DBG(2, "Setting timeout udp:%d\n",
> +		  u->udp_timeout);
> +
>  	if (u->udp_timeout) {
>  		pd = ip_vs_proto_data_get(net, IPPROTO_UDP);
>  		pd->timeout_table[IP_VS_UDP_S_NORMAL]
> -- 
> 1.7.10

Regards

--
Julian Anastasov <ja@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/