Re: [trill] QA review of draft-ietf-trill-ecn-support
Donald Eastlake <d3e3e3@gmail.com> Mon, 19 June 2017 19:56 UTC
Return-Path: <d3e3e3@gmail.com>
X-Original-To: trill@ietfa.amsl.com
Delivered-To: trill@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 542A3129494; Mon, 19 Jun 2017 12:56:36 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.448
X-Spam-Level:
X-Spam-Status: No, score=-2.448 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aW6nrjiYcaf6; Mon, 19 Jun 2017 12:56:32 -0700 (PDT)
Received: from mail-it0-x235.google.com (mail-it0-x235.google.com [IPv6:2607:f8b0:4001:c0b::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B8E8F127B52; Mon, 19 Jun 2017 12:56:32 -0700 (PDT)
Received: by mail-it0-x235.google.com with SMTP id m47so2003214iti.1; Mon, 19 Jun 2017 12:56:32 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=SmiXoWlFzRx7yYrvJ6z03gAbI4cNhIkWt80bJP/jKSo=; b=a6cROIBac7aCesE+QSovVgvos8ic7xWEE4aZ02mH19T9gv1xkLi1BNGp8VNRRrJ/V+ GHiJRnUO38WN8XT6q2tHrnXqX25vjcj9pCA0m5WznwZpqERb5q/VAJVR912/pZu2JNdl uQYDZSQw6mhkg2Z59OCs5SvlDfk2ZUqQVDNC8FzHXP2cMfywXbCupgqNfPNbfFSAFLwv +WLvYF6ATcPzNzskuRffAu10RV8A0mQEf3eCkG7su4FpZnTMTM6gk6cRvvy4wFMtx2vA 19eFNUr/W9FDx1wVPfuixNpNggDVibq/wxo0E3o2B26aE+3nN2u+f8kdUwp5Aps1vk7Y b+Hg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=SmiXoWlFzRx7yYrvJ6z03gAbI4cNhIkWt80bJP/jKSo=; b=rueW9Z4SurvhzIwhhmTuMC2r3EaKPJt/4AZ0w+6O5qKGt0rPPG9WfQ1+sk0okyg3wt /fLvIz69xk6c7OgFA/8+bM00s9p1a+GIHz1XCQuE4+yvaKnanGflMM+G4tEuHWUWY75V FClD3ic9vgGBhupbq769fy9qbyPD97RYkLv8yCIJU4tX9Ia4IbYS3zblXOgHMbEjIuZm aHgbrwvErn7+s0y2ltSV74uAxaNnBCMPWiC2Gn6q9d4W3lTXrwQq/KuFltG6SL0oBNAZ wPeIyty45e5/GDJR9kGudQqUhhSeDJFigQ96n6QRrcIqxOAA2qASo/aJYnTaXXQT+03r g5yw==
X-Gm-Message-State: AKS2vOwKVCWFhEkNE+g6JECOnwzrFJmMwQo6gT2JpFOcUCvmZME59X7K LTnWPppmd57TOEFZclElRKchJk3UPA==
X-Received: by 10.36.112.20 with SMTP id f20mr459874itc.104.1497902192097; Mon, 19 Jun 2017 12:56:32 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.107.131.152 with HTTP; Mon, 19 Jun 2017 12:56:16 -0700 (PDT)
In-Reply-To: <44ca7760-b1c8-cf98-2e8c-2ad47fa6d3c5@pi.nu>
References: <e32f157e-c466-ec37-56aa-ebabdac91464@pi.nu> <44ca7760-b1c8-cf98-2e8c-2ad47fa6d3c5@pi.nu>
From: Donald Eastlake <d3e3e3@gmail.com>
Date: Mon, 19 Jun 2017 15:56:16 -0400
Message-ID: <CAF4+nEGo9bVZxn6TG5UOY=GYr8zZZYwcKqurw7ROObu14eGk7w@mail.gmail.com>
To: Loa Andersson <loa@pi.nu>
Cc: "rtg-dir@ietf.org" <rtg-dir@ietf.org>, "trill-chairs@ietf.org" <trill-chairs@ietf.org>, draft-ietf-trill-ecn-support@ietf.org, "trill@ietf.org" <trill@ietf.org>, "Yemin (Amy" <amy.yemin@huawei.com>
Content-Type: multipart/alternative; boundary="001a113f890211a9c3055255888d"
Archived-At: <https://mailarchive.ietf.org/arch/msg/trill/k4FHrRqXWXyp6UrkhQOoHViuSd0>
Subject: Re: [trill] QA review of draft-ietf-trill-ecn-support
X-BeenThere: trill@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: "Developing a hybrid router/bridge." <trill.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/trill>, <mailto:trill-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/trill/>
List-Post: <mailto:trill@ietf.org>
List-Help: <mailto:trill-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/trill>, <mailto:trill-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 19 Jun 2017 19:56:36 -0000
Hi Loa, Thanks for the update. Donald =============================== Donald E. Eastlake 3rd +1-508-333-2270 (cell) 155 Beaver Street, Milford, MA 01757 USA d3e3e3@gmail.com On Mon, Jun 19, 2017 at 6:00 AM, Loa Andersson <loa@pi.nu> wrote: > Authors, Working Group, > > Sorry for the late reply. > > I've been asked to re-review the document since it is (was) in working > group last call. > > I've reviewed again > > I find that > - my earlier comments has been satisfactorily addressed > - I have no further comments on this document > - I believe we are ready to ask for publication. > > /Loa > > > On 2017-01-21 06:02, Loa Andersson wrote: > >> Authors, >> >> I have been asked to do a Routing Area Directorate QA review of >> draft-ietf-trill-ecn-support. >> >> Caveat - I'm not a congestion control expert, and this will show up in >> my comments. The place where I ask for clarifications might be perfectly >> clear for a reader that is up to speed in the area. >> >> Summary: >> >> I think the document is on the right track, for a reader not an expert >> in the area there are a lot of abbreviations that are not properly >> expanded. I also think that it would be a good idea to more clearly >> make the the case why the document is needed (abstract and/or >> introduction). >> >> After a while I stop trying to distinguish between "Minor issues" and >> "Nits" and placed everything as Minor Issues. I guess I could have >> done everything as Nits :). >> >> Other than the Comment/Minor Issues I find the document pretty solid, >> though I sometimes found it hard to parse sentences. >> If you want I can take a look at that aspect once what is in this >> review has been addressed. >> >> >> Comments: >> >> Last paragraph of the Introduction >> ---------------------------------- >> >> Whichever RBridges do not support ECN, this >> specification ensures congestion notification will propagate safely >> to Destination because the packet will be dropped if explicit >> congestion notification cannot be propagated and drop is itself an >> implicit form of congestion notification. >> >> Is this logic really watertight? What if the packet is dropped because >> of a checksum error? >> >> >> >> Major Issues: >> >> >> Minor Issues: >> >> Abstract >> -------- >> I find the Abstract a bit meager, a little more context would be good. >> >> Maybe lead with some short words about what ECN is good for. >> >> And maybe use this from the Introduction: >> >> This specification provides for any ECN marking in the traffic at the >> ingress to be copied into the TRILL Extension Header Flags Word. It >> also enables congestion marking by a congested RBridge such as RBn or >> RB1 above in the TRILL Header Extension Flags Word [RFC7179]. >> >> ECNencapGuide >> ------------- >> >> This reference has expired, probably not a problem since Bob is a >> co-author of both documents. >> >> TRILL Header >> ------------ >> >> Referred to in section in the Introduction, I think a reference would be >> good. >> >> The ECN Specific Extended Header Flags >> -------------------------------------- >> >> The pictures is less than intuitive, it took me quite some time de-code >> it. >> I did the following: >> Critical (?) flags >> 0 - CRHbH (no expansion found in document) >> 1 - CRItE (no expansion found in document) >> 2 - CRRsv (no expansion found in document) >> >> CHbH flags (Critical Hop by Hop flags - no expansion found in document) >> 3 - unassigned >> 4 - unassigned >> 5 - unassigned >> 6 - unassigned >> 7 - CRCAF (no expansion found in document) >> >> NCHbH flags = Non-Critical Hop-by-Hop flags >> 8 - NCCAF (no expansion found in document) >> 9 - unassigned >> 10 - unassigned >> 11 - unassigned >> ------------------------------------------- >> 12 - ECN = Explicit Congestion Notification >> 13 (two bit flags) >> ------------------------------------------- >> >> CRSV flags (no expansion found in document) >> ------------------------------------------- >> 14 - Ext Hop Cnt (no expansion found in document) >> 15 three bit field >> 16 >> ------------------------------------------ >> >> NCRSV flags (no expansion found in document) >> 17 - unassigned >> 18 - unassigned >> 19 - unassigned >> 20 - unassigned >> ------------------------------------------ >> >> CItE flags = Critical Ingress-to-Egress >> ------------------------------------------ >> 21 - unassigned >> 22 - unassigned >> 23 - unassigned >> 24 - unassigned >> 25 - unassigned >> 26 - CCE = Critical Congestion Experienced >> ------------------------------------------ >> >> NCItE flags = Non Critical Ingress-to-Egress >> -------------------------------------------- >> 27 - Ext Clr (no expansion found in document) >> 28 two bit field >> -------------------------------------------- >> 29 - unassigned >> 30 - unassigned >> 31 - unassigned >> >> Multi-bit flags >> --------------- >> >> In the context I've been active "flags" are one bit, if there are >> multiple bits they are called fields. I understand that in the context >> this is written there are multiple bit flags. >> >> Bit 11 and 12 >> ------------- >> >> Bit 11 and 12 has the following code points: >> >> Binary Name Meaning >> ------ ------- ----------------------------------- >> 00 Not-ECT Not ECN-Capable Transport >> 01 ECT(1) ECN-Capable Transport (1) >> 10 ECT(0) ECN-Capable Transport (0) >> 11 NCCE Non-Critical Congestion Experienced >> >> Table 1. TRILL-ECN Field Codepoints >> >> There is no good explanation what ECT(0) and ECT(1) means, even though >> it is (page 9) said that "ECT(1) as a lesser severity level, termed the >> Threshold-Marked (ThM) codepoint". It could be inferred that ECT(0) is >> a higher severity level, but this should be clearly spelled out. >> >> RFC 3168 does not make the distinction between ECT(0) and ECT(1), but >> says that it will be done in future RFCs; since this is about 3000 RFCs >> ago it might have happened but I couldn't find it. If this has been done >> I think a reference would be good. >> >> Code Point 0b11 >> --------------- >> The text above Table 1 says: >> OLD >> "However codepoint 11 is called Non-Critical Congestion Experienced >> (NCCE)..." >> I think this should be: >> However code point 0b11 is called Non-Critical Congestion Experienced >> (NCCE)..." >> >> The text further says that the code point is call NCCE to distinguish >> it from Congestion Experienced in IP. The question I have is why it is >> necessary to distinguish code point 0b11, but not 0b00, 0b01 and 0b10? >> >> ECN SUpport >> ----------- >> >> The first paragraph has "logically" at three places, what would be the >> difference if these "logically" are dropped? >> >> >> First sentence in sectuion 3.1 >> ------------------------------ >> >> The sentence says: >> "The ingress behavior is as follows:" >> >> I would say >> "The behavior of an ingress RBridge is as follows:" >> or even >> "The behavior of an ingress RBridge MUST be as follows:" >> >> cleared vs set to zero >> ---------------------- >> The last sub-bullet in the first main bullet of section 3.1 says: >> "ensure the CCE flag is cleared to zero (Flags Word bit 26)." I would >> have used "cleared" or "swt to zero". >> >> First line section 3,2 >> ---------------------- >> s/ahow/shown >> Caveat I normally don't English grammar reviews, but sometimes I can't >> stop myself :) >> >> Second line first main bullet in section 3.2 >> -------------------------------------------- >> >> I prefer the format "guidelines in RFC 7567 [RFC7567]" >> >> Third sub-bullet in the third main bullet of section 3.2 >> --------------------------------------------------------- >> >> It says: >> "+ set the TRILL-ECN field to Not-ECT (00);" >> >> Here you use "field" instead of "flag", which is fine, but the document >> should be consistent. Either: >> + set the TRILL-ECN field to Not-ECT (0b00); >> or >> + set the TRILL-ECN flag to Not-ECT (0b00); >> >> Egress ECN Support >> ------------------ >> First sentence: >> "If the egress RBridge does not support ECN, it will ignore bits 12 >> and 13 of any Flags Word that is present, because it does not contain >> any special ECN logic." >> >> in "it will ignore" what does "it" refer to? >> >> SHould it be: >> >> "If the egress RBridge does not support ECN, the RBridge will ignore >> the TRILL-ECN field (bits 12 and 13) if a Flags Word that is >> present, because it has no ECN logic." >> >> TRILL Support for ECN Variants >> ------------------------------ >> The sedond sentence of section four says: >> >> Section 3 specifies interworking between TRILL and the original >> standardized form of ECN in IP [RFC3168]. >> >> RFC 3168 is updated by RFC 4301, RFC 6040, does section 3 relate to >> RFC 3168 or does the updates come into plan. IF the updates are in >> scope I think the sentence should say: >> >> Section 3 specifies interworking between TRILL and the original >> standardized form of ECN in IP RFC 3168 [RFC3168], and the updates >> in RFC4310 [RFC4301] and RFC 6040 [6040]. >> >> >> >> >> >> Nits: >> >> Codepoints >> ---------- >> at several places "codepoint(s)" I think the words IANA and the >> RFC Editor use is "code point(s)" >> >> >> >> /Loa >> > > -- > > > Loa Andersson email: loa@mail01.huawei.com > Senior MPLS Expert loa@pi.nu > Huawei Technologies (consultant) phone: +46 739 81 21 64 >
- [trill] QA review of draft-ietf-trill-ecn-support Loa Andersson
- Re: [trill] QA review of draft-ietf-trill-ecn-sup… Susan Hares
- Re: [trill] QA review of draft-ietf-trill-ecn-sup… Donald Eastlake
- Re: [trill] QA review of draft-ietf-trill-ecn-sup… Loa Andersson
- Re: [trill] QA review of draft-ietf-trill-ecn-sup… Donald Eastlake
- Re: [trill] QA review of draft-ietf-trill-ecn-sup… Loa Andersson
- Re: [trill] QA review of draft-ietf-trill-ecn-sup… Donald Eastlake