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

On 3/9/2021 10:19 AM, Ciara Loftus wrote:

quoted

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 <redacted>

<...>

quoted

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

quoted

+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");

quoted

+		goto err_prefer;
+	}
+
+	sock_opt = ETH_AF_XDP_DFLT_BUSY_TIMEOUT;
+	ret = setsockopt(fd, SOL_SOCKET, SO_BUSY_POLL, (void

*)&sock_opt,

quoted

+			sizeof(sock_opt));
+	if (ret < 0) {
+		AF_XDP_LOG(DEBUG, "Failed to set SO_BUSY_POLL\n");

[1]

quoted

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

How about introducing a check when parsing the argument at init and failing then, instead of here?
In that case if we fail here it should not be due to invalid value. It would be due to insufficient permissions.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/core/sock.c?h=v5.11#n1150
In that case I think issuing a log, rolling back and continuing with setup would be best, instead of returning an error and aborting and forcing the user to explicitly disable busy polling via busy_budget=0 in order to get the PMD to initialize.

quoted

+	} 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,

quoted

+			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");

quoted

+		return -1;
+	}
+
+err_prefer:
+	rxq->busy_budget = 0;
+	return 0;
+}
+

<...>