DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Loftus, Ciara" <ciara.loftus@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 3/3] net/af_xdp: preferred busy polling
Date: Thu, 4 Mar 2021 12:26:32 +0000	[thread overview]
Message-ID: <201ce585e5cd4214aa73e36ec09317aa@intel.com> (raw)
In-Reply-To: <887a3d36-8197-61af-5b34-46f71921bc16@intel.com>

> 
> On 2/24/2021 11:18 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>
> > ---
> >   doc/guides/nics/af_xdp.rst          | 38 ++++++++++++-
> >   drivers/net/af_xdp/compat.h         | 13 +++++
> >   drivers/net/af_xdp/rte_eth_af_xdp.c | 85
> ++++++++++++++++++++++++-----
> >   3 files changed, 121 insertions(+), 15 deletions(-)
> 
> Can you please update the release notes too to announce the feature?

Will do.

> 
> <...>
> 
> 
> > @@ -39,3 +39,16 @@ create_shared_socket(struct xsk_socket **xsk_ptr
> __rte_unused,
> >   	return -1;
> >   }
> >   #endif
> > +
> > +#ifdef XDP_USE_NEED_WAKEUP
> > +static int
> > +syscall_needed(struct xsk_ring_prod *q, uint32_t busy_budget)
> > +{
> > +	return xsk_ring_prod__needs_wakeup(q) | busy_budget;
> > +}
> > +#else
> > +syscall_needed(struct xsk_ring_prod *q __rte_unused, uint32_t
> busy_budget)
> > +{
> > +	return busy_budget;
> > +}
> 
> Is the return type missing in the definition?

Yes. Thanks for spotting this.

> 
> Also for the case when both 'XDP_USE_NEED_WAKEUP' &
> 'SO_PREFER_BUSY_POLL' this
> function will always return '0', but current implementation doesn't know this
> in
> the compile time and compiler can't optimize for it, do you think does it make
> sense to do this optimization?

It makes sense assuming the compile environment and run environment are the same.
However you make a valid point below. If the environments are different, we can't make this optimization because we can’t rely on the presence of the flags alone to tell us if these features are supported. More below.

> 
> <...>
> 
> > @@ -1628,8 +1670,22 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device
> *dev)
> >   		return -EINVAL;
> >   	}
> >
> > +#ifdef SO_PREFER_BUSY_POLL
> > +	busy_budget = busy_budget == -1 ?
> ETH_AF_XDP_DFLT_BUSY_BUDGET :
> > +					busy_budget;
> > +	if (!busy_budget)
> > +		AF_XDP_LOG(ERR, "Preferred busy polling disabled\n");
> 
> Is this an error case? What do you think changing the log level to DEBUG or
> INFO?

+1 for INFO

> 
> Also how these compile time flags will work if the compiled environment and
> run
> environment kernel version are different and incompatible?

This is a valid point. Right now if XDP_USE_NEED_WAKEUP is defined we assume the functionality is available in the kernel. If it's not, socket creation will fail and we abort. Perhaps we should retry socket creation without the flag if we get this failure. And record if support is available in a runtime variable. I'll look at adding this as another patch to the v2 series.

> 
> Overall can it be possible to detect the support on runtime via 'setsockopt()'
> without compile time macros and eliminate the compile time flags? Does it
> make
> sense?

I think this can be done. It should allow applications compiled on older kernels without SO_PREFER_BUSY_POLL run on newer kernels with the feature.
I will tackle this in the v2.

Thanks for your feedback!

Ciara


  reply	other threads:[~2021-03-04 12:26 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 [this message]
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
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=201ce585e5cd4214aa73e36ec09317aa@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).