[PATCH net-next] sctp: support MSG_ERRQUEUE flag in recvmsg()

[PATCH net-next] sctp: support MSG_ERRQUEUE flag in recvmsg()

[Eric Dumazet]

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(+)

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

Re: [PATCH net-next] sctp: support MSG_ERRQUEUE flag in recvmsg()

[Xin Long]

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
>

Re: [PATCH net-next] sctp: support MSG_ERRQUEUE flag in recvmsg()

[Eric Dumazet]

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

Re: [PATCH net-next] sctp: support MSG_ERRQUEUE flag in recvmsg()

[Xin Long]

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>

Re: [PATCH net-next] sctp: support MSG_ERRQUEUE flag in recvmsg()

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html