From bc3a9e9e231524811b47f401169d49b470bf94c8 Mon Sep 17 00:00:00 2001 From: Dmitry Selyutin Date: Tue, 28 Aug 2018 23:45:29 +0300 Subject: [PATCH 1/3] fix naming for write evfilt callbacks --- src/linux/write.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/linux/write.c b/src/linux/write.c index 4c95a678..ae4395e5 100644 --- a/src/linux/write.c +++ b/src/linux/write.c @@ -31,7 +31,7 @@ #include "private.h" int -evfilt_socket_copyout(struct kevent *dst, struct knote *src, void *ptr) +evfilt_write_copyout(struct kevent *dst, struct knote *src, void *ptr) { struct epoll_event * const ev = (struct epoll_event *) ptr; @@ -58,7 +58,7 @@ evfilt_socket_copyout(struct kevent *dst, struct knote *src, void *ptr) } int -evfilt_socket_knote_create(struct filter *filt, struct knote *kn) +evfilt_write_knote_create(struct filter *filt, struct knote *kn) { struct epoll_event ev; @@ -84,7 +84,7 @@ evfilt_socket_knote_create(struct filter *filt, struct knote *kn) } int -evfilt_socket_knote_modify(struct filter *filt, struct knote *kn, +evfilt_write_knote_modify(struct filter *filt, struct knote *kn, const struct kevent *kev) { (void) filt; @@ -94,7 +94,7 @@ evfilt_socket_knote_modify(struct filter *filt, struct knote *kn, } int -evfilt_socket_knote_delete(struct filter *filt, struct knote *kn) +evfilt_write_knote_delete(struct filter *filt, struct knote *kn) { if (kn->kev.flags & EV_DISABLE) return (0); @@ -103,7 +103,7 @@ evfilt_socket_knote_delete(struct filter *filt, struct knote *kn) } int -evfilt_socket_knote_enable(struct filter *filt, struct knote *kn) +evfilt_write_knote_enable(struct filter *filt, struct knote *kn) { struct epoll_event ev; @@ -115,7 +115,7 @@ evfilt_socket_knote_enable(struct filter *filt, struct knote *kn) } int -evfilt_socket_knote_disable(struct filter *filt, struct knote *kn) +evfilt_write_knote_disable(struct filter *filt, struct knote *kn) { return epoll_update(EPOLL_CTL_DEL, filt, kn, NULL); } @@ -124,10 +124,10 @@ const struct filter evfilt_write = { EVFILT_WRITE, NULL, NULL, - evfilt_socket_copyout, - evfilt_socket_knote_create, - evfilt_socket_knote_modify, - evfilt_socket_knote_delete, - evfilt_socket_knote_enable, - evfilt_socket_knote_disable, + evfilt_write_copyout, + evfilt_write_knote_create, + evfilt_write_knote_modify, + evfilt_write_knote_delete, + evfilt_write_knote_enable, + evfilt_write_knote_disable, }; From 1783d3083fe8f31b87d4b069b6361524b233061d Mon Sep 17 00:00:00 2001 From: Dmitry Selyutin Date: Wed, 29 Aug 2018 21:45:30 +0300 Subject: [PATCH 2/3] knote: support more kn_flags and socket errors It is possible to determine the actual socket error via SO_ERROR socket option. However, this system call does not work for descriptors other than sockets, and it'd be a total waste of time to call it just to get ENOTSOCK. In order to avoid the cost of the context switch, let's store some additional information from the times the knote was created. This change mostly deals with the Linux code only; likely the same changes can be applied to Windows as well. However, this issue is to be addressed by a separate commit. --- src/common/kevent.c | 2 +- src/common/private.h | 17 +++++-- src/linux/platform.c | 101 +++++++++++++++++++++++++++++++---------- src/linux/read.c | 25 ++++++---- src/linux/write.c | 16 +++++-- src/windows/platform.c | 8 ++-- src/windows/read.c | 2 +- 7 files changed, 123 insertions(+), 48 deletions(-) diff --git a/src/common/kevent.c b/src/common/kevent.c index fe808f18..21bf2341 100644 --- a/src/common/kevent.c +++ b/src/common/kevent.c @@ -144,7 +144,7 @@ kevent_copyin_one(struct kqueue *kq, const struct kevent *src) memcpy(&kn->kev, src, sizeof(kn->kev)); kn->kev.flags &= ~EV_ENABLE; kn->kev.flags |= EV_ADD;//FIXME why? - kn->kn_kq = kq; + kn->kn_kq = kq; assert(filt->kn_create); if (filt->kn_create(filt, kn) < 0) { knote_release(kn); diff --git a/src/common/private.h b/src/common/private.h index 39ffe674..89e2281c 100644 --- a/src/common/private.h +++ b/src/common/private.h @@ -66,14 +66,21 @@ struct eventfd { /* * Flags used by knote->kn_flags */ -#define KNFL_PASSIVE_SOCKET (0x01) /* Socket is in listen(2) mode */ -#define KNFL_REGULAR_FILE (0x02) /* File descriptor is a regular file */ -#define KNFL_STREAM_SOCKET (0x03) /* File descriptor is a stream socket */ -#define KNFL_KNOTE_DELETED (0x10) /* The knote object is no longer valid */ +#define KNFL_FILE (1U << 0U) +#define KNFL_PIPE (1U << 1U) +#define KNFL_SOCKET (1U << 2U) +#define KNFL_BLOCKDEV (1U << 3U) +#define KNFL_CHARDEV (1U << 4U) +#define KNFL_SOCKET_PASSIVE (KNFL_SOCKET | (1U << 5U)) +#define KNFL_SOCKET_STREAM (KNFL_SOCKET | (1U << 6U)) +#define KNFL_SOCKET_DGRAM (KNFL_SOCKET | (1U << 7U)) +#define KNFL_SOCKET_RDM (KNFL_SOCKET | (1U << 8U)) +#define KNFL_SOCKET_SEQPACKET (KNFL_SOCKET | (1U << 9U)) +#define KNFL_KNOTE_DELETED (1U << 31U) struct knote { struct kevent kev; - int kn_flags; + unsigned int kn_flags; union { /* OLD */ int pfd; /* Used by timerfd */ diff --git a/src/linux/platform.c b/src/linux/platform.c index fec7655b..7067933b 100644 --- a/src/linux/platform.c +++ b/src/linux/platform.c @@ -552,32 +552,95 @@ linux_get_descriptor_type(struct knote *kn) { socklen_t slen; struct stat sb; - int i, lsock, stype; + int ret, lsock, stype; socklen_t out_len; + const int fd = (int)kn->kev.ident; /* - * Test if the descriptor is a socket. + * Determine the actual descriptor type. */ - if (fstat(kn->kev.ident, &sb) < 0) { + if (fstat(fd, &sb) < 0) { dbg_perror("fstat(2)"); return (-1); } - if (S_ISREG(sb.st_mode)) { - kn->kn_flags |= KNFL_REGULAR_FILE; - dbg_printf("fd %d is a regular file\n", (int)kn->kev.ident); - return (0); + switch (sb.st_mode & S_IFMT) { + case S_IFREG: + dbg_printf("fd %d is a regular file\n", fd); + kn->kn_flags |= KNFL_FILE; + break; + + case S_IFIFO: + dbg_printf("fd %d is a pipe\n", fd); + kn->kn_flags |= KNFL_PIPE; + break; + + case S_IFBLK: + dbg_printf("fd %d is a block device\n", fd); + kn->kn_flags |= KNFL_BLOCKDEV; + break; + + case S_IFCHR: + dbg_printf("fd %d is a character device\n", fd); + kn->kn_flags |= KNFL_CHARDEV; + break; + + case S_IFSOCK: + dbg_printf("fd %d is a socket\n", fd); + kn->kn_flags |= KNFL_SOCKET; + break; + + default: + errno = EBADF; + dbg_perror("unknown fd type"); + return -1; } /* * Test if the socket is active or passive. */ - if (! S_ISSOCK(sb.st_mode)) + if (!S_ISSOCK(sb.st_mode)) return (0); + /* + * Determine socket type. + */ + slen = sizeof(stype); + stype = 0; + ret = getsockopt(fd, SOL_SOCKET, SO_TYPE, &stype, &slen); + if (ret < 0) { + dbg_perror("getsockopt(3)"); + return (-1); + } + switch (stype) { + case SOCK_STREAM: + dbg_printf("fd %d is a stream socket\n", fd); + kn->kn_flags |= KNFL_SOCKET_STREAM; + break; + + case SOCK_DGRAM: + dbg_printf("fd %d is a datagram socket\n", fd); + kn->kn_flags |= KNFL_SOCKET_DGRAM; + break; + + case SOCK_RDM: + dbg_printf("fd %d is a reliable datagram socket\n", fd); + kn->kn_flags |= KNFL_SOCKET_RDM; + break; + + case SOCK_SEQPACKET: + dbg_printf("fd %d is a sequenced and reliable datagram socket\n", fd); + kn->kn_flags |= KNFL_SOCKET_SEQPACKET; + break; + + default: + errno = EBADF; + dbg_perror("unknown socket type"); + return (-1); + } slen = sizeof(lsock); lsock = 0; - i = getsockopt(kn->kev.ident, SOL_SOCKET, SO_ACCEPTCONN, (char *) &lsock, &slen); - if (i < 0) { + ret = getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &lsock, &slen); + if (ret < 0) { switch (errno) { case ENOTSOCK: /* same as lsock = 0 */ break; @@ -587,7 +650,7 @@ linux_get_descriptor_type(struct knote *kn) } } else { if (lsock) - kn->kn_flags |= KNFL_PASSIVE_SOCKET; + kn->kn_flags |= KNFL_SOCKET_PASSIVE; } /* @@ -597,8 +660,8 @@ linux_get_descriptor_type(struct knote *kn) * Looking at SO_GET_FILTER is a good way of doing this. */ out_len = 0; - i = getsockopt(kn->kev.ident, SOL_SOCKET, SO_GET_FILTER, NULL, &out_len); - if (i < 0) { + ret = getsockopt(fd, SOL_SOCKET, SO_GET_FILTER, NULL, &out_len); + if (ret < 0) { switch (errno) { case ENOTSOCK: /* same as lsock = 0 */ break; @@ -608,18 +671,8 @@ linux_get_descriptor_type(struct knote *kn) } } else { if (out_len) - kn->kn_flags |= KNFL_PASSIVE_SOCKET; - } - - slen = sizeof(stype); - stype = 0; - i = getsockopt(kn->kev.ident, SOL_SOCKET, SO_TYPE, (char *) &lsock, &slen); - if (i < 0) { - dbg_perror("getsockopt(3)"); - return (-1); + kn->kn_flags |= KNFL_SOCKET_PASSIVE; } - if (stype == SOCK_STREAM) - kn->kn_flags |= KNFL_STREAM_SOCKET; return (0); } diff --git a/src/linux/read.c b/src/linux/read.c index 125aaa8a..efb71224 100644 --- a/src/linux/read.c +++ b/src/linux/read.c @@ -56,10 +56,13 @@ get_eof_offset(int fd) int evfilt_read_copyout(struct kevent *dst, struct knote *src, void *ptr) { + int ret; + int serr; + socklen_t slen = sizeof(serr); struct epoll_event * const ev = (struct epoll_event *) ptr; /* Special case: for regular files, return the offset from current position to end of file */ - if (src->kn_flags & KNFL_REGULAR_FILE) { + if (src->kn_flags & KNFL_FILE) { memcpy(dst, &src->kev, sizeof(*dst)); dst->data = get_eof_offset(src->kev.ident); @@ -113,10 +116,14 @@ evfilt_read_copyout(struct kevent *dst, struct knote *src, void *ptr) if (ev->events & EPOLLHUP) dst->flags |= EV_EOF; #endif - if (ev->events & EPOLLERR) - dst->fflags = 1; /* FIXME: Return the actual socket error */ + if (ev->events & EPOLLERR) { + if (src->kn_flags & KNFL_SOCKET) { + ret = getsockopt(src->kev.ident, SOL_SOCKET, SO_ERROR, &serr, &slen); + dst->fflags = ((ret < 0) ? errno : serr); + } else { dst->fflags = EIO; } + } - if (src->kn_flags & KNFL_PASSIVE_SOCKET) { + if (src->kn_flags & KNFL_SOCKET_PASSIVE) { /* On return, data contains the length of the socket backlog. This is not available under Linux. */ @@ -132,7 +139,7 @@ evfilt_read_copyout(struct kevent *dst, struct knote *src, void *ptr) dst->data = 0; } else { dst->data = i; - if (dst->data == 0 && src->kn_flags & KNFL_STREAM_SOCKET) + if (dst->data == 0 && src->kn_flags & KNFL_SOCKET_STREAM) dst->flags |= EV_EOF; } } @@ -164,7 +171,7 @@ evfilt_read_knote_create(struct filter *filt, struct knote *kn) ev.data.ptr = kn; /* Special case: for regular files, add a surrogate eventfd that is always readable */ - if (kn->kn_flags & KNFL_REGULAR_FILE) { + if (kn->kn_flags & KNFL_FILE) { int evfd; kn->kn_epollfd = filter_epfd(filt); @@ -210,7 +217,7 @@ evfilt_read_knote_delete(struct filter *filt, struct knote *kn) if (kn->kev.flags & EV_DISABLE) return (0); - if ((kn->kn_flags & KNFL_REGULAR_FILE) && (kn->kdata.kn_eventfd != -1)) { + if ((kn->kn_flags & KNFL_FILE) && (kn->kdata.kn_eventfd != -1)) { if (kn->kn_registered && epoll_ctl(kn->kn_epollfd, EPOLL_CTL_DEL, kn->kdata.kn_eventfd, NULL) < 0) { dbg_perror("epoll_ctl(2)"); return (-1); @@ -236,7 +243,7 @@ evfilt_read_knote_enable(struct filter *filt, struct knote *kn) ev.events = kn->data.events; ev.data.ptr = kn; - if (kn->kn_flags & KNFL_REGULAR_FILE) { + if (kn->kn_flags & KNFL_FILE) { if (epoll_ctl(kn->kn_epollfd, EPOLL_CTL_ADD, kn->kdata.kn_eventfd, &ev) < 0) { dbg_perror("epoll_ctl(2)"); return (-1); @@ -254,7 +261,7 @@ evfilt_read_knote_enable(struct filter *filt, struct knote *kn) int evfilt_read_knote_disable(struct filter *filt, struct knote *kn) { - if (kn->kn_flags & KNFL_REGULAR_FILE) { + if (kn->kn_flags & KNFL_FILE) { if (epoll_ctl(kn->kn_epollfd, EPOLL_CTL_DEL, kn->kdata.kn_eventfd, NULL) < 0) { dbg_perror("epoll_ctl(2)"); return (-1); diff --git a/src/linux/write.c b/src/linux/write.c index ae4395e5..3305ee01 100644 --- a/src/linux/write.c +++ b/src/linux/write.c @@ -33,6 +33,9 @@ int evfilt_write_copyout(struct kevent *dst, struct knote *src, void *ptr) { + int ret; + int serr; + socklen_t slen = sizeof(serr); struct epoll_event * const ev = (struct epoll_event *) ptr; epoll_event_dump(ev); @@ -44,8 +47,12 @@ evfilt_write_copyout(struct kevent *dst, struct knote *src, void *ptr) if (ev->events & EPOLLHUP) dst->flags |= EV_EOF; #endif - if (ev->events & EPOLLERR) - dst->fflags = 1; /* FIXME: Return the actual socket error */ + if (ev->events & EPOLLERR) { + if (src->kn_flags & KNFL_SOCKET) { + ret = getsockopt(src->kev.ident, SOL_SOCKET, SO_ERROR, &serr, &slen); + dst->fflags = ((ret < 0) ? errno : serr); + } else { dst->fflags = EIO; } + } /* On return, data contains the the amount of space remaining in the write buffer */ if (ioctl(dst->ident, SIOCOUTQ, &dst->data) < 0) { @@ -65,9 +72,10 @@ evfilt_write_knote_create(struct filter *filt, struct knote *kn) if (linux_get_descriptor_type(kn) < 0) return (-1); - /* TODO: return EBADF? */ - if (kn->kn_flags & KNFL_REGULAR_FILE) + if (kn->kn_flags & KNFL_FILE) { + errno = EBADF; return (-1); + } /* Convert the kevent into an epoll_event */ kn->data.events = EPOLLOUT; diff --git a/src/windows/platform.c b/src/windows/platform.c index 849b3c36..b09e108b 100644 --- a/src/windows/platform.c +++ b/src/windows/platform.c @@ -197,24 +197,24 @@ windows_get_descriptor_type(struct knote *kn) lsock = 0; i = getsockopt(kn->kev.ident, SOL_SOCKET, SO_ACCEPTCONN, (char *)&lsock, &slen); if (i == 0 && lsock) - kn->kn_flags |= KNFL_PASSIVE_SOCKET; + kn->kn_flags |= KNFL_SOCKET_PASSIVE; slen = sizeof(stype); stype = 0; - i = getsockopt(kn->kev.ident, SOL_SOCKET, SO_TYPE, (char *) &lsock, &slen); + i = getsockopt(kn->kev.ident, SOL_SOCKET, SO_TYPE, (char *)&stype, &slen); if (i < 0) { dbg_perror("getsockopt(3)"); return (-1); } if (stype == SOCK_STREAM) - kn->kn_flags |= KNFL_STREAM_SOCKET; + kn->kn_flags |= KNFL_SOCKET_STREAM; break; } default: { struct stat sb; if (fstat((int)kn->kev.ident, &sb) == 0) { dbg_printf("HANDLE %d appears to be a regular file", kn->kev.ident); - kn->kn_flags |= KNFL_REGULAR_FILE; + kn->kn_flags |= KNFL_FILE; } } } diff --git a/src/windows/read.c b/src/windows/read.c index 2b32e597..07b19c12 100644 --- a/src/windows/read.c +++ b/src/windows/read.c @@ -94,7 +94,7 @@ evfilt_read_copyout(struct kevent *dst, struct knote *src, void *ptr) //struct event_buf * const ev = (struct event_buf *) ptr; /* TODO: handle regular files - if (src->flags & KNFL_REGULAR_FILE) { ... } */ + if (src->flags & KNFL_FILE) { ... } */ memcpy(dst, &src->kev, sizeof(*dst)); if (src->kn_flags & KNFL_PASSIVE_SOCKET) { From 6d9638cdec5ed63981a4a2cb96185d7b71e24c63 Mon Sep 17 00:00:00 2001 From: Dmitry Selyutin Date: Thu, 30 Aug 2018 00:52:21 +0300 Subject: [PATCH 3/3] knote: rethink KNFL_SOCKET flag --- src/common/private.h | 19 +++++++++++-------- src/linux/platform.c | 3 +-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/common/private.h b/src/common/private.h index 89e2281c..6939097b 100644 --- a/src/common/private.h +++ b/src/common/private.h @@ -68,15 +68,18 @@ struct eventfd { */ #define KNFL_FILE (1U << 0U) #define KNFL_PIPE (1U << 1U) -#define KNFL_SOCKET (1U << 2U) -#define KNFL_BLOCKDEV (1U << 3U) -#define KNFL_CHARDEV (1U << 4U) -#define KNFL_SOCKET_PASSIVE (KNFL_SOCKET | (1U << 5U)) -#define KNFL_SOCKET_STREAM (KNFL_SOCKET | (1U << 6U)) -#define KNFL_SOCKET_DGRAM (KNFL_SOCKET | (1U << 7U)) -#define KNFL_SOCKET_RDM (KNFL_SOCKET | (1U << 8U)) -#define KNFL_SOCKET_SEQPACKET (KNFL_SOCKET | (1U << 9U)) +#define KNFL_BLOCKDEV (1U << 2U) +#define KNFL_CHARDEV (1U << 3U) +#define KNFL_SOCKET_PASSIVE (1U << 4U) +#define KNFL_SOCKET_STREAM (1U << 5U) +#define KNFL_SOCKET_DGRAM (1U << 6U) +#define KNFL_SOCKET_RDM (1U << 7U) +#define KNFL_SOCKET_SEQPACKET (1U << 8U) #define KNFL_KNOTE_DELETED (1U << 31U) +#define KNFL_SOCKET (KNFL_SOCKET_STREAM |\ + KNFL_SOCKET_DGRAM |\ + KNFL_SOCKET_RDM |\ + KNFL_SOCKET_SEQPACKET) struct knote { struct kevent kev; diff --git a/src/linux/platform.c b/src/linux/platform.c index 7067933b..dd335b03 100644 --- a/src/linux/platform.c +++ b/src/linux/platform.c @@ -586,8 +586,7 @@ linux_get_descriptor_type(struct knote *kn) case S_IFSOCK: dbg_printf("fd %d is a socket\n", fd); - kn->kn_flags |= KNFL_SOCKET; - break; + break; /* deferred type determination */ default: errno = EBADF;