Hi Pablo, On Sat, 26 Feb 2011, Pablo Neira Ayuso wrote: > On 26/02/11 19:30, Jozsef Kadlecsik wrote: > > Maybe I'm just blind, but I don't see what you try to achieve. Actually, > > with your patch you switch off the path which would handle the case when > > conntrack holds an outdated entry from whatever reason. > > > > Present code, cases: > > > > a. conntrack in sync, SYN/ACK lost and client resends the SYN > > - we ignore it but it does not do any harm > > b. conntrack entry is outdated, new SYN received > > - we ignore it but save the initialization data from it > > - when the reply SYN/ACK receives and it matches the saved data, > > we pick up the new connection > > This is what it seems at first sight, but this is not what it happens if > the conntrack is in SYN_RECV state. > > Initially, the SYN packet hits first item of 'b', thus we save data from > it. But the SYN/ACK packet is considered a retransmission given that > we're in SYN_RECV state. Therefore, we never hit the second item of 'b'. Yes, you're right! We don't hit 'b' in the SYN_RECV state. > > With your patch, cases: > > > > a. conntrack in sync, SYN/ACK lost and client resends the SYN > > - SYN is out of window and will be marked as invalid > > b. conntrack entry is outdated, new SYN received > > - SYN is out of window and will be marked as invalid > > Damn, you're right, this patch is broken, that SYN packet won't pass > boundary III, IIRC. > > > What do I miss? > > That we're in SYN_RECV state :-). > > >> This patch also helps a lot to conntrackd in stress scenarios > >> (assumming a client that generates lots of small TCP connections). > >> During the failover, consider that the new primary has injected > >> one outdated flow in SYN_RECV state (this is likely to happen if > >> the conntrack event rate is high because the backup will be a bit > >> delayed from the primary). With the current code, if the client > >> starts a new fresh connection that matches the tuple, the SYN > >> packet will be ignored without updating the state tracking, and > >> the SYN+ACK in reply will blocked as it will not pass checkings > >> III or IV (since all state tracking in the original direction is > >> not initialized because of the SYN packet was ignored). > > > > The last_dir entries are updated from the ignored SYN packet and thus the > > SYN+ACK won't be blocked - if the packet is a true answer to the SYN. > > This is what it happens with other states, but not for SYN_RECV. > > I have reworked the patch and the description. With this, the SYN_RECV > state if we're out of sync receives the same handling. > > I have test it here, it works fine. Let me know if you're OK with it. The patch looks OK but I think Changli Gao is also right and it'd be simpler to set the [reply][synack][SR] state to sIG. What do you think? Best regards, Jozsef - E-mail : kadlec@xxxxxxxxxxxxxxxxx, kadlec@xxxxxxxxxxxx PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : KFKI Research Institute for Particle and Nuclear Physics H-1525 Budapest 114, POB. 49, Hungary -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
