Re: [PATCH net-next 4/6] netdevsim: psp: implement kdf from psp spec

[Daniel Zahka <daniel.zahka@gmail.com>]

On 5/11/26 3:49 PM, Willem de Bruijn wrote:
> Daniel Zahka wrote:
>> Implement the PSP key derivation function (KDF) per the PSP
>> Architecture Spec.
>>
>> The kdf is used to generate spi + session key pairs, and will also be
> Text is a bit ambiguous here: the kdf does not generate the spi. It
> derives a session key from the master key and spi.
>
>> used in the rx path to re-derive the tx key used by the peer.
>>
>> Also, remove support for psd->generation, as it is not needed for
>> netdevsim after removing the fake authentication hack.
> Is psd->generation only used inside driver code, not by the core PSP
> stack? Else it should be set to !!(ns->psp.spi & PSP_SPI_KEY_PHASE) on
> key rotation. If only used by the driver, no need to reset it on each
> rotation.


Core tries to 'suggest' a generation to the driver, which is the last 
generation + 1, before calling into key_rotate(), but this won't work 
for netdevsim. I could set the generation to !!(ns->psp.spi & 
PSP_SPI_KEY_PHASE) so that it aliases the device key selection bit, but 
I think this is basically the same as just setting it to 0. This series 
makes the generation field dead code in core. The old netdevsim 
implementation relied on it to do its fake authentication hack, but that 
is removed with this series. The only real hw implementation we have, 
mlx5, does not support device key generations. Maybe we need to just 
remove that until a real driver comes along as a user.


>> Assisted-by: Claude:claude-opus-4.6
>> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
>>   enum skb_drop_reason
>>   nsim_psp_handle_tx(struct sk_buff *skb, struct netdevsim *ns)
>>   {
>> @@ -155,7 +189,7 @@ nsim_rx_spi_alloc(struct psp_dev *psd, u32 version,
>>   		  struct netlink_ext_ack *extack)
>>   {
>>   	struct netdevsim *ns = psd->drv_priv;
>> -	int i;
>> +	unsigned int phase;
>>   
>>   	if ((ns->psp.spi ^ (ns->psp.spi + 1)) & PSP_SPI_KEY_PHASE) {
>>   		NL_SET_ERR_MSG(extack, "SPI space exhausted");
>> @@ -163,9 +197,11 @@ nsim_rx_spi_alloc(struct psp_dev *psd, u32 version,
>>   	}
>>   
>>   	assoc->spi = cpu_to_be32(++ns->psp.spi);
>> -	assoc->key[0] = psd->generation;
>> -	for (i = 1; i < PSP_MAX_KEY; i++)
>> -		assoc->key[i] = ns->psp.spi + i;
>> +	phase = !!(ns->psp.spi & PSP_SPI_KEY_PHASE);
>> +
>> +	/* dev_keys_lock not needed because of psd->lock */
> Can you elaborate a bit?
>
> Is dev_keys_lock only used to synchronize the writers, then? Which after
> device init would only be concurrent invocations of nsim_key_rotate. But
> that operation correctly also holds the device lock using
> psp_device_get_locked.


This is an error in splitting changes in the series on my part. The 
spinlock is used to synchronize the writer with the reader running the 
kdf in the napi_poll() path in the later aes-gcm commit. I should have 
added the spinlock stuff when that reader was introduced. The comment is 
just pointing out that all of the psp_dev_ops on a psd are serialized 
with the psd->lock. I'll fix that in the respin.