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