Hi Pablo! Thanks for the updates in -20. A few more comments inline ...
> -----Original Message----- > From: Pablo Camarillo (pcamaril) <[email protected]> > Sent: Tuesday, September 22, 2020 3:59 PM > To: Roman Danyliw <[email protected]>; The IESG <[email protected]> > Cc: [email protected]; spring- > [email protected]; [email protected]; Bruno Decraene > <[email protected]>; Joel Halpern <[email protected]> > Subject: RE: Roman Danyliw's Discuss on draft-ietf-spring-srv6-network- > programming-19: (with DISCUSS and COMMENT) > > Hi Roman, > > Many thanks for your thorough review. Please see inline with [PC]. > > Thanks, > Pablo. > > -----Original Message----- > From: Roman Danyliw via Datatracker <[email protected]> > Sent: lunes, 21 de septiembre de 2020 20:24 > To: The IESG <[email protected]> > Cc: [email protected]; spring- > [email protected]; [email protected]; Bruno Decraene > <[email protected]>; Joel Halpern <[email protected]>; > [email protected] > Subject: Roman Danyliw's Discuss on draft-ietf-spring-srv6-network- > programming-19: (with DISCUSS and COMMENT) > > Roman Danyliw has entered the following ballot position for > draft-ietf-spring-srv6-network-programming-19: Discuss > > When responding, please keep the subject line intact and reply to all email > addresses included in the To and CC lines. (Feel free to cut this introductory > paragraph, however.) > > > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html > for more information about IESG DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-spring-srv6-network-programming/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > Section 3.1. (In case I missed it, please provide the obvious reference) Per > “In > such a case, the semantics and format of the ARG bits are defined as part of > the > SRv6 endpoint behavior specification”, is “endpoint behavior specification” > Section 4 or another document? If the former, I don’t see any references to > argument bits in the pseudo-code of the Section 4.* subsections. > If the latter, what document? Can the behaviors be polymorphic (i.e., same > network behavior accepting different arguments)? > > [PC] The End.DT2M behavior in section 4.12 has an argument called “Arg.FE2”. > The behavior specification explains the semantics of the argument and the > pseudocode describes how it is handled. The arguments to the function can > indeed vary. Thanks for pointing this out. What I still don't understand is whether End.DT2M the only behavior that uses the arguments? > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > Thanks for responding to the SECDIR review (and thank you to Brian Weis for > doing it) > > ** Section 3.2. Per “An SR Source Node cannot infer the behavior by > examination of the FUNCT value of a SID”, isn’t there at least some hint > provided by the use of the FUNCT from the IANA registry? > [PC] Recall that Section 3.1 says the following (there is no hint): > The FUNCT is an opaque identification of a local behavior bound to > the SID. > The term "function" refers to the bit-string in the SRv6 SID. The > term "behavior" identifies the behavior bound to the SID. The > behaviors are defined in Section 4 of this document. > > [PC] The FUNCT is part of the SID value assigned by the operator. This SID is > associated with a behavior (defined in the IANA registry). The control plane > is > going to advertise the association of the SID (including the FUNCT) and the > particular behavior (using the IANA codepoint). > An intermediate node that only has access to the SRv6 user traffic, only by > looking at the FUNCT value (and with no control plane information), cannot > know what the behavior is associated to such SID. Looking ahead at your next > comment, I believe the confusion might arise from the values used in the > example in this section. > > ** Section 3.2. > Per “Function 11 associated with the behavior End.X …” AND > > Per “Function 12 associated with the behavior End.X …” > > The initial registrations for Endpoint Behavior per Section 10.2.1 suggest > that > Function 11 and 12 are End.T. > [PC] Please see response to previous comment. Indeed, the example can be > improved. I’ll replace 11 and 12 for 100 and 101 that are unassigned on the > IANA registry to more clearly differentiate between FUNCT and Behavior. Understood. > ** Section 4.1. Pseudo-code without formal definition might be open to > interpretation. My nits (which likely apply to other sections) are that: -- > it > might not be clear that S03, S06 and S10 are effectively returns/exits from > this > “behavior”, and if they are run, S12-S15 should never be reached (otherwise > Segments Left or Hop Limit = -1) [PC] Indeed, it would be clearer if we call > out > where there is a return/exit. > <OLD> > S03. Proceed to process the next header in the packet, > whose type is identified by the Next Header field in > the routing header. > </OLD> > <NEW> > S03. Stop processing the SRH, proceed to process the next > header in the packet, whose type is identified by the > Next Header field in the routing header. > </NEW> > > [PC] For lines S06 and S10 currently include “Interrupt packet processing and > discard the packet”. Do you think this is sufficient to indicate there is no > more > SRH processing done? I'd recommend using consistent language. Since you're using "stop" in S03, I'd recommend using the same thing in S06 and S10 > -- (for consistency) the IF … ELSEIF … END construct is used in Section 4.8, > but > not here. > [PC] Indeed, pseudocodes should be consistent. > > -- What does it mean to indent the code? For example S06 is: > S06. Send an ICMP Time Exceeded message to the Source Address, > Code 0 (Hop limit exceeded in transit), > Interrupt packet processing and discard the packet. > > I can see why “Code 0 …” is indented under “Send an … “ as this is a line > wrap, > but why is “Interrupt packet processing?” > [PC] Yes. The indentation of interrupt packet processing is incorrect. It > should > be part of the same statement as above (the period should not be there). I’ll > review the indentation of all pseudocodes. > > > ** A few other psuedo code nits: > -- Section 4.1. S05 uses “IPv6 Hop Limit” but S12 uses “Hop Limit”. > Recommend consistency. > [PC] Ack. Fixed for next next rev. > > > -- Section 4.4. Per S05. Is “next header” purposely lower case? I asked > because > it looks like explicit field names are in upper case (e.g., S03 says > “Segments Left > field”). > [PC] Ack. Fixed for next next rev. > > -- Section 4.16.3. As a style/consistency note, the other sections which > check > the upper header type seem use of a construct of != 41 or 4 to “Process per > Section 4.1.1”. (e.g., Section 4.4, 4.5, 4.6, etc.) [PC] Ack. Fixed for next > next rev. > > ** Section 8. Given how meticulously the Section 4 guidance was described, it > would be helpful to say that the HMAC TLV processing, if present, happens as a > notional Step “S0” per guidance in Section 2.1.2.1 of RFC8754. > [PC] Ack. I would propose the following addition at the end of the security > considerations. > <NEW> > When enabled by local configuration, HMAC processing occurs at the beginning > of SRH processing as defined in Sec 2.1.2.1 of RFC8754. > </NEW> > > ** Editorial Nits: > -- Section 3.2. Typo. s/an network/a network/ > -- Section 3.2. Typo. s/is minimum/is minimal/ [PC] Acking both. Fixed for > next > next rev. Thanks for all of the changes described above. Regards, Roman _______________________________________________ spring mailing list [email protected] https://www.ietf.org/mailman/listinfo/spring
