From: Chris Friesen <hidden> Date: 2012-08-30 15:21:33
Hi,
I'm seeing some behaviour that looks unintentional in the TIPC connect() call.
I'm running TIPC 1.7.7. My userspace code (stripped of error handling) looks
like this:
int sd = socket (AF_TIPC, SOCK_SEQPACKET,0);
connect(sd,(struct sockaddr*)&topsrv,sizeof(topsrv));
where topsrv is the address of the TIPC topology server. The thing that's weird
is that intermittently we get a EISCONN error on the connect call.
Looking at the TIPC connect() code, I think what is happening is that
wait_event_interruptible_timeout() is being interrupted by a signal and returns
-ERESTARTSYS. This sets sock->state to SS_DISCONNECTING and exits. Userspace
sees the ERESTARTSYS and retries the syscall, then we hit the
"sock->state != SS_UNCONNECTED" check and exit with -EISCONN.
I think current mainline is susceptible to this as well.
I'm not sure what the proper fix would be--can we detect coming back in that we were
waiting for a message and just skip down to the wait_event_interruptible_timeout()
call?
Chris
--
Chris Friesen
Software Designer
3500 Carling Avenue
Ottawa, Ontario K2H 8E9
www.genband.com
Hi Chris,
Although this is a known issue, still thanks for your report.
Regardless of 1.7.7 or mainline, the issue really exists.
Can you please check and verify the attached patch?
PS: the patch is based on 1.7.7 rather than mainline.
Thanks,
Ying
Chris Friesen wrote:
Hi,
I'm seeing some behaviour that looks unintentional in the TIPC connect() call.
I'm running TIPC 1.7.7. My userspace code (stripped of error handling) looks
like this:
int sd = socket (AF_TIPC, SOCK_SEQPACKET,0);
connect(sd,(struct sockaddr*)&topsrv,sizeof(topsrv));
where topsrv is the address of the TIPC topology server. The thing that's weird
is that intermittently we get a EISCONN error on the connect call.
Looking at the TIPC connect() code, I think what is happening is that
wait_event_interruptible_timeout() is being interrupted by a signal and returns
-ERESTARTSYS. This sets sock->state to SS_DISCONNECTING and exits. Userspace
sees the ERESTARTSYS and retries the syscall, then we hit the
"sock->state != SS_UNCONNECTED" check and exit with -EISCONN.
I think current mainline is susceptible to this as well.
I'm not sure what the proper fix would be--can we detect coming back in that we were
waiting for a message and just skip down to the wait_event_interruptible_timeout()
call?
Chris
From: Chris Friesen <hidden> Date: 2012-08-31 15:11:56
On 08/31/2012 03:37 AM, Ying Xue wrote:
Hi Chris,
Although this is a known issue, still thanks for your report.
Regardless of 1.7.7 or mainline, the issue really exists.
Can you please check and verify the attached patch?
PS: the patch is based on 1.7.7 rather than mainline.
I haven't had a chance to test it yet but from visual inspection the
patch looks pretty good.
It looks like the case where we come in with "sock->state ==
SS_LISTENING" has changed. Previously we would return -EOPNOTSUPP but
now it'll be -EINVAL. Is that intentional?
Chris
From: Chris Friesen <hidden> Date: 2012-08-31 15:19:21
On 08/31/2012 09:09 AM, Chris Friesen wrote:
On 08/31/2012 03:37 AM, Ying Xue wrote:
quoted
Hi Chris,
Although this is a known issue, still thanks for your report.
Regardless of 1.7.7 or mainline, the issue really exists.
Can you please check and verify the attached patch?
PS: the patch is based on 1.7.7 rather than mainline.
I haven't had a chance to test it yet but from visual inspection the
patch looks pretty good.
It looks like the case where we come in with "sock->state ==
SS_LISTENING" has changed. Previously we would return -EOPNOTSUPP but
now it'll be -EINVAL. Is that intentional?
Just noticed something else. In the SS_CONNECTING case I don't think
there's any point in setting "res = -EALREADY" since a bit further down
it gets set unconditionally anyway.
Chris
It looks like the case where we come in with "sock->state ==
SS_LISTENING" has changed. Previously we would return -EOPNOTSUPP but
now it'll be -EINVAL. Is that intentional?
Just noticed something else. In the SS_CONNECTING case I don't think
there's any point in setting "res = -EALREADY" since a bit further
down it gets set unconditionally anyway.
Yes, you are right. Please check v2 version.
Thanks,
Ying
From: Chris Friesen <hidden> Date: 2012-09-04 14:54:36
On 09/02/2012 09:16 PM, Ying Xue wrote:
quoted
quoted
It looks like the case where we come in with "sock->state ==
SS_LISTENING" has changed. Previously we would return -EOPNOTSUPP but
now it'll be -EINVAL. Is that intentional?
Just noticed something else. In the SS_CONNECTING case I don't think
there's any point in setting "res = -EALREADY" since a bit further
down it gets set unconditionally anyway.
Yes, you are right. Please check v2 version.
Looks good to me. Still haven't had a chance to actually test it
though, we're pushing out a release and time is scarce.
Chris
From: Chris Friesen <hidden> Date: 2012-10-31 23:01:39
On 09/04/2012 08:54 AM, Chris Friesen wrote:
On 09/02/2012 09:16 PM, Ying Xue wrote:
quoted
quoted
quoted
It looks like the case where we come in with "sock->state ==
SS_LISTENING" has changed. Previously we would return -EOPNOTSUPP but
now it'll be -EINVAL. Is that intentional?
Just noticed something else. In the SS_CONNECTING case I don't think
there's any point in setting "res = -EALREADY" since a bit further
down it gets set unconditionally anyway.
Yes, you are right. Please check v2 version.
Looks good to me. Still haven't had a chance to actually test it though,
we're pushing out a release and time is scarce.
I finally got a chance to test this out, and it does indeed fix the
problem. Please push it upstream.
Tested-by: Chris Friesen <redacted>
Chris
I finally got a chance to test this out, and it does indeed fix the
problem. Please push it upstream.
Tested-by: Chris Friesen <redacted>
Thanks for your hard working, I will submit it again.
Regards,
Ying
Chris
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html