"Close connection fast if TIME_WAIT support is off" broke active close support

[pfalcon]

#539 / 210c308 broke standard Internet services implementations, when client makes a request, server sends a response, and closes connection. After the above patch, a client doesn't notice connection close (e.g. telnet/curl keep running, waiting for more data).

This issue wasn't spotted apparently because we don't have an implementation of a standard Internet server in Zephyr codebase (instead we have more advanced things like HTTP 1.1 pipeline connections which don't do server close).

The issue was found using https://github.com/micropython/micropython/blob/master/examples/network/http_server.py sample application with MicroPython Zephyr port.

[pfalcon]

Looking at the above patch, it doesn't do the right thing. Its description talks about TIME_WAIT, but the code doesn't touch anything directly affecting TIME_WAIT. A patch optimizing out TIME_WAIT, should do following: queue FIN (as the original code does), wait for ACK to it (including retransmit on timeout), and only after that, it can unref context immediately, whereas the RFC requires it to go into TIME_WAIT state, and live in it for quite some time.

Suggestion re: above patch: revert.

[jukkar]

Ok, lets revert the commit 210c308 then, please submit the patch to master. There was a reason I submitted the patch but now I do not recall the exact reasoning for that and the commit message does not tell enough info.
Can you add to the revert patch some more info why we are reverting so that it is not a mystery later why we did this?

[pfalcon]

@jukkar, If you think that reverting is good idea, that per info in #729, I'd rather instead submit a patch on top of it which fixes an immediate issue. Let me wait a bit though, if there will be comments re: synchronizing access to global array raised #729 (comment) . If there won't be, I'll submit just quick fix by the end of day, leaving sync issues for later. Thanks.

[jukkar]

I investigated this a bit more and the reasoning in commit 210c308 for putting the queue_fin() behind CONFIG_NET_TCP_TIME_WAIT was that if the TIME_WAIT support is not enabled (default), then the context was never released properly if the peer FIN was lost.
Anyway, after looking the code, we should probably send a FIN always, and just unref the context unconditionally, otherwise the context might be left hanging a long time.

[jukkar]

I did some experimenting, and it seems that we need a timer to make this work nicely. So we should send a FIN when closing the connection, then install a timer (just in case the peer FIN is lost), and then unref the net_context inside the close timer if the peer FIN was lost. I can cook a patch for doing this.

[pfalcon]

Thanks!

[jukkar]

Related to this, the passive close support got broken when implementing the syn-ack backlog support. I will send a patch that fixes this too.

[jukkar]

Fixes for active and passive close issues at #736

[pfalcon]

Fixed.