Issue 6300 - Add kqueue support to slapd
Summary: Add kqueue support to slapd
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: 2.4.16
Hardware: All All
: --- normal
Target Milestone: 2.5.0
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-22 19:05 UTC by bduncan@apple.com
Modified: 2020-10-14 21:01 UTC (History)
0 users

See Also:


Attachments
slapd-workaround.patch (1.77 KB, text/plain)
2018-04-24 21:45 UTC, Quanah Gibson-Mount
Details

Note You need to log in before you can comment on or make changes to this issue.
Description bduncan@apple.com 2009-09-22 19:05:32 UTC
Full_Name: Bryan Duncan
Version: 2.4.16
OS: Mac OS X 10.6
URL: ftp://ftp.openldap.org/incoming/bryan-duncan-kqueue-090922.patch
Submission from: (NULL) (17.224.21.109)


Added support for using kqueue in slapd (for systems that support kqueue(2).
Comment 1 bduncan@apple.com 2009-09-22 19:22:58 UTC
Subject should have been: "PATCH - Added kqueue support to slapd" to  
make clear this ITS tracks a patch that has been submitted.

-- Bryan

Comment 2 Quanah Gibson-Mount 2009-09-22 20:02:16 UTC
--On Tuesday, September 22, 2009 7:23 PM +0000 bduncan@apple.com wrote:

> Subject should have been: "PATCH - Added kqueue support to slapd" to
> make clear this ITS tracks a patch that has been submitted.

What OS(es) has this kqueue support been tested with?  For example, the 
last time I looked at kqueue on OSX 10.5, it was very marginal, and missing 
most of the important pieces, which meant it wouldn't work for OpenLDAP or 
Postfix, etc.

--Quanah



--

Quanah Gibson-Mount
Principal Software Engineer
Zimbra, Inc
--------------------
Zimbra ::  the leader in open source messaging and collaboration

Comment 3 bduncan@apple.com 2009-09-22 21:40:54 UTC
On Sep 22, 2009, at 1:02 PM, Quanah Gibson-Mount wrote:

> --On Tuesday, September 22, 2009 7:23 PM +0000 bduncan@apple.com  
> wrote:
>
>> Subject should have been: "PATCH - Added kqueue support to slapd" to
>> make clear this ITS tracks a patch that has been submitted.
>
> What OS(es) has this kqueue support been tested with?  For example,  
> the last time I looked at kqueue on OSX 10.5, it was very marginal,  
> and missing most of the important pieces, which meant it wouldn't  
> work for OpenLDAP or Postfix, etc.

It was tested on OS X 10.6.  Should work on all BSDs; nothing OSX- 
specific about the kqueue usage in the patch.  As far as kqueue  
support in OS X 10.5, from what I see, most everything is there.   
Certainly all of the kqueue features in this patch are supported in OS  
X 10.5.

-- Bryan Duncan

Comment 4 Quanah Gibson-Mount 2009-11-11 17:04:35 UTC
--On Tuesday, September 22, 2009 2:40 PM -0700 Bryan Duncan 
<bduncan@apple.com> wrote:


> It was tested on OS X 10.6.  Should work on all BSDs; nothing
> OSX-specific about the kqueue usage in the patch.  As far as kqueue
> support in OS X 10.5, from what I see, most everything is there.
> Certainly all of the kqueue features in this patch are supported in OS X
> 10.5.

Hi Bryan,

The URL to the patch you submitted with this ITS doesn't work.  Can you 
please put the patch somewhere accessible and let us know where it is?

Thanks,
Quanah



--

Quanah Gibson-Mount
Principal Software Engineer
Zimbra, Inc
--------------------
Zimbra ::  the leader in open source messaging and collaboration

Comment 5 bduncan@apple.com 2009-11-11 17:36:51 UTC
On Nov 11, 2009, at 9:04 AM, Quanah Gibson-Mount wrote:

> --On Tuesday, September 22, 2009 2:40 PM -0700 Bryan Duncan <bduncan@apple.com> wrote:
> 
> 
>> It was tested on OS X 10.6.  Should work on all BSDs; nothing
>> OSX-specific about the kqueue usage in the patch.  As far as kqueue
>> support in OS X 10.5, from what I see, most everything is there.
>> Certainly all of the kqueue features in this patch are supported in OS X
>> 10.5.
> 
> Hi Bryan,
> 
> The URL to the patch you submitted with this ITS doesn't work.  Can you please put the patch somewhere accessible and let us know where it is?
> 
> Thanks,
> Quanah

Hi Quanah,

My apologies for the bad URL.

The new URL is: http://public.me.com/bryan.duncan/bryan-duncan.kqueue.090922.patch
If you use a web browser to get it, there are extra steps: select the file "bryan-duncan.kqueue.090922.patch" and click download.  ftp & curl can pull it down directly with no extra steps.

-- Bryan
Comment 6 Quanah Gibson-Mount 2009-11-11 17:45:59 UTC
--On Wednesday, November 11, 2009 9:36 AM -0800 Bryan Duncan 
<bduncan@apple.com> wrote:

>
> On Nov 11, 2009, at 9:04 AM, Quanah Gibson-Mount wrote:
>
>> --On Tuesday, September 22, 2009 2:40 PM -0700 Bryan Duncan
>> <bduncan@apple.com> wrote:
>>
>>
>>> It was tested on OS X 10.6.  Should work on all BSDs; nothing
>>> OSX-specific about the kqueue usage in the patch.  As far as kqueue
>>> support in OS X 10.5, from what I see, most everything is there.
>>> Certainly all of the kqueue features in this patch are supported in OS X
>>> 10.5.
>>
>> Hi Bryan,
>>
>> The URL to the patch you submitted with this ITS doesn't work.  Can you
>> please put the patch somewhere accessible and let us know where it is?
>>
>> Thanks,
>> Quanah
>
> Hi Quanah,
>
> My apologies for the bad URL.
>
> The new URL is:
> http://public.me.com/bryan.duncan/bryan-duncan.kqueue.090922.patch If you
> use a web browser to get it, there are extra steps: select the file
> "bryan-duncan.kqueue.090922.patch" and click download.  ftp & curl can
> pull it down directly with no extra steps.

Hi Bryan,

Thanks for the quick response.  Hopefully we can get this incorporated for 
OpenLDAP 2.5 series, and I'm interested in playing with it against 2.4 to 
see how things change when used with Mac.

Thanks!

--Quanah

--

Quanah Gibson-Mount
Principal Software Engineer
Zimbra, Inc
--------------------
Zimbra ::  the leader in open source messaging and collaboration

Comment 7 bduncan@apple.com 2009-11-11 19:29:42 UTC
On Nov 11, 2009, at 9:45 AM, Quanah Gibson-Mount wrote:

> --On Wednesday, November 11, 2009 9:36 AM -0800 Bryan Duncan <bduncan@apple.com> wrote:
> 
>> 
>> On Nov 11, 2009, at 9:04 AM, Quanah Gibson-Mount wrote:
>> 
>>> --On Tuesday, September 22, 2009 2:40 PM -0700 Bryan Duncan
>>> <bduncan@apple.com> wrote:
>>> 
>>> 
>>>> It was tested on OS X 10.6.  Should work on all BSDs; nothing
>>>> OSX-specific about the kqueue usage in the patch.  As far as kqueue
>>>> support in OS X 10.5, from what I see, most everything is there.
>>>> Certainly all of the kqueue features in this patch are supported in OS X
>>>> 10.5.
>>> 
>>> Hi Bryan,
>>> 
>>> The URL to the patch you submitted with this ITS doesn't work.  Can you
>>> please put the patch somewhere accessible and let us know where it is?
>>> 
>>> Thanks,
>>> Quanah
>> 
>> Hi Quanah,
>> 
>> My apologies for the bad URL.
>> 
>> The new URL is:
>> http://public.me.com/bryan.duncan/bryan-duncan.kqueue.090922.patch If you
>> use a web browser to get it, there are extra steps: select the file
>> "bryan-duncan.kqueue.090922.patch" and click download.  ftp & curl can
>> pull it down directly with no extra steps.
> 
> Hi Bryan,
> 
> Thanks for the quick response.  Hopefully we can get this incorporated for OpenLDAP 2.5 series, and I'm interested in playing with it against 2.4 to see how things change when used with Mac.
> 
> Thanks!
> 
> --Quanah


Awesome!  I look forward to seeing it OpenLDAP.  If there's anything I can do to help, please let me know.

Thanks. 
-- Bryan

Comment 8 Howard Chu 2009-11-23 02:43:58 UTC
bduncan@apple.com wrote:
> On Sep 22, 2009, at 1:02 PM, Quanah Gibson-Mount wrote:
>
>> --On Tuesday, September 22, 2009 7:23 PM +0000 bduncan@apple.com
>> wrote:
>>
>>> Subject should have been: "PATCH - Added kqueue support to slapd" to
>>> make clear this ITS tracks a patch that has been submitted.
>>
>> What OS(es) has this kqueue support been tested with?  For example,
>> the last time I looked at kqueue on OSX 10.5, it was very marginal,
>> and missing most of the important pieces, which meant it wouldn't
>> work for OpenLDAP or Postfix, etc.
>
> It was tested on OS X 10.6.  Should work on all BSDs; nothing OSX-
> specific about the kqueue usage in the patch.  As far as kqueue
> support in OS X 10.5, from what I see, most everything is there.
> Certainly all of the kqueue features in this patch are supported in OS
> X 10.5.

To expand on this: there are a lot of comments around the web that kqueue was 
fairly broken at least up to 10.5. E.g. this:

http://lists.schmorp.de/pipermail/libev/2008q4/000523.html

Since we also use a pipe for the wake_sds[] in our event loop, if kqueue only 
supports sockets, then it cannot be used reliably in slapd.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 9 Hallvard Furuseth 2009-11-23 21:11:26 UTC
moved from Incoming to Development
Comment 10 bduncan@apple.com 2009-12-03 20:41:10 UTC
On Nov 22, 2009, at 6:43 PM, Howard Chu wrote:

> bduncan@apple.com wrote:
>> On Sep 22, 2009, at 1:02 PM, Quanah Gibson-Mount wrote:
>> 
>>> --On Tuesday, September 22, 2009 7:23 PM +0000 bduncan@apple.com
>>> wrote:
>>> 
>>>> Subject should have been: "PATCH - Added kqueue support to slapd" to
>>>> make clear this ITS tracks a patch that has been submitted.
>>> 
>>> What OS(es) has this kqueue support been tested with?  For example,
>>> the last time I looked at kqueue on OSX 10.5, it was very marginal,
>>> and missing most of the important pieces, which meant it wouldn't
>>> work for OpenLDAP or Postfix, etc.
>> 
>> It was tested on OS X 10.6.  Should work on all BSDs; nothing OSX-
>> specific about the kqueue usage in the patch.  As far as kqueue
>> support in OS X 10.5, from what I see, most everything is there.
>> Certainly all of the kqueue features in this patch are supported in OS
>> X 10.5.
> 
> To expand on this: there are a lot of comments around the web that kqueue was fairly broken at least up to 10.5. E.g. this:
> 
> http://lists.schmorp.de/pipermail/libev/2008q4/000523.html

Perhaps I missed something in that thread, but the only specific complaint was regarding kqueue and stdin.  And it's true that OS X's kqueue didn't support EVFILT_{READ|WRITE} on *devices* (stdin, tty's, /dev/* ) until 10.6.  I don't believe OpenLDAP actually needs that feature, though.

> 
> Since we also use a pipe for the wake_sds[] in our event loop, if kqueue only supports sockets, then it cannot be used reliably in slapd.

The OS X kqueue has *always* supported EVFILT_{READ|WRITE} on sockets, pipes, fifos, files.  Same for kqueue in the BSDs.  (FWIW, I believe the BSDs have always supported devices in kqueue.)

I think everything OpenLDAP needs from kqueue has been, and is, supported by OS X and the BSDs.  If there is a specific kqueue filter or fd type that OpenLDAP needs and isn't supported, please let me know.

Although I don't think it's necessary, you could add a test to the configure script to ensure kqueue supports the types of fd's OpenLDAP needs by doing something like this:

    int fd; /* or int fds[2]; */
    /* do something to create an fd here:  open(2), pipe(2), socket(2), etc. */
    struct kevent ev;
    EV_SET(&ev, fd, EVFILT_READ, EV_ADD, 0, 0, 0);
    int kq = kqueue();
    if (kq < 0) {
        ac_have_kqueue = false;
    } else {
        struct timespec to = {0,1};  /* anything so that kevent will return */
        int ret = kevent(kq, &ev, 1, &ev, 1, &to);
        close(kq);
        ac_have_kqueue = (ret >= 0);
    }

If the fd isn't supported 'ret' should be -1 and errno should be ENOTSUP.

-- Bryan Duncan

Comment 11 Quanah Gibson-Mount 2017-09-08 21:53:25 UTC
changed notes
Comment 12 Quanah Gibson-Mount 2017-09-11 22:45:09 UTC
Hi Bryan,

Unfortunately this patch is no longer available with Apple's switch to 
iCloud.  Would you be able to upload it to the OpenLDAP FTP server?

Thanks,
Quanah

Comment 13 Quanah Gibson-Mount 2017-09-12 16:15:17 UTC
changed notes
Comment 14 Quanah Gibson-Mount 2017-09-13 20:06:18 UTC
Hi Bryan,

I managed to track down a copy of your patch, and uploaded it to the 
OpenLDAP FTP server as bryan-duncan-its6300.patch.  I will see about 
getting this updated for current OpenLDAP, for possible inclusion in 
OpenLDAP 2.5.

Regards,
Quanah

--On Monday, September 11, 2017 11:45 PM +0000 quanah@fast-mail.org wrote:

> Hi Bryan,
>
> Unfortunately this patch is no longer available with Apple's switch to
> iCloud.  Would you be able to upload it to the OpenLDAP FTP server?
>
> Thanks,
> Quanah
>
>
>



--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:
<http://www.symas.com>


Comment 15 Quanah Gibson-Mount 2017-09-13 20:10:06 UTC
changed notes
Comment 16 Quanah Gibson-Mount 2017-10-20 14:52:45 UTC
changed notes
changed state Open to Test
Comment 17 Quanah Gibson-Mount 2018-04-24 21:45:06 UTC
Date: Monday, October 23, 2017 12:43 AM -0700
From: Xin Li
To: Quanah Gibson-Mount

Hi Quanah,

I finally got some time to twiddle with OpenLDAP (again).  It looks like
the EBADF is expected, because fork(2) says:

           •   The child process has its own copy of the parent's
               descriptors, except for descriptors returned by
               kqueue(2), which are not inherited from the parent
               process.

And slapd does have a fork() after the initial kqueue(), which rendered
it invalid.

The kqueue() is part of SLAP_SOCK_INIT(), which is part of
slapd_daemon_init().  Then, after that, fork() happen in lutil_detach(),
and doing this hack would (incorrectly) make slapd to start:

diff --git a/libraries/liblutil/detach.c b/libraries/liblutil/detach.c
index c6464c038..f47d36548 100644
--- a/libraries/liblutil/detach.c
+++ b/libraries/liblutil/detach.c
@@ -73,7 +73,7 @@ lutil_detach( int debug, int do_close )
 #ifdef HAVE_THR
                        pid = fork1();
 #else
-                       pid = fork();
+                       pid = rfork(RFPROC);
 #endif
                        switch ( pid )
                        {

It is incorrect because the code in slapd/main.c seems to expect the
child to write a "1" to it before exiting with EXIT_SUCCESS() and
obviously if the two processes shares the same file table, the parent
would consider the start was failed because read() would fail.

I think the right fix would be to move the lutil_detach() to before
slapd_daemon_init(), see attachment, but it seems that some code after
the initial daemon initialization is still trying to output to stderr, etc.

What do you think?

Cheers,

--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:
<http://www.symas.com>
Comment 18 Quanah Gibson-Mount 2018-04-24 21:45:06 UTC
kqueue issues with slapd

--Quanah

------------ Forwarded Message ------------
Date: Monday, October 23, 2017 12:43 AM -0700
From: Xin Li <delphij@delphij.net>
To: Quanah Gibson-Mount <quanah@symas.com>, Xin LI <delphij@gmail.com>
Cc: d@delphij.net, Pietro Cerutti <gahr@gahr.ch>, Xin LI 
<delphij@freebsd.org>
Subject: Re: kqueue in OpenLDAP for FreeBSD

Hi, Quanah,

I finally got some time to twiddle with OpenLDAP (again).  It looks like
the EBADF is expected, because fork(2) says:

           •   The child process has its own copy of the parent's
               descriptors, except for descriptors returned by
               kqueue(2), which are not inherited from the parent
               process.

And slapd does have a fork() after the initial kqueue(), which rendered
it invalid.

The kqueue() is part of SLAP_SOCK_INIT(), which is part of
slapd_daemon_init().  Then, after that, fork() happen in lutil_detach(),
and doing this hack would (incorrectly) make slapd to start:

diff --git a/libraries/liblutil/detach.c b/libraries/liblutil/detach.c
index c6464c038..f47d36548 100644
--- a/libraries/liblutil/detach.c
+++ b/libraries/liblutil/detach.c
@@ -73,7 +73,7 @@ lutil_detach( int debug, int do_close )
 #ifdef HAVE_THR
                        pid = fork1();
 #else
-                       pid = fork();
+                       pid = rfork(RFPROC);
 #endif
                        switch ( pid )
                        {

It is incorrect because the code in slapd/main.c seems to expect the
child to write a "1" to it before exiting with EXIT_SUCCESS() and
obviously if the two processes shares the same file table, the parent
would consider the start was failed because read() would fail.

I think the right fix would be to move the lutil_detach() to before
slapd_daemon_init(), see attachment, but it seems that some code after
the initial daemon initialization is still trying to output to stderr, etc.

What do you think?

Cheers,

On 10/20/17 09:00, Quanah Gibson-Mount wrote:
> --On Friday, October 20, 2017 8:15 AM -0700 Quanah Gibson-Mount
> <quanah@symas.com> wrote:
>
>> Thanks, I'll play around with the build some.  I'm limiting slapd to as
>> minimal a build as possible with my configure:
>>
>> ./configure --without-tls --without-cyrus-sasl --enable-ldap
>> --enable-meta --enable-accesslog --enable-ppolicy --enable-rewrite
>> --enable-rwm --prefix=/tmp/q
>
>
> I've updated my configure to:
>
> CFLAGS="-O2 -pipe  -fstack-protector -DLDAP_DEPRECATED
> -fno-strict-aliasing" \
> ./configure \
> --with-threads=posix  --with-tls=openssl --disable-dependency-tracking \
> --enable-dynamic --without-cyrus-sasl \
> --localstatedir=/var/db  --enable-crypt  --enable-ldap \
> --enable-meta  --enable-rewrite  --enable-null \
> --enable-monitor --disable-seqmod --enable-syncprov \
> --enable-mdb --without-fetch \
> --prefix=/tmp/q
>
>
> And I still have no issues.  I'm unable to get the freebsd version of
> libtool to build shared libraries, so I can't test dynamic modules.
>
> --Quanah
>
>
> --
>
> Quanah Gibson-Mount
> Product Architect
> Symas Corporation
> Packaged, certified, and supported LDAP solutions powered by OpenLDAP:
> <http://www.symas.com>
>

---------- End Forwarded Message ----------



--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:
<http://www.symas.com>
Comment 19 Quanah Gibson-Mount 2018-04-24 21:45:06 UTC
Hi, Quanah,

I finally got some time to twiddle with OpenLDAP (again).  It looks like
the EBADF is expected, because fork(2) says:

           •   The child process has its own copy of the parent's
               descriptors, except for descriptors returned by
               kqueue(2), which are not inherited from the parent
               process.

And slapd does have a fork() after the initial kqueue(), which rendered
it invalid.

The kqueue() is part of SLAP_SOCK_INIT(), which is part of
slapd_daemon_init().  Then, after that, fork() happen in lutil_detach(),
and doing this hack would (incorrectly) make slapd to start:

diff --git a/libraries/liblutil/detach.c b/libraries/liblutil/detach.c
index c6464c038..f47d36548 100644
--- a/libraries/liblutil/detach.c
+++ b/libraries/liblutil/detach.c
@@ -73,7 +73,7 @@ lutil_detach( int debug, int do_close )
 #ifdef HAVE_THR
                        pid = fork1();
 #else
-                       pid = fork();
+                       pid = rfork(RFPROC);
 #endif
                        switch ( pid )
                        {

It is incorrect because the code in slapd/main.c seems to expect the
child to write a "1" to it before exiting with EXIT_SUCCESS() and
obviously if the two processes shares the same file table, the parent
would consider the start was failed because read() would fail.

I think the right fix would be to move the lutil_detach() to before
slapd_daemon_init(), see attachment, but it seems that some code after
the initial daemon initialization is still trying to output to stderr, etc.

What do you think?

Cheers,

On 10/20/17 09:00, Quanah Gibson-Mount wrote:
> --On Friday, October 20, 2017 8:15 AM -0700 Quanah Gibson-Mount
> <quanah@symas.com> wrote:
> 
>> Thanks, I'll play around with the build some.  I'm limiting slapd to as
>> minimal a build as possible with my configure:
>>
>> ./configure --without-tls --without-cyrus-sasl --enable-ldap
>> --enable-meta --enable-accesslog --enable-ppolicy --enable-rewrite
>> --enable-rwm --prefix=/tmp/q
> 
> 
> I've updated my configure to:
> 
> CFLAGS="-O2 -pipe  -fstack-protector -DLDAP_DEPRECATED
> -fno-strict-aliasing" \
> ./configure \
> --with-threads=posix  --with-tls=openssl --disable-dependency-tracking \
> --enable-dynamic --without-cyrus-sasl \
> --localstatedir=/var/db  --enable-crypt  --enable-ldap \
> --enable-meta  --enable-rewrite  --enable-null \
> --enable-monitor --disable-seqmod --enable-syncprov \
> --enable-mdb --without-fetch \
> --prefix=/tmp/q
> 
> 
> And I still have no issues.  I'm unable to get the freebsd version of
> libtool to build shared libraries, so I can't test dynamic modules.
> 
> --Quanah
> 
> 
> -- 
> 
> Quanah Gibson-Mount
> Product Architect
> Symas Corporation
> Packaged, certified, and supported LDAP solutions powered by OpenLDAP:
> <http://www.symas.com>
> 
Comment 20 OpenLDAP project 2019-01-15 21:47:07 UTC
Added to master
fixed in master
Comment 21 Howard Chu 2019-01-15 21:47:07 UTC
changed notes