Subject: [bess] Shepherd's review of draft-ietf-bess-nsh-bgp-control-plane-06

From: <[email-masked]>
Date: Wed, 27 Feb 2019 10:21:51 +0000

Hi,

Here is my review of draft-ietf-bess-nsh-bgp-control-plane-06

General comment:

The document is globally well written with good examples that help the understanding. However it requires some refinements in the normative language used: some statement should be normative but are not using upper case words.

Detailed comments:

Abstract:

-          Please expand "BGP" on first use

-          Please give a reference associated to "Network Service Header"

Introduction:

-          When "classifier" is used as an example of SF, do you refer to the classifier used to steer the traffic onto an SFC ? If yes, I don't think it could be considered as an SF.

-          I think the "conventional" approach described in the intro is really old school and even before SFC, we had some means, like dynamic routing protocols and other mechanisms like separate routing tables, to steer the traffic through a set of services. I agree that there is also some drawback associated with such approaches like the operational complexity of provisioning.

Section 1.2:
It would be good to give a definition for the additional terms that are defined in this document.

Section 2.1:

·         As the section is focused on dataplane, it would be good to rename it as "Reminder on NSH dataplane" or something like that. It does not provide a functional overview of the controlplane which is what was expected based on the title.

·         "A special Service Function, called a Classifier, is located at each
   ingress point to a service function overlay network.  It assigns the
   packets of a given packet flow to a specific Service Function Path.
"
Again, based on my understanding, the Classifier cannot really be considered as an SF , at least this is a component out of the SFC which steers the traffic onto an SFC.

·         "An unknown or invalid SPI SHALL be treated as an error and the SFF

   MUST drop the packet.  Such errors SHOULD be logged, and such logs

   MUST be subject to rate limits."

I found strange to have such statement in this document which is focused on the controlplane while this statement is a dataplane statement. Isn't it part of RFC8300 Section 3 : "3.  Update the NSH: SFs MUST decrement the service index by one.  If

       an SFF receives a packet with an SPI and SI that do not

       correspond to a valid next hop in a valid SFP, that packet MUST

       be dropped by the SFF."

The next paragraph deals also with normative dataplane statement which is IMO out of scope of this document.

Section 2.2

·         "The SFIR describes a particular instance of a

      particular Service Function". Would it be good to talk about "Service Function Instance" rather than instance of a Service Function ? That's understandable, of course, however I think it's good to reuse the exact terminology.

·         I think the SFT definition is appearing a bit late in this section and in the document as it as been referenced already multiple times before.

·         "Service Function Type (SFT) that

   is the category of SF that is supported by an SFF". Don't you mean SFI rather than SFF ?

·         "Thus the SFF can be seen as a portal...". Would "gateway" be more suitable rather than "portal" ?

·         The Figure 1 is not really used in this section as part of the existing text. I would be better to have a companion text that explains the figure.

·         I don't like (personal opinion), the "grouping" of SFIs in the Figure 1 as part of an SFT. What strikes me is that it could be confused with the usual representation of an SF composed of multiple SFIs. Again that's just a personal feeling.

Section 3:

·         "they must use BGP Capabilities": is it a normative MUST ?

Section 3.1:

·         s/a two byte Type field and a six byte/a two bytes Type field and a six bytes/

·         "Two SFIs of the same SFT must be associated". Is it a normative MUST ? Same comment for next sentences in the paragraph (multiple occurences)

·         "The Service Function Type identifies a service function". I don't think we can really say that, it identifies the type of service the SF is providing but not the SF itself.

·         "Each node hosting an SFI

   must originate an SFIR for each type of SF that it hosts, and it may

   advertise an SFIR for each instance of each type of SF." Is it really "and" ? I mean can we just summarize by using " Each node hosting an SFI MUST originate an SFIR for each instance of each type of SF" ?

·         "A BGP Update containing one or more SFIRs will also include". Is it a MUST or SHOULD "also include"?

·         How is the nexthop encoded in the NLRI ?

Section 3.1.1

·         s/"It can be included"/"It MAY be included"/ ?

·         That would be great to give more details about the usage of pools

·         The encoding in Figure 4 is not matching the text. Figure 4 has Type=0x80, Sub-type=TBD6, text says type=TBD6 and subtype=0x00

·         How is the pool ID managed ?

Section 3.1.2

·         Same comment about Figure 5 not matching the text.

