Hi Quentin, Thanks much for your detailed review!!! See inline. I’m posting version -08.
Thanks, Acee > On Jul 28, 2023, at 09:03, Quentin Armitage <[email protected]> wrote: > > I have now had the opportunity to read draft 7 of rfc5798bis in detail and > have the > following comments (apologies that the list is rather long). The vast > majority of the > comments relate to the original RFC5798, but it seems a good opportunity to > make these > updates while the RFC is being updated. > > 1. All occurrences of "virtual router" should be "Virtual Router", including > plurals and > possessives (it is a defined term). Deferred. Given the number of occurrences, I’m going to consider this in a separate edit. > > 2. All occurrences of "VRRP router" should be "VRRP Router", including > plurals and > possessives (defined term). Fixed. > > 3. All occurrences of "VRRP Active Router" in Abstract, section 1 > (Introduction) and section > 9 should be "Active Router" (defined term). Fixed. > > 4. All occurrences of "VRRP Backup Router" in Abstract and section 1 > (Introduction) should > be "Active Router" (defined term). I believe you mean “Backup Router”. Fixed. > > 5. Abstract, final paragraph, insert "to" after "updated". Fixed. > > 6. Section 1 (Introduction) second paragraph, insert "to" after "updated". Fixed. > > 7. Section 1.1 paragraph 3 should have "for IPv4" inserted at the end. Fixed. > > 8. Section 1.1 paragraph 8 should have "to" inserted after "augmented". > "ethernet" should be > "Ethernet". Fixed. > > 9. Section 1.3 second paragraph delete "be" between "may" and "not". Also > delete > "associated" before "protocol" ("associated" is specified 3 words earlier), > and delete the > comma between "and" and "such". Fixed. > > 10. Section 1.4 The second sentence does not read easily. Suggest replacing > "The Router > Advertisements ... than 10 seconds" with "The Router Advertisements are > multicast > periodically at a rate such that the hosts will normally take more than 10 > seconds to learn > the default routers on a LAN". Fixed. > > 11. Section 1.4 second paragraph delete "(ND)"; this is specified in the > previous > paragraph. Fixed. > > 12. Section 1.4 last paragraph. Replace "for a failed default router" to > "from a failed > Active Router". Change "around 3 seconds" to "3 to 4 seconds". I suggest > adding at the end > of that sentence "or within 1/64th second when using the lowest > Advertisement_Interval". I think “take over for” is better than “take over from”. > > 13. Section 1.6 change "guarantees" to "guarantee". Fixed. I’m not sure whether the clause was originally intended to refer to all three components but keeping it all plural is simpler. > > 14. Section 1.7 definition of "Active Router".Change "become the" to "be". Ok. > > 15. Section 2.2 replace "preferential router" with "Virtual Router". “Most preferred virtual router”. > > 16. Section 2.4 delete ", i.e., sending either IPv4 or IPv6". IPvX is defined > in section > 1.2. I think it is ok to repeat it with this first usage. > > 17. Section 2.5 second paragraph. Move "may occur" to after "problematic > scenario". Change > both occurrences of "VRRP_Advertisement_Interval" to "Advertisement_Interval" > and add "(see > section 6.1)" after first occurrence. Change "transmitted onto" to > "transmitted on" (this is > consistent with change in previous sentence). Insert "(see section 6.1)" after > "Active_Down_Interval" since it is defined later in the document. Insert > "original" before > "Active Router cause a". Fixed. > > 18. Section 3 second paragraph. Change "virtual router identifier" to > "Virtual Router > Identifier (see section 6.1)". Fixed. > > 19. Section 3 fourth paragraph. Change "VRRP Advertisement messages" to "VRRP > advertisement Leaving it since it is a specific message type. > messages" (it is not a defined term). Change "unless it has" to "unless the > Backup Router > has" ("it" refers to Active Router as the last noun). Fixed. > > 20. Section 3 last paragraph. Insert "Router" after "Backup". "(< 1 second )" > should be "(< > 4 seconds when using the default 1 second Advertisement_Interval (see section > 6.1))" (if the > Active Router fails, takeover time is 3 * Advertisement_Interval + Skew_Time). Ok - reworded. > > 21. Section 4.1 first and second paragraphs after diagram, delete comma after > "i.e.". Nope. “i.e., “ should be followed by a comma. See other RFCs. > > 22. Section 4.1. Move the final paragraph "Note that ..." to before the > diagram, and delete > the words "in both cases". I think it fits better where it is. > > 23. Section 4.1 paragraph starting "In the IPv6 case" change "LAN interface > for the VRRP > protocol and" to "LAN interface, and for the VRRP protocoal there is". Change > "other > routers" to "Virtual Routers". Ok. > > 24. Section 4.1 in two paragraphs starting "In an IPv? VRRP environment" > where ? is 4 or 6, > change "virtual router ID" to "VRID", and change "owned by a router " to > "owned by Router- > 1". Ok > > 25. Section 4.1 in paragraph starting "In an IPv6 VRRP environment", delete > the first > sentence (it duplicates the second sentence). In the (current) second > sentence delte "Link- > Local" (the second and subsequent IPv6 address don't need to be link-local). > Change "with > both VRIDs" to "with the VRID" (there is only one VRID in this example). Ok > > 26. Section 4.1 paragraphs starting "The IPv? example above", if "IPv4" and > "IP" in the > first paragraph are replaced with IPvX, the second of the paragraphs can be > deleted, . If > the paragraphs are not combined, the last sentence of the IPv4 example needs > to be added to > the IPv6 paragraph. Used IPvX. > > 27. Section 5 first paragraph replace "VRRP packet" with "VRRP > advertisement". Replace "and > the state" with ", the Max Advertise Interval and backed up addresses". Ok > > 28. Section 5.2.3 After "field" add "(in conjunction with the protocol > family)". Ok > > 29. Section 5.2.4 third paragraph add sentence at end "Note, each Virtual > Router for a VRID > should be configured with different priorities (unless there are more than > 254 non address- > owner virtual routers) since if two (or more) Backup Routers are configured > with the same > priority, when the Active Router fails, both Backup Routers will transition > to be an Active > Router simultaneously, both sending VRRP advertisements and gratuitous > ARP/unsolicited ND > messages, causing confusion for learning bridges (see section 3)". Nope. The primary address is used as a tie-breaker. > > 30. Section 5.2.4 last paragraph "Active Router to timeout" to > "Active_Down_Interval (see > section 6.1) to expire". Ok > > 31. Section 5.2.7 to end. I do not think that "ADVERTISEMENT" should be > capitalised. Ok. > > 32. Section 5.2.7 second paragraph. Change "VRRP routers" to "Backup Routers". Ok. > > 33. Section 5.2.8 third paragraph, would the reference to Section 5.2 be > better as a > reference to Section 5.2.9? No. It refers to the entire message. > > 34. Section 5.2.9 last paragraph replace "VRRP protocol packet" with "IP > header family" (the > VRRP PDU does not identify whether the packet is IPv4 or IPv6). Fixed. > > 35. Section 6.1 Advertisement_Interval. Add "sent by this Virtual Router" > after > (centiseconds)". Not sure this is needed but added. > > 36. Section 6.1 Add definition "IPvX_Addresses" as "IPv4_Addresses or > IPv6_Addresses" after > definition of "IPv6_Addresses". Fixed. > > 37. Section 6.1 Skew_Time. Replace "priority" with "Priority". Fixed > > 38. Section 6.1 Preempt_Mode Note - replace "The" with "the". No. > > 39. Section 6.1 Virtual_Router_MAC_Address change "ARP" to "ARP/ND" and > change "IPvX > Addresses to "IPvX_Addresses". Fixed. > > 40. Section 6.2 add "(Backup Routers only)" at end of definition of > "Active_Down_Timer". Ok. > > 41. Section 6.2 add "(Active Routers only)" at end of definition of > "Adver_Timer". Ok. > > 42. Section 6.4 second paragraph delete "election". Ok. > > 43. Section 6.4.1 (105) change "address" to "address(es)". Fixed. > > 44. Section 6.4.1 (120) change "request" to "message" (gratuitous ARP > messages are not > requests). Ok. > > 45. Section 6.4.1 (150) change "does not own the virtual address" to "is not > the address > owner". Ok. > > 46. Section 6.4.1 (380) change "request" to "message". Ok. > > 47. Section 6.4.1 (450) insert "Max" before "Advertise". Ok. > > 48. Section 6.4.1 after (450) add "(452) @ Recompute Skew_Time" (it needs to > be recomputed > after every change of Active_Adver_Interval, such as at (750)). Fixed. > > 49. (460) Change "Reset" to "Set" (to match (760)). Ok. > > 50. (465) Add "than the local priority" at the end. Ok > > 50. (640) Delete "+". Yup. > > 51. Section 6.4.1 (745) insert "Max" before "Advertise". Fixed. > > 52. Section 6.4.1 after (775) add "(778) @ Send an advertisement" (the reason > for this is to > update/correct any learning bridges caches and to make the lower priority > Active Router > revert to Backup state. This is a change in procedure but not a protocol > change). I don’t think the virtual router transitioning to Backup Router should send an advertisement here. > > 53. Section 7.1 after "- MUST verify that the VRRP version is 3" add a new > check "- MUST > verify that the VRRP packet Type is 1 - ADVERTISEMENT". Ok. > > 54. Section 7.1 paragraph beginning "A receiver SHOULD" change '"Maximum > Advertisement > Interval"' to 'Max Advertisement Interval' (i.e. delete "imum" and both > double quotes). Ok. > > 55. Section 7.2 Insert "- Set the source MAC address to virtual router MAC > Address" before > "- If the protected address is an IPv4 address, then:" and delete both > occurrences of > "+ Set the source MAC address to virtual router MAC Address" since it > applies to both IPv4 > and IPv6 and avoids duplication. Also add "the" before "Virtual Router". Ok > > 56. Section 7.2 In the two lines starting "+ Set the source IPv? address" add > "the" before > "interface". Ok. > > 57. Section 7.3 third and fifth paragraphs delete "VRRP" before "Virtual > Router Identifier". > Replace "network" with "LAN". Ok. > > 58. Section 8.1.2 replace "gratuitous ARP request" with "gratuitous ARP > message" (both > singular and plural in following paragraph). Ok. > > 59. Section 8.2.2 In "for the Virtual Router IPv6 address" replace "the" with > "a". Ok. > > 60. Section 8.4.2.1.2 and "be" before configured at the end. Fixed. Thanks, Acee > > > I will send separate emails relating to section 5.2.8 (checksums) and 8.3.2 > (recommendations > for priority when multiple Backup Routers), since they are more than simple > editorial > changes and probably require more discussion. > > With regards, > > Quentin Armitage > _______________________________________________ rtgwg mailing list [email protected] https://www.ietf.org/mailman/listinfo/rtgwg
