Subject: Re: [bess] Alvaro Retana's Discuss on draft-ietf-bess-nsh-bgp-control-plane-13: (with DISCUSS and COMMENT)

From: "Adrian Farrel" <[email-masked]>
Date: Sun, 31 May 2020 21:10:30 +0100

Hi Alvaro,

Here is a more considered response to your review...

> DISCUSS:
> -------------------------------------------------------------
>
> (1) Controllers and other nodes.
>
> Background: This document defines the new SFC NLRI, which has two distinct
> route types, originated by either a node hosting an SFI or a Controller.  Each
> route type has a specific function and it is reasonable to expect that the
> originators represent different nodes in the network, with different functions,
> location, etc..  BGP Capabilities Advertisement doesn't have a route type
> granularity; instead, a BGP speaker known to support the SFC NLRI could
> originate any type of route.
>
> The facts above open the possibility that any node in the network can originate
> an SFPR and take over an SFP.  §3.2.2 does a very good job of explaining the
> potential existence of multiple Controllers and even offers the appropriate tie
> breaker to take control of the SFP: "MUST use the SFPR with the numerically
> lowest SFPR-RD".
>
> The document proposes no mitigation to the possibility of any node (a rogue
> node, for example) issuing SFPRs.  The assumption (§2.2) of "BGP connectivity
> between the Controllers and all SFFs" introduces also the ability to locate a
> rogue controller anywhere; I interpret "BGP connectivity" to include the
> presence of a router reflector (for example), which then allows distribution of
> SFPRs without going through a central policy point.
>
> On one hand I think this condition is a feature (the Controller can be
> anywhere), but the case of a rogue node that wants to act as a controller is
> not considered.
>
> To address this issue, I would like to see text that (1) acknowledges the issue
> (maybe in the security considerations section), and (2) discusses operational
> considerations for the placement, control, filtering, etc. of Controllers and
> the corresponding UPDATES from them and/or other nodes in the network.  IOW,
> the considerations around proper initial setup of the system should be clear.

1. The controllers are not SDN controllers. They are not single omnipotent, all-seeing god boxes. They are boxes that control. They could be dedicated devices, management systems, distributed servers, or network devices. They are, simply put, BGP speakers. 

2. The challenges and concerns that you raise are not dissimilar to the general attack vectors in a routing system, and should be thought of in the same way. What happens if a "rogue" router starts issuing bogus routes?

So, you are right and we are highlighting it in the security section, and we can note the existing mitigations in a BGP-based routing system against rogue players.

> (2) New Flow Specification Traffic Filtering Action
>
> §7.4 (Flow Spec for SFC Classifiers) defines a new Traffic Filtering Action to
> be used with the Flow Specification NLRI.  rfc5775bis allows for any
> combination of Traffic Filtering Actions to be present, but this document says
> that "other action extended communities MUST NOT be present".  I believe that
> specifying the use of treat-as-withdraw is ok as a case of Traffic Filtering
> Action Interference -- I just say "ok" because it is not clear to me (nor
> explained anywhere) why other Traffic Filtering Actions cannot be used; for
> example, I could imagine rate-limiting the traffic into an SFP.
>
> What concerns me more (and the reason for this DISCUSS point) is that
> rfc5575-only implementations will not consider the new Traffic Filtering
> Action, but could, depending on the components encoded in the NLRI, take
> actions based on other Traffic Filtering Actions.  The result can then be an
> inconsistent application of Traffic Filtering Actions in the network -- for
> example, some nodes may want to drop the matching traffic (traffic-rate of 0),
> while others may want to have the same traffic entering an SFP.
>
> What are the operational considerations of using the new Traffic Filtering
> Action in a network where "legacy" (rfc5575bis-only) nodes exist?  Is there a
> potential migration path?  What might be the impact?  How can correct operation
> be verified?

I said:
: Hmmm, we don't tent to explain why "MUST NOT" in our specifications.
: The reasoning here is that it would be highly confusing to mix and match
: SFC Classification with BGP routing. Perhaps I am wrong about that
: confusion.
: I think that when you program an SFC classifier, that is a single peer-to-
: peer communication targeted at an SFC Classifier.
: A 5575-only node will not be a classifier.

You responded:
| I raised this point as a DISCUSS because the text is at odds with other
| standards track documents.  That is the reason I'm asking for an explanation.

An SFC overlay network consists of SFIs, SFFs, Controllers, and Classifiers. The fact that we re-use RFC 5575 in our Classifiers
does not mean that an RFC 5575-only node is a Classifier. I.e., a service provider that is installing an SFC overlay network needs
to install SFIs, SFFs, Controllers, and Classifiers.