Section 3.2:

·         s/a two byte Type field and a six byte/a two bytes Type field and a six bytes/

·         Same comment about normative language "must" vs "MUST"

·         How is the nexthop encoded in the NLRI ?

Section 3.2.1:

·         From a readability point of view, it would be good to tell in the second sentence that the requested attribute is Optional Transitive (we can see it in the description but it is also good to have it in the intro of the attribute).

·         "The presence rules and meanings are as follows.". It would be good to use normative language in the following sentences.

·         I don't see the "error handling" behavior associated with this attribute (discard, treat-as-withdraw...)

Section 3.2.1.1

·         "It may be present" or "It MAY be present" ?

Section 4.1

Having an SFF being part of a single overlay network is IMO a very limited use case, in such a case, I don't think that you need an RT at all. The SFF could be part of a L3VPN which will be used as an underlay.
An SFF being "multitenant" is more valid and defacto requires to maintain separate forwarding states (VRF like...). This is mandatory to maintain the tenant isolation and RTs are very useful here to know the appropriate context to put the routes in.

Section 7.1
While I understand that the node doing the classification can perform a deep packet inspection to get an entropy indicator, any intermediate node cannot set it again as the NSH header will be there.
Here is an example:

PacketsIn ---> Classifier ->SFF1 (SFI1) -> SFF2(SFI2) ->SFF3(SFI3)->PacketsOut to dest

Classifier pushes NSH header onto the packets as well as the underlay tunnel (using an entropy indicator in UDP or ELI/EL). Where the packets comes in at SFF1, IMO, it strips the underlay tunnels including the entropy indicator. Which means that when the packet with NSH header will go to SFF2, it will not have anymore entropy indicator and there is no way to build it again as the NSH header prevents a new deep packet inspection. Am I missing something ? Maintaing a per packet state is IMO not doable , especially if the SFI modifies the packet content.

Section 8
I would have been good to have two SFIs with the same SFT on the same SFF in the example.
I don't like the representation of RD using "192.0.2.1,1" as the "," can be confusing with a regular separator. Why not using :"192.0.2.1:1" notation which is well known ?

Section 8.9.1:
How does an SFF know that an attached SFI is stateful ? I don't think it can know that.
I don't think that the fact that SFF2 is used in both direction is safe from a loadbalancing perspective.
If the hashing algorithm used by SFF2 is sensible to the order of the keys (like source vs dest address, or source vs dest port), it may provide a different SFI as a result of the hashing between the forward and the reverse flow.

Section 8.9.2:
How can the controller instruct the classifier how to place traffic ?
In addition, this could involve multiple classifiers that need to be coordinated.
The ingress forward classifier and the ingress reverse classifier should use SFPs within the same assoc.

Section 9:
Do we have to set limits on receiving nodes in term of number of states received from the controller to mitigate some attack ?
The text talks about security of BGP, what kind of mechanism should be put in place ?
Do we have any interdomain considerations ?
This is a controller based approach, so the security of the controller itself is also important.

References:
Shouldn't RFC7665 be normative ?
I think that the mpls-sfc and mpls-sfc-encaps should also be normative as you are defining a controlplane to use them.

Brgds,

[Orange logo]<http://www.orange.com/>

Stephane Litkowski
Network Architect
Orange/SCE/EQUANT/OINIS/NET
Orange Expert Future Networks
phone: +33 2 23 06 49 83 <https://monsi.sso.francetelecom.fr/index.asp?target=http%3A%2F%2Fclicvoice.sso.francetelecom.fr%2FClicvoiceV2%2FToolBar.do%3Faction%3Ddefault%26rootservice%3DSIGNATURE%26to%3D+33%202%2023%2028%2049%2083%20>  NEW !
mobile: +33 6 71 63 27 50 <https://monsi.sso.francetelecom.fr/index.asp?target=http%3A%2F%2Fclicvoice.sso.francetelecom.fr%2FClicvoiceV2%2FToolBar.do%3Faction%3Ddefault%26rootservice%3DSIGNATURE%26to%3D+33%206%2037%2086%2097%2052%20>  NEW !
[email-masked]<mailto:[email-masked]>

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

---

From: "Andrew G. Malis" <[email-masked]>
Date: Wed, 27 Feb 2019 09:20:59 -0500

Stephane,

