Re: [PATCH 2/3] set SEEN_REPLY before destroying conntrack on TCP RST

[Fabian Hugelshofer <hugelshofer2006@gmx.ch>]

Patrick McHardy wrote:
> Fabian Hugelshofer wrote:
>> On Thu, 2008-05-22 at 10:13 +0100, Fabian Hugelshofer wrote:
>>> If a connection fails with a TCP reset, the conntrack is destroyed 
>>> immediately. This patch sets the SEEN_REPLY bit before destroying the 
>>> conntrack.
>>
>> This updated version also increments the accounting counters.
> 
> Thanks, but this needs to be changed slightly.
[...]
> I think a better way is to encapsulate the del_timer/timeout.function
> calls in a nf_ct_kill() function and perform accounting there.
> Since all manual invocations of timeout.function are/should be
> performed only while handling packets (that are usually not
> accounted), this seems like the right way.

Ok, I see. But for accounting ctinfo and skbuf are required. I'll 
include them in the argument list of nf_ct_kill() and update the 
function invocations, ok? Or should I introduce an nf_ct_kill_acct()?

I just did another test where my SEEN_REPLY patch was not applied. 
Surprisingly the SEEN_REPLY bit was set in the destroy events. I am 
afraid, but I have to assume, that I did not evaluate the bahavior 
carefully enough. Probably I confused the accounting, no status and no 
related packets issues.

Unless a race condition might be thinkable, we should leave the 
SEEN_REPLY patch. If it is possible, that the timeout function 
immediately triggers the destroy event to be exported over netlink, then 
the patch is still necessary. I don't see things detailed enough to 
judge this. If it is necessary, should it be included in nf_ct_kill()?
--
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