Re: [PATCH 1/2] updates for [nf|ct]netlink and event API

[Patrick McHardy <kaber@trash.net>]

Pablo Neira wrote:
> Patrick McHardy wrote:
> 
>> +/* ctnetlink multicast groups: reports any change of ctinfo,
>> + * ctstatus, or protocol state change.
>> + */
>> +#define NFGRP_IPV4_CT_TCP      0x01
>> +#define NFGRP_IPV4_CT_UDP      0x02
>> +#define NFGRP_IPV4_CT_ICMP     0x04
>> +#define NFGRP_IPV4_CT_OTHER    0x08
>>
>> I'm not sure how useful these groups are. I think groups for different
>> event-types might be more useful to reduce the noise.
>
> Yes, this looks fine. So we could kill those and use an event
> subscription.

Yes. The question is how to define the groups. One group per event seems
wasteful, we probably need some sensible grouping. What events types
are there currently?

>> I couldn't find this function, but in 2.6.12 expectations aren't
>> refcounted anymore. If they are again by this patch, the refcnt would
>> be leaked in the following lines:
>>
>> +       skb2 = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
>> +       if (!skb2)
>> +               return -ENOMEM;
> 
> 
> Thanks for catching up this, I'll fix it. I recovered the refcounting
> for expectations to avoid any possible race condition since I could be
> working with an expectation whose timeout has expired.

Great. I wanted to restore it anyway for "permanent expectations" for
the H.323 helper.

> Does this help? Hope so.
> http://people.netfilter.org/~pablo/ctnetlink-2.6.12/2.6.11-vs-2.6.12/04ctnetlink.patch

I'll continue tomorrow, its getting late here ..

Regards
Patrick