Bug 498096 - 2 max file patches for xinetd
2 max file patches for xinetd
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: xinetd (Show other bugs)
10
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jan Zeleny
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 527311 639447
  Show dependency treegraph
 
Reported: 2009-04-28 16:42 EDT by James M. Leddy
Modified: 2014-08-11 01:42 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 527311 (view as bug list)
Environment:
Last Closed: 2009-10-20 09:58:52 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
patch allowing fd limits in xinetd.conf (4.86 KB, patch)
2009-04-28 16:42 EDT, James M. Leddy
no flags Details | Diff
better accounting mechanism for select() (10.94 KB, patch)
2009-04-28 17:06 EDT, James M. Leddy
no flags Details | Diff
Vince's patch w/ doc (5.75 KB, patch)
2009-05-22 12:24 EDT, James M. Leddy
no flags Details | Diff
Patch replacing select() with poll() (34.82 KB, patch)
2009-06-18 10:26 EDT, Jan Zeleny
no flags Details | Diff
Completed poll patch (cleaned) (37.18 KB, patch)
2009-09-14 08:54 EDT, Jan Zeleny
no flags Details | Diff

  None (edit)
Description James M. Leddy 2009-04-28 16:42:52 EDT
Created attachment 341649 [details]
patch allowing fd limits in xinetd.conf

Description of problem:

Two patches to address the issue of xinetd limiting fd's to FD_SETSIZE.

The first (xinetd-2.3.14-add-files-limit-vw4-2.patch) allows one to set a limit on the max number of files for a child service, in the same way that we already do with rlimit_as and friends in xinetd.conf.

The second (xinetd-2.3.14-fdsetsize.patch) fixes a known issue with xinetd imposing a limit of FD_SETSIZE on itself to get around needing to account for file descriptors in select().  This patch puts a good accounting mechanism in, so that the setrlimit() is no longer required.

Both are against the upstream 2.3.14 release, though they should apply cleanly to the fedora version.

I'll be notifying the upstream maintainer, since last time he accepted a patch it was keyed off a fedora bugzilla.
Comment 1 James M. Leddy 2009-04-28 17:06:26 EDT
Created attachment 341653 [details]
better accounting mechanism for select()
Comment 2 James M. Leddy 2009-04-28 17:10:10 EDT
These patches don't apply in consecutively, because they both remove 

- #ifdef RLIMIT_NOFILE
-    rl.rlim_max = ps.ros.orig_max_descriptors ;
-    rl.rlim_cur = ps.ros.max_descriptors ;
-    (void) setrlimit( RLIMIT_NOFILE, &rl ) ;
- #endif

from child.c.rej

Aside from that it compiles cleanly.
Comment 3 Jan Zeleny 2009-05-11 09:48:03 EDT
I reviewed the patches. There is one thing I don't fully understand - when I take patch adding new config options into consideration, is the other one (better accounting mechanism for select function) really necessary?
Comment 4 James M. Leddy 2009-05-14 16:05:38 EDT
The other patch taken on it's own fixes a longstanding hack.  The process should not need to set file descriptor limits just because it doesn't check FD_SETSIZE.  It should be left to the discretion of the users and sysadmins what resources a process is given.

That said, I can change the patch to correctly apply the old limits after fork().  That's the way they were hacking around that, but it turns out there is a bug in that code as well.

upstream still has yet to comment on the patches.
Comment 11 James M. Leddy 2009-05-22 12:24:44 EDT
Created attachment 345118 [details]
Vince's patch w/ doc

I forgot to put the patch up that included the documentation.

diff -pru xinetd-2.3.14/xinetd/xinetd.conf.man xinetd-2.3.14-add_fd_limit/xinetd/xinetd.conf.man
--- xinetd-2.3.14/xinetd/xinetd.conf.man        2005-10-05 13:15:33.000000000 -0400
+++ xinetd-2.3.14-add_fd_limit/xinetd/xinetd.conf.man   2009-04-28 17:32:34.000000000 -0400
@@ -564,6 +564,11 @@ is implemented, it is more useful to set
 rlimit_rss and rlimit_stack. This resource limit is only implemented on
 Linux systems.
 .TP
+.B rlimit_files
+Sets the maximum number of open files that the service may use.
+One parameter is required, which is either a positive integer representing
+the number of open file descriptors, or "UNLIMITED".
+.TP
 .B rlimit_cpu
 Sets the maximum number of CPU seconds that the service may use.
 One parameter is required, which is either a positive integer representing
Comment 16 Jan Zeleny 2009-06-18 10:26:07 EDT
Created attachment 348478 [details]
Patch replacing select() with poll()

I've created patch which replaces fdsetsize patch. This one uses poll() instead of select(). Patch was not tested yet, it's really very first version. Feel free to test it and post here the results. Any comments and suggestions are welcomed as well.
Comment 30 Jan Zeleny 2009-09-14 08:54:35 EDT
Created attachment 360936 [details]
Completed poll patch (cleaned)

Here is the updated final version of the patch. It has been reviewed and significantly cleaned up.
Comment 36 Jan Zeleny 2009-10-20 09:58:52 EDT
The second patch is in cvs for F-13 as well as an update candidate for F-12, closing this bug.

Note You need to log in before you can comment on or make changes to this bug.