On Wed, Aug 05, 2009 at 07:04:21PM +0200, Claudio Jeker wrote:
> >
> > Index: tcpbench.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
> > retrieving revision 1.8
> > diff -d -p -u -r1.8 tcpbench.c
> > --- tcpbench.c 18 Sep 2008 10:23:33 -0000 1.8
> > +++ tcpbench.c 1 Aug 2009 22:32:09 -0000
> > @@ -18,6 +18,7 @@
> > #include <sys/time.h>
> > #include <sys/socket.h>
> > #include <sys/socketvar.h>
> > +#include <sys/resource.h>
> >
> > #include <net/route.h>
> >
> > @@ -47,22 +48,47 @@
> > #include <kvm.h>
> > #include <nlist.h>
> >
> > -#define DEFAULT_PORT "12345"
> > -#define DEFAULT_STATS_INTERVAL 1000 /* ms */
> > -#define DEFAULT_BUF 256 * 1024
> > +#define DEFAULT_PORT "12345"
> > +#define DEFAULT_STATS_INTERVAL 1000 /* ms */
> > +#define DEFAULT_BUF 256 * 1024
> > +#define DEFAULT_MAINSTATS_COUNT 4 /* print mainstats
> > + after 10 interactions */
> > +#define MAX_FD 1024
> >
> > sig_atomic_t done = 0;
> > -sig_atomic_t print_stats = 0;
> > +sig_atomic_t proc_slice = 0;
> > +
> > +static char **kflag;
> > +static size_t Bflag;
> > +static int Sflag;
> > +static int mflag;
> > +static int rflag;
> > +static int sflag;
> > +static int vflag;
> >
> > +/* stats for a single connection */
> > struct statctx {
> > - struct timeval t_start, t_last, t_cur;
> > + struct timeval t_start, t_last;
> > unsigned long long bytes;
> > - pid_t pid;
> > u_long tcbaddr;
> > - kvm_t *kh;
> > char **kvars;
> > + kvm_t *kh;
> > };
> >
> > +/*
> > + * We account the mainstats here, that is the stats
> > + * for all connections, all variables starting with slice
> > + * are used to account information for the timeslice
> > + * between each output. Peak variables record the highest
> > + * between all slices so far.
> > + */
> > +static struct {
> > + unsigned long long slice_bytes; /* bytes for last slice */
> > + struct timeval t_start; /* when we started counting */
> > + long double peak_mbps; /* peak mbps so far */
> > + int nconns; /* connected clients */
> > +} mainstats;
> > +
> > /* When adding variables, also add to stats_display() */
> > static const char *allowed_kvars[] = {
> > "inpcb.inp_flags",
> > @@ -103,7 +129,7 @@ exitsighand(int signo)
> > static void
> > alarmhandler(int signo)
> > {
> > - print_stats = 1;
> > + proc_slice = 1;
> > signal(signo, alarmhandler);
> > }
> >
> > @@ -113,7 +139,7 @@ usage(void)
> > fprintf(stderr,
> > "usage: tcpbench -l\n"
> > " tcpbench [-v] [-B buf] [-k kvars] [-n connections]"
> > - " [-p port] [-r rate]\n"
> > + " [-p port] [-r rate] [-m interval]\n"
> > " [-S space] hostname\n"
> > " tcpbench -s [-v] [-B buf] [-k kvars] [-p port] [-r rate]"
> > " [-S space]\n");
> > @@ -137,6 +163,40 @@ saddr_ntop(const struct sockaddr *addr,
> > }
> >
> > static void
> > +set_timer(int toggle)
> > +{
> > + struct itimerval itv;
> > +
> > + if (rflag <= 0)
> > + return;
> > +
> > + if (toggle) {
> > + itv.it_interval.tv_sec = rflag / 1000;
> > + itv.it_interval.tv_usec = (rflag % 1000) * 1000;
> > + itv.it_value = itv.it_interval;
> > + }
> > + else
> > + bzero(&itv, sizeof(itv));
> > +
> > + setitimer(ITIMER_REAL, &itv, NULL);
> > +}
> > +
> > +static void
> > +print_header(void)
> > +{
> > + char **kv;
> > +
> > + printf("%12s %14s %12s %8s ", "elapsed_ms", "bytes", "mbps",
> > + "bwidth");
> > +
> > + for (kv = kflag; kflag != NULL && *kv != NULL; kv++)
> > + printf("%s%s", kv != kflag ? "," : "", *kv);
> > +
> > + printf("\n");
> > + fflush(stdout);
> > +}
> > +
> > +static void
> > kget(kvm_t *kh, u_long addr, void *buf, int size)
> > {
> > if (kvm_read(kh, addr, buf, size) != size)
> > @@ -144,7 +204,7 @@ kget(kvm_t *kh, u_long addr, void *buf,
> > }
> >
> > static u_long
> > -kfind_tcb(kvm_t *kh, u_long ktcbtab, int sock, int vflag)
> > +kfind_tcb(kvm_t *kh, u_long ktcbtab, int sock)
> > {
> > struct inpcbtable tcbtab;
> > struct inpcb *head, *next, *prev;
> > @@ -304,98 +364,81 @@ check_prepare_kvars(char *list)
> > }
> >
> > static void
> > -stats_prepare(struct statctx *sc, int fd, kvm_t *kh, u_long ktcbtab,
> > - int rflag, int vflag, char **kflag)
> > +stats_prepare(struct statctx *sc, int fd, kvm_t *kh, u_long ktcbtab)
> > {
> > - struct itimerval itv;
> > - int i;
> > -
> > if (rflag <= 0)
> > return;
> > sc->kh = kh;
> > sc->kvars = kflag;
> > if (kflag)
> > - sc->tcbaddr = kfind_tcb(kh, ktcbtab, fd, vflag);
> > - gettimeofday(&sc->t_start, NULL);
> > + sc->tcbaddr = kfind_tcb(kh, ktcbtab, fd);
> > + if (gettimeofday(&sc->t_start, NULL) == -1)
> > + err(1, "gettimeofday");
> > sc->t_last = sc->t_start;
> > - signal(SIGALRM, alarmhandler);
> > - itv.it_interval.tv_sec = rflag / 1000;
> > - itv.it_interval.tv_usec = (rflag % 1000) * 1000;
> > - itv.it_value = itv.it_interval;
> > - setitimer(ITIMER_REAL, &itv, NULL);
> > sc->bytes = 0;
> > - sc->pid = getpid();
> > -
> > - printf("%8s %12s %14s %12s ", "pid", "elapsed_ms", "bytes", "Mbps");
> > - if (sc->kvars != NULL) {
> > - for (i = 0; sc->kvars[i] != NULL; i++)
> > - printf("%s%s", i > 0 ? "," : "", sc->kvars[i]);
> > - }
> > - printf("\n");
> > - fflush(stdout);
> > }
> >
> > static void
> > stats_update(struct statctx *sc, ssize_t n)
> > {
> > sc->bytes += n;
> > + mainstats.slice_bytes += n;
> > }
> >
> > static void
> > -stats_display(struct statctx *sc)
> > +stats_cleanslice(void)
> > {
> > - struct timeval t_diff;
> > - unsigned long long total_elapsed, since_last;
> > - size_t i;
> > - struct inpcb inpcb;
> > - struct tcpcb tcpcb;
> > - struct socket sockb;
> > -
> > - gettimeofday(&sc->t_cur, NULL);
> > - timersub(&sc->t_cur, &sc->t_start, &t_diff);
> > - total_elapsed = t_diff.tv_sec * 1000 + t_diff.tv_usec / 1000;
> > - timersub(&sc->t_cur, &sc->t_last, &t_diff);
> > - since_last = t_diff.tv_sec * 1000 + t_diff.tv_usec / 1000;
> > - printf("%8ld %12llu %14llu %12.3Lf ", (long)sc->pid,
> > - total_elapsed, sc->bytes,
> > - (long double)(sc->bytes * 8) / (since_last * 1000.0));
> > - sc->t_last = sc->t_cur;
> > - sc->bytes = 0;
> > + mainstats.slice_bytes = 0;
> > +}
> >
> > +static void
> > +stats_display(unsigned long long total_elapsed, long double mbps,
> > + float bwperc, struct statctx *sc, struct inpcb *inpcb,
> > + struct tcpcb *tcpcb, struct socket *sockb)
> > +{
> > + int j;
> > +
> > + printf("%12llu %14llu %12.3Lf %7.2f%% ", total_elapsed, sc->bytes,
> > + mbps, bwperc);
> > +
> > if (sc->kvars != NULL) {
> > - kupdate_stats(sc->kh, sc->tcbaddr, &inpcb, &tcpcb, &sockb);
> > - for (i = 0; sc->kvars[i] != NULL; i++) {
> > -#define P(v, f) \
> > - if (strcmp(sc->kvars[i], #v) == 0) { \
> > - printf("%s"f, i > 0 ? "," : "", v); \
> > - continue; \
> > - }
> > - P(inpcb.inp_flags, "0x%08x")
> > - P(sockb.so_rcv.sb_cc, "%lu")
> > - P(sockb.so_rcv.sb_hiwat, "%lu")
> > - P(sockb.so_snd.sb_cc, "%lu")
> > - P(sockb.so_snd.sb_hiwat, "%lu")
> > - P(tcpcb.snd_una, "%u")
> > - P(tcpcb.snd_nxt, "%u")
> > - P(tcpcb.snd_wl1, "%u")
> > - P(tcpcb.snd_wl2, "%u")
> > - P(tcpcb.snd_wnd, "%lu")
> > - P(tcpcb.rcv_wnd, "%lu")
> > - P(tcpcb.rcv_nxt, "%u")
> > - P(tcpcb.rcv_adv, "%u")
> > - P(tcpcb.snd_max, "%u")
> > - P(tcpcb.snd_cwnd, "%lu")
> > - P(tcpcb.snd_ssthresh, "%lu")
> > - P(tcpcb.t_rcvtime, "%u")
> > - P(tcpcb.t_rtttime, "%u")
> > - P(tcpcb.t_rtseq, "%u")
> > - P(tcpcb.t_srtt, "%hu")
> > - P(tcpcb.t_rttvar, "%hu")
> > - P(tcpcb.t_rttmin, "%hu")
> > - P(tcpcb.max_sndwnd, "%lu")
> > - P(tcpcb.snd_scale, "%u")
> > - P(tcpcb.rcv_scale, "%u")
> > - P(tcpcb.last_ack_sent, "%u")
> > + kupdate_stats(sc->kh, sc->tcbaddr, inpcb, tcpcb,
> > + sockb);
> > +
> > + for (j = 0; sc->kvars[j] != NULL; j++) {
> > +#define S(a) #a
>
> This macro is used just a single time and I don't realy see the advantage
> of your new P() macro to the old one.
I had to do it in order to keep the same kvars interface at the
command line, prior sockb/inpcb/tcpcb were not a pointer, so the
contruct had to expand to sockb.inp_flags, and now to
sockb->inp_flags.
>
> > +#define P(b, v, f) \
> > + if (strcmp(sc->kvars[j], S(b.v)) == 0) { \
> > + printf("%s"f, j > 0 ? "," : "", b->v); \
> > + continue; \
> > + }
> > + P(inpcb, inp_flags, "0x%08x")
> > + P(sockb, so_rcv.sb_cc, "%lu")
> > + P(sockb, so_rcv.sb_hiwat, "%lu")
> > + P(sockb, so_snd.sb_cc, "%lu")
> > + P(sockb, so_snd.sb_hiwat, "%lu")
> > + P(tcpcb, snd_una, "%u")
> > + P(tcpcb, snd_nxt, "%u")
> > + P(tcpcb, snd_wl1, "%u")
> > + P(tcpcb, snd_wl2, "%u")
> > + P(tcpcb, snd_wnd, "%lu")
> > + P(tcpcb, rcv_wnd, "%lu")
> > + P(tcpcb, rcv_nxt, "%u")
> > + P(tcpcb, rcv_adv, "%u")
> > + P(tcpcb, snd_max, "%u")
> > + P(tcpcb, snd_cwnd, "%lu")
> > + P(tcpcb, snd_ssthresh, "%lu")
> > + P(tcpcb, t_rcvtime, "%u")
> > + P(tcpcb, t_rtttime, "%u")
> > + P(tcpcb, t_rtseq, "%u")
> > + P(tcpcb, t_srtt, "%hu")
> > + P(tcpcb, t_rttvar, "%hu")
> > + P(tcpcb, t_rttmin, "%hu")
> > + P(tcpcb, max_sndwnd, "%lu")
> > + P(tcpcb, snd_scale, "%u")
> > + P(tcpcb, rcv_scale, "%u")
> > + P(tcpcb, last_ack_sent, "%u")
> > +#undef S
> > #undef P
> > }
> > }
> > @@ -404,94 +447,104 @@ stats_display(struct statctx *sc)
> > }
> >
> > static void
> > -stats_finish(struct statctx *sc)
> > +mainstats_display(long double slice_mbps, long double avg_mbps)
> > {
> > - struct itimerval itv;
> > + printf("Conn: %3d Mbps: %12.3Lf Peak Mbps: %12.3Lf Avg Mbps: %12.3Lf\n",
> > + mainstats.nconns, slice_mbps, mainstats.peak_mbps, avg_mbps);
> >
> > - signal(SIGALRM, SIG_DFL);
> > - bzero(&itv, sizeof(itv));
> > - setitimer(ITIMER_REAL, &itv, NULL);
> > + fflush(stdout);
>
> Is the fflush(0 realy needed. printf() will flush on \n
> automatically.
True, not needed, removing.
>
> > }
> >
> > -static void __dead
> > -handle_connection(kvm_t *kvmh, u_long ktcbtab, int sock, int vflag,
> > - int rflag, char **kflag, int Bflag)
> > +static void
> > +process_slice(struct statctx *sc, size_t nsc)
> > {
> > - char *buf;
> > - struct pollfd pfd;
> > - ssize_t n;
> > - int r;
> > - struct statctx sc;
> > -
> > - if ((buf = malloc(Bflag)) == NULL)
> > - err(1, "malloc");
> > - if ((r = fcntl(sock, F_GETFL, 0)) == -1)
> > - err(1, "fcntl(F_GETFL)");
> > - r |= O_NONBLOCK;
> > - if (fcntl(sock, F_SETFL, r) == -1)
> > - err(1, "fcntl(F_SETFL, O_NONBLOCK)");
> > + int i;
> > + static int total_display = 0;
> > + float bwperc;
> > + long double mbps, slice_mbps = 0;
> > + unsigned long long total_elapsed, since_last;
> > + struct timeval t_cur, t_diff;
> > + struct inpcb inpcb;
> > + struct tcpcb tcpcb;
> > + struct socket sockb;
> >
> > - signal(SIGINT, exitsighand);
> > - signal(SIGTERM, exitsighand);
> > - signal(SIGHUP, exitsighand);
> > - signal(SIGPIPE, SIG_IGN);
> > + if (gettimeofday(&t_cur, NULL) == -1)
> > + err(1, "gettimeofday");
> >
> > - bzero(&pfd, sizeof(pfd));
> > - pfd.fd = sock;
> > - pfd.events = POLLIN;
> > + for (i = 0; i < nsc; i++, sc++) {
> > + if (sc->kvars != NULL) /* process kernel stats */
> > + kupdate_stats(sc->kh, sc->tcbaddr, &inpcb, &tcpcb,
> > + &sockb);
> > + timersub(&t_cur, &sc->t_start, &t_diff);
> > + total_elapsed = t_diff.tv_sec * 1000 + t_diff.tv_usec / 1000;
> > + timersub(&t_cur, &sc->t_last, &t_diff);
> > + since_last = t_diff.tv_sec * 1000 + t_diff.tv_usec / 1000;
> > + bwperc = (sc->bytes * 100.0) / mainstats.slice_bytes;
> > + mbps = (sc->bytes * 8) / (since_last * 1000.0);
> > + slice_mbps += mbps;
> > +
> > + stats_display(total_elapsed, mbps, bwperc, sc,
> > + &inpcb, &tcpcb, &sockb);
> > +
> > + sc->t_last = t_cur;
> > + sc->bytes = 0;
> >
> > - stats_prepare(&sc, sock, kvmh, ktcbtab, rflag, vflag, kflag);
> > + }
> >
> > - while (!done) {
> > - if (print_stats) {
> > - stats_display(&sc);
> > - print_stats = 0;
> > - }
> > - if (poll(&pfd, 1, INFTIM) == -1) {
> > - if (errno == EINTR)
> > - continue;
> > - err(1, "poll");
> > - }
> > - if ((n = read(pfd.fd, buf, Bflag)) == -1) {
> > - if (errno == EINTR || errno == EAGAIN)
> > - continue;
> > - err(1, "read");
> > - }
> > - if (n == 0) {
> > - fprintf(stderr, "%8ld closed by remote end\n",
> > - (long)getpid());
> > - done = -1;
> > - break;
> > - }
> > - if (vflag >= 3)
> > - fprintf(stderr, "read: %zd bytes\n", n);
> > - stats_update(&sc, n);
> > + /* process stats for this slice */
> > + if (slice_mbps > mainstats.peak_mbps)
> > + mainstats.peak_mbps = slice_mbps;
> > + if (++total_display == mflag) {
> > + mainstats_display(slice_mbps, slice_mbps / mainstats.nconns);
> > + total_display = 0;
> > }
> > - stats_finish(&sc);
> > +}
> >
> > - free(buf);
> > - close(sock);
> > - exit(1);
> > +static int
> > +handle_connection(struct statctx *sc, int fd, char *buf, size_t buflen)
> > +{
> > + ssize_t n;
> > +
> > +again:
> > + n = read(fd, buf, buflen);
> > + if (n == -1) {
> > + fprintf(stderr, "fd = %d read errno = %d\n",fd, errno);
>
> use warn() or strerror() to make the errno human understandable.
> Shouldn't the fprintf be moved after the EINTR and EWOULDBLOCK
> check?
Correct, this was a mistake, I was measuring EWOULDBLOCK hits and
forgot to take out the fprintf, I'll correct it and put it after EINTR
and EWOULDBLOCK checks.
>
> > + if (errno == EINTR)
> > + goto again;
> > + else if (errno == EWOULDBLOCK)
> > + return 0;
> > + return -1;
> > + }
> > + else if (n == 0) {
> > + fprintf(stderr, "%8d closed by remote end\n", fd);
> > + close(fd);
> > + return -1;
> > + }
> > + if (vflag >= 3)
> > + fprintf(stderr, "read: %zd bytes\n", n);
> > +
> > + stats_update(sc, n);
> > + return 0;
> > }
> >
> > -static void __dead
> > -serverloop(kvm_t *kvmh, u_long ktcbtab, struct addrinfo *aitop,
> > - int vflag, int rflag, char **kflag, int Sflag, int Bflag)
> > +static int
> > +serverbind(struct pollfd *pfd, nfds_t max_nfds, struct addrinfo *aitop)
> > {
> > char tmp[128];
> > - int r, sock, client_id, on = 1;
> > + int sock, on = 1;
> > struct addrinfo *ai;
> > - struct pollfd *pfd;
> > - struct sockaddr_storage ss;
> > - socklen_t sslen;
> > - size_t nfds, i, j;
> > + int lnfds;
> >
> > - pfd = NULL;
> > - nfds = 0;
> > + lnfds = 0;
> > for (ai = aitop; ai != NULL; ai = ai->ai_next) {
> > + if (lnfds == max_nfds) {
> > + fprintf(stderr,
> > + "maximum number of listening fds reached\n");
> > + break;
> > + }
> > saddr_ntop(ai->ai_addr, ai->ai_addrlen, tmp, sizeof(tmp));
> > if (vflag)
> > - fprintf(stderr, "Try listen on %s\n", tmp);
> > + fprintf(stderr, "Try to listen on %s\n", tmp);
> > if ((sock = socket(ai->ai_family, ai->ai_socktype,
> > ai->ai_protocol)) == -1) {
> > if (ai->ai_next == NULL)
> > @@ -519,109 +572,189 @@ serverloop(kvm_t *kvmh, u_long ktcbtab,
> > if (listen(sock, 64) == -1) {
> > if (ai->ai_next == NULL)
> > err(1, "listen");
> > - if (vflag)
> > - warn("listen");
>
> Why removing this? It would be nice to include the address/port that
> failed though.
Oooops, correct :)
>
> > close(sock);
> > continue;
> > }
> > - if (nfds > 128)
> > - break;
> > - if ((pfd = realloc(pfd, ++nfds * sizeof(*pfd))) == NULL)
> > - errx(1, "realloc(pfd * %zu)", nfds);
> > - pfd[nfds - 1].fd = sock;
> > - pfd[nfds - 1].events = POLLIN;
> > + if (vflag >= 3)
> > + fprintf(stderr, "listening on fd %d\n", sock);
> > + lnfds++;
> > + pfd[lnfds - 1].fd = sock;
> > + pfd[lnfds - 1].events = POLLIN;
> > +
> > }
> > freeaddrinfo(aitop);
> > - if (nfds == 0)
> > + if (lnfds == 0)
> > errx(1, "No working listen addresses found");
> >
> > - signal(SIGINT, exitsighand);
> > - signal(SIGTERM, exitsighand);
> > - signal(SIGHUP, exitsighand);
> > - signal(SIGPIPE, SIG_IGN);
> > - signal(SIGCHLD, SIG_IGN);
> > + return lnfds;
> > +}
> > +
> > +static void
> > +set_listening(struct pollfd *pfd, nfds_t lfds, int toggle) {
> > + int i;
> > +
> > + for (i = 0; i < lfds; i++) {
> > + fprintf(stderr, "tirando fd %d\n",
>
> Wrong language.
Another oops.
>
> > + pfd[i].fd);
> > + if (toggle)
> > + pfd[i].events = POLLIN;
> > + else
> > + pfd[i].events = 0;
> > + }
> > +
> > +}
> > +static void __dead
> > +serverloop(kvm_t *kvmh, u_long ktcbtab, struct addrinfo *aitop)
> > +{
> > + struct rlimit rl;
> > + socklen_t sslen;
> > + struct pollfd *pfd;
> > + char tmp[128], *buf;
> > + struct statctx *psc;
> > + struct sockaddr_storage ss;
> > + nfds_t nfds, lfds;
> > + size_t nalloc;
> > + int i, r, sock, client_id;
> >
> > + sslen = sizeof(ss);
> > + if (getrlimit(RLIMIT_NOFILE, &rl) == -1)
> > + err(1, "getrlimit");
> > + if (rl.rlim_cur < MAX_FD)
> > + rl.rlim_cur = MAX_FD;
> > + if (setrlimit(RLIMIT_NOFILE, &rl))
> > + err(1, "setrlimit");
> > + if (getrlimit(RLIMIT_NOFILE, &rl) == -1)
> > + err(1, "getrlimit");
> > +
> > + nalloc = 128;
> > + if ((pfd = calloc(sizeof(*pfd), nalloc)) == NULL)
> > + err(1, "calloc");
> > + if ((psc = calloc(sizeof(*psc), nalloc)) == NULL)
> > + err(1, "calloc");
> > + if ((buf = malloc(Bflag)) == NULL)
> > + err(1, "malloc");
> > + lfds = nfds = serverbind(pfd, nalloc - 1, aitop);
> > + if (vflag >= 3)
> > + fprintf(stderr, "listening on %d fds\n", lfds);
> > if (setpgid(0, 0) == -1)
> > err(1, "setpgid");
> > -
> > +
> > + print_header();
> > +
> > client_id = 0;
> > - while (!done) {
> > + while (!done) {
> > + if (proc_slice) {
> > + process_slice(psc + lfds, nfds - lfds);
> > + stats_cleanslice();
> > + proc_slice = 0;
> > + }
> > + if (vflag >= 3)
> > + fprintf(stderr, "mainstats.nconns = %u\n",
> > + mainstats.nconns);
> > if ((r = poll(pfd, nfds, INFTIM)) == -1) {
> > if (errno == EINTR)
> > continue;
> > warn("poll");
> > break;
> > }
> > +
> > if (vflag >= 3)
> > fprintf(stderr, "poll: %d\n", r);
> > for (i = 0 ; r > 0 && i < nfds; i++) {
> > if ((pfd[i].revents & POLLIN) == 0)
> > continue;
> > - if (vflag >= 3)
> > - fprintf(stderr, "fd %d active\n", pfd[i].fd);
> > + if (pfd[i].fd == -1)
> > + errx(1, "pfd insane");
> > r--;
> > - sslen = sizeof(ss);
> > - if ((sock = accept(pfd[i].fd, (struct sockaddr *)&ss,
> > - &sslen)) == -1) {
> > - if (errno == EINTR)
> > + if (vflag >= 3)
> > + fprintf(stderr, "fd %d active i = %d\n",
> > + pfd[i].fd, i);
> > + /* new connection */
> > + if (i < lfds) {
> > + if ((sock = accept(pfd[i].fd,
> > + (struct sockaddr *)&ss,
> > + &sslen)) == -1) {
> > + if (errno == EINTR)
> > + continue;
> > + else if (errno == EMFILE ||
> > + errno == ENFILE)
> > + set_listening(pfd, lfds, 0);
> > + warn("accept");
> > continue;
> > - warn("accept");
> > - break;
> > + }
> > + if ((r = fcntl(sock, F_GETFL, 0)) == -1)
> > + err(1, "fcntl(F_GETFL)");
> > + r |= O_NONBLOCK;
> > + if (fcntl(sock, F_SETFL, r) == -1)
> > + err(1, "fcntl(F_SETFL, O_NONBLOCK)");
> > + saddr_ntop((struct sockaddr *)&ss, sslen,
> > + tmp, sizeof(tmp));
> > + if (vflag)
> > + fprintf(stderr,
> > + "Accepted connection %d from "
> > + "%s, fd = %d\n", client_id++, tmp,
> > + sock);
> > + /* alloc more space if we're full */
> > + if (nfds == nalloc) {
> > + nalloc *= 2;
> > + if ((pfd = realloc(pfd,
> > + sizeof(*pfd) * nalloc)) == NULL)
> > + err(1, "realloc");
> > + if ((psc = realloc(psc,
> > + sizeof(*psc) * nalloc)) == NULL)
> > + err(1, "realloc");
> > + }
> > + stats_prepare(&psc[nfds], sock, kvmh, ktcbtab);
> > + pfd[nfds].fd = sock;
> > + pfd[nfds].events = POLLIN;
> > + nfds++;
> > + if (!mainstats.nconns++)
> > + set_timer(1);
> > + continue;
> > }
> > - saddr_ntop((struct sockaddr *)&ss, sslen,
> > - tmp, sizeof(tmp));
> > - if (vflag)
> > - fprintf(stderr, "Accepted connection %d from "
> > - "%s, fd = %d\n", client_id++, tmp, sock);
> > - switch (fork()) {
> > - case -1:
> > - warn("fork");
> > - done = -1;
> > - break;
> > - case 0:
> > - for (j = 0; j < nfds; j++)
> > - if (j != i)
> > - close(pfd[j].fd);
> > - handle_connection(kvmh, ktcbtab, sock,
> > - vflag, rflag, kflag, Bflag);
> > - /* NOTREACHED */
> > - _exit(1);
> > - default:
> > - close(sock);
> > - break;
> > + /* event in fd */
> > + if (vflag >= 3)
> > + fprintf(stderr,
> > + "fd %d active", pfd[i].fd);
> > + if (handle_connection(&psc[i], pfd[i].fd,
> > + buf, Bflag) == -1) {
> > + pfd[i] = pfd[nfds - 1];
> > + pfd[nfds].fd = -1;
>
> This feels wrong. You swap with a not yet checked fd so that one is
> skipped at the same time you don't reset the flags so if that fd had data
> you would end up with a -1 fd that you try to process.
I'm not sure if I understood you.
Doing like this we guarantee that poll is not looking at any structure
with a disabled fd, we always keep all interested fds lined up, so the
last one + 1 is always the next free.
I didn't pay attention to it in my first implementation and there was
a significant drop in performance, sometimes poll would be called with
n == 600+ but there were only a few interested fds.
But yes, this is not optimal because we may only process that swapped
fd in the next loop iteraction. Maybe we could avoid this by
decrementing i when we have a disconnection ? so we would force the
processing of the swapped fd.
>
> > + psc[i] = psc[nfds - 1];
> > + mainstats.nconns--;
> > + nfds--;
> > + /* stop display if no clients */
> > + if (!mainstats.nconns) {
> > + proc_slice = 1;
> > + set_timer(0);
> > + }
> > + /* if we were full */
> > + set_listening(pfd, lfds, 1);
> > }
> > - if (done == -1)
> > - break;
> > }
> > }
> > - for (i = 0; i < nfds; i++)
> > - close(pfd[i].fd);
> > - if (done > 0)
> > - warnx("Terminated by signal %d", done);
> > - signal(SIGTERM, SIG_IGN);
> > - killpg(0, SIGTERM);
> > exit(1);
> > }
> >
> > static void __dead
> > -clientloop(kvm_t *kvmh, u_long ktcbtab, const char *host, const char *port,
> > - int vflag, int rflag, char **kflag, int Sflag, int Bflag, int nconn)
> > +clientloop(kvm_t *kvmh, u_long ktcbtab, const char *host, const char
> > *port, int nconn)
> > {
> > - char tmp[128];
> > - char *buf;
> > - int r, sock, herr;
> > struct addrinfo *aitop, *ai, hints;
> > + struct statctx *psc;
> > struct pollfd *pfd;
> > - ssize_t n;
> > - struct statctx sc;
> > + char tmp[128], *buf;
> > + int r, herr, sock = -1;
> > u_int i, scnt = 0;
> > + ssize_t n;
> >
> > if ((buf = malloc(Bflag)) == NULL)
> > err(1, "malloc");
> >
> > - if ((pfd = calloc(nconn, sizeof(struct pollfd))) == NULL)
> > + if ((pfd = calloc(nconn, sizeof(*pfd))) == NULL)
> > err(1, "clientloop pfd calloc");
> > -
> > + if ((psc = calloc(nconn, sizeof(*psc))) == NULL)
> > + err(1, "clientloop psc calloc");
> > +
> > for (i = 0; i < nconn; i++) {
> > bzero(&hints, sizeof(hints));
> > hints.ai_socktype = SOCK_STREAM;
> > @@ -674,6 +807,8 @@ clientloop(kvm_t *kvmh, u_long ktcbtab,
> >
> > pfd[i].fd = sock;
> > pfd[i].events = POLLOUT;
> > + stats_prepare(psc + i, sock, kvmh, ktcbtab);
> > + mainstats.nconns++;
> > scnt++;
> > }
> >
> > @@ -681,17 +816,14 @@ clientloop(kvm_t *kvmh, u_long ktcbtab,
> > fprintf(stderr, "%u connections established\n", scnt);
> > arc4random_buf(buf, Bflag);
> >
> > - signal(SIGINT, exitsighand);
> > - signal(SIGTERM, exitsighand);
> > - signal(SIGHUP, exitsighand);
> > - signal(SIGPIPE, SIG_IGN);
> > -
> > - stats_prepare(&sc, sock, kvmh, ktcbtab, rflag, vflag, kflag);
> > + print_header();
> > + set_timer(1);
> >
> > while (!done) {
> > - if (print_stats) {
> > - stats_display(&sc);
> > - print_stats = 0;
> > + if (proc_slice) {
> > + process_slice(psc, scnt);
> > + stats_cleanslice();
> > + proc_slice = 0;
> > }
> > if (poll(pfd, nconn, INFTIM) == -1) {
> > if (errno == EINTR)
> > @@ -713,12 +845,11 @@ clientloop(kvm_t *kvmh, u_long ktcbtab,
> > if (vflag >= 3)
> > fprintf(stderr, "write: %zd bytes\n",
> > n);
> > - stats_update(&sc, n);
> > + stats_update(psc + i, n);
> > }
> > }
> > }
> > - stats_finish(&sc);
> > -
> > +
> > if (done > 0)
> > warnx("Terminated by signal %d", done);
> >
> > @@ -744,20 +875,23 @@ main(int argc, char **argv)
> > extern char *optarg;
> >
> > char kerr[_POSIX2_LINE_MAX], *tmp;
> > - const char *errstr;
> > - int ch, herr;
> > struct addrinfo *aitop, hints;
> > + const char *errstr;
> > kvm_t *kvmh = NULL;
> > + int ch, herr;
> >
> > const char *host = NULL, *port = DEFAULT_PORT;
> > - char **kflag = NULL;
> > - int sflag = 0, vflag = 0, rflag = DEFAULT_STATS_INTERVAL, Sflag = 0;
> > - int Bflag = DEFAULT_BUF;
> > int nconn = 1;
> >
> > + Bflag = DEFAULT_BUF;
> > + Sflag = sflag = vflag = 0;
> > + kflag = NULL;
> > + rflag = DEFAULT_STATS_INTERVAL;
> > + mflag = DEFAULT_MAINSTATS_COUNT;
> > +
> > struct nlist nl[] = { { "_tcbtable" }, { "" } };
> >
> > - while ((ch = getopt(argc, argv, "B:hlk:n:p:r:sS:v")) != -1) {
> > + while ((ch = getopt(argc, argv, "B:hlk:m:n:p:r:sS:v")) != -1) {
> > switch (ch) {
> > case 'l':
> > list_kvars();
> > @@ -775,6 +909,12 @@ main(int argc, char **argv)
> > errx(1, "statistics interval is %s: %s",
> > errstr, optarg);
> > break;
> > + case 'm':
> > + mflag = strtonum(optarg, 1, 65535, &errstr);
> > + if (errstr != NULL)
> > + errx(1, "mainstats interval is %s: %s",
> > + errstr, optarg);
> > + break;
> > case 'p':
> > port = optarg;
> > break;
> > @@ -815,9 +955,6 @@ main(int argc, char **argv)
> > if (argc != (sflag ? 0 : 1))
> > usage();
> >
> > - if (kflag != NULL && nconn > 1)
> > - errx(1, "-k currently only works with a single tcp connection");
> > -
> > if (!sflag)
> > host = argv[0];
> >
> > @@ -843,12 +980,16 @@ main(int argc, char **argv)
> > } else
> > drop_gid();
> >
> > + signal(SIGINT, exitsighand);
> > + signal(SIGTERM, exitsighand);
> > + signal(SIGHUP, exitsighand);
> > + signal(SIGPIPE, SIG_IGN);
> > + signal(SIGALRM, alarmhandler);
> > +
> > if (sflag)
> > - serverloop(kvmh, nl[0].n_value, aitop, vflag, rflag, kflag,
> > - Sflag, Bflag);
> > + serverloop(kvmh, nl[0].n_value, aitop);
> > else
> > - clientloop(kvmh, nl[0].n_value, host, port, vflag, rflag, kflag,
> > - Sflag, Bflag, nconn);
> > + clientloop(kvmh, nl[0].n_value, host, port, nconn);
> >
> > return 0;
> > }
> >
>
> This needs a bit more work and maybe it would make sense to switch away
> from poll to kqueue or libevent. poll() gets inefficient when handling
> large ammount of fds. But that's maybe for later.
Cool, thanks a lot for your time, I'll do the corrections that you
pointed.
--
Christiano Farina HAESBAERT
Do NOT send me html mail.
