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

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

On 12-06-01 07:07 AM, Erik Hugne wrote:

[Eric asked for comments on this patch ; I know it was resent 
 recently but I could only find this older version in my inbox
 (but the patch itself looks the same)]

> If an implied connect is attempted during link congestion,
> the connect message will be discarded and sendmsg will return
> EAGAIN. The application then need to retry the connection attempt
> after congestion have abated.
> This fixes a problem where polling on a socket in an
> unconnected state always returned a zero mask.

Would be nice to expand on why "connect message will be discarded
and sendmsg will return EAGAIN" is a problem -- i.e. what is the
end user symptom this causes, since it is not obvious to me.

In a similar vein, it is not obvious to me how "polling on a
socket in an unconnected state always returned a zero mask" is
a problem (again, what are end user symptoms) and it is also
not obvious what the "new" behaviour is after this commit
is applied.

This is just a repeat of the generalization for commit logs,
where the requirement is to 1) describe the problem, 2) describe
the end user symptoms (the use case it impacts, and how it
breaks it) and 3) describe the fix - i.e. how the fix
works and why it was chosen as the _right_ fix.

Also it looks like there are space-before-tab in the comment
line -- using checkpatch.pl will alert you about these.

Thanks,
Paul.

> 
> Signed-off-by: Erik Hugne <eri...@er...>
> ---
>  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 03a2610..7710145 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)
>