* Re: [PATCH 0/6] tcp: support preloading data on a listening socket
From: Jeremy Harris @ 2025-05-16 20:10 UTC (permalink / raw)
To: Neal Cardwell; +Cc: netdev, linux-api, edumazet
In-Reply-To: <CADVnQymxsOGLnUfurhDLXNUaK4gpaYm2zTDEWRxy8JPqH6O6vg@mail.gmail.com>
Hi Neal,
Thanks for the initial review.
On 2025/05/16 7:19 PM, Neal Cardwell wrote:
> On Fri, May 16, 2025 at 11:55 AM Jeremy Harris <jgh@exim.org> wrote:
>>
>> Support write to a listen TCP socket, for immediate
>> transmission on passive connection establishments.
>>
>> On a normal connection transmission is triggered by the receipt of
>> the 3rd-ack. On a fastopen (with accepted cookie) connection the data
>> is sent in the synack packet.
>>
>> The data preload is done using a sendmsg with a newly-defined flag
>> (MSG_PRELOAD); the amount of data limited to a single linear sk_buff.
>> Note that this definition is the last-but-two bit available if "int"
>> is 32 bits.
>
> Can you please add a bit more context, like:
>
> + What is the motivating use case? (Accelerating Exim?)
Accelerating any server-first ULP, SMTP being the major use I
know of (and yes, Exim is my primary testcase and is operational
against a test kernel with this patch series).
One caveat: the initial server data cannot change from one passive
connection to another.
> Is this
> targeted for connections using encryption (like TLS/SSL), or just
> plain-text connections?
TLS-on-connect cannot benefit, being client-first. SMTP that uses
STARTTLS can take advantage of it, as can plaintext SMTP.
I would not expect https to be able to use it.
> + What are the exact performance improvements you are seeing in your
> benchmarks that (a) motivate this, and (b) justify any performance
> impact on the TCP stack?
Because of the lack of userland roundtrip needed for the initial server
data, there is a latency benefit. This is better for the TFO-C case,
but also significant for the non-TFO case.
Packet capture (laptop, loopback, TFO-C case) for initial SYN to first
client data packet (5 samples):
- baseline TFO_C 1064 1470 1455 1547 1595 usec
- patched non-TFO 140 150 159 144 153 usec
- patched TFO_C 142 149 149 125 125 usec
One fewer packet is sent by the server in most packet captures, sometimes
one fewer in each direction. There is one less application kernel entry/exit
on the server.
I'm hoping those differences will add up to both less cpu time (on both
endpoints) and less wire-time. However, I have not run benchmarks looking
for a change in peak rate of connection-handling.
In summary, this is the mirror of TCP Fast Open client data: the latency
benefit is probably the most useful aspect.
> + Regarding "Support write to a listen TCP socket, for immediate
> transmission on passive connection establishments.": can you please
> make it explicitly clear whether the data written to the listening
> socket is saved and transmitted on all future successful passive
> sockets that are created for the listener,
This. The data is copied for each future passive socket from this
listener,
> or is just transmitted on
> the next connection that is created?
(and not this option).
I'll copy these comments in any future v2.
As Eric says, I should run KASAN/syzbot first.
--
Cheers,
Jeremy
^ permalink raw reply
* Re: close(2) with EINTR has been changed by POSIX.1-2024
From: Al Viro @ 2025-05-16 19:13 UTC (permalink / raw)
To: Jan Kara
Cc: Alejandro Colomar, Christian Brauner, linux-fsdevel, linux-api,
linux-man
In-Reply-To: <ddqmhjc2rpzk2jjvunbt3l3eukcn4xzkocqzdg3j4msihdhzko@fizekvxndg2d>
On Fri, May 16, 2025 at 12:48:56PM +0200, Jan Kara wrote:
> I'm not aware of any discussions but indeed we are returning EINTR while
> closing the fd. Frankly, changing the error code we return in that case is
> really asking for userspace regressions so I'm of the opinion we just
> ignore the standard as in my opinion it goes against a long established
> reality.
AFAICS what happens is that relevance of Austin Group has dropped so low
that they stopped caring about any BS filters they used to have. What
we are seeing now is assorted pet idiocies that used to sit in their
system, periodically getting shot down while there had been anyone who
cared to do that.
Sad, of course, but what can we do, other than politely ignoring the... output?
^ permalink raw reply
* Re: [PATCH 0/6] tcp: support preloading data on a listening socket
From: Neal Cardwell @ 2025-05-16 18:19 UTC (permalink / raw)
To: Jeremy Harris; +Cc: netdev, linux-api, edumazet
In-Reply-To: <cover.1747409911.git.jgh@exim.org>
On Fri, May 16, 2025 at 11:55 AM Jeremy Harris <jgh@exim.org> wrote:
>
> Support write to a listen TCP socket, for immediate
> transmission on passive connection establishments.
>
> On a normal connection transmission is triggered by the receipt of
> the 3rd-ack. On a fastopen (with accepted cookie) connection the data
> is sent in the synack packet.
>
> The data preload is done using a sendmsg with a newly-defined flag
> (MSG_PRELOAD); the amount of data limited to a single linear sk_buff.
> Note that this definition is the last-but-two bit available if "int"
> is 32 bits.
Can you please add a bit more context, like:
+ What is the motivating use case? (Accelerating Exim?) Is this
targeted for connections using encryption (like TLS/SSL), or just
plain-text connections?
+ What are the exact performance improvements you are seeing in your
benchmarks that (a) motivate this, and (b) justify any performance
impact on the TCP stack?
+ Regarding "Support write to a listen TCP socket, for immediate
transmission on passive connection establishments.": can you please
make it explicitly clear whether the data written to the listening
socket is saved and transmitted on all future successful passive
sockets that are created for the listener, or is just transmitted on
the next connection that is created?
thanks,
neal
^ permalink raw reply
* Re: [PATCH 2/6] tcp: copy write-data from listen socket to accept child socket
From: Eric Dumazet @ 2025-05-16 17:51 UTC (permalink / raw)
To: Jeremy Harris; +Cc: netdev, linux-api, ncardwell
In-Reply-To: <702375d7dcc673fa1f97c92ddf86b47d11106db4.1747409911.git.jgh@exim.org>
On Fri, May 16, 2025 at 8:56 AM Jeremy Harris <jgh@exim.org> wrote:
>
> Set the request_sock flag for fastopen earlier, making it available
> to the af_ops SYN-handler function.
>
> In that function copy data from the listen socket write queue into an
> sk_buff, allocating if needed and adding to the write queue of the
> newly-created child socket.
> Set sequence number values depending on the fastopen status.
I do not see any locking. I think you should run a local KASAN/syzbot
instance and you will be shocked.
Honestly we need to be convinced of why adding code in sendmsg() fast
path is worth this.
^ permalink raw reply
* Re: [PATCH 0/6] tcp: support preloading data on a listening socket
From: Jeremy Harris @ 2025-05-16 16:58 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell
In-Reply-To: <cover.1747409911.git.jgh@exim.org>
On 2025/05/16 4:54 PM, Jeremy Harris wrote:
> Support write to a listen TCP socket, for immediate
> transmission on passive connection establishments.
This series should have included "net-next" in their Subjects.
--
Apologies,
Jeremy
^ permalink raw reply
* [PATCH 3/6] tcp: fastopen: add write-data to fastopen synack packet
From: Jeremy Harris @ 2025-05-16 15:55 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747409911.git.jgh@exim.org>
While building the synack packet, for a fastopen socket
copy data from write queue to the packet.
Move the data from write queue to retransmit queue.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_output.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 3ac8d2d17e1f..c50553c1c795 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3702,7 +3702,7 @@ int tcp_send_synack(struct sock *sk)
/**
* tcp_make_synack - Allocate one skb and build a SYNACK packet.
- * @sk: listener socket
+ * @sk: listener socket (or child socket for fastopen)
* @dst: dst entry attached to the SYNACK. It is consumed and caller
* should not use it again.
* @req: request_sock pointer
@@ -3719,6 +3719,7 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
struct inet_request_sock *ireq = inet_rsk(req);
const struct tcp_sock *tp = tcp_sk(sk);
struct tcp_out_options opts;
+ struct sock *fastopen_sk = (struct sock *)sk;
struct tcp_key key = {};
struct sk_buff *skb;
int tcp_header_size;
@@ -3748,7 +3749,7 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
* cpu might call us concurrently.
* sk->sk_wmem_alloc in an atomic, we can promote to rw.
*/
- skb_set_owner_w(skb, (struct sock *)sk);
+ skb_set_owner_w(skb, fastopen_sk);
break;
}
skb_dst_set(skb, dst);
@@ -3831,6 +3832,33 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
th->window = htons(min(req->rsk_rcv_wnd, 65535U));
tcp_options_write(th, NULL, tcp_rsk(req), &opts, &key);
th->doff = (tcp_header_size >> 2);
+
+ /* If this is a FASTOPEN, and there is write-data on the accept socket,
+ * re-copy it to the synack segment. If not FASTOPEN. any data waits
+ * until 3rd-ack arrival.
+ */
+
+ if (synack_type == TCP_SYNACK_FASTOPEN &&
+ !skb_queue_empty(&sk->sk_write_queue)) {
+ struct sk_buff *a_skb = tcp_write_queue_tail(sk);
+ int copy = min_t(int, a_skb->len, skb_tailroom(skb));
+
+ skb_put_data(skb, a_skb->data, copy);
+ TCP_SKB_CB(skb)->end_seq += copy;
+
+ tcp_skb_pcount_set(a_skb, 1);
+ WRITE_ONCE(tcp_sk(fastopen_sk)->write_seq,
+ TCP_SKB_CB(a_skb)->end_seq);
+
+ skb_set_delivery_time(a_skb, now, SKB_CLOCK_MONOTONIC);
+
+ /* Move the data to the retransmit queue.
+ * Code elsewhere implies this is a full child socket and
+ * can be treated as writeable - permitting the cast.
+ */
+ tcp_event_new_data_sent(fastopen_sk, a_skb);
+ }
+
TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTSEGS);
/* Okay, we have all we need - do the md5 hash if needed */
--
2.49.0
^ permalink raw reply related
* [PATCH 6/6] tcp: fastopen: extend retransmit-queue trimming to handle linear sk_buff
From: Jeremy Harris @ 2025-05-16 15:55 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747409911.git.jgh@exim.org>
A corner-case for the 3rd-ack after a data-on-synack is for
some but not all of the data to be acked. Support this by
adding to the retransmit-queue trim routine to handle a
linear sk_buff.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_output.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index c50553c1c795..bff5934ff04b 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1708,8 +1708,22 @@ static int __pskb_trim_head(struct sk_buff *skb, int len)
struct skb_shared_info *shinfo;
int i, k, eat;
- DEBUG_NET_WARN_ON_ONCE(skb_headlen(skb));
- eat = len;
+ eat = skb_headlen(skb);
+ if (unlikely(eat)) {
+ if (len < eat)
+ eat = len;
+ skb->head += eat;
+ skb->len -= eat;
+ if (skb->data_len)
+ skb->data_len -= eat;
+
+ eat = len - eat;
+ if (eat == 0)
+ return len;
+ } else {
+ eat = len;
+ }
+
k = 0;
shinfo = skb_shinfo(skb);
for (i = 0; i < shinfo->nr_frags; i++) {
--
2.49.0
^ permalink raw reply related
* [PATCH 4/6] tcp: transmit any pending data on receipt of 3rd-ack
From: Jeremy Harris @ 2025-05-16 15:55 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747409911.git.jgh@exim.org>
For the non-fastopen case of prelaod, when the 3rd-ack arrives there
will be data on the write queue. Transmit it immediately
by allowing the SYN_SENT state to run the xmit-recovery code.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_input.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8ec92dec321a..345a08baaf02 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3900,7 +3900,8 @@ static void tcp_xmit_recovery(struct sock *sk, int rexmit)
{
struct tcp_sock *tp = tcp_sk(sk);
- if (rexmit == REXMIT_NONE || sk->sk_state == TCP_SYN_SENT)
+ if ((rexmit == REXMIT_NONE && sk->sk_state != TCP_SYN_RECV) ||
+ sk->sk_state == TCP_SYN_SENT)
return;
if (unlikely(rexmit == REXMIT_NEW)) {
--
2.49.0
^ permalink raw reply related
* [PATCH 5/6] tcp: fastopen: retransmit data when only the SYN of a synack-with-data is acked
From: Jeremy Harris @ 2025-05-16 15:55 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747409911.git.jgh@exim.org>
A corner-case for the 3rd-ack after a data-on-synack is for only
the SYN to be acked. Handle this by, in ack processing, when in
SYN_RECV state (the state is not yet updated to ESTABLISHED)
marking the retransmit-queue sk_buff as having been lost.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_input.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 345a08baaf02..a53021edddd5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4069,6 +4069,18 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
&rexmit);
}
+ /* On receiving a 3rd-ack, if we never sent a packet via
+ * the normal means (which counts them), yet there is data
+ * remaining for retransmit, it was data-on-synack not acked;
+ * mark the skb for retransmission.
+ */
+ if (sk->sk_state == TCP_SYN_RECV && tp->segs_out == 0) {
+ struct sk_buff *skb = tcp_rtx_queue_head(sk);
+
+ if (skb)
+ tcp_mark_skb_lost(sk, skb);
+ }
+
/* If needed, reset TLP/RTO timer when RACK doesn't set. */
if (flag & FLAG_SET_XMIT_TIMER)
tcp_set_xmit_timer(sk);
--
2.49.0
^ permalink raw reply related
* [PATCH 2/6] tcp: copy write-data from listen socket to accept child socket
From: Jeremy Harris @ 2025-05-16 15:55 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747409911.git.jgh@exim.org>
Set the request_sock flag for fastopen earlier, making it available
to the af_ops SYN-handler function.
In that function copy data from the listen socket write queue into an
sk_buff, allocating if needed and adding to the write queue of the
newly-created child socket.
Set sequence number values depending on the fastopen status.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_fastopen.c | 3 ++-
net/ipv4/tcp_ipv4.c | 4 +--
net/ipv4/tcp_minisocks.c | 58 ++++++++++++++++++++++++++++++++++++----
3 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 9b83d639b5ac..03a86d0b87ba 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -245,6 +245,8 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
struct sock *child;
bool own_req;
+ tcp_rsk(req)->tfo_listener = true;
+
child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
NULL, &own_req);
if (!child)
@@ -261,7 +263,6 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
tp = tcp_sk(child);
rcu_assign_pointer(tp->fastopen_rsk, req);
- tcp_rsk(req)->tfo_listener = true;
/* RFC1323: The window in SYN & SYN/ACK segments is never
* scaled. So correct it appropriately.
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 6a14f9e6fef6..e488effdbdb2 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1747,8 +1747,8 @@ EXPORT_IPV6_MOD(tcp_v4_conn_request);
/*
- * The three way handshake has completed - we got a valid synack -
- * now create the new socket.
+ * The three way handshake has completed - we got a valid synack
+ * (or a FASTOPEN syn) - now create the new socket.
*/
struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 43d7852ce07e..d471531b4a78 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -529,7 +529,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
struct inet_connection_sock *newicsk;
const struct tcp_sock *oldtp;
struct tcp_sock *newtp;
- u32 seq;
+ u32 seq, a_seq, n_seq;
if (!newsk)
return NULL;
@@ -550,9 +550,55 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
newtp->segs_in = 1;
seq = treq->snt_isn + 1;
- newtp->snd_sml = newtp->snd_una = seq;
- WRITE_ONCE(newtp->snd_nxt, seq);
- newtp->snd_up = seq;
+ n_seq = seq;
+ a_seq = seq;
+ newtp->write_seq = seq;
+ newtp->snd_una = seq;
+
+ /* If there is write-data sitting on the listen socket, copy it to
+ * the accept socket. If FASTOPEN we will send it on the synack,
+ * otherwise it sits there until 3rd-ack arrives.
+ */
+
+ if (unlikely(!skb_queue_empty(&sk->sk_write_queue))) {
+ struct sk_buff *l_skb = tcp_send_head(sk),
+ *a_skb = tcp_write_queue_tail(newsk);
+ ssize_t copy = 0;
+
+ if (a_skb)
+ copy = l_skb->len - a_skb->len;
+
+ if (copy <= 0 || !tcp_skb_can_collapse_to(a_skb)) {
+ bool first_skb = tcp_rtx_and_write_queues_empty(newsk);
+
+ a_skb = tcp_stream_alloc_skb(newsk,
+ newsk->sk_allocation,
+ first_skb);
+ if (!a_skb) {
+ /* is this the correct free? */
+ bh_unlock_sock(newsk);
+ sk_free(newsk);
+ return NULL;
+ }
+
+ tcp_skb_entail(newsk, a_skb);
+ }
+ copy = min_t(int, l_skb->len, skb_tailroom(a_skb));
+ skb_put_data(a_skb, l_skb->data, copy);
+
+ TCP_SKB_CB(a_skb)->end_seq += copy;
+
+ a_seq += l_skb->len;
+
+ if (treq->tfo_listener)
+ seq = a_seq;
+
+ /* assumes only one skb on the listen write queue */
+ }
+
+ newtp->snd_sml = seq;
+ WRITE_ONCE(newtp->snd_nxt, a_seq);
+ newtp->snd_up = n_seq;
INIT_LIST_HEAD(&newtp->tsq_node);
INIT_LIST_HEAD(&newtp->tsorted_sent_queue);
@@ -567,7 +613,9 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
newtp->total_retrans = req->num_retrans;
tcp_init_xmit_timers(newsk);
- WRITE_ONCE(newtp->write_seq, newtp->pushed_seq = treq->snt_isn + 1);
+
+ newtp->pushed_seq = n_seq;
+ WRITE_ONCE(newtp->write_seq, a_seq);
if (sock_flag(newsk, SOCK_KEEPOPEN))
tcp_reset_keepalive_timer(newsk, keepalive_time_when(newtp));
--
2.49.0
^ permalink raw reply related
* [PATCH 1/6] tcp: support writing to a socket in listening state
From: Jeremy Harris @ 2025-05-16 15:54 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747409911.git.jgh@exim.org>
In the tcp sendmsg handler, permit a write in LISTENING state if
a MSG_PRELOAD flag is used. Copy from iovec to a linear sk_buff
for placement on the socket write queue.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
include/linux/socket.h | 1 +
net/ipv4/tcp.c | 17 +++++++++++++----
tools/perf/trace/beauty/include/linux/socket.h | 1 +
tools/perf/trace/beauty/msg_flags.c | 3 +++
4 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 3b262487ec06..b41f4cd4dc97 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -330,6 +330,7 @@ struct ucred {
#define MSG_SOCK_DEVMEM 0x2000000 /* Receive devmem skbs as cmsg */
#define MSG_ZEROCOPY 0x4000000 /* Use user data in kernel path */
#define MSG_SPLICE_PAGES 0x8000000 /* Splice the pages from the iterator in sendmsg() */
+#define MSG_PRELOAD 0x10000000 /* Preload tx data while listening */
#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
#define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file
descriptor received through
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b7b6ab41b496..72b5d7cad351 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1136,12 +1136,13 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
tcp_rate_check_app_limited(sk); /* is sending application-limited? */
- /* Wait for a connection to finish. One exception is TCP Fast Open
+ /* Wait for a connection to finish. Exceptions are TCP Fast Open
* (passive side) where data is allowed to be sent before a connection
- * is fully established.
+ * is fully established, and a message marked as preload which is
+ * allowed to be placed in the send queue of a listening socket.
*/
if (((1 << sk->sk_state) & ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)) &&
- !tcp_passive_fastopen(sk)) {
+ !tcp_passive_fastopen(sk) && !(flags & MSG_PRELOAD)) {
err = sk_stream_wait_connect(sk, &timeo);
if (err != 0)
goto do_error;
@@ -1226,7 +1227,13 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
if (copy > msg_data_left(msg))
copy = msg_data_left(msg);
- if (zc == 0) {
+ if (unlikely(flags & MSG_PRELOAD)) {
+ copy = min_t(int, copy, skb_tailroom(skb));
+ err = skb_add_data_nocache(sk, skb, &msg->msg_iter,
+ copy);
+ if (err)
+ goto do_error;
+ } else if (zc == 0) {
bool merge = true;
int i = skb_shinfo(skb)->nr_frags;
struct page_frag *pfrag = sk_page_frag(sk);
@@ -1330,6 +1337,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
if (!msg_data_left(msg)) {
if (unlikely(flags & MSG_EOR))
TCP_SKB_CB(skb)->eor = 1;
+ if (unlikely(flags & MSG_PRELOAD))
+ goto out_nopush;
goto out;
}
diff --git a/tools/perf/trace/beauty/include/linux/socket.h b/tools/perf/trace/beauty/include/linux/socket.h
index c3322eb3d686..e9ea498169f3 100644
--- a/tools/perf/trace/beauty/include/linux/socket.h
+++ b/tools/perf/trace/beauty/include/linux/socket.h
@@ -330,6 +330,7 @@ struct ucred {
#define MSG_SOCK_DEVMEM 0x2000000 /* Receive devmem skbs as cmsg */
#define MSG_ZEROCOPY 0x4000000 /* Use user data in kernel path */
#define MSG_SPLICE_PAGES 0x8000000 /* Splice the pages from the iterator in sendmsg() */
+#define MSG_PRELOAD 0x10000000 /* Preload tx data while listening */
#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
#define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file
descriptor received through
diff --git a/tools/perf/trace/beauty/msg_flags.c b/tools/perf/trace/beauty/msg_flags.c
index 2da581ff0c80..27e40da9b02d 100644
--- a/tools/perf/trace/beauty/msg_flags.c
+++ b/tools/perf/trace/beauty/msg_flags.c
@@ -20,6 +20,9 @@
#ifndef MSG_SPLICE_PAGES
#define MSG_SPLICE_PAGES 0x8000000
#endif
+#ifndef MSG_PRELOAD
+#define MSG_PRELOAD 0x10000000
+#endif
#ifndef MSG_FASTOPEN
#define MSG_FASTOPEN 0x20000000
#endif
--
2.49.0
^ permalink raw reply related
* [PATCH 0/6] tcp: support preloading data on a listening socket
From: Jeremy Harris @ 2025-05-16 15:54 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
Support write to a listen TCP socket, for immediate
transmission on passive connection establishments.
On a normal connection transmission is triggered by the receipt of
the 3rd-ack. On a fastopen (with accepted cookie) connection the data
is sent in the synack packet.
The data preload is done using a sendmsg with a newly-defined flag
(MSG_PRELOAD); the amount of data limited to a single linear sk_buff.
Note that this definition is the last-but-two bit available if "int"
is 32 bits.
Testing:
A) packetdrill scripts for
- normal non-TFO
- normal TFO
- synack lost
- 3rd-ack acks only the SYN
- 3rd-ack acks partial data
(NB: packetdrill can only check the data size, not actual content)
B) Application use, running the application testsuite
and manual check of specific cases via packet capture
C) Daily-driver laptop use (not expected to trigger the feature;
only regression-test)
Jeremy Harris (6):
tcp: support writing to a socket in listening state
tcp: copy write-data from listen socket to accept child socket
tcp: fastopen: add write-data to fastopen synack packet
tcp: transmit any pending data on receipt of 3rd-ack
tcp: fastopen: retransmit data when only the SYN of a synack-with-data
is acked
tcp: fastopen: extend retransmit-queue trimming to handle linear
sk_buff
include/linux/socket.h | 1 +
net/ipv4/tcp.c | 17 ++++--
net/ipv4/tcp_fastopen.c | 3 +-
net/ipv4/tcp_input.c | 15 ++++-
net/ipv4/tcp_ipv4.c | 4 +-
net/ipv4/tcp_minisocks.c | 58 +++++++++++++++++--
net/ipv4/tcp_output.c | 50 ++++++++++++++--
.../perf/trace/beauty/include/linux/socket.h | 1 +
tools/perf/trace/beauty/msg_flags.c | 3 +
9 files changed, 135 insertions(+), 17 deletions(-)
base-commit: 2da35e4b4df99d3dd29bacf0c054e6988013d4ec
--
2.49.0
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Rich Felker @ 2025-05-16 15:28 UTC (permalink / raw)
To: Vincent Lefevre, Alejandro Colomar, Jan Kara, Alexander Viro,
Christian Brauner, linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <20250516143957.GB5388@qaa.vinc17.org>
On Fri, May 16, 2025 at 04:39:57PM +0200, Vincent Lefevre wrote:
> On 2025-05-16 09:05:47 -0400, Rich Felker wrote:
> > FWIW musl adopted the EINPROGRESS as soon as we were made aware of the
> > issue, and later changed it to returning 0 since applications
> > (particularly, any written prior to this interpretation) are prone to
> > interpret EINPROGRESS as an error condition rather than success and
> > possibly misinterpret it as meaning the fd is still open and valid to
> > pass to close again.
>
> If I understand correctly, this is a poor choice. POSIX.1-2024 says:
>
> ERRORS
> The close() and posix_close() functions shall fail if:
> [...]
> [EINPROGRESS]
> The function was interrupted by a signal and fildes was closed
> but the close operation is continuing asynchronously.
>
> But this does not mean that the asynchronous close operation will
> succeed.
It always succeeds in the way that's important: the file descriptor is
freed and the process no longer has this reference to the open file
description.
What might or might not succeed is:
(1) other ancient legacy behaviors coupled to close(), like rewinding
a tape drive. If the application cares how that behaves, it needs to
be performing an explicit rewind *before* calling close, when it still
has a handle on the open file so that it can respond to exceptional
conditions, not relying on a legacy behavior like "close also rewinds"
that's device-specific and outside the scope of any modern
cross-platform standard.
(2) deferred operations in unsafe async NFS setups. This is a huge
mess with no real reliable solution except "don't configure your NFS
to have unsafe and nonconforming behaviors in the pursuit of
performance".
Rich
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Vincent Lefevre @ 2025-05-16 15:28 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, Alejandro Colomar, Jan Kara, Alexander Viro,
Christian Brauner, linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <87cyc8oben.fsf@oldenburg.str.redhat.com>
On 2025-05-16 16:52:48 +0200, Florian Weimer wrote:
> * Vincent Lefevre:
>
> > On 2025-05-16 09:05:47 -0400, Rich Felker wrote:
> >> FWIW musl adopted the EINPROGRESS as soon as we were made aware of the
> >> issue, and later changed it to returning 0 since applications
> >> (particularly, any written prior to this interpretation) are prone to
> >> interpret EINPROGRESS as an error condition rather than success and
> >> possibly misinterpret it as meaning the fd is still open and valid to
> >> pass to close again.
> >
> > If I understand correctly, this is a poor choice. POSIX.1-2024 says:
> >
> > ERRORS
> > The close() and posix_close() functions shall fail if:
> > [...]
> > [EINPROGRESS]
> > The function was interrupted by a signal and fildes was closed
> > but the close operation is continuing asynchronously.
> >
> > But this does not mean that the asynchronous close operation will
> > succeed.
> >
> > So the application could incorrectly deduce that the close operation
> > was done without any error.
>
> But on Linux, close traditionally has poor error reporting anyway. You
> have to fsync (or equivalent) before calling close if you want error
> checking. On other systems, the fsync is more or less implied by the
> close, leading to rather poor performance.
According to its documentation, fsync is only for storage devices,
while not all file descriptors are associated with storage devices.
So I'm wondering the consequences in the other cases.
--
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / Pascaline project (LIP, ENS-Lyon)
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Florian Weimer @ 2025-05-16 14:52 UTC (permalink / raw)
To: Vincent Lefevre
Cc: Rich Felker, Alejandro Colomar, Jan Kara, Alexander Viro,
Christian Brauner, linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <20250516143957.GB5388@qaa.vinc17.org>
* Vincent Lefevre:
> On 2025-05-16 09:05:47 -0400, Rich Felker wrote:
>> FWIW musl adopted the EINPROGRESS as soon as we were made aware of the
>> issue, and later changed it to returning 0 since applications
>> (particularly, any written prior to this interpretation) are prone to
>> interpret EINPROGRESS as an error condition rather than success and
>> possibly misinterpret it as meaning the fd is still open and valid to
>> pass to close again.
>
> If I understand correctly, this is a poor choice. POSIX.1-2024 says:
>
> ERRORS
> The close() and posix_close() functions shall fail if:
> [...]
> [EINPROGRESS]
> The function was interrupted by a signal and fildes was closed
> but the close operation is continuing asynchronously.
>
> But this does not mean that the asynchronous close operation will
> succeed.
>
> So the application could incorrectly deduce that the close operation
> was done without any error.
But on Linux, close traditionally has poor error reporting anyway. You
have to fsync (or equivalent) before calling close if you want error
checking. On other systems, the fsync is more or less implied by the
close, leading to rather poor performance.
Thanks,
Florian
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Vincent Lefevre @ 2025-05-16 14:39 UTC (permalink / raw)
To: Rich Felker
Cc: Alejandro Colomar, Jan Kara, Alexander Viro, Christian Brauner,
linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <20250516130547.GV1509@brightrain.aerifal.cx>
On 2025-05-16 09:05:47 -0400, Rich Felker wrote:
> FWIW musl adopted the EINPROGRESS as soon as we were made aware of the
> issue, and later changed it to returning 0 since applications
> (particularly, any written prior to this interpretation) are prone to
> interpret EINPROGRESS as an error condition rather than success and
> possibly misinterpret it as meaning the fd is still open and valid to
> pass to close again.
If I understand correctly, this is a poor choice. POSIX.1-2024 says:
ERRORS
The close() and posix_close() functions shall fail if:
[...]
[EINPROGRESS]
The function was interrupted by a signal and fildes was closed
but the close operation is continuing asynchronously.
But this does not mean that the asynchronous close operation will
succeed.
So the application could incorrectly deduce that the close operation
was done without any error.
--
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / Pascaline project (LIP, ENS-Lyon)
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Theodore Ts'o @ 2025-05-16 14:20 UTC (permalink / raw)
To: Rich Felker
Cc: Alejandro Colomar, Jan Kara, Alexander Viro, Christian Brauner,
linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <20250516130547.GV1509@brightrain.aerifal.cx>
On Fri, May 16, 2025 at 09:05:47AM -0400, Rich Felker wrote:
> In general, raw kernel interfaces do not conform to any version of
> POSIX; they're just a low-impedance-mismatch set of inferfaces that
> facilitate implementing POSIX at the userspace libc layer. So I don't
> think this should be documented as "Linux doesn't conform" but
> (hopefully, once glibc fixes this) "old versions of glibc did not
> conform".
If glibc maintainers want to deal with breaking userspace, then as a
kernel developer, I'm happy to let them deal with the
angry/disappointed users and application programmers. :-)
- Ted
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Rich Felker @ 2025-05-16 13:05 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Jan Kara, Alexander Viro, Christian Brauner, linux-fsdevel,
linux-api, libc-alpha
In-Reply-To: <efaffc5a404cf104f225c26dbc96e0001cede8f9.1747399542.git.alx@kernel.org>
On Fri, May 16, 2025 at 02:52:05PM +0200, Alejandro Colomar wrote:
> POSIX.1-2024 now mandates a behavior different from what Linux (and many
> other implementations) does. It requires that we report EINPROGRESS for
> what now is EINTR.
>
> There are no plans to conform to POSIX.1-2024 within the Linux kernel,
> so document this divergence. Keep POSIX.1-2008 as the standard to
> which we conform in STANDARDS.
>
> Link: <https://sourceware.org/bugzilla/show_bug.cgi?id=14627>
> Link: <https://pubs.opengroup.org/onlinepubs/9799919799/functions/close.html>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Rich Felker <dalias@libc.org>
> Cc: <linux-fsdevel@vger.kernel.org>
> Cc: <linux-api@vger.kernel.org>
> Cc: <libc-alpha@sourceware.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
>
> Hi,
>
> I've prepared this draft for discussion. While doing so, I've noticed
> the glibc bug ticket, which sounds possibly reasonable: returning 0
> instead of reporting an error on EINTR. That would be an option that
> would make us conforming to POSIX.1-2024. And given that a user can
> (and must) do nothing after seeing EINTR, returning 0 wouldn't change
> things.
>
> So, I'll leave this patch open for discussion.
FWIW musl adopted the EINPROGRESS as soon as we were made aware of the
issue, and later changed it to returning 0 since applications
(particularly, any written prior to this interpretation) are prone to
interpret EINPROGRESS as an error condition rather than success and
possibly misinterpret it as meaning the fd is still open and valid to
pass to close again.
In general, raw kernel interfaces do not conform to any version of
POSIX; they're just a low-impedance-mismatch set of inferfaces that
facilitate implementing POSIX at the userspace libc layer. So I don't
think this should be documented as "Linux doesn't conform" but
(hopefully, once glibc fixes this) "old versions of glibc did not
conform".
Rich
^ permalink raw reply
* [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Alejandro Colomar @ 2025-05-16 12:52 UTC (permalink / raw)
Cc: Alejandro Colomar, Jan Kara, Alexander Viro, Christian Brauner,
Rich Felker, linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <a5tirrssh3t66q4vpwpgmxgxaumhqukw5nyxd4x6bevh7mtuvy@wtwdsb4oloh4>
POSIX.1-2024 now mandates a behavior different from what Linux (and many
other implementations) does. It requires that we report EINPROGRESS for
what now is EINTR.
There are no plans to conform to POSIX.1-2024 within the Linux kernel,
so document this divergence. Keep POSIX.1-2008 as the standard to
which we conform in STANDARDS.
Link: <https://sourceware.org/bugzilla/show_bug.cgi?id=14627>
Link: <https://pubs.opengroup.org/onlinepubs/9799919799/functions/close.html>
Cc: Jan Kara <jack@suse.cz>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Rich Felker <dalias@libc.org>
Cc: <linux-fsdevel@vger.kernel.org>
Cc: <linux-api@vger.kernel.org>
Cc: <libc-alpha@sourceware.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
Hi,
I've prepared this draft for discussion. While doing so, I've noticed
the glibc bug ticket, which sounds possibly reasonable: returning 0
instead of reporting an error on EINTR. That would be an option that
would make us conforming to POSIX.1-2024. And given that a user can
(and must) do nothing after seeing EINTR, returning 0 wouldn't change
things.
So, I'll leave this patch open for discussion.
Have a lovely day!
Alex
man/man2/close.2 | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/man/man2/close.2 b/man/man2/close.2
index b25ea4de9..9d5e26eed 100644
--- a/man/man2/close.2
+++ b/man/man2/close.2
@@ -191,10 +191,7 @@ .SS Dealing with error returns from close()
meaning that the file descriptor was invalid)
even if they subsequently report an error on return from
.BR close ().
-POSIX.1 is currently silent on this point,
-but there are plans to mandate this behavior in the next major release
-.\" Issue 8
-of the standard.
+POSIX.1-2008 was silent on this point.
.P
A careful programmer who wants to know about I/O errors may precede
.BR close ()
@@ -206,7 +203,7 @@ .SS Dealing with error returns from close()
error is a somewhat special case.
Regarding the
.B EINTR
-error, POSIX.1-2008 says:
+error, POSIX.1-2008 said:
.P
.RS
If
@@ -243,16 +240,10 @@ .SS Dealing with error returns from close()
error, and on at least one,
.BR close ()
must be called again.
-There are plans to address this conundrum for
-the next major release of the POSIX.1 standard.
-.\" FIXME . for later review when Issue 8 is one day released...
-.\" POSIX proposes further changes for EINTR
-.\" http://austingroupbugs.net/tag_view_page.php?tag_id=8
-.\" http://austingroupbugs.net/view.php?id=529
-.\"
-.\" FIXME .
-.\" Review the following glibc bug later
-.\" https://sourceware.org/bugzilla/show_bug.cgi?id=14627
+.P
+POSIX.1-2024 standardized the behavior of HP-UX,
+making Linux and many other implementations non-conforming.
+There are no plans to change the behavior on Linux.
.SH SEE ALSO
.BR close_range (2),
.BR fcntl (2),
Range-diff against v0:
-: --------- > 1: efaffc5a4 man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
base-commit: 978b017d93e4e32b752b33877e44a8365644630c
--
2.49.0
^ permalink raw reply related
* Re: close(2) with EINTR has been changed by POSIX.1-2024
From: Theodore Ts'o @ 2025-05-16 12:41 UTC (permalink / raw)
To: Jan Kara
Cc: Alejandro Colomar, Alexander Viro, Christian Brauner,
linux-fsdevel, linux-api, linux-man
In-Reply-To: <ddqmhjc2rpzk2jjvunbt3l3eukcn4xzkocqzdg3j4msihdhzko@fizekvxndg2d>
On Fri, May 16, 2025 at 12:48:56PM +0200, Jan Kara wrote:
> > Now, POSIX.1-2024 says:
> >
> > If close() is interrupted by a signal that is to be caught, then
> > it is unspecified whether it returns -1 with errno set to
> > [EINTR] and fildes remaining open, or returns -1 with errno set
> > to [EINPROGRESS] and fildes being closed, or returns 0 to
> > indicate successful completion; [...]
> >
> > <https://pubs.opengroup.org/onlinepubs/9799919799/functions/close.html>
> >
> > Which seems to bless HP-UX and screw all the others, requiring them to
> > report EINPROGRESS.
> >
> > Was there any discussion about what to do in the Linux kernel?
>
> I'm not aware of any discussions but indeed we are returning EINTR while
> closing the fd. Frankly, changing the error code we return in that case is
> really asking for userspace regressions so I'm of the opinion we just
> ignore the standard as in my opinion it goes against a long established
> reality.
Yeah, it appears that the Austin Group has lost all connection with
reality, and we should treat POSIX 2024 accordingly. Not breaking
userspace applications is way more important that POSIX 2024
compliance. Which is sad, because I used to really care about POSIX.1
standard as being very useful. But that seems to be no longer the
case...
- Ted
^ permalink raw reply
* Re: close(2) with EINTR has been changed by POSIX.1-2024
From: Mateusz Guzik @ 2025-05-16 12:41 UTC (permalink / raw)
To: Jan Kara
Cc: Alejandro Colomar, Alexander Viro, Christian Brauner,
linux-fsdevel, linux-api, linux-man
In-Reply-To: <ddqmhjc2rpzk2jjvunbt3l3eukcn4xzkocqzdg3j4msihdhzko@fizekvxndg2d>
On Fri, May 16, 2025 at 12:48:56PM +0200, Jan Kara wrote:
> Hi!
>
> On Thu 15-05-25 23:33:22, Alejandro Colomar wrote:
> > I'm updating the manual pages for POSIX.1-2024, and have some doubts
> > about close(2). The manual page for close(2) says (conforming to
> > POSIX.1-2008):
> >
> > The EINTR error is a somewhat special case. Regarding the EINTR
> > error, POSIX.1‐2008 says:
> >
> > If close() is interrupted by a signal that is to be
> > caught, it shall return -1 with errno set to EINTR and
> > the state of fildes is unspecified.
> >
> > This permits the behavior that occurs on Linux and many other
> > implementations, where, as with other errors that may be re‐
> > ported by close(), the file descriptor is guaranteed to be
> > closed. However, it also permits another possibility: that the
> > implementation returns an EINTR error and keeps the file de‐
> > scriptor open. (According to its documentation, HP‐UX’s close()
> > does this.) The caller must then once more use close() to close
> > the file descriptor, to avoid file descriptor leaks. This di‐
> > vergence in implementation behaviors provides a difficult hurdle
> > for portable applications, since on many implementations,
> > close() must not be called again after an EINTR error, and on at
> > least one, close() must be called again. There are plans to ad‐
> > dress this conundrum for the next major release of the POSIX.1
> > standard.
> >
> > TL;DR: close(2) with EINTR is allowed to either leave the fd open or
> > closed, and Linux leaves it closed, while others (HP-UX only?) leaves it
> > open.
> >
> > Now, POSIX.1-2024 says:
> >
> > If close() is interrupted by a signal that is to be caught, then
> > it is unspecified whether it returns -1 with errno set to
> > [EINTR] and fildes remaining open, or returns -1 with errno set
> > to [EINPROGRESS] and fildes being closed, or returns 0 to
> > indicate successful completion; [...]
> >
> > <https://pubs.opengroup.org/onlinepubs/9799919799/functions/close.html>
> >
> > Which seems to bless HP-UX and screw all the others, requiring them to
> > report EINPROGRESS.
> >
> > Was there any discussion about what to do in the Linux kernel?
>
> I'm not aware of any discussions but indeed we are returning EINTR while
> closing the fd. Frankly, changing the error code we return in that case is
> really asking for userspace regressions so I'm of the opinion we just
> ignore the standard as in my opinion it goes against a long established
> reality.
I wonder what are they thinking there.
Any program which even bothers to check for EINTR assumes the fd is
already closed, so one has to assume augmenting behavior to support this
would result in fd leaks.
But that crappery aside, I do wonder if a close() variant which can fail
and leaves the fd intact would be warranted.
For example one of the error modes is ENOSPC (or at least the manpage
claims as much). As is the error is not actionable as the fd is gone.
If instead a magic flag was passed down to indicate what to do (e.g.,
leave the fd in place), the program could try to do some recovery (for
examples unlinking temp files it knows it stores there).
Similar deal with EINTR, albeit this error for close() would preferably get
eradicated instead.
Just some meh rambling.
^ permalink raw reply
* Re: close(2) with EINTR has been changed by POSIX.1-2024
From: Alejandro Colomar @ 2025-05-16 12:11 UTC (permalink / raw)
To: Jan Kara
Cc: Alexander Viro, Christian Brauner, linux-fsdevel, linux-api,
linux-man
In-Reply-To: <ddqmhjc2rpzk2jjvunbt3l3eukcn4xzkocqzdg3j4msihdhzko@fizekvxndg2d>
[-- Attachment #1: Type: text/plain, Size: 844 bytes --]
Hi Jan!
On Fri, May 16, 2025 at 12:48:56PM +0200, Jan Kara wrote:
> > <https://pubs.opengroup.org/onlinepubs/9799919799/functions/close.html>
> >
> > Which seems to bless HP-UX and screw all the others, requiring them to
> > report EINPROGRESS.
> >
> > Was there any discussion about what to do in the Linux kernel?
>
> I'm not aware of any discussions but indeed we are returning EINTR while
> closing the fd. Frankly, changing the error code we return in that case is
> really asking for userspace regressions so I'm of the opinion we just
> ignore the standard as in my opinion it goes against a long established
> reality.
Yep, sounds like what I was expecting. I'll document that we'll ignore
the new POSIX for close(2) on purpose. Thanks!
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: close(2) with EINTR has been changed by POSIX.1-2024
From: Jan Kara @ 2025-05-16 10:48 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Alexander Viro, Christian Brauner, Jan Kara, linux-fsdevel,
linux-api, linux-man
In-Reply-To: <fskxqmcszalz6dmoak6de4c7bxt4juvc5zrpboae4dqw4y6aih@lskezjrbnsws>
Hi!
On Thu 15-05-25 23:33:22, Alejandro Colomar wrote:
> I'm updating the manual pages for POSIX.1-2024, and have some doubts
> about close(2). The manual page for close(2) says (conforming to
> POSIX.1-2008):
>
> The EINTR error is a somewhat special case. Regarding the EINTR
> error, POSIX.1‐2008 says:
>
> If close() is interrupted by a signal that is to be
> caught, it shall return -1 with errno set to EINTR and
> the state of fildes is unspecified.
>
> This permits the behavior that occurs on Linux and many other
> implementations, where, as with other errors that may be re‐
> ported by close(), the file descriptor is guaranteed to be
> closed. However, it also permits another possibility: that the
> implementation returns an EINTR error and keeps the file de‐
> scriptor open. (According to its documentation, HP‐UX’s close()
> does this.) The caller must then once more use close() to close
> the file descriptor, to avoid file descriptor leaks. This di‐
> vergence in implementation behaviors provides a difficult hurdle
> for portable applications, since on many implementations,
> close() must not be called again after an EINTR error, and on at
> least one, close() must be called again. There are plans to ad‐
> dress this conundrum for the next major release of the POSIX.1
> standard.
>
> TL;DR: close(2) with EINTR is allowed to either leave the fd open or
> closed, and Linux leaves it closed, while others (HP-UX only?) leaves it
> open.
>
> Now, POSIX.1-2024 says:
>
> If close() is interrupted by a signal that is to be caught, then
> it is unspecified whether it returns -1 with errno set to
> [EINTR] and fildes remaining open, or returns -1 with errno set
> to [EINPROGRESS] and fildes being closed, or returns 0 to
> indicate successful completion; [...]
>
> <https://pubs.opengroup.org/onlinepubs/9799919799/functions/close.html>
>
> Which seems to bless HP-UX and screw all the others, requiring them to
> report EINPROGRESS.
>
> Was there any discussion about what to do in the Linux kernel?
I'm not aware of any discussions but indeed we are returning EINTR while
closing the fd. Frankly, changing the error code we return in that case is
really asking for userspace regressions so I'm of the opinion we just
ignore the standard as in my opinion it goes against a long established
reality.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* close(2) with EINTR has been changed by POSIX.1-2024
From: Alejandro Colomar @ 2025-05-15 21:33 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, linux-fsdevel,
linux-api
Cc: linux-man
[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]
Hi,
I'm updating the manual pages for POSIX.1-2024, and have some doubts
about close(2). The manual page for close(2) says (conforming to
POSIX.1-2008):
The EINTR error is a somewhat special case. Regarding the EINTR
error, POSIX.1‐2008 says:
If close() is interrupted by a signal that is to be
caught, it shall return -1 with errno set to EINTR and
the state of fildes is unspecified.
This permits the behavior that occurs on Linux and many other
implementations, where, as with other errors that may be re‐
ported by close(), the file descriptor is guaranteed to be
closed. However, it also permits another possibility: that the
implementation returns an EINTR error and keeps the file de‐
scriptor open. (According to its documentation, HP‐UX’s close()
does this.) The caller must then once more use close() to close
the file descriptor, to avoid file descriptor leaks. This di‐
vergence in implementation behaviors provides a difficult hurdle
for portable applications, since on many implementations,
close() must not be called again after an EINTR error, and on at
least one, close() must be called again. There are plans to ad‐
dress this conundrum for the next major release of the POSIX.1
standard.
TL;DR: close(2) with EINTR is allowed to either leave the fd open or
closed, and Linux leaves it closed, while others (HP-UX only?) leaves it
open.
Now, POSIX.1-2024 says:
If close() is interrupted by a signal that is to be caught, then
it is unspecified whether it returns -1 with errno set to
[EINTR] and fildes remaining open, or returns -1 with errno set
to [EINPROGRESS] and fildes being closed, or returns 0 to
indicate successful completion; [...]
<https://pubs.opengroup.org/onlinepubs/9799919799/functions/close.html>
Which seems to bless HP-UX and screw all the others, requiring them to
report EINPROGRESS.
Was there any discussion about what to do in the Linux kernel?
Have a lovely night!
Alex
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 0/7] fs: introduce file_getattr and file_setattr syscalls
From: Amir Goldstein @ 2025-05-15 10:33 UTC (permalink / raw)
To: Christian Brauner
Cc: Arnd Bergmann, Andrey Albershteyn, Richard Henderson, Matt Turner,
Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
Michal Simek, Thomas Bogendoerfer, James E . J . Bottomley,
Helge Deller, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S . Miller, Andreas Larsson,
Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Chris Zankel, Max Filippov,
Alexander Viro, Jan Kara, Mickaël Salaün,
Günther Noack, Pali Rohár, Paul Moore, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Tyler Hicks,
Miklos Szeredi, linux-alpha, linux-kernel, linux-arm-kernel,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, linux-security-module,
linux-api, Linux-Arch, selinux, ecryptfs, linux-unionfs,
linux-xfs, Andrey Albershteyn
In-Reply-To: <20250515-bedarf-absagen-464773be3e72@brauner>
On Thu, May 15, 2025 at 11:02 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Tue, May 13, 2025 at 11:53:23AM +0200, Arnd Bergmann wrote:
> > On Tue, May 13, 2025, at 11:17, Andrey Albershteyn wrote:
> >
> > >
> > > long syscall(SYS_file_getattr, int dirfd, const char *pathname,
> > > struct fsxattr *fsx, size_t size, unsigned int at_flags);
> > > long syscall(SYS_file_setattr, int dirfd, const char *pathname,
> > > struct fsxattr *fsx, size_t size, unsigned int at_flags);
> >
> > I don't think we can have both the "struct fsxattr" from the uapi
> > headers, and a variable size as an additional argument. I would
> > still prefer not having the extensible structure at all and just
>
> We're not going to add new interfaces that are fixed size unless for the
> very basic cases. I don't care if we're doing that somewhere else in the
> kernel but we're not doing that for vfs apis.
>
> > use fsxattr, but if you want to make it extensible in this way,
> > it should use a different structure (name). Otherwise adding
> > fields after fsx_pad[] would break the ioctl interface.
>
> Would that really be a problem? Just along the syscall simply add
> something like:
>
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index c91fd2b46a77..d3943805c4be 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -868,12 +868,6 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> case FS_IOC_SETFLAGS:
> return ioctl_setflags(filp, argp);
>
> - case FS_IOC_FSGETXATTR:
> - return ioctl_fsgetxattr(filp, argp);
> -
> - case FS_IOC_FSSETXATTR:
> - return ioctl_fssetxattr(filp, argp);
> -
> case FS_IOC_GETFSUUID:
> return ioctl_getfsuuid(filp, argp);
>
> @@ -886,6 +880,20 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> break;
> }
>
> + switch (_IOC_NR(cmd)) {
> + case _IOC_NR(FS_IOC_FSGETXATTR):
> + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FS_IOC_FSGETXATTR)))
> + return SOMETHING_SOMETHING;
> + /* Only handle original size. */
> + return ioctl_fsgetxattr(filp, argp);
> +
> + case _IOC_NR(FFS_IOC_FSSETXATTR):
> + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FFS_IOC_FSSETXATTR)))
> + return SOMETHING_SOMETHING;
> + /* Only handle original size. */
> + return ioctl_fssetxattr(filp, argp);
> + }
> +
I think what Arnd means is that we will not be able to change struct
sfxattr in uapi
going forward, because we are not going to deprecate the ioctls and
certainly not
the XFS specific ioctl XFS_IOC_FSGETXATTRA.
This struct is part of XFS uapi:
https://man7.org/linux/man-pages/man2/ioctl_xfs_fsgetxattr.2.html
Should we will need to depart from this struct definition and we might
as well do it for the initial release of the syscall rather than later on, e.g.:
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -148,6 +148,17 @@ struct fsxattr {
unsigned char fsx_pad[8];
};
+/*
+ * Variable size structure for file_[sg]et_attr().
+ */
+struct fsx_fileattr {
+ __u32 fsx_xflags; /* xflags field value (get/set) */
+ __u32 fsx_extsize; /* extsize field value (get/set)*/
+ __u32 fsx_nextents; /* nextents field value (get) */
+ __u32 fsx_projid; /* project identifier (get/set) */
+ __u32 fsx_cowextsize; /* CoW extsize field value (get/set)*/
+};
+
+#define FSXATTR_SIZE_VER0 20
+#define FSXATTR_SIZE_LATEST FSXATTR_SIZE_VER0
+
Right?
Thanks,
Amir.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox