Re: [PATCH net-next v2 3/4] NTB: ntb_transport: Add per-payload client metadata

On 8/16/26 11:49 PM, Koichiro Den wrote:
> ntb_transport currently carries only payload bytes, with no way for clients
> to associate metadata with an individual payload.
> 
> The payload header has a 32-bit flags field, with only BIT(0) and BIT(1) in
> use. Carry opaque client metadata in the upper 24 bits. Expose it through
> the transmit enqueue interface and receive callback. Reject values that do
> not fit. Keep the low byte for transport flags so future flags can continue
> from BIT(2).
> 
> No protocol version bump is needed. Existing Linux version 4 peers ignore
> the upper bits on receive and always transmit them as zero.
> 
> Adapt ntb_netdev to the new interfaces without using metadata.
> 
> Signed-off-by: Koichiro Den <[email protected]>

Reviewed-by: Dave Jiang <[email protected]>


> ---
> Changes in v2:
>   - No changes.
> 
>  drivers/net/ntb_netdev.c      |  4 ++--
>  drivers/ntb/ntb_transport.c   | 18 +++++++++++++-----
>  include/linux/ntb_transport.h |  6 ++++--
>  3 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
> index 029a4a532a10..5c7fe6883cb9 100644
> --- a/drivers/net/ntb_netdev.c
> +++ b/drivers/net/ntb_netdev.c
> @@ -123,7 +123,7 @@ static void ntb_netdev_event_handler(void *data, int link_is_up)
>  }
>  
>  static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
> -				  void *data, int len)
> +				  void *data, int len, unsigned int meta)
>  {
>  	struct ntb_netdev_queue *q = qp_data;
>  	struct ntb_netdev *dev = q->ntdev;
> @@ -258,7 +258,7 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
>  
>  	ntb_netdev_maybe_stop_tx(ndev, q, tx_stop);
>  
> -	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len);
> +	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len, 0);
>  	if (rc)
>  		goto err;
>  
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 967a5ad38164..70be06a42201 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -167,7 +167,7 @@ struct ntb_transport_qp {
>  	unsigned int tx_max_frame;
>  
>  	void (*rx_handler)(struct ntb_transport_qp *qp, void *qp_data,
> -			   void *data, int len);
> +			   void *data, int len, unsigned int meta);
>  	struct list_head rx_post_q;
>  	struct list_head rx_pend_q;
>  	struct list_head rx_free_q;
> @@ -264,6 +264,10 @@ enum {
>  	LINK_DOWN_FLAG = BIT(1),
>  };
>  
> +/* Reserve the low byte for transport flags. */
> +#define DESC_META_SHIFT		8
> +#define DESC_META_MASK		(~0U << DESC_META_SHIFT)
> +
>  struct ntb_payload_header {
>  	__le32 ver;
>  	__le32 len;
> @@ -1436,6 +1440,7 @@ static void ntb_complete_rxc(struct ntb_transport_qp *qp)
>  	struct ntb_queue_entry *entry;
>  	void *cb_data;
>  	unsigned int len;
> +	unsigned int meta;
>  	unsigned long irqflags;
>  
>  	spin_lock_irqsave(&qp->ntb_rx_q_lock, irqflags);
> @@ -1451,13 +1456,14 @@ static void ntb_complete_rxc(struct ntb_transport_qp *qp)
>  
>  		cb_data = entry->cb_data;
>  		len = entry->len;
> +		meta = entry->flags >> DESC_META_SHIFT;
>  
>  		list_move_tail(&entry->entry, &qp->rx_free_q);
>  
>  		spin_unlock_irqrestore(&qp->ntb_rx_q_lock, irqflags);
>  
>  		if (qp->rx_handler && qp->client_ready)
> -			qp->rx_handler(qp, qp->cb_data, cb_data, len);
> +			qp->rx_handler(qp, qp->cb_data, cb_data, len, meta);
>  
>  		spin_lock_irqsave(&qp->ntb_rx_q_lock, irqflags);
>  	}
> @@ -1656,6 +1662,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
>  
>  	entry->rx_hdr = hdr;
>  	entry->rx_index = qp->rx_index;
> +	entry->flags = flags & DESC_META_MASK;
>  
>  	if (len > entry->len) {
>  		dev_dbg(&qp->ndev->pdev->dev,
> @@ -2341,6 +2348,7 @@ EXPORT_SYMBOL_GPL(ntb_transport_rx_enqueue);
>   * @cb: per buffer pointer for callback function to use
>   * @data: pointer to data buffer that will be sent
>   * @len: length of the data buffer
> + * @meta: client metadata to send with the buffer
>   *
>   * Enqueue a new transmit buffer onto the transport queue from which a NTB
>   * payload will be transmitted.  This assumes that a lock is being held to
> @@ -2349,12 +2357,12 @@ EXPORT_SYMBOL_GPL(ntb_transport_rx_enqueue);
>   * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
>   */
>  int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
> -			     unsigned int len)
> +			     unsigned int len, unsigned int meta)
>  {
>  	struct ntb_queue_entry *entry;
>  	int rc;
>  
> -	if (!qp || !len)
> +	if (!qp || !len || meta > NTB_TRANSPORT_MAX_META)
>  		return -EINVAL;
>  
>  	/* If the qp link is down already, just ignore. */
> @@ -2370,7 +2378,7 @@ int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
>  	entry->cb_data = cb;
>  	entry->buf = data;
>  	entry->len = len;
> -	entry->flags = 0;
> +	entry->flags = meta << DESC_META_SHIFT;
>  	entry->errors = 0;
>  	entry->tx_index = 0;
>  
> diff --git a/include/linux/ntb_transport.h b/include/linux/ntb_transport.h
> index 7243eb98a722..9e807542b6c4 100644
> --- a/include/linux/ntb_transport.h
> +++ b/include/linux/ntb_transport.h
> @@ -50,6 +50,8 @@
>  
>  struct ntb_transport_qp;
>  
> +#define NTB_TRANSPORT_MAX_META	0x00ffffffU
> +
>  struct ntb_transport_client {
>  	struct device_driver driver;
>  	int (*probe)(struct device *client_dev);
> @@ -63,7 +65,7 @@ void ntb_transport_unregister_client_dev(char *device_name);
>  
>  struct ntb_queue_handlers {
>  	void (*rx_handler)(struct ntb_transport_qp *qp, void *qp_data,
> -			   void *data, int len);
> +			   void *data, int len, unsigned int meta);
>  	void (*tx_handler)(struct ntb_transport_qp *qp, void *qp_data,
>  			   void *data, int len);
>  	void (*event_handler)(void *data, int status);
> @@ -78,7 +80,7 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp);
>  int ntb_transport_rx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
>  			     unsigned int len);
>  int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
> -			     unsigned int len);
> +			     unsigned int len, unsigned int meta);
>  void *ntb_transport_rx_remove(struct ntb_transport_qp *qp, unsigned int *len);
>  void ntb_transport_link_up(struct ntb_transport_qp *qp);
>  void ntb_transport_link_down(struct ntb_transport_qp *qp);