| Message ID | 20231212145550.3872051-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4746b36b1abe11ca32987b2d21e1e770deab17cc |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] sctp: support MSG_ERRQUEUE flag in recvmsg() | expand |
On Tue, Dec 12, 2023 at 9:55 AM Eric Dumazet <edumazet@google.com> wrote: > > For some reason sctp_poll() generates EPOLLERR if sk->sk_error_queue > is not empty but recvmsg() can not drain the error queue yet. I'm checking the code but can't see how SCTP may enqueue skbs into sk->sk_error_queue. Have you ever seen it happen in any of your cases? > > This is needed to better support timestamping. I think SCTP doesn't support timestamping, there's no functions like tcp_tx_timestamp()/tcp_gso_tstamp() to enable it. Or do you mean SO_TXTIME socket option, and then tc-etf may enqueue a skb into sk->sk_error_queue if it's enabled? > > I had to export inet_recv_error(), since sctp > can be compiled as a module. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Xin Long <lucien.xin@gmail.com> > Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Cc: Willem de Bruijn <willemb@google.com> > --- > net/ipv4/af_inet.c | 1 + > net/sctp/socket.c | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > index fbeacf04dbf3744e5888360e0b74bf6f70ff214f..835f4f9d98d25559fb8965a7531c6863448a55c2 100644 > --- a/net/ipv4/af_inet.c > +++ b/net/ipv4/af_inet.c > @@ -1633,6 +1633,7 @@ int inet_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len) > #endif > return -EINVAL; > } > +EXPORT_SYMBOL(inet_recv_error); > > int inet_gro_complete(struct sk_buff *skb, int nhoff) > { > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 7f89e43154c091f6f7a3c995c1ba8abb62a8e767..5fb02bbb4b349ef9ab9c2790cccb30fb4c4e897c 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -2099,6 +2099,9 @@ static int sctp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > pr_debug("%s: sk:%p, msghdr:%p, len:%zd, flags:0x%x, addr_len:%p)\n", > __func__, sk, msg, len, flags, addr_len); > > + if (unlikely(flags & MSG_ERRQUEUE)) > + return inet_recv_error(sk, msg, len, addr_len); > + > lock_sock(sk); > > if (sctp_style(sk, TCP) && !sctp_sstate(sk, ESTABLISHED) && > -- > 2.43.0.472.g3155946c3a-goog >
On Wed, Dec 13, 2023 at 4:19 PM Xin Long <lucien.xin@gmail.com> wrote: > > On Tue, Dec 12, 2023 at 9:55 AM Eric Dumazet <edumazet@google.com> wrote: > > > > For some reason sctp_poll() generates EPOLLERR if sk->sk_error_queue > > is not empty but recvmsg() can not drain the error queue yet. > I'm checking the code but can't see how SCTP may enqueue skbs into > sk->sk_error_queue. Have you ever seen it happen in any of your cases? > > > > > This is needed to better support timestamping. > I think SCTP doesn't support timestamping, there's no functions like > tcp_tx_timestamp()/tcp_gso_tstamp() to enable it. > > Or do you mean SO_TXTIME socket option, and then tc-etf may > enqueue a skb into sk->sk_error_queue if it's enabled? > I think this is the goal yes. Also this prevents developers from using MSG_ERRQUEUE and having the kernel read the receive queue. mptcp had a similar issue in the past, Paolo fixed it. > > > > I had to export inet_recv_error(), since sctp > > can be compiled as a module. > > > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > Cc: Xin Long <lucien.xin@gmail.com> > > Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > Cc: Willem de Bruijn <willemb@google.com> > > --- > > net/ipv4/af_inet.c | 1 + > > net/sctp/socket.c | 3 +++ > > 2 files changed, 4 insertions(+) > > > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > > index fbeacf04dbf3744e5888360e0b74bf6f70ff214f..835f4f9d98d25559fb8965a7531c6863448a55c2 100644 > > --- a/net/ipv4/af_inet.c > > +++ b/net/ipv4/af_inet.c > > @@ -1633,6 +1633,7 @@ int inet_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len) > > #endif > > return -EINVAL; > > } > > +EXPORT_SYMBOL(inet_recv_error); > > > > int inet_gro_complete(struct sk_buff *skb, int nhoff) > > { > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > > index 7f89e43154c091f6f7a3c995c1ba8abb62a8e767..5fb02bbb4b349ef9ab9c2790cccb30fb4c4e897c 100644 > > --- a/net/sctp/socket.c > > +++ b/net/sctp/socket.c > > @@ -2099,6 +2099,9 @@ static int sctp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > > pr_debug("%s: sk:%p, msghdr:%p, len:%zd, flags:0x%x, addr_len:%p)\n", > > __func__, sk, msg, len, flags, addr_len); > > > > + if (unlikely(flags & MSG_ERRQUEUE)) > > + return inet_recv_error(sk, msg, len, addr_len); > > + > > lock_sock(sk); > > > > if (sctp_style(sk, TCP) && !sctp_sstate(sk, ESTABLISHED) && > > -- > > 2.43.0.472.g3155946c3a-goog > >
On Wed, Dec 13, 2023 at 10:50 AM Eric Dumazet <edumazet@google.com> wrote: > > On Wed, Dec 13, 2023 at 4:19 PM Xin Long <lucien.xin@gmail.com> wrote: > > > > On Tue, Dec 12, 2023 at 9:55 AM Eric Dumazet <edumazet@google.com> wrote: > > > > > > For some reason sctp_poll() generates EPOLLERR if sk->sk_error_queue > > > is not empty but recvmsg() can not drain the error queue yet. > > I'm checking the code but can't see how SCTP may enqueue skbs into > > sk->sk_error_queue. Have you ever seen it happen in any of your cases? > > > > > > > > This is needed to better support timestamping. > > I think SCTP doesn't support timestamping, there's no functions like > > tcp_tx_timestamp()/tcp_gso_tstamp() to enable it. > > > > Or do you mean SO_TXTIME socket option, and then tc-etf may > > enqueue a skb into sk->sk_error_queue if it's enabled? > > > > I think this is the goal yes. > > Also this prevents developers from using MSG_ERRQUEUE and having > the kernel read the receive queue. > > mptcp had a similar issue in the past, Paolo fixed it. > > > > > > > I had to export inet_recv_error(), since sctp > > > can be compiled as a module. > > > > > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > > Cc: Xin Long <lucien.xin@gmail.com> > > > Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > > Cc: Willem de Bruijn <willemb@google.com> > > > --- > > > net/ipv4/af_inet.c | 1 + > > > net/sctp/socket.c | 3 +++ > > > 2 files changed, 4 insertions(+) > > > > > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > > > index fbeacf04dbf3744e5888360e0b74bf6f70ff214f..835f4f9d98d25559fb8965a7531c6863448a55c2 100644 > > > --- a/net/ipv4/af_inet.c > > > +++ b/net/ipv4/af_inet.c > > > @@ -1633,6 +1633,7 @@ int inet_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len) > > > #endif > > > return -EINVAL; > > > } > > > +EXPORT_SYMBOL(inet_recv_error); > > > > > > int inet_gro_complete(struct sk_buff *skb, int nhoff) > > > { > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > > > index 7f89e43154c091f6f7a3c995c1ba8abb62a8e767..5fb02bbb4b349ef9ab9c2790cccb30fb4c4e897c 100644 > > > --- a/net/sctp/socket.c > > > +++ b/net/sctp/socket.c > > > @@ -2099,6 +2099,9 @@ static int sctp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > > > pr_debug("%s: sk:%p, msghdr:%p, len:%zd, flags:0x%x, addr_len:%p)\n", > > > __func__, sk, msg, len, flags, addr_len); > > > > > > + if (unlikely(flags & MSG_ERRQUEUE)) > > > + return inet_recv_error(sk, msg, len, addr_len); > > > + > > > lock_sock(sk); > > > > > > if (sctp_style(sk, TCP) && !sctp_sstate(sk, ESTABLISHED) && > > > -- > > > 2.43.0.472.g3155946c3a-goog > > > Acked-by: Xin Long <lucien.xin@gmail.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 12 Dec 2023 14:55:50 +0000 you wrote: > For some reason sctp_poll() generates EPOLLERR if sk->sk_error_queue > is not empty but recvmsg() can not drain the error queue yet. > > This is needed to better support timestamping. > > I had to export inet_recv_error(), since sctp > can be compiled as a module. > > [...] Here is the summary with links: - [net-next] sctp: support MSG_ERRQUEUE flag in recvmsg() https://git.kernel.org/netdev/net-next/c/4746b36b1abe You are awesome, thank you!
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index fbeacf04dbf3744e5888360e0b74bf6f70ff214f..835f4f9d98d25559fb8965a7531c6863448a55c2 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1633,6 +1633,7 @@ int inet_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len) #endif return -EINVAL; } +EXPORT_SYMBOL(inet_recv_error); int inet_gro_complete(struct sk_buff *skb, int nhoff) { diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 7f89e43154c091f6f7a3c995c1ba8abb62a8e767..5fb02bbb4b349ef9ab9c2790cccb30fb4c4e897c 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -2099,6 +2099,9 @@ static int sctp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, pr_debug("%s: sk:%p, msghdr:%p, len:%zd, flags:0x%x, addr_len:%p)\n", __func__, sk, msg, len, flags, addr_len); + if (unlikely(flags & MSG_ERRQUEUE)) + return inet_recv_error(sk, msg, len, addr_len); + lock_sock(sk); if (sctp_style(sk, TCP) && !sctp_sstate(sk, ESTABLISHED) &&
For some reason sctp_poll() generates EPOLLERR if sk->sk_error_queue is not empty but recvmsg() can not drain the error queue yet. This is needed to better support timestamping. I had to export inet_recv_error(), since sctp can be compiled as a module. Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Xin Long <lucien.xin@gmail.com> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Cc: Willem de Bruijn <willemb@google.com> --- net/ipv4/af_inet.c | 1 + net/sctp/socket.c | 3 +++ 2 files changed, 4 insertions(+)