[dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue status

[Noa Ezra]

Hi Maxime,
For me, the solution you suggested is good enough:
"call rte_eth_vhost_get_queue_event() in loop at startup until you get -1 and build the states based on that"
Getting the state on a direct way can be good and easier than reading it in a loop, but I'm not sure that your suggestion will solve the problem of getting queue state for requested queue directly in the application.

Thanks,
Noa.

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin at redhat.com]
> Sent: Friday, August 30, 2019 11:56 AM
> To: Noa Ezra <noae at mellanox.com>; tiwei.bie at intel.com;
> zhihong.wang at intel.com
> Cc: Matan Azrad <matan at mellanox.com>; dev at dpdk.org
> Subject: Re: [Suspected-Phishing][PATCH] net/vhost: add an API for get
> queue status
> 
> Hi Noa,
> 
> I was thinking about an alternative that would avoid adding an API.
> What about the Vhost-user library to replay the queue status for all
> configured queues when the device is ready (i.e. after it has called its
> .new_device() callback)?
> 
> On 6/24/19 1:08 PM, Noa Ezra wrote:
> > Hi,
> > What do you say about this patch?
> >
> > Thanks,
> > Noa.
> >
> >> -----Original Message-----
> >> From: Noa Ezra [mailto:noae at mellanox.com]
> >> Sent: Wednesday, June 19, 2019 9:15 AM
> >> To: maxime.coquelin at redhat.com
> >> Cc: Matan Azrad <matan at mellanox.com>; dev at dpdk.org; Noa Ezra
> >> <noae at mellanox.com>
> >> Subject: [Suspected-Phishing][PATCH] net/vhost: add an API for get
> >> queue status
> >>
> >> Add an API that returns queue status for requested queue in the port.
> >> The queue's status can be changed before the user has signed for the
> >> queue state event interrupt. In this case the user can't know the
> >> current queue's status. This API returns the current status.
> >>
> >> Signed-off-by: Noa Ezra <noae at mellanox.com>
> >> Reviewed-by: Matan Azrad <matan at mellanox.com>
> >> ---
> >>  drivers/net/vhost/rte_eth_vhost.c           | 47
> >> +++++++++++++++++++++++++++++
> >>  drivers/net/vhost/rte_eth_vhost.h           | 18 +++++++++++
> >>  drivers/net/vhost/rte_pmd_vhost_version.map |  6 ++++
> >>  3 files changed, 71 insertions(+)
> >>
> >> diff --git a/drivers/net/vhost/rte_eth_vhost.c
> >> b/drivers/net/vhost/rte_eth_vhost.c
> >> index 9a54020..cad1e5c 100644
> >> --- a/drivers/net/vhost/rte_eth_vhost.c
> >> +++ b/drivers/net/vhost/rte_eth_vhost.c
> >> @@ -855,6 +855,7 @@ struct vhost_xstats_name_off {
> >>  	/* won't be NULL */
> >>  	state = vring_states[eth_dev->data->port_id];
> >>  	rte_spinlock_lock(&state->lock);
> >> +
> >>  	state->cur[vring] = enable;
> >>  	state->max_vring = RTE_MAX(vring, state->max_vring);
> >>  	rte_spinlock_unlock(&state->lock);
> >> @@ -874,6 +875,52 @@ struct vhost_xstats_name_off {  };
> >>
> >>  int
> >> +rte_eth_vhost_get_queue_status(uint16_t port_id, bool rx, uint16_t
> >> queue_id,
> >> +		bool *queue_status)
> >> +{
> >> +	struct rte_vhost_vring_state *state;
> >> +	struct internal_list *list;
> >> +	struct rte_eth_dev *eth_dev;
> >> +	int found = 0;
> >> +	uint16_t nb_q = 0;
> >> +
> >> +	if (port_id >= RTE_MAX_ETHPORTS) {
> >> +		VHOST_LOG(ERR, "Invalid port id\n");
> >> +		return -1;
> >> +	}
> >> +	TAILQ_FOREACH(list, &internal_list, next) {
> >> +		eth_dev = list->eth_dev;
> >> +		if (eth_dev->data->port_id == port_id) {
> >> +			nb_q = rx ? eth_dev->data->nb_rx_queues :
> >> +					eth_dev->data->nb_tx_queues;
> >> +			found = 1;
> >> +			break;
> >> +		}
> >> +	}
> >> +	if (!found) {
> >> +		VHOST_LOG(ERR, "No device found for port id %u\n",
> >> port_id);
> >> +		return -1;
> >> +	}
> >> +	if (queue_id >= nb_q) {
> >> +		VHOST_LOG(ERR, "Invalid queue id\n");
> >> +		return -1;
> >> +	}
> >> +
> >> +	state = vring_states[port_id];
> >> +	if (!state) {
> >> +		VHOST_LOG(ERR, "Unused port\n");
> >> +		return -1;
> >> +	}
> >> +
> >> +	rte_spinlock_lock(&state->lock);
> >> +	*queue_status = rx ? state->cur[queue_id * 2 + 1] :
> >> +			state->cur[queue_id * 2];
> >> +	rte_spinlock_unlock(&state->lock);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +int
> >>  rte_eth_vhost_get_queue_event(uint16_t port_id,
> >>  		struct rte_eth_vhost_queue_event *event)  { diff --git
> >> a/drivers/net/vhost/rte_eth_vhost.h
> >> b/drivers/net/vhost/rte_eth_vhost.h
> >> index 0e68b9f..1e65c69 100644
> >> --- a/drivers/net/vhost/rte_eth_vhost.h
> >> +++ b/drivers/net/vhost/rte_eth_vhost.h
> >> @@ -44,6 +44,24 @@ int rte_eth_vhost_get_queue_event(uint16_t
> port_id,
> >>  		struct rte_eth_vhost_queue_event *event);
> >>
> >>  /**
> >> + * Get queue status for specific queue in the port.
> >> + *
> >> + * @param[in] port_id
> >> + *  Port id.
> >> + * @param[in] rx
> >> + *  True is rx, False if tx
> >> + * @paran[in] queue_id
> >> + *  Queue_id
> >> + * @param[out] queue_status
> >> + *  Pointer to a boolean, True is enable, False if disable.
> >> + * @return
> >> + *  - On success, zero, queue_status is updated.
> >> + *  - On failure, a negative value, queue_status is not updated.
> >> + */
> >> +int rte_eth_vhost_get_queue_status(uint16_t port_id, bool rx,
> >> +uint16_t
> >> queue_id,
> >> +		bool *queue_status);
> >> +
> >> +/**
> >>   * Get the 'vid' value associated with the specified port.
> >>   *
> >>   * @return
> >> diff --git a/drivers/net/vhost/rte_pmd_vhost_version.map
> >> b/drivers/net/vhost/rte_pmd_vhost_version.map
> >> index 695db85..1eabfd2 100644
> >> --- a/drivers/net/vhost/rte_pmd_vhost_version.map
> >> +++ b/drivers/net/vhost/rte_pmd_vhost_version.map
> >> @@ -11,3 +11,9 @@ DPDK_16.11 {
> >>
> >>  	rte_eth_vhost_get_vid_from_port_id;
> >>  };
> >> +
> >> +DPDK_19.08 {
> >> +	global:
> >> +
> >> +	rte_eth_vhost_get_queue_status;
> >> +};
> >> --
> >> 1.8.3.1
> >