On 06/12/2024 15:58, Maxime Coquelin wrote:
> This patch uses the new rte_vhost_driver_set_max_queue_num
> API to set the maximum number of queue pairs supported by
> the Vhost-user port.
>
> This is required for VDUSE which needs to specify the
> maximum number of queue pairs at creation time. Without it
> 128 queue pairs metadata would be allocated.
>
> To configure it, a new 'vhost-max-queue-pairs' option is
> introduced.
>
> Signed-off-by: Maxime Coquelin <[email protected]>
> Acked-by: Eelco Chaudron <[email protected]>
> ---
>
> Note: depends on DPDK v24.11.
>
> Changes in v4:
> ==============
> - Change to vhost-queue-pairs-max to vhost-max-queue-pairs (Eelco)
> - Remove mention to DPDK version in documentation (Eelco)
> - Add missing parameter description in vswitch.xml (Eelco)
> - Define min and max values for the new option (Maxime)
> Changes in v3:
> ==============
> - Introduce a new option to set the number of max queue pairs (Kevin)
> - Add documentation for new option
>
> Changes in v2:
> ==============
> - Address checkpatch warnings.
> ---
Hi Maxime,
Testing with vhost-user backend with DPDK 24.11 worked well - in that it
called the API with the right number and didn't break anything :-)
A few comments on the code/docs below.
fyi - I initially tested on OVS main branch with 23.11 and I saw a loop
between the destroy_connection() callback triggering
netdev_dpdk_vhost_client_reconfigure(), repeat, etc when i started a VM
with 4 queues. So OVS having DPDK 24.11 support will need to be a
dependency I think (even aside from the experimental API issue).
thanks,
Kevin.
> Documentation/topics/dpdk/vhost-user.rst | 15 ++++++++++++
> lib/netdev-dpdk.c | 30 ++++++++++++++++++++++++
> vswitchd/vswitch.xml | 10 ++++++++
> 3 files changed, 55 insertions(+)
>
> diff --git a/Documentation/topics/dpdk/vhost-user.rst
> b/Documentation/topics/dpdk/vhost-user.rst
> index 7bba08ac2..656f7f69f 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -375,6 +375,21 @@ Tx retries max can be set for vhost-user-client ports::
>
> Configurable vhost tx retries are not supported with vhost-user ports.
>
> +vhost-user-client max queue pairs config
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +For vhost-user-client interfaces using the VDUSE backend, the maximum umber
> of
typo number
> +queue pairs the Virtio device will support can be set at port creation time.
> If
> +not set, the default value is 1 queue pair. This value is ignored for
> +Vhost-user backends.
> +
> +Maximum number of queue pairs can be set for vhost-user-client-ports::
> +
> + $ ovs-vsctl add-port br0 vduse0 \
> + -- set Interface vduse0 type=dpdkvhostuserclient \
> + options:vhost-server-path=/dev/vduse/vduse0 \
> + options:vhost-max-queue-pairs=4
> +
> .. _dpdk-testpmd:
>
> DPDK in the Guest
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index dc52a2b56..a8b605113 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -19,6 +19,7 @@
>
> #include <errno.h>
> #include <signal.h>
> +#include <stdint.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> @@ -153,6 +154,11 @@ typedef uint16_t dpdk_port_t;
> /* Legacy default value for vhost tx retries. */
> #define VHOST_ENQ_RETRY_DEF 8
>
> +/* VDUSE-only, ignore for Vhost-user */
> +#define VHOST_MAX_QUEUE_PAIRS_MIN 1
> +#define VHOST_MAX_QUEUE_PAIRS_DEF VHOST_MAX_QUEUE_PAIRS_MIN
> +#define VHOST_MAX_QUEUE_PAIRS_MAX 128
> +
> #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
>
> /* List of required flags advertised by the hardware that will be used
> @@ -497,6 +503,9 @@ struct netdev_dpdk {
>
> atomic_uint8_t vhost_tx_retries_max;
>
> + /* Ignored by DPDK for Vhost-user backends, only for VDUSE */
> + uint32_t vhost_max_queue_pairs;
> +
I noticed this added a cacheline - perhaps we could use something
smaller and squash it in ?
/* --- cacheline 1 boundary (64 bytes) --- */
union {
OVS_CACHE_LINE_MARKER cacheline1; /* 64 1 */
struct {
struct ovs_mutex mutex; /* 64 48 */
struct dpdk_mp * dpdk_mp; /* 112 8 */
ovsrcu_index vid; /* 120 4 */
_Bool vhost_reconfigured; /* 124 1 */
atomic_uint8_t vhost_tx_retries_max; /* 125 1 */
/* XXX 2 bytes hole, try to pack */
/* --- cacheline 2 boundary (128 bytes) --- */
uint32_t vhost_max_queue_pairs; /* 128 4 */
uint8_t virtio_features_state; /* 132 1 */
}; /* 64 72 */
uint8_t pad55[128]; /* 64 128 */
}; /* 64 128 */
/* --- cacheline 3 boundary (192 bytes) --- */
> /* Flags for virtio features recovery mechanism. */
> uint8_t virtio_features_state;
>
> @@ -1609,6 +1618,8 @@ vhost_common_construct(struct netdev *netdev)
>
> atomic_init(&dev->vhost_tx_retries_max, VHOST_ENQ_RETRY_DEF);
>
> + dev->vhost_max_queue_pairs = VHOST_MAX_QUEUE_PAIRS_DEF;
> +
> return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID,
> DPDK_DEV_VHOST, socket_id);
> }
> @@ -2491,6 +2502,7 @@ netdev_dpdk_vhost_client_set_config(struct netdev
> *netdev,
> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> const char *path;
> int max_tx_retries, cur_max_tx_retries;
> + uint32_t max_queue_pairs;
>
> ovs_mutex_lock(&dev->mutex);
> if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) {
> @@ -2498,6 +2510,15 @@ netdev_dpdk_vhost_client_set_config(struct netdev
> *netdev,
> if (!nullable_string_is_equal(path, dev->vhost_id)) {
> free(dev->vhost_id);
> dev->vhost_id = nullable_xstrdup(path);
> +
> + max_queue_pairs = smap_get_int(args, "vhost-max-queue-pairs",
> + VHOST_MAX_QUEUE_PAIRS_DEF);
> + if (max_queue_pairs < VHOST_MAX_QUEUE_PAIRS_MIN
> + || max_queue_pairs > VHOST_MAX_QUEUE_PAIRS_MAX) {
> + max_queue_pairs = VHOST_MAX_QUEUE_PAIRS_DEF;
> + }
> + dev->vhost_max_queue_pairs = max_queue_pairs;
> +
> netdev_request_reconfigure(netdev);
> }
> }
> @@ -2514,6 +2535,7 @@ netdev_dpdk_vhost_client_set_config(struct netdev
> *netdev,
> VLOG_INFO("Max Tx retries for vhost device '%s' set to %d",
> netdev_get_name(netdev), max_tx_retries);
> }
> +
> ovs_mutex_unlock(&dev->mutex);
>
> return 0;
> @@ -6400,6 +6422,14 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev
> *netdev)
> goto unlock;
> }
>
> + err = rte_vhost_driver_set_max_queue_num(dev->vhost_id,
> + dev->vhost_max_queue_pairs);
The log below is printed:
2024-12-06T18:28:51Z|00100|dpdk|INFO|VHOST_CONFIG: (/tmp/vhost0) Setting
max queue pairs to 1
It's kind of true, but it could be confusing when using vhost-user
backend. Maybe we should add an OVS info log before or after as a
reminder that the max queue pairs setting is only valid for VDUSE backend.
> + if (err) {
> + VLOG_ERR("rte_vhost_driver_set_max_queue_num failed for "
> + "vhost-user client port: %s\n", dev->up.name);
> + goto unlock;
> + }
> +
> err = rte_vhost_driver_start(dev->vhost_id);
> if (err) {
> VLOG_ERR("rte_vhost_driver_start failed for vhost user "
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 36cb4e495..0b5c5dcd6 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -3520,6 +3520,16 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> type=patch options:peer=p1 \
> </p>
> </column>
>
> + <column name="options" key="vhost-max-queue-pairs"
> + type='{"type": "integer", "minInteger" : 1, "maxInteger":
> 128}'>
> + <p>
> + The value specifies the maximum number of queue pairs supported by
> + a vHost device. This is ignored for Vhost-user backends, only VDUSE
> + is supported.
> + Only supported by dpdkvhsotuserclient interfaces.
typo dpdkvhostuserclient
> + </p>
It would be good to state the default here (like tx-retries-max below)
> + </column>
> +
> <column name="options" key="tx-retries-max"
> type='{"type": "integer", "minInteger": 0, "maxInteger": 32}'>
> <p>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
