stoffi92 / rfc5575bis

0 stars 0 forks source link

IESG Benjamin Kaduk: Section 4.2.2.7 #194

Closed stoffi92 closed 4 years ago

stoffi92 commented 4 years ago

Section 4.2.2.7

In case of the presence of the ICMP type (code) component only ICMP packets can match the entire Flow Specification. The ICMP type

I'm not sure why the parenthetical is needed given that there's a separate NRLI type for "ICMP code".

suehares commented 4 years ago

you can remove (code).

stoffi92 commented 4 years ago

@suehares do you agree? Please comment and assign back to me.

suehares commented 4 years ago

ICMP type (code) component - reads poorly.

What do you think of using Old /ICMP type (code) component/ New / ICMP type component code/

Or is my /NEW/ suggestion was that what we replaced earlier?

stoffi92 commented 4 years ago

Maybe I have not been clear...

The last paragraph of Section 4.2.2.7 (ICMP type) is also cross-referenced in Section 4.2.2.8 (ICMP code), because the same principle that applies for ICMP type also applies for the ICMP code component. So Section 4.2.2.8 reads:

The last paragraph of Section 4.2.2.7 also applies to this component.

If we would remove the (code) from that paragraph in Section 4.2.2.7 we end up with a paragraph that only talks about ICMP type but should be applied to the ICMP code component as well.

In case of the presence of the ICMP type (code) component only ICMP
   packets can match the entire Flow Specification.  The ICMP type
   (code) component, if present, never matches when the packet's IP
   protocol value is not 1 (ICMP), if the packet is fragmented and this
   is not the first fragment, or if the system is unable to locate the
   transport header.  Different implementations may or may not be able
   to decode the transport header in the presence of IP options or
   Encapsulating Security Payload (ESP) NULL [RFC4303] encryption.

Maybe removing (code) from the paragraph and simply copying the same text to the following section (4.2.2.8) while replacing type by code instead of this cross reference is the better solution. It definitely reads easier.

I think I will do this.

stoffi92 commented 4 years ago

//doc The reason for this is that the following section (icmp code) also refers to the last paragraph of Section 4.2.2.7 ... otherwise we need to repeat that paragraph in section 4.2.2.8 with "type" replaced by "code". However it seems to be misleading -> copied the whole paragraph to the code component and removed the parentheses.