Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
>> Moreover, someone may remove the module in the middle just after the
>> module loading but, well, we have lost the race in the case.
> 
> I'd do something similar to qdiscs etc:
> 
> - lookup helper
> - if not found: request_module, take lock again, repeat lookup, return
> EAGAIN if found now
> - in the nfnetlink command handler: if ret == EAGAIN replay message
> 
> grep for "replay" in net/ for a few examples of this. This also
> handles the race BTW.

Nice. Thanks for the explanation :). I'll check if I can cook a couple
of patches to do it that way at some point.

>>> Second, this implicit helper assignment is also a bit unusual,
>>> why don't we simply insist that the conntrack has a helper
>>> assigned through the ctnetlink conntrack interface?
>>
>> If I understood well, then we simply assign the helper to the conntrack
>> and the expectation part of ctnetlink should rely on the existing
>> assigned helper, right?
> 
> Yes, I think thats cleaner.
>>
>> Please, have a look at the patch attached.
> 
> Looks fine.

Would you also apply the following two patches before looking at [PATCH
5/7]?

The first patch is a refactorization that was in the previous version of
4/7, the latter is a minor return value change to make it consistent
with others error values in ctnetlink.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers