* [PATCH] Fix connlimit bug when receive RST packet in ESTABLISHED state
@ 2008-06-02 11:22 Dong Wei
2008-06-02 12:20 ` Patrick McHardy
0 siblings, 1 reply; 7+ messages in thread
From: Dong Wei @ 2008-06-02 11:22 UTC (permalink / raw)
To: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]
Hi, all
In xt_connlimit match module, the counter of an IP is decreased when
the TCP packet is go through the chain with ip_conntrack state TW.
Well, it's very natural that the server and client close the socket
with FIN packet. But when the client/server close the socket with RST
packet(using so_linger), the counter for this connection still exsit.
The following patch can fix it which is based on linux-2.6.25.4
diff -ruN a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
--- a/net/netfilter/xt_connlimit.c 2008-06-02 18:48:38.000000000 +0800
+++ b/net/netfilter/xt_connlimit.c 2008-06-02 18:50:40.000000000 +0800
@@ -75,7 +75,8 @@
u_int16_t proto = conn->tuplehash[0].tuple.dst.protonum;
if (proto == IPPROTO_TCP)
- return conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT;
+ return (conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT
+ || conn->proto.tcp.state == TCP_CONNTRACK_CLOSE);
else
return 0;
}
[-- Attachment #2: xt_connlimit.txt --]
[-- Type: text/plain, Size: 525 bytes --]
diff -ruN a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
--- a/net/netfilter/xt_connlimit.c 2008-06-02 18:48:38.000000000 +0800
+++ b/net/netfilter/xt_connlimit.c 2008-06-02 18:50:40.000000000 +0800
@@ -75,7 +75,8 @@
u_int16_t proto = conn->tuplehash[0].tuple.dst.protonum;
if (proto == IPPROTO_TCP)
- return conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT;
+ return (conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT
+ || conn->proto.tcp.state == TCP_CONNTRACK_CLOSE);
else
return 0;
}
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Fix connlimit bug when receive RST packet in ESTABLISHED state
2008-06-02 11:22 [PATCH] Fix connlimit bug when receive RST packet in ESTABLISHED state Dong Wei
@ 2008-06-02 12:20 ` Patrick McHardy
2008-06-02 12:46 ` Jan Engelhardt
0 siblings, 1 reply; 7+ messages in thread
From: Patrick McHardy @ 2008-06-02 12:20 UTC (permalink / raw)
To: Dong Wei; +Cc: netfilter-devel, Jan Engelhardt
Dong Wei wrote:
> Hi, all
> In xt_connlimit match module, the counter of an IP is decreased when
> the TCP packet is go through the chain with ip_conntrack state TW.
> Well, it's very natural that the server and client close the socket
> with FIN packet. But when the client/server close the socket with RST
> packet(using so_linger), the counter for this connection still exsit.
> The following patch can fix it which is based on linux-2.6.25.4
>
> diff -ruN a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
> --- a/net/netfilter/xt_connlimit.c 2008-06-02 18:48:38.000000000 +0800
> +++ b/net/netfilter/xt_connlimit.c 2008-06-02 18:50:40.000000000 +0800
> @@ -75,7 +75,8 @@
> u_int16_t proto = conn->tuplehash[0].tuple.dst.protonum;
>
> if (proto == IPPROTO_TCP)
> - return conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT;
> + return (conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT
> + || conn->proto.tcp.state == TCP_CONNTRACK_CLOSE);
Looks fine to me. Jan?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix connlimit bug when receive RST packet in ESTABLISHED state
2008-06-02 12:20 ` Patrick McHardy
@ 2008-06-02 12:46 ` Jan Engelhardt
2008-06-02 13:01 ` Patrick McHardy
0 siblings, 1 reply; 7+ messages in thread
From: Jan Engelhardt @ 2008-06-02 12:46 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Dong Wei, netfilter-devel
On Monday 2008-06-02 14:20, Patrick McHardy wrote:
> Dong Wei wrote:
>> Hi, all
>> In xt_connlimit match module, the counter of an IP is decreased when
>> the TCP packet is go through the chain with ip_conntrack state TW.
>> Well, it's very natural that the server and client close the socket
>> with FIN packet. But when the client/server close the socket with RST
>> packet(using so_linger), the counter for this connection still exsit.
>> The following patch can fix it which is based on linux-2.6.25.4
>>
>> diff -ruN a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
>> --- a/net/netfilter/xt_connlimit.c 2008-06-02 18:48:38.000000000 +0800
>> +++ b/net/netfilter/xt_connlimit.c 2008-06-02 18:50:40.000000000 +0800
>> @@ -75,7 +75,8 @@
>> u_int16_t proto = conn->tuplehash[0].tuple.dst.protonum;
>>
>> if (proto == IPPROTO_TCP)
>> - return conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT;
>> + return (conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT
>> + || conn->proto.tcp.state == TCP_CONNTRACK_CLOSE);
>
> Looks fine to me. Jan?
The check for TCP_CONNTRACK_TIME_WAIT was introduced since there is
the 2*MSL delay before the TIME_WAIT->CLOSED transition, and not
counting a connection beginning with TIME_WAIT is common sense/what
people expect.
Though the cleanup delay between TCP_CONNTRACK_CLOSE and (deallocated
state) is much less than 2*MSL, it makes sense to also add this case
per common sense.
Patch is fine, yes, but you do not need the redundant
( ) that were introduced.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix connlimit bug when receive RST packet in ESTABLISHED state
2008-06-02 12:46 ` Jan Engelhardt
@ 2008-06-02 13:01 ` Patrick McHardy
2008-06-02 13:44 ` Dong Wei
2008-06-02 14:35 ` Jan Engelhardt
0 siblings, 2 replies; 7+ messages in thread
From: Patrick McHardy @ 2008-06-02 13:01 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: Dong Wei, netfilter-devel
Jan Engelhardt wrote:
> On Monday 2008-06-02 14:20, Patrick McHardy wrote:
>> Dong Wei wrote:
>>> diff -ruN a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
>>> --- a/net/netfilter/xt_connlimit.c 2008-06-02 18:48:38.000000000 +0800
>>> +++ b/net/netfilter/xt_connlimit.c 2008-06-02 18:50:40.000000000 +0800
>>> @@ -75,7 +75,8 @@
>>> u_int16_t proto = conn->tuplehash[0].tuple.dst.protonum;
>>>
>>> if (proto == IPPROTO_TCP)
>>> - return conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT;
>>> + return (conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT
>>> + || conn->proto.tcp.state == TCP_CONNTRACK_CLOSE);
>> Looks fine to me. Jan?
>
> The check for TCP_CONNTRACK_TIME_WAIT was introduced since there is
> the 2*MSL delay before the TIME_WAIT->CLOSED transition, and not
> counting a connection beginning with TIME_WAIT is common sense/what
> people expect.
Yes, though the end-result might not be what people expect.
The connection can be reopened, exceeding the configured
limit, and lots of TIME_WAIT/CLOSE connections might linger
around.
> Though the cleanup delay between TCP_CONNTRACK_CLOSE and (deallocated
> state) is much less than 2*MSL, it makes sense to also add this case
> per common sense.
>
> Patch is fine, yes, but you do not need the redundant
> ( ) that were introduced.
I'll remove them when applying the patch.
Dong, I need a Signed-off-by: line from you before I can apply this.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix connlimit bug when receive RST packet in ESTABLISHED state
2008-06-02 13:01 ` Patrick McHardy
@ 2008-06-02 13:44 ` Dong Wei
2008-06-02 13:51 ` Patrick McHardy
2008-06-02 14:35 ` Jan Engelhardt
1 sibling, 1 reply; 7+ messages in thread
From: Dong Wei @ 2008-06-02 13:44 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Jan Engelhardt, netfilter-devel
Hi, Jan
My Signed-off-by message is:
Signed-off-by: Dong Wei <dwei.zh@gmail.com>
Thanks
On Mon, Jun 2, 2008 at 9:01 PM, Patrick McHardy <kaber@trash.net> wrote:
> Jan Engelhardt wrote:
>>
>> On Monday 2008-06-02 14:20, Patrick McHardy wrote:
>>>
>>> Dong Wei wrote:
>>>>
>>>> diff -ruN a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
>>>> --- a/net/netfilter/xt_connlimit.c 2008-06-02 18:48:38.000000000
>>>> +0800
>>>> +++ b/net/netfilter/xt_connlimit.c 2008-06-02 18:50:40.000000000
>>>> +0800
>>>> @@ -75,7 +75,8 @@
>>>> u_int16_t proto = conn->tuplehash[0].tuple.dst.protonum;
>>>>
>>>> if (proto == IPPROTO_TCP)
>>>> - return conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT;
>>>> + return (conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT
>>>> + || conn->proto.tcp.state ==
>>>> TCP_CONNTRACK_CLOSE);
>>>
>>> Looks fine to me. Jan?
>>
>> The check for TCP_CONNTRACK_TIME_WAIT was introduced since there is
>> the 2*MSL delay before the TIME_WAIT->CLOSED transition, and not
>> counting a connection beginning with TIME_WAIT is common sense/what
>> people expect.
>
> Yes, though the end-result might not be what people expect.
> The connection can be reopened, exceeding the configured
> limit, and lots of TIME_WAIT/CLOSE connections might linger
> around.
>
>> Though the cleanup delay between TCP_CONNTRACK_CLOSE and (deallocated
>> state) is much less than 2*MSL, it makes sense to also add this case
>> per common sense.
>>
>> Patch is fine, yes, but you do not need the redundant
>> ( ) that were introduced.
>
> I'll remove them when applying the patch.
>
> Dong, I need a Signed-off-by: line from you before I can apply this.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix connlimit bug when receive RST packet in ESTABLISHED state
2008-06-02 13:01 ` Patrick McHardy
2008-06-02 13:44 ` Dong Wei
@ 2008-06-02 14:35 ` Jan Engelhardt
1 sibling, 0 replies; 7+ messages in thread
From: Jan Engelhardt @ 2008-06-02 14:35 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Dong Wei, netfilter-devel
On Monday 2008-06-02 15:01, Patrick McHardy wrote:
> Jan Engelhardt wrote:
>> On Monday 2008-06-02 14:20, Patrick McHardy wrote:
>> > Dong Wei wrote:
>> > > diff -ruN a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
>> > > --- a/net/netfilter/xt_connlimit.c 2008-06-02 18:48:38.000000000
>> > > +0800
>> > > +++ b/net/netfilter/xt_connlimit.c 2008-06-02 18:50:40.000000000
>> > > +0800
>> > > @@ -75,7 +75,8 @@
>> > > u_int16_t proto = conn->tuplehash[0].tuple.dst.protonum;
>> > >
>> > > if (proto == IPPROTO_TCP)
>> > > - return conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT;
>> > > + return (conn->proto.tcp.state == TCP_CONNTRACK_TIME_WAIT
>> > > + || conn->proto.tcp.state == TCP_CONNTRACK_CLOSE);
>> > Looks fine to me. Jan?
Acked-by: Jan Engelhardt <jengelh@medozas.de>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-06-02 14:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-02 11:22 [PATCH] Fix connlimit bug when receive RST packet in ESTABLISHED state Dong Wei
2008-06-02 12:20 ` Patrick McHardy
2008-06-02 12:46 ` Jan Engelhardt
2008-06-02 13:01 ` Patrick McHardy
2008-06-02 13:44 ` Dong Wei
2008-06-02 13:51 ` Patrick McHardy
2008-06-02 14:35 ` Jan Engelhardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.