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

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Daniel Zahka wrote:
> 
> 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(),


Oh right, I recall the feature now. It is more than just the MSB bit flip.

> but this won't work for netdevsim.

Why not, if it otherwise dead code to the driver?

That seems most in line with intent. I'd say keep it and allow this driver
to ignore it, if it has purpose to the stack. Or remove it otherwise. Does
not have to be in this series. But I don't see a need to explicitly touch
the field.

I think the intent was previousy to avoid on double rotate to deliver
data from gen N+2 to sockets associated with an SPI from gen N. But I
don't know the details and whether that is obsolete (probably).

> 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.
>