Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fails on FreeBSD 13+ #7656

Closed
Johanhen opened this issue Mar 30, 2021 · 11 comments
Closed

Build fails on FreeBSD 13+ #7656

Johanhen opened this issue Mar 30, 2021 · 11 comments
Assignees

Comments

@Johanhen
Copy link

The build of traffic server 8.1.1 (and probably other versions as well fails on FreeBSD 13.x and newer.

As noted by Brian in the mailing list

FreeBSD 13 added eventfd(2) [1], which means that the check on HAVE_EVENTFD in EventNotify.cc [2] can no longer assume that if you have eventfd(2) you also have epoll.

  1. freebsd/freebsd-src@7a20282#diff-7ea785358f7ef658d52a5d985ea53f03c09cd0bfee8e6a7403063774d3dc58a8
  2. #ifdef HAVE_EVENTFD
@bgaff
Copy link
Member

bgaff commented Mar 30, 2021

The easiest fix:

sed -Ei 's/#ifdef HAVE_EVENTFD/#if defined(HAVE_EVENTFD) \&\& defined(TS_USE_EPOLL)' src/tscore/EventNotify.cc

@Johanhen
Copy link
Author

Unfortunately i still receive that error after the replacement off #ifdef HAVE_EVENTFD by #if defined(HAVE_EVENTFD) && defined(TS_USE_EPOLL)

libtool: compile: c++ -DHAVE_CONFIG_H -I. -I../../include -D_GLIBCXX_USE_C99 -D_GLIBCXX_USE_C99_MATH -D_GLIBCXX_USE_C99_MATH_TR1 -Dfreebsd -D_LARGEFILE64_SOURCE=1 -D_COMPILE64BIT_SOURCE=1 -D_REENTRANT -D__STDC_LIMIT_MACROS=1 -D__STDC_FORMAT_MACROS=1 -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/iocore/eventsystem -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/iocore/net -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/iocore/aio -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/iocore/hostdb -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/iocore/cache -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/iocore/utils -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/iocore/dns -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/include -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/include/records -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/lib -I/usr/local/include -I/usr/local/include/tcl8.6 -I/usr/ports/www/trafficserver/work/trafficserver-8.1.1/lib/yamlcpp/include -isystem /usr/local/include -D_GNU_SOURCE -I/usr/include -DOPENSSL_NO_SSL_INTERN -I/usr/local/include -std=c++17 -g -pipe -Wall -Wno-deprecated-declarations -Qunused-arguments -Wextra -Wno-ignored-qualifiers -Wno-unused-parameter -fno-strict-aliasing -Wno-invalid-offsetof -mcx16 -O2 -pipe -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -isystem /usr/local/include -c EventNotify.cc -fPIC -DPIC -o .libs/EventNotify.o
EventNotify.cc:37:10: fatal error: 'sys/epoll.h' file not found
#include <sys/epoll.h>
^~~~~~~~~~~~~

@bgaff
Copy link
Member

bgaff commented Mar 31, 2021

Sorry my bad the autoconf script always defines TS_USE_EPOLL, you need to do:

The easiest fix is:

sed -Ei 's/#ifdef HAVE_EVENTFD/#if defined(HAVE_EVENTFD) \&\& TS_USE_EPOLL == 1' src/tscore/EventNotify.cc

That should fix it, but that whole class is kinda ridiculous anyway. Using eventfd + epoll in this way is overkill, the ink_condwait methods should be preferred for many reasons as they solve the same problem:

  1. ink_condwait and friends will wrap into pthread_cond_(timed)wait which are as the name implies posix compliant.
  2. When using pthread_cond_wait In the case of a signal before a wait no syscall is ever required on most platforms, ie. linux uses futex(2) on contention but it will check the state in userspace before making a syscall.
  3. Using eventfd(2) in this way doesn't make sense as you're never acutally using the FD with anything other than this epoll fd.
  4. Even if you choose to use eventfd(2) in this way it should probably use poll(2) as it's posix compliant, whereas epoll is linux specific.

Eventfds make sense in situations where you have an existing pollfd / epollfd / etc where you want to tack on some event notification, when you're creating an eventfd to use only with poll/epoll/etc there usually is a better way.

My suggestion is: just yank out all of this eventfd / epoll code and make it use ink_cond_wait/ink_cond_timedwait and be done with it.

@Johanhen
Copy link
Author

Johanhen commented Mar 31, 2021 via email

@bgaff
Copy link
Member

bgaff commented Mar 31, 2021

Yah sorry you need to do the same for the header file:

sed -Ei 's/#ifdef HAVE_EVENTFD/#if defined(HAVE_EVENTFD) \&\& TS_USE_EPOLL == 1' src/tscore/EventNotify.cc
sed -Ei 's/#ifdef HAVE_EVENTFD/#if defined(HAVE_EVENTFD) \&\& TS_USE_EPOLL == 1' include/tscore/EventNotify.h

@Johanhen
Copy link
Author

Johanhen commented Mar 31, 2021 via email

@zwoop
Copy link
Contributor

zwoop commented Apr 7, 2021

Someone want to make a PR for this? :-)

@bgaff
Copy link
Member

bgaff commented Apr 7, 2021 via email

@diizzyy
Copy link

diizzyy commented Apr 21, 2021

@bgaff
Any progress regarding this issue?

@bgaff
Copy link
Member

bgaff commented Apr 22, 2021

Sending pull request right now.

@diizzyy
Copy link

diizzyy commented Apr 23, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants