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

[Noa Ezra]

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin at redhat.com]
> Sent: Monday, June 24, 2019 7:47 PM
> 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
> 
> 
> 
> On 6/24/19 1:08 PM, Noa Ezra wrote:
> > Hi,
> > What do you say about this patch?
> 
> I acknowledge we miss a way to get the queue state, but I don't like
> introducing new APIs for PMD drivers.
> 
> I looked at ethdev ops on Friday, but it seems no one currently available can
> do the job.
> 
> I would suggest to create new ethdev ops like:
> .get_tx_queue_state(int port_id, int queue_id) .get_rx_queue_state(int
> port_id, int queue_id)
> 
> That would return wether the queue is enabled or not.
> 
> What do you think?

I understand what you say about not having new APIs in the PMD, but I see that there are already APIs in the vhost pmd.
rte_eth_vhost_get_queue_event allows to get the queue_state asynchronously, so it sounds reasonable to add an API that allows to get the queue_state synchronously.

Maybe in the future we can create new ethdev ops like you suggested and remove both APIs for getting queue_state.
Currently I don’t have the resources for such implementation and we really need the ability to get queue_state.

Thanks,
Noa.


> Thanks,
> Maxime
> > 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
> >