Bonjour Med,

Thanks for the proposed changes, they address all my points but the one about a 
dynamic processing for TCP flags (there was recently an TCPM I-D at IESG 
evaluation changing these bits, hence my proposal).

Regards

-éric

From: [email protected] <[email protected]>
Date: Monday, 31 March 2025 at 16:18
To: Eric Vyncke (evyncke) <[email protected]>, The IESG <[email protected]>
Cc: [email protected] 
<[email protected]>, [email protected] 
<[email protected]>, [email protected] <[email protected]>, [email protected] 
<[email protected]>, [email protected] <[email protected]>
Subject: RE: Éric Vyncke's No Objection on draft-ietf-netmod-acl-extensions-15: 
(with COMMENT)
Éric,

Thanks for the careful review.

A diff to track changes to address your review can be seen at: 
https://author-tools.ietf.org/api/iddiff?url_1=https://netmod-wg.github.io/enhanced-acl-netmod/draft-ietf-netmod-acl-extensions.txt&url_2=https://netmod-wg.github.io/enhanced-acl-netmod/eric-review/draft-ietf-netmod-acl-extensions.txt.

Please see more context inline.

Cheers,
Med

> -----Message d'origine-----
> De : Éric Vyncke via Datatracker <[email protected]>
> Envoyé : lundi 31 mars 2025 14:49
> À : The IESG <[email protected]>
> Cc : [email protected]; netmod-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Objet : Éric Vyncke's No Objection on draft-ietf-netmod-acl-
> extensions-15: (with COMMENT)
>
> Éric Vyncke has entered the following ballot position for
> draft-ietf-netmod-acl-extensions-15: No Objection
>
> When responding, please keep the subject line intact and reply to
> all email addresses included in the To and CC lines. (Feel free to
> cut this introductory paragraph, however.)
>
>
> ------------------------------------------------------------------
> COMMENT:
> ------------------------------------------------------------------
>
>
> # Éric Vyncke, INT AD, comments for draft-ietf-netmod-acl-
> extensions-15
> CC @evyncke
>
> Thank you for the work put into this document. It is easy to read
> and add real
> value to ACL.
>
> Please find below  some non-blocking COMMENT points (but replies
> would be
> appreciated even if only for my own education), and some nits.
>
> Special thanks to Lou Berger for the shepherd's write-up including
> the WG
> 'limited' interest/consensus and the justification of the intended
> status.
>
> Other thanks to Tim Wicinski, the Internet directorate reviewer,
> please
> consider this int-dir last-call review:
> > (status "ready")
>
> I hope that this review helps to improve the document,
>
> Regards,
>
> -éric
>
> ## COMMENTS (non-blocking)
>
> ### Abstract
>
> s/This document discusses a set of extensions/This document
> specifies a set of
> extensions/ after all, its intended status is proposed standard.

[Med] ACK

>
> ### Section 1
>
> Humm... I understand what is meant but this paragraph appears to
> be
> self-contradicting `Network operators maintain sets of IP prefixes
> ... These
> lists are maintained and manipulated by security expert teams`
> (suggest adding
> "of the network operators").

[Med] ACK

>
> It took me a while to parse `supporting means to easily map to the
> filtering
> rules conveyed in messages triggered by these tools is valuable
> from a network
> operation standpoint` mainly because the subject of "is valuable"
> is too long.

[Med] Tweaked a bit for better readability.

>
> ### Section 2
>
> In `IP address, IP prefixes,` any reason why the plural form is
> used for "IP
> prefixes" ?

[Med] Updated all to use the plural form.

>
> ### Section 3.2
>
> Where are the names defined in ` A protocol can be identified
> either by a
> number (e.g., 17) or a name (e.g., UDP).`
>
> Should the example for aliases be dual-stack ? I.e., having both
> an IPV6
> address and an IPv4 one ? Same comment for section D.1

[Med] These are only examples. Nothing prescriptive. What is important is that:

     grouping alias:
       +-- ...
       +-- prefix*       inet:ip-prefix  <==== dual-stack is supported
       +-- ...
       +-- protocol*     uint8
       +-- ...

No change is made.

>
> I was about the ballot a DISCUSS on `beyond just the header
> information` which
> header is it ? Layer-2 ? IP ? Based on `identity offset-type`
> appearing later,
> I am balloting NoObjection but the clarification should already be
> in this
> section.

[Med] Added "An offset type (e.g., layer 2 or layer 3) is used to indicates the 
position of the data in packet to use for the match."  right after the sentence 
you quoted.

>
> ### Section 3.6
>
> Related to my near-DISCUSS on section 3.2, `data offset` from
> which start ?

[Med] Added "The data offset indicates the position to look at in a packet 
(e.g., starts at the beginning of the IP header or transport header)." right 
after the text you quoted.

>
> ### Section 4
>
> Generic comment: why next-header-set for IPv4 and not protocol-set
> as in IPv6
> as they refer to the same identities ?

[Med] Good catch. protocol-set seems OK.


 Or even having protocol
> subtree to be
> version agnostic (like TCP), i.e., some operators would probably
> like to allow
> protocol == 50 (ESP) on both IPv6 and IPv4.
>

[Med] Aliases can be used for such case. I prefer to keep the current structure.


> Like Erik Kline, I think that `identity layer4` for offset is not
> correct and
> Erik's suggestion is correct.
>
> `The offset start right after the end of the transport payload.`,
> I think that
> the authors mean "transport header".

[Med] Agree, fixed in the PR shared in the Erik's thread. Thanks.

>
> Rather than defining identities for all TCP flags (e.g., `identity
> ack`), why
> not using the same technique as for ICMP type, i.e., rely on the
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> www.iana.org%2Fassignments%2Ftcp-parameters%2Ftcp-<http://www.iana.org%2Fassignments%2Ftcp-parameters%2Ftcp->
> parameters.xhtml%23tcp-header-
> flags&data=05%7C02%7Cmohamed.boucadair%40orange.com%7Ca5ed51ed2167
> 4b6b5cee08dd70527995%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0%7C0%7C
> 638790221711845959%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWU
> sIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D
> %3D%7C0%7C%7C%7C&sdata=SvWtMTyED%2B2ChKMgY8GJ6i9zhlzK0B0Shzvj297Wk
> YM%3D&reserved=0
> IANA registry?

[Med] This is for practical reasons: the list is short and can be handled in 
the IETF module itself. The set of currently defined flags is likely to be 
stable for a while ;-)

>
> ### Section 6.3
>
> Several IANA instructions are similar to `"enum": Replicates the
> name from the
> registry with all spaces striped.`, I am unsure whether the result
> will be
> readable and useful, it there a reason why the spaces must be
> removed ?

[Med] This is because space is not allowed in identifiers:

   identifier          = (ALPHA / "_")
                         *(ALPHA / DIGIT / "_" / "-" / ".")

>
> The "(deprecated)" and "(obsolete)" status appears only in the
> ICMPv4 registry,
> unsure whether they are applicable to ICMPv6 and extension headers
> registries.
> I will trust IANA review on this section.

[Med] ACK

>
> ### Section 7.2
>
> As some IANA registries are used as input by the XSLT in appendix
> A, I wonder
> whether they should be normative references.

[Med] Good point. Fixed.

>
> ### Section E.3
>
> Should there also be a match on the 'protocol' ?

[Med] Added check on 41.

 I.e., do not
> match for TCP
> packets having "2001:db8::1"
>
> Moreover, I guess that the payload match is a binary comparison so
> it will
> never match the ASCII "2001:db8::1"

[Med] The base64 encoding should be used here, but we went for simplification 
and this is why we have this note :-)

"  For the readers' convenience, the textual representation of the
   pattern is used in the example instead of the binary form. "


, suggest using an hexadecimal
> string in
> this example.
>
> ## NITS (non-blocking / cosmetic)
>
> s/transpot/transport/ (saw it at least once)
>

[Med] Fixed. Thanks.

>

____________________________________________________________________________________________________________
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.
_______________________________________________
netmod mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to