[IPsec] AD Review of draft-ietf-ipsecme-multi-sa-performance-05

[Roman Danyliw <[e-mail]>]

Hi 

I performed an AD review of draft-ietf-ipsecme-multi-sa-performance-05.  I have a mostly editorial feedback below:

** Abstract.  Editorial.  s/crypto/cryptography/

** Section 1.
   Most IPsec implementations are currently limited to using one queue
   or CPU resource for a Child SA.

-- (Editorial) What kind of queue?  Should it be “network queues”?

-- Why couldn’t implementations be changed to use multiple CPUs?

** Section 1.  Editorial.
   An
   unencrypted link of 10Gbps or more is commonly reduced to 2-5Gbps
   when IPsec is used to encrypt the link using AES-GCM.  By using the
   implementation specified in this document, aggregate throughput
   increased from 5Gbps using 1 CPU to 40-60 Gbps using 25-30 CPUs

-- Will this text age well?  

-- Can these statistics be cited?

** Section 1.  Editorial.
   While this could be (partially) mitigated by setting up multiple
   narrowed Child SAs, for example using Populate From Packet (PFP) as
   specified in IPsec Architecture [RFC4301], this IPsec feature would
   cause too many Child SAs (one per netflow)

Is it “netflow” or “network flow”?  “netflow” is a logging format.

** Section 1.  Editorial.
When an IKEv2 peer is receiving more additional Child SA's

It is “more additional Child SA’s” or “additional Child SA’s”?

** Section 3.
   If this initial Child SA
   will be tied to a specific resource, it MAY indicate this by
   including an identifier in the Notification Data.  

How does one tie the Child SA to the specific resource if the identifier is NOT included in the Notification data?  Wouldn’t it be mandatory to include this identifier?

** Section 4.  Is this section normative?  Why are RFC2119 key words used in an example?

** Section 4.  Doesn’t the example in the paragraph starting with “A simple distribution could be to install one additional Child SA on each CPU” violate the situation described in Section 2 (i.e., coordination across CPUs)?

** Section 5.  The diagrams in Section 5.* show “Next Payload”, a fields flags and a “Payload length”.  Where are those defined?

** Section 5.1.  Editorial. The diagram says “optional resource identifier”.  The description of the fields says “Optional Payload Data”

** Section 5.1
      The opaque data SHOULD be a
      unique identifier within all the Child SAs with the same TS
      payloads and the peer SHOULD only use it for debugging purposes.

-- Why is normative guidance being provided on the contents or semantics of an “opaque data” blob?

-- I don’t understand how to read the “SHOULD” in this text.  If not intended to be a unique identifier, what else should this opaque data be used for?

-- When and why should this be used for “non-debugging purposes”?

** Section 6.
   Peers SHOULD be lenient with the maximum number of Child SAs they
   allow for a given TSi/TSr combination to account for corner cases.

What does “lenient” mean here?

** Section 6.
   As additional Child SAs consume
   little additional overhead, allowing at the very least double its
   intended CPUs is RECOMMENDED.

Can this language please be clarified?  I don’t understand.  “Double” relative to what baseline?  Is this recommending the number Child SAs per CPU?  

** Section 6.
   An implementation MAY limit the number
   of Child SAs only based on its resources - that is only limit these
   based on regular denial of service protection.

Why can’t an implementation limit SAs based on any policy?

** Section 7.
   Similar to how an implementation should limit the number of half-open
   SAs to limit the impact of a denial of service attack, an
   implementation SHOULD limit the maximum number of additional Child
   SAs allowed per unique TSi/TSr.

Is it a SHOULD or RECOMMENDED?

** Section 7.  Editorial.
   Using multiple resource specific child SAs makes sense for high
   volume IPsec connections on IPsec gateway machines where the
   administrator has a reasonable trust relationship with the peer's
   administrator and abuse is unlikely and easilly escalated to resolve.

-- What makes a trust relationship “reasonable”?  Would it be clear to omit that word?

-- Typo.  s/easilly/easily/

** Section 7.  Typo. s/identifer/identifier/?

** Section 9.  Typo.  The registry names is incorrect (i.e., s/... Notify Messages/... Notify Messages Types/)

OLD

   This document defines one new registration for the IANA "IKEv2 Notify
   Messages - Status Types" registry.

NEW

   This document defines one new registration for the IANA "IKEv2 Notify
   Messages Types - Status Types" registry.

OLD

   This document defines one new registration for the IANA "IKEv2 Notify
   Messages - Error Types" registry.

NEW

   This document defines one new registration for the IANA "IKEv2 Notify
   Messages Types - Error Types" registry.

Regards,
Roman