Responding to one of your comments - making mpls-sfc-encaps a normative
reference would be a downref, as it's an informational draft. That's OK as
long as everyone is aware of it and it's documented in the IESG writeup,
but that does have to happen. It may just be easier to keep it as an
informative reference. As the document shepherd, it's your call at this
point.

Cheers,
Andy

On Wed, Feb 27, 2019 at 5:22 AM <[email-masked]> wrote:

> The document is globally well written with good examples that help the
> understanding. However it requires some refinements in the normative
> language used: some statement should be normative but are not using upper
> case words.
>
>
>
>
>
> Detailed comments:
>
>
>
> Abstract:
>
> -          Please expand “BGP” on first use
>
> -          Please give a reference associated to “Network Service Header”
>
>
>
> Introduction:
>
> -          When “classifier” is used as an example of SF, do you refer to
> the classifier used to steer the traffic onto an SFC ? If yes, I don’t
> think it could be considered as an SF.
>
> -          I think the “conventional” approach described in the intro is
> really old school and even before SFC, we had some means, like dynamic
> routing protocols and other mechanisms like separate routing tables, to
> steer the traffic through a set of services. I agree that there is also
> some drawback associated with such approaches like the operational
> complexity of provisioning.
>
>
>
> Section 1.2:
>
> ·         As the section is focused on dataplane, it would be good to rename it as “Reminder on NSH dataplane” or something like that. It does not provide a functional overview of the controlplane which is what was expected based on the title.
>
>
>
> ·         “A special Service Function, called a Classifier, is located at each
>
> “
> Again, based on my understanding, the Classifier cannot really be
> considered as an SF , at least this is a component out of the SFC which
> steers the traffic onto an SFC.
>
>
>
> ·         “An unknown or invalid SPI SHALL be treated as an error and the SFF
>
>    MUST be subject to rate limits.”
>
>
>
> I found strange to have such statement in this document which is focused on the controlplane while this statement is a dataplane statement. Isn’t it part of RFC8300 Section 3 : “3.  Update the NSH: SFs MUST decrement the service index by one.  If
>
>        be dropped by the SFF.”
> ·         “The SFIR describes a particular instance of a
>
>       particular Service Function”. Would it be good to talk about “Service Function Instance” rather than instance of a Service Function ? That’s understandable, of course, however I think it’s good to reuse the exact terminology.
>
> ·         “Service Function Type (SFT) that
>
>    is the category of SF that is supported by an SFF”. Don’t you mean SFI rather than SFF ?
>
> ·         “Thus the SFF can be seen as a portal…”. Would “gateway” be more suitable rather than “portal” ?
>
> ·         I don’t like (personal opinion), the “grouping” of SFIs in the Figure 1 as part of an SFT. What strikes me is that it could be confused with the usual representation of an SF composed of multiple SFIs. Again that’s just a personal feeling.
>
>
>
> Section 3:
>
> ·         “they must use BGP Capabilities”: is it a normative MUST ?
>
>
>
> Section 3.1:
>
> ·         “Two SFIs of the same SFT must be associated”. Is it a normative MUST ? Same comment for next sentences in the paragraph (multiple occurences)
>
> ·         “The Service Function Type identifies a service function”. I don’t think we can really say that, it identifies the type of service the SF is providing but not the SF itself.
>
> ·         “Each node hosting an SFI
>
>    advertise an SFIR for each instance of each type of SF.” Is it really “and” ? I mean can we just summarize by using “ Each node hosting an SFI MUST originate an SFIR for each instance of each type of SF” ?
>
> ·         “A BGP Update containing one or more SFIRs will also include”.
> Is it a MUST or SHOULD “also include”?
>
> ·         s/“It can be included”/”It MAY be included”/ ?
>
> ·         Same comment about normative language “must” vs “MUST”
>
> ·         “The presence rules and meanings are as follows.”. It would be good to use normative language in the following sentences.
>
> ·         I don’t see the “error handling” behavior associated with this attribute (discard, treat-as-withdraw…)
>
>
>
> Section 3.2.1.1
>
> ·         “It may be present” or “It MAY be present” ?
>
>
>
> Section 4.1
>
>
>
> Having an SFF being part of a single overlay network is IMO a very limited
> use case, in such a case, I don’t think that you need an RT at all. The SFF
> could be part of a L3VPN which will be used as an underlay.
>
> An SFF being “multitenant” is more valid and defacto requires to maintain
> separate forwarding states (VRF like…). This is mandatory to maintain the
> tenant isolation and RTs are very useful here to know the appropriate
> context to put the routes in.
>
>
>
> Section 7.1
>
> While I understand that the node doing the classification can perform a
> deep packet inspection to get an entropy indicator, any intermediate node
> cannot set it again as the NSH header will be there.
>
> Here is an example:
>
>
>
>
>
> PacketsIn -à Classifier ->SFF1 (SFI1) -> SFF2(SFI2)
> ->SFF3(SFI3)->PacketsOut to dest
>
>
>
> Classifier pushes NSH header onto the packets as well as the underlay
> tunnel (using an entropy indicator in UDP or ELI/EL). Where the packets
> comes in at SFF1, IMO, it strips the underlay tunnels including the entropy
> indicator. Which means that when the packet with NSH header will go to
> SFF2, it will not have anymore entropy indicator and there is no way to
> build it again as the NSH header prevents a new deep packet inspection. Am
> I missing something ? Maintaing a per packet state is IMO not doable ,
> especially if the SFI modifies the packet content.
>
>
>
> Section 8
>
> I don’t like the representation of RD using “192.0.2.1,1” as the “,” can
> be confusing with a regular separator. Why not using :”192.0.2.1:1”
> notation which is well known ?
>
>
>
>
>
> Section 8.9.1:
>
> How does an SFF know that an attached SFI is stateful ? I don’t think it
> can know that.
>
> I don’t think that the fact that SFF2 is used in both direction is safe
> from a loadbalancing perspective.
>
> If the hashing algorithm used by SFF2 is sensible to the order of the keys
> (like source vs dest address, or source vs dest port), it may provide a
> different SFI as a result of the hashing between the forward and the
> reverse flow.
>
>
>
> Section 8.9.2:
>
> Do we have to set limits on receiving nodes in term of number of states
> received from the controller to mitigate some attack ?
>
> Shouldn’t RFC7665 be normative ?
>
> I think that the mpls-sfc and mpls-sfc-encaps should also be normative as
> you are defining a controlplane to use them.
>
>
>
>
>
>
>
> Brgds,
>
>
>
>
>
>
>
>  <http://www.orange.com/>
>
>
>
> *Stephane Litkowski *
> Network Architect
> phone: +33 2 23 *06* 49 83
> <https://monsi.sso.francetelecom.fr/index.asp?target=http%3A%2F%2Fclicvoice.sso.francetelecom.fr%2FClicvoiceV2%2FToolBar.do%3Faction%3Ddefault%26rootservice%3DSIGNATURE%26to%3D+33%202%2023%2028%2049%2083%20>
>  NEW !
> mobile: +33 6 71 63 27 50
> <https://monsi.sso.francetelecom.fr/index.asp?target=http%3A%2F%2Fclicvoice.sso.francetelecom.fr%2FClicvoiceV2%2FToolBar.do%3Faction%3Ddefault%26rootservice%3DSIGNATURE%26to%3D+33%206%2037%2086%2097%2052%20>
>  NEW !
> [email-masked]
>
>
>
> _______________________________________________
> BESS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/bess
>

