On 03.07.2011 17:00, Mikolaj Golub wrote:
Hi,
Hi Mikolaj,
Trying soreceive_stream I found that many applications (like firefox, pidgin, gnus) might hang in soreceive_stream/sbwait.
Many thanks for testing soreceive_stream.
It was shown up that the issue was with O_NONBLOCK connections -- they blocked in recv() when should not have been.
>
This can be checked with this simple test: http://people.freebsd.org/~trociny/test_nonblock.c In soreceive_stream we have the following code that looks wrong: 1968 /* Socket buffer is empty and we shall not block. */ 1969 if (sb->sb_cc == 0&& 1970 ((sb->sb_flags& SS_NBIO) || (flags& (MSG_DONTWAIT|MSG_NBIO)))) { 1971 error = EAGAIN; 1972 goto out; 1973 } It should check so->so_state agains SS_NBIO, not sb->sb_flags. But just
Indeed.
changing this is not enough. This check is called too early -- before checking that socket state is not SBS_CANTRCVMORE. As a result, if the peer closes the connection recv() returns EAGAIN instead of 0. See this example: http://people.freebsd.org/~trociny/test_close.c So I moved the "nonblock" check below SBS_CANTRCVMORE check and ended up with this patch: http://people.freebsd.org/~trociny/uipc_socket.c.soreceive_stream.patch It works for me fine.
OK.
Also, this part looks wrong: 1958 /* We will never ever get anything unless we are connected. */ 1959 if (!(so->so_state& (SS_ISCONNECTED|SS_ISDISCONNECTED))) { 1960 /* When disconnecting there may be still some data left. */ 1961 if (sb->sb_cc> 0) 1962 goto deliver; 1963 if (!(so->so_state& SS_ISDISCONNECTED)) 1964 error = ENOTCONN; 1965 goto out; 1966 } Why we check in 1959 that state is not SS_ISDISCONNECTED? If it is valid then the check at 1963 is useless becase it will be always true. Shouldn't it be something like below?
>
if (!(so->so_state& (SS_ISCONNECTED|SS_ISCONNECTING))) { /* When disconnecting there may be still some data left. */ if (sb->sb_cc> 0) goto deliver; error = ENOTCONN; goto out; } (I don't see why we souldn't set ENOTCONN if the state is SS_ISDISCONNECTED).
ENOTCONN is only valid in the beginning of the connection. When we reach SS_INDISCONNECTED the connection was connected before and we have to return a 0-read to signal EOF. It can be simplified by immediately setting the error and going out. The other cases can't ever be true. Please try this patch: http://people.freebsd.org/~andre/soreceive_stream.diff-20110707 I can't fully test it myself due to being 1000km from my development machine and having only limited hw access for rebooting.
And the last :-). Currently, to try soreceive_stream one need to rebuild kernel with TCP_SORECEIVE_STREAM and then set tunable net.inet.tcp.soreceive_stream. Why do we need TCP_SORECEIVE_STREAM option? Wouldn't tunable be enough? It would simplify trying soreceive_stream by users and we might have more testing/feedback.
Done. See r223839. -- Andre _______________________________________________ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"