Re: [PATCH nf-next v2 1/3] netfilter: nf_conntrack: push zone object into functions

[Daniel Borkmann <daniel@iogearbox.net>]

On 07/15/2015 07:35 PM, Pablo Neira Ayuso wrote:
> On Sat, Jul 11, 2015 at 03:14:05AM +0200, Daniel Borkmann wrote:
>> This patch replaces the zone id which is pushed down into functions
>> with the actual zone object. It's a bigger one-time change, but
>> needed for later on extending zones with a direction parameter, and
>> thus decoupling this additional information from all call-sites.
>>
>> It was suggested during NFWS to let the nf_ct_zone() helper store
>> the meta data on the stack as we currently do, but in a similar
>> fashion as skb_header_pointer() does, so we can avoid keeping track
>> of the object lifetime. In general, dealing directly with the object
>> also facilitates for adding other possible meta data in future.
>>
>> No functional changes in this patch.
> [...]
>> diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
>> index 80d5554..75f7860 100644
>> --- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
>> +++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
>> @@ -134,9 +134,10 @@ icmp_error_message(struct net *net, struct nf_conn *tmpl, struct sk_buff *skb,
>>   	struct nf_conntrack_tuple innertuple, origtuple;
>>   	const struct nf_conntrack_l4proto *innerproto;
>>   	const struct nf_conntrack_tuple_hash *h;
>> -	u16 zone = tmpl ? nf_ct_zone(tmpl) : NF_CT_DEFAULT_ZONE;
>> +	struct nf_conntrack_zone *zone, __zone;
>>
>>   	NF_CT_ASSERT(skb->nfct == NULL);
>> +	zone = nf_ct_zone_tmpl(tmpl, &__zone);
>
> This was one possible idea, but I also suggested to have a dummy:
>
> static const struct nf_ct_zone nf_ct_zone_dflt = {
>          .zone_id        = NF_CT_DEFAULT_ZONE,
> };
>
> that we return in case no zone extension is set, and use it instead of
> your nf_ct_zone_dflt() function.

There are multiple possibilities, no doubt, I liked this one better and
therefore took that path as it integrated nicely with the 3rd patch in
this series, and it also consumed less stack space, it's 4 byte after all.