On Tue, Mar 08, 2011 at 03:26:23PM +0100, Claudio Jeker wrote:
> 
> I played a bit with it and cleaned up the diff a bit (KNF fixes plus one
> error see below). A few comments inline plus my diff at the end.
> 
> > Index: tcpbench.1
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.1,v
> > retrieving revision 1.10
> > diff -d -u -p -w -r1.10 tcpbench.1
> > --- tcpbench.1      26 Oct 2010 20:26:37 -0000      1.10
> > +++ tcpbench.1      6 Mar 2011 19:51:00 -0000
> ...
> > +.It Fl u
> > +Use UDP instead of TCP, this must be specified on both client and
> > +server. Transmitted packets per second (TX PPS) will be accounted on the 
> > client
> > +side, while received packets per second (RX PPS) whill be accounted on the
> > +server side. UDP has no Protocol Control Block (PCB) so the
> > +.Fl k
> > +flags don't apply.
> 
> This is a lie. UDP has a PCB but that one does not include the tcp
> specific control block (TCB). It would be possible to get the UDP PCB and
> show at least the socket and inpcb specific values. Not sure how usable
> those are (but I would be interested in the socketbuffer stats on the
> reciever).
> 

Right, what I wanted to state is that there is no per-proto PCB in UDP (inp_ppcb
member). I didn't find anything useful of showing in in inpcb, but I can change
that if someone wants.

> > Index: tcpbench.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
> > retrieving revision 1.19
> > diff -d -u -p -w -r1.19 tcpbench.c
> > --- tcpbench.c      19 Oct 2010 10:03:23 -0000      1.19
> > +++ tcpbench.c      6 Mar 2011 19:51:00 -0000
> 
> > +again:
> > +   if ((n = write(sc->fd, sc->buf, sc->buflen)) == -1) {
> > +           if (errno == EINTR || errno == EAGAIN || (UDP_MODE && ENOBUFS))
> > +                   goto again;
> 

Wow, how did I miss that :D.

> I fixed this if statement. In UDP_MODE all errors would cause a busy loop.
> E.g. when calling "tcpdump -u -B 32768 127.0.0.1"
> 
> > +   if (ptb->sflag) {
> > +           (void)server_init(aitop, udp_sc);
> > +           if (setpgid(0, 0) == -1)
> > +                   err(1, "setpgid");
> 
> I moved the setpgid call into server_init() and made server_init a void
> function. Make more sense that way.
> 

Agreed.

-- 
Christiano Farina HAESBAERT
Do NOT send me html mail.

Reply via email to