[IPsec] Shepherd review of the draft-ietf-ipsecme-multi-sa-performance

[Tero Kivinen <[e-mail]>]

In section 1 Introduction:

   While this could be (partially) mitigated by setting up multiple
   narrowed Child SAs, for example using Populate From Packet (PFP) as
   specified in [RFC4301], this IPsec feature is not widely
   implemented.

I think it would be better to give better reason why PFP is not that
useful, for example because it could cause way too many Child SAs,
and/or way too few Child SAs as the number of Child SAs would be
depending on the traffic flow patterns.

I.e., if you have all traffic in one TCP/IP session (for example
running backup), then you will get only one Child SA for all traffic.
On the other hand if you have lots of separate short lived TCP
connections (like web browser opening multiple connections to
different web servers ect), you would have lots of Child SAs, which
would be useless after short while.

--

In section 1 change:

   "as specified in [RFC4301]"

with

   "as specifeid in IPsec Architecture [RFC4301]"

--

Add section 1.2 Terminology where you say you use terms Child SA, TSi,
TSr, Notification Data, Traffic Selectors, CREATE_CHILD_SA,
Configuration Payloads, etc defined in the RFC7296. Expand the list to
include all terms used from RFC7296.

--

Section 2 typo:

s/Implementations/implementations/

--

Section 3 typo:

s/multple/multiple/

Also you are using Notification Data (as specified n RFC7296), but
then in the end of 1st paragraph you use "notify data payload".
Changing those to be consistent would be good.

--

In section 4 change twice:

   in [RFC7296] Section 2.9.

with

   in IKEv2 [RFC7296] Section 2.9.

also change

   As per RFC 7296,

with

   As per IKEv2,

(the IKEv2 rfc might not stay 7296 forever :)

--

In section 5 say that the Notify Payload format is defined in IKEv2
[RFC7296] section 3.10, and are copied only here to make this document
easier to read.

--

In section 5.1 you say that Protocol id MUST contain either 2 for AH
and 3 for ESP, but on the RFC7296 says that "If the SPI field is
empty, this field MUST be sent as zero and MUST be ignored on
receipt." and as this notify is sent with empty SPI field, then the
Protocol ID field MUST be 0 also.

--

In section 5.1 add text saying that SPI Size MUST be zero.

--

In section 5.1 fix s/opague/opaque/ twice.

--

In section 6 there is text saying:

   If the IKEv2 extension defined in this document is negotiated with
   the peer, an implementation which does not support receiving
   per-CPU packet trigger messages MAY initiate all its Child SAs
   immediately upon receiving the (only) packet trigger message it
   will receive from the IPsec stack.

On the other hand there is no negotiation of the this extension. What
is this text trying to say? Perhaps simply remove change to say "If an
implementation does not support ... it MAY ..."

--

Section 9 the correct heading for the IANA registries 2nd column are

 	Notify Messages - Status Types

and

	Notify Messages - Error Types

Currently the figure 2 is using status type header and even that does
not match iana registry.
-- 
[e-mail]