Re: [9fans] fossil+venti performance question

[David du Colombier]

>> however, after fixing things so the initial cwind isn't hosed, i get a 
>> little better story:
>
> so, actually, i think this is the root cause.  the intial cwind is misset for 
> loopback.
> i but that the symptom folks will see is that /net/tcp/stats shows 
> fragmentation when
> performance sucks.  evidently there is a backoff bug in sources' tcp, too.

What is your cwind change?

-- 
David du Colombier

Re: [9fans] fossil+venti performance question

[cinap_lenrek]

> * the SYN-ACK needs to send the local mss, not echo the remote mss.
> asymmetry is "fine" in the other side, even if ip/tcp.c isn't smart enough to
> keep tx and rx mss seperate.  (scare quotes = untested, there may be
> some performance niggles if the sender is sending legal packets larger than
> tcb->mss.)

that is what it already does as far as i can see. on the server side, we 
receive a
SYN, put it in limbo and reply with SYN|ACK (sndsynack()) sending our local
mss straight from tcpmtu(), no adjust. at this point heres no connection or tcb 
as
everything is still in limbo. only once we receive the ACK, tcpincoming() gets 
called which
pulls the info we got so far (including the mss sent by the client in the SYN 
pakcet) out of
limbo and sets up a connection with its tcb.

to summarize what happens on the server for incoming connection:

1.a) tcpiput() gets a SYN packet for Listening connection, calls limbo().
1.b) limbo() saves the info (including mss) from SYN in limbo database and 
calls sndsynack().
1.c) sndsynack() sends SYN|ACK packet with mss option set from tcpmtu() without 
any adjust.

2.a) tcpiput() gets a ACK packet for Listening connection, calls tcpincoming().
2.b) tcpincoming() looks in limbo, finds lp. and makes new connection.
3.c) initialize our connections tcb->mss.

> * the setting of tcb->mss in tcpincoming is not correct, tcp->mss is
> set by SYN, not by ACK, and may not be reset.  (see snoopy below.)

you say we shouldnt initialize tcb->mss in 3.c and not use the mss from the
initial SYN to adjust it. i dont understand why not as i dont see where it
would be initialized otherwise. it appears that was what the initial patch
from david was about to fix which made sense to me.

as far as i can see, the procsyn() is unrelated to server side incoming 
connections. it only gets called on behalf of a client outgoing connect
when the connection is in Syn_sent state and processes the SYN|ACK that
was generated by the process descibed in 1.c above.

--
cinap

Re: [9fans] fossil+venti performance question

[erik quanstrom]

On Sun May 10 10:58:55 PDT 2015, 0in...@gmail.com wrote:
> >> however, after fixing things so the initial cwind isn't hosed, i get a 
> >> little better story:
> >
> > so, actually, i think this is the root cause.  the intial cwind is misset 
> > for loopback.
> > i but that the symptom folks will see is that /net/tcp/stats shows 
> > fragmentation when
> > performance sucks.  evidently there is a backoff bug in sources' tcp, too.
> 
> What is your cwind change?
> 

the patch is here: /n/atom/patch/tcpmss

note i applied a patch to nettest(8) to simulate a rpc-style protocol.  i still 
~500MB/s
with my test machine simulating rpc-style transactions, or 15µs per 8k 
transaction.

we're at least an order of magnitude off the performance mark for this.

a similar test using pipe(2) shows a latency of 5.7µs (!) for a pipe-based rpc, 
which limits
us to about 1.4 GB/s for 8k pipe-based ping-poing rpc.

- erik

Re: [9fans] fossil+venti performance question

[erik quanstrom]

> 2.a) tcpiput() gets a ACK packet for Listening connection, calls 
> tcpincoming().
> 2.b) tcpincoming() looks in limbo, finds lp. and makes new connection.
> 3.c) initialize our connections tcb->mss.
> 
> > * the setting of tcb->mss in tcpincoming is not correct, tcp->mss is
> > set by SYN, not by ACK, and may not be reset.  (see snoopy below.)
> 
> you say we shouldnt initialize tcb->mss in 3.c and not use the mss from the
> initial SYN to adjust it. i dont understand why not as i dont see where it
> would be initialized otherwise. it appears that was what the initial patch
> from david was about to fix which made sense to me.

that was the opposite of what i was saying.  the issue was i misread 
tcpincoming().

- erik

Re: [9fans] fossil+venti performance question

[cinap_lenrek]

how is this the opposite? your patch shows the tcb->mss init being removed
completely from tcpincoming().
  
-   /* our sending max segment size cannot be bigger than what he asked for 
*/
-   if(lp->mss != 0 && lp->mss < tcb->mss) {
-   tcb->mss = lp->mss;
-   tpriv->stats[Mss] = tcb->mss;
-   }
+   /* per rfc, we can't set the mss any more */
+ //tcb->mss = tcpmtu(s->p, lp->laddr, lp->version, lp->mss, &tcb->scale);

--
cinap

Re: [9fans] fossil+venti performance question

[erik quanstrom]

On Sun May 10 14:36:15 PDT 2015, cinap_len...@felloff.net wrote:
> how is this the opposite? your patch shows the tcb->mss init being removed
> completely from tcpincoming().
>   
> - /* our sending max segment size cannot be bigger than what he asked for 
> */
> - if(lp->mss != 0 && lp->mss < tcb->mss) {
> - tcb->mss = lp->mss;
> - tpriv->stats[Mss] = tcb->mss;
> - }
> + /* per rfc, we can't set the mss any more */
> + //  tcb->mss = tcpmtu(s->p, lp->laddr, lp->version, lp->mss, &tcb->scale);

i haven't updated the patch.

- erik