Linux userland API discussions
 help / color / mirror / Atom feed
* Re: close(2) with EINTR has been changed by POSIX.1-2024
From: Christian Brauner @ 2025-05-19  9:48 UTC (permalink / raw)
  To: Jan Kara
  Cc: Alejandro Colomar, Alexander Viro, 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.

Ignore. We've long since stopped designing apis with input from that
standard in mind. And I think that was a very wise decision.

^ permalink raw reply

* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Alejandro Colomar @ 2025-05-17 13:46 UTC (permalink / raw)
  To: Rich Felker
  Cc: Vincent Lefevre, Jan Kara, Alexander Viro, Christian Brauner,
	linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <20250517133251.GY1509@brightrain.aerifal.cx>

[-- Attachment #1: Type: text/plain, Size: 3465 bytes --]

On Sat, May 17, 2025 at 09:32:52AM -0400, Rich Felker wrote:
> 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.
> 
> There are no asynchronous behaviors specified for there to be a
> conformance distinction here. The only observable behaviors happen
> instantly, mainly the release of the file descriptor and the process's
> handle on the underlying resource. Abstractly, there is no async
> operation that could succeed or fail.
> 
> > So the application could incorrectly deduce that the close operation
> > was done without any error.
> 
> This deduction is correct, not incorrect. Rather, failing with
> EINPROGRESS would make the application incorrectly deduce that there
> might be some error it missed (even if it's aware of the new error
> code), and absolutely does make all existing applications written
> prior to the new text in POSIX 2024 unable to determine if the fd was
> even released and needs to be passed to close again or not.

Hi Rich,

I think this is not correct; at least on Linux.  The manual page is very
clear that close(2) should not be retried on error:

   Dealing with error returns from close()
       A  careful  programmer  will  check the return value of close(),
       since it is quite possible that errors on  a  previous  write(2)
       operation  are  reported only on the final close() that releases
       the open file description.  Failing to check  the  return  value
       when  closing  a file may lead to silent loss of data.  This can
       especially be observed with NFS and with disk quota.

       Note, however, that a failure return should be used only for di‐
       agnostic purposes (i.e., a warning to the application that there
       may still be I/O pending or there may have been failed  I/O)  or
       remedial  purposes (e.g., writing the file once more or creating
       a backup).

       Retrying the close() after a failure return is the  wrong  thing
       to  do,  since  this may cause a reused file descriptor from an‐
       other thread to be closed.  This can  occur  because  the  Linux
       kernel  always  releases  the file descriptor early in the close
       operation, freeing it for reuse; the steps that  may  return  an
       error,  such as flushing data to the filesystem or device, occur
       only later in the close operation.

	...

       A careful programmer who wants to know about I/O errors may pre‐
       cede close() with a call to fsync(2).


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Rich Felker @ 2025-05-17 13:43 UTC (permalink / raw)
  To: Alejandro Colomar
  Cc: Theodore Ts'o, Jan Kara, Alexander Viro, Christian Brauner,
	linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <qthuiudgbwuxh4bwwpcvpbosqrz6rl4js46atvenhmujkbjnz4@crakvrigxnz6>

On Sat, May 17, 2025 at 03:03:52PM +0200, Alejandro Colomar wrote:
> Hi,
> 
> On Sat, May 17, 2025 at 07:46:48AM +0200, Alejandro Colomar wrote:
> > Hi Ted, Rich,
> > 
> > On Fri, May 16, 2025 at 09:05:47AM -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.
> 
> BTW, I don't think that's a correct interpretation.  The manual page
> clearly says after close(2), even on error, the fd is closed and not
> usable.  The issue I see is a program thinking it failed and trying to
> copy the file again or reporting an error.

The authoritative source here is POSIX not the man page, assuming
you're writing a portable application and not a "Linux application".

Until the lastest issue (POSIX 2024/Issue 8), the state of the fd
after EINTR was explicitly unspecified, and after other errors was
unspecified by omission. So there is no way for a program written to
prior versions of the standard to have known how to safely handle
getting EINPROGRESS -- or any error from close for that matter.
Really, the only safe error for close to return, *ever*, is EBADF. On
valid input, it *must succeed*. This is a general principle for
"deallocation/destruction functions". Not an explicit requirement of
this or any standard; just a logical requirement for forward progress
to be possible.

> On the other hand, as Vincent said, maybe this is not so bad.  For
> certain files, fsync(2) is only described for storage devices, so in
> some cases there's no clear way to make sure close(2) won't fail after
> EINTR (maybe calling sync(2)?).  So, maybe considering it an error
> wouldn't be a terrible idea.

Whether data is committed to physical storage in a way that's robust
against machine faults is a completely separate issue from whether
it's committed to the abstract storage. The latter happens at the
moment of write, not close.

If an application is trying to ensure that kind of robustness, the
return value of close is not the tool. It needs the Synchronized IO
interfaces (fsync, etc.) or something specific to whatever it's
writing to.

Rich

^ permalink raw reply

* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Rich Felker @ 2025-05-17 13:32 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.

There are no asynchronous behaviors specified for there to be a
conformance distinction here. The only observable behaviors happen
instantly, mainly the release of the file descriptor and the process's
handle on the underlying resource. Abstractly, there is no async
operation that could succeed or fail.

> So the application could incorrectly deduce that the close operation
> was done without any error.

This deduction is correct, not incorrect. Rather, failing with
EINPROGRESS would make the application incorrectly deduce that there
might be some error it missed (even if it's aware of the new error
code), and absolutely does make all existing applications written
prior to the new text in POSIX 2024 unable to determine if the fd was
even released and needs to be passed to close again or not.

Rich

^ permalink raw reply

* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Alejandro Colomar @ 2025-05-17 13:03 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Rich Felker, Jan Kara, Alexander Viro, Christian Brauner,
	linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <bsvslfjgcmzvcanxp3ay6ohitqulwuawwgzy234nfkj6ecdxbq@2uhld4vpitou>

[-- Attachment #1: Type: text/plain, Size: 2731 bytes --]

Hi,

On Sat, May 17, 2025 at 07:46:48AM +0200, Alejandro Colomar wrote:
> Hi Ted, Rich,
> 
> On Fri, May 16, 2025 at 09:05:47AM -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.

BTW, I don't think that's a correct interpretation.  The manual page
clearly says after close(2), even on error, the fd is closed and not
usable.  The issue I see is a program thinking it failed and trying to
copy the file again or reporting an error.

On the other hand, as Vincent said, maybe this is not so bad.  For
certain files, fsync(2) is only described for storage devices, so in
some cases there's no clear way to make sure close(2) won't fail after
EINTR (maybe calling sync(2)?).  So, maybe considering it an error
wouldn't be a terrible idea.

I don't know.


Cheers,
Alex

> 
> Hmmm, this page will need a kernel/libc differences section where I
> should explain this.
> 
> On Fri, May 16, 2025 at 10:20:24AM -0400, Theodore Ts'o wrote:
> > 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.  :-)
> 
> Which breakage do you expect from the behavior that musl has chosen?
> 
> I agree that the POSIX invention of EINPROGRESS is something that would
> break users.  However, in removing the error completely and making it a
> success, I don't see the same problem.  That is, if a program calls
> close(2) and sees a return of 0, or sees a return of -1 with EINTR on
> Linux, both mean "the file descriptor has been closed, and the contents
> of the file will *eventually* reach the file".
> 
> In which cases do you expect any existing Linux program to behave
> differently on 0 and on EINTR?
> 
> 
> Have a lovely day!
> Alex
> 
> -- 
> <https://www.alejandro-colomar.es/>



-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
From: John Johansen @ 2025-05-17  7:59 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Song Liu, Maxime Bélair, linux-security-module, paul,
	jmorris, serge, kees, stephen.smalley.work, casey, takedakn,
	penguin-kernel, linux-api, apparmor, linux-kernel, Arnd Bergmann
In-Reply-To: <20250512.Uong6eCaVuwu@digikod.net>

On 5/12/25 03:20, Mickaël Salaün wrote:
> On Sun, May 11, 2025 at 03:47:21AM -0700, John Johansen wrote:
>> On 5/9/25 03:26, Mickaël Salaün wrote:
>>> On Thu, May 08, 2025 at 01:18:20AM -0700, John Johansen wrote:
>>>> On 5/7/25 23:06, Song Liu wrote:
>>>>> On Wed, May 7, 2025 at 8:37 AM Maxime Bélair
>>>>> <maxime.belair@canonical.com> wrote:
>>>>> [...]
> 
>>>>> permission check to each pseudo file. The downside of the syscall, however,
>>>>> is that all the permission checks are hard-coded in the kernel (except for
>>>>
>>>> The permission checks don't have to be hard coded. Each LSM can define how it handles
>>>> or manages the syscall. The default is that it isn't supported, but if an lsm decides
>>>> to support it, there is now reason that its policy can't determine the use of the
>>>> syscall.
>>>
>>>   From an interface design point of view, it would be better to clearly
>>> specify the scope of a command (e.g. which components could be impacted
>>> by a command), and make sure the documentation reflect that as well.
>>> Even better, have a syscalls per required privileges and impact (e.g.
>>> privileged or unprivileged).  Going this road, I'm not sure if a
>>> privileged syscall would make sense given the existing filesystem
>>> interface.
>>>
>>
>> uhhhmmm, not just privileged. As you well know we are looking to use
>> this for unprivileged policy. The LSM can limit to privileged if it
>> wants but it doesn't have to limit it to privileged policy.
> 
> Yes, I meant to say having a syscall for unprivileged actions, and maybe
> another one for privileged ones, but this might be a hard sell. :)
> 
indeed, in the apparmor case context would be important. Just exactly
what is privileged. It may be a privileged operation to load policy to one
namespace, but not to another that you are setting up for a child.

> To say it another way, for your use case, do you need this syscall(s)
> for privileged operations?  Do you plan to drop (or stop extending) the

need, probably. That is to say, loading of policy have varying levels
of privilege. root within the container has privilege to load policy
to its namespace, but it might have authority to setup a child namespace
that does not require privilege for it to load policy into, and it
will determine if the child has privilege or unprivleged policy within
it.

Ideally we won't have to use the fs interface within the "privileged"
container, as there are cases where this is currently not done or
undesirable.

> filesystem interface or do you think it would be good for (AppArmor)
> privileged operations too?  I know syscalls might be attractive and
> could be used for everything, but it's good to have a well-defined plan
> and semantic to avoid using such syscall as another multiplexer with
> unrelated operations and required privileges.
> 
sure. But the privilege level is use case dependent, to which policy
namespace is policy being loaded, replaced, ...  The privilege level
very much will depend on what is in the stack/bounding of policy.

> If this syscall should also be a way to do privileged operations, should
> we also agree on a common set of permissions (e.g. global CAP_MAC_ADMIN
> or user namespace one)?
> 
I think requiring something like CAP_MAC_ADMIN would be a per LSM
decision.


> [...]
> 
>>>>> Overall, I am still not convinced a syscall for all LSMs is needed. To
>>>>> justify such
>>>>
>>>> its not needed by all LSMs, just a subset of them, and some nebulous
>>>> subset of potentially future LSMs that is entirely undefinable.
>>>>
>>>> If we had had appropriate LSM syscalls landlock wouldn't have needed
>>>> to have landlock specific syscalls. Having another LSM go that route
>>>> feels wrong especially now that we have some LSM syscalls.
>>>
>>> I don't agree.  Dedicated syscalls are a good thing.  See my other
>>> reply.
>>>
>>
>> I think we can just disagree on this point.
>>
>>>> If a
>>>> syscall is needed by an LSM its better to try hashing something out
>>>> that might have utility for multiple LSMs or at the very least,
>>>> potentially have utility in the future.
>>>>
>>>>
>>>>> a syscall, I think we need to show that it is useful in multiple LSMs.
>>>>> Also, if we
>>>>> really want to have single set of APIs for all LSMs, we may also need
>>>>> get_policy,
>>>>
>>>> We are never going to get a single set of APIs for all LSMs. I will
>>>> settle for an api that has utility for a subset
>>>>
>>>>> remove_policy, etc. This set as-is appears to be an incomplete design. The
>>>>
>>>> To have a complete design, there needs to be feedback and discussion
>>>> from multiple LSMs. This is a starting point.
>>>>
>>>>> implementation, with call_int_hook, is also problematic. It can easily
>>>>> cause some> controversial behaviors.
>>>>>
>>>> agreed it shouldn't be doing a straight call_int_hook, it should only
>>>> call it against the lsm identified by the lsmid
>>>
>>> Yes, but then, I don't see the point of a "generic" LSM syscall.
>>
>> its not a generic LSM syscall. Its a syscall or maybe a set of syscalls
>> for a specific scoped problem of loading/managing policy.
>>
>> Can we come to something acceptable? I don't know but we are going to
>> look at it before trying for an apparmor specific syscall.
> 
> I understand and it's good to have this discussion.


^ permalink raw reply

* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Alejandro Colomar @ 2025-05-17  5:46 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Rich Felker, Jan Kara, Alexander Viro, Christian Brauner,
	linux-fsdevel, linux-api, libc-alpha
In-Reply-To: <20250516142024.GA21503@mit.edu>

[-- Attachment #1: Type: text/plain, Size: 1912 bytes --]

Hi Ted, Rich,

On Fri, May 16, 2025 at 09:05:47AM -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.

Hmmm, this page will need a kernel/libc differences section where I
should explain this.

On Fri, May 16, 2025 at 10:20:24AM -0400, Theodore Ts'o wrote:
> 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.  :-)

Which breakage do you expect from the behavior that musl has chosen?

I agree that the POSIX invention of EINPROGRESS is something that would
break users.  However, in removing the error completely and making it a
success, I don't see the same problem.  That is, if a program calls
close(2) and sees a return of 0, or sees a return of -1 with EINTR on
Linux, both mean "the file descriptor has been closed, and the contents
of the file will *eventually* reach the file".

In which cases do you expect any existing Linux program to behave
differently on 0 and on EINTR?


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: [PATCH 2/6] tcp: copy write-data from listen socket to accept child socket
From: Jeremy Harris @ 2025-05-16 20:11 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, linux-api, ncardwell
In-Reply-To: <CANn89iLNEnF1YYwOmPsuQn6n-O9N9yVxnZ_djEGxaZ11i4AsVA@mail.gmail.com>

On 2025/05/16 6:51 PM, Eric Dumazet wrote:
> I do not see any locking. I think you should run a local KASAN/syzbot
> instance and you will be shocked.

Thanks for the suggestion; I'll look into doing that.

-- 
Cheers,
   Jeremy

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox