Message-ID: <66729953651ba_2751bc294fa@willemb.c.googlers.com.notmuch>
Date: Wed, 19 Jun 2024 04:39:47 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: "Singhai, Anjali" <anjali.singhai@...el.com>, 
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc: Paolo Abeni <pabeni@...hat.com>, 
 "willemdebruijn.kernel@...il.com" <willemdebruijn.kernel@...il.com>, 
 Boris Pismenny <borisp@...dia.com>, 
 "gal@...dia.com" <gal@...dia.com>, 
 "cratiu@...dia.com" <cratiu@...dia.com>, 
 "rrameshbabu@...dia.com" <rrameshbabu@...dia.com>, 
 "steffen.klassert@...unet.com" <steffen.klassert@...unet.com>, 
 "tariqt@...dia.com" <tariqt@...dia.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 "Samudrala, Sridhar" <sridhar.samudrala@...el.com>, 
 "Acharya, Arun Kumar" <arun.kumar.acharya@...el.com>
Subject: Re: [RFC net-next 00/15] add basic PSP encryption for TCP connections

Singhai, Anjali wrote:
> 
> In reference to this patch series
> https://lore.kernel.org/netdev/20240510030435.120935-1-kuba@kernel.org/#t
> 
> Thanks a lot  for the PSP crypto enabling patches in the kernel.
> Some points that we noticed that could use some enhancements/fixes
> 
> 1. Why do we need  ndo_op set_config() at device level which is setting only one version, instead the description above the psp_dev struct which had a mask for enabled versions at a  device level is better and device lets the stack know at psp_dev create time what all versions it is capable of.  Later on, version is negotiated with the peer and set per session.
> Even the Mellanox driver does not implement this set_config ndo_op. 
>  
> 2. Where is the association_index/handle returned to the stack to be used with the packet on TX by the driver and device? ( if an SADB is in use on Tx side in the device), what we understand from Mellanox driver is, its not doing an SADB in TX in HW, but passing the key directly into the Tx descriptor? Is that right, but other devices may not support this and will have an SADB on TX and this allowed as per PSP protocol. Of course on RX there is no SADB for any device.
> In our device we have 2 options, 
>              1. Using SADB on TX and just passing SA_Index in the descriptor (trade off between performance and memory. 
>               As  passing key in descriptor makes for a much larger TX descriptor which will have perf penalty.)
>              2. Passing key in the descriptor.
>               For us we need both these options, so please allow for enhancements.
> 
> 3. About the PSP and UDP header addition, why is the driver doing it? I guess it's because the SW equivalent for PSP support in the kernel does not exist and just an offload for the device. Again in this case the assumption is either the driver does it or the device will do it.
> Hope that is irrelevant for the stack. In our case most likely it will be the device doing it.
> 
> 4. Why is the driver adding the PSP trailer? Hoping this is between the driver and the device, in our case it's the device that will add the trailer.

This does not adhere to the spec:

"An option must be provided that enables upper-level software to send packets that are
pre-formatted to include the headers required for PSP encapsulation. In this case, the
NIC will modify the contents of the headers appropriately, apply
encryption/authentication, and add the PSP trailer to the packet."

https://raw.githubusercontent.com/google/psp/main/doc/PSP_Arch_Spec.pdf

  
> 5. Five way handshake, can this be optimized to 3 way?
> Here is what we think is happening right now at the IKE level interaction for the two ends of the session, as PSP protocol does not define it but based on the implementation this is what we gathered.
>   Looks like its 5 way handshake that is happening with the session partner.
>    For example two sides are called Tx session partner and RX session partner, just because TX initiates the session creation, of course it's a full duplex session.
>              1. TX session partner sends over sideband tls channel to the Rx session partner what all versions the TX can do based on caps learnt from the device driver.
>              2. The Rx session partner replies with what common versions it can do back to TX session partner based on dev caps it learnt from the device.

In practice in reasonably homogeneous hyperscale environments, this
step can be skipped. To be in spec devices must support both 128b
and 256b AES-GCM (and nothing else). It is an organizational choice
which to deploy.

>              3. Tx session partner tells the driver to generate a spi an session key for the rx side by specifying the version it wants to use for this session. And then sends to Rx session partner the spi and session key and version selected etc.
>              4. The Rx session partner at its end then talks to its driver to generate a spi an session key for the rx side by specifying the version sent from TX session partner. And then sends to Tx session partner the spi and session key and version selected etc. The RX session partner also programs the session key and spi it received from TX session partner in HW in its Tx SADB (psp_assoc_add)
>             5. Tx session partner programs the received SPI and session key in its HW in its Tx SADB (psp_assoc_add). When done it sends an ACK back to Rx session partner that the session is ready.
>  
> Should this be optimized to a  3 way handshake to allow for faster session setup? 1 and 3 and 2 and 4 could be combined in an intelligent way. Again may be there is already an optimized way to do 3 way handshake here and the kernel-driver flow still looks the same.
> 
> Thanks
> Anjali
> 

Powered by blists - more mailing lists