* 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.