* [dpdk-dev] [PATCH] net/vhost: add an API for get queue status @ 2019-06-19 6:14 Noa Ezra 2019-06-24 11:08 ` [dpdk-dev] [Suspected-Phishing][PATCH] " Noa Ezra 0 siblings, 1 reply; 8+ messages in thread From: Noa Ezra @ 2019-06-19 6:14 UTC (permalink / raw) To: maxime.coquelin; +Cc: matan, dev, noae 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue status 2019-06-19 6:14 [dpdk-dev] [PATCH] net/vhost: add an API for get queue status Noa Ezra @ 2019-06-24 11:08 ` Noa Ezra 2019-06-24 16:47 ` Maxime Coquelin 2019-08-30 8:55 ` Maxime Coquelin 0 siblings, 2 replies; 8+ messages in thread From: Noa Ezra @ 2019-06-24 11:08 UTC (permalink / raw) To: Noa Ezra, maxime.coquelin, tiwei.bie, zhihong.wang; +Cc: Matan Azrad, dev Hi, What do you say about this patch? 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue status 2019-06-24 11:08 ` [dpdk-dev] [Suspected-Phishing][PATCH] " Noa Ezra @ 2019-06-24 16:47 ` Maxime Coquelin 2019-06-25 7:00 ` Noa Ezra 2019-08-30 8:55 ` Maxime Coquelin 1 sibling, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2019-06-24 16:47 UTC (permalink / raw) To: Noa Ezra, tiwei.bie, zhihong.wang; +Cc: Matan Azrad, dev 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue status 2019-06-24 16:47 ` Maxime Coquelin @ 2019-06-25 7:00 ` Noa Ezra 2019-06-25 7:36 ` Maxime Coquelin 0 siblings, 1 reply; 8+ messages in thread From: Noa Ezra @ 2019-06-25 7:00 UTC (permalink / raw) To: Maxime Coquelin, tiwei.bie, zhihong.wang; +Cc: Matan Azrad, dev > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] > Sent: Monday, June 24, 2019 7:47 PM > To: Noa Ezra <noae@mellanox.com>; tiwei.bie@intel.com; > zhihong.wang@intel.com > Cc: Matan Azrad <matan@mellanox.com>; dev@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@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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue status 2019-06-25 7:00 ` Noa Ezra @ 2019-06-25 7:36 ` Maxime Coquelin 2019-06-25 7:49 ` Maxime Coquelin 0 siblings, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2019-06-25 7:36 UTC (permalink / raw) To: Noa Ezra, tiwei.bie, zhihong.wang; +Cc: Matan Azrad, dev On 6/25/19 9:00 AM, Noa Ezra wrote: > > >> -----Original Message----- >> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >> Sent: Monday, June 24, 2019 7:47 PM >> To: Noa Ezra <noae@mellanox.com>; tiwei.bie@intel.com; >> zhihong.wang@intel.com >> Cc: Matan Azrad <matan@mellanox.com>; dev@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. Indeed, and I hope we can get rid off them one day. > 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. I think we can get rid off rte_eth_vhost_get_queue_event easily. Maybe we can even just drop it if we implement the new ethdev ops. > 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. It is anyway too late for v19.08, as the proposal deadline is over 3 weeks now. I will only accept fixes or small patches that have limited impact. Here, the patch is adding a new API, which is significant. And with the new API stability rules that is being put in place, it may imply that we'll need to support it for more than 2 years. So for v19.11, if you think you don't have the bandwidth, maybe I can try to do it. I really prefer that than having an API already deprecated when merged in master. Thanks, Maxime > Thanks, > Noa. > > >> 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 >>> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue status 2019-06-25 7:36 ` Maxime Coquelin @ 2019-06-25 7:49 ` Maxime Coquelin 0 siblings, 0 replies; 8+ messages in thread From: Maxime Coquelin @ 2019-06-25 7:49 UTC (permalink / raw) To: Noa Ezra, tiwei.bie, zhihong.wang; +Cc: Matan Azrad, dev And about short term, can't you just call rte_eth_vhost_get_queue_event() in loop at startup until you get -1 and build the states based on that? As states->seen is zero-initialized, you would get all queues that have been enabled before event handler is registered, right? Thanks, Maxime On 6/25/19 9:36 AM, Maxime Coquelin wrote: > > > On 6/25/19 9:00 AM, Noa Ezra wrote: >> >> >>> -----Original Message----- >>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >>> Sent: Monday, June 24, 2019 7:47 PM >>> To: Noa Ezra <noae@mellanox.com>; tiwei.bie@intel.com; >>> zhihong.wang@intel.com >>> Cc: Matan Azrad <matan@mellanox.com>; dev@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. > > Indeed, and I hope we can get rid off them one day. > >> 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. > > I think we can get rid off rte_eth_vhost_get_queue_event easily. > Maybe we can even just drop it if we implement the new ethdev ops. > >> 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. > > It is anyway too late for v19.08, as the proposal deadline is over 3 > weeks now. I will only accept fixes or small patches that have limited > impact. > > Here, the patch is adding a new API, which is significant. > And with the new API stability rules that is being put in place, it may > imply that we'll need to support it for more than 2 years. > > So for v19.11, if you think you don't have the bandwidth, maybe I can > try to do it. I really prefer that than having an API already deprecated > when merged in master. > > Thanks, > Maxime >> Thanks, >> Noa. >> >> >>> 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 >>>> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue status 2019-06-24 11:08 ` [dpdk-dev] [Suspected-Phishing][PATCH] " Noa Ezra 2019-06-24 16:47 ` Maxime Coquelin @ 2019-08-30 8:55 ` Maxime Coquelin 2019-09-12 10:04 ` Noa Ezra 1 sibling, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2019-08-30 8:55 UTC (permalink / raw) To: Noa Ezra, tiwei.bie, zhihong.wang; +Cc: Matan Azrad, dev 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@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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue status 2019-08-30 8:55 ` Maxime Coquelin @ 2019-09-12 10:04 ` Noa Ezra 0 siblings, 0 replies; 8+ messages in thread From: Noa Ezra @ 2019-09-12 10:04 UTC (permalink / raw) To: Maxime Coquelin, tiwei.bie, zhihong.wang; +Cc: Matan Azrad, dev 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@redhat.com] > Sent: Friday, August 30, 2019 11:56 AM > To: Noa Ezra <noae@mellanox.com>; tiwei.bie@intel.com; > zhihong.wang@intel.com > Cc: Matan Azrad <matan@mellanox.com>; dev@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@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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-09-12 10:04 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-19 6:14 [dpdk-dev] [PATCH] net/vhost: add an API for get queue status Noa Ezra 2019-06-24 11:08 ` [dpdk-dev] [Suspected-Phishing][PATCH] " Noa Ezra 2019-06-24 16:47 ` Maxime Coquelin 2019-06-25 7:00 ` Noa Ezra 2019-06-25 7:36 ` Maxime Coquelin 2019-06-25 7:49 ` Maxime Coquelin 2019-08-30 8:55 ` Maxime Coquelin 2019-09-12 10:04 ` Noa Ezra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).