| Message ID | 20241204092932.58809-1-maxime.coquelin@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Kevin Traynor |
| Headers | show |
| Series | [ovs-dev,v3] netdev-dpdk: Set Vhost port maximum number of queue pairs. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On 4 Dec 2024, at 10:29, 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-queue-pairs-max' option is > introduced. > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > Acked-by: Eelco Chaudron <echaudro@redhat.com> > --- I only code-reviewed this patch (no actual testing) and I have some comments below. > 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. > --- > Documentation/topics/dpdk/vhost-user.rst | 15 +++++++++++++ > lib/netdev-dpdk.c | 27 ++++++++++++++++++++++++ > 2 files changed, 42 insertions(+) > > diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst > index 7bba08ac2..5c3f1046f 100644 > --- a/Documentation/topics/dpdk/vhost-user.rst > +++ b/Documentation/topics/dpdk/vhost-user.rst We also need to add the definition to the vswitch.xml file. > @@ -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 > +queue pairs the Virtio device will support can be set at port creation time. If > +not set, the default value is 1 queue pair. As of DPDK v24.11, this value is > +ignored for Vhost-user backends. I think we should not reference a DPDK version here. We should just state that this value is ignored for non-VDUSE 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-queue-pairs-max=4 The order in the naming looks odd, what about vhost-max-queue-pairs? If you agree also all defines and code variables should change. > + > .. _dpdk-testpmd: > > DPDK in the Guest > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index dc52a2b56..b2d14b678 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,9 @@ 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_QUEUE_PAIRS_MAX_DEF 1 > + > #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) > > /* List of required flags advertised by the hardware that will be used > @@ -497,6 +501,9 @@ struct netdev_dpdk { > > atomic_uint8_t vhost_tx_retries_max; > > + /* Ignored by DPDK for Vhost-user backends, only for VDUSE */ > + uint32_t vhost_queue_pairs_max; > + > /* Flags for virtio features recovery mechanism. */ > uint8_t virtio_features_state; > > @@ -1609,6 +1616,8 @@ vhost_common_construct(struct netdev *netdev) > > atomic_init(&dev->vhost_tx_retries_max, VHOST_ENQ_RETRY_DEF); > > + dev->vhost_queue_pairs_max = VHOST_QUEUE_PAIRS_MAX_DEF; > + > return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID, > DPDK_DEV_VHOST, socket_id); > } > @@ -2491,6 +2500,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 queue_pairs_max; > > ovs_mutex_lock(&dev->mutex); > if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { > @@ -2498,6 +2508,14 @@ 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); > + > + queue_pairs_max = smap_get_int(args, "vhost-queue-pairs-max", > + VHOST_QUEUE_PAIRS_MAX_DEF); > + if (queue_pairs_max < 1) { > + queue_pairs_max = VHOST_QUEUE_PAIRS_MAX_DEF; > + } > + dev->vhost_queue_pairs_max = queue_pairs_max; > + > netdev_request_reconfigure(netdev); > } > } > @@ -2514,6 +2532,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 +6419,14 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev *netdev) > goto unlock; > } > > + err = rte_vhost_driver_set_max_queue_num(dev->vhost_id, > + dev->vhost_queue_pairs_max); > + 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 " > -- > 2.47.0
On 12/5/24 10:43, Eelco Chaudron wrote: > > > On 4 Dec 2024, at 10:29, 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-queue-pairs-max' option is >> introduced. >> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> Acked-by: Eelco Chaudron <echaudro@redhat.com> >> --- > > I only code-reviewed this patch (no actual testing) and I have some comments below. > >> 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. >> --- >> Documentation/topics/dpdk/vhost-user.rst | 15 +++++++++++++ >> lib/netdev-dpdk.c | 27 ++++++++++++++++++++++++ >> 2 files changed, 42 insertions(+) >> >> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst >> index 7bba08ac2..5c3f1046f 100644 >> --- a/Documentation/topics/dpdk/vhost-user.rst >> +++ b/Documentation/topics/dpdk/vhost-user.rst > > We also need to add the definition to the vswitch.xml file. Ok, will do in next revision. >> @@ -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 >> +queue pairs the Virtio device will support can be set at port creation time. If >> +not set, the default value is 1 queue pair. As of DPDK v24.11, this value is >> +ignored for Vhost-user backends. > > I think we should not reference a DPDK version here. We should just state that this value is ignored for non-VDUSE backends. Ack. >> + >> +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-queue-pairs-max=4 > > The order in the naming looks odd, what about vhost-max-queue-pairs? If you agree also all defines and code variables should change. Sure, I can change. I used this order to be consistent with "tx-retries-max", but I am fine with changing it. Thanks for the review, Maxime
diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst index 7bba08ac2..5c3f1046f 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 +queue pairs the Virtio device will support can be set at port creation time. If +not set, the default value is 1 queue pair. As of DPDK v24.11, 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-queue-pairs-max=4 + .. _dpdk-testpmd: DPDK in the Guest diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index dc52a2b56..b2d14b678 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,9 @@ 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_QUEUE_PAIRS_MAX_DEF 1 + #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) /* List of required flags advertised by the hardware that will be used @@ -497,6 +501,9 @@ struct netdev_dpdk { atomic_uint8_t vhost_tx_retries_max; + /* Ignored by DPDK for Vhost-user backends, only for VDUSE */ + uint32_t vhost_queue_pairs_max; + /* Flags for virtio features recovery mechanism. */ uint8_t virtio_features_state; @@ -1609,6 +1616,8 @@ vhost_common_construct(struct netdev *netdev) atomic_init(&dev->vhost_tx_retries_max, VHOST_ENQ_RETRY_DEF); + dev->vhost_queue_pairs_max = VHOST_QUEUE_PAIRS_MAX_DEF; + return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID, DPDK_DEV_VHOST, socket_id); } @@ -2491,6 +2500,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 queue_pairs_max; ovs_mutex_lock(&dev->mutex); if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { @@ -2498,6 +2508,14 @@ 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); + + queue_pairs_max = smap_get_int(args, "vhost-queue-pairs-max", + VHOST_QUEUE_PAIRS_MAX_DEF); + if (queue_pairs_max < 1) { + queue_pairs_max = VHOST_QUEUE_PAIRS_MAX_DEF; + } + dev->vhost_queue_pairs_max = queue_pairs_max; + netdev_request_reconfigure(netdev); } } @@ -2514,6 +2532,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 +6419,14 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev *netdev) goto unlock; } + err = rte_vhost_driver_set_max_queue_num(dev->vhost_id, + dev->vhost_queue_pairs_max); + 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 "