Re: [tcpm] RFC793bis AD review

[Martin Duke <[e-mail]>]

Wes,

It looks like draft-24 of 793bis gets it all, except:

Meanwhile, it appears we've uncovered a contradiction! In 3.8.5 we have
>
> If the urgent pointer is updated while the user is in "urgent mode", the
>    update will be invisible to the user.
>
> A TCP implementation MUST (MUST-32) inform the application layer
>    asynchronously whenever it receives an Urgent pointer and there was
>    previously no pending urgent data, or whenever the Urgent pointer
>    advances in the data stream.
>
>  If the latter text is correct, AND we decide to allow receding, we should
> decide what to do with recedes here.
>

I'm not holding up Last Call over this, but please fix it before IESG
Evaluation.

Thanks!


On Thu, Jul 1, 2021 at 11:24 AM Martin Duke <[e-mail]> wrote:

>
>
> On Wed, Jun 30, 2021 at 2:50 PM Wesley Eddy <[e-mail]> wrote:
>
>> Here are my replies on each finding, with proposed changes to make sure
>> you agree, before cutting another revision of the document:
>>
>>
>> On 6/24/2021 4:12 PM, Martin Duke wrote:
>>
>>
>> (Document Header) Please update the Intended Status to "Internet
>> Standard".
>>
>> There doesn't seem to be a way to force this in xml2rfc3; the 'category'
>> attribute is set to 'std' (Standards Track) and there's no distinction
>> (this looks consistent with other recent Internet Standard RFCs).
>>
>> So, I think no change is needed on this.
>>
>
> OK
>
>
>>
>> (3.8.3) R1 and R2 can be measured in retransmissions or seconds; the
>> proposed bounds are in retransmissions for R1 and seconds for R2. So if I
>> have a time-based R1, should that be 3? If I have a retransmission based
>> R2, should I compare that to the value of RTO somehow?
>>
>> This is heritage from RFC 1122.  The text on R1 mentions "at the current
>> RTO", so I think RO is definitely the intended conversion factor, but since
>> you asked, maybe it's not clear enough?  Do you think maybe in part (a) of
>> 3.8.3 we could just change:
>>
>>     R1 and R2 might be measured in time units or as a count of
>> retransmissions.
>>
>> to:
>>
>>     R1 and R2 might be measured in time units or as a count of
>> retransmissions (with the current RTO as a conversion factor, if needed).
>>
>> I like that, thanks.
>
>
>>
>> (3.8.5)  This section is unclear as to whether the sender can cause the
>> Urgent Pointer to recede (move to the left), and if so, if that MUST be
>> reported to the application like a pointer advance.
>>
>> Receding looks the same as advancing so much that it wraps.  So, if not
>> already in urgent mode, that would seem to cause a change to urgent mode,
>> while if not already in urgent mode, that would fall under the existing
>> rule of:
>>
>>     If the urgent pointer is updated while the user is in "urgent mode",
>> the update will be invisible to the user.
>>
>> So ... also given that the use of urgent pointer is deprecated anyways
>> and we don't want to cause changes to TCP in this doc, I'm proposing we do
>> nothing to clarify this, if you agree?
>>
> I don't think
>
> SEQ 5000 URG 3000
> SEQ 5001 URG 1500
>
> would have a common-sense interpretation that the urg ptr was now 7501 +
> 2^32. I share your grumpiness about sweating the useless urgent pointer,
> but it would appear there is a gap in the spec and that in principle these
> recedes could exist in the wild. This spec mostly matters for new
> implementations; maybe the safest thing is for those new implementations to
> prepare to handle a receding pointer, but that senders MUST NOT recede. I
> would welcome a quick question to the WG if that logic seems spurious to
> you.
>
> Meanwhile, it appears we've uncovered a contradiction! In 3.8.5 we have
>
> If the urgent pointer is updated while the user is in "urgent mode", the
>    update will be invisible to the user.
>
> A TCP implementation MUST (MUST-32) inform the application layer
>    asynchronously whenever it receives an Urgent pointer and there was
>    previously no pending urgent data, or whenever the Urgent pointer
>    advances in the data stream.
>
>  If the latter text is correct, AND we decide to allow receding, we should
> decide what to do with recedes here.
>
>> (3.8.6.1)
>>
>> The sending TCP peer must be prepared to accept from the user and
>>    send at least one octet of new data even if the send window is zero.
>>    The sending TCP peer must regularly retransmit to the receiving TCP
>>    peer even when the window is zero
>>
>> Please clarify that these probes only happen when there is new data, if
>> that's the intent.
>>
>> The "accept from the user" part means that new data was written already,
>> right?  If the user doesn't write data into the socket, there's nothing to
>> be sent.
>>
> I have been messing with QUIC too long and may have forgotten how this
> works. But IIRC, this text kind of confuses a few different kinds of
> transmissions with zero window probes. It might be helpful to actually
> define "zero window probe" -- a segment of one byte of data designed to
> elicit an ack with a window update. The retransmission discussion is
> related but somewhat confusing as currently written. There are four
> zero-window scenarios that I can see.
>
> 1) If there is outstanding in-window data, we will get ACKs so there is no
> need for a ZWP.
>
> 2) No send buffer data, but the application generates data while in
> zero-window. Clearly, the spec says TCP accepts a byte and sends it.
>
> 3) TCP has previously refused application data due to window restrictions,
> and now we're in zero-window. TCP needs to wake the app to send again so
> that we have material for ZWP.
>
> 4) TCP accepted some data, held it for Nagle, and then the receiver
> reneged; or all in-flight data is now out of window due to reneging. This
> data ought to constitute a ZWP whether or not it is strictly a
> "retransmission".
>
> If you believe all of my statements correct, I am happy to propose
> alternate text that is not as wordy as the above.,
>
>
> (3.8.6.1) the first paragraph says
>
> Two minutes is recommended for the retransmission interval when the
>    window is zero.
>
> but the last one says
>
> The transmitting host SHOULD send the first zero-window probe when a
>    zero window has existed for the retransmission timeout period (SHLD-
>    29) (Section 3.8.1 <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-rfc793bis-23#section-3.8.1>), and SHOULD increase exponentially the interval
>    between successive probes (SHLD-30).
>
> so is it 2 min or the RTO?
>
> Good question ... The first sentence comes from 793 and the other from
> 1122.  I think we should erase the first sentence, since the 1122 intent
> seems to be updating this (replacing the hard-coded 2 minute heuristic with
> the RTO one which adapts to reality).  Do you agree?
> Yes, I agree.
>
>>
>> (3.8.6.2.1) If I read this algorithm correctly, an un-pushed send buffer of < MSS, if D < Fs*Max(SND.WND), will sit there indefinitely without
>> being transmitted; is this intentional?
>>
>> This comes from 1122.  I think the key to preventing what you mention is
>> this part of the preceding paragraph:
>>
>>   To avoid a resulting deadlock, it is necessary to have a
>>    timeout to force transmission of data, overriding the SWS avoidance
>>    algorithm.
>>
>> So, SWS avoidance is not supposed to keep a sender waiting forever for
>> the receiver to possibly advertise a bigger window; the sender is supposed
>> to wake up and just send eventually.
>>
>
> Yes, but the override timeout is embedded in step (4), which applies PUSH
> as a condition. Maybe that PUSH condition is incorrect?
>
>>
>> (3.8.2) and (3.8.6.3) Should RFC 5681 be a normative reference, given the
>> congestion control requirement? If not, (3.8.6.3) shouldn't require the
>> reader to go there to fully implement delayed ACKs. I suggest
>>
>> s/an ACK for at least every second segment/an ACK for least every 2*MSS
>> bytes of data
>>
>> to fully state what is in 5681.
>>
>> Yeah, I think 5681 should be normative (since we also rely on the for
>> descriptions of slow start, etc. that we didn't try to duplicate within
>> 793bis), but we should also fix the "second segment" sentence to match more
>> exactly the text in 5681 about 2*RMSS.
>>
>
> OK, there's currently the pointer to 5681 about sub-MSS stuff, so it's
> technically correct, but really clunky to force people to go there when
> it's so easy to state correctly. I think we agree.
>
>>
>> <snip>
>>
>>
>> (3.10, OPEN/LISTEN)
>>
>> If active and the remote socket is specified, then change the
>>          connection from passive to active,
>>
>> I think you mean "If passive"?
>>
>> Actually, I think to be more clear this should say:
>>
>> "If the OPEN call is active ...", since it's referring to the case where
>> an active OPEN is called when the socket is in LISTEN (passive).
>>
>> It's technically correct currently (just not very clear), if you read the
>> "If active" as referring to the OPEN call rather than the socket (and it
>> must be that way since a socket in the LISTEN state is the result of a
>> passive OPEN).
>>
>
> Yes, let's clarify
>
>
>> (3.10, SEGMENT ARRIVES/LISTEN) Step 4 mentions RST but those have already
>> been handled in Step 1.
>>
>> Yeah, you're definitely right, and in both cases the action is to ignore
>> it, so this part of step 4 is irrelevant, but at least consistent.
>>
>> It looks like this is an errata to file on 793; should we do that quickly
>> and then fix this doc accordingly?
>>
> If that is better for your process, then sure.
>
>
>> *NITS:*
>>
>> <snip>
>>
>>
>> (3.1, Options) "
>>
>>    Options: [TCP Option]; Options#Size == (DOffset-5)*32; present only
>>    when DOffset > 5.
>>
>> What is this notation?
>>
>> It is a little odd, but comes from reference [61] for
>> draft-mcquistin-augmented-ascii-diagrams-08.
>>
>
> OK
>
>
>> <snip>
>>
>> I'm good with changing all of these above.
>>
>> Thank you for the detailed review!
>>
>