Hi Jozsef,

I'm having a look at your tcp-window-tracking patch, I made some 
modifications. Attached to this email a patch which applies to an 
already applied patch (I know, a bit tricky), but I just wanted to show 
you clearly my modifications. After you've reviewed this email, I have 
no problem to send a patch which applies to the CVS with the stuff that 
you considered that it's ok.

Well, let me comment you a bit the patch.

a) Use flags defined in tcp.h instead of bsd flags: I found that these 
flags are already defined in linux with different names in tcp.h, so we 
could use linux flags instead.

-#define        TH_FIN  0x01
-#define        TH_SYN  0x02
...

----- snipped from tcp.h ----
#define TCPCB_FLAG_FIN 0x01
#define TCPCB_FLAG_SYN 0x02
...

b) check that tcp is not malformed (doff has a faked value): I trust the 
value of skb->len, so we could use it to check that the doff is correct 
instead of that skb_copy_bits and that buff var, couldn't we?

+       if ((skb->len - iph->ihl*4 - sizeof(struct tcphdr) == tcph->doff*4)
+            && tcph->doff*4 < sizeof(struct tcphdr)) {

I added this to the unclean function.

c) I replaced this:

-       } else if ((old_state == TCP_CONNTRACK_SYN_RECV
-                   || old_state == TCP_CONNTRACK_ESTABLISHED)
-                  && new_state == TCP_CONNTRACK_ESTABLISHED) {
-               /* Set ASSURED if we see see valid ack in ESTABLISHED 
after SYN_RECV
-                  or a valid answer for a picked up connection. */
-                       set_bit(IPS_ASSURED_BIT, &conntrack->status);

for this:

+       case TCP_CONNTRACK_ESTABLISHED:
+               if ((old_state == TCP_CONNTRACK_SYN_RECV
+                    || old_state == TCP_CONNTRACK_ESTABLISHED)
+                    && !test_bit(IPS_ASSURED_BIT, &conntrack->status)) {
+                       /* Set ASSURED if we see see valid ack in 
ESTABLISHED
+                        * after SYN_RECV or a valid answer for a picked
+                        * up connection. */
+                       set_bit(IPS_ASSURED_BIT, &conntrack->status);
+               }

actually I also added that test_bit, so the bit assured is set once.

d) I think that we don't need this anymore, this is part of the old 
tracking:

-       if (!test_bit(IPS_SEEN_REPLY_BIT, &conntrack->status)) {
-               /* If only reply is a RST, we can consider ourselves not 
to           
-                  have an established connection: this is a fairly common
-                  problem case, so we can delete the 
conntrack                       
-                  immediately.  --RR */
-               if (tcph->rst) 
{                                                      
-                       WRITE_UNLOCK(&tcp_lock);
-                       if 
(del_timer(&conntrack->timeout))                           
-                               conntrack->timeout.function((unsigned 
long)conntrack);
-                       return 
NF_ACCEPT;                                             
-               }

because this should be always check in TCP_CONNTRACK_CLOSE which is, if 
i'm not wrong, the state that we reach when a rst is received.

e) I defined an inline __u32 segment_seq_plus_len instead a macro, it 
gives me more flexibility.

f) Do you think that it's interesting adding fine grain locking in 
tcp-window-tracking?
http://lists.netfilter.org/pipermail/netfilter-devel/2004-May/015166.html

I'm still understanding the current secuence tracking, I have some ideas 
but I want to think a bit more about them, I'll let you know.

regards,
Pablo