Hi Eric, Thanks for your review and comments/feedback. Please check inline below for responses.
We've also just posted an update to address your comments: https://datatracker.ietf.org/doc/html/draft-ietf-bess-srv6-services-10 On Tue, Feb 8, 2022 at 3:16 AM Éric Vyncke via Datatracker <[email protected]> wrote: > Éric Vyncke has entered the following ballot position for > draft-ietf-bess-srv6-services-09: 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/blog/handling-iesg-ballot-positions/ > for more information about how to handle DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-bess-srv6-services/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > Thank you for the work put into this document. This protocol is important > for > scalable and deployable SRv6 services. > > Please find below some blocking DISCUSS points (easy to address), some > non-blocking COMMENT points (but replies would be appreciated even if only > for > my own education). > > Special thanks to Matthew Bocci for the shepherd's write-up including the > section about the WG consensus and document history. > > Please also expect an INT directorate review before the IESG telechat (I > may > update this ballot accordingly). > > I hope that this helps to improve the document, > > Regards, > > -éric > > # DISCUSS > > As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a > DISCUSS ballot is a request to have a discussion on the following topics: > > ## Section 3.1 > > "IANA registry defined in section 9.2 of [RFC8986]" but there is no > section 9.2 > in RFC 8986. I guess it is section 10.2. Moreover, IANA registries are > usually > referred to via their name/URL, e.g., > https://www.iana.org/assignments/segment-routing/segment-routing.xhtml, > and not > by a section of the RFC that created them. > KT> Ack. It should have been Section 10.2. We have updated the text as below: Encodes SRv6 Endpoint behavior codepoint value that is associated with SRv6 SID. The codepoints used are from the SRv6 Endpoint Behavior registry under the IANA Segment Routing Parameters registry that was introduced by [RFC8986]. > > ## Section 3.2.1 > > Where is "locator node" defined ? "locator block" is defined in section > 3.1 of > RFC 8986 but not the node (I can only guess that this is the "N" in the > "B:N" > notation used in RFC 8986). > KT> Your understanding is correct. We have added the text below The terms Locator Block and Locator Node correspond to the B and N parts respectively of the SRv6 Locator that are defined in section 3.1 of [RFC8986]. > > ## Section 6 > > Section 9 of draft-ietf-bess-evpn-igmp-mld-proxy-16 indeed defines route > types > 7 and 8 but it uses non IPv4-only wording. So, s/IGMP join sync > route/Multicast > Membership Report Synch Route/ + same for type 8. > KT> Ack. We have fixed to use route type names as per the latest version of draft-ietf-bess-evpn-igmp-mld-proxy. > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > # COMMENTS > > ## Section 1 > > More details on the encapsulation (plain IP in IPv6 ?) will be welcome in > "The > ingress PE encapsulates the payload in an outer IPv6 header where the > destination address is the SRv6 Service SID provided by the egress > Provider Edge (PE). " > KT> The inner packet being the service traffic, will vary (IPv4, IPv6, or Ethernet) and hence is not described here. We have added text in sections 5 and 6 to refer to the H.Encaps and H.Encaps.L2 behaviors introduced in RFC8986 to explain the encapsulations. > > ## Section 3.2.1 > > The transposition field appears to be about "labels", which are not > qualified. > Should the reader assume that those are "MPLS labels" ? A reference to some > text would be welcome. Not being a SRv6 expert (but somehow knowledgeable > on > the topic), all the text about transposition is completely opaque to me. > > The section 4 appears to give some information, but there should at least > be a > forward reference to it in section 3.2.1 or even better the section 4 > should be > moved before section 3.2.1. > KT> We have qualified the word "label" as "MPLS label" in section 3.2.1. Changing the order of sections may affect readability - we now introduce the TLVs first and then explain how they are used for encoding. > > ## Section 4 > > This section would benefit from examples & figures. > > While the specification probably works well, mapping a 128-bit IPv6 SID > into > what looks like a 20-bit MPLS label looks like a smart kludge (for > compression > efficiency) but still... > KT> The last two paragraphs of this section do provide examples; they do assume a good understanding of the concerned BGP features though. > > ## Section 5 > > "...optionally insert an SRH [RFC8754] when required..." looks like an > oxymoron > to me, i.e., if it is required then it is no more optional. Same issue in > other > places (notably section 6). > KT> We have removed the "optionally" from it. > > 5th § "the ingress PE encapsulates the payload in an outer IPv6 header", > is it > "payload" of the ingress packet or the whole packet itself ? > KT> The payload is the inner IPv4/IPv6/Ethernet packet. We have clarified this. > > ## Section 10 > > The protocol defined in this document is a sibling of the EPVN and other > layer-3 VPN using BGP as a control plane. I would have expected to have a > mostly identical security section. The similarity is indeed indicated in > the > 2nd § but this 2nd § would benefit by also giving the RFC titles rather > than a > dry list of RFCs. > KT> We have provided abbreviated RFC titles. > > The 3rd § appears a little self-contradicting to my reading. The first part > rightfully confirms that SRv6 domain should be closed and makes reference > to > SRH and network-programming security sections, i.e., isolation/protection > in > the data plane. Then, the last part of this § is "Therefore, precaution is > necessary to ensure that the BGP service information (including associated > SRv6 > SID) advertised via BGP sessions are limited to peers within this trusted > SR > domain. " which seems to contradict the first part. IMHO, the words "is > necessary" is an overkill, something like "Precautions should be taken to > ensure..." would be more appropriate. > KT> Agree. Please see the further responses as well. > > To elaborate on the previous comment: > > - isn't it *exactly* the same security issues as for EPVN and other > BGP-based > VPN ? So, already covered by the 2nd § ? > KT> Yes > > - even if there is yet-another BGP leak with SRv6 SID, then what are the > consequences ? SRv6 data plane (as explained in the first sentences of > this 3rd > §) MUST be protected anyway and will protect the SRv6 domain completely, > else > the operator has more critical issues. > KT> Yes, this is correct. > > In short, readers will benefit from a shorter and clearer 3rd paragraph. > KT> We have updated the text for clarity. > > Final suggestion for the security section: what happens when the length of > all > sub-sub-TLVs exceeds the length of the sub-TLV ? And similar corner cases. > This > is of course more an implementation issue but should it be mentioned here ? > KT> This is covered in section 8 and the reference to RFC7606 explains how these errors are to be handled. Thanks, Ketan > > I note that the security directorate review result is "ready". > > > >
_______________________________________________ BESS mailing list [email protected] https://www.ietf.org/mailman/listinfo/bess
