Re: [PATCH v1 1/3] raw/ntb: generalize framework for multiple vendors

[Bruce Richardson <bruce.richardson@intel.com>]

On Sun, Aug 23, 2026 at 07:36:37PM +0530, Raghavendra Ningoji wrote:
> The NTB rawdev framework was written around the Intel back-to-back
> topology and the built-in scratchpad handshake protocol. To allow
> other vendors to plug into the same framework, add vendor-neutral
> hooks and make the common code dispatch through them:
> 
> - Add NTB_TOPO_PRI/NTB_TOPO_SEC topology types for hardware that uses
>   a primary/secondary topology instead of back-to-back.
> - Add optional ntb_dev_ops hooks: interrupt_handler (vendor-specific
>   MSI-X handler), dev_handshake (vendor-specific link handshake) and
>   read_peer_config (vendor-specific peer-config read at start). When a
>   hook is NULL the common code keeps using the existing built-in path,
>   so the Intel driver is unaffected.
> - Add a pmd_private pointer to struct ntb_hw for vendor-specific state.
> - Guard the receive path against a malformed stream with no end-of-packet
>   marker so it cannot overflow the descriptor ring.
> 
> Signed-off-by: Raghavendra Ningoji <raghavendra.ningoji@amd.com>
> ---
>  drivers/raw/ntb/ntb.c | 94 ++++++++++++++++++++++++++++---------------
>  drivers/raw/ntb/ntb.h | 18 +++++++++
>  2 files changed, 80 insertions(+), 32 deletions(-)
> 
<snip>
>  enum ntb_link {
> @@ -100,6 +103,8 @@ enum ntb_spad_idx {
>   * for those db bits.
>   * @peer_db_set: Set doorbell bit to generate peer interrupt for that bit.
>   * @vector_bind: Bind vector source [intr] to msix vector [msix].
> + * @interrupt_handler: Vendor-specific interrupt handler. If NULL, the
> + * built-in handler is used.
>   */
>  struct ntb_dev_ops {
>  	int (*ntb_dev_init)(const struct rte_rawdev *dev);
> @@ -119,6 +124,16 @@ struct ntb_dev_ops {
>  	int (*peer_db_set)(const struct rte_rawdev *dev, uint8_t db_bit);
>  	int (*vector_bind)(const struct rte_rawdev *dev, uint8_t intr,
>  			   uint8_t msix);
> +	void (*interrupt_handler)(void *param);
> +	/* Optional vendor-specific handshake. If NULL, the built-in
> +	 * scratchpad handshake is used. Used by hardware (e.g. AMD) whose
> +	 * scratchpad layout differs from the built-in protocol.
> +	 */
> +	int (*dev_handshake)(const struct rte_rawdev *dev);
> +	/* Optional vendor-specific peer-config read at device start. If NULL,
> +	 * the built-in scratchpad reads are used.
> +	 */
> +	int (*read_peer_config)(const struct rte_rawdev *dev);
>  };

For these new op fields, do you foresee cases where other drivers might use
the "default" functions as you have now? Might it be better to simplify
things and always use driver-supplied ops, converting the existing
functions into intel-specific ops, rather than making them fallback
functions?

/Bruce

<snip>