Re: [dpdk-dev] [PATCH v2 3/3] net/af_xdp: preferred busy polling

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 3/9/2021 10:19 AM, Ciara Loftus wrote:
> This commit introduces support for preferred busy polling
> to the AF_XDP PMD. This feature aims to improve single-core
> performance for AF_XDP sockets under heavy load.
> 
> A new vdev arg is introduced called 'busy_budget' whose default
> value is 64. busy_budget is the value supplied to the kernel
> with the SO_BUSY_POLL_BUDGET socket option and represents the
> busy-polling NAPI budget. To set the budget to a different value
> eg. 256:
> 
> --vdev=net_af_xdp0,iface=eth0,busy_budget=256
> 
> Preferred busy polling is enabled by default provided a kernel with
> version >= v5.11 is in use. To disable it, set the budget to zero.
> 
> The following settings are also strongly recommended to be used in
> conjunction with this feature:
> 
> echo 2 | sudo tee /sys/class/net/eth0/napi_defer_hard_irqs
> echo 200000 | sudo tee /sys/class/net/eth0/gro_flush_timeout
> 
> .. where eth0 is the interface being used by the PMD.
> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>

<...>

> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> @@ -70,6 +70,10 @@ New Features
>     * Added command to display Rx queue used descriptor count.
>       ``show port (port_id) rxq (queue_id) desc used count``
>   
> +* **Updated the AF_XDP driver.**
> +
> +  * Added support for preferred busy polling.
> +
>   

Can you please move the update after above the testpmd updates?
For more details the expected order is in the section comment.

> +static int
> +configure_preferred_busy_poll(struct pkt_rx_queue *rxq)
> +{
> +	int sock_opt = 1;
> +	int fd = xsk_socket__fd(rxq->xsk);
> +	int ret = 0;
> +
> +	ret = setsockopt(fd, SOL_SOCKET, SO_PREFER_BUSY_POLL,
> +			(void *)&sock_opt, sizeof(sock_opt));
> +	if (ret < 0) {
> +		AF_XDP_LOG(DEBUG, "Failed to set SO_PREFER_BUSY_POLL\n");
> +		goto err_prefer;
> +	}
> +
> +	sock_opt = ETH_AF_XDP_DFLT_BUSY_TIMEOUT;
> +	ret = setsockopt(fd, SOL_SOCKET, SO_BUSY_POLL, (void *)&sock_opt,
> +			sizeof(sock_opt));
> +	if (ret < 0) {
> +		AF_XDP_LOG(DEBUG, "Failed to set SO_BUSY_POLL\n");

[1]

> +		goto err_timeout;
> +	}
> +
> +	sock_opt = rxq->busy_budget;
> +	ret = setsockopt(fd, SOL_SOCKET, SO_BUSY_POLL_BUDGET,
> +			(void *)&sock_opt, sizeof(sock_opt));
> +	if (ret < 0) {
> +		AF_XDP_LOG(DEBUG, "Failed to set SO_BUSY_POLL_BUDGET\n");

In above [1] and here, shouldn't the function return error, even the rollback is 
successful.
I am thinking a case an invalid 'busy_budget' provided, like a very big number 
or negative value.

> +	} else {
> +		AF_XDP_LOG(INFO, "Busy polling budget set to: %u\n",
> +					rxq->busy_budget);
> +		return 0;
> +	}
> +
> +	/* setsockopt failure - attempt to restore xsk to default state and
> +	 * proceed without busy polling support.
> +	 */
> +	sock_opt = 0;
> +	ret = setsockopt(fd, SOL_SOCKET, SO_BUSY_POLL, (void *)&sock_opt,
> +			sizeof(sock_opt));
> +	if (ret < 0) {
> +		AF_XDP_LOG(ERR, "Failed to unset SO_BUSY_POLL\n");
> +		return -1;
> +	}
> +
> +err_timeout:
> +	sock_opt = 0;
> +	ret = setsockopt(fd, SOL_SOCKET, SO_PREFER_BUSY_POLL,
> +			(void *)&sock_opt, sizeof(sock_opt));
> +	if (ret < 0) {
> +		AF_XDP_LOG(ERR, "Failed to unset SO_PREFER_BUSY_POLL\n");
> +		return -1;
> +	}
> +
> +err_prefer:
> +	rxq->busy_budget = 0;
> +	return 0;
> +}
> +

<...>