Hello,
I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft.
Document: draft-ietf-nvo3-geneve-14
Reviewer: Tal Mizrahi
Review Date: 28-Nov-2019
Intended Status: Standards Track
Summary:
========
I have some minor concerns about this document that I think should be resolved before publication.
Comments:
=========
The draft is clear and well-written.
I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft.
Document: draft-ietf-nvo3-geneve-14
Reviewer: Tal Mizrahi
Review Date: 28-Nov-2019
Intended Status: Standards Track
Summary:
========
I have some minor concerns about this document that I think should be resolved before publication.
Comments:
=========
The draft is clear and well-written.
Specific comments are provided below.
Section 2.2:
"The presence of a Geneve variable length header SHOULD NOT prevent the tunnel endpoints and transit devices from using such offload capabilities." - this is purely an implementation consideration, and uppercase requirement language is not appropriate.
Section 3.4:
"Transit devices MUST maintain consistent forwarding behavior
irrespective of the value of 'Opt Len', including ECMP link
selection. These devices SHOULD be able to forward packets
containing options without resorting to a slow path."
The first sentence makes sense, but the second sentence is implementation-specific. Given the first sentence, I would recommend to remove the second sentence.
Section 3.5:
OLD:
for standardized and experimental options
NEW:
for allocated and for experimental options
Explanation:
the word "standardized" is misleading, because the document is requesting an "IETF Review" allocation policy.
Section 3.5.1:
I could not find a specification of what a receiving endpoint should do if the total length of the Geneve header + options exceeds its processing depth. Drop? Notify the peer tunnel endpoint?
Section 4.2:
"Geneve MUST be used with congestion controlled traffic or within a network that is traffic managed to avoid congestion (TMCE)" - the MUST does not seem appropriate. It is up to the operator whether congestion control is required or not.
Section 4.6:
This section should be rephrased. This section currently defines how hardware offloading in NICs should be implemented, including MUST requirements. A network protocol should not define requirements for specific implementation approaches. Instead, the section should describe what is expected from ANY intermediate node in the network (switch, router, or hardware component along the path) to do or not do (for example not to change the order of Geneve options).
Section 5:
- I suggest to remove this section, or to significantly revise it. As opposed to its title, it does not discuss interoperability, but migration from other tunnel protocols to Geneve.
- The following sentences are inaccurate: "Geneve does not introduce any interoperability issues as it appears to most devices as UDP packets.", and "Geneve is a superset of the functionality of the most common protocols used for network virtualization (VXLAN,NVGRE)"
- It appears that removing this section would not remove significant information.
Section 7:
OLD:
are to be reserved for standardized options for allocation by IETF Review
NEW:
are to be assigned by the IETF Review policy
Explanation:
This sentence is confusing, since "Standards Action" and "IETF Review" are two different IANA policies.
Section 1:
Currently section 1.1 starts a whole page after section 1 starts. I suggest to separate the Requirement Language and the Terminology from the Introduction (two different sections).
References:
IEEE.802.1Q_2014 is not the latest version of 802.1Q.
Section 2.2:
"The presence of a Geneve variable length header SHOULD NOT prevent the tunnel endpoints and transit devices from using such offload capabilities." - this is purely an implementation consideration, and uppercase requirement language is not appropriate.
Section 3.4:
"Transit devices MUST maintain consistent forwarding behavior
irrespective of the value of 'Opt Len', including ECMP link
selection. These devices SHOULD be able to forward packets
containing options without resorting to a slow path."
The first sentence makes sense, but the second sentence is implementation-specific. Given the first sentence, I would recommend to remove the second sentence.
Section 3.5:
OLD:
for standardized and experimental options
NEW:
for allocated and for experimental options
Explanation:
the word "standardized" is misleading, because the document is requesting an "IETF Review" allocation policy.
Section 3.5.1:
I could not find a specification of what a receiving endpoint should do if the total length of the Geneve header + options exceeds its processing depth. Drop? Notify the peer tunnel endpoint?
Section 4.2:
"Geneve MUST be used with congestion controlled traffic or within a network that is traffic managed to avoid congestion (TMCE)" - the MUST does not seem appropriate. It is up to the operator whether congestion control is required or not.
Section 4.6:
This section should be rephrased. This section currently defines how hardware offloading in NICs should be implemented, including MUST requirements. A network protocol should not define requirements for specific implementation approaches. Instead, the section should describe what is expected from ANY intermediate node in the network (switch, router, or hardware component along the path) to do or not do (for example not to change the order of Geneve options).
Section 5:
- I suggest to remove this section, or to significantly revise it. As opposed to its title, it does not discuss interoperability, but migration from other tunnel protocols to Geneve.
- The following sentences are inaccurate: "Geneve does not introduce any interoperability issues as it appears to most devices as UDP packets.", and "Geneve is a superset of the functionality of the most common protocols used for network virtualization (VXLAN,NVGRE)"
- It appears that removing this section would not remove significant information.
Section 7:
OLD:
are to be reserved for standardized options for allocation by IETF Review
NEW:
are to be assigned by the IETF Review policy
Explanation:
This sentence is confusing, since "Standards Action" and "IETF Review" are two different IANA policies.
Section 1:
Currently section 1.1 starts a whole page after section 1 starts. I suggest to separate the Requirement Language and the Terminology from the Introduction (two different sections).
References:
IEEE.802.1Q_2014 is not the latest version of 802.1Q.
Cheers,
Tal Mizrahi.
