* [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 @ 2018-04-27 14:19 Ciara Loftus 2018-04-27 14:36 ` Maxime Coquelin 0 siblings, 1 reply; 6+ messages in thread From: Ciara Loftus @ 2018-04-27 14:19 UTC (permalink / raw) To: dev rte_eth_vhost_get_vid_from_port_id returns a value of 0 if called before the first call to the new_device callback. A vid value >=0 suggests the device is active which is not the case in this instance. Initialise vid to a negative value to prevent this. Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> --- drivers/net/vhost/rte_eth_vhost.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c index 99a7727..f47950c 100644 --- a/drivers/net/vhost/rte_eth_vhost.c +++ b/drivers/net/vhost/rte_eth_vhost.c @@ -1051,6 +1051,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id, return -ENOMEM; } + vq->vid = -1; vq->mb_pool = mb_pool; vq->virtqueue_id = rx_queue_id * VIRTIO_QNUM + VIRTIO_TXQ; dev->data->rx_queues[rx_queue_id] = vq; -- 2.7.5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 2018-04-27 14:19 [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 Ciara Loftus @ 2018-04-27 14:36 ` Maxime Coquelin 2018-04-30 13:57 ` Loftus, Ciara 0 siblings, 1 reply; 6+ messages in thread From: Maxime Coquelin @ 2018-04-27 14:36 UTC (permalink / raw) To: Ciara Loftus, dev On 04/27/2018 04:19 PM, Ciara Loftus wrote: > rte_eth_vhost_get_vid_from_port_id returns a value of 0 if called before > the first call to the new_device callback. A vid value >=0 suggests the > device is active which is not the case in this instance. Initialise vid > to a negative value to prevent this. > > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> > --- > drivers/net/vhost/rte_eth_vhost.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c > index 99a7727..f47950c 100644 > --- a/drivers/net/vhost/rte_eth_vhost.c > +++ b/drivers/net/vhost/rte_eth_vhost.c > @@ -1051,6 +1051,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id, > return -ENOMEM; > } > > + vq->vid = -1; > vq->mb_pool = mb_pool; > vq->virtqueue_id = rx_queue_id * VIRTIO_QNUM + VIRTIO_TXQ; > dev->data->rx_queues[rx_queue_id] = vq; > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Thanks, Maxime ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 2018-04-27 14:36 ` Maxime Coquelin @ 2018-04-30 13:57 ` Loftus, Ciara 2018-05-03 13:18 ` Loftus, Ciara 0 siblings, 1 reply; 6+ messages in thread From: Loftus, Ciara @ 2018-04-30 13:57 UTC (permalink / raw) To: Maxime Coquelin, dev > > On 04/27/2018 04:19 PM, Ciara Loftus wrote: > > rte_eth_vhost_get_vid_from_port_id returns a value of 0 if called before > > the first call to the new_device callback. A vid value >=0 suggests the > > device is active which is not the case in this instance. Initialise vid > > to a negative value to prevent this. > > > > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> > > --- > > drivers/net/vhost/rte_eth_vhost.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/net/vhost/rte_eth_vhost.c > b/drivers/net/vhost/rte_eth_vhost.c > > index 99a7727..f47950c 100644 > > --- a/drivers/net/vhost/rte_eth_vhost.c > > +++ b/drivers/net/vhost/rte_eth_vhost.c > > @@ -1051,6 +1051,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, > uint16_t rx_queue_id, > > return -ENOMEM; > > } > > > > + vq->vid = -1; > > vq->mb_pool = mb_pool; > > vq->virtqueue_id = rx_queue_id * VIRTIO_QNUM + VIRTIO_TXQ; > > dev->data->rx_queues[rx_queue_id] = vq; > > > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > Thanks, > Maxime On second thoughts, self-NACK. We need to provision for the case where we want to call eth_rx_queue_setup AFTER new_device. For instance when we want to change the mb_pool. In this case we need to maintain the same vid and not reset it to -1. Without this patch the original problem still exists and need to find an alternative workaround. Thanks, Ciara ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 2018-04-30 13:57 ` Loftus, Ciara @ 2018-05-03 13:18 ` Loftus, Ciara 2018-05-04 13:09 ` Maxime Coquelin 0 siblings, 1 reply; 6+ messages in thread From: Loftus, Ciara @ 2018-05-03 13:18 UTC (permalink / raw) To: dev; +Cc: Maxime Coquelin, Chen, Junjie J > > > > > On 04/27/2018 04:19 PM, Ciara Loftus wrote: > > > rte_eth_vhost_get_vid_from_port_id returns a value of 0 if called before > > > the first call to the new_device callback. A vid value >=0 suggests the > > > device is active which is not the case in this instance. Initialise vid > > > to a negative value to prevent this. > > > > > > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> > > > --- > > > drivers/net/vhost/rte_eth_vhost.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/net/vhost/rte_eth_vhost.c > > b/drivers/net/vhost/rte_eth_vhost.c > > > index 99a7727..f47950c 100644 > > > --- a/drivers/net/vhost/rte_eth_vhost.c > > > +++ b/drivers/net/vhost/rte_eth_vhost.c > > > @@ -1051,6 +1051,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, > > uint16_t rx_queue_id, > > > return -ENOMEM; > > > } > > > > > > + vq->vid = -1; > > > vq->mb_pool = mb_pool; > > > vq->virtqueue_id = rx_queue_id * VIRTIO_QNUM + VIRTIO_TXQ; > > > dev->data->rx_queues[rx_queue_id] = vq; > > > > > > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > > > Thanks, > > Maxime > > On second thoughts, self-NACK. > > We need to provision for the case where we want to call > eth_rx_queue_setup AFTER new_device. For instance when we want to > change the mb_pool. In this case we need to maintain the same vid and not > reset it to -1. > > Without this patch the original problem still exists and need to find an > alternative workaround. Junjie's patches fix the issue I was observing. Thanks Junjie! https://dpdk.org/browse/dpdk/commit/?id=30a701a53737a0b6f7953412cc3b3d36c1d49122 https://dpdk.org/browse/dpdk/commit/?id=e6722dee533cda3756fbc5c9ea4ddfbf30276f1b Along with the v2 of this patch could they be considered for the 17.11 stable branch? Thanks, Ciara > > Thanks, > Ciara ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 2018-05-03 13:18 ` Loftus, Ciara @ 2018-05-04 13:09 ` Maxime Coquelin 2018-05-07 3:18 ` Chen, Junjie J 0 siblings, 1 reply; 6+ messages in thread From: Maxime Coquelin @ 2018-05-04 13:09 UTC (permalink / raw) To: Loftus, Ciara, dev; +Cc: Chen, Junjie J On 05/03/2018 03:18 PM, Loftus, Ciara wrote: >> >>> >>> On 04/27/2018 04:19 PM, Ciara Loftus wrote: >>>> rte_eth_vhost_get_vid_from_port_id returns a value of 0 if called before >>>> the first call to the new_device callback. A vid value >=0 suggests the >>>> device is active which is not the case in this instance. Initialise vid >>>> to a negative value to prevent this. >>>> >>>> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> >>>> --- >>>> drivers/net/vhost/rte_eth_vhost.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/net/vhost/rte_eth_vhost.c >>> b/drivers/net/vhost/rte_eth_vhost.c >>>> index 99a7727..f47950c 100644 >>>> --- a/drivers/net/vhost/rte_eth_vhost.c >>>> +++ b/drivers/net/vhost/rte_eth_vhost.c >>>> @@ -1051,6 +1051,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, >>> uint16_t rx_queue_id, >>>> return -ENOMEM; >>>> } >>>> >>>> + vq->vid = -1; >>>> vq->mb_pool = mb_pool; >>>> vq->virtqueue_id = rx_queue_id * VIRTIO_QNUM + VIRTIO_TXQ; >>>> dev->data->rx_queues[rx_queue_id] = vq; >>>> >>> >>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> >>> >>> Thanks, >>> Maxime >> >> On second thoughts, self-NACK. >> >> We need to provision for the case where we want to call >> eth_rx_queue_setup AFTER new_device. For instance when we want to >> change the mb_pool. In this case we need to maintain the same vid and not >> reset it to -1. >> >> Without this patch the original problem still exists and need to find an >> alternative workaround. > > Junjie's patches fix the issue I was observing. Thanks Junjie! > https://dpdk.org/browse/dpdk/commit/?id=30a701a53737a0b6f7953412cc3b3d36c1d49122 > https://dpdk.org/browse/dpdk/commit/?id=e6722dee533cda3756fbc5c9ea4ddfbf30276f1b > > Along with the v2 of this patch could they be considered for the 17.11 stable branch? Yes, I think it is a good idea. It wasn't planned for -stable initially as it fixed a new use-case in v18.05. Junjie, can you please generate a backport against v17.11 and post it to stable@dpdk.org, adding "PATCH v17.11 LTS" as subject prefix, and using -x option of when cherry-picking so that it references the patches in master? Thanks in advance, Maxime > Thanks, > Ciara > >> >> Thanks, >> Ciara ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 2018-05-04 13:09 ` Maxime Coquelin @ 2018-05-07 3:18 ` Chen, Junjie J 0 siblings, 0 replies; 6+ messages in thread From: Chen, Junjie J @ 2018-05-07 3:18 UTC (permalink / raw) To: Maxime Coquelin, Loftus, Ciara, dev Hi Maxime I saw Yuanhan already did this. Two patches already in 17.11 LTS. Cheers JJ > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] > Sent: Friday, May 4, 2018 9:09 PM > To: Loftus, Ciara <ciara.loftus@intel.com>; dev@dpdk.org > Cc: Chen, Junjie J <junjie.j.chen@intel.com> > Subject: Re: [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 > > > > On 05/03/2018 03:18 PM, Loftus, Ciara wrote: > >> > >>> > >>> On 04/27/2018 04:19 PM, Ciara Loftus wrote: > >>>> rte_eth_vhost_get_vid_from_port_id returns a value of 0 if called > >>>> before the first call to the new_device callback. A vid value >=0 > >>>> suggests the device is active which is not the case in this > >>>> instance. Initialise vid to a negative value to prevent this. > >>>> > >>>> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> > >>>> --- > >>>> drivers/net/vhost/rte_eth_vhost.c | 1 + > >>>> 1 file changed, 1 insertion(+) > >>>> > >>>> diff --git a/drivers/net/vhost/rte_eth_vhost.c > >>> b/drivers/net/vhost/rte_eth_vhost.c > >>>> index 99a7727..f47950c 100644 > >>>> --- a/drivers/net/vhost/rte_eth_vhost.c > >>>> +++ b/drivers/net/vhost/rte_eth_vhost.c > >>>> @@ -1051,6 +1051,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, > >>> uint16_t rx_queue_id, > >>>> return -ENOMEM; > >>>> } > >>>> > >>>> + vq->vid = -1; > >>>> vq->mb_pool = mb_pool; > >>>> vq->virtqueue_id = rx_queue_id * VIRTIO_QNUM + VIRTIO_TXQ; > >>>> dev->data->rx_queues[rx_queue_id] = vq; > >>>> > >>> > >>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > >>> > >>> Thanks, > >>> Maxime > >> > >> On second thoughts, self-NACK. > >> > >> We need to provision for the case where we want to call > >> eth_rx_queue_setup AFTER new_device. For instance when we want to > >> change the mb_pool. In this case we need to maintain the same vid and > >> not reset it to -1. > >> > >> Without this patch the original problem still exists and need to find > >> an alternative workaround. > > > > Junjie's patches fix the issue I was observing. Thanks Junjie! > > > https://dpdk.org/browse/dpdk/commit/?id=30a701a53737a0b6f7953412cc3b3 > d > > 36c1d49122 > > > https://dpdk.org/browse/dpdk/commit/?id=e6722dee533cda3756fbc5c9ea4dd > f > > bf30276f1b > > > > Along with the v2 of this patch could they be considered for the 17.11 stable > branch? > > Yes, I think it is a good idea. > It wasn't planned for -stable initially as it fixed a new use-case in v18.05. > > Junjie, can you please generate a backport against v17.11 and post it to > stable@dpdk.org, adding "PATCH v17.11 LTS" as subject prefix, and using -x > option of when cherry-picking so that it references the patches in master? > > Thanks in advance, > Maxime > > Thanks, > > Ciara > > > >> > >> Thanks, > >> Ciara ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-07 3:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-27 14:19 [dpdk-dev] [PATCH] net/vhost: Initialise vid to -1 Ciara Loftus 2018-04-27 14:36 ` Maxime Coquelin 2018-04-30 13:57 ` Loftus, Ciara 2018-05-03 13:18 ` Loftus, Ciara 2018-05-04 13:09 ` Maxime Coquelin 2018-05-07 3:18 ` Chen, Junjie J
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).