Subject: [bess] Éric Vyncke's No Objection on draft-ietf-bess-srv6-services-10: (with COMMENT)

From: Éric Vyncke via Datatracker <[email-masked]>
Date: Tue, 15 Feb 2022 07:11:06 -0800

Éric Vyncke has entered the following ballot position for
draft-ietf-bess-srv6-services-10: No Objection

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/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

[updating after the intdir review]

Thank you for the work put into this document. This protocol is important for
scalable and deployable SRv6 services. With revised -10, all my blocking
DISCUSS points are addressed, I am therefore clearing my DISCUSS ballot (the
points are kept below for archiving). As replied by email, the -10 also
addresses most of my previous COMMENTs.

Special thanks to Matthew Bocci for the shepherd's write-up including the
section about the WG consensus and document history.

Thanks as well for Ron Bonica's INT directorate review
(<https://datatracker.ietf.org/doc/review-ietf-bess-srv6-services-10-intdir-telechat-bonica-2022-02-14/>),
I am satisfied to see the authors exchanging with Ron on his review.

I hope that this helps to improve the document,

Regards,

-éric

# previous DISCUSS (for archiving)

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.

## 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).

## 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.

# 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). "

## 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.

## 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...

## 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).

5th § "the ingress PE encapsulates the payload in an outer IPv6 header", is it
"payload" of the ingress packet or the whole packet itself ?

## 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.

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.

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 § ?

- 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.

In short, readers will benefit from a shorter and clearer 3rd paragraph.

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 ?

I note that the security directorate review result is "ready".