Samba 3 socket status after [RST, ACK]

[Petr Sumbera]

Hi Simo,

simo wrote:
> On Thu, 2006-11-30 at 15:51 +0100, Petr Sumbera wrote:
> 
>> Where I see the problem:
>> ========================
>>
>> Don't know why Windows is reseting TCP connection. But it seems that 
>> there are usually in such a case two attempts to connect to samba server 
>> (windows is trying to connect twice to ports 139 or to port 139 and 445) 
>> at the same time.
>>
>> Samba is not functionally affected in any way. There are only several 
>> messages in log and samba is once forked more. But if there is many 
>> Windows clients, there is many messages. And in that case it can be a 
>> problem (messages and performance)...
> 
> 
> This is a well known behavior of Windows clients, instead of trying 445
> first and then falling back to 139, they connect to both at the same
> time and then drop one of the 2 if they get answers from both, this
> probably to avoid long timeouts if port 445 is filtered and packets are
> dropped.
> 
>> How to avoid it?
>> ================
>>
>> I think we should add into samba after accept() is returned some check 
>> for socket sanity. If we find that socket is in some wrong state we 
>> shouldn't even do fork (just a close socket and continue in main loop).
> 
> I don't think that is easy nor necessarily a good way to deal with this.
> 
>> But I see there some possible race condition. I believe [RST, ACK] can 
>> arrive little bit later (after we returned from accept() and maybe even 
>> after we are forked).
> 
> Exactly, you can't wait just to see if the connection will be dropped.

But we know that since getpeername returns error. I know that there is 
race condition but in most cases I have seen [RST,ACK] arrived before we 
returned from accept(). If we handled this error, we would save a lot of 
resources:
- forking new process
- some other socket handling
- creating log file for client 0.0.0.0, ...

(I believe these are not very cheap indeed and worst for thinking about 
some change around accept().)

We don't have to use getpeername for that check (if we don't want change 
code much), I tried for example calling recv() immediately after 
accept() with "len" equal to zero.

If the above race condition happen (and we are checking for socket 
sanity after accept() return only) we can still have solution with 
different log level for those errors.

>> So, probably the best idea is to check return values from all socket 
>> calls and behave according them (even for getpeername() and setsockopt()).
> 
> The fix we put in current code IIRC is to just raise the debug level so
> that it will not show up at lower debug levels. I am not sure there is
> any pressing reason to do anything else.

I tried samba 3.0.23d and it's still there. Can you please check if the 
change was already committed?

Thanks,

Petr