Re: [PATCH] socket.7: be explicit that connect(2) respects SO_*TIMEO

[Alejandro Colomar <alx.manpages@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7078 bytes --]

Hi Tycho,

On 11/28/22 21:58, Tycho Andersen wrote:
> Our group recently had some confusion around this. Although f327722042df
> ("socket.7: Explain effect of SO_SNDTIMEO for connect()") adds a mention of
> connect(2), the wording around "Timeouts  only  have  effect  for  system
> calls  that perform socket I/O" is slightly confusing: is connect(2) I/O?.

I believe it is, because of the following table used in the same page:

        ┌──────────────────────────────────────────────────────────────────────┐
        │                             I/O events                               │
        ├───────────┬───────────┬──────────────────────────────────────────────┤
        │Event      │ Poll flag │ Occurrence                                   │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Read       │ POLLIN    │ New data arrived.                            │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Read       │ POLLIN    │ A  connection  setup has been completed (for │
        │           │           │ connection‐oriented sockets)                 │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Read       │ POLLHUP   │ A disconnection request has  been  initiated │
        │           │           │ by the other end.                            │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Read       │ POLLHUP   │ A connection is broken (only for connection‐ │
        │           │           │ oriented  protocols).   When  the  socket is │
        │           │           │ written SIGPIPE is also sent.                │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Write      │ POLLOUT   │ Socket has  enough  send  buffer  space  for │
        │           │           │ writing new data.                            │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Read/Write │ POLLIN |  │ An outgoing connect(2) finished.             │
        │           │ POLLOUT   │                                              │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Read/Write │ POLLERR   │ An asynchronous error occurred.              │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Read/Write │ POLLHUP   │ The other end has shut down one direction.   │
        ├───────────┼───────────┼──────────────────────────────────────────────┤
        │Exception  │ POLLPRI   │ Urgent data arrived.  SIGURG is sent then.   │
        └───────────┴───────────┴──────────────────────────────────────────────┘

But I agree with you that it's unclear.

> Let's just add connect(2) to the list of things that time out explicitly to
> avoid any confusion.

So, yes, I like your patch.  So, patch applied.

> 
> Test program for grins:
> 
> #include <stdio.h>
> #include <sys/socket.h>
> #include <netinet/ip.h>
> #include <arpa/inet.h>

BTW, I'm curious, how did you get the headers in the commit message?  I always 
have a space before the # to avoid git doing harmful stuff.


Cheers,

Alex

P.S.:  I don't know if you could please review some patches about which you may 
be more familiar than I am.  The are somewhat related to this.  They are here:

<https://lore.kernel.org/linux-man/20221122153027.10943-1-henri.van.de.water@xs4all.nl/T/#t>
<https://lore.kernel.org/linux-man/2d4d8b7b-5890-cd9f-061d-6d259d8ed6ee@xs4all.nl/T/#t>

> 
> int main(void)
> {
> 	struct sockaddr_in servaddr = {
>                  /* tycho.pizza */
>                  .sin_addr.s_addr = inet_addr("192.241.255.151"),
>                  .sin_port = htons(443),
>                  .sin_family = AF_INET,
>          };
> 	int fd;
> 	struct timeval timeout = {
> 		.tv_sec = 0,
> 		.tv_usec = 100,
> 	};
> 
> 	fd = socket(AF_INET, SOCK_STREAM, 0);
> 	if (fd < 0) {
> 		perror("socket");
> 		return 1;
> 	}
> 
> 	if (setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout)) < 0) {
> 		perror("setsockopt");
> 		return 1;
> 	}
> 
> 	if (connect(fd, (struct sockaddr *)&servaddr, sizeof(servaddr)) < 0) {
> 		perror("connect");
> 		return 1;
> 	}
> 
> 	printf("connect successful\n");
> 	return 0;
> }
> 
> $ ./so_sndtimeo
> connect: Operation now in progress
> 
> Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
> ---
>   man7/socket.7 | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/man7/socket.7 b/man7/socket.7
> index 2b191c783..c3c13cda6 100644
> --- a/man7/socket.7
> +++ b/man7/socket.7
> @@ -838,6 +838,7 @@ just as if the socket was specified to be nonblocking.
>   If the timeout is set to zero (the default),
>   then the operation will never timeout.
>   Timeouts only have effect for system calls that perform socket I/O (e.g.,
> +.BR connect (2),
>   .BR read (2),
>   .BR recvmsg (2),
>   .BR send (2),
> 
> base-commit: 60eb580d1e836977d57355b6519f32e37bdc3392

-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]