Re: [tipc-discussion] [PATCH v3] tipc: return POLLOUT for sockets in an unconnected state

[Paul G. <pau...@wi...>]

On 12-10-16 10:47 AM, eri...@er... wrote:
> From: Erik Hugne <eri...@er...>
> 
> If an implied connect is attempted on a nonblocking STREAM/SEQPACKET
> socket during link congestion, the connect message will be discarded
> and sendmsg will return EAGAIN. This is normal behavior, and the
> application is expected to poll the socket until POLLOUT is set,
> after which the connection attempt can be retried.
> However, the POLLOUT flag is never set for unconnected sockets and
> poll() always returns a zero mask. The application is then left without
> a trigger for when it can make another attempt at sending the message.
> 
> The solution is to check if we're polling on an unconnected socket
> and set the POLLOUT flag if the TIPC port owned by this socket
> is not congested. The TIPC ports waiting on a specific link will be
> marked as 'not congested' when the link congestion have abated.
> 
> Signed-off-by: Erik Hugne <eri...@er...>

Looks good, thanks Erik.  Help me gauge the impact of this -- i.e.
is it a rare corner case, or an easy to trigger bug that is
impacting a lot of people?    If it has a high impact, we can
suggest it for "net" and hence 3.7 inclusion, otherwise we
should not needlessly add to the post merge window burden, and
instead nominate it for net-next which means 3.8.

Paul.
--

> ---
>  net/tipc/socket.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 09dc5b9..e9a9dfd 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -407,7 +407,7 @@ static int get_name(struct socket *sock, struct sockaddr *uaddr,
>   * socket state		flags set
>   * ------------		---------
>   * unconnected		no read flags
> - *			no write flags
> + *			POLLOUT if port is not congested
>   *
>   * connecting		POLLIN/POLLRDNORM if ACK/NACK in rx queue
>   *			no write flags
> @@ -437,6 +437,10 @@ static unsigned int poll(struct file *file, struct socket *sock,
>  	poll_wait(file, sk_sleep(sk), wait);
>  
>  	switch ((int)sock->state) {
> +	case SS_UNCONNECTED:
> +		if (!tipc_sk_port(sk)->congested)
> +			mask |= POLLOUT;
> +		break;
>  	case SS_READY:
>  	case SS_CONNECTED:
>  		if (!tipc_sk_port(sk)->congested)
>