Hi, > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Tuesday, June 25, 2019 11:24 AM > To: Noa Ezra <n...@mellanox.com> > Cc: Matan Azrad <ma...@mellanox.com>; dev@dpdk.org; sta...@dpdk.org > Subject: Re: [Suspected-Phishing][PATCH] net/vhost: fix redundant queue > state event > > > > On 6/25/19 9:04 AM, Noa Ezra wrote: > > Hi, > > What do you think about this patch? > > > > Thanks, > > Noa. > > > >> -----Original Message----- > >> From: Noa Ezra [mailto:n...@mellanox.com] > >> Sent: Wednesday, June 19, 2019 9:16 AM > >> To: maxime.coque...@redhat.com > >> Cc: Matan Azrad <ma...@mellanox.com>; dev@dpdk.org; Noa Ezra > >> <n...@mellanox.com>; sta...@dpdk.org > >> Subject: [Suspected-Phishing][PATCH] net/vhost: fix redundant queue > >> state event > >> > >> In some situations, when a virtual machine is starting, > >> vring_state_changed can be called while there was no change in the > >> queue state. This fix makes sure that there was really a change in > >> the queue state before calling the callback for EVENT_QUEUE_STATE. > >> > >> Fixes: ee584e9710b9 ("vhost: add driver on top of the library") > >> Cc: sta...@dpdk.org > >> Reviewed-by: Matan Azrad <ma...@mellanox.com> > >> --- > >> drivers/net/vhost/rte_eth_vhost.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/net/vhost/rte_eth_vhost.c > >> b/drivers/net/vhost/rte_eth_vhost.c > >> index cad1e5c..fbe7a37 100644 > >> --- a/drivers/net/vhost/rte_eth_vhost.c > >> +++ b/drivers/net/vhost/rte_eth_vhost.c > >> @@ -855,6 +855,10 @@ struct vhost_xstats_name_off { > >> /* won't be NULL */ > >> state = vring_states[eth_dev->data->port_id]; > >> rte_spinlock_lock(&state->lock); > >> + if (state->cur[vring] == enable) { > >> + rte_spinlock_unlock(&state->lock); > >> + return 0; > >> + } > >> > >> state->cur[vring] = enable; > >> state->max_vring = RTE_MAX(vring, state->max_vring); > > Maybe the application would want to be notified a new queue is available, > even if it is disabled?
Can you please look again? As I understand it, "enable" is the "new state" parameter (can be enable/disable). In this fix I make sure that there was really a change in the state before calling EVENT_QUEUE_STATE (with no change in the state). > > >> -- > >> 1.8.3.1 > >
