Hi Joel,
Thank you for the review.
Please see inline.
Cheers,
Med
-----Message d'origine-----
De : Joel Halpern via Datatracker [mailto:nore...@ietf.org]
Envoyé : vendredi 30 juillet 2021 18:38
À : gen-...@ietf.org
Cc : draft-ietf-opsawg-vpn-common....@ietf.org; last-c...@ietf.org;
opsawg@ietf.org
Objet : Genart last call review of draft-ietf-opsawg-vpn-common-09
Reviewer: Joel Halpern
Review result: Ready with Issues
I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed by
the IESG for the IETF Chair. Please treat these comments just like
any other last call comments.
For more information, please see the FAQ at
<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
Document: draft-ietf-opsawg-vpn-common-09
Reviewer: Joel Halpern
Review Date: 2021-07-30
IETF LC End Date: 2021-08-06
IESG Telechat date: Not scheduled for a telechat
Summary: This document is ready for publication as a Proposed
Standard RFC
Major issues: N/A
Minor issues:
I just want to confirm that one form of reference is normal for
YANG
models? There are a few identities whose meaning is defined by
I-Ds that
are under way. The references section of the identity gives the
I-D name.
Which is fine. The references at the bottom of the document
makes those
informative references. That seems a little odd since those
references are
necessary to understand the meaning of those identities. Is
this a normal
practice for YANG models?
[Med] I confirm. We are following this part from RFC8407:
" If a YANG module
contains reference or "description" statements that refer to an
I-D, then the I-D is included as an informative reference. "
I am a little confused as to why the srv6 identity includes in
its
references clause RFC 8663 (MPLS SR). Is this a copy-and-paste
error?
[Med] Please note that we are referring RFC8663, not RFC8660. This is because
we assumed that RFC8663 uses IPv6 as an underlay, but that's may be confusing.
I suggest to go with this change:
OLD:
identity srv6 {
base sr;
description
"Transport is based on SR over IPv6.";
reference
"RFC 8663: MPLS Segment Routing over IP
RFC 8754: IPv6 Segment Routing Header (SRH)";
}
NEW:
identity srv6 {
base sr;
description
"Transport is based on SR over IPv6.";
reference
"RFC 8754: IPv6 Segment Routing Header (SRH)";
}
identity sr-mpls-over-ip {
base sr;
description
"Transport is based on SR over MPLS over IP.";
reference
"RFC 8663: MPLS Segment Routing over IP";
}
Please let me know if this is better. Thanks.
I hope I am misreading the qos-classification-policy definition.
It
appears to have an id, a match rule, and a match action. The
match rule
can be a match-flow or a match-application. So far, so good.
(putting
aside the nit below on customer-application.) But the match
rule is a
choice between an L3 and an L4 rule.
[Med] This is not a choice between these two, but each of them is a choice in
its own. FWIW, here is an excerpt from RFC8340 to distinguish between choices
and cases:
==
<name> is the name of the node
(<name>) means that the node is a choice node
:(<name>) means that the node is a case node
==
Things would be problematic if we defined L3/L4 as "cases" of the same choice,
but we aren't.
As I understand it, there
are many
cases where the actual classification is based on a combination
of l3 and
l4 information. How is that to be represented?
[Med] An example to illustrate how both can be included in shown below (DNS
traffic destined to 2001:db8::/32)
A valid rule for DNS traffic for example is shown below:
{
"id": "a-rule-id",
"ipv6": {
"destination-ipv6-network": "2001:db8::/32"
},
"udp": {
"destination-port-range-or-operator": {
"operator": "eq",
"port": 53
}
}
}
Nits/editorial comments:
The "customer-application" identity seems to be asking for
problems in two
regards.
[Med] FYI, we inherited this from service modules (RFC8299 and RFC8466) where
these common identities are used.
It seems that it is a shorthand way of expressing
some
combination of protocols and ports.
[Med] This can be indeed expressed as such, but at more likely at the network
level. We are covering both as the common module can be used by both service
and network models.
The larger concern I have
is that
there is no reference that explains what classification rules
should be
used for any of the derived identities. Which means that for
an
interoperable implementation, there seems to be some difficulty
in ensuring
that the client and server mean the same thing when using them.
(For
example, just what filer corresponds to "voice"?) As a lesser
issue, the
current IAB work on path signals and the care that should be
taken with
them would seem to suggest that this is a less than desirable
approach to
the problem.
_________________________________________________________________________________________________________________________
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.