---

From: <[email-masked]>
Date: Thu, 28 Feb 2019 08:38:30 +0000

Agree

From: Andrew G. Malis [mailto:[email-masked]]
Sent: Wednesday, February 27, 2019 15:21
To: LITKOWSKI Stephane OBS/OINIS
Cc: [email-masked]; [email-masked]
Subject: Re: [bess] Shepherd's review of draft-ietf-bess-nsh-bgp-control-plane-06

Stephane,

Responding to one of your comments - making mpls-sfc-encaps a normative reference would be a downref, as it's an informational draft. That's OK as long as everyone is aware of it and it's documented in the IESG writeup, but that does have to happen. It may just be easier to keep it as an informative reference. As the document shepherd, it's your call at this point.

Cheers,
Andy

On Wed, Feb 27, 2019 at 5:22 AM <[email-masked]<mailto:[email-masked]>> wrote:
Hi,

General comment:

Detailed comments:

Abstract:

Introduction:

-          When “classifier” is used as an example of SF, do you refer to the classifier used to steer the traffic onto an SFC ? If yes, I don’t think it could be considered as an SF.

-          I think the “conventional” approach described in the intro is really old school and even before SFC, we had some means, like dynamic routing protocols and other mechanisms like separate routing tables, to steer the traffic through a set of services. I agree that there is also some drawback associated with such approaches like the operational complexity of provisioning.

