Susan,
Thank you for your review. See [BB]
On 28/10/2023 23:17, Susan Hares via Datatracker wrote:
Reviewer: Susan Hares
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://wiki.ietf.org/en/group/gen/GenArtFAQ>.
Document: draft-ietf-tsvwg-ecn-encap-guidelines-??
Reviewer: Susan Hares
Review Date: 2023-10-28
IETF LC End Date: 2023-11-02
IESG Telechat date: Not scheduled for a telechat
Summary: The document summarizes decades of work on congestion work in IEEE
802.1 and IETF. It provides a set of good guidelines/recommendations for
designers of Layer-2 (L2) or L2/L3 shim layer.
The authors (John Kaippallimalil and Bob Briscoe) and the original author/
now-contributor (Pat Thaler) should be commended for their work.
The text is generally readable with relatively few English and editorial
issues. However, the authors would be wise to fix the editorial issues prior
to sending it to the RFC editor since phrasing needs to be precise.
Major issues: none.
Minor issues:
1. Have the authors considered the SR-routing pathways with tunnels in this
draft?
If so, the authors might add a side note in the document.
[BB] Not specifically. The idea is to provide guidelines for any
protocol designers who aim to add ECN. Might there be something specific
in SR that would make this generic guidance inapplicable? Other than not
having protocol space, which is obviously not something that guidelines
can solve (similarly for all IEEE802 protocols).
2. Has this document been circulated by the IETF liaisons to IEEE 802.1, MEF,
and 3GPP? 3.
[BB]
* IEEE802.1: no current relevant work (Pat Thaler was a deliberate
choice of co-author at the time, given she was the IEEE802 liaison,
and worked on 802.1Q congestion signalling - separate control plane
messages)
o Here's the liaison request:
https://datatracker.ietf.org/liaison/1364/ (I can't find the
response)
* 3GPP: responded with 6 relevant WGs and 20 relevant TRs, which were
subsequently analysed and a formal response returned.
o Formal liaison response: https://datatracker.ietf.org/liaison/1499/
o Or, for summary, see slides 3-8 from the time:
https://www.ietf.org/proceedings/95/slides/slides-95-tsvwg-11.pdf
* MEF: No liaison. The rationale was to focus on SDOs that might
define ECN-like extensions to their lower layer protocols. I think
the MEF is unlikely to define protocols at that level; it is more
about using protocols defined in other bodies like IEEE802. IYO, was
this a bad judgement call?
Are you really sure your security and manageability sections are
complete?
[BB]
* Security:
o yes definitely complete
* Manageability:
o We didn't write a separate manageability section, but I believe
we covered the various aspects well within the text, but I'll
let you decide by quickly listing them:
o Config:
+ ECN has been designed to have zero to one alternatives
# decap has no choices
# encap is fairly minimal wrt config, the main aspect
being config of encaps where it doesn't know whether the
egress supports ECN decap because it doesn't negotiate
with it (which is heavily covered in the draft).
+ in numerous places, the draft distinguishes approaches that
will work in managed environments from those that are more
appropriate in non-managed plug-and-play scenarios.
o Monitoring:
+ The guidelines on encap justify themselves based on being
able to monitor congestion over the whole path from the
origin load source, but also allow the congestion introduced
over the scope of the tunnel, link or subnet to be measured.
o Anomaly detection:
+ propagating ECN over encaps doesn't change the usefulness or
otherwise of ECN in detecting anomalies, so there's no
mention of this.
o Deployment & Coexistence:
+ A large part of the draft is about addressing the problem of
incrementally deploying ECN support, in particular where the
egress of a subnet or tunnel does not support ECN decap.
o Scaling:
+ The good scaling properties of ECN itself are not changed by
encaps, other than whether the propagation process can be
implemented as line rate scales. The design of the ECN
protocol determines that, so there's nothing more this draft
can say about that than, you will have to work out how to
implement efficiently (which goes without saying).
RFC7713 predates large deployment of SR routing.
[BB] I was rather surprised to see these two mentioned in the same
sentence under security considerations, as if they might offer similar
functions.
However, all I knew about segment routing was what Rob Shakir had
described to me many years ago, so I've been reading up. I still don't
see anything in SR that could help with assurance of e2e integrity of
congestion signals. The problem addressed by RFC7713 is how to ensure
that everything around the whole end-to-end congestion control loop is
reporting congestion to the next device honestly. Just as each network
node in isolation can lie about how congested it is, so can a segment,
or an SR domain. So I can't see how SR could make any contribution to a
solution to this problem.
But if I'm missing some key insight here, pls enlighten me.
BTW, altho RFC7713 is cited here, it's only to say that an integrity
solution exists if its needed. It's not been deployed widely, not least
because the problem hasn't arisen. Hence the draft describes these
bullets as "experimental or proposed".
Nits/editorial comments:
Formatting in the pdf seems to be problematic. I am not commenting on this
point since html form does not have hte same problem.
[BB] I've just looked through the PDF - what problem should I look for?
I can't see anything wrong.
Nits in Editing:
Section 2
Old/Not-ECN-PDU:
A PDU at the IP layer or below that is part of a congestion control
feedback-loop within which at least one node necessary to propagate any
explicit congestion notification signals back to the Load Regulator is not
capable of doing that propagation./
New:/Not-ECN-PDU: A PDU at the IP layer or
below that is part of a congestion control feedback-loop within which at least
one node necessary to propagate any explicit congestion notification signals
back to the Load Regulator this PDU is not capable of doing that propagation./
[BB] This isn't what it meant, which implies that the sentence might be
on the edge of parseability.
Whether it's an ECN-PDU or a Not-ECN-PDU is defined as a property of the
control loop that the PDU is bound to (by its addressing or labelling),
not necessariiy a direct property of the PDU itself. This was meant to
be a way of getting round the problem of a packet adopting multiple L2
headers of multiple different L2 protocols as it traverses the path.
Then each PDU has to have some way of indicating whether it is an
ECN-PDU or not. In some protocols that will be self-described. But in
others, it might be described indirectly in the control plane. I think
I'm going to need to explicitly say all this in the terminology explanation.
Normally, I'd give proposed new text, but I'm just heading for my
flight, so I'll press sent now.
[BTW, I've also noticed there are places where it says a congested
buffer marks or drops packets without making it clear that it only marks
or drops a proportion, not all packets. Apparently this caused problems
with the whole attempt to use ECN for Voice over LTE (VoLTE) in 3GPP,
where they defined ECN as either 100% on or 100% off, because it didn't
say otherwise anywhere in RFC3168 (!). So I'd better fix this.]
Section 3
Text:/
The router forwards the marked L3 header into subnet 2, and when it adds a new
L2 header it copies the L3 marking into the L2 header as well, as shown by the
'C's in both layers (assuming the technology of subnet 2 also supports explicit
congestion marking)./
Question - did you mean subnet b instead of subnet 2 (per figure 1)
[BB] Yes. Also caught by another reviewer - strange no-one in the WG
noticed this.
Section 4.3
Text:/
This ensures that bulk congestion monitoring of outer headers (e.g. by a
network management node monitoring ECN in passing frames) will measure
congestion accumulated along the whole upstream path — since the Load Regulator
not just since the ingress of the subnet. /
The portion of this sentence that has me confused is "since the Load Regulator
not just since the ingress of the subnet.". I'm not really sure what you are
trying to say - so I have no suggested new text.
[BB]
will measure
congestion accumulated along the whole upstream path —*starting from* the Load
Regulator
not just*starting from* the ingress of the subnet.
Would that make it comprehensible?
Bob
--
________________________________________________________________
Bob Briscoehttp://bobbriscoe.net/
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art
