Ben, Thank you for the review . Some comments below. Luca
On 10/04/11 16:13, Ben Campbell wrote: > I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, > please see the FAQ at > <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. > > Please resolve these comments along with any other Last Call comments you may > receive. > > Document: draft-ietf-pwe3-static-pw-status-08 > Reviewer: Ben Campbell > Review Date: 2011-10-04 > IETF LC End Date: 2011-10-05 > > Summary: The draft is almost ready for publication as a proposed standard, > but there are a few minor issues and editorial issues that need addressing > first. > > Major issues: > > None > > Minor issues: > > -- 5.3: > > Has the work group considered how the retransmit scheme and 30 second refresh > default will scale to very large deployments? Seems like this could result in > a lot of retransmissions. Yes. that is correct. This will most likely not scale for large deployments. We have another document draft-ietf-pwe3-status-reduction-00.txt that addresses this issue. That extension is not necessary for most common small deployments in the order of 100 PWs per access PE. > -- 5.3, last paragraph: " This will cause the PE sending the all clear > message to stop sending, and to continue normal operation." > > Where is that specified? Can you offer a reference? An "all clear message" is just a pw status message with status value =0 . I'll make this clear in the text. > -- 5.3.1, 1st paragraph: "The receiving PE will then set its timeout timer > according to the timer value that is in the packet received, regardless of > what timer value it sent." > > It's probably worth making a normative statement here, since it seems like > this could easily result in an argument if the PEs disagree on the timer > value. An argument between who ? I see a problem is the fact that we need to state a good practice implementation policy. Basically the PE should not insist on the value that was just refused. I'll add some text to clarify this. What that what you intended ? > -- 5.3.1, 2nd paragraph: > > Please elaborate on "match exactly". What uniquely matches a message to an > ack? How do you compare them? > > "… the sender PE MUST use the new timer value received." > > Doesn't this contradict the previous paragraph that lets the sender disagree? > Is the subsequent treated different than the first attempt? (If so, is there > a state machine that could be elaborated on?) no, this text is redundant, I'll remove it. > -- 5.3.2: > > I don't understand the guidance here. Are you saying a PE should send status > even when it can't? yes, this section is redundant, and adds no value. I'll remove it. > -- 5.5, 1st paragraph: "… MUST be supported by both T-PEs to be enabled." > > How does one T-PE know another supports this? Via a bit in the interface parameters as explained in sec 5.5.1. > -- 5.5, last paragraph: > > This could use some elaboration. What is the purpose of having to send both > ways? > > Keep the state of S-PEs in sync between LDP and the in-band bypass mode. That is what is stated here. S-PEs are PE that are in the middle of the path. They can also originate PW status messages , but only using LDP. There is fairly complicated state machine described in rfc6073 that would break if the messages are not sent in LDP as well. > Nits/editorial comments: > > -- general: > > IDNits returns some comments. I think something about the header format is > confusing it. Yes, I think that there is a bug in IDNits. I cannot figure out what is causing this. > -- abstract: > > Please expand BFD on first mention fixed. > -- Section 2: > > Please expand LDP and PE on first mention. (even though they are in the > terminology section, since they are mentioned here before that section.) fixed. > -- section 2: "… without control plane." > > Missing article ("a" or "the") fixed > -- section 4: > > Please expand PSN, MPLS-TP, and BFD on first mention. fixed. > -- section 5.1, 2nd to last paragraph: "...PW OAM Message code" > > Missing "the" fixed. > -- section 5.1, last paragraph: > > This seems redundant with the IANA considerations section. yes - removed. > -- 5.2, 1st paragraph: "PW Status messages are indicated…" > > Indicated by what? (passive voice obscures responsible actor). reworded this text. > -- same paragraph: "PW Status TLV defined in [RFC4447]. The PW Status TLV > format is as follows:" > > I'm confused is defined in 4447 and just repeated here, or defined here? If > the first, please be very clear about that, so there's no question about > where the normative definition lives. For example: "PW Status TLV defined in > [RFC4447], and is repeated here for the reader's convenience:" yes- fixed. > -- 5.2, last paragraph: "PW OAM Status Messages MUST NOT be used as a > connectivity verification method." > > That sounds like it belongs in the "applicability" section. I think that it's missing an "also" here. "PW OAM Status Messages MUST NOT be also used as a connectivity verification method." > -- 5.3, 2nd paragraph: "...will be transmitted immediately." > > Transmitted by what? (passive voice obscures responsible actor). fixed. > -- 5.3.1, 1st paragraph: "The timer value set in the reply packet SHOULD then > be used as the new transmit interval." > > By what? (passive voice obscures responsible actor). fixed. > -- 5.3.1, last paragraph: "standard procedures" > > Reference? yes rfc4447. > -- 5.5.1, last paragraph: "If Bit "B" is set , then the T-PE can receive S-PE > bypass status messages in the G-ACH. If Bit "B" is not set the T-PE MUST NOT > send S-PE bypass status messages in the G-ACH." > > I think the sender and receiver are mixed up here. The text seems to say if > the bit is set you may receive but if the bit is not set you must not send? re-wrote the paragraph: If the T-PE receives an LDP label mapping message containing a generic protocol flags interface parameter TLV with the bit "B" set, then the T-PE receiving the label mapping message MAY send S-PE bypass status messages in the G-ACH. If Bit "B" of said TLV, is not set, or the TLV is not present, then the T-PE receiving the label mapping message MUST NOT send S-PE bypass status messages in the G-ACH. > -- 8 : IANA Considerations > > It would help to include the formal definition tables here, or reference them > here. Also, can you include the URLs for each registry? Tables have always been called out by name. What are "formal definition tables" ? I do not understand this comment. Iana section is quite clear. > 0x0022 appears to be already assigned to MPLS-TP CC message. Iana will have to sort that out then. > "Pseudowire Switching Point PE TLV Type"" > > I don't find that registry. Did you mean sub-type? It was missing the "sub-" in the TLV , fixed. > 0x16 appears to be already assigned to "Stack capability" > > > I fixed the rest of the IANA The PWE3 chairs and IANA will have to resolve the conflicts. > _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