I suspect that the way to clarify this is to explain that all 2119 language in this document applies to implementations of this document and does not change other implementations of other specifications. (Actually, I think this is normal unless there is an explicit "updates", but it doesn't hurt to clarify this point.) Would that be enough for you? That would leave us with...

        Note that implementation of this specification MUST NOT include 
        other action extended communities at the same time as an SFC Classifier: the inclusion of the
        "Flow Spec for SFC Classifiers" action extended community along with any other action MUST be
        treated by implementation of this specification as an error which SHOULD result in the Flow 
        Specification UPDATE message being handled as Treat-as-withdraw according to 
        [RFC7606] Section 2.

> (3) Use of the Control Plane
>
> This last point is not specifically for the authors, but for the Responsible AD
> and the Chairs for the sfc WG (cc'd).
>
> The SFPRs are built on, among other things, knowledge of the SFT(s) supported
> at a specific node.  I note that only one Special Purpose SFT is defined in
> this document.  The lack of SFT definitions means that no actual SFP can be
> instantiated.  IOW, without additional work to define new SFTs it seems to me
> that the control plane as specified in this document cannot be used. :-(
>
> I couldn't find any related work (referencing this document) where new SFTs are
> proposed/defined.  What are the plans to develop that work?  Is there interest
> in the sfc WG to take advantage of the control plane?

I answered...

: I do not agree that knowledge of SFTs is essential to the construction 
: of SFPs. I think it is very helpful, but I note that an initial system 
: would have good knowledge of the location and capabilities of SFIs in 
: the network, specifically because the "orchestrator" has created and 
: located them. Thus, the creator of the SFP also knows the locations and types of the SFIs.

And you said...

| Everywhere I look at (§3.2, §4.3, for example) seems to indicate to me
| that an SFT is necessary in the definition of the SPF.  I may of course be
| wrong or have missed where it clearly isn't.  An example of how to do
| it would be very useful.

We see a number of types discussed
- in this document
- RFC 7665
- https://tools.ietf.org/html/draft-dawra-idr-bgp-ls-sr-service-segments-02#section-4.1
- https://tools.ietf.org/html/draft-dawra-idr-bgp-ls-sr-service-segments-02#section-4.2

We have decided to group these all together and request population of the registry in Section 10.5.
We are discussing with the authors of draft-dawra-idr-bgp-ls-sr-service-segments precisely what we should include and what they will ask for in their own draft.

> COMMENT:
> ---------------------------------------------------------------
>
> (1) I support Ben's DISCUSS point about the extension to the SFC Architecture.

Hopefully my response to Ben also satisfies you.

> (2) rfc7665/§5.2 (SFC Control Plane) lists the expected functionality provided
> by the control plane.  The last two points in the list are not part of the
> specification in this document, but there is no explanation why or if there is
> an alternate mechanism -- these are those points:
>
>   5.  Provides the metadata and usage information classifiers need so
>       that they in turn can provide this metadata for appropriate
>       packets in the data plane.
>
>   6.  When needed, provide information including policy information to
>       other SFC elements to be able to properly interpret metadata.

That is a good and interesting question.
We weren't guided by that list, but it turns out we achieved reasonably well.
Item 6 is, perhaps, a little bit of, "We're not quite sure how metadata will work so we had better include this." In my opinion it is the "other SFC elements" that demand the metadata, and the classifiers that have to supply it. Which takes us to the much more robust item 5.

We haven't given a lot of attention to metadata because (mainly) it is an issue for the data plane. But you're right to highlight two things:
- We should give reference to this list of items in RFC 7665. We'll put this in our section 2.2.
- We should mention instructing the classifiers about metadata in our section 4.4.

> (3) Add a Normative reference to rfc4360 (BGP Extended Communities Attribute).

Yes. Good point.

> (4) §3.1.1/§3.1.2: The two new Extended Communities are defined as different
> types.  Is it really necessary to request two different types, instead of one
> type with sub-types?  The transitive space is not close to exhaustion, but
> having a single type would make it easier for any future SFC-related extended
> communities to be identified/grouped.  Just a thought...

Yeah, that's an easy change.

> (5) Operational Considerations: there are multiple pieces of the puzzle that
> need to be coordinated, from RDs and RTs to pool identifiers and the assignment
> of SFIs to pools...  It would be very nice if all the information that needs to
> be operationally managed/provisioned was mentioned in a single place in the
> document...and if recommendations for the operator where included.

We looked at this and we're going to pass. A lot of the text in the document addresses these topics, and we feel that we would either duplicate material or just drag and drop it. That's a lot of effort, and while we see the point you're making, it feels that the gain would not be so very large.

> (6) §3.2.1 says that "Each TLV may include sub-TLVs", but that is not always
> true; for example, the length of the Association TLV is specified as being 12
> (with no room for anything else).  This is a minor and easy issue to fix.

Ack

> (7) §3.2.1: "Unknown TLVs SHOULD be ignored"  When would an unknown TLV not be
> ignored?  IOW, why not use MUST?

Scrabbled around for why we wrote that and concluded that it was just polite British speak 😊
Changed to MUST.

> (8) §3.2.1.1 specifies a series of errors in the Association TLV that result in
> "SHOULD be ignored".  When would these values not be ignored, when would they
> be useful?  IOW, why not use MUST?

The alternative, which some MAY consider reasonable, is to reject the message. This is not unacceptable, but also not good for future extensibility.

Added almost exactly those words.

> (9) Please review §7.4 against the language used in rfc5575bis for consistency.
> For example "flow spec" is only used in the name of IANA registries or related
> entries; Flow Specification is used instead.

Oh yes.

> (10) Please include a pointer to I-D.ietf-idr-tunnel-encaps somewhere in §7.5.

Yes

> (11) I would really like to see a registry set up for the bits in the new
> SPI/SI Representation sub-TLV.

That is relatively cheap to achieve, so we've done it.

> (12) Other nits:
>
> s/set to loopback address/set to the loopback address

Yes, but used indefinite article.

> s/[RFC4271] defines the BGP Path attribute./[RFC4271] defines BGP Path
> attributes.

OK

Very many thanks for a very thorough review and some excellent comment.

Best,
Adrian