* [PATCH] common/sfc_efx/base: remove VQ index check during VQ start
@ 2022-07-08 7:37 abhimanyu.saini
[not found] ` <SA0PR12MB43523906A4BF7F13DF9B9D0FE3829@SA0PR12MB4352.namprd12.prod.outlook.com>
2022-07-11 8:36 ` Andrew Rybchenko
0 siblings, 2 replies; 4+ messages in thread
From: abhimanyu.saini @ 2022-07-08 7:37 UTC (permalink / raw)
To: dev
Cc: chenbo.xia, maxime.coquelin, andrew.rybchenko, Abhimanyu Saini, stable
From: Abhimanyu Saini <absaini@amd.com>
The used/avail queue indexes are not bound by queue
size, because the descriptor entry index is calculated
by a simple modulo between queue index and queue_size
So, do not check initial used and avail queue indexes
against queue size because it is possible for these
indexes to be greater than queue size in the
following cases:
1) The queue is created to be migrated into, or
2) The client issues a qstop/qstart after running datapath
Fixes: 4dda72dbdeab3 ("common/sfc_efx/base: add base virtio support for vDPA")
Cc: stable@dpdk.org
Signed-off-by: Abhimanyu Saini <absaini@amd.com>
---
drivers/common/sfc_efx/base/rhead_virtio.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/common/sfc_efx/base/rhead_virtio.c b/drivers/common/sfc_efx/base/rhead_virtio.c
index 335cb74..7f08717 100644
--- a/drivers/common/sfc_efx/base/rhead_virtio.c
+++ b/drivers/common/sfc_efx/base/rhead_virtio.c
@@ -47,14 +47,6 @@
goto fail2;
}
- if (evvdp != NULL) {
- if ((evvdp->evvd_vq_cidx > evvcp->evvc_vq_size) ||
- (evvdp->evvd_vq_pidx > evvcp->evvc_vq_size)) {
- rc = EINVAL;
- goto fail3;
- }
- }
-
req.emr_cmd = MC_CMD_VIRTIO_INIT_QUEUE;
req.emr_in_buf = payload;
req.emr_in_length = MC_CMD_VIRTIO_INIT_QUEUE_REQ_LEN;
@@ -116,15 +108,13 @@
if (req.emr_rc != 0) {
rc = req.emr_rc;
- goto fail4;
+ goto fail3;
}
evvp->evv_vi_index = vi_index;
return (0);
-fail4:
- EFSYS_PROBE(fail4);
fail3:
EFSYS_PROBE(fail3);
fail2:
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread[parent not found: <SA0PR12MB43523906A4BF7F13DF9B9D0FE3829@SA0PR12MB4352.namprd12.prod.outlook.com>]
* RE: [PATCH] common/sfc_efx/base: remove VQ index check during VQ start
[not found] ` <SA0PR12MB43523906A4BF7F13DF9B9D0FE3829@SA0PR12MB4352.namprd12.prod.outlook.com>
@ 2022-07-08 9:17 ` Srivastava, Vijay
0 siblings, 0 replies; 4+ messages in thread
From: Srivastava, Vijay @ 2022-07-08 9:17 UTC (permalink / raw)
To: dev
Cc: chenbo.xia, Maxime Coquelin, Andrew Rybchenko, abhimanyu.saini,
Saini, Abhimanyu, stable
>From: Abhimanyu Saini <absaini@amd.com>
>
>The used/avail queue indexes are not bound by queue size, because the
>descriptor entry index is calculated by a simple modulo between queue index
>and queue_size
>
>So, do not check initial used and avail queue indexes against queue size
>because it is possible for these indexes to be greater than queue size in the
>following cases:
>1) The queue is created to be migrated into, or
>2) The client issues a qstop/qstart after running datapath
>
>Fixes: 4dda72dbdeab3 ("common/sfc_efx/base: add base virtio support for
>vDPA")
>Cc: stable@dpdk.org
>
>Signed-off-by: Abhimanyu Saini <absaini@amd.com>
>---
> drivers/common/sfc_efx/base/rhead_virtio.c | 12 +-----------
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
>diff --git a/drivers/common/sfc_efx/base/rhead_virtio.c
>b/drivers/common/sfc_efx/base/rhead_virtio.c
>index 335cb74..7f08717 100644
>--- a/drivers/common/sfc_efx/base/rhead_virtio.c
>+++ b/drivers/common/sfc_efx/base/rhead_virtio.c
>@@ -47,14 +47,6 @@
> goto fail2;
> }
>
>- if (evvdp != NULL) {
>- if ((evvdp->evvd_vq_cidx > evvcp->evvc_vq_size) ||
>- (evvdp->evvd_vq_pidx > evvcp->evvc_vq_size)) {
>- rc = EINVAL;
>- goto fail3;
>- }
>- }
>-
> req.emr_cmd = MC_CMD_VIRTIO_INIT_QUEUE;
> req.emr_in_buf = payload;
> req.emr_in_length = MC_CMD_VIRTIO_INIT_QUEUE_REQ_LEN; @@ -
>116,15 +108,13 @@
>
> if (req.emr_rc != 0) {
> rc = req.emr_rc;
>- goto fail4;
>+ goto fail3;
> }
>
> evvp->evv_vi_index = vi_index;
>
> return (0);
>
>-fail4:
>- EFSYS_PROBE(fail4);
> fail3:
> EFSYS_PROBE(fail3);
> fail2:
>--
>1.8.3.1
Acked-by: Vijay Srivastava <vijays@amd.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] common/sfc_efx/base: remove VQ index check during VQ start
2022-07-08 7:37 [PATCH] common/sfc_efx/base: remove VQ index check during VQ start abhimanyu.saini
[not found] ` <SA0PR12MB43523906A4BF7F13DF9B9D0FE3829@SA0PR12MB4352.namprd12.prod.outlook.com>
@ 2022-07-11 8:36 ` Andrew Rybchenko
2022-07-12 5:00 ` Saini, Abhimanyu
1 sibling, 1 reply; 4+ messages in thread
From: Andrew Rybchenko @ 2022-07-11 8:36 UTC (permalink / raw)
To: abhimanyu.saini, dev
Cc: chenbo.xia, maxime.coquelin, Abhimanyu Saini, stable,
Vijay Kumar Srivastava
On 7/8/22 10:37, abhimanyu.saini@xilinx.com wrote:
> From: Abhimanyu Saini <absaini@amd.com>
>
> The used/avail queue indexes are not bound by queue
> size, because the descriptor entry index is calculated
> by a simple modulo between queue index and queue_size
"is calculated" is a bit vague since looking at the code
I've failed to find the place where modulo operation is
done. Don't we need to apply it these values are put
into MCDI message?
>
> So, do not check initial used and avail queue indexes
> against queue size because it is possible for these
> indexes to be greater than queue size in the
> following cases:
> 1) The queue is created to be migrated into, or
> 2) The client issues a qstop/qstart after running datapath
>
> Fixes: 4dda72dbdeab3 ("common/sfc_efx/base: add base virtio support for vDPA")
> Cc: stable@dpdk.org
>
> Signed-off-by: Abhimanyu Saini <absaini@amd.com>
> ---
> drivers/common/sfc_efx/base/rhead_virtio.c | 12 +-----------
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/common/sfc_efx/base/rhead_virtio.c b/drivers/common/sfc_efx/base/rhead_virtio.c
> index 335cb74..7f08717 100644
> --- a/drivers/common/sfc_efx/base/rhead_virtio.c
> +++ b/drivers/common/sfc_efx/base/rhead_virtio.c
> @@ -47,14 +47,6 @@
> goto fail2;
> }
>
> - if (evvdp != NULL) {
> - if ((evvdp->evvd_vq_cidx > evvcp->evvc_vq_size) ||
> - (evvdp->evvd_vq_pidx > evvcp->evvc_vq_size)) {
> - rc = EINVAL;
> - goto fail3;
> - }
> - }
> -
> req.emr_cmd = MC_CMD_VIRTIO_INIT_QUEUE;
> req.emr_in_buf = payload;
> req.emr_in_length = MC_CMD_VIRTIO_INIT_QUEUE_REQ_LEN;
> @@ -116,15 +108,13 @@
>
> if (req.emr_rc != 0) {
> rc = req.emr_rc;
> - goto fail4;
> + goto fail3;
> }
>
> evvp->evv_vi_index = vi_index;
>
> return (0);
>
> -fail4:
> - EFSYS_PROBE(fail4);
> fail3:
> EFSYS_PROBE(fail3);
> fail2:
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH] common/sfc_efx/base: remove VQ index check during VQ start
2022-07-11 8:36 ` Andrew Rybchenko
@ 2022-07-12 5:00 ` Saini, Abhimanyu
0 siblings, 0 replies; 4+ messages in thread
From: Saini, Abhimanyu @ 2022-07-12 5:00 UTC (permalink / raw)
To: Andrew Rybchenko, abhimanyu.saini, dev
Cc: chenbo.xia, maxime.coquelin, stable, Vijay Kumar Srivastava
[AMD Official Use Only - General]
On 7/8/22 10:37, abhimanyu.saini@xilinx.com wrote:
> > From: Abhimanyu Saini <absaini@amd.com>
> >
> > The used/avail queue indexes are not bound by queue
> > size, because the descriptor entry index is calculated
> > by a simple modulo between queue index and queue_size
>
> "is calculated" is a bit vague since looking at the code
> I've failed to find the place where modulo operation is
> done. Don't we need to apply it these values are put
> into MCDI message?
>
The values are added to the MCDI as is and,
the modulo is performed by the hardware.
I can append the commit message to reflect the same?
> >
> > So, do not check initial used and avail queue indexes
> > against queue size because it is possible for these
> > indexes to be greater than queue size in the
> > following cases:
> > 1) The queue is created to be migrated into, or
> > 2) The client issues a qstop/qstart after running datapath
> >
> > Fixes: 4dda72dbdeab3 ("common/sfc_efx/base: add base virtio support for
> vDPA")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Abhimanyu Saini <absaini@amd.com>
> > ---
> > drivers/common/sfc_efx/base/rhead_virtio.c | 12 +-----------
> > 1 file changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/drivers/common/sfc_efx/base/rhead_virtio.c
> b/drivers/common/sfc_efx/base/rhead_virtio.c
> > index 335cb74..7f08717 100644
> > --- a/drivers/common/sfc_efx/base/rhead_virtio.c
> > +++ b/drivers/common/sfc_efx/base/rhead_virtio.c
> > @@ -47,14 +47,6 @@
> > goto fail2;
> > }
> >
> > - if (evvdp != NULL) {
> > - if ((evvdp->evvd_vq_cidx > evvcp->evvc_vq_size) ||
> > - (evvdp->evvd_vq_pidx > evvcp->evvc_vq_size)) {
> > - rc = EINVAL;
> > - goto fail3;
> > - }
> > - }
> > -
> > req.emr_cmd = MC_CMD_VIRTIO_INIT_QUEUE;
> > req.emr_in_buf = payload;
> > req.emr_in_length = MC_CMD_VIRTIO_INIT_QUEUE_REQ_LEN;
> > @@ -116,15 +108,13 @@
> >
> > if (req.emr_rc != 0) {
> > rc = req.emr_rc;
> > - goto fail4;
> > + goto fail3;
> > }
> >
> > evvp->evv_vi_index = vi_index;
> >
> > return (0);
> >
> > -fail4:
> > - EFSYS_PROBE(fail4);
> > fail3:
> > EFSYS_PROBE(fail3);
> > fail2:
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-12 5:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-08 7:37 [PATCH] common/sfc_efx/base: remove VQ index check during VQ start abhimanyu.saini
[not found] ` <SA0PR12MB43523906A4BF7F13DF9B9D0FE3829@SA0PR12MB4352.namprd12.prod.outlook.com>
2022-07-08 9:17 ` Srivastava, Vijay
2022-07-11 8:36 ` Andrew Rybchenko
2022-07-12 5:00 ` Saini, Abhimanyu
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).