Resend with unified diffs, sorry...
On 28 March 2011 20:11, Ben Gould ben.go...@gmail.com wrote:
Hi,
A bug fix, performance improvement and a new feature for pppoe(8); and
a performance improvement for ppp(8).
pppoe(8):
- Under load my low power atom-based router kept hitting the
usleep(10); case in pppoe which seemed to throw LCP echos off
causing line drops, this is now gone and the line doesn't drop.
- select() replaced for kqueue()/kevent() calls in pppoe's client loop.
- RFC 4638 support added to pppoe's client mode for negotiating 1500
byte mtu with the server.
ppp(8)
- A very quick and dirty drop-in kqueue()/kevent() replacement for
select() in the main loop. B It works for me, works well with the above
pppoe mods, you'll flame. B Suggestions would be welcome. B Can anyone
test this with dialup?
Some background for these changes: B Faster broadband got rolled out
here; all was working on ADSL 2+ (with random but tolerable line drops
during bittorrent downloads). B Upgraded to VDSL (here in the UK BT are
rolling out fibre to a roadside cabinet...) which is provided as a
PPPoE mode. B Line now drops were happening under normal usage so I
investigated... B Once all that was fixed I thought I'd add support for
the larger MTU. B I probably wouldn't have needed to rewrite the
select() calls as kqueue()/kevent() if I had a faster computer acting
as a router. B pppoe(4) seems to cause panics and wasn't setting up
IPv6 properly for me - sorry I've not investigated that further.
Diffs and dmesg follow.
-- ben
diff -upr ppp/ppp/main.c ppp.kqueue/ppp/main.c
--- ppp/ppp/main.c Wed Nov 12 16:01:08 2008
+++ ppp.kqueue/ppp/main.c Mon Mar 28 19:14:59 2011
@@ -48,6 +48,8 @@
#include termios.h
#include unistd.h
#include sys/stat.h
+#include sys/types.h
+#include sys/event.h
#ifndef NONAT
#ifdef LOCALNAT
@@ -533,27 +535,47 @@ main(int argc, char **argv)
static void
DoLoop(struct bundle *bundle)
{
- fd_set *rfds, *wfds, *efds;
- int i, nfds, nothing_done;
+ int kq;
+ struct kevent *changelist, *pevent;
+ fd_set *rfds, *wfds, *efds;
+ int i, nfds, nfds_done, nothing_done, x, nchanges, maxnchanges, neret;
- if ((rfds = mkfdset()) == NULL) {
-log_Printf(LogERROR, DoLoop: Cannot create fd_set\n);
-return;
- }
+ nfds_done = 0;
+ nchanges = 0;
+ maxnchanges = getdtablesize();
+ changelist = malloc(maxnchanges * sizeof(struct kevent));
- if ((wfds = mkfdset()) == NULL) {
-log_Printf(LogERROR, DoLoop: Cannot create fd_set\n);
-free(rfds);
-return;
- }
+ kq = kqueue();
+ if (kq 0) {
+ log_Printf(LogERROR, DoLoop: Cannot create kqueue\n);
+ free(changelist);
+ return;
+ }
- if ((efds = mkfdset()) == NULL) {
-log_Printf(LogERROR, DoLoop: Cannot create fd_set\n);
-free(rfds);
-free(wfds);
-return;
- }
+ if ((rfds = mkfdset()) == NULL) {
+ log_Printf(LogERROR, DoLoop: Cannot create fd_set\n);
+ free(changelist);
+ close(kq);
+ return;
+ }
+ if ((wfds = mkfdset()) == NULL) {
+ log_Printf(LogERROR, DoLoop: Cannot create fd_set\n);
+ free(changelist);
+ close(kq);
+ free(rfds);
+ return;
+ }
+
+ if ((efds = mkfdset()) == NULL) {
+ log_Printf(LogERROR, DoLoop: Cannot create fd_set\n);
+ free(changelist);
+ close(kq);
+ free(rfds);
+ free(wfds);
+ return;
+ }
+
for (; !bundle_IsDead(bundle); bundle_CleanDatalinks(bundle)) {
nfds = 0;
zerofdset(rfds);
@@ -571,6 +593,28 @@ DoLoop(struct bundle *bundle)
/* Don't select - we'll be here forever */
break;
+ /* Quick and dirty way to convert a complicated use of
+* select into a quicker call to kevent(2)
+*/
+ nfds_done = nfds nfds_done ? nfds : nfds_done;
+ for (x = 0; x nfds; ++x) {
+ int r = 0, w = 0;
+ if (FD_ISSET(x, rfds)) {
+ EV_SET(changelist + nchanges, x, EVFILT_READ,
EV_ADD|EV_ONESHOT, 0, 0,
0);
+ ++nchanges;
+ r = 1;
+ }
+ if (FD_ISSET(x, wfds)) {
+ EV_SET(changelist + nchanges, x, EVFILT_WRITE,
EV_ADD|EV_ONESHOT, 0, 0,
0);
+ ++nchanges;
+ w = 1;
+ }
+ if (!r !w FD_ISSET(x, efds)) {
+ EV_SET(changelist + nchanges, x, EVFILT_READ,
EV_ADD|EV_ONESHOT, 0, 0,
0);
+ ++nchanges;
+ }
+ }
+
/*
* It's possible that we've had a signal since we last checked. If
* we don't check again before calling select(), we may end up stuck
@@ -582,48 +626,36 @@ DoLoop(struct bundle
