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

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

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.

> +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?

>  struct psp_assoc {
>  	struct psp_dev *psd;
>  
> @@ -159,6 +233,23 @@ struct psp_assoc {
>  	u8 generation;
>  	u8 version;
>  	u8 peer_tx;
> +	/* enum psp_assoc_flags. Written under psd->lock, additionally read
> +	 * on the Tx fast path without it. A snapshot of the device config
> +	 * taken when the association was created, so that the header size,
> +	 * and with it the MSS, cannot change under an established
> +	 * connection.
> +	 */
> +	u8 flags;
> +
> +	/* Queue IDs for the VC, ours and the peer's. @vc_loc is refreshed
> +	 * from the Tx queue selection and goes out as the cookie's request,
> +	 * @vc_rem is learned from the peer's requests and goes back out as
> +	 * the destination. Both are PSP_VC_QID_NONE until something is
> +	 * learned. Written without the socket lock, always use
> +	 * READ_ONCE()/WRITE_ONCE().
> +	 */
> +	u16 vc_loc;
> +	u16 vc_rem;
>  

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