> -----Original Message----- > From: Ricardo Biehl Pasquali [mailto:pasqualirb@xxxxxxxxx] > Sent: Saturday, October 20, 2018 8:35 PM > To: linux-man@xxxxxxxxxxxxxxx > Cc: Michael Kerrisk <mtk.manpages@xxxxxxxxx>; Keller, Jacob E > <jacob.e.keller@xxxxxxxxx> > Subject: [PATCH] socket.7: Add description of SO_SELECT_ERR_QUEUE > > Signed-off-by: Ricardo Biehl Pasquali <pasqualirb@xxxxxxxxx> > --- > man7/socket.7 | 27 +++++++++++++++++++++++---- > 1 file changed, 23 insertions(+), 4 deletions(-) > > I've commented some issues of error queue polling in > network mailing list: > > https://www.spinics.net/lists/netdev/msg529722.html > > Ricardo Biehl Pasquali wrote (2018-10-18): > > The commit 7d4c04fc170087119727 ("net: add option to enable > > error queue packets waking select") (2013-03-28) introduced > > SO_SELECT_ERR_QUEUE, which masks POLLPRI with POLLERR event > > return in some socket poll callbacks. > > > > POLLERR event issued with sock_queue_err_skb() did not wake > > up a poll when POLLERR is the only requested event because > > sk_data_ready() (sock_def_readable()) was used and it > > doesn't mask POLLERR in poll wake up: > > > > wake_up_interruptible_sync_poll(&wq->wait, > > EPOLLIN | EPOLLPRI | > > EPOLLRDNORM | EPOLLRDBAND); > > > > If POLLIN or POLLPRI are requested, for example, poll does > > wake up. > > > > POLLERR wakeup by requesting POLLPRI is possible without > > set SO_SELECT_ERR_QUEUE. All the option does is masking > > POLLPRI as a returned event before poll returns. poll > > would return anyway because of POLLERR. > > > > Also, the sentence "[...] enable software to wait on error > > queue packets without waking up for regular data on the > > socket." from the above commit is not true. > > > > A POLLIN event issued via sock_def_readable() wakes up > > threads waiting for POLLPRI, and vice versa. However, > > poll() does not return, sleeping again, as the requested > > events do not match events. > > > > The commit 6e5d58fdc9bedd0255a8 ("skbuff: Fix not waking > > applications when errors are enqueued") (2018-03-14) make > > POLLERR alone wake up poll. It replaces sk_data_ready() > > (sock_def_readable()) with sk_error_report() > > (sock_def_error_report()). This makes "POLLERR wake up by > > requesting POLLPRI" obsolete. > > > > Rationale: > > > > POLLIN-only and POLLERR-only wake up are useful when there > > is a receiving thread, a sending thread, and a thread that > > get transmit timestamps. The thread polling on POLLERR will > > not wake up when regular data arrives (POLLIN). The thread > > polling on POLLIN will not wake up when tx timestamps are > > ready (POLLERR). > > > > One solution is adding an option that disable POLLERR as > > requested event. This is in the Virtual File System > > subsystem, not in the network, though. > > > > This solves the problem of waking up other threads that > > not interested in error queue. Thus allowing a separate > > thread take care of error queue (useful for receiving > > transmit timestamps). > > > > However, this may not be a good solution as it depends on > > polling behavior. Thoughts? > > > > By the way, as the kernel development shouldn't break user > > space, SO_SELECT_ERR_QUEUE can become a compatibility > > option. > > I have tried to explain the effect of the option and the > context in which it was introduced (more or less the reason > why it was introduced). The POLLIN-POLLPRI mutual wake up > issue was not commented. > I think the patch makes sense, and the detailed outline of how we got to having the option, and why it probably isn't necessary anymore is very useful! I think the change in description is good as is. Thanks, Jake > Is it clear? Should a NOTE be written about the other > issues? > > Cheers! > > pasquali > > diff --git a/man7/socket.7 b/man7/socket.7 > index 478fa364c..1ba308198 100644 > --- a/man7/socket.7 > +++ b/man7/socket.7 > @@ -49,10 +49,6 @@ > .\" commit a8fc92778080c845eaadc369a0ecf5699a03bef0 > .\" Author: Pavel Emelyanov <xemul@xxxxxxxxxxxxx> > .\" > -.\" SO_SELECT_ERR_QUEUE (3.10) > -.\" commit 7d4c04fc170087119727119074e72445f2bb192b > -.\" Author: Keller, Jacob E <jacob.e.keller@xxxxxxxxx> > -.\" > .\" SO_MAX_PACING_RATE (3.13) > .\" commit 62748f32d501f5d3712a7c372bbb92abc7c62bc7 > .\" Author: Eric Dumazet <edumazet@xxxxxxxxxx> > @@ -869,6 +865,29 @@ Indicates that an unsigned 32-bit value ancillary message > (cmsg) > should be attached to received skbs indicating > the number of packets dropped by the socket since its creation. > .TP > +.BR SO_SELECT_ERR_QUEUE " (since Linux 3.10)" > +.\" commit 7d4c04fc170087119727119074e72445f2bb192b > +.\" Author: Keller, Jacob E <jacob.e.keller@xxxxxxxxx> > +Masks > +.B POLLPRI > +with > +.B POLLERR > +event return. This was created when > +.B POLLERR > +wake up depended on requesting > +.B POLLIN > +or > +.BR POLLPRI . > +After the commit 6e5d58fdc9bedd0255a8 ("skbuff: Fix not > +waking applications when errors are enqueued"), introduced > +in Linux 4.16, > +.B POLLERR > +wakes up normally. > +The option has no effect in wake up behavior, all it does > +is masking > +.B POLLPRI > +before poll returns. > +.TP > .B SO_SNDBUF > Sets or gets the maximum socket send buffer in bytes. > The kernel doubles this value (to allow space for bookkeeping overhead) > -- > 2.17.1
