Re: [RFC net-next 1/6] psp: steer Rx queues with the virtualization cookie

[Jakub Kicinski <kuba@kernel.org>]

On Sun, 23 Aug 2026 11:31:03 -0400 Daniel Zahka wrote:
> On Sat Aug 22, 2026 at 6:55 PM EDT, Jakub Kicinski wrote:
> > @@ -72,6 +90,27 @@ name: psp
> >            Present when in associated namespace, absent when in primary/host
> >            namespace.
> >          type: flag
> > +      -
> > +        name: vc-steer-cap
> > +        doc: |
> > +          Device can steer received traffic on the PSP virtualization
> > +          cookie (VC). The VC is split into a 32b reserved part, a 16b
> > +          queue ID the sender is asking the peer to send to, and a 16b
> > +          queue ID granting the peer's own request. Steering installs low
> > +          priority rules matching the latter, which win over the RSS table
> > +          result. Only needed for the rx direction; granting a peer's
> > +          request is just header generation and needs no device support.
> > +        type: flag
> > +      -
> > +        name: vc-steer-ena
> > +        doc: |
> > +          Directions taking part in VC based queue steering. Leave the
> > +          attribute out of a dev-set request to keep the current setting.
> > +          Applies to associations created from then on, existing ones keep
> > +          the setting they were created with.
> > +        type: u32
> > +        enum: vc-steer
> > +        enum-as-flags: true
> >    
> 
> Should vc-steer-ena be a connection level setting? Maybe it could go
> into rx-assoc. The way it's implemented here, the state is already per
> assoc.

That's what I started with but then given the security implications
of the current simple design I could not think of a reason do configure
this connection by connection. Besides the SW stack responsible for
security is likely somewhat orthogonal to steering configuration. 
Should have put this in the cover letter as well.

> > +Every driver has to carry the cookie, not just those which advertise
> > +``vc-steer-cap`` - granting a peer's request needs no help from the
> > +device, so the ``tx`` direction of ``vc-steer-ena`` may be turned on
> > +anywhere. Drivers must ask ``psp_assoc_vc_tx_get()`` for the cookie to
> > +place in the Tx header, and report the queue IDs a received cookie held
> > +in ``psp_skb_ext.vc_req`` and ``vc_dst`` (``psp_dev_rcv()`` does this for
> > +drivers which let the core strip the headers). Reporting is what allows
> > +the core to grant the peer's request. The steering itself is only
> > +expected of drivers which advertise ``vc-steer-cap``.
> > +
> > +When VC steering is enabled GRO implementations are allowed to ignore
> > +changes in the cookie for transport mode PSP.
> > +  
> 
> Makes sense. Do we would need to update __psp_skb_coalesce_diff() here
> then for the sw gro?

Ack.

> Unrelated, but this reminds me: some of the other fields here:
> generation, spi, peer_tx, have some reads/writes that probably require
> READ/WRITE_ONCE() and maybe deserve a similar comment here... Probably
> all bugs that I introduced :/
> 
> >  /* Encapsulate a TCP packet with PSP by adding the UDP+PSP headers and filling
> > - * them in.
> > + * them in. @vc is the virtualization cookie to place in the header, 0 for
> > + * a header with no optional fields.
> >   */
> >  bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi,
> > -			 u8 ver, __be16 sport)
> > +			 u8 ver, __be16 sport, u64 vc)
> >  {
> >  	u32 network_len = skb_network_header_len(skb);
> >  	u32 ethr_len = skb_mac_header_len(skb);
> >  	u32 bufflen = ethr_len + network_len;
> > +	u32 encap_len = PSP_ENCAP_HLEN;
> >  
> >  	if (skb->protocol != htons(ETH_P_IP) &&
> >  	    skb->protocol != htons(ETH_P_IPV6))
> >  		return false;
> >  
> > -	if (skb_cow_head(skb, PSP_ENCAP_HLEN))
> > +	if (vc)
> > +		encap_len += PSP_VC_SIZE;
> > +  
> 
> I suppose vc of 0 is probably valid. We might need something else to
> disambiguate "not present".

Ah, good point. We should probably encode the queue as idx+1,
I was going back and forth on this. Doesn't feel super clean but
it solves a bunch of corner cases, so the benefits probably outweigh
the slightly confusing indexing.