Section 1.2:

Section 2.1:

•         As the section is focused on dataplane, it would be good to rename it as “Reminder on NSH dataplane” or something like that. It does not provide a functional overview of the controlplane which is what was expected based on the title.

•         “A special Service Function, called a Classifier, is located at each
   ingress point to a service function overlay network.  It assigns the
   packets of a given packet flow to a specific Service Function Path.
“
Again, based on my understanding, the Classifier cannot really be considered as an SF , at least this is a component out of the SFC which steers the traffic onto an SFC.

•         “An unknown or invalid SPI SHALL be treated as an error and the SFF

       be dropped by the SFF.”

Section 2.2

•         “The SFIR describes a particular instance of a

•         I think the SFT definition is appearing a bit late in this section and in the document as it as been referenced already multiple times before.

•         “Service Function Type (SFT) that

•         “Thus the SFF can be seen as a portal…”. Would “gateway” be more suitable rather than “portal” ?

•         The Figure 1 is not really used in this section as part of the existing text. I would be better to have a companion text that explains the figure.

•         I don’t like (personal opinion), the “grouping” of SFIs in the Figure 1 as part of an SFT. What strikes me is that it could be confused with the usual representation of an SF composed of multiple SFIs. Again that’s just a personal feeling.

Section 3:

•         “they must use BGP Capabilities”: is it a normative MUST ?

Section 3.1:

•         s/a two byte Type field and a six byte/a two bytes Type field and a six bytes/

•         “Two SFIs of the same SFT must be associated”. Is it a normative MUST ? Same comment for next sentences in the paragraph (multiple occurences)

•         “The Service Function Type identifies a service function”. I don’t think we can really say that, it identifies the type of service the SF is providing but not the SF itself.

•         “Each node hosting an SFI

•         “A BGP Update containing one or more SFIRs will also include”. Is it a MUST or SHOULD “also include”?

•         How is the nexthop encoded in the NLRI ?

Section 3.1.1

•         s/“It can be included”/”It MAY be included”/ ?

•         That would be great to give more details about the usage of pools

•         The encoding in Figure 4 is not matching the text. Figure 4 has Type=0x80, Sub-type=TBD6, text says type=TBD6 and subtype=0x00

•         How is the pool ID managed ?

Section 3.1.2

•         Same comment about Figure 5 not matching the text.

Section 3.2:

•         s/a two byte Type field and a six byte/a two bytes Type field and a six bytes/

•         Same comment about normative language “must” vs “MUST”

•         How is the nexthop encoded in the NLRI ?

Section 3.2.1:

•         From a readability point of view, it would be good to tell in the second sentence that the requested attribute is Optional Transitive (we can see it in the description but it is also good to have it in the intro of the attribute).

•         “The presence rules and meanings are as follows.”. It would be good to use normative language in the following sentences.

•         I don’t see the “error handling” behavior associated with this attribute (discard, treat-as-withdraw…)

Section 3.2.1.1

•         “It may be present” or “It MAY be present” ?

Section 4.1

Having an SFF being part of a single overlay network is IMO a very limited use case, in such a case, I don’t think that you need an RT at all. The SFF could be part of a L3VPN which will be used as an underlay.
An SFF being “multitenant” is more valid and defacto requires to maintain separate forwarding states (VRF like…). This is mandatory to maintain the tenant isolation and RTs are very useful here to know the appropriate context to put the routes in.

Section 7.1

Section 8
I would have been good to have two SFIs with the same SFT on the same SFF in the example.
I don’t like the representation of RD using “192.0.2.1,1” as the “,” can be confusing with a regular separator. Why not using :”192.0.2.1:1” notation which is well known ?

Section 8.9.1:
How does an SFF know that an attached SFI is stateful ? I don’t think it can know that.
I don’t think that the fact that SFF2 is used in both direction is safe from a loadbalancing perspective.
If the hashing algorithm used by SFF2 is sensible to the order of the keys (like source vs dest address, or source vs dest port), it may provide a different SFI as a result of the hashing between the forward and the reverse flow.

Section 8.9.2:

Section 9:

References:

Brgds,

Stephane Litkowski
Network Architect

Thank you.
_______________________________________________
BESS mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/bess