DPDK patches and discussions
 help / color / mirror / Atom feed
From: Matan Azrad <matan@mellanox.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
	Xiao Wang <xiao.w.wang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v1 1/4] vhost: support host notifier queue configuration
Date: Fri, 19 Jun 2020 13:28:59 +0000
Message-ID: <AM0PR0502MB40194ED9CE8ECC829993C692D2980@AM0PR0502MB4019.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <b97d38a9-3264-41d0-4ccf-ce3d4cb2620a@redhat.com>



From: Maxime Coquelin:
> On 6/18/20 6:28 PM, Matan Azrad wrote:
> > As an arrangement to per queue operations in the vDPA device it is
> > needed to change the next experimental API:
> >
> > The API ``rte_vhost_host_notifier_ctrl`` was changed to be per queue
> > instead of per device.
> >
> > A `qid` parameter was added to the API arguments list.
> >
> > Setting the parameter to the value VHOST_QUEUE_ALL will configure the
> > host notifier to all the device queues as done before this patch.
> >
> > Signed-off-by: Matan Azrad <matan@mellanox.com>
> > ---
> >  doc/guides/rel_notes/release_20_08.rst |  2 ++
> >  drivers/vdpa/ifc/ifcvf_vdpa.c          |  6 +++---
> >  drivers/vdpa/mlx5/mlx5_vdpa.c          |  5 +++--
> >  lib/librte_vhost/rte_vdpa.h            |  8 ++++++--
> >  lib/librte_vhost/rte_vhost.h           |  2 ++
> >  lib/librte_vhost/vhost.h               |  3 ---
> >  lib/librte_vhost/vhost_user.c          | 18 ++++++++++++++----
> >  7 files changed, 30 insertions(+), 14 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/release_20_08.rst
> > b/doc/guides/rel_notes/release_20_08.rst
> > index ba16d3b..9732959 100644
> > --- a/doc/guides/rel_notes/release_20_08.rst
> > +++ b/doc/guides/rel_notes/release_20_08.rst
> > @@ -111,6 +111,8 @@ API Changes
> >     Also, make sure to start the actual text at the margin.
> >
> =========================================================
> >
> > +* vhost: The API of ``rte_vhost_host_notifier_ctrl`` was changed to
> > +be per
> > +  queue and not per device, a qid parameter was added to the arguments
> list.
> >
> >  ABI Changes
> >  -----------
> > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > b/drivers/vdpa/ifc/ifcvf_vdpa.c index ec97178..336837a 100644
> > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> > @@ -839,7 +839,7 @@ struct internal_list {
> >  	vdpa_ifcvf_stop(internal);
> >  	vdpa_disable_vfio_intr(internal);
> >
> > -	ret = rte_vhost_host_notifier_ctrl(vid, false);
> > +	ret = rte_vhost_host_notifier_ctrl(vid, VHOST_QUEUE_ALL, false);
> >  	if (ret && ret != -ENOTSUP)
> >  		goto error;
> >
> > @@ -858,7 +858,7 @@ struct internal_list {
> >  	if (ret)
> >  		goto stop_vf;
> >
> > -	rte_vhost_host_notifier_ctrl(vid, true);
> > +	rte_vhost_host_notifier_ctrl(vid, VHOST_QUEUE_ALL, true);
> >
> >  	internal->sw_fallback_running = true;
> >
> > @@ -893,7 +893,7 @@ struct internal_list {
> >  	rte_atomic32_set(&internal->dev_attached, 1);
> >  	update_datapath(internal);
> >
> > -	if (rte_vhost_host_notifier_ctrl(vid, true) != 0)
> > +	if (rte_vhost_host_notifier_ctrl(vid, VHOST_QUEUE_ALL, true) != 0)
> >  		DRV_LOG(NOTICE, "vDPA (%d): software relay is used.", did);
> >
> >  	return 0;
> > diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c
> > b/drivers/vdpa/mlx5/mlx5_vdpa.c index 9e758b6..8ea1300 100644
> > --- a/drivers/vdpa/mlx5/mlx5_vdpa.c
> > +++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
> > @@ -147,7 +147,8 @@
> >  	int ret;
> >
> >  	if (priv->direct_notifier) {
> > -		ret = rte_vhost_host_notifier_ctrl(priv->vid, false);
> > +		ret = rte_vhost_host_notifier_ctrl(priv->vid,
> VHOST_QUEUE_ALL,
> > +						   false);
> >  		if (ret != 0) {
> >  			DRV_LOG(INFO, "Direct HW notifier FD cannot be "
> >  				"destroyed for device %d: %d.", priv->vid,
> ret); @@ -155,7 +156,7
> > @@
> >  		}
> >  		priv->direct_notifier = 0;
> >  	}
> > -	ret = rte_vhost_host_notifier_ctrl(priv->vid, true);
> > +	ret = rte_vhost_host_notifier_ctrl(priv->vid, VHOST_QUEUE_ALL,
> > +true);
> >  	if (ret != 0)
> >  		DRV_LOG(INFO, "Direct HW notifier FD cannot be configured
> for"
> >  			" device %d: %d.", priv->vid, ret); diff --git
> > a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h index
> > ecb3d91..2db536c 100644
> > --- a/lib/librte_vhost/rte_vdpa.h
> > +++ b/lib/librte_vhost/rte_vdpa.h
> > @@ -202,22 +202,26 @@ struct rte_vdpa_device *  int
> > rte_vdpa_get_device_num(void);
> >
> > +#define VHOST_QUEUE_ALL VHOST_MAX_VRING
> > +
> >  /**
> >   * @warning
> >   * @b EXPERIMENTAL: this API may change without prior notice
> >   *
> > - * Enable/Disable host notifier mapping for a vdpa port.
> > + * Enable/Disable host notifier mapping for a vdpa queue.
> >   *
> >   * @param vid
> >   *  vhost device id
> >   * @param enable
> >   *  true for host notifier map, false for host notifier unmap
> > + * @param qid
> > + *  vhost queue id, VHOST_QUEUE_ALL to configure all the device
> > + queues
> I would prefer two APIs that passing a special ID that means all queues:
> 
> rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable);
> rte_vhost_host_notifier_ctrl_all(int vid, bool enable);
> 
> I think it is clearer for the user of the API.
> Or if you think an extra API is overkill, just let the driver loop on all the
> queues.

We have a lot of options here with pros and cons.
I took the rte_eth_dev_callback_register style.

It is less intrusive with minimum code change.  

I'm not sure what is the clearest option but the current suggestion is well defined and 
allows to configure all the queues too.

Let me know what you prefer....

> >   * @return
> >   *  0 on success, -1 on failure
> >   */
> >  __rte_experimental
> >  int
> > -rte_vhost_host_notifier_ctrl(int vid, bool enable);
> > +rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable);
> >
> >  /**


  reply	other threads:[~2020-06-19 13:29 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-18 16:28 [dpdk-dev] [PATCH v1 0/4] vhost: improve ready state Matan Azrad
2020-06-18 16:28 ` [dpdk-dev] [PATCH v1 1/4] vhost: support host notifier queue configuration Matan Azrad
2020-06-19  6:44   ` Maxime Coquelin
2020-06-19 13:28     ` Matan Azrad [this message]
2020-06-19 14:01       ` Maxime Coquelin
2020-06-21  6:26         ` Matan Azrad
2020-06-22  8:06           ` Maxime Coquelin
2020-06-18 16:28 ` [dpdk-dev] [PATCH v1 2/4] vhost: skip access lock when vDPA is configured Matan Azrad
2020-06-19  6:49   ` Maxime Coquelin
2020-06-18 16:28 ` [dpdk-dev] [PATCH v1 3/4] vhost: improve device ready definition Matan Azrad
2020-06-19  7:41   ` Maxime Coquelin
2020-06-19 12:04     ` Maxime Coquelin
2020-06-19 13:11     ` Matan Azrad
2020-06-19 13:54       ` Maxime Coquelin
2020-06-21  6:20         ` Matan Azrad
2020-06-22  8:04           ` Maxime Coquelin
2020-06-22  8:41             ` Matan Azrad
2020-06-22  8:56               ` Maxime Coquelin
2020-06-22 10:06                 ` Matan Azrad
2020-06-22 12:32                   ` Maxime Coquelin
2020-06-22 13:43                     ` Matan Azrad
2020-06-22 14:55                       ` Maxime Coquelin
2020-06-22 15:51                         ` Matan Azrad
2020-06-22 16:47                           ` Maxime Coquelin
2020-06-23  9:02                             ` Matan Azrad
2020-06-23  9:19                               ` Maxime Coquelin
2020-06-23 11:53                                 ` Matan Azrad
2020-06-23 13:55                                   ` Maxime Coquelin
2020-06-23 14:33                                     ` Maxime Coquelin
2020-06-23 14:52                                     ` Matan Azrad
2020-06-23 15:18                                       ` Maxime Coquelin
2020-06-24  5:54                                         ` Matan Azrad
2020-06-24  7:22                                           ` Maxime Coquelin
2020-06-24  8:38                                             ` Matan Azrad
2020-06-24  9:12                                               ` Maxime Coquelin
2020-06-18 16:28 ` [dpdk-dev] [PATCH v1 4/4] vdpa/mlx5: support queue update Matan Azrad
2020-06-25 13:38 ` [dpdk-dev] [PATCH v2 0/5] vhost: improve ready state Matan Azrad
2020-06-25 13:38   ` [dpdk-dev] [PATCH v2 1/5] vhost: skip access lock when vDPA is configured Matan Azrad
2020-06-28  3:06     ` Xia, Chenbo
2020-06-25 13:38   ` [dpdk-dev] [PATCH v2 2/5] vhost: improve device readiness notifications Matan Azrad
2020-06-26 12:10     ` Maxime Coquelin
2020-06-28  3:08     ` Xia, Chenbo
2020-06-25 13:38   ` [dpdk-dev] [PATCH v2 3/5] vhost: handle memory hotplug with vDPA devices Matan Azrad
2020-06-26 12:15     ` Maxime Coquelin
2020-06-28  3:18     ` Xia, Chenbo
2020-06-25 13:38   ` [dpdk-dev] [PATCH v2 4/5] vhost: notify virtq file descriptor update Matan Azrad
2020-06-26 12:19     ` Maxime Coquelin
2020-06-28  3:19     ` Xia, Chenbo
2020-06-25 13:38   ` [dpdk-dev] [PATCH v2 5/5] vdpa/mlx5: support queue update Matan Azrad
2020-06-26 12:29     ` Maxime Coquelin
2020-06-29 14:08   ` [dpdk-dev] [PATCH v3 0/6] vhost: improve ready state Matan Azrad
2020-06-29 14:08     ` [dpdk-dev] [PATCH v3 1/6] vhost: support host notifier queue configuration Matan Azrad
2020-06-29 14:08     ` [dpdk-dev] [PATCH v3 2/6] vhost: skip access lock when vDPA is configured Matan Azrad
2020-06-29 14:08     ` [dpdk-dev] [PATCH v3 3/6] vhost: improve device readiness notifications Matan Azrad
2020-06-29 14:08     ` [dpdk-dev] [PATCH v3 4/6] vhost: handle memory hotplug with vDPA devices Matan Azrad
2020-06-29 14:08     ` [dpdk-dev] [PATCH v3 5/6] vhost: notify virtq file descriptor update Matan Azrad
2020-06-29 14:08     ` [dpdk-dev] [PATCH v3 6/6] vdpa/mlx5: support queue update Matan Azrad
2020-06-29 17:24     ` [dpdk-dev] [PATCH v3 0/6] vhost: improve ready state Maxime Coquelin
2020-07-17  1:41       ` Wang, Yinan

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=AM0PR0502MB40194ED9CE8ECC829993C692D2980@AM0PR0502MB4019.eurprd05.prod.outlook.com \
    --to=matan@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=maxime.coquelin@redhat.com \
    --cc=xiao.w.wang@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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git