Re: [RFC net-next 03/15] net: Add memory provider capabilities

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 10/02, Björn Töpel wrote:
> The current memory providers, devmem and io_uring zero-copy RX, give
> memory that the CPU cannot read. The core allows them only on queues
> with header-data split on, a zero header-split threshold, and no XDP
> program. AF_XDP needs a provider that the CPU can read and that
> works with XDP.
> 
> Add a capability field to the provider operations, with
> MP_CAP_READABLE as the first flag. Apply the header-split and XDP
> rules only to providers without it. A queue with an AF_XDP buffer
> pool still rejects other providers, but it accepts the provider made
> from that same pool.
> 
> Provider memory can become free while a queue is restarted, and no
> device event reports it. After a restart, and after a failed restart
> is rolled back, schedule the queue's NAPI if the queue has a
> provider, so the driver tries to refill.
> 
> Signed-off-by: Björn Töpel <bjorn@kernel.org>
> ---
>  Documentation/networking/netmem.rst     |  7 ++-
>  include/net/page_pool/memory_provider.h | 11 +++++
>  include/net/page_pool/types.h           |  7 +--
>  net/core/dev.c                          |  4 +-
>  net/core/netdev_rx_queue.c              | 57 ++++++++++++++++++++-----
>  net/ethtool/rings.c                     | 18 +++-----
>  net/xdp/xsk_buff_pool.c                 |  3 +-
>  7 files changed, 78 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/networking/netmem.rst b/Documentation/networking/netmem.rst
> index 217869d1108d..aacd185843ac 100644
> --- a/Documentation/networking/netmem.rst
> +++ b/Documentation/networking/netmem.rst
> @@ -48,8 +48,11 @@ Driver RX Requirements
>     - PP_FLAG_DMA_SYNC_DEV: netmem dma addr is not necessarily dma-syncable
>       by the driver. The driver must delegate the dma syncing to the page_pool,
>       which knows when dma-syncing is (or is not) appropriate.
> -   - PP_FLAG_ALLOW_UNREADABLE_NETMEM. The driver must specify this flag iff
> -     tcp-data-split is enabled.
> +   - PP_FLAG_ALLOW_UNREADABLE_NETMEM. This opts the page pool into the memory
> +     provider configured on its RX queue. For a provider without
> +     MP_CAP_READABLE, the driver must specify it only for a payload pool
> +     with tcp-data-split enabled. A readable provider may also back a
> +     regular or header pool.
>  
>  5. The driver must not assume the netmem is readable and/or backed by pages.
>     The netmem returned by the page_pool may be unreadable, in which case
> diff --git a/include/net/page_pool/memory_provider.h b/include/net/page_pool/memory_provider.h
> index 255ce4cfd975..d18ec079ffe2 100644
> --- a/include/net/page_pool/memory_provider.h
> +++ b/include/net/page_pool/memory_provider.h
> @@ -9,6 +9,15 @@ struct netdev_rx_queue;
>  struct netlink_ext_ack;
>  struct sk_buff;
>  
> +/**
> + * enum mp_caps - memory provider capabilities
> + * @MP_CAP_READABLE: The CPU can access provider buffers. They may back
> + *	header and regular page pools, and XDP programs may run on them.
> + */
> +enum mp_caps {
> +	MP_CAP_READABLE		= BIT(0),
> +};
> +
>  struct memory_provider_ops {
>  	netmem_ref (*alloc_netmems)(struct page_pool *pool, gfp_t gfp);
>  	bool (*release_netmem)(struct page_pool *pool, netmem_ref netmem);
> @@ -17,11 +26,13 @@ struct memory_provider_ops {
>  	int (*nl_fill)(void *mp_priv, struct sk_buff *rsp,
>  		       struct netdev_rx_queue *rxq);
>  	void (*uninstall)(void *mp_priv, struct netdev_rx_queue *rxq);
> +	u32 caps;
>  };
>  
>  bool net_mp_niov_set_dma_addr(struct net_iov *niov, dma_addr_t addr);
>  void net_mp_niov_set_page_pool(struct page_pool *pool, struct net_iov *niov);
>  void net_mp_niov_clear_page_pool(struct net_iov *niov);
> +bool netif_mp_lacks_cap(struct net_device *dev, u32 cap);
>  
>  int netif_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
>  		      const struct pp_memory_provider_params *p,
> diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
> index 03da138722f5..a96376613dda 100644
> --- a/include/net/page_pool/types.h
> +++ b/include/net/page_pool/types.h
> @@ -22,9 +22,10 @@
>  					*/
>  #define PP_FLAG_SYSTEM_POOL	BIT(2) /* Global system page_pool */
>  
> -/* Allow unreadable (net_iov backed) netmem in this page_pool. Drivers setting
> - * this must be able to support unreadable netmem, where netmem_address() would
> - * return NULL. This flag should not be set for header page_pools.
> +/* Allow memory-provider (net_iov backed) netmem in this page_pool. Drivers
> + * setting this must honor the provider's capabilities. Unless the provider has
> + * MP_CAP_READABLE, netmem_address() returns NULL, and the flag must not be set
> + * for a header page_pool.
>   *
>   * If the driver sets PP_FLAG_ALLOW_UNREADABLE_NETMEM, it should also set
>   * page_pool_params.slow.queue_idx.
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5ac08da8b9d7..12b532c09a9c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10423,7 +10423,7 @@ int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf)
>  		return -EBUSY;
>  	}
>  
> -	if (dev_get_min_mp_channel_count(dev)) {
> +	if (netif_mp_lacks_cap(dev, MP_CAP_READABLE)) {
>  		NL_SET_ERR_MSG(bpf->extack, "unable to propagate XDP to device using memory provider");
>  		return -EBUSY;
>  	}
> @@ -10499,7 +10499,7 @@ static int dev_xdp_install(struct net_device *dev, enum bpf_xdp_mode mode,
>  		return -EBUSY;
>  	}
>  
> -	if (dev_get_min_mp_channel_count(dev)) {
> +	if (netif_mp_lacks_cap(dev, MP_CAP_READABLE)) {
>  		NL_SET_ERR_MSG(extack, "unable to install XDP to device using memory provider");
>  		return -EBUSY;
>  	}
> diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
> index 00a7011eb4d5..610e31d1a717 100644
> --- a/net/core/netdev_rx_queue.c
> +++ b/net/core/netdev_rx_queue.c
> @@ -82,8 +82,12 @@ __netif_get_rx_queue_lease(struct net_device **dev, unsigned int *rxq_idx,
>  /* See also page_pool_is_unreadable() */
>  bool netif_rxq_has_unreadable_mp(struct net_device *dev, unsigned int rxq_idx)
>  {
> -	if (rxq_idx < dev->real_num_rx_queues)
> -		return __netif_get_rx_queue(dev, rxq_idx)->mp_params.mp_ops;
> +	const struct memory_provider_ops *ops;
> +
> +	if (rxq_idx < dev->real_num_rx_queues) {
> +		ops = __netif_get_rx_queue(dev, rxq_idx)->mp_params.mp_ops;
> +		return ops && !(ops->caps & MP_CAP_READABLE);
> +	}
>  	return false;
>  }
>  EXPORT_SYMBOL(netif_rxq_has_unreadable_mp);
> @@ -95,6 +99,32 @@ bool netif_rxq_has_mp(struct net_device *dev, unsigned int rxq_idx)
>  	return false;
>  }
>  
> +/* Return true if an installed memory provider lacks @cap. */
> +bool netif_mp_lacks_cap(struct net_device *dev, u32 cap)
> +{
> +	const struct memory_provider_ops *ops;
> +	int i;
> +
> +	netdev_assert_locked_ops_compat(dev);
> +
> +	for (i = dev->real_num_rx_queues - 1; i >= 0; i--) {
> +		ops = dev->_rx[i].mp_params.mp_ops;
> +		if (ops && (ops->caps & cap) != cap)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +/* Provider memory may become available while the queue is replaced,
> + * without a device event to report it.
> + */
> +static void netdev_rx_queue_kick(struct netdev_rx_queue *rxq)
> +{
> +	if (rxq->mp_params.mp_ops && rxq->napi)
> +		napi_schedule(rxq->napi);
> +}
> +
>  static int netdev_rx_queue_reconfig(struct net_device *dev,
>  				    unsigned int rxq_idx,
>  				    struct netdev_queue_config *qcfg_old,

[..]

> @@ -142,6 +172,8 @@ static int netdev_rx_queue_reconfig(struct net_device *dev,
>  	}
>  
>  	qops->ndo_queue_mem_free(dev, old_mem);
> +	if (netif_running(dev))
> +		netdev_rx_queue_kick(rxq);
>  
>  	kvfree(old_mem);
>  	kvfree(new_mem);
> @@ -165,6 +197,11 @@ static int netdev_rx_queue_reconfig(struct net_device *dev,
>  
>  err_free_new_queue_mem:
>  	qops->ndo_queue_mem_free(dev, new_mem);
> +	/* Freeing the replacement can hand provider memory back to the old
> +	 * queue, which still runs unless the rollback failed.
> +	 */
> +	if (err != -ENETDOWN && netif_running(dev))
> +		netdev_rx_queue_kick(rxq);
>  
>  err_free_old_mem:
>  	kvfree(old_mem);

These are separate fixes?