* [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario @ 2022-06-01 2:47 Frederik Deweerdt 2022-06-01 13:24 ` Willem de Bruijn 0 siblings, 1 reply; 7+ messages in thread From: Frederik Deweerdt @ 2022-06-01 2:47 UTC (permalink / raw) To: netdev; +Cc: willemb, Frederik Deweerdt Hi folks, Based on my understanding, retransmissions of zero copied buffers can happen after `close(2)`, the patch below amends the docs to suggest how notifications should be handled in that case. Explicitly mention that applications shouldn't be calling `close(2)` on a TCP socket without draining the error queue. --- Documentation/networking/msg_zerocopy.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/networking/msg_zerocopy.rst b/Documentation/networking/msg_zerocopy.rst index 15920db8d35d..cb44fc1f3e3e 100644 --- a/Documentation/networking/msg_zerocopy.rst +++ b/Documentation/networking/msg_zerocopy.rst @@ -144,6 +144,10 @@ the socket. A socket that has an error queued would normally block other operations until the error is read. Zerocopy notifications have a zero error code, however, to not block send and recv calls. +For protocols like TCP, where retransmissions can occur after the +application is done with a given connection, applications should signal +the close to the peer via shutdown(2), and keep polling the error queue +until all transmissions have completed. Notification Batching ~~~~~~~~~~~~~~~~~~~~~ -- 2.36.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario 2022-06-01 2:47 [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario Frederik Deweerdt @ 2022-06-01 13:24 ` Willem de Bruijn 2022-06-01 23:01 ` Frederik Deweerdt 0 siblings, 1 reply; 7+ messages in thread From: Willem de Bruijn @ 2022-06-01 13:24 UTC (permalink / raw) To: Frederik Deweerdt; +Cc: netdev On Tue, May 31, 2022 at 10:48 PM Frederik Deweerdt <frederik.deweerdt@gmail.com> wrote: > > Hi folks, > > Based on my understanding, retransmissions of zero copied buffers can > happen after `close(2)`, the patch below amends the docs to suggest how > notifications should be handled in that case. Not just retransmissions. The first transmission similarly may be queued. > > Explicitly mention that applications shouldn't be calling `close(2)` on > a TCP socket without draining the error queue. > --- > Documentation/networking/msg_zerocopy.rst | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/networking/msg_zerocopy.rst b/Documentation/networking/msg_zerocopy.rst > index 15920db8d35d..cb44fc1f3e3e 100644 > --- a/Documentation/networking/msg_zerocopy.rst > +++ b/Documentation/networking/msg_zerocopy.rst > @@ -144,6 +144,10 @@ the socket. A socket that has an error queued would normally block > other operations until the error is read. Zerocopy notifications have > a zero error code, however, to not block send and recv calls. > > +For protocols like TCP, where retransmissions can occur after the > +application is done with a given connection, applications should signal > +the close to the peer via shutdown(2), and keep polling the error queue > +until all transmissions have completed. A socket must not be closed until all completion notifications have been received. Calling shutdown is an optional step. It may be sufficient to simply delay close. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario 2022-06-01 13:24 ` Willem de Bruijn @ 2022-06-01 23:01 ` Frederik Deweerdt 2022-06-02 0:29 ` Willem de Bruijn 2022-06-03 13:09 ` David Laight 0 siblings, 2 replies; 7+ messages in thread From: Frederik Deweerdt @ 2022-06-01 23:01 UTC (permalink / raw) To: Willem de Bruijn; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 1328 bytes --] Hi Willem, On Wed, Jun 01, 2022 at 09:24:32AM -0400, Willem de Bruijn wrote: > On Tue, May 31, 2022 at 10:48 PM Frederik Deweerdt > <frederik.deweerdt@gmail.com> wrote: > > > > Hi folks, > > > > Based on my understanding, retransmissions of zero copied buffers can > > happen after `close(2)`, the patch below amends the docs to suggest how > > notifications should be handled in that case. > > Not just retransmissions. The first transmission similarly may be queued. > > > [...] > > @@ -144,6 +144,10 @@ the socket. A socket that has an error queued would normally block > > other operations until the error is read. Zerocopy notifications have > > a zero error code, however, to not block send and recv calls. > > > > +For protocols like TCP, where retransmissions can occur after the > > +application is done with a given connection, applications should signal > > +the close to the peer via shutdown(2), and keep polling the error queue > > +until all transmissions have completed. > > A socket must not be closed until all completion notifications have > been received. > > Calling shutdown is an optional step. It may be sufficient to simply > delay close. Thank you for the feedback, that helps! What do you think of the attached patch? Frederik Signed-off-by: Frederik Deweerdt <frederik.deweerdt@gmail.com> [-- Attachment #2: patch --] [-- Type: text/plain, Size: 1155 bytes --] commit 3218d973b68bc6d9f88d9e2374f3ada3df5ee7ff Author: Frederik Deweerdt <frederik.deweerdt@gmail.com> Date: Tue May 31 18:23:54 2022 -0700 [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario Explicitly mention that applications shouldn't be calling `close(2)` on a TCP socket without draining the error queue. diff --git a/Documentation/networking/msg_zerocopy.rst b/Documentation/networking/msg_zerocopy.rst index 15920db8d35d..9373631d0a82 100644 --- a/Documentation/networking/msg_zerocopy.rst +++ b/Documentation/networking/msg_zerocopy.rst @@ -144,6 +144,11 @@ the socket. A socket that has an error queued would normally block other operations until the error is read. Zerocopy notifications have a zero error code, however, to not block send and recv calls. +For protocols like TCP, transmissions can occur after the application +has called close(2). In cases where it's undesirable to delay calling +close(2) until all notifications have been processed, the application +can use shutdown(2), and keep polling the error queue until all +transmissions have completed. Notification Batching ~~~~~~~~~~~~~~~~~~~~~ ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario 2022-06-01 23:01 ` Frederik Deweerdt @ 2022-06-02 0:29 ` Willem de Bruijn 2022-06-03 13:09 ` David Laight 1 sibling, 0 replies; 7+ messages in thread From: Willem de Bruijn @ 2022-06-02 0:29 UTC (permalink / raw) To: Frederik Deweerdt; +Cc: netdev On Wed, Jun 1, 2022 at 7:03 PM Frederik Deweerdt <frederik.deweerdt@gmail.com> wrote: > > Hi Willem, > > On Wed, Jun 01, 2022 at 09:24:32AM -0400, Willem de Bruijn wrote: > > On Tue, May 31, 2022 at 10:48 PM Frederik Deweerdt > > <frederik.deweerdt@gmail.com> wrote: > > > > > > Hi folks, > > > > > > Based on my understanding, retransmissions of zero copied buffers can > > > happen after `close(2)`, the patch below amends the docs to suggest how > > > notifications should be handled in that case. > > > > Not just retransmissions. The first transmission similarly may be queued. > > > > > > [...] > > > @@ -144,6 +144,10 @@ the socket. A socket that has an error queued would normally block > > > other operations until the error is read. Zerocopy notifications have > > > a zero error code, however, to not block send and recv calls. > > > > > > +For protocols like TCP, where retransmissions can occur after the > > > +application is done with a given connection, applications should signal > > > +the close to the peer via shutdown(2), and keep polling the error queue > > > +until all transmissions have completed. > > > > A socket must not be closed until all completion notifications have > > been received. > > > > Calling shutdown is an optional step. It may be sufficient to simply > > delay close. > > Thank you for the feedback, that helps! > > What do you think of the attached patch? Please always share patches inline. > +++ b/Documentation/networking/msg_zerocopy.rst > @@ -144,6 +144,11 @@ the socket. A socket that has an error queued would normally block > other operations until the error is read. Zerocopy notifications have > a zero error code, however, to not block send and recv calls. > > +For protocols like TCP, transmissions can occur after the application > +has called close(2). In cases where it's undesirable to delay calling > +close(2) until all notifications have been processed, the application > +can use shutdown(2), and keep polling the error queue until all > +transmissions have completed. Sorry to nitpick, but calling close(2) is still delayed. You meant where it's undesirable to delay closing the TCP connection? Shutdown can be used then. But that is not unique to msg_zerocopy? I don't feel strongly either way. Perhaps it would be helpful to describe what makes it is undesirable to wait with calling close. Btw, packets leaving the host after a socket is closed is not unique to the TCP protocol, but an effect of queuing, such as in the qdisc layer. ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario 2022-06-01 23:01 ` Frederik Deweerdt 2022-06-02 0:29 ` Willem de Bruijn @ 2022-06-03 13:09 ` David Laight 2022-06-03 13:30 ` Willem de Bruijn 1 sibling, 1 reply; 7+ messages in thread From: David Laight @ 2022-06-03 13:09 UTC (permalink / raw) To: 'Frederik Deweerdt', Willem de Bruijn; +Cc: netdev From: Frederik Deweerdt <frederik.deweerdt@gmail.com> > Sent: 02 June 2022 00:01 > > > > A socket must not be closed until all completion notifications have > > been received. > > > > Calling shutdown is an optional step. It may be sufficient to simply > > delay close. What happens if the process gets killed - eg by SIGSEGV? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario 2022-06-03 13:09 ` David Laight @ 2022-06-03 13:30 ` Willem de Bruijn 2022-06-06 7:58 ` David Laight 0 siblings, 1 reply; 7+ messages in thread From: Willem de Bruijn @ 2022-06-03 13:30 UTC (permalink / raw) To: David Laight; +Cc: Frederik Deweerdt, Willem de Bruijn, netdev On Fri, Jun 3, 2022 at 9:18 AM David Laight <David.Laight@aculab.com> wrote: > > From: Frederik Deweerdt <frederik.deweerdt@gmail.com> > > Sent: 02 June 2022 00:01 > > > > > > A socket must not be closed until all completion notifications have > > > been received. > > > > > > Calling shutdown is an optional step. It may be sufficient to simply > > > delay close. > > What happens if the process gets killed - eg by SIGSEGV? The skb frags hold independent references on the pages. Once skbs are freed on transmit completion or socket purge the pages are released if there are no other page references. Otherwise there is nothing zerocopy specific about closing TCP connections when a process crashes. ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario 2022-06-03 13:30 ` Willem de Bruijn @ 2022-06-06 7:58 ` David Laight 0 siblings, 0 replies; 7+ messages in thread From: David Laight @ 2022-06-06 7:58 UTC (permalink / raw) To: 'Willem de Bruijn'; +Cc: Frederik Deweerdt, netdev From: Willem de Bruijn > Sent: 03 June 2022 14:30 > > On Fri, Jun 3, 2022 at 9:18 AM David Laight <David.Laight@aculab.com> wrote: > > > > From: Frederik Deweerdt <frederik.deweerdt@gmail.com> > > > Sent: 02 June 2022 00:01 > > > > > > > > A socket must not be closed until all completion notifications have > > > > been received. > > > > > > > > Calling shutdown is an optional step. It may be sufficient to simply > > > > delay close. > > > > What happens if the process gets killed - eg by SIGSEGV? > > The skb frags hold independent references on the pages. Once skbs are > freed on transmit completion or socket purge the pages are released if > there are no other page references. > > Otherwise there is nothing zerocopy specific about closing TCP > connections when a process crashes. So the proposed text for the documentation is just wrong. Something that says that retransmissions can re-read the pages after the socket is closed would be more relevant. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-06-06 7:59 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-01 2:47 [PATCH] [doc] msg_zerocopy.rst: clarify the TCP shutdown scenario Frederik Deweerdt 2022-06-01 13:24 ` Willem de Bruijn 2022-06-01 23:01 ` Frederik Deweerdt 2022-06-02 0:29 ` Willem de Bruijn 2022-06-03 13:09 ` David Laight 2022-06-03 13:30 ` Willem de Bruijn 2022-06-06 7:58 ` David Laight
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.