Re: [spring] Roman Danyliw's Discuss on draft-ietf-spring-srv6-network-programming-19: (with DISCUSS and COMMENT)

[Roman Danyliw <[e-mail]>]

Hi Pablo!

Thanks for the explanation.  I've cleared my DISCUSS.

Regards,
Roman

> -----Original Message-----
> From: Pablo Camarillo (pcamaril) <[e-mail]>
> Sent: Friday, September 25, 2020 2:29 PM
> To: Roman Danyliw <[e-mail]>; The IESG <[e-mail]>
> Cc: [e-mail]; spring-
> [e-mail]; [e-mail]; Bruno Decraene
> <[e-mail]>; Joel Halpern <[e-mail]>
> Subject: RE: Roman Danyliw's Discuss on draft-ietf-spring-srv6-network-
> programming-19: (with DISCUSS and COMMENT)
> 
> Hi Roman,
> 
> Please see inline with [PC2].
> 
> Thanks,
> Pablo.
> 
> -----Original Message-----
> From: Roman Danyliw <[e-mail]>
> Sent: jueves, 24 de septiembre de 2020 15:20
> To: Pablo Camarillo (pcamaril) <[e-mail]>; The IESG
> <[e-mail]>
> Cc: [e-mail]; spring-
> [e-mail]; [e-mail]; Bruno Decraene
> <[e-mail]>; Joel Halpern <[e-mail]>
> Subject: RE: Roman Danyliw's Discuss on draft-ietf-spring-srv6-network-
> programming-19: (with DISCUSS and COMMENT)
> 
> Hi Pablo!
> 
> Thanks for the updates in -20.  A few more comments inline ...
> 
> > -----Original Message-----
> > From: Pablo Camarillo (pcamaril) <[e-mail]>
> > Sent: Tuesday, September 22, 2020 3:59 PM
> > To: Roman Danyliw <[e-mail]>; The IESG <[e-mail]>
> > Cc: [e-mail]; spring-
> > [e-mail]; [e-mail]; Bruno Decraene
> > <[e-mail]>; Joel Halpern <[e-mail]>
> > 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 <[e-mail]>
> > Sent: lunes, 21 de septiembre de 2020 20:24
> > To: The IESG <[e-mail]>
> > Cc: [e-mail]; spring-
> > [e-mail]; [e-mail]; Bruno Decraene
> > <[e-mail]>; Joel Halpern <[e-mail]>;
> > [e-mail]
> > 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-progra
> > mming/
> >
> >
> >
> > ----------------------------------------------------------------------
> > 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?
> [PC2] Correct, the only behavior defined in this document with arguments is
> End.DT2M.
> 
> > ----------------------------------------------------------------------
> > 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 [PC2] We believe that it might
> be good to use "Stop" on S03 because we are only stopping the SRH processing,
> but proceeding to process the next headers in the chain. While S06 and S10 are
> really stopping the packet processing at all (packet will be discarded after
> generating an ICMP error). If you still believe we should use the same word in
> the three, please let me know and Ill do in the next revision of the document.
> 
> > -- (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