Re-, Thanks for the follow-up, Per. The new version is now public: https://datatracker.ietf.org/doc/draft-ietf-netmod-acl-extensions/13/. It also includes some pending editorial changes I had in my list. Please also inline for more context. Cheers, Med > -----Message d'origine----- > De : Per Andersson <per.ietf@xxxxxxxx> > Envoyé : jeudi 19 décembre 2024 12:43 > À : BOUCADAIR Mohamed INNOV/NET <mohamed.boucadair@xxxxxxxxxx> > Cc : yang-doctors@xxxxxxxx; draft-ietf-netmod-acl- > extensions.all@xxxxxxxx; last-call@xxxxxxxx; netmod@xxxxxxxx > Objet : Re: Yangdoctors last call review of draft-ietf-netmod- > acl-extensions-12 > > > Hi Med, > > On Thu, Dec 19, 2024 at 7:50 AM <mohamed.boucadair@xxxxxxxxxx> > wrote: > > > > Hi Per, > > > > Thank you for the review. > > > > The changes to address your comments can be tracked at: > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2 > Fauthor- > tools.ietf.org%2Fapi%2Fiddiff%3Furl_1%3Dhttps%3A%2F%2Fnetmod- > wg.github.io%2Fenhanced-acl-netmod%2Fdraft-ietf-netmod-acl- > extensions.txt%26url_2%3Dhttps%3A%2F%2Fnetmod- > wg.github.io%2Fenhanced-acl-netmod%2FPer-review%2Fdraft-ietf- > netmod-acl- > extensions.txt&data=05%7C02%7Cmohamed.boucadair%40orange.com%7C88 > e1e86c1b604681b86008dd202245eb%7C90c7a20af34b40bfbc48b9253b6f5d20 > %7C0%7C0%7C638702053746427579%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1 > hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIs > IldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=6FQpf2P5XfXY0JzGuybSEIb2dhGei > JTBR5udCHIycdY%3D&reserved=0. These will be published in -13. > > Thanks! > > > > Please see inline. > > See my responses inline below. > > > > Cheers, > > Med > > > > > -----Message d'origine----- > > > De : Per Andersson via Datatracker <noreply@xxxxxxxx> Envoyé > : jeudi > > > 19 décembre 2024 04:13 À : yang-doctors@xxxxxxxx Cc : > > > draft-ietf-netmod-acl-extensions.all@xxxxxxxx; last- > call@xxxxxxxx; > > > netmod@xxxxxxxx Objet : Yangdoctors last call review of > > > draft-ietf-netmod-acl- > > > extensions-12 > > > > > > > > > Reviewer: Per Andersson > > > Review result: Ready with Issues > > > > > > Dear WG, > > > > > > This is my Last Call review of draft-ietf-netmod-acl- > extensions- 12. > > > My conclusion is that the four modules are Ready with issues. > > > > > > In general the modules are in good shape. The following issue > from > > > the early yangdoctor review is still not addressed and should > be > > > fixed. > > > > > > * RFCs cited in the YANG modules should be listed in the > > > introductory > > > section as references. Section 4 needs to add references to > RFC > > > 9293, > > > 4443, 3032, 792, and IEEE 802.1Q, IEEE 802.1ah > > > > > > > [Med] Hmm. I confirm that all the following are already cited > in the document: > > > > * 9293, 4443, 3032, 0792, IEEE802.1Qcp, and IEEE-802-1ah > > They are indeed listed in the references, but the comment asks to > list them in the first paragraph in Section 4. Currently this is > the only preamble > > This model imports types from [RFC6991], [RFC8519], and > [RFC8294]. > > It is helpful for me to have all references briefly listed up > front. [Med] I understand this is subjective. FWIW, the reco we have in 8407/8407bis does not restrict the location of the citations, e.g.,: Be sure citations for all imported modules are present somewhere in the document text (outside the YANG module). > But I'll leave it to you to decide. > [Med] I will leave this unchanged as we already cite them. Thanks. > > > > A general comment when using XSLT to create the initial IANA > > > modules. > > > The supplied XSLT stylesheets use > > > > < >. > > > I suggest this is mentioned because I did not know about this > > > project and had search for it myself by the import of iana- > > > yinx.xsl. > > > > > > > [Med] This is mentioned several times in the draft: e.g., > > > > In the ACK: > > > > The IANA-maintained modules were generated using an XSLT > stylesheet > > from the 'iana-yang' project > > > > In the modules: > > This version of this YANG module was generated from the > > corresponding IANA registry using an XSLT stylesheet > from the > > 'iana-yang' project > > > > > > Added a new statement to in the introduction. > > Thank you. I managed to miss where it was already mentioned. [Med] :-) > > > > > Regarding match-on-payload, it is not understandable from > Section > > > 3.6 alone how the mechanism works. From my reading of the > rest of > > > the document it works by matching the data after a given > offset, > > > with a specific length, and computing the result of the > operator > > > applied to the extracted payload and the binary pattern. I > find the > > > naming of "offset-end" and "prefix" confusing, since "offset- > end" > > > is really a length and not a position; suggest renaming it to > e.g. > > > "payload-length" > > > or "length". Regarding "prefix" I suggest renaming it to > "pattern" > > > or "bitmask", since it might not be a prefix but an arbitrary > part > > > of a payload and it is used for the matching. > > > > [Med] Work for me. Went for length and pattern. Thanks. > > Thanks! > > > > > iana-icmpv4-types@xxxxxxxxxxxxxxx: > > > > > > * pyang fails with > > > > > > iana-icmpv4-types@xxxxxxxxxxxxxxx:280: error: > unterminated > > > statement > > > definition for keyword "enum", looking at I > > > > > > > [Med] This is because of folding. The module will be removed > anyway from the final document. > > As long as the final YANG module validates accord to our rules. > > > > > * yanglint validation fails: > > > > > > libyang err : Invalid character sequence > > > > > > > "ICMPmessagesutilizedbyexperimentalmobilityprotocolssuchasSeamoby > > > ", > > > expected a keyword. (Line number 280.) libyang err : > Parsing > > > module > > > "iana-icmpv4-types" failed. libyang err : Loading "iana- > > > icmpv4-types" > > > module failed. libyang err : Parsing module "ietf-acl- > enh" > > > failed. > > > > > > Missing reference to RFC 2780 for Type 0, 3, 8, 9, 10, 11, > 12, 13, > > > 14, and 40. > > > > Med: The registry is authoritative here. We are mirroring > exactly what is in the registry. Won't make this modification. > > > > I notice that there is a discrepancy in the IANA > > > ICMP Parameters registry listing of ICMP Type Numbers section > and > > > the Code Fields section. Suggest to notify IANA and rebuild > the YANG > > > module. > > > > Med: RFC2780 is not useful for understanding these types. I > don't think we need to touch this, but this is beyond the scope > of this doc. > > Ok. Perhaps a good idea to notify IANA anyway. > I can do that, any pointers for whom to contact is greatly > appreciated! [Med] Thank you. You can send an email to <iana-issues-comment@xxxxxxxx>. > > > > > > > > The enum > > > > ICMPmessagesutilizedbyexperimentalmobilityprotocolssuchasSeamoby > > > is problematic because it is so long (when YANG modules > should fit > > > in 69 characters line length). This is why the tools can't > validate > > > the module. Perhaps truncate it to > > > ICMPmessagesutilizedbyexperimentalmobilityprotocols? > > > > > > Reading RFC 4065 it seems that ICMPExperimentalMobilityType > is a > > > fitting name, since it is designed not only for Seamoby but > also for > > > other experimental mobility protocols.. > > > > > > The following XSLT patch would remove "suchasSeamoby" from > the enum > > > name. > > > > Med: OK > > This would also resolve the pyang and yanglint errors. [Med] Indeed. > > > > > iana-icmpv6-types@xxxxxxxxxxxxxxx: > > > * pyang OK > > > * yanglint OK > > > > > > The following change is done when running > > > > > > $ pyang -f yang --yang-line-length=69 iana-icmpv6- > types@2023- > > > 04-28.yang \ > > > > iana-icmpv6-types@xxxxxxxxxxxxxxxxxxxx > > > > > > $ diff -u iana-icmpv6-types@xxxxxxxxxxxxxxx \ > > > > iana-icmpv6-types@xxxxxxxxxxxxxxxxxxxx > > > > > > - enum > > > > ICMPmessagesutilizedbyexperimentalmobilityprotocolssuchasSeamoby > > > { > > > + enum > > > + > > > + > > > > ICMPmessagesutilizedbyexperimentalmobilityprotocolssuchasSeamoby > > > { > > The XSLT stylesheet change for icmpv4 is applicable for icmpv6 as > well. > > [Med] Agree. > Thank you for your contribution! > > > -- > Per ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
