When getting reqs from the avail ring, the id may exceed inflight queue size. Then the dpdk will crash forever. Signed-off-by: Li Feng <fengli@smartx.com> --- lib/vhost/vhost_user.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 29a4c9af60..f09d0f6a48 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct virtio_net *dev, last_io = inflight_split->last_inflight_io; if (inflight_split->used_idx != used->idx) { - inflight_split->desc[last_io].inflight = 0; - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); + if (unlikely(last_io >= inflight_split->desc_num)) { + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' exceeds inflight " + "queue size (%"PRIu16").\n", last_io, + inflight_split->desc_num); + } else { + inflight_split->desc[last_io].inflight = 0; + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); + } inflight_split->used_idx = used->idx; } -- 2.31.1
ping... BTW, how could I retrigger the failure CI? The failure is not about this patch. Thanks, Feng Li Li Feng <fengli@smartx.com> 于2021年8月27日周五 下午1:16写道: > > When getting reqs from the avail ring, the id may exceed inflight > queue size. Then the dpdk will crash forever. > > Signed-off-by: Li Feng <fengli@smartx.com> > --- > lib/vhost/vhost_user.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c > index 29a4c9af60..f09d0f6a48 100644 > --- a/lib/vhost/vhost_user.c > +++ b/lib/vhost/vhost_user.c > @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct virtio_net *dev, > last_io = inflight_split->last_inflight_io; > > if (inflight_split->used_idx != used->idx) { > - inflight_split->desc[last_io].inflight = 0; > - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > + if (unlikely(last_io >= inflight_split->desc_num)) { > + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' exceeds inflight " > + "queue size (%"PRIu16").\n", last_io, > + inflight_split->desc_num); > + } else { > + inflight_split->desc[last_io].inflight = 0; > + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > + } > inflight_split->used_idx = used->idx; > } > > -- > 2.31.1 > Li Feng <fengli@smartx.com> 于2021年8月27日周五 下午1:16写道: > > When getting reqs from the avail ring, the id may exceed inflight > queue size. Then the dpdk will crash forever. > > Signed-off-by: Li Feng <fengli@smartx.com> > --- > lib/vhost/vhost_user.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c > index 29a4c9af60..f09d0f6a48 100644 > --- a/lib/vhost/vhost_user.c > +++ b/lib/vhost/vhost_user.c > @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct virtio_net *dev, > last_io = inflight_split->last_inflight_io; > > if (inflight_split->used_idx != used->idx) { > - inflight_split->desc[last_io].inflight = 0; > - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > + if (unlikely(last_io >= inflight_split->desc_num)) { > + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' exceeds inflight " > + "queue size (%"PRIu16").\n", last_io, > + inflight_split->desc_num); > + } else { > + inflight_split->desc[last_io].inflight = 0; > + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > + } > inflight_split->used_idx = used->idx; > } > > -- > 2.31.1 >
Hi Li, Adding Jin Yu who introduced this function. On 8/27/21 07:12, Li Feng wrote: > When getting reqs from the avail ring, the id may exceed inflight > queue size. Then the dpdk will crash forever. You need to add Fixes tag and Cc stable@dpdk.org so that it can be backported. > Signed-off-by: Li Feng <fengli@smartx.com> > --- > lib/vhost/vhost_user.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c > index 29a4c9af60..f09d0f6a48 100644 > --- a/lib/vhost/vhost_user.c > +++ b/lib/vhost/vhost_user.c > @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct virtio_net *dev, > last_io = inflight_split->last_inflight_io; > > if (inflight_split->used_idx != used->idx) { > - inflight_split->desc[last_io].inflight = 0; > - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > + if (unlikely(last_io >= inflight_split->desc_num)) { > + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' exceeds inflight " > + "queue size (%"PRIu16").\n", last_io, > + inflight_split->desc_num); If such error happens, shouldn't we return RTE_VHOST_MSG_RESULT_ERR instead of just logging an error? > + } else { > + inflight_split->desc[last_io].inflight = 0; > + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > + } > inflight_split->used_idx = used->idx; > } > > Regards, Maxime
On 10/14/21 10:17, Maxime Coquelin wrote: > Hi Li, > > Adding Jin Yu who introduced this function. Looks like Jin Yu has left Intel, Chenbo, could you find someone from the Intel SPDK team to look at it? > On 8/27/21 07:12, Li Feng wrote: >> When getting reqs from the avail ring, the id may exceed inflight >> queue size. Then the dpdk will crash forever. > > You need to add Fixes tag and Cc stable@dpdk.org so that it can be > backported. > >> Signed-off-by: Li Feng <fengli@smartx.com> >> --- >> lib/vhost/vhost_user.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c >> index 29a4c9af60..f09d0f6a48 100644 >> --- a/lib/vhost/vhost_user.c >> +++ b/lib/vhost/vhost_user.c >> @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct >> virtio_net *dev, >> last_io = inflight_split->last_inflight_io; >> if (inflight_split->used_idx != used->idx) { >> - inflight_split->desc[last_io].inflight = 0; >> - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); >> + if (unlikely(last_io >= inflight_split->desc_num)) { >> + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' >> exceeds inflight " >> + "queue size (%"PRIu16").\n", last_io, >> + inflight_split->desc_num); > > If such error happens, shouldn't we return RTE_VHOST_MSG_RESULT_ERR > instead of just logging an error? > >> + } else { >> + inflight_split->desc[last_io].inflight = 0; >> + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); >> + } >> inflight_split->used_idx = used->idx; >> } >> > > Regards, > Maxime
Hi Changpeng, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Thursday, October 14, 2021 4:26 PM > To: Li Feng <fengli@smartx.com>; Xia, Chenbo <chenbo.xia@intel.com> > Cc: dev@dpdk.org > Subject: Re: [PATCH v1] vhost: add sanity check for resubmiting reqs in split > ring > > > > On 10/14/21 10:17, Maxime Coquelin wrote: > > Hi Li, > > > > Adding Jin Yu who introduced this function. > > Looks like Jin Yu has left Intel, Chenbo, could you find someone from > the Intel SPDK team to look at it? Could you or your team member help check this? Thanks, Chenbo > > > On 8/27/21 07:12, Li Feng wrote: > >> When getting reqs from the avail ring, the id may exceed inflight > >> queue size. Then the dpdk will crash forever. > > > > You need to add Fixes tag and Cc stable@dpdk.org so that it can be > > backported. > > > >> Signed-off-by: Li Feng <fengli@smartx.com> > >> --- > >> lib/vhost/vhost_user.c | 10 ++++++++-- > >> 1 file changed, 8 insertions(+), 2 deletions(-) > >> > >> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c > >> index 29a4c9af60..f09d0f6a48 100644 > >> --- a/lib/vhost/vhost_user.c > >> +++ b/lib/vhost/vhost_user.c > >> @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct > >> virtio_net *dev, > >> last_io = inflight_split->last_inflight_io; > >> if (inflight_split->used_idx != used->idx) { > >> - inflight_split->desc[last_io].inflight = 0; > >> - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > >> + if (unlikely(last_io >= inflight_split->desc_num)) { > >> + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' > >> exceeds inflight " > >> + "queue size (%"PRIu16").\n", last_io, > >> + inflight_split->desc_num); > > > > If such error happens, shouldn't we return RTE_VHOST_MSG_RESULT_ERR > > instead of just logging an error? > > > >> + } else { > >> + inflight_split->desc[last_io].inflight = 0; > >> + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > >> + } > >> inflight_split->used_idx = used->idx; > >> } > >> > > > > Regards, > > Maxime
Thank you for your response. On Thu, Oct 14, 2021 at 4:17 PM Maxime Coquelin <maxime.coquelin@redhat.com> wrote: > > Hi Li, > > Adding Jin Yu who introduced this function. > > On 8/27/21 07:12, Li Feng wrote: > > When getting reqs from the avail ring, the id may exceed inflight > > queue size. Then the dpdk will crash forever. > > You need to add Fixes tag and Cc stable@dpdk.org so that it can be > backported. OK, I will send the v2 version. > > > Signed-off-by: Li Feng <fengli@smartx.com> > > --- > > lib/vhost/vhost_user.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c > > index 29a4c9af60..f09d0f6a48 100644 > > --- a/lib/vhost/vhost_user.c > > +++ b/lib/vhost/vhost_user.c > > @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct virtio_net *dev, > > last_io = inflight_split->last_inflight_io; > > > > if (inflight_split->used_idx != used->idx) { > > - inflight_split->desc[last_io].inflight = 0; > > - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > > + if (unlikely(last_io >= inflight_split->desc_num)) { > > + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' exceeds inflight " > > + "queue size (%"PRIu16").\n", last_io, > > + inflight_split->desc_num); > > If such error happens, shouldn't we return RTE_VHOST_MSG_RESULT_ERR > instead of just logging an error? I think ignoring the error is ok. No one could handle this error correctly. At this time the guest virtio driver of this virtqueue may be in an incorrect state. > > > + } else { > > + inflight_split->desc[last_io].inflight = 0; > > + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > > + } > > inflight_split->used_idx = used->idx; > > } > > > > > > Regards, > Maxime >
On 10/14/21 13:25, Li Feng wrote: > Thank you for your response. > > On Thu, Oct 14, 2021 at 4:17 PM Maxime Coquelin > <maxime.coquelin@redhat.com> wrote: >> >> Hi Li, >> >> Adding Jin Yu who introduced this function. >> >> On 8/27/21 07:12, Li Feng wrote: >>> When getting reqs from the avail ring, the id may exceed inflight >>> queue size. Then the dpdk will crash forever. >> >> You need to add Fixes tag and Cc stable@dpdk.org so that it can be >> backported. > OK, I will send the v2 version. > >> >>> Signed-off-by: Li Feng <fengli@smartx.com> >>> --- >>> lib/vhost/vhost_user.c | 10 ++++++++-- >>> 1 file changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c >>> index 29a4c9af60..f09d0f6a48 100644 >>> --- a/lib/vhost/vhost_user.c >>> +++ b/lib/vhost/vhost_user.c >>> @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct virtio_net *dev, >>> last_io = inflight_split->last_inflight_io; >>> >>> if (inflight_split->used_idx != used->idx) { >>> - inflight_split->desc[last_io].inflight = 0; >>> - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); >>> + if (unlikely(last_io >= inflight_split->desc_num)) { >>> + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' exceeds inflight " >>> + "queue size (%"PRIu16").\n", last_io, >>> + inflight_split->desc_num); >> >> If such error happens, shouldn't we return RTE_VHOST_MSG_RESULT_ERR >> instead of just logging an error? > I think ignoring the error is ok. No one could handle this error correctly. > At this time the guest virtio driver of this virtqueue may be in an > incorrect state. Not sure to understand how it can happen. But I see that last_io is actually vq->inflight_split->last_inflight_io, which is set only by rte_vhost_set_last_inflight_io_split() API. Shouldn't there be a sanity check there to ensure that last_inflight_io is smaller than desc_num value set by the frontend? Returning an error is the right thing to do anyway. >> >>> + } else { >>> + inflight_split->desc[last_io].inflight = 0; >>> + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); >>> + } >>> inflight_split->used_idx = used->idx; >>> } >>> >>> >> >> Regards, >> Maxime >> >
On Thu, Oct 14, 2021 at 7:38 PM Maxime Coquelin <maxime.coquelin@redhat.com> wrote: > > > > On 10/14/21 13:25, Li Feng wrote: > > Thank you for your response. > > > > On Thu, Oct 14, 2021 at 4:17 PM Maxime Coquelin > > <maxime.coquelin@redhat.com> wrote: > >> > >> Hi Li, > >> > >> Adding Jin Yu who introduced this function. > >> > >> On 8/27/21 07:12, Li Feng wrote: > >>> When getting reqs from the avail ring, the id may exceed inflight > >>> queue size. Then the dpdk will crash forever. > >> > >> You need to add Fixes tag and Cc stable@dpdk.org so that it can be > >> backported. > > OK, I will send the v2 version. > > > >> > >>> Signed-off-by: Li Feng <fengli@smartx.com> > >>> --- > >>> lib/vhost/vhost_user.c | 10 ++++++++-- > >>> 1 file changed, 8 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c > >>> index 29a4c9af60..f09d0f6a48 100644 > >>> --- a/lib/vhost/vhost_user.c > >>> +++ b/lib/vhost/vhost_user.c > >>> @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct virtio_net *dev, > >>> last_io = inflight_split->last_inflight_io; > >>> > >>> if (inflight_split->used_idx != used->idx) { > >>> - inflight_split->desc[last_io].inflight = 0; > >>> - rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > >>> + if (unlikely(last_io >= inflight_split->desc_num)) { > >>> + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"' exceeds inflight " > >>> + "queue size (%"PRIu16").\n", last_io, > >>> + inflight_split->desc_num); > >> > >> If such error happens, shouldn't we return RTE_VHOST_MSG_RESULT_ERR > >> instead of just logging an error? > > I think ignoring the error is ok. No one could handle this error correctly. > > At this time the guest virtio driver of this virtqueue may be in an > > incorrect state. > > Not sure to understand how it can happen. > But I see that last_io is actually vq->inflight_split->last_inflight_io, > which is set only by rte_vhost_set_last_inflight_io_split() API. The polluted value is from the frontend driver. My environment occurs this issue, and a VM is hang, so I guess this bad value comes from it. > > Shouldn't there be a sanity check there to ensure that last_inflight_io > is smaller than desc_num value set by the frontend? Yes, putting a check in rte_vhost_set_last_inflight_io_split is also ok. I will send the v2 version that includes this. Thanks. > > Returning an error is the right thing to do anyway. OK. > > >> > >>> + } else { > >>> + inflight_split->desc[last_io].inflight = 0; > >>> + rte_atomic_thread_fence(__ATOMIC_SEQ_CST); > >>> + } > >>> inflight_split->used_idx = used->idx; > >>> } > >>> > >>> > >> > >> Regards, > >> Maxime > >> > > >
The idx in rte_vhost_set_last_inflight_io_split is from the frontend driver, check if it's in the virtqueue range. Fixes: bb0c2de9602b ("vhost: add APIs to operate inflight ring") Cc: stable@dpdk.org Signed-off-by: Li Feng <fengli@smartx.com> --- lib/vhost/vhost.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 9540522dac..3b674ac320 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -1226,6 +1226,9 @@ rte_vhost_set_last_inflight_io_split(int vid, uint16_t vring_idx, if (unlikely(!vq->inflight_split)) return -1; + if (unlikely(idx >= vq->size)) + return -1; + vq->inflight_split->last_inflight_io = idx; return 0; } -- 2.31.1
The title is too vague, I would put something like: vhost: add sanity check on inflight last index On 10/14/21 14:40, Li Feng wrote: > The idx in rte_vhost_set_last_inflight_io_split is from the frontend s/idx/index/ > driver, check if it's in the virtqueue range. > > Fixes: bb0c2de9602b ("vhost: add APIs to operate inflight ring") > Cc: stable@dpdk.org > > Signed-off-by: Li Feng <fengli@smartx.com> > --- > lib/vhost/vhost.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c > index 9540522dac..3b674ac320 100644 > --- a/lib/vhost/vhost.c > +++ b/lib/vhost/vhost.c > @@ -1226,6 +1226,9 @@ rte_vhost_set_last_inflight_io_split(int vid, uint16_t vring_idx, > if (unlikely(!vq->inflight_split)) > return -1; > > + if (unlikely(idx >= vq->size)) > + return -1; > + > vq->inflight_split->last_inflight_io = idx; > return 0; > } > Other than that, this is fine to me: Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> If my suggestions are fine for you, I can fix while applying. Thanks, Maxime
On Fri, Oct 15, 2021 at 4:52 PM Maxime Coquelin <maxime.coquelin@redhat.com> wrote: > > The title is too vague, I would put something like: > > vhost: add sanity check on inflight last index > > On 10/14/21 14:40, Li Feng wrote: > > The idx in rte_vhost_set_last_inflight_io_split is from the frontend > > s/idx/index/ > > > driver, check if it's in the virtqueue range. > > > > Fixes: bb0c2de9602b ("vhost: add APIs to operate inflight ring") > > Cc: stable@dpdk.org > > > > Signed-off-by: Li Feng <fengli@smartx.com> > > --- > > lib/vhost/vhost.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c > > index 9540522dac..3b674ac320 100644 > > --- a/lib/vhost/vhost.c > > +++ b/lib/vhost/vhost.c > > @@ -1226,6 +1226,9 @@ rte_vhost_set_last_inflight_io_split(int vid, uint16_t vring_idx, > > if (unlikely(!vq->inflight_split)) > > return -1; > > > > + if (unlikely(idx >= vq->size)) > > + return -1; > > + > > vq->inflight_split->last_inflight_io = idx; > > return 0; > > } > > > > Other than that, this is fine to me: > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > If my suggestions are fine for you, I can fix while applying. > It's fine. > Thanks, > Maxime >
I agree with Maxime, just add an error log doesn't help anything, there might be something wrong in other places,
I don't have the context for this issue, if this can be reproduced in SPDK, I suggest to submit an issue to SPDK first.
> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Thursday, October 14, 2021 4:28 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Li Feng
> <fengli@smartx.com>; Liu, Changpeng <changpeng.liu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v1] vhost: add sanity check for resubmiting reqs in split ring
>
> Hi Changpeng,
>
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: Thursday, October 14, 2021 4:26 PM
> > To: Li Feng <fengli@smartx.com>; Xia, Chenbo <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH v1] vhost: add sanity check for resubmiting reqs in split
> > ring
> >
> >
> >
> > On 10/14/21 10:17, Maxime Coquelin wrote:
> > > Hi Li,
> > >
> > > Adding Jin Yu who introduced this function.
> >
> > Looks like Jin Yu has left Intel, Chenbo, could you find someone from
> > the Intel SPDK team to look at it?
>
> Could you or your team member help check this?
>
> Thanks,
> Chenbo
>
> >
> > > On 8/27/21 07:12, Li Feng wrote:
> > >> When getting reqs from the avail ring, the id may exceed inflight
> > >> queue size. Then the dpdk will crash forever.
> > >
> > > You need to add Fixes tag and Cc stable@dpdk.org so that it can be
> > > backported.
> > >
> > >> Signed-off-by: Li Feng <fengli@smartx.com>
> > >> ---
> > >> lib/vhost/vhost_user.c | 10 ++++++++--
> > >> 1 file changed, 8 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> > >> index 29a4c9af60..f09d0f6a48 100644
> > >> --- a/lib/vhost/vhost_user.c
> > >> +++ b/lib/vhost/vhost_user.c
> > >> @@ -1823,8 +1823,14 @@ vhost_check_queue_inflights_split(struct
> > >> virtio_net *dev,
> > >> last_io = inflight_split->last_inflight_io;
> > >> if (inflight_split->used_idx != used->idx) {
> > >> - inflight_split->desc[last_io].inflight = 0;
> > >> - rte_atomic_thread_fence(__ATOMIC_SEQ_CST);
> > >> + if (unlikely(last_io >= inflight_split->desc_num)) {
> > >> + VHOST_LOG_CONFIG(ERR, "last_inflight_io '%"PRIu16"'
> > >> exceeds inflight "
> > >> + "queue size (%"PRIu16").\n", last_io,
> > >> + inflight_split->desc_num);
> > >
> > > If such error happens, shouldn't we return RTE_VHOST_MSG_RESULT_ERR
> > > instead of just logging an error?
> > >
> > >> + } else {
> > >> + inflight_split->desc[last_io].inflight = 0;
> > >> + rte_atomic_thread_fence(__ATOMIC_SEQ_CST);
> > >> + }
> > >> inflight_split->used_idx = used->idx;
> > >> }
> > >>
> > >
> > > Regards,
> > > Maxime
On 10/15/21 11:25, Li Feng wrote:
> On Fri, Oct 15, 2021 at 4:52 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> The title is too vague, I would put something like:
>>
>> vhost: add sanity check on inflight last index
>>
>> On 10/14/21 14:40, Li Feng wrote:
>>> The idx in rte_vhost_set_last_inflight_io_split is from the frontend
>>
>> s/idx/index/
>>
>>> driver, check if it's in the virtqueue range.
>>>
>>> Fixes: bb0c2de9602b ("vhost: add APIs to operate inflight ring")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Li Feng <fengli@smartx.com>
>>> ---
>>> lib/vhost/vhost.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
>>> index 9540522dac..3b674ac320 100644
>>> --- a/lib/vhost/vhost.c
>>> +++ b/lib/vhost/vhost.c
>>> @@ -1226,6 +1226,9 @@ rte_vhost_set_last_inflight_io_split(int vid, uint16_t vring_idx,
>>> if (unlikely(!vq->inflight_split))
>>> return -1;
>>>
>>> + if (unlikely(idx >= vq->size))
>>> + return -1;
>>> +
>>> vq->inflight_split->last_inflight_io = idx;
>>> return 0;
>>> }
>>>
>>
>> Other than that, this is fine to me:
>>
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> If my suggestions are fine for you, I can fix while applying.
>>
> It's fine.
>
>> Thanks,
>> Maxime
>>
>
Applied to dpdk-next-virtio/main.
Thanks,
Maxime