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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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?

Thanks,
Maxime
> Thanks,
> Noa.
> 
>> -----Original Message-----
>> From: Noa Ezra [mailto:noae@mellanox.com]
>> Sent: Wednesday, June 19, 2019 9:15 AM
>> To: maxime.coquelin@redhat.com
>> Cc: Matan Azrad <matan@mellanox.com>; dev@dpdk.org; Noa Ezra
>> <noae@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@mellanox.com>
>> Reviewed-by: Matan Azrad <matan@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
>