Re: [PATCH 0/2] rework of userspace expectation support

[Patrick McHardy <kaber@trash.net>]

Am 13.04.2011 14:11, schrieb Pablo Neira Ayuso:
> On 13/04/11 13:55, Patrick McHardy wrote:
>> Am 13.04.2011 13:47, schrieb Pablo Neira Ayuso:
>>> On 13/04/11 13:37, Patrick McHardy wrote:
>>>> Am 12.04.2011 23:59, schrieb Pablo Neira Ayuso:
>>>>> Hi Patrick,
>>>>>
>>>>> The following patches rework the userspace expectation support
>>>>> to fix one problematic scenario: if the master conntrack vanishes
>>>>> while there are still userspace expectations, we hit an oops
>>>>> in the destroy event path for expectations.
>>>>
>>>> Just wondering, how can this happen? We take a reference for
>>>> userspace expectations just as we do for kernel expectations.
>>>>
>>>> Ok, I see, we are releasing it again at the end of
>>>> ctnetlink_create_expect(), that seems to be the actual problem
>>>> if I'm not mistaken.
>>>
>>> Indeed, we have keep that reference, that would fix the problem.
>>
>> We definitely need to hold it anyways since destroy_conntrack()
>> releases it again.
>>
>>> Still, with the curent approach the userspace expectation will be valid
>>> after the master conntrack has expired.
>>>
>>> So we can do the following: Fix this refcount issue in -stable and
>>> current, and schedule these patches for nf-next to change the behaviour.
>>>
>>> What do you think?
>>
>> I don't know, what's the difference to non-userspace expectations?
>> The same applies to them, we don't require the master to still be
>> active for expectations.
> 
> kernelspace expectations are released if the master is destroyed.
> userspace expectations will not.

I see, you're talking about unfulfilled expectations. Still, we're
releasing expectations in destroy_conntrack(), if we fix the refcount
issue, the master conntrack will not be destroyed while userspace
expectations exist.

> The helper extension is used to store a list of expectations for this
> master conntrack, so we can release the expectations that are
> associated. This is not true for userspace expectations, since the
> master has no list with expectations.

This raises the question - why are we treating userspace differently
in this regard?