[PATCH] ip_conntrack_proto_tcp

[PATCH] ip_conntrack_proto_tcp

[Vlad Drukker]

[-- Attachment #1: Type: text/plain, Size: 207 bytes --]

Hi there,

Attached patch for ip_conntrack to account TCP sessions started with SYN
+PUSH flags. Looks weird, but some HW vendors do TCP their own way. 

Let's earn some points from RFC 1025.

Cheers,
Vlad.

[-- Attachment #2: SYN_PUSH.patch --]
[-- Type: text/x-patch, Size: 1085 bytes --]

[NETFILTER] ip_conntrack SYN PUSH patch
	
Some Qlogic iSCSI HBA hardware like QLA4010 up to firmware 3.0.0.4
initiates TCP with SYN and PUSH flags set.
Linux TCP/IP stack works fine with that.
Make ip_conntrack do the same.

Signed-off-by: Vlad Drukker <vlad@storewiz.com>
---
commit c02dd55183e5f8330da96fb82291a76492b6e7c4
tree 0aca6b3836eef97bfc552ed00883ce8f73c3e5f9
parent 407cf928e19d8cd8bb91160da6fd7a900b1e4705
author <vlad@storewiz.com> Thu, 10 Nov 2005 17:07:25 +0200
committer <vlad@storewiz.com> Thu, 10 Nov 2005 17:07:25 +0200

 net/ipv4/netfilter/ip_conntrack_proto_tcp.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/netfilter/ip_conntrack_proto_tcp.c b/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
+++ b/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
@@ -813,6 +813,7 @@ static u8 tcp_valid_flags[(TH_FIN|TH_SYN
 {
 	[TH_SYN]			= 1,
 	[TH_SYN|TH_ACK]			= 1,
+	[TH_SYN|TH_PUSH]		= 1,
 	[TH_SYN|TH_ACK|TH_PUSH]		= 1,
 	[TH_RST]			= 1,
 	[TH_RST|TH_ACK]			= 1,

Re: [PATCH] ip_conntrack_proto_tcp

[Pablo Neira]

Vlad Drukker wrote:
> Attached patch for ip_conntrack to account TCP sessions started with SYN
> +PUSH flags. Looks weird, but some HW vendors do TCP their own way. 
> 
> Let's earn some points from RFC 1025.

I see this patch like a sort of workaround to make broken devices with
the TCP connection tracking, right? In that case, I don't think that
it's a good idea polluting our code with workarounds for every existing
broken device. The HW vendors must fix their devices.

-- 
Pablo

Re: [PATCH] ip_conntrack_proto_tcp

[Pablo Neira]

Pablo Neira wrote:
> Vlad Drukker wrote:
> 
>>Attached patch for ip_conntrack to account TCP sessions started with SYN
>>+PUSH flags. Looks weird, but some HW vendors do TCP their own way. 
>>
>>Let's earn some points from RFC 1025.
> 
> 
> I see this patch like a sort of workaround to make broken devices with

Sorry, I meant: ... to get working broken devices with ...

-- 
Pablo

Re: [netfilter-core] Re: [PATCH] ip_conntrack_proto_tcp

[Patrick McHardy]

Pablo Neira wrote:
> Vlad Drukker wrote:
> 
>>Attached patch for ip_conntrack to account TCP sessions started with SYN
>>+PUSH flags. Looks weird, but some HW vendors do TCP their own way. 
>>
>>Let's earn some points from RFC 1025.
> 
> I see this patch like a sort of workaround to make broken devices with
> the TCP connection tracking, right? In that case, I don't think that
> it's a good idea polluting our code with workarounds for every existing
> broken device. The HW vendors must fix their devices.

Unfortunately this is unlikely to happen, and if Linux itself
accepts SYN|PSH, I don't see a reason why ip_conntrack shouldn't
as well.

Re: [netfilter-core] Re: [PATCH] ip_conntrack_proto_tcp

[David S. Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Fri, 11 Nov 2005 14:58:01 +0100

> Unfortunately this is unlikely to happen, and if Linux itself
> accepts SYN|PSH, I don't see a reason why ip_conntrack shouldn't
> as well.

I totally agree.

[PATCH] [NETFILTER]: Allow SYN+PSH as valid flag combination

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 2544 bytes --]

On Fri, Nov 11, 2005 at 11:48:01AM -0800, David S. Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Fri, 11 Nov 2005 14:58:01 +0100
> 
> > Unfortunately this is unlikely to happen, and if Linux itself
> > accepts SYN|PSH, I don't see a reason why ip_conntrack shouldn't
> > as well.
> 
> I totally agree.

Me, too.  However, we also need to update nf_conntrack now:


[NETFILTER] {ip,nf}_conntrack TCP: Accept SYN+PUSH like SYN

Some devices (e.g. Qlogic iSCSI HBA hardware like QLA4010 up to firmware
3.0.0.4) initiates TCP with SYN and PUSH flags set.

The Linux TCP/IP stack deals fine with that, but the connection tracking
code doesn't.

This patch alters TCP connection tracking to accept SYN+PUSH as a valid
flag combination.

Signed-off-by: Vlad Drukker <vlad@storewiz.com>
Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 933c4adfa34fbc151dd9908b2ce59335b22e6fe4
tree 041e7fc4116cd3b2774429cf8da9caee6e77a74b
parent 106c77b9b51b61bcd926ab5100761b2832791b74
author Harald Welte <laforge@netfilter.org> Sat, 12 Nov 2005 11:36:56 +0100
committer Harald Welte <laforge@netfilter.org> Sat, 12 Nov 2005 11:36:56 +0100

 net/ipv4/netfilter/ip_conntrack_proto_tcp.c |    1 +
 net/netfilter/nf_conntrack_proto_tcp.c      |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/netfilter/ip_conntrack_proto_tcp.c b/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
+++ b/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
@@ -814,6 +814,7 @@ static u8 tcp_valid_flags[(TH_FIN|TH_SYN
 {
 	[TH_SYN]			= 1,
 	[TH_SYN|TH_ACK]			= 1,
+	[TH_SYN|TH_PUSH]		= 1,
 	[TH_SYN|TH_ACK|TH_PUSH]		= 1,
 	[TH_RST]			= 1,
 	[TH_RST|TH_ACK]			= 1,
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -779,6 +779,7 @@ static u8 tcp_valid_flags[(TH_FIN|TH_SYN
 {
 	[TH_SYN]			= 1,
 	[TH_SYN|TH_ACK]			= 1,
+	[TH_SYN|TH_PUSH]		= 1,
 	[TH_SYN|TH_ACK|TH_PUSH]		= 1,
 	[TH_RST]			= 1,
 	[TH_RST|TH_ACK]			= 1,

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] [NETFILTER]: Allow SYN+PSH as valid flag combination

[David S. Miller]

From: Harald Welte <laforge@netfilter.org>
Date: Sat, 12 Nov 2005 11:39:18 +0100

> [NETFILTER] {ip,nf}_conntrack TCP: Accept SYN+PUSH like SYN
> 
> Some devices (e.g. Qlogic iSCSI HBA hardware like QLA4010 up to firmware
> 3.0.0.4) initiates TCP with SYN and PUSH flags set.
> 
> The Linux TCP/IP stack deals fine with that, but the connection tracking
> code doesn't.
> 
> This patch alters TCP connection tracking to accept SYN+PUSH as a valid
> flag combination.
> 
> Signed-off-by: Vlad Drukker <vlad@storewiz.com>
> Signed-off-by: Harald Welte <laforge@netfilter.org>

Applied.

I think this one is a -stable candidate (sans the nf_conntrack
bits of course :-).  But that's up to you.