Re: [OPSAWG] Genart last call review of draft-ietf-opsawg-vpn-common-09

2021-09-03 Thread Joel M. Halpern 

Thank you for the clarifications Med.  That all seems good.

Yours,
Joel

On 9/3/2021 8:38 AM, mohamed.boucad...@orange.com wrote:

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

.

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:

==
 is the name of the node
  () means that the node is a choice node
 :() 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

Re: [OPSAWG] Genart last call review of draft-ietf-opsawg-vpn-common-09

2021-09-03 Thread mohamed.boucadair 
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
> 
> .
> 
> 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:

==
is the name of the node
 () means that the node is a choice node
:() 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
>

[OPSAWG] Genart last call review of draft-ietf-opsawg-vpn-common-09

2021-07-30 Thread Joel Halpern via Datatracker 
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

.

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

Nits/editorial comments:
The "customer-application" identity seems to be asking for problems in two
regards.It seems that it is a shorthand way of expressing some
combination of protocols and ports.   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.



___
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg