Thanks! Lgtm

On Sat, 8 Mar 2025, 23:43 , <[email protected]> wrote:

> Hi Erik,
>
> Thank you for the review.
>
> A diff to track the changes to address your feedback:
> 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/iesg-review-erik-kline/draft-ietf-netmod-acl-extensions.txt
>
> Please see inline for more context.
>
> Cheers,
> Med
>
> > -----Message d'origine-----
> > De : Erik Kline via Datatracker <[email protected]>
> > Envoyé : samedi 8 mars 2025 20:09
> > À : The IESG <[email protected]>
> > Cc : [email protected]; netmod-
> > [email protected]; [email protected]; [email protected]; [email protected]
> > Objet : Erik Kline's No Objection on draft-ietf-netmod-acl-extensions-
> > 15: (with COMMENT)
> >
> >
> > Erik Kline 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:
> > ----------------------------------------------------------------------
> >
> > # Internet AD comments for draft-ietf-netmod-acl-extensions-15
> > CC @ekline
> >
> >
> > ## Comments
> >
> > ### S4
> >
> > * The `identity layer4` description doesn't address whether IPv6
> > Extension
> >   Headers, or other "IP-layer" headers like AH, are to be skipped over
> > or
> >   not.  I suspect they are, but this description could say explicitly.
>
> [Med] Yes, I confirm.
>
> >
> >   In the spirit of "send text", here's one attempt:
> >
> >   identity layer4 {
> >     base offset-type;
> >     description
> >       "The offset start right after the IP header and any headers
> >        pertaining to that IP layer, e.g. IPv6 Extension Headers and
> > the
> >        Authentication Header (AH). This can be typically the beginning
> > of
> >        a transport header (e.g., TCP or UDP) or any encapsulation
> > scheme
> >        over IP such as IP-in-IP.";
> >   }
> >
> >   but that's just for your consideration.
>
> [Med] Thanks. Tweaked the text as follows (also mentioned other transport
> protocol per another comment below):
>
> NEW:
>       "The offset starts right after the IP header (including
>        any options or headers pertaining to that IP layer, e.g.,
>        IPv6 Extension Headers and the Authentication Header (AH)).
>
>        This can be typically the beginning of transport header
>        (e.g., UDP, TCP, SCTP, and DCCP) or any encapsulation
>        scheme over IP such as IP-in-IP.";
>
> >
> > * For the `payload` identity and the length in the `payload-match` for
> >   an `offset` of type `payload`, where is the end of the payload?
>
> [Med] The end will be the end of the data enclosed in a packet.
>
> >
> >   Specifically, does this allow matching into the UDP Options space
> > that
> >   is beyond the UDP payload but still within the IP payload?
>
> [Med] Any trailer/surplus area (not only with UDP options but also
> draft-herbert-udp-space-hdr, etc.) can be matched against in theory with a
> payload as offset + length=typical UDP Length. However, the match may not
> be deterministic as packets with non-uniform UDP lengths can be used to
> bypass such filters.
>
>
> Updated the description with this NEW:
>
>        This type may be used for matches against any data in
>        the transport payload and any surplus area (if any,
>        such as in UDP).
>
> >
> >   If the UDP Options space is excluded (or punted until future work),
> > then
> >   it might be good to have some clarification about that here (we
> > intend
> >   to include it in the payload match, exclude it, or leave it up to
> > the
> >   implementer).
> >
>
> [Med] It is tempting to add a new identity for trailers, but I prefer to
> leave this for future work.
>
> > * In `payload-match`, the `description` for `operator` reads:
> >
> >     "How to interpret the prefix match."
> >
> >   Should that be s/prefix/pattern/?  (this seems like it might be a
> >   copy-paste error?)
>
> [Med] Good catch. Fixed. This is a stale text as we used to have the name
> of the parameter "prefix" instead of "pattern" but changed that after a
> yangdoctor review.
>
> >
> > * Not important for this document, but we should probably consider
> > whether
> >   it should be good practice to include SCTP and maybe DCCP, even if
> > it's
> >   only for the port set ACL definitions and nothing fancier.
> >
> >   Just a comment, not a request for any change.
> >
> >
>
> ____________________________________________________________________________________________________________
> 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