search

ietf-wg-ccwg / rfc5033bis

IETF drafts
Other
4 stars 6 forks source link

Review inserted on behalf of M Welzl #96

Closed gorryfair closed 5 months ago

gorryfair commented 5 months ago
  1. General:

In some places, a new proposed congestion control algorithm that this document addresses is repeatedly called "The alternate congestion control algorithm". In some places, it's repeatedly called "A proposed congestion control algorithm". In some places, it's repeatedly called "The proposal”. In some places, it’s called “New CCs” (only in a few places - else the abbreviation CC isn’t introduced).

I think it would be good to unify this. My suggestion: mixing “The proposal” and “A proposed congestion control algorithm” doesn’t really matter, but “The alternative congestion control algorithm” stands out - I would replace these occurrences with “The proposed congestion control algorithm”, and replace “New CCs” in the same way.

  1. Section 1 Intro: OLD: In 2007, TCP was the dominant consumer of this work, and proposals were typically discussed in research groups, for example the Internet Congestion Control Research Group (ICCRG). NEW: In 2007, TCP was the dominant consumer of this work, and proposals were typically discussed in the Internet Congestion Control Research Group (ICCRG). REASON: I don’t remember seeing any other proposal being discussed in any other RG.
    • accepted
  1. Since RFC 5033 was published, many conditions have changed. The set of protocols using these algorithms has spread beyond TCP and SCTP to include DCCP, QUIC, and beyond.

The DCCP RFCs pre-date RFC 5033, and so I find it a bit odd to see it mentioned in this context. On the other hand, the problem that this text is about is largely related to the deployment of cc. mechanisms without first getting an OK from the IETF, and so it fits to talk about QUIC which is mostly written in user space. Similarly, it would fit to talk about the RMCAT CC’s, or LEDBAT - much more, it seems to me, than to talk about DCCP. So, I suggest to instead say the “WebRTC protocol suite” or something like that.

  1. Section 3.1.2:

    Bufferbloat [Bufferbloat] refers to the building of long queues in the network.

    I think the [Bufferbloat] reference should be replaced with a “proper” one with longer-term archival value, instead of pointing at the IETF blog. E.g., this: https://ieeexplore.ieee.org/document/5755608 https://ieeexplore.ieee.org/document/5755608 which is also freely available: https://queue.acm.org/detail.cfm?id=2071893 https://queue.acm.org/detail.cfm?id=2071893

  2. Section 3.2.1:

    Evaluate the impact on TCP [RFC9293], SCTP [RFC9260], DCCP [RFC4340], and QUIC [RFC9000].

    I would recommend to remove this. I find it odd: this is about the interaction between congestion control mechanisms, not transport protocols - why would there be a significant difference between running a mechanism against SCTP or TCP, for example? Indeed this is clarified in the next paragraph, which says: "A proposed congestion control algorithm SHOULD be evaluated when competing with standard IETF congestion control [RFC5681], [RFC9260], [RFC4340], [RFC9002], [RFC9438].” - so why then list these protocols here? (also: they seem strange to include as references in the sentence I mention here - instead, I would only cite RFC 5681 and RFC 9438). It seems to me that listing the actual protocols expects a proponent of a new mechanism to carry out a huge amount of actually rather useless work.

Besides, DCCP is an odd one in this list, because evaluating the impact “on it” would mean to also evaluate against TFRC and TFRC-SP. Seriously, at this time and day? Is anyone using these controls, still, in any context? (technically, RMCAT controls and LEDBAT would make more sense IMO, but I think it’s ok to skip them because they’re all Experimental).

6. I would swap section 4.1 on tunnel behavior with section 4.2 on paths with Tail-drop Queues, because the latter is the more common case. No big deal, but it’s just a strange sequence IMO.

  1. Section 5.7.1:

    New CCs MUST evaluate the impact of changes in the path, and be robust to changes in path characteristics on the interval of common Internet re-routing intervals.

    First, I don’t think that the abbreviation “CC” has been used before. it is also later used in section 5.8 - I suggest to fix this, to be uniform.

  2. Issue:

More importantly though, the beginning of Section 5 says, about the whole section, that “The community MAY allow a proposal to progress even if the criteria indicate an unsatisfactory result for these scenarios.” This seems to collide with the “MUST” above.

  1. I have the same concern about the two MUSTs in section 5.8,
  2. and there, also, we have the use of “CC”.
gorryfair commented 5 months ago

A PR attempts to resolve this,

    • Harmonised to "proposed congestion control algorithm"
    • New text accepted
    • Working towards this in the PR
    • Done
    • Updated, I agree protocols =/= CC algorithms.
    • Swapped
    • CC changed to be consistent
    • It ought not to collide, will revise text.
    • Updated the use of “CC”.

Still to-do:

    • review the two MUSTs in section 5.8, #94
gorryfair commented 5 months ago

The PR has updated the words CC. The two MUSTs originate from current practice around the use of multipath in IETF specs, and seem to reflect what I thought was the case.