Re: [tsvwg] WGLC for draft-ietf-tsvwg-sctp-ndata as PS, closes 22 Dec 2016

["Proshin, Maksim" <[e-mail]>]

Thanks Michael! No more comments from me.

BR, Maksim


-----Original Message-----
From: Michael Tuexen [mailto:[e-mail]] 
Sent: Monday, April 24, 2017 22:19
To: Proshin, Maksim <[e-mail]>
Cc: [e-mail]
Subject: Re: [tsvwg] WGLC for draft-ietf-tsvwg-sctp-ndata as PS, closes 22 Dec 2016

> On 24. Apr 2017, at 14:05, Proshin, Maksim <[e-mail]> wrote:
> 
> Hi Michael,
> 
> I'm ok with your proposals. I've answered your questions below inline.
Comments in-line.

Best regards
Michael
> 
> Best regards,
> Maksim
> 
> -----Original Message-----
> From: Michael Tuexen [mailto:[e-mail]]
> Sent: Monday, April 24, 2017 00:40
> To: Proshin, Maksim <[e-mail]>
> Cc: [e-mail]
> Subject: Re: [tsvwg] WGLC for draft-ietf-tsvwg-sctp-ndata as PS, 
> closes 22 Dec 2016
> 
>> On 21. Mar 2017, at 07:16, Proshin, Maksim <[e-mail]> wrote:
>> 
>> Hi Michael,
>> 
>> Basically I'm ok with all your answers and updates. 
>> I don't have any major comments but I have a few proposals below inline (by [Maksim] tag).
> Hi Maksim,
> 
> see my comments below.
> 
> Best regards
> Michael
>> 
>> BR, Maksim
>> 
>> 
>> -----Original Message-----
>> From: Michael Tuexen [mailto:[e-mail]]
>> Sent: Monday, March 13, 2017 14:00
>> To: Proshin, Maksim <[e-mail]>
>> Cc: [e-mail]
>> Subject: Re: [tsvwg] WGLC for draft-ietf-tsvwg-sctp-ndata as PS, 
>> closes 22 Dec 2016
>> 
>>> On 19 Dec 2016, at 17:16, Proshin, Maksim <[e-mail]> wrote:
>>> 
>>> Hi,
>>> 
>>> Thanks for the document! Please find below some comments. Only major comments are included while editorial/minor comments have been sent directly to Michael.
>> Hi Maxim,
>> 
>> thanks a lot for the review. I addresses the minor comments and responded to your mail.
>> See my comments in-line.
>> 
>> Best regards
>> Michael
>>> 
>>> 1. Section 1.1: "DATA chunks are not permitted when I-DATA support 
>>> has been negotiated."
>>> What exactly does it mean for implementation (both sending and 
>>> receiving sides)? I think to avoid any problems with the sending 
>>> side it should be "MUST NOT" in the sentence. For the receiving side 
>>> I suppose it means stop processing of the packet and silently 
>>> discard all further chunks in the packet, right? I think this should 
>>> be clearly stated. It could also be good to clearly state that after 
>>> association
>> I think it is clearly stated in 2.2.1:
>> 
>> If I-DATA support has been negotiated for an association, I-DATA chunks MUST be used for all user messages and DATA-chunks MUST NOT be used. If I-DATA support has not been negotiated for an association, DATA chunks MUST be used for all user messages and I-DATA chunks MUST NOT be used.
>> 
>> [Maksim] Ok, I agree that Section 2.2.1 clearly explains it for the sender. Can the same be explained for the receiver?
>> 
>> So if these rules are violated, I think sending an ABORT with protocol violation might be appropriate.
>> [Maksim] Ok. Can it be clearly stated (e.g. in 2.2.1)? I mean it 
>> would be good to state that such case should be
> OK. I added to the 'Receiver Side Considerations' the following text:
> <t>If I-DATA support has been negotiated for an association, the reception of a DATA chunk is a violation of the above rules and therefore the receiver of the DATA chunk MUST abort the association by sending an ABORT chunk.
> The ABORT chunk MAY include the 'Protocol Violation' error cause.
> The same applies if I-DATA support has not be negotiated for an 
> association and an I-DATA chunk is received.</t> [Maksim] Ok with me!
> 
>> treated as a protocol violation. I also wonder that it should be mentioned that the restart case is different and should not be treated as a violation. 
> In the restart case the rules apply based on what has been negotiated during the restart.
> I think this is the general behaviour and is not handled in a special way. So I don't we need to describe this is detail.
> [Maksim] Ok, keep it as it is. Please also see below.
> 
>> 
>>> restart which was with DATA previously but became with I-DATA after the restart, SCTP still may receive packets with DATA and they should be silently discarded due to VTag mismatch following the rules of Section 8.5. from RFC4960 (I suppose this is important to avoid any "protocol violation" reaction). 
>> Sure. But this is normal restart behavior.
>> [Maksim] The reason I've asked it is because the statement is that this is "permitted" while in the restart case it is still possible to receive DATA while I-DATA was already negotiated.
> So if originally I-DATA support was not negotiated and during the restart it was negotiated, no DATA should be received after the restart. Am I missing something?
> [Maksim] I meant the case when stale DATA is received, e.g. due to reordering. In this case it will be still the same association but with I-DATA support and new VTags and such DATA should be silently discarded according to VTag rules and not treated as protocol violation. This is what I proposed to be clarified but I don't really insist.
OK I see. But I think this is covered. The rules for association lookup and checking the verification tag are done before the chunk processing. So I think we don't need to cover that.
> 
>>> 
>>> 2. Section 2.1: 
>>> The only differences between the I-DATA chunk in Figure 3 and the 
>>> DATA chunk defined in [RFC4960] and [RFC7053] is the addition of the 
>>> new Message Identifier (MID) and Fragment Sequence Number (FSN) and 
>>> the removal of the Stream Sequence Number (SSN).
>>> The B bit now has a different (extended) purpose and also specifies if PPID or FSN is used. I think it should be stated.
>> OK. I added:
>> The Payload Protocol Identifier (PPID) and the FSN are stored at the same location of the packet using the B-bit to determine which value is stored at the location.
>>> 
>>> 3. I suppose the draft assumes that SCTP still MAY (i.e. not SHOULD/MUST) support fragmentation even with I-DATA and MUST (i.e. not MAY/SHOULD) support re-assembling. If so, I suggest to clearly state it.
>> Yes, this hasn't changed. I think we don't need to repeat this point. Especially since I have not see an implementation not supporting fragmentation...
>>> 
>>> 4. Is RFC6096 impacted by new chunks?
>> No. It only deals with old chunks. When defining a new chunk, you have to define the corresponding flags registry as we do in this document.
>>> 
>>> 5. Section 2.3.1: Support for the I-FORWARD-TSN chunk is negotiated 
>>> during the SCTP  association setup via the Supported Extensions 
>>> Parameter as defined  in [RFC5061].  Only if both end points support 
>>> the I-DATA chunk and  the I-FORWARD-TSN chunk, the partial 
>>> reliability extension can be  used in combination with user message interleaving.
>>> Not sure how implementation should behave if either of these chunks is unsupported. I think it should be clearly stated.
>>> Namely if I-DATA is supported but not I-FORWARD-TSN, I believe the partial reliability functionality should be completely off, i.e. not fallback to FORWARD-TSN. 
>>> If I-DATA is not supported but I-FORWARD-TSN is supported (strange implementation but still according to the draft as it does not prohibit the opposite), it seems both functionalities message interleaving and partial reliability should be off, i.e. there is no way to fallback to FORWARD-TSN, right?  
>> Right.
>>> Should the draft state something like "If I-DATA chunk is not used, I-FORWARD-TSN chunk MUST NOT be used."?
>> There is already text there:
>> 
>> The FORWARD-TSN chunk MUST be used in combination with the DATA chunk and MUST NOT be used in combination with the I-DATA chunk.  The I- FORWARD-TSN chunk MUST be used in combination with the I-DATA chunk and MUST NOT be used in combination with the DATA chunk.
>>> 
>>> 6. Section 4.1: 
>>> MID is 32 bits while SSN is 16 bits so why a new SCTP Receive Information Structure cannot be introduced instead of using the existing one?
>> One could add such a structure, but is it needed? I don't see a use-case...
>> [Maksim] Isn't the drawback of the current approach is that 16 bits are lost?
> I don't see why this would be critical... 
> [Maksim] Perhaps it depends on how the MID value is used by the 
> application. When SCTP implementation reports just a half of MID, it 
> actually means that another value is reported, isn't it? Then it means 
> that we have two different
Yes, the lower 16 bits. It is easy to get from the MID the value being reported.
Just as if DATA chunk would be used.
> values: one on the application level and the other one on the SCTP level which might be a problem for e.g. troubleshooting. I understand that it is too late to change the concept but I wonder if the drawback (if there is really a drawback) is mentioned in the draft.
I don't see a problem...
> 
>>> If the new SCTP Receive Information Structure is introduced then a new message receive function should also be introduced (instead of recv_msg() ). Is that the main problem?
>> recvmsg() is generic. It doesn't need to be changed. You would add another CMSG.
>>> Of course, it would also be a problem if the interleaving functionality requires enabling only on the sender side (see the next comment).
>> No, it is required to be enabled on both sides.
>>> 
>>> 
>>> 7. Section 4.3.1:
>>> User message interleaving is disabled per default.
>>> I'm not sure about it. First of all I think it should be clearly stated what does it mean for the sender and for the receiver.
>>> If the intention was that I-DATA is included to INIT and INIT ACK only when the user enabled this functionality then I think this is not a good way because then it impacts not only the sender but also the receiver. 
>>> In other words, if the feature is disabled by default on both sides while SCTP on both sides support it then it is not enough to just enable it on one side, it requires enabling it on the other side.  
>> Exactly. I think this is stated in the first sentence of 2.2.1.
>> [Maksim] I think it is not clearly stated in the document. "Supported" does not really mean "enabled". I would clearly state that this functionality should be enabled on both sides and only in this case it could be successfully negotiated. Especially it is important to state for the receiver side because Section 2.2.1 has the following statement "A sender MUST NOT use the I-DATA chunk unless the user has requested its use (e.g. via the socket API, see Section 4.3)." 
> I can change the wording to make sure that I-DATA support is negotiated only if both sides indicated its support. And for each side user consent is required to indicate its support (in case the socket API is used). Would that address your issue?
> [Maksim] Yes!
OK. Will do.
> 
>>> 
>>> 7. Section 4.3.1:  Why don't specify exactly value 1 for the enabled feature? Otherwise, in the future it will be difficult to use another value of this parameter for a different purpose as it will be non-backward compatible (old application and new SCTP may work improperly).
>> Since it is the standard way of encoding a flag in an int...
>>> 
>>> 8. Section 5 does not cover I-FORWARD-TSN.
>> Good point. I added corresponding text.
>>> 
>>> 9. Section 6: In case of DTLS over SCTP support of the message interleaving functionality may increase the risk of receiving messages with new epoch after ChangeCipherSpec message and before Finished message.
>> Possibly. But implementations have to deal with it anyway. So what is the point?
>> [Maksim] Mainly my point was that it could be clarified so that future SCTP implementations of this draft/RFC supporting DTLS could consider it. 
> Thinking about it... Why do you think that using I-DATA would increase the risk?
> Both messages are small and sent back to back. Other messages can already be sent on other streams. So the situation stays the same. Implementations have to deal with this scenario.
> [Maksim] Ok, I don't insist to clarify it. 
OK. I'll submit a new revision.

Thanks for the feedback.

Best regards
Michael
> 
> Best regards
> Michael
>>> 
>>> 
>>> BR, Maksim
>>> 
>>>> Am 08.12.2016 um 08:36 schrieb [e-mail]:
>>>> 
>>>> 
>>>> This mail announces a TSVWG Working Group Last Call (WGLC) on:
>>>> 
>>>>          Stream Schedulers and User Message Interleaving forSCTP
>>>>          draft-ietf-tsvwg-sctp-ndata 
>>>> https://datatracker.ietf.org/doc/draft-ietf-tsvwg-sctp-ndata
>>>> 
>>>> This WGLC will last for 2 weeks weeks.
>>>> 
>>>> Comments should be sent to the [e-mail] list, although purely 
>>>> editorial comments may be sent directly to the authors. Please cc:
>>>> the WG chairs at [e-mail]  if you would like the 
>>>> chairs to track such editorial comments as part of the WGLC process.
>>>> 
>>>> No IPR disclosures have been submitted directly on 
>>>> draft-ietf-tsvwg-sctp-ndata.
>>>> 
>>>> Thanks,
>>>> Gorry
>>>> (TSVWG Co-Chair)
>> 
>