Bug 240590 - Linuxulator: EPOLLONESHOT is broken
Summary: Linuxulator: EPOLLONESHOT is broken
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.0-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Vladimir Kondratyev
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2019-09-15 07:46 UTC by Alex S
Modified: 2019-12-03 23:49 UTC (History)
4 users (show)

See Also:
koobs: mfc-stable12?
koobs: mfc-stable11?


Attachments
EPOLLONESHOT.patch (1.45 KB, patch)
2019-09-22 01:56 UTC, Vladimir Kondratyev
no flags Details | Diff
EPOLLHUP.patch (15.26 KB, patch)
2019-09-28 12:39 UTC, Vladimir Kondratyev
no flags Details | Diff
EPOLLNUL.patch (6.50 KB, patch)
2019-09-28 23:56 UTC, Vladimir Kondratyev
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex S 2019-09-15 07:46:38 UTC
Here's a reproducer:

#include <assert.h>
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/epoll.h>

int main() {

  int epfd = epoll_create(8);
  assert(epfd);

  struct epoll_event event = {
    .events = EPOLLOUT | EPOLLONESHOT,
    .data   = 0
  };

  int err1 = epoll_ctl(epfd, EPOLL_CTL_ADD, STDIN_FILENO, &event);
  assert(err1 == 0);

  struct epoll_event events[1];
  int n = epoll_wait(epfd, events, 1, -1);
  assert(n > 0);

  int err2 = epoll_ctl(epfd, EPOLL_CTL_MOD, STDIN_FILENO, &event);
  if (err2 == 0) {
    printf("ok\n");
  } else {
    perror("EPOLL_CTL_MOD failed");
  }

  return 0;
}

