Implement getdtablecount() to count open file descriptors.

[• rodrigc]

Summary

OpenBSD has this function implemented as a system call.
This implements getdtablecount() in terms of kinfo_getfile(3)
in libutil.

[mjg]

Is not this going to be too slow for practical use? This will do plenty of completely unnecessary work, including path name resolving etc.

Quick look at opensmtpd shows they use it before accept() and during connection teardown.

As such, I would expect it t get dog slow when actual traffic comes.

There was an attempt to implement this as a syscall last year made by hardenedbsd people. I see they ended up implemented something as a sysctl handle which seems like a reasonable idea. The code is not suitable for stealing though - it does a weird loop over the bitmap inspecting fields separately instead of doing population count, the handler is not marked mpsafe (I guess an omission), the sysctl does not have a static number and has a rather weird name.

That said, putting something under kern.proc.* alongside current handlers should do the trick. I can do that later.

lib/libopenbsd/getdtablecount.c

36 ↗

(On Diff #9953)

Should not this be in a separate header?

51 ↗

(On Diff #9953)

Is not this going to be too slow for practical use? This will do plenty of completely unnecessary work, including path name resolving etc.

Quick look at opensmtpd shows they use it before accept() and during connection teardown.

As such, I would expect it t get dog slow when actual traffic comes.

[• rodrigc]

lib/libopenbsd/getdtablecount.c

51 ↗

(On Diff #9953)

I went down this path, because that is what @bapt suggested to implement. Let's see if @bapt has any feedback.

[bapt]

lib/libopenbsd/getdtablecount.c

51 ↗

(On Diff #9953)

kinfo_getfile, is calling a sysctl, imho getdtablecount() should directly call the same sysctl, just to count

[• rodrigc]

@bapt: if I call the same sysctl, I will pretty much be duplicating the code in kinfo_getfile(): https://reviews.freebsd.org/diffusion/S/browse/head/lib/libutil/kinfo_getfile.c;290404$13
I can do that. Are you OK with that approach?

[mjg]

lib/libopenbsd/getdtablecount.c

51 ↗

(On Diff #9953)

All the hard work is done by the sysctl in question, kinfo_getfile plays little role here. Additional overhead resulting from said sysctl is prohitibtive.

I hacked up a separate sysctl: https://people.freebsd.org/~mjg/patches/kern_proc_nfds.diff

I'm too lazy to implement a nice userspace part.

Can be easily called with

int nfds;
 size_t len = sizeof(nfds);
 mib[0] = CTL_KERN;
 mib[1] = KERN_PROC;
 mib[2] = KERN_PROC_NFDS;
 mib[3] = 0;

 error = sysctl(mib, 4, &nfds, &len, NULL, 0);

i.e. the required 4th argument is 0.

[• rodrigc]

Implement getdtablecount() using sysctl().

[jhb]

I agree with @mjg. Fetching kinfo_file is quite heavyweight. Having a new KERN_PROC_NFDS (or NOPENFILES or something) would be prudent.

[• rodrigc]

@bapt are you OK with me implementing getdtablecount() in terms of @mjg's new sysctl?

[bapt]

Of course, that I why I reply to @mjg that I like his sysctl :)

lib/libopenbsd/getdtablecount.c

51 ↗

(On Diff #9953)

I do like this new sysctl and it will be useful for other use cases

[mjg]

I committed said sysctl in https://svnweb.freebsd.org/changeset/base/290473

Usage is shown in my previous comment.

[• rodrigc]

Implement getdtablecount() using new sysctl implemented by mjg.

[• rodrigc]

lib/libopenbsd/getdtablecount.c

37 ↗

(On Diff #10008)

I will implement a separate header, I just want to get some agreement on the implementation
first to make sure I am going down the right path.

[bapt]

I thnk you can cleanup a bit your headers but otherwise looks good

[• rodrigc]

@bapt: what should I do about the prototype for getdtablecount()? On OpenBSD, they put the prototype in <unistd.h>.
I was thinking of creating a header file libopenbsd.h and making it private to this
library, and putting it in there. Do you think I should do that, or do you have a better suggestion?

[bapt]

Depends what do you plan to use that for

is it something you want to publicly expose, in that case it should be added to libc? or is it something only for use in base in which case then you want it in a libopenbsd and yes you would need to have a dedicated header

[• rodrigc]

@bapt: For now, I need to expose the prototype somehow so that I can use it inside imsg code inside libopenbsd.
I can make a header internal to libopenbsd, or stick: `extern int getdtablecount(void);` in the imsg code to get things to compile.
Adding it to libc is doable, but I want to be conservative about adding a new symbol to libc. I've never done that before, and
am not familiar with all the things that need to be done when adding new stuff to libc in terms of symbol versioning, version bump, ports exp-run, etc.

[mjg]

There is already an example in lib/libnetbsd - create unistd.h in lib/libopenbsd (but i would prefer lib/libopenbsd/include), put the declaration in and #include_next <unistd.h>. see lib/libnetbsd/sys/time.h for an example.