On Sat, Aug 01, 2009 at 07:33:00PM -0300, Christiano Farina Haesbaert wrote:
> Hi there,
> 
> Here is a much better diff proposal than my previous one.
> 
> - Don't fork use poll instead.
> 
> - Collect usefull overall statistics, as % bandwith being used by each
> connection, total and average rate since last calculation,and peak
> transfer rate.
> 
> - There is still a lot of work to be done, as the calculation time overhead
> for a lot of fds is a factor.
> 
> - I've added an option (-m) which sets how many displays are done for each
> individual connection prior to displaying overall stats.
> 
> I don't know if the following is acceptable:
> - If we've reached the maximum number of fds, we stop polling the
> listening sockets until one fd is free.
> - I'm testing RLIMIT_NOFILE, and if it's lower than 1024, I'm setting it
> to 1024.
> 
> My thanks to Damien Miller who helped me understanding kvm.
> 
> What do you guys think ? I'm open to any changes that might be
> necessary.
> 

See inline comments.

> 
> 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.

> +#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.

>  }
>  
> -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?

> +             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.

>                       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.

> +                 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.

> +                             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.

-- 
:wq Claudio

Reply via email to