Bug 1502

Created attachment 1444 [details]
Proposed patch

I don't know if the expected behaviour is that one. I should try a real implementation.
The proposed patch sounds good (always assuming the real implementaiton test confirms it), however something is missing from it.

Let's summarize the issue. Right now ns-3 behaves as following:
- The receiver will receive the in-flight packets, but not those queued in the transmitter.
- If a real socket will send the queued packets as well, you're right about the patch (I'd remove the whole check), BUT...
... but the "Send" and "SendTo" should check for m_shudownSend and return the appropriate error, thus restoring the intended behaviour, also about m_errno.

The patch is only slighty more complex.

T.

Created attachment 1483 [details]
New patch

New proposed patch taking care of the send and sendto behavior.

I believe on a shutdown the sender should send a FIN once his buffer is empty.  From what I can tell testing this patch, this does not occur.  Shouldn't this behave similarly to Close?  Couldn't it even use the same boolean, m_closeOnEmpty?  That would cause a Node 1 to send a FIN once its buffer was empty and close that 1/2 of the connection.  Please correct me if I am misunderstaind something.

(In reply to comment #4)
> I believe on a shutdown the sender should send a FIN once his buffer is
> empty.  From what I can tell testing this patch, this does not occur. 
> Shouldn't this behave similarly to Close?  Couldn't it even use the same
> boolean, m_closeOnEmpty?  That would cause a Node 1 to send a FIN once its
> buffer was empty and close that 1/2 of the connection.  Please correct me if
> I am misunderstaind something.

I do not know what it should be done on this level, 
At the application level I would like this behavior :

  if a sender write something on a socket then do a shutdown on send part,
  I expect that the receiver will receive all the data sended by the writer 
  before the shutdown.

I thing these 2 links are clarifying the point:
http://michael.toren.net/mirrors/sock-faq/unix-socket-faq-2.html
http://msdn.microsoft.com/en-us/library/windows/desktop/ms738547(v=vs.85).aspx

Despite the second one being msdn, it's quite clear about what should happen on a shutdown.

The answer is: no, shutdown and close are quite different. Now, in ns-3 I don't think we have (yet) the problem of two processes using the same socket, however it could happen. I don't know how this can handled, but I'd leave it as an enhancement for the future.

About the specific problem, as is if ShutdownSend should send a FIN, the anser is: no. It's the receiver that should send the FIN when it doesn't have any more data to send. If the sender would send a FIN, then the receiver couldn't send the (eventual) reply to the sender.

Now, from http://alas.matf.bg.ac.rs/manuals/lspe/snode=19.html
Shutting down the writing end of a socket solves a number of thorny problems. They are
- Flushes out the kernel buffers that contain any pending data to be sent. Data is buffered by the kernel networking software to improve performance.
- Sends an end-of-file indication to the remote socket. This tells the remote reading process that no more data will be sent to it on this socket.
- Leaves the partially shutdown socket open for reading. This makes it possible to receive confirmation messages after the end-of-file indication has been sent on the socket.
- Disregards the number of open references on the socket. Only the last close(2) on a socket will cause an end-of-file indication to be sent.

Hence, the sender shouldn't send a FIN, it should send an EOF (null), thus indicating that the sender did a ShutdownSend.
How the receiver is reacting to this is another issue, as it does involve the application layer. According to the idea, it's receiver's responsibility to tear down the socket when finished.

Sorry if this is becoming more and more complex...

T.

(In reply to comment #6)
> - Sends an end-of-file indication to the remote socket. This tells the
> remote reading process that no more data will be sent to it on this socket.

I'm guessing that the above mention of the EOF refers to a TCP FIN.  

(In reply to comment #4)
> I believe on a shutdown the sender should send a FIN once his buffer is
> empty.  From what I can tell testing this patch, this does not occur. 
> Shouldn't this behave similarly to Close?  Couldn't it even use the same
> boolean, m_closeOnEmpty?  That would cause a Node 1 to send a FIN once its
> buffer was empty and close that 1/2 of the connection. 

Please check that m_closeOnEmpty does not block the socket for reading.  If m_closeOnEmpty == true only acts on the sending direction of data transfer, then potentially this can be used.

(In reply to comment #7)
> (In reply to comment #6)
> > - Sends an end-of-file indication to the remote socket. This tells the
> > remote reading process that no more data will be sent to it on this socket.
> 
> I'm guessing that the above mention of the EOF refers to a TCP FIN.  

Nope, a FIN would trigger immediately a FIN-ACK, thus closing the socket. The documents I've read are referring to something like an EOF. The only thing I can think of it's a null byte, I'll have to read the TCP RFC to have a definitive answer.

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > - Sends an end-of-file indication to the remote socket. This tells the
> > > remote reading process that no more data will be sent to it on this socket.
> > 
> > I'm guessing that the above mention of the EOF refers to a TCP FIN.  
> 
> Nope, a FIN would trigger immediately a FIN-ACK, thus closing the socket.

Not true in general, depends on whether the far side application has already closed the connection.  Shutdown send should cause a FIN to be sent after all of the data has been sent, and the far side (if it hasn't closed or shutdown the socket for reading) should be able to read all of the data.

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #6)
> > > > - Sends an end-of-file indication to the remote socket. This tells the
> > > > remote reading process that no more data will be sent to it on this
> socket.
> > >
> > > I'm guessing that the above mention of the EOF refers to a TCP FIN.
> >
> > Nope, a FIN would trigger immediately a FIN-ACK, thus closing the socket.
> 
> Not true in general, depends on whether the far side application has already
> closed the connection.  Shutdown send should cause a FIN to be sent after all of
> the data has been sent, and the far side (if it hasn't closed or shutdown the
> socket for reading) should be able to read all of the data.

I see your point.

Now, the only one way to check it and be sure about what's the right thing to do is to implement two simple programs and watch their pcaps. And check as well the TCP RFC.

I'll do as soon as I can.

T.

Created attachment 1485 [details]
test programs and pcap

Ok, final word (not because it's mine, because it's the RFC's one).
-----
  There are essentially three cases:

    1) The user initiates by telling the TCP to CLOSE the connection

    2) The remote TCP initiates by sending a FIN control signal

    3) Both users CLOSE simultaneously

  Case 1:  Local user initiates the close

    In this case, a FIN segment can be constructed and placed on the
    outgoing segment queue.  No further SENDs from the user will be
    accepted by the TCP, and it enters the FIN-WAIT-1 state.  RECEIVEs
    are allowed in this state.  All segments preceding and including FIN
    will be retransmitted until acknowledged.  When the other TCP has
    both acknowledged the FIN and sent a FIN of its own, the first TCP
    can ACK this FIN.  Note that a TCP receiving a FIN will ACK but not
    send its own FIN until its user has CLOSED the connection also.

  Case 2:  TCP receives a FIN from the network

    If an unsolicited FIN arrives from the network, the receiving TCP
    can ACK it and tell the user that the connection is closing.  The
    user will respond with a CLOSE, upon which the TCP can send a FIN to
    the other TCP after sending any remaining data.  The TCP then waits
    until its own FIN is acknowledged whereupon it deletes the
    connection.  If an ACK is not forthcoming, after the user timeout
    the connection is aborted and the user is told.
-----
The 3rd case is not interesting for this discussion.
 
I modified two small programs found in Internet (they're in the attachments) and the client DOES send a FIN.
The point (Tom was right), is that the server is not forced to send a FIN back, it can just send an ACK and do whatever it wants. When it's done, it will reply with its FIN.
 
This basically close the discussion. Upon a ShutdownSend(), TCP should *queue* a FIN (to be sent after all the queued data). The other connected node should *not* immediately send a FIN, it should instead warn the application level that the socket half-close.

This is signaled by "recv()" on unix in a very simple way... "man recv"...
-----
RETURN VALUES
     These calls return the number of bytes received, or -1 if an error
     occurred.

     For TCP sockets, the return value 0 means the peer has closed its half
     side of the connection.
-----
     
How to fix our sockets to be fully compliant with this is another point.

Cheers,

T.

Created attachment 1513 [details]
Proposed patch

Attached is a patch to adds to Frederic's patch by having the shutdown initiator send a FIN once his buffer is empty.  

Assume TCP connection between A and B

When A does shutdown he will be unable to send any more data and a FIN will be sent once his buffer is empty. He will go into FIN_WAIT_1 and when the FIN+ACK comes from B he will go into FIN_WAIT_2.  During both of these states, A will still be able to accept and read incoming data from B.   

If B wants to know about A shutting down, there is already a callback offered in socket.  

I'm going to write a 2 way data transfer tcp test to verify that things are working correctly.

Any comments/suggestions would be greatly appreciated.

Thanks

(In reply to comment #13)
> If B wants to know about A shutting down, there is already a callback
> offered in socket.  

Seems all fine, although I didn't test it. I'll wait for you test program

On the other hand, I have a question... what is the callback you're referring to ?

ns-3 (basically) is implementing nonblocking sockets, hence a callback is the way to go, but inspecting the TcpSocketBase I haven't found it.

Could be me, of course, since it's late and I have a flu.

Cheers,

T.

Created attachment 1518 [details]
YAP - Yet another patch

During testing found that the last patch wouldn't send a fin if shutdown was called and the tx buffer was already empty.

Created attachment 1519 [details]
Two way tcp test program

The callback is in base class socket.  That is the correct one to use right?

Below is a partial log.  Source (node 1) and Server (node 0) sending data to each other.  Server sends less and calls shutdown.  Server still receives and reads data received after shutdown.  Source application notified that server closed 1/2 of connection midway down.

0 [node 0] I'm calling shutdown!!
...
0 [node 0] ESTABLISHED -> FIN_WAIT_1

0 [node 0] TcpSocketBase:ReceivedData(0x118d650, 49153 > 50000 [ ACK ] Seq=501 Ack=501 Win=65535)

0 [node 0] TcpSocketBase:ReceivedData(0x118d650, 49153 > 50000 [ ACK ] Seq=1001 Ack=501 Win=65535)
...
0 [node 1] TcpSocketBase:ProcessEstablished(0x118cbd0, 50000 > 49153 [ FIN  ACK ] Seq=501 Ack=501 Win=65535)
0 [node 1] TcpSocketBase:PeerClose(0x118cbd0, 50000 > 49153 [ FIN  ACK ] Seq=501 Ack=501 Win=65535)
0 [node 1] Accepted FIN at seq 1001
...
0 [node 1] TcpSocketBase:ReceivedData(0x118cbd0, 50000 > 49153 [ FIN  ACK ] Seq=501 Ack=501 Win=65535)
...
0 [node 1] ESTABLISHED -> CLOSE_WAIT
0 [node 1] TCP 0x118cbd0 calling NotifyNormalClose
...
0 [node 1] Server has closed
...
0 [node 0] TCP 0x118d650 NewAck 1002 numberAck 501
...
0 [node 0] FIN_WAIT_1 -> FIN_WAIT_2
...
0 [node 0] TcpSocketBase:ReceivedData(0x118d650, 49153 > 50000 [ ACK ] Seq=2001 Ack=1002 Win=65535)

1 [node 1] CLOSE_WAIT -> LAST_ACK

1 [node 0] TcpSocketBase:ProcessWait(0x118d650, 49153 > 50000 [ FIN  ACK ] Seq=20001 Ack=1002 Win=65535)
1 [node 0] TcpSocketBase:ReceivedAck(0x118d650, 49153 > 50000 [ FIN  ACK ] Seq=20001 Ack=1002 Win=65535)
1 [node 0] FIN_WAIT_2 -> TIME_WAIT


1 [node 0] TcpSocketBase:SendEmptyPacket(0x118d650, 16)
1 [node 1] TcpSocketBase:ProcessLastAck(0x118cbd0, 50000 > 49153 [ ACK ] Seq=1002 Ack=20002 Win=65535)
1 [node 1] TcpSocketBase:CloseAndNotify(0x118cbd0)
1 [node 1] LAST_ACK -> CLOSED

Source has closed
241 [node 0] TIME_WAIT -> CLOSED

m_currentSourceTxBytes: 20000
m_currentSourceRxBytes: 1000
m_currentServerRxBytes: 20000
m_currentServerTxBytes: 1000

All seems to behave as intended. I'd love to see a pcap file of this data exchange (they're easier to read for me), however the trace seems to show the right behaviour.

I have a last doubt (it's about piggybacked ACKs and their timers with a half-closed comm), but I guess that it goes beyond the point of this bug and more into the "TCP implementatin validation".

I guess you checked also the tests and everything is working as before. If so, I have no further issues with this patch.

T.

PS: why this bug status is stll "New" ? Shouldn't it be "Confirmed" or, even better, "Patch Pending" ? Anyway, changing to Confirmed.

 9258:620eeaa0e173