Re: [sfc] Benjamin Kaduk's Discuss on draft-ietf-sfc-hierarchical-09: (with DISCUSS and COMMENT)

[<[e-mail]>]

Hi Benjamin, 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Benjamin Kaduk [mailto:[e-mail]]
> Envoyé : vendredi 22 juin 2018 20:21
> À : BOUCADAIR Mohamed IMT/OLN
> Cc : The IESG; [e-mail]; Behcet Sarikaya; sfc-
> [e-mail]; [e-mail]
> Objet : Re: Benjamin Kaduk's Discuss on draft-ietf-sfc-hierarchical-09: (with
> DISCUSS and COMMENT)
> 
> Hi Med,
> 
> I see that you already uploaded a -10 that moves the intended status to
> Experimental, with a nice clear introduction that lays out the scope and
> goals of the proposed experiment(s).  That's a big improvement, so I will
> clear my DISCUSS.

[Med] Thank you. 

  More inline...
> 
> On Thu, Jun 21, 2018 at 06:42:11AM +0000, [e-mail] wrote:
> > Hi Benjamin,
> >
> > Thank you for the review.
> >
> > Please see inline.
> >
> > Cheers,
> > Med
> >
> > > -----Message d'origine-----
> > > De : Benjamin Kaduk [mailto:[e-mail]]
> > > Envoyé : jeudi 21 juin 2018 03:45
> > > À : The IESG
> > > Cc : [e-mail]; Behcet Sarikaya; sfc-
> > > [e-mail]; [e-mail]; [e-mail]
> > > Objet : Benjamin Kaduk's Discuss on draft-ietf-sfc-hierarchical-09: (with
> > > DISCUSS and COMMENT)
> > >
> > > Benjamin Kaduk has entered the following ballot position for
> > > draft-ietf-sfc-hierarchical-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/iesg/statement/discuss-criteria.html
> > > for more information about IESG DISCUSS and COMMENT positions.
> > >
> > >
> > > The document, along with other ballot positions, can be found here:
> > > https://datatracker.ietf.org/doc/draft-ietf-sfc-hierarchical/
> > >
> > >
> > >
> > > ----------------------------------------------------------------------
> > > DISCUSS:
> > > ----------------------------------------------------------------------
> > >
> > > This does seem like an interesting and potentially useful idea -- thanks
> > > for doing the work!
> >
> > [Med] Thank you.
> >
> >  However, I am not sure that the document as-is is
> > > suitable for publication.
> > >
> > > In Section 4.1.5 we hear that preserving metadata and applying metadata
> to
> > > injected packets is not special and is "usual" functionality, but
> > > section 4.1.2 calls out that the 4.1.2 approach requires the SFs in the
> > > path to be capable of forwarding metadata and attaching metadata to
> > > injected packets as if it is a nontrivial requirement.  This apparent
> > > internal inconsistency needs to be resolved before publication.
> >
> > [Med] I guess you are referring to:
> >
> > (4.1.2)
> >
> > "This approach requires the SFs in the path to be capable of
> >    forwarding the metadata and appropriately attaching metadata to any
> >    packets injected for a flow."
> >
> > And
> >
> > (4.1.5)
> >
> > "No special functionality is required to be supported by an SFC-
> >       aware SF, other than the usual ability to preserve metadata and to
> >       apply metadata to injected packets."
> 
> Yes, that's the spot(s).
> 
> >
> > "usual" is used in the sense that manipulating metadata (update context
> header and service policy selection) is part of the roles of an SFC-aware SF.
> Please refer to " Figure 8: NSH Action and Role Mapping" in RFC8300.
> >
> > I suggest to update 4.1.5 as follows:
> >
> > "No new SFC-related functionality is required to be supported by an SFC-
> >       aware SF, other than the ability to preserve metadata and to
> >       apply metadata to injected packets."
> 
> "No new [...], other than <Y>" still can be read to say that the
> functionality Y is in fact new.  I think it would be more clear to say
> something like "only the existing ability to preserve and apply metadata is
> needed" or "The SFC-related functionality required by this approach in an
> SFC-aware SF is to be able to preserve and apply metadata, which is a
> requirement that was already present in RFC 8300".  But your
> explanation/pointer to 8300 suffices to clear my discuss point, so please
> treat this as a non-blocking comment.
> 

