Re: [PATCH v2] net/af_xdp: add rx/tx queue support for af_xdp

[Shivaji Kant <shivajikant@google.com>]

[-- Attachment #1: Type: text/plain, Size: 5708 bytes --]

Hi Stephen,

Thank you for your review and comments.

Regarding the first sentence of the commit message, you are absolutely
right. I've rephrased it for clarity in v2.

> Is there anything about queues in the driver documentation?

For your question about queues in the driver documentation, the GVE driver
documentation (specifically the
Documentation/networking/device_drivers/google/gve.rst  in the Linux kernel
source) describes the GVE's queue model as having separate RX and TX
queues. While it doesn't explicitly state "combined queues are not
supported," the design clearly indicates a preference for distinct RX and
TX queues, which this patch aims to accommodate for AF_XDP.

I have also corrected the spelling error from "dirvers" to "drivers" in the
code comment.

I've pushed a new version (v2) with these changes. Please let me know if
there are any further issues.

Thanks,
Shivaji

On Mon, Jun 30, 2025 at 2:20 PM Shivaji Kant <shivajikant@google.com> wrote:

> Drivers like GVE work with rx/tx queue configuration
> rather than combined queue. Enable AF_XDP vdev to use
> rx/tx queue configuration instead of combined queue
> configuration if available.
>
> Signed-off-by: Shivaji Kant <shivajikant@google.com>
> Reviewed-by: Joshua Washington <joshwash@google.com>
>
> ---
> Changes in v2:
> - Rephrased the first sentence of the commit message for clarity.
> - Corrected spelling error "dirvers" to "drivers" in code comment.
> ---
>  .mailmap                            |  1 +
>  drivers/net/af_xdp/rte_eth_af_xdp.c | 30 ++++++++++++++++++-----------
>  2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/.mailmap b/.mailmap
> index 8483d96ec5..c67caa7c9d 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -1423,6 +1423,7 @@ Shijith Thotton <sthotton@marvell.com> <
> shijith.thotton@caviumnetworks.com>
>  Shiqi Liu <835703180@qq.com>
>  Shiri Kuzin <shirik@nvidia.com> <shirik@mellanox.com>
>  Shivah Shankar S <sshankarnara@marvell.com>
> +Shivaji Kant <shivajikant@google.com>
>  Shivanshu Shukla <shivanshu.shukla@intel.com>
>  Shiweixian <shiweixian@huawei.com>
>  Shiyang He <shiyangx.he@intel.com>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 05115150a7..5f65850a27 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -168,7 +168,7 @@ struct pmd_internals {
>         int start_queue_idx;
>         int queue_cnt;
>         int max_queue_cnt;
> -       int combined_queue_cnt;
> +       int configured_queue_cnt;
>         bool shared_umem;
>         char prog_path[PATH_MAX];
>         bool custom_prog_configured;
> @@ -2043,11 +2043,11 @@ parse_prog_arg(const char *key __rte_unused,
>
>  static int
>  xdp_get_channels_info(const char *if_name, int *max_queues,
> -                               int *combined_queues)
> +                               int *configured_queues)
>  {
>         struct ethtool_channels channels;
>         struct ifreq ifr;
> -       int fd, ret;
> +       int fd, ret, rxtx_q_count;
>
>         fd = socket(AF_INET, SOCK_DGRAM, 0);
>         if (fd < 0)
> @@ -2066,15 +2066,23 @@ xdp_get_channels_info(const char *if_name, int
> *max_queues,
>                 }
>         }
>
> -       if (channels.max_combined == 0 || errno == EOPNOTSUPP) {
> +       /* For drivers with rx/tx queue configured */
> +       rxtx_q_count = RTE_MIN(channels.rx_count, channels.tx_count);
> +
> +       if ((channels.max_combined == 0 && rxtx_q_count == 0) || errno ==
> EOPNOTSUPP) {
>                 /* If the device says it has no channels, then all traffic
>                  * is sent to a single stream, so max queues = 1.
>                  */
>                 *max_queues = 1;
> -               *combined_queues = 1;
> -       } else {
> +               *configured_queues = 1;
> +       } else if (channels.max_combined > 0) {
>                 *max_queues = channels.max_combined;
> -               *combined_queues = channels.combined_count;
> +               *configured_queues = channels.combined_count;
> +               AF_XDP_LOG_LINE(INFO, "Using Combined queues
> configuration");
> +       } else {
> +               *max_queues = RTE_MIN(channels.max_rx, channels.max_tx);
> +               *configured_queues = rxtx_q_count;
> +               AF_XDP_LOG_LINE(INFO, "Using Rx/Tx queues configuration");
>         }
>
>   out:
> @@ -2215,15 +2223,15 @@ init_internals(struct rte_vdev_device *dev, const
> char *if_name,
>         strlcpy(internals->dp_path, dp_path, PATH_MAX);
>
>         if (xdp_get_channels_info(if_name, &internals->max_queue_cnt,
> -                                 &internals->combined_queue_cnt)) {
> +                                 &internals->configured_queue_cnt)) {
>                 AF_XDP_LOG_LINE(ERR, "Failed to get channel info of
> interface: %s",
>                                 if_name);
>                 goto err_free_internals;
>         }
>
> -       if (queue_cnt > internals->combined_queue_cnt) {
> -               AF_XDP_LOG_LINE(ERR, "Specified queue count %d is larger
> than combined queue count %d.",
> -                               queue_cnt, internals->combined_queue_cnt);
> +       if (queue_cnt > internals->configured_queue_cnt) {
> +               AF_XDP_LOG_LINE(ERR, "Specified queue count %d is larger
> than configured queue count %d.",
> +                               queue_cnt,
> internals->configured_queue_cnt);
>                 goto err_free_internals;
>         }
>
> --
> 2.50.0.727.gbf7dc18ff4-goog
>
>

[-- Attachment #2: Type: text/html, Size: 7665 bytes --]