This program runs fine on Linux, while on FreeBSD it prints "EPOLL_CTL_MOD failed: No such file or directory". The man page on kevent says about EV_ONESHOT "After the user retrieves the event from the kqueue, it is deleted", so presumably that's why.
Comment 1 Alex S 2019-09-15 08:10:06 UTC
(Forgot to replace STDIN_FILENO with STDOUT_FILENO, but that doesn't really matter. Anything will do.)

The actual real life use case here is the Linux Steam client, which uses the same exact pattern for monitoring opened sockets (at least since mid 2017). Right now it is impossible to log in to Steam without some kind of workaround (https://github.com/shkhln/linuxulator-steam-utils/blob/5ec9e681e97e0c797202f058210602c71e17e009/src/steamfix.c#L9) for this issue.
Comment 2 Alex S 2019-09-16 10:52:18 UTC
Found that for EPOLL_CTL_ADD/EPOLL_CTL_MOD operations Steam also tends to occasionally set event mask to zero, which is apparently a valid value and should be interpreted as EPOLLERR | EPOLLHUP, or, rather, per epoll_ctl man page everything gets subscribed to those events in addition to what was explicitly requested.
Comment 3 Vladimir Kondratyev freebsd_committer freebsd_triage 2019-09-22 01:56:15 UTC
Created attachment 207708 [details]
EPOLLONESHOT.patch

You can try attached patch.
Comment 4 Alex S 2019-09-22 14:17:11 UTC
(In reply to Vladimir Kondratyev from comment #3)

That patch is totally on point with regard to the comment #0, although Steam still requires a fix for the comment #2 as well.

> Mon Dec 25 20:32:19 2017 +0300
Was this already uploaded somewhere?
Comment 5 Vladimir Kondratyev freebsd_committer freebsd_triage 2019-09-28 12:39:18 UTC
Created attachment 207916 [details]
EPOLLHUP.patch

> Was this already uploaded somewhere?
Try EPOLLHUP.patch
Comment 6 Vladimir Kondratyev freebsd_committer freebsd_triage 2019-09-28 23:56:44 UTC
Created attachment 207926 [details]
EPOLLNUL.patch

This patch just allows NUL event mask (requires EPOLLONESHOT.patch applied), no EPOLLHUP on remote close or bidirectional shutdown.
EPOLLHUP support in previous patch is not 100% correct, so I created a simplified version.
It might make sense to start with it.
Comment 7 Alex S 2019-09-29 18:23:36 UTC
(In reply to Vladimir Kondratyev from comment #6)

EPOLLONESHOT.patch + EPOLLNUL.patch works for me.

> EPOLLHUP support in previous patch is not 100% correct, so I created a simplified version.
To be clear, EPOLLERR and EPOLLHUP are only mentioned as an explanation why zero is a valid value for event mask in the first place. I have absolutely zero ability to verify whether Steam is doing anything with these events. I don't have any reason to think they are required for Steam either.
Comment 8 commit-hook freebsd_committer freebsd_triage 2019-11-24 20:42:38 UTC
A commit references this bug:

Author: wulf
Date: Sun Nov 24 20:41:47 UTC 2019
New revision: 355065
URL: https://svnweb.freebsd.org/changeset/base/355065

Log:
  Linux epoll: Don't deregister file descriptor after EPOLLONESHOT is fired

  Linux epoll does not remove descriptor after one-shot event has been triggered.
  Set EV_DISPATCH kqueue flag rather then EV_ONESHOT to get the same behavior.

  Required by Linux Steam client.

  PR:		240590
  Reported by:	Alex S <iwtcex@gmail.com>
  Reviewed by:	emaste, imp
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D22513

Changes:
  head/sys/compat/linux/linux_event.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2019-11-24 20:48:40 UTC
A commit references this bug:

Author: wulf
Date: Sun Nov 24 20:47:40 UTC 2019
New revision: 355067
URL: https://svnweb.freebsd.org/changeset/base/355067

Log:
  Linux epoll: Register events with zero event mask

  Such an events are legal and should be interpreted as EPOLLERR | EPOLLHUP.
  Register a disabled kqueue event in that case as we do not support EPOLLHUP yet.

  Required by Linux Steam client.

  PR:		240590
  Reported by:	Alex S <iwtcex@gmail.com>
  Reviewed by:	emaste
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D22516

Changes:
  head/sys/compat/linux/linux_event.c
  head/sys/compat/linux/linux_event.h
Comment 10 commit-hook freebsd_committer freebsd_triage 2019-12-03 23:12:20 UTC
A commit references this bug:

Author: wulf
Date: Tue Dec  3 23:11:41 UTC 2019
New revision: 355372
URL: https://svnweb.freebsd.org/changeset/base/355372

Log:
  MFC r355065 - r355068: Linux epoll improvements.

  r355065:
  Linux epoll: Don't deregister file descriptor after EPOLLONESHOT is fired

  Linux epoll does not remove descriptor after one-shot event has been triggered.
  Set EV_DISPATCH kqueue flag rather then EV_ONESHOT to get the same behavior.

  Required by Linux Steam client.

  PR:		240590
  Reported by:	Alex S <iwtcex@gmail.com>
  Reviewed by:	emaste, imp
  Differential Revision:	https://reviews.freebsd.org/D22513

  r355066:
  Linux epoll: Check both read and write kqueue events existence in EPOLL_CTL_ADD

  Linux epoll EPOLL_CTL_ADD op handler should always check registration
  of both EVFILT_READ and EVFILT_WRITE kevents to deceide if supplied
  file descriptor fd is already registered with epoll instance.

  Reviewed by:	emaste
  Differential Revision:	https://reviews.freebsd.org/D22515

  r355067:
  Linux epoll: Register events with zero event mask

  Such an events are legal and should be interpreted as EPOLLERR | EPOLLHUP.
  Register a disabled kqueue event in that case as we do not support EPOLLHUP yet.

  Required by Linux Steam client.

  PR:		240590
  Reported by:	Alex S <iwtcex@gmail.com>
  Reviewed by:	emaste
  Differential Revision:	https://reviews.freebsd.org/D22516

  r355068:
  Linux epoll: Allow passing of any negative timeout value to epoll_wait

  Linux epoll allow passing of any negative timeout value to epoll_wait()
  to cause unbound blocking

  Reviewed by:	emaste
  Differential Revision:	https://reviews.freebsd.org/D22517

Changes:
_U  stable/12/
  stable/12/sys/compat/linux/linux_event.c
  stable/12/sys/compat/linux/linux_event.h
Comment 11 commit-hook freebsd_committer freebsd_triage 2019-12-03 23:12:22 UTC
A commit references this bug:

Author: wulf
Date: Tue Dec  3 23:11:41 UTC 2019
New revision: 355372
URL: https://svnweb.freebsd.org/changeset/base/355372

Log:
  MFC r355065 - r355068: Linux epoll improvements.

  r355065:
  Linux epoll: Don't deregister file descriptor after EPOLLONESHOT is fired

  Linux epoll does not remove descriptor after one-shot event has been triggered.
  Set EV_DISPATCH kqueue flag rather then EV_ONESHOT to get the same behavior.

  Required by Linux Steam client.

  PR:		240590
  Reported by:	Alex S <iwtcex@gmail.com>
  Reviewed by:	emaste, imp
  Differential Revision:	https://reviews.freebsd.org/D22513

  r355066:
  Linux epoll: Check both read and write kqueue events existence in EPOLL_CTL_ADD

  Linux epoll EPOLL_CTL_ADD op handler should always check registration
  of both EVFILT_READ and EVFILT_WRITE kevents to deceide if supplied
  file descriptor fd is already registered with epoll instance.

  Reviewed by:	emaste
  Differential Revision:	https://reviews.freebsd.org/D22515

  r355067:
  Linux epoll: Register events with zero event mask

  Such an events are legal and should be interpreted as EPOLLERR | EPOLLHUP.
  Register a disabled kqueue event in that case as we do not support EPOLLHUP yet.

  Required by Linux Steam client.

  PR:		240590
  Reported by:	Alex S <iwtcex@gmail.com>
  Reviewed by:	emaste
  Differential Revision:	https://reviews.freebsd.org/D22516

  r355068:
  Linux epoll: Allow passing of any negative timeout value to epoll_wait

  Linux epoll allow passing of any negative timeout value to epoll_wait()
  to cause unbound blocking

  Reviewed by:	emaste
  Differential Revision:	https://reviews.freebsd.org/D22517

Changes:
_U  stable/12/
  stable/12/sys/compat/linux/linux_event.c
  stable/12/sys/compat/linux/linux_event.h