DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Loftus, Ciara" <ciara.loftus@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 3/3] net/af_xdp: preferred busy polling
Date: Wed, 10 Mar 2021 07:55:11 +0000	[thread overview]
Message-ID: <7b1443509db749fa8f757a2b82c0ccff@intel.com> (raw)
In-Reply-To: <c9eec25f-b6ef-a441-38f9-6c88ed7e854e@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.

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.

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

  reply	other threads:[~2021-03-10  7:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-18  9:23 [dpdk-dev] [PATCH RFC 0/3] AF_XDP Preferred Busy Polling Ciara Loftus
2021-02-18  9:23 ` [dpdk-dev] [PATCH RFC 1/3] net/af_xdp: Increase max batch size to 512 Ciara Loftus
2021-02-18  9:23 ` [dpdk-dev] [PATCH RFC 2/3] net/af_xdp: Use recvfrom() instead of poll() Ciara Loftus
2021-02-18  9:23 ` [dpdk-dev] [PATCH RFC 3/3] net/af_xdp: preferred busy polling Ciara Loftus
2021-02-24 11:18 ` [dpdk-dev] [PATCH 0/3] AF_XDP Preferred Busy Polling Ciara Loftus
2021-02-24 11:18   ` [dpdk-dev] [PATCH 1/3] net/af_xdp: Increase max batch size to 512 Ciara Loftus
2021-03-01 16:31     ` Ferruh Yigit
2021-03-03 15:07       ` Loftus, Ciara
2021-03-03 15:38         ` Ferruh Yigit
2021-02-24 11:18   ` [dpdk-dev] [PATCH 2/3] net/af_xdp: Use recvfrom() instead of poll() Ciara Loftus
2021-02-24 11:18   ` [dpdk-dev] [PATCH 3/3] net/af_xdp: preferred busy polling Ciara Loftus
2021-03-01 16:32     ` Ferruh Yigit
2021-03-04 12:26       ` Loftus, Ciara
2021-03-08 15:54         ` Loftus, Ciara
2021-03-09 10:19   ` [dpdk-dev] [PATCH v2 0/3] AF_XDP Preferred Busy Polling Ciara Loftus
2021-03-09 10:19     ` [dpdk-dev] [PATCH v2 1/3] net/af_xdp: allow bigger batch sizes Ciara Loftus
2021-03-09 16:33       ` Ferruh Yigit
2021-03-10  7:21         ` Loftus, Ciara
2021-03-09 10:19     ` [dpdk-dev] [PATCH v2 2/3] net/af_xdp: Use recvfrom() instead of poll() Ciara Loftus
2021-03-09 10:19     ` [dpdk-dev] [PATCH v2 3/3] net/af_xdp: preferred busy polling Ciara Loftus
2021-03-09 16:33       ` Ferruh Yigit
2021-03-10  7:55         ` Loftus, Ciara [this message]
2021-03-10  7:48     ` [dpdk-dev] [PATCH v3 0/3] AF_XDP Preferred Busy Polling Ciara Loftus
2021-03-10  7:48       ` [dpdk-dev] [PATCH v3 1/3] net/af_xdp: allow bigger batch sizes Ciara Loftus
2021-03-10  7:48       ` [dpdk-dev] [PATCH v3 2/3] net/af_xdp: Use recvfrom() instead of poll() Ciara Loftus
2021-03-10  7:48       ` [dpdk-dev] [PATCH v3 3/3] net/af_xdp: preferred busy polling Ciara Loftus
2021-03-10 17:50       ` [dpdk-dev] [PATCH v3 0/3] AF_XDP Preferred Busy Polling Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7b1443509db749fa8f757a2b82c0ccff@intel.com \
    --to=ciara.loftus@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).