Re: [PATCH] netfilter: don't always initialize ct->proto

[Changli Gao <xiaosuo@gmail.com>]

On Sun, Oct 24, 2010 at 4:27 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>
> On Sunday 2010-10-24 04:40, Changli Gao wrote:
>
>>ct->proto is big(60 bytes) due to structure ip_ct_tcp, and we don't need
>>to initialize the whole for all the other protocols. This patch moves
>>proto to the end of structure nf_conn, and pushes the initialization down
>>to the individual protocols.
>>
>>diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
>>index 5292560..9ae57c5 100644
>>--- a/net/netfilter/nf_conntrack_proto_dccp.c
>>+++ b/net/netfilter/nf_conntrack_proto_dccp.c
>>@@ -452,6 +452,9 @@ static bool dccp_new(struct nf_conn *ct, const struct sk_buff *skb,
>>       ct->proto.dccp.role[IP_CT_DIR_ORIGINAL] = CT_DCCP_ROLE_CLIENT;
>>       ct->proto.dccp.role[IP_CT_DIR_REPLY] = CT_DCCP_ROLE_SERVER;
>>       ct->proto.dccp.state = CT_DCCP_NONE;
>>+      ct->proto.dccp.last_pkt = DCCP_PKT_REQUEST;
>>+      ct->proto.dccp.last_dir = IP_CT_DIR_ORIGINAL;
>>+      ct->proto.dccp.handshake_seq = 0;
>>       return true;
>>
>> out_invalid:
>
> This hunk seems unrelated to the actual move of the union member -

There are only three fields left uninitialized, so I use assignments
instead of memset().

>
>>diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
>>index c4c885d..69da4b0 100644
>>--- a/net/netfilter/nf_conntrack_proto_tcp.c
>>+++ b/net/netfilter/nf_conntrack_proto_tcp.c
>>@@ -1066,9 +1066,7 @@ static bool tcp_new(struct nf_conn *ct, const struct sk_buff *skb,
>>       BUG_ON(th == NULL);
>>
>>       /* Don't need lock here: this conntrack not in circulation yet */
>>-      new_state
>>-              = tcp_conntracks[0][get_conntrack_index(th)]
>>-              [TCP_CONNTRACK_NONE];
>>+      new_state = tcp_conntracks[0][get_conntrack_index(th)][TCP_CONNTRACK_NONE];
>>
>>       /* Invalid: delete conntrack */
>>       if (new_state >= TCP_CONNTRACK_MAX) {
>
> as does this change. Can you elaborate on why they are now needed,
> or did you happen to erroneously squash multiple patches?
>

It is trivial code cleanup, and isn't worth an individual patch IMO. Thanks.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html