Date: Thu, 20 Dec 2018 08:34:58 +0000 From: "Scheffenegger, Richard" <Richard.Scheffenegger@netapp.com> To: "hiren@strugglingcoder.info" <hiren@strugglingcoder.info>, "jtl@FreeBSD.org" <jtl@FreeBSD.org> Cc: "freebsd-transport@freebsd.org" <freebsd-transport@freebsd.org> Subject: RE: possible RFC6675 compute_pipe counting issue. Message-ID: <CY4PR0601MB37156234E44BE5B6D45A8F3386BF0@CY4PR0601MB3715.namprd06.prod.outlook.com> In-Reply-To: <20181219231724.GJ75110@strugglingcoder.info> References: <SN4PR0601MB3728CD36E28E1D473715F51586BE0@SN4PR0601MB3728.namprd06.prod.outlook.com> <20181219231724.GJ75110@strugglingcoder.info>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Hiren, Jonathan, > As your patch also touches tcpcb, it needs more eyes. NF may have a bunch= of changes here so best to coordinate with them. Sure; I still have to work a bit on the PRR patch - I don't like replicatin= g the mechanism in two places, and outside of the normal SACK procedures, w= hen PRR really is an improvement upon 3517. As 6675 pipe (and entering loss= recovery) would also be bundled up there, may take me some time too, to ge= t all that in a nice, concise patch. =20 > I guess you can open a phabricator review request and go from there? Surely, once I believe the code is ready. In the meantime, still looking fo= r reviewers for this simple one=20 https://reviews.freebsd.org/D17614 I recently added two packetdrill scripts to demonstrate the (timing) differ= ent w/o (std) and with that patch. Potentially, these scripts can be added = to Michaels collection of FreeBSd network unit test scripts:=20 https://github.com/freebsd-net/tcp-testsuite/ Best regards, Richard -----Original Message----- From: hiren@strugglingcoder.info <hiren@strugglingcoder.info>=20 Sent: Donnerstag, 20. Dezember 2018 00:17 To: Scheffenegger, Richard <Richard.Scheffenegger@netapp.com>; jtl@FreeBSD.= org Cc: freebsd-transport@freebsd.org Subject: Re: possible RFC6675 compute_pipe counting issue. On 12/19/18 at 09:21P, Scheffenegger, Richard wrote: > Hi Hiren, Hi Richard, (I may not get time to look at this soon but responding right now before I = forget :-)) >=20 > I'm trying to reactivate my old Lost Retransmission Detection patch. >=20 > And I found your 2015 patch around sacked_bytes (RFC6675) here >=20 > https://github.com/rscheff/freebsd/commit/d3c29dd43ff23e293d5988258493 > 015fb4a072ba >=20 > (The line ~389 in tcp_sack.c) >=20 > But I think the code is problematic in two reasons: >=20 > a) very thin clients, which may only send SACK deltas instead of full=20 > excerpts of the scoreboard (e.g. only a sack block covering the most=20 > recent received data) > b) whenever the receiver scoreboard has more discontinuous entries than w= hat a single ACK can carry as SACK fields (typically 3, possibly 4 or as li= ttle as one). >=20 > However, these will overestimate the outstanding data (pipe). >=20 > (a) A cheating receiver sending multiple, identical sack blocks could gam= e the sender, though, as the check is done on a ack sack block by block che= ck, rather than when the scoreboard is updated... >=20 >=20 > Reason: Your patch only looks at the SACK data contained in the most rece= nt ACK, rather than proper accounting of the non-sack holes in the scoreboa= rd. Yes, you are right. And I tried to note this in code/commitlog/code-review = somewhere. >=20 > When I improved Aris PRR code, I did the accounting when the scoreboard i= s being updated; sacked_bytes need to exclude any snd.una move to the right= , but should be similar: >=20 > https://github.com/rscheff/freebsd/commit/4a4877478fe55e8ebdfa0daf9a2b > e4b212d07cde >=20 >=20 > As PRR needs a proper value for the delivered delta bytes per ACK, it sho= uld be simple to keep track of the correct value for the SACKed bytes (not = holes) in the scoreboard too... >=20 > Obviously, this can not be that much of an issue, as RFC6675 pipe is disa= bled by default, and would underestimate the number of sacked bytes at the = receiver, unless the receiver has malicious intent... >=20 > Any thoughts? I'll try to look at the patch as time permits and it may take a long time. = I am ccing Jonathan who was involved iirc in the discussion of better 6675 = support. As your patch also touches tcpcb, it needs more eyes. NF may have a bunch o= f changes here so best to coordinate with them. I guess you can open a phabricator review request and go from there? Cheers, Hiren
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CY4PR0601MB37156234E44BE5B6D45A8F3386BF0>