[Med] The second proposal works for me. 

> > >
> > > Why does Section 4.1 offer five different choices with no guidance on how
> > > to make a cost/benefit analysis amongst them?
> >
> > [Med] The rationale of the document is to describe options and leave it to
> the taste of those who (will) deploy to decide which ones among those is more
> appropriate for their deployment context. Key advantages and issues are
> called out.
> >
> > We cannot speculate about cost analysis. Really.
> >
> >   Is the full spread of five
> > > choices really necessary?  Complexity breeds implementation bugs which
> > > breed security issues, so I feel that this breadth of choice needs to be
> > > justified.
> >
> > [Med] I would agree with you if we are recommending all (or most) of them.
> >
> >   This also ties into some confusion I have as to the goal of
> > > this document (which currently targets Informational status),
> >
> > [Med] The main contributions of the document are as follows:
> > - Define an architecture for hSFC (hierarchy level, introduce IBN, define
> IBN role). This proposed hsFC architecture is generic enough.
> > - Identify and describe options to glue levels
> >
> > The document is informational. As such, it does not make use of normative
> language or pick a favorite option.
> >
> > akin to what
> > > is stated in Alvaro's ballot position: is it just providing information
> on
> > > how to assemble existing pieces in a novel way, or presenting a new
> > > protocol specification that is not yet fit for Proposed Standard status,
> or
> > > is it listing out potential solutions to a problem so that they can be
> > > implemented and experience gained as to which are useful in practice and
> > > which are not?
> >
> > [Med] As mentioned above, the document does not declare a favorite option
> to be deployed (I have one though), but documents candidates that can be
> considered when deploying hSFC. Some of the options are implementation-
> specific (4.1.1), some of them are deployment-specific (4.1.3), some require
> more standardization effort.
> 
> The desire to get more deployment experience does seem to fit pretty well
> with the Experimental status.

[Med] Yes. 

> 
> >   Accordingly, I would be interested to hear about what
> > > deployment experience exists already and whether this abstraction proves
> to
> > > be as useful as the use cases seem to promise; if there is little
> > > implementation experience, perhaps Experimental status is more
> appropriate.
> > >
> > > I further am uncertain as to whether the approach described in Section
> > > 4.1.3 even merits consideration at all; the technique it describes seems
> to
> > > have a very leaky abstraction barrier (e.g., w.r.t TTL modifications).
> >
> > [Med] Glad to see that the description of that section is clear enough
> about the complications that are inherent with this option. Our target reader
> is likely to have the same reaction as you.
> >
> > The question is whether it is harmful to have the option described.
> 
> For an experiment, it seems okay to leave it in.

[Med] OK. 

> 
> > > This seems especially poignant given the already large size of candidate
> > > approaches.
> >
> > [Med] 5 is not a "large size" of candidates, IMO. If this is an issue, we
> can consider moving some of the options into an appendix.
> 
> I don't think 5 is a large size for an experiment, only for a final
> protocol. 

[Med] In full agreement. 

 (Sometimes we still end up with that many, but I remember at
> least one case where I complained about it in my ballot position.)
> 
> > >
> > > The approach described in Section 4.1.5 also seems to be incompletely
> > > specified, in that the hSFC Flow ID semantics are not covered at all.  On
> > > my initial reading I assumed that this field's encoding and semantics
> were
> > > intended to be left as entirely local matters to the lower domain,
> avoiding
> > > a need to specify them in this document.
> >
> > [Med] The semantic is local to a domain.
> 
> Agreed, it will not escape a single domain.
> 
> >   However, I'm not sure that it's
> > > actually true, since we generally want multiple vendors to be able to
> > > interoperate,
> >
> > [Med] Intermediate SFC elements do not need to understand the semantic of
> flowID. They will handle the flowID as an opaque value.
> 
> Agreed.  I think it might help to call out (as is done in Section 4.1.1)
> that if the egress IBN differs from the ingress IBN, there is a need for
> state synchronization between those nodes.

