Francesca, thanks for your detailed review. Peter, thank you for addressing Francesca’s comments. I’ve entered a No Objection ballot.
Alissa > On Jul 17, 2018, at 6:22 AM, Dawes, Peter, Vodafone Group > <[email protected]> wrote: > > Hello Francesca, > Thanks a lot for reviewing the draft, our responses in-line below. We have > submitted revision -12 > (https://datatracker.ietf.org/doc/draft-ietf-insipid-logme-marking/) which > incorporates these responses and we believe resolves all of the issues. > > Best regards, > Peter and Arun > >> -----Original Message----- >> From: Francesca Palombini <[email protected]> >> Sent: 06 July 2018 12:30 >> To: [email protected] >> Cc: [email protected]; [email protected]; >> [email protected] >> Subject: Genart last call review of >> draft-ietf-insipid-logme-marking-11 >> >> Reviewer: Francesca Palombini >> Review result: Almost Ready >> >> I am the assigned Gen-ART reviewer for this draft. The General Area >> Review Team (Gen-ART) reviews all IETF documents being processed by >> the IESG for the IETF Chair. Please treat these comments just like >> any other last call comments. >> >> For more information, please see the FAQ at >> >> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. >> >> Document: draft-ietf-insipid-logme-marking-11 >> Reviewer: Francesca Palombini >> Review Date: 2018-07-06 >> IETF LC End Date: 2018-07-10 >> IESG Telechat date: Not scheduled for a telechat >> >> Summary: This draft is on the right track but has open issues, >> described in the review. >> >> Major issues: - >> >> Minor issues: The following are, in order of importance, the issues I have. >> Following, I tried to point out the specific details in the doc, >> which, when specified, might refer to one of those more general issues. >> >> *A) Although the scenarios shown (4.1.2.x and 5.1.x) are very >> descriptive and useful, it is quite difficult to extrapolate from the >> examples what is the required behavior of UAs or proxies supporting >> the "log me" marking. For example, Section 4.5.2.5 really gives >> requirements/rules on Proxy 1 (from the originating Network), although >> from the way the document is structured, at first glance one could >> think the section is about terminating network requirements. Some >> rules are given in Section 4.2 and 4.3, but they seem to cover only >> the stateless interaction. I think some additional text (possibly >> introducing stateful processing, i.e. the first paragraph of 4.5.2 at >> the same >> time) with rules for stateful processing is missing and could be added there. > > We added explanation text at the beginning of Section 4.5.2 "Stateful > processing" to fully describe the reasons for the behaviour of stateful > intermediaries. The new text states which entities are impacted by each > stateful processing case and includes a reference to the Section that > illustrates the case. > >> >> *B) It is said in the doc that UAs or intermediaries MAY mark dialogs >> that are "related" to the originally marked dialog. A pointer to what >> dialogs are related to each other would be useful: from the doc, the >> example is REFER relates to INVITE, and INVITE relates to REFER, any >> else? is there a section in a specification that could be referenced, >> or a "rule" given (for example dependent on local-UUID value, or other)? > > In section 3.7. "Marking Related Dialogs" added text that describes a > "related dialog" and includes a reference to RFC 7989 section 6 > >> >> *C) About error cases (section 5) I have several concerns: >> >> - I don't see anywhere that those two described (marker missing from a >> previously marked dialog and marker appearing mid dialog) are the >> comprehensive list of error cases. Is that correct? What if a marker >> disappear and then reappears? Is the intermediary supposed to keep >> logging >> (because the marker did not appear mid-dialog) or not? > > Added new section 5. "Error Cases" with a summary list of all error types and > a new section 6. "Non-Error Cases" to describe exceptions. All errors can be > considered as either a missing marker or a marker that appears when it should > not, which was already described but the summary list adds the case you > mention and also a marker missing from a message retransmission. >> >> - The document explains that detecting error cases is difficult, and only >> possible for stateful intermediaries. I'd like to see explicit >> requirements >> for stateful intermediaries to be able to detect errors, and that such >> requirements cover the comprehensive list of error cases described above. >> >> - I seem to understand that the only consequence of an "error" case (for >> the only 2 cases listed so far) is "stop logging and stop marking". Is >> that >> correct? What about previously logged messages in the dialog? >> Should those >> be kept? Deleted? It is not specified. This could be part of some security >> considerations. > > Also, we restructured the error cases section 5. to clearly spell out how > errors are detected and then added a new subsection 4.6 "error handling" to > the end of the SIP entity behaviour section to define the error handling > requirements for SIP entities. The text in 4.6 was previously in the error > cases descriptions in section 5 but fits better in Section 4 because it > specifies SIP entity behaviour. > > The new Section 4.6 "Error Handling" subsection of SIP entity behaviour also > has new text stating that previously logged messages are retained. > >> >> *D) I don't think Section 4.5.2.1.1 is at the right place. I think >> such a section would fit better in the error handling section (Section >> 5) to explain why some "missing markers" are errors while others >> aren't. Also, it actually shows an example where log in marking is in >> fact supported by the originating UA, so it should definitely not be >> under section 4.5.2.1, at worse it should be under >> 4.5.2.2 (that is probably just an overlook). > > Section 4.5.2.1.1 is moved to section 6. "Non-Error Cases" as it is normal > (non-error) behaviour. > >> >> *E) Terminology section missing: the drafts including the terminology >> are referenced in the document, but it would have been good to have a >> section in the beginning (sub-section of introduction is common) >> mentioning which ones these are. At least RFC7989, possibly also RFC8123. > > So far we didn't make a change. We believe that our responses to your other > comments mean that all cases in the draft where new terminology appears now > has a reference to explain that terminology, e.g. test case identifier, > dialog-initiating request. > >> >> *F) I would suggest a reformulation of the rules in 4.2 and 4.3 to >> have a >> separation: originating endpoint rules, then terminating endpoint >> rules for >> 4.2 and stateless intermediary, stateful intermediary in 4.3. This is >> just a suggestion for better readability in my opinion, feel free to >> disregard. >> > > The lists of rules in sections 4.2 and 4.3 are now divided into originating > or terminating and user agent or intermediary as per the comment. > >> Detailed Comments: >> >> This document defines a new header field parameter "logme" for the >> "Session-ID" header field. >> >> Good with a reference for "Session-ID" header field, at the end of >> this sentence. --- > > In Section 1 "Introduction", the reference to RFC 7989 moved to end of first > sentence as commented. > >> >> as a "log me" parameter since the session identifier is typically >> passed through SIP B2BUAs or other intermediaries, as per the >> Session-ID requirement REQ3 in ([RFC7206]). The "logme" parameter >> >> For B2BUA, see point E. >> --- > > To explain the term B2BUA, when it first appears in section 3.1 we added a > reference to RFC 7092 https://tools.ietf.org/html/rfc7092 (A Taxonomy of > Session Initiation Protocol (SIP) Back-to-Back User Agents) . > >> >> Marking starts with a dialog-initiating request and continues for the >> lifetime of the dialog, and applies to each request and response in >> that dialog. >> >> Can a ref be added for which are the "dialog-initiating request"? Or >> put it in terminology (point E). --- > > In 3.2. "Starting and Stopping Logging" a reference is added to section 12.1 > of RFC 3261 to describe a dialog-initiating request. > >> >> A user agent or intermediary adds a "log me" marker in a request or >> response in two cases: firstly because it is configured to do so, or >> secondly because it has detected that a dialog is being "log me" >> marked, causing it to maintain state to ensure that all requests and >> responses in the dialog are similarly "log me" marked. Once the >> "log >> >> Can we change to: "A user agent or intermediary adds a "log me" marker >> in an unmarked request or response in two cases: firstly because it is >> configured to do so, or ...". --- > > In 3.2. "Starting and Stopping Logging" changed "adds a 'log me' marker in a > request..." to "adds a 'log me' marker in an unmarked request..." as > commented. > >> >> Section 3.2: Can you add some text about an intermediary that does not >> support marking? It seems hinted from the following examples that it >> would just remove it. Can it be made explicit in this section? --- > > Added a new subsection of section 3.4. "Passing the Marker". Section 3.4.3. > "Across a Non-Supporting SIP Intermediary" explains that "log me" might be > forwarded or dropped by a non-supporting intermediary. > >> >> If a request or response is "log me" marked, then all re- >> transmissions of the request or response MUST be similarly "log me" >> marked. Likewise, re-transmissions of a request or response that was >> not "log me" marked MUST NOT be "log me" marked. >> >> What if the MUST NOT is not complied with? What should the endpoint or >> intermediary do in that case? I assume error, but it should be >> specified. (See point C.2 above) --- > > Missing log me marker in retransmissions is added to the list of error cases > in section 5. "Error Cases". > >> >> marking extends to the lifetime of the dialog. In addition, as >> discussed in Section 3.7, "log me" marked dialogs that create related >> dialogs (REFER) may transfer the marking to the related dialogs. In >> such cases, the entire "session", identified by the Session-ID >> header, is "log me" marked. >> >> See point B. >> --- > > Covered by added text that describes a "related dialog" and includes a > reference to RFC 7989 section 6 in response to point B > >> >> The local Universally Unique Identifier (UUID) portion of Session-ID >> [RFC7989] in the initial SIP request of a dialog is used as a random >> test case identifier. This provides the ability to collate all >> >> Although it is pretty much clear what it means, it sounds like test >> case identifier is defined somewhere? Ref? Terminology? (point E) --- > > In order to define a test case identifier, added a reference to REQ5 in RFC > 8123 at the end of 3.3. "Identifying Test Cases". > >> >> creation and ends when the dialog ends. However, dialogs related to >> a "log me" marked dialog MAY also be "log me" marked. An example >> is >> >> I guess that MAY depend on a policy in place. Can that be explicit? >> --- > > So far we didn't make a change. We don't think this is defined by an > individual policy. SIP devices that are compliant to RFC 7989 will typically > add Session-ID in related dialogs that are considered to be part of the same > communication session. > >> >> transfer. Alice's terminal inserts a "logme" marker in the REFER >> request and 200 OK responses to NOTIFY requests in dialog2. Both >> >> See point B. >> --- > > The reference to RFC 7989 added to Section 3.7 points to an explanation of > signalling and SIP messages for related dialogs. > >> >> dialog3 is not logged by Alice's terminal, however the test case >> identifier ab30317f1a784dc48ff824d0d3715d86 is also the test case >> identifier local-uuid) in INVITE F5. Also, the test case identifier >> of dialog2, which is logged by Alice's terminal, can be linked to >> dialog1 and dialog3 because the remote-uuid component of dialog2 is >> the test case identifier ab30317f1a784dc48ff824d0d3715d86. >> >> This is more for my own understanding: is this the case all the time >> in SIP? Or is this what this doc specify? If it is the latter, then it >> should be probably clarified. Maybe it would be good to clarify >> anyway. --- > > The behaviour in section 3.7. "Marking Related Dialogs" is how SIP devices > that support Session-ID will behave because they will follow Section 6 of RFC > 7989. > >> >> Section 4.2 and 4.3: Don't see any rule for related dialogs. I >> understand that is optional, but I think it should be here (stated as >> optional). (see point B) >> --- > > Added four new bullets in the lists of rules in 4.2 and 4.3 for related SIP > dialogs. The four bullets cover originating or terminating UAs and > originating or terminating intermediaries. > >> >> The retention time period for logged messages should be the minimum >> needed for each particular troubleshooting or testing case. The >> retention period is configured based on the data retention policies >> of service providers and enterprises. >> >> should -> SHOULD? >> --- > > Capitalized SHOULD in 7.4.5 "Retaining Logs" > >> >> Consent to turn on "log me" marking for a given session must be >> provided by the end user or by the network administrator. It is >> >> may -> MAY? >> --- > > Capitalized MUST in 7.4.6. "User Control of Logging". > >> >> Nits/editorial comments: >> >> when this dialog and it's related dialogs end. It is considered an >> >> it's -> its >> --- > > Corrected it's to its in section 3.2 "Starting and Stopping Logging" text > "when this dialog and its related dialogs end". > >> >> A SIP intermediary MUST copy the "log me" marker into forked >> requests. >> >> Can a ref be added to the spec describing forked requests? >> --- > > In 3.8. "Forked Requests", added a reference to RFC 3261 sections 4, 16.6. > >> >> o The originating user agent itself logs signaling. >> >> Change to: The originating user agent itself logs marked signaling requests. >> Additionally, see point F. --- > > In 4.2 and 4.3, changed "requests" to "marked requests" in four of the rules > bullets > >> >> echoed by a terminating UA. SIP intermediaries on the signaling path >> between these UAs that do not perform the tasks described in >> Section 4.5.2 can simply log any request or response that contains a >> "log me" marker in a stateless manner, if it is allowed per local >> policy. >> >> That can should be a MUST, I think. >> --- > > Changed "can" to "MUST" in 4.5. "Stateless SIP Intermediaries". > >> >> Section 4.5.2.1: >> >> F12 - Proxy 1 receives ACK with with no "log me" marker. It doesn't >> consider this as an error since it is configured to "log me" mark on >> behalf of Bob's UA. >> >> Bob -> Alice >> --- > > Corrected Bob to Alice in message F12 in Figure 3. > >> >> 4.5.2.3: Typo in the figure: after F2, missing characters on the lines >> of Proxy >> 2 and Bob. Same just before F19. --- >> >> requests and responses entering network B. However, Proxy 2 supports >> maintains the marking state of the dialog and "log me" marks >> requests >> >> remove supports (or maintains) >> --- > > Corrected the spacing in two places in Figure 4. Also changed "supports > maintains" to "supports maintaining". > >> >> 4.5.2.4: Typo in the figure: wrongly aligned characters under F9. >> --- > > Corrected alignment in Figure 6. > >> >> 4.5.2.4: >> >> F2 - Proxy 2 removes "log me" marker in the INVITE request F2 before >> forwarding it as F7. >> >> F7 -> F4. Also, maybe add "The same applies to F13 and F19", to be >> consistent. >> --- > > In Figure 6, corrected F7 to F4 and added text about F13, F19. > >> >> 4.5.2.5: Typo in the figure: wrongly aligned characters under F9. >> --- > > Corrected character alignment in Figure 7. > >> >> 4.5.2.5: >> >> F2 - Proxy 2 removes "log me" marker in the INVITE request F2 before >> forwarding it as F7. >> >> F7 -> F4. Also, maybe add "The same applies to F13 and F19", to be >> consistent. >> --- > > In Figure 7, corrected F7 to F4 and added text about F13, F19. > >> >> 7.4.4: "Log me" in the beginning of the paragraph, but 7.4.5 second >> paragraph starts with "log me". Which one is it? (I personally have no >> preference). Same for "log me" marking and "log me" Marking (in 7.4.5) >> >> --- > > "Log me" is capitalized when it is the start of the sentence so we corrected > "log me" to "Log me" at the beginning of paragraph 2 in 7.4.5 "Retaining > Logs". > >> (please keep my address in the to or cc in responses to this review, >> for easier >> tracking) >> >> Best regards, >> Francesca > _______________________________________________ > Gen-art mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/gen-art _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
