Hi, I have looked at the changes. I think they adress the issues I saw when reviewing. Thanks Magnus On Fri, 2021-04-09 at 06:22 +0000, Zafar Ali (zali) wrote: > Hi Magnus, > > Many thanks for your comments and detailed review. > > We just posted version 10 to address your comments. > https://datatracker.ietf.org/doc/html/draft-ietf-6man-spring-srv6-oam-10 > > Please see [ZA] in-line on how your comments are addressed. > > Thanks > > Regards … Zafar > > From: Magnus Westerlund <magnus.westerlund@xxxxxxxxxxxx> > Date: Tuesday, March 16, 2021 at 6:42 AM > To: "last-call@xxxxxxxx" <last-call@xxxxxxxx> > Cc: "draft-ietf-6man-spring-srv6-oam@xxxxxxxx" < > draft-ietf-6man-spring-srv6-oam@xxxxxxxx>, "ot@xxxxxxxxx" <ot@xxxxxxxxx>, " > ipv6@xxxxxxxx" <ipv6@xxxxxxxx>, "6man-chairs@xxxxxxxx" <6man-chairs@xxxxxxxx>, > "tsv-art@xxxxxxxx" <tsv-art@xxxxxxxx> > Subject: RE: Last Call: <draft-ietf-6man-spring-srv6-oam-09.txt> (Operations, > Administration, and Maintenance (OAM) in Segment Routing Networks with IPv6 > Data plane (SRv6)) to Proposed Standard > Resent-From: <alias-bounces@xxxxxxxx> > Resent-To: <zali@xxxxxxxxx>, <cfilsfil@xxxxxxxxx>, < > satoru.matsushima@xxxxxxxxxxxxxxxx>, <daniel.voyer@xxxxxxx>, < > mach.chen@xxxxxxxxxx> > Resent-Date: Tue, 16 Mar 2021 03:42:05 -0700 > > This document has been reviewed as part of the transport area review team's > ongoing effort to review key IETF documents. These comments were written > primarily for the transport area directors, but are copied to the document's > authors and WG to allow them to address any issues raised and also to the > IETF > discussion list for information. > > When done at the time of IETF Last Call, the authors should consider this > review as part of the last-call comments they receive. Please always CC > tsv-art@xxxxxxxx if you reply to or forward this review. > > > A) Section 2.1: > > I first missed that "SRH.Flags" is intended as reference to the flags field > in the Segment Router Header. As this notation is prevalent through out the > document, could you be more explicit about this notation and also maybe be > clear on what a particular label represents. > > [ZA] Updated text in the Section 2.1 to make it clear that the draft defines a > flag in the Flags field of SRH [RFC8754]. > > B) Section 2.1.1: > > When N receives a packet whose IPv6 DA is S and S is a local SID, the > line S01 of the pseudo-code associated with the SID S, as defined in > section 4.3.1.1 of [RFC8754], is modified as follows for the O-flag > processing. > S01.1. IF SRH.Flags.O-flag is set and local configuration permits > O-flag processing THEN > a. Make a copy of the packet. > b. Send the copied packet, along with a timestamp > to the OAM process for telemetry data collection > and export. ;; Ref1 > > I would note that this results in the following psudo code as it states > that it modifies line S01. > Did you intended an insert so that S01.1 would be between S01 and SO2? > > [ZA] Yes, the S01.1 is inserted after between S01 and S02. Specific text > stating the same is added to the Section 2.1.1. > > This > Psudeo code just lost its scoping to SRH. I would also note that it doesn't > match the style of what is in RFC 8754. For example a { } the steps a and b > should be present. Also with the modification in place you also have a > spurious } at the end. > > [ZA] Added the RFC8754 style bracket { }. The added text has its own block of > brackets { }. With the comment that the line S01.1 is inserted after between > S01 and S02, the modified pseudo-code is now well contained in its own block, > as follows: > > S01. When an SRH is processed { > S01.1. If O-flag is set and local configuration permits O-flag processing { > a. Make a copy of the packet. > b. Send the copied packet, along with a timestamp > to the OAM process for telemetry data collection > and export. ;; Ref1 > } /* Matches S01.1 */ > S02. If Segments Left is equal to zero { … } > … > S26. } /* Matches S01 */ > > S01.1. IF SRH.Flags.O-flag is set and local configuration permits > O-flag processing THEN > a. Make a copy of the packet. > b. Send the copied packet, along with a timestamp > to the OAM process for telemetry data collection > and export. ;; Ref1 > S02. If Segments Left is equal to zero { > S03. Proceed to process the next header in the packet, > whose type is identified by the Next Header field in > the routing header. > S04. } > S05. Else { > S06. If local configuration requires TLV processing { > S07. Perform TLV processing (see TLV Processing) > S08. } > S09. max_last_entry = ( Hdr Ext Len / 2 ) - 1 > S10. If ((Last Entry > max_last_entry) or > S11. (Segments Left is greater than (Last Entry+1)) { > S12. Send an ICMP Parameter Problem, Code 0, message to > the Source Address, pointing to the Segments Left > field, and discard the packet. > S13. } > S14. Else { > S15. Decrement Segments Left by 1. > S16. Copy Segment List[Segments Left] from the SRH to the > destination address of the IPv6 header. > S17. If the IPv6 Hop Limit is less than or equal to 1 { > S18. Send an ICMP Time Exceeded -- Hop Limit Exceeded in > Transit message to the Source Address and discard > the packet. > S19. } > S20. Else { > S21. Decrement the Hop Limit by 1 > S22. Resubmit the packet to the IPv6 module for transmission > to the new destination. > S23. } > S24. } > S25. } > S26. } > > I would also note that if you wanted more generality you could replace a and > b with > > Execute the configured OAM process. E.g. if using IPFix that would be . > > C) Section 2.1.1 > > The processing node SHOULD rate-limit the number of packets punted to > the OAM process to avoid hitting any performance impact. > > So this is protection against misconfiguration or an actual security breach > isn't it. Because no entity outside of the domain should be able to set this > on packets. > > [ZA] Indeed this is the case. We have updated the security section of the > draft addressing your comment. > > So should this limit be configured by the management system or > baked into the implementation. > > [ZA] The rate limit should be configured by the management system. Text has > been added to mandate the configurable rate limiting. > > And in the later case, isn't the nodes > capability to handle marked packets dependent on the total load across all > packets processed at least in one processing pipe? > > [ZA] An implementation may implement rate limit for O-flag independent of any > other rate limits applied at the box for any other purposes. The packets are > throttled before they reached the telemetry subsystem (or OAM process). > Section 2.1.1 text is updated to indicate the rate limit also needed to > protect the telemetry subsystem. > > And to me it appears that > the limit will be dependent on what OAM Process that has been configured to > be executed. Thus, how can this be robustly implemented so that it doesn't > interfere with the telemetry? Because if there are a mismatch between the O > flag marking entity and the rate limiting, then the telemetry usefulness > could be compromised. Can you clarify the thoughts here? > > [ZA] As these are sampled packets anyway (like in the case of IPFIX), the > controller is able to work with the subset of the packets reaching it (when > the rate limit is effective). > > D) Section 3.1 and 3.2: I have the impression that quite a lot hides behind > the "The echo reply message is IP routed." or corresponding. First of all > there need to exist a return route for the source of the PING or Trace > route. > Secondly, as this is routed without SRV the packet can take a > different return route, and in case any firewall or other filtering is in > place the reply may not make it. Is this correctly interpret here? If > correctly understood I am uncertain if any changes are needed. > > [ZA] The underlying assumption for ping/ tracing in IP network is that > responder has IP reachability back to the initiator. The drafts is relying on > the IPv6 ping and traceroute capability. Also, [RFC8754] defines the notion of > an SR domain and use of SRH within the SR domain. [RFC8754] security section > provides security mechanisms to secure the SR domain. The use of ping/ > traceroute defined in this document is restricted to an SR domain. The > firewall or other filtering are typically performed outside the trusted SR > domain. > > Cheers > > Magnus >
<<attachment: smime.p7s>>
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call