* non-blocking connect for kernel SCTP sockets @ 2018-05-02 9:06 ` Michal Kubecek 0 siblings, 0 replies; 8+ messages in thread From: Michal Kubecek @ 2018-05-02 9:06 UTC (permalink / raw) To: netdev Cc: linux-sctp, linux-kernel, Vlad Yasevich, Neil Horman, Gang He, GuoQing Jiang Hello, while investigating a bug, we noticed that DLM tries to connect an SCTP socket in non-blocking mode using result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len, O_NONBLOCK); which does not work. The reason is that inet_dgram_connect() cannot pass its flags argument to sctp_connect() so that __sctp_connect() which does the actual waiting resorts to checking sk->sk_socket->file->f_flags instead. As the socket used by DLM is a kernel socket with no associated file, it ends up blocking. TCP doesn't suffer from this problem as for TCP, the waiting is done in inet_stream_connect() which has the flags argument. I also checked other proto::connect handlers and sctp_connect() seems to be the only one with this kind of problem. This could be worked around in DLM and further experiments indicate current DLM code wouldn't actually handle the non-blocking connect properly. But I still feel ignoring the flags argument is rather a trap that should be fixed. I have prepared a series adding flags argument to proto::connect and using it in sctp_connect() and __sctp_connect(). But I'm not sure if it's not too big hammer to address issue only affecting one handler. So my question is: would such generic approach be preferred or should we rather make SCTP work the way TCP does, i.e. move the waiting from proto::connect() to proto_ops::connect()? This would require introducing inet_seqpacket_connect() as inet_dgram_connect() is primarily intended for use with UDP.) Michal Kubecek ^ permalink raw reply [flat|nested] 8+ messages in thread
* non-blocking connect for kernel SCTP sockets @ 2018-05-02 9:06 ` Michal Kubecek 0 siblings, 0 replies; 8+ messages in thread From: Michal Kubecek @ 2018-05-02 9:06 UTC (permalink / raw) To: netdev Cc: linux-sctp, linux-kernel, Vlad Yasevich, Neil Horman, Gang He, GuoQing Jiang Hello, while investigating a bug, we noticed that DLM tries to connect an SCTP socket in non-blocking mode using result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len, O_NONBLOCK); which does not work. The reason is that inet_dgram_connect() cannot pass its flags argument to sctp_connect() so that __sctp_connect() which does the actual waiting resorts to checking sk->sk_socket->file->f_flags instead. As the socket used by DLM is a kernel socket with no associated file, it ends up blocking. TCP doesn't suffer from this problem as for TCP, the waiting is done in inet_stream_connect() which has the flags argument. I also checked other proto::connect handlers and sctp_connect() seems to be the only one with this kind of problem. This could be worked around in DLM and further experiments indicate current DLM code wouldn't actually handle the non-blocking connect properly. But I still feel ignoring the flags argument is rather a trap that should be fixed. I have prepared a series adding flags argument to proto::connect and using it in sctp_connect() and __sctp_connect(). But I'm not sure if it's not too big hammer to address issue only affecting one handler. So my question is: would such generic approach be preferred or should we rather make SCTP work the way TCP does, i.e. move the waiting from proto::connect() to proto_ops::connect()? This would require introducing inet_seqpacket_connect() as inet_dgram_connect() is primarily intended for use with UDP.) Michal Kubecek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: non-blocking connect for kernel SCTP sockets 2018-05-02 9:06 ` Michal Kubecek @ 2018-05-02 9:46 ` Xin Long -1 siblings, 0 replies; 8+ messages in thread From: Xin Long @ 2018-05-02 9:46 UTC (permalink / raw) To: Michal Kubecek Cc: network dev, linux-sctp, LKML, Vlad Yasevich, Neil Horman, Gang He, GuoQing Jiang On Wed, May 2, 2018 at 5:06 PM, Michal Kubecek <mkubecek@suse.cz> wrote: > Hello, > > while investigating a bug, we noticed that DLM tries to connect an SCTP > socket in non-blocking mode using > > result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len, > O_NONBLOCK); > > which does not work. The reason is that inet_dgram_connect() cannot pass > its flags argument to sctp_connect() so that __sctp_connect() which does > the actual waiting resorts to checking sk->sk_socket->file->f_flags > instead. As the socket used by DLM is a kernel socket with no associated > file, it ends up blocking. > > TCP doesn't suffer from this problem as for TCP, the waiting is done in > inet_stream_connect() which has the flags argument. I also checked other > proto::connect handlers and sctp_connect() seems to be the only one with > this kind of problem. > > This could be worked around in DLM and further experiments indicate > current DLM code wouldn't actually handle the non-blocking connect > properly. But I still feel ignoring the flags argument is rather a trap > that should be fixed. It is a bug, https://bugzilla.redhat.com/show_bug.cgi?id=1251530 We have the fix which also includes some cleanup, and needs to do more testing. > > I have prepared a series adding flags argument to proto::connect and > using it in sctp_connect() and __sctp_connect(). But I'm not sure if > it's not too big hammer to address issue only affecting one handler. > So my question is: would such generic approach be preferred or should we, > rather make SCTP work the way TCP does, i.e. move the waiting from, > proto::connect() to proto_ops::connect()? This would require introducing > inet_seqpacket_connect() as inet_dgram_connect() is primarily intended > for use with UDP.) We don't fix it in the generic proto::connect, which will afftect many other places. We're replacing only sctp's proto_ops::connect with sctp_connect and leave its proto::connect as NULL, so that it can get this flags param without touching the generic struct and code. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: non-blocking connect for kernel SCTP sockets @ 2018-05-02 9:46 ` Xin Long 0 siblings, 0 replies; 8+ messages in thread From: Xin Long @ 2018-05-02 9:46 UTC (permalink / raw) To: Michal Kubecek Cc: network dev, linux-sctp, LKML, Vlad Yasevich, Neil Horman, Gang He, GuoQing Jiang On Wed, May 2, 2018 at 5:06 PM, Michal Kubecek <mkubecek@suse.cz> wrote: > Hello, > > while investigating a bug, we noticed that DLM tries to connect an SCTP > socket in non-blocking mode using > > result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len, > O_NONBLOCK); > > which does not work. The reason is that inet_dgram_connect() cannot pass > its flags argument to sctp_connect() so that __sctp_connect() which does > the actual waiting resorts to checking sk->sk_socket->file->f_flags > instead. As the socket used by DLM is a kernel socket with no associated > file, it ends up blocking. > > TCP doesn't suffer from this problem as for TCP, the waiting is done in > inet_stream_connect() which has the flags argument. I also checked other > proto::connect handlers and sctp_connect() seems to be the only one with > this kind of problem. > > This could be worked around in DLM and further experiments indicate > current DLM code wouldn't actually handle the non-blocking connect > properly. But I still feel ignoring the flags argument is rather a trap > that should be fixed. It is a bug, https://bugzilla.redhat.com/show_bug.cgi?id\x1251530 We have the fix which also includes some cleanup, and needs to do more testing. > > I have prepared a series adding flags argument to proto::connect and > using it in sctp_connect() and __sctp_connect(). But I'm not sure if > it's not too big hammer to address issue only affecting one handler. > So my question is: would such generic approach be preferred or should we, > rather make SCTP work the way TCP does, i.e. move the waiting from, > proto::connect() to proto_ops::connect()? This would require introducing > inet_seqpacket_connect() as inet_dgram_connect() is primarily intended > for use with UDP.) We don't fix it in the generic proto::connect, which will afftect many other places. We're replacing only sctp's proto_ops::connect with sctp_connect and leave its proto::connect as NULL, so that it can get this flags param without touching the generic struct and code. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: non-blocking connect for kernel SCTP sockets 2018-05-02 9:46 ` Xin Long @ 2018-05-02 12:32 ` Michal Kubecek -1 siblings, 0 replies; 8+ messages in thread From: Michal Kubecek @ 2018-05-02 12:32 UTC (permalink / raw) To: Xin Long Cc: network dev, linux-sctp, LKML, Vlad Yasevich, Neil Horman, Gang He, GuoQing Jiang On Wed, May 02, 2018 at 05:46:23PM +0800, Xin Long wrote: > On Wed, May 2, 2018 at 5:06 PM, Michal Kubecek <mkubecek@suse.cz> wrote: > > Hello, > > > > while investigating a bug, we noticed that DLM tries to connect an SCTP > > socket in non-blocking mode using > > > > result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len, > > O_NONBLOCK); > > > > which does not work. The reason is that inet_dgram_connect() cannot pass > > its flags argument to sctp_connect() so that __sctp_connect() which does > > the actual waiting resorts to checking sk->sk_socket->file->f_flags > > instead. As the socket used by DLM is a kernel socket with no associated > > file, it ends up blocking. > > > > TCP doesn't suffer from this problem as for TCP, the waiting is done in > > inet_stream_connect() which has the flags argument. I also checked other > > proto::connect handlers and sctp_connect() seems to be the only one with > > this kind of problem. > > > > This could be worked around in DLM and further experiments indicate > > current DLM code wouldn't actually handle the non-blocking connect > > properly. But I still feel ignoring the flags argument is rather a trap > > that should be fixed. > It is a bug, https://bugzilla.redhat.com/show_bug.cgi?id=1251530 Not authorized. :-) > We have the fix which also includes some cleanup, and needs to do > more testing. OK, I'll wait for your submission. > > I have prepared a series adding flags argument to proto::connect and > > using it in sctp_connect() and __sctp_connect(). But I'm not sure if > > it's not too big hammer to address issue only affecting one handler. > > So my question is: would such generic approach be preferred or should we, > > rather make SCTP work the way TCP does, i.e. move the waiting from, > > proto::connect() to proto_ops::connect()? This would require introducing > > inet_seqpacket_connect() as inet_dgram_connect() is primarily intended > > for use with UDP.) > We don't fix it in the generic proto::connect, which will afftect > many other places. That was my concern, too. On the other hand, the TCP specific waiting code in inet_stream_connect() makes me wonder if it wouldn't be cleaner to move it into the TCP specific handler as well (which is something this approach would allow). > We're replacing only sctp's proto_ops::connect with sctp_connect and > leave its proto::connect as NULL, so that it can get this flags param > without touching the generic struct and code. Yes, that should do the trick (and makes backporting to distribution kernels with frozen kABI much easier). I guess I was too fixed on the split between proto_ops::connect and proto::connect to see this solution. Michal Kubecek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: non-blocking connect for kernel SCTP sockets @ 2018-05-02 12:32 ` Michal Kubecek 0 siblings, 0 replies; 8+ messages in thread From: Michal Kubecek @ 2018-05-02 12:32 UTC (permalink / raw) To: Xin Long Cc: network dev, linux-sctp, LKML, Vlad Yasevich, Neil Horman, Gang He, GuoQing Jiang On Wed, May 02, 2018 at 05:46:23PM +0800, Xin Long wrote: > On Wed, May 2, 2018 at 5:06 PM, Michal Kubecek <mkubecek@suse.cz> wrote: > > Hello, > > > > while investigating a bug, we noticed that DLM tries to connect an SCTP > > socket in non-blocking mode using > > > > result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len, > > O_NONBLOCK); > > > > which does not work. The reason is that inet_dgram_connect() cannot pass > > its flags argument to sctp_connect() so that __sctp_connect() which does > > the actual waiting resorts to checking sk->sk_socket->file->f_flags > > instead. As the socket used by DLM is a kernel socket with no associated > > file, it ends up blocking. > > > > TCP doesn't suffer from this problem as for TCP, the waiting is done in > > inet_stream_connect() which has the flags argument. I also checked other > > proto::connect handlers and sctp_connect() seems to be the only one with > > this kind of problem. > > > > This could be worked around in DLM and further experiments indicate > > current DLM code wouldn't actually handle the non-blocking connect > > properly. But I still feel ignoring the flags argument is rather a trap > > that should be fixed. > It is a bug, https://bugzilla.redhat.com/show_bug.cgi?id\x1251530 Not authorized. :-) > We have the fix which also includes some cleanup, and needs to do > more testing. OK, I'll wait for your submission. > > I have prepared a series adding flags argument to proto::connect and > > using it in sctp_connect() and __sctp_connect(). But I'm not sure if > > it's not too big hammer to address issue only affecting one handler. > > So my question is: would such generic approach be preferred or should we, > > rather make SCTP work the way TCP does, i.e. move the waiting from, > > proto::connect() to proto_ops::connect()? This would require introducing > > inet_seqpacket_connect() as inet_dgram_connect() is primarily intended > > for use with UDP.) > We don't fix it in the generic proto::connect, which will afftect > many other places. That was my concern, too. On the other hand, the TCP specific waiting code in inet_stream_connect() makes me wonder if it wouldn't be cleaner to move it into the TCP specific handler as well (which is something this approach would allow). > We're replacing only sctp's proto_ops::connect with sctp_connect and > leave its proto::connect as NULL, so that it can get this flags param > without touching the generic struct and code. Yes, that should do the trick (and makes backporting to distribution kernels with frozen kABI much easier). I guess I was too fixed on the split between proto_ops::connect and proto::connect to see this solution. Michal Kubecek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: non-blocking connect for kernel SCTP sockets 2018-05-02 12:32 ` Michal Kubecek @ 2018-05-02 13:36 ` Marcelo Ricardo Leitner -1 siblings, 0 replies; 8+ messages in thread From: Marcelo Ricardo Leitner @ 2018-05-02 13:36 UTC (permalink / raw) To: Michal Kubecek Cc: Xin Long, network dev, linux-sctp, LKML, Vlad Yasevich, Neil Horman, Gang He, GuoQing Jiang On Wed, May 02, 2018 at 02:32:28PM +0200, Michal Kubecek wrote: > On Wed, May 02, 2018 at 05:46:23PM +0800, Xin Long wrote: ... > > It is a bug, https://bugzilla.redhat.com/show_bug.cgi?id=1251530 > > Not authorized. :-) Oups! I just made it public. Marcelo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: non-blocking connect for kernel SCTP sockets @ 2018-05-02 13:36 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 8+ messages in thread From: Marcelo Ricardo Leitner @ 2018-05-02 13:36 UTC (permalink / raw) To: Michal Kubecek Cc: Xin Long, network dev, linux-sctp, LKML, Vlad Yasevich, Neil Horman, Gang He, GuoQing Jiang On Wed, May 02, 2018 at 02:32:28PM +0200, Michal Kubecek wrote: > On Wed, May 02, 2018 at 05:46:23PM +0800, Xin Long wrote: ... > > It is a bug, https://bugzilla.redhat.com/show_bug.cgi?id\x1251530 > > Not authorized. :-) Oups! I just made it public. Marcelo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-02 13:36 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-02 9:06 non-blocking connect for kernel SCTP sockets Michal Kubecek 2018-05-02 9:06 ` Michal Kubecek 2018-05-02 9:46 ` Xin Long 2018-05-02 9:46 ` Xin Long 2018-05-02 12:32 ` Michal Kubecek 2018-05-02 12:32 ` Michal Kubecek 2018-05-02 13:36 ` Marcelo Ricardo Leitner 2018-05-02 13:36 ` Marcelo Ricardo Leitner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.