[Med] This is the intent of "state replication mentioned in Section 4.1.1". 

I changed the text to "state synchronization mentioned in Section 4.1.1".

> 
> >  and this scheme does not appear to require that the node
> > > applying the hSFC Flow ID (and saving state) is the same node that
> removes
> > > it (and restores state).  That is, these two nodes could potentially be
> > > implemented by different vendors.
> > >
> >
> > [Med] State sync/replication is indicated in the text:
> >
> >    Disadvantages include those of other stateful approaches, including
> >    state timeout and replication mentioned in Section 4.1.1.
> 
> I had read the Section 4.1.1 text as only applying to that particular
> approach, implying that state synchronization would not necessarily be
> needed for the other approaches.
> 
> > > Even once the above issues are resolved, I will only be able to move to
> an
> > > Abstain position, since this document proposes additional usage of
> > > (meta)data in the NSH headers that is not subject to mandatory integrity
> > > protection, as was discussed at length for RFC 8300 and is mandated to be
> > > available by RFC 7665.
> > >
> >
> > [Med] The document does not specify any metadata. It does only define an
> architecture for hSFC and explores deployment options.
> >
> >
> > >
> > > ----------------------------------------------------------------------
> > > COMMENT:
> > > ----------------------------------------------------------------------
> > >
> > > Section 4
> > >
> > >    To achieve the benefits of hierarchy, the IBN should be applying more
> > >    granular traffic classification rules at the lower level than the
> > >    traffic passed to it.  This means that the number of SFPs within the
> > >    lower level is greater than the number of SFPs arriving to the IBN.
> > >
> > > "more granular" and "less granular" are unfortunately ambiguous in
> > > practical usage; I suggest "fine-grained" as an alternative for this
> usage.
> >
> > [Med] Fixed. Thank you.
> >
> > >
> > > Section 4.1.5
> > >
> > > Figure 4's caption should indicate where the base NSH fixed-length
> context
> > > header is originally defined.
> >
> > [Med] I added "[RFC8300], Section 2.4"
> >
> > >
> > > Section 4.4 presents another operational choice that contributes to
> > > exponential complexity growth, and further highlights unique properties
> of
> > > the Section 4.1.3 approach that may render it unsuitable for inclusion.
> > >
> >
> > [Med] What about moving Section 4.1.3 into an appendix?
> 
> I'd say leave it where it is, if we are going with Experimental status.
> 
> > > Section 7.2
> > >
> > >    Other fundamental functions required as IBN (e.g., maintaining
> > >    metadata of upper level or decrementing Service Index) are same as
> > >    normal usage.
> > >
> > > nit: "the same as in normal usage"
> >
> > [Med] Fixed.
> >
> > >
> > > Also, I think the two occurrences of "to permit specific metadata types"
> > > should be "to *only* permit specific metadata types".
> > >
> >
> > [Med] Agree.
> >
> > >
> > > Section 10.1
> > >
> > >    Security considerations related to the control plane should be
> > >    discussed in the corresponding control specification documents (e.g.,
> > >    [I-D.ietf-bess-nsh-bgp-control-plane],
> > >    [I-D.wu-pce-traffic-steering-sfc], or [I-D.maglione-sfc-nsh-radius]).
> > >
> > > I'm going to call this a nit, but "should be discussed" sounds as if this
> > > document is trying to direct the behavior of the other listed documents;
> > > maybe "are discussed" is better.
> >
> > [Med] Works for me.
> >
> > >
> > > Section A.1 could perhaps note the potential drawback that the
> > > classification functionality is now distributed across the domains
> instead
> > > of being totally centralized at the initial entry, which requires greater
> > > coordination between the different classifers.
> >
> > [Med] The text in Section 4 is clear that the hSFC requires IBN to behave
> as a classifier:
> >
> >    To achieve the benefits of hierarchy, the IBN should be applying more
> >    granular traffic classification rules at the lower level than the
> >    traffic passed to it.
> >
> > We can add text to A.1 but IMHO this is redundant.
> 
> I'm happy to defer to you here.

[Med] OK, thanks. I'm planning to leave the text as it is. 

> 
> -Benjamin