From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B319FA0C47; Thu, 14 Oct 2021 13:54:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7E33A41244; Thu, 14 Oct 2021 13:54:47 +0200 (CEST) Received: from mail-ua1-f46.google.com (mail-ua1-f46.google.com [209.85.222.46]) by mails.dpdk.org (Postfix) with ESMTP id 4D36E40041 for ; Thu, 14 Oct 2021 13:54:46 +0200 (CEST) Received: by mail-ua1-f46.google.com with SMTP id h4so10748533uaw.1 for ; Thu, 14 Oct 2021 04:54:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=smartx-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=qldIImoiy+ZGyxAHRFAAhPHjLkDSIOWizddcrJxyl0w=; b=duUoL6HU//eYljIlM8jL1/EiQ1eXuTRTOmo03yJ2HebFBaMArRQRrETgT2EKDpv+65 +7Jdlk9zvtmAQpq2fV5lj0ENTNu/4guQFtEhIeuo+AM4LgUeWvFCOnh05Byvh23l7n7/ wvEV31/192eMRLimeBjAHGijJwRLQhkT/xkhrr88LxOGEZx29pbA7qSwuJCttxpi1I7D nE2a66pxBzol2u+zIsfm+vXyifyJ2f3GH0cqj11kayVhvS0dfMKTRn9MVPtNrARt6tZG RJ62iMApFc7tP1Jz8rCvOfNNUNC+LEXXn3o35xNgAqmbnwcIMacC7RmRd1lbTmRmdE2D Nq2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qldIImoiy+ZGyxAHRFAAhPHjLkDSIOWizddcrJxyl0w=; b=LwNxyRa4sXGcoW2ktXDYkvjYzhX3RFFD/Pw8nnJsy3dMPJyDFv2ewchPZ+wpRw+fVQ QsmuwnSz4r/7Ay2JXHmmJF/jBCzOh0ENzXiEU+HQQuM/lbamJBc/udtRVMVheq0AcDsA KVXLC34hJNm0/LJBLHLxejoAbZzmIV7bxr+uoSLATDpvCcIlpVkxFZSu7C+ItLqf6C8R aCJETaeaBNZXQnAovVFgj4q8Bd68XLRx+ZFCquVMpIyW8OW33rwT5ryO+go88qJ6Zuyq FBA6r6f3MWNebm7gZU8o6a815VtMYgNoNBT2OXGqkS3JVqzSyq1HHGqG46HBzUmpvIFZ A8rw== X-Gm-Message-State: AOAM5331QLIniChI0gmLwKlOf25v4bc/txK0j0+MWl+PwjsaBkv20gkd unaMKTUi4C6tSW8D4L46NfTaj34B5YrYlp/cxbVBE3AxZsxwwrCI X-Google-Smtp-Source: ABdhPJw9MqWoWynWF1agpZd685nV39Kq1jvUFJQPrCmxzHZ1NQueSw8WpyvK+i5v9WJ68c31eXxx49hYHGPvFUrQsxs= X-Received: by 2002:ab0:7a53:: with SMTP id a19mr5560055uat.126.1634212485708; Thu, 14 Oct 2021 04:54:45 -0700 (PDT) MIME-Version: 1.0 References: <20210827051241.2448098-1-fengli@smartx.com> <1d3701ab-071b-e92c-6e95-639bdff048de@redhat.com> In-Reply-To: <1d3701ab-071b-e92c-6e95-639bdff048de@redhat.com> From: Li Feng Date: Thu, 14 Oct 2021 19:54:34 +0800 Message-ID: To: Maxime Coquelin Cc: Chenbo Xia , dev Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v1] vhost: add sanity check for resubmiting reqs in split ring X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Oct 14, 2021 at 7:38 PM Maxime Coquelin 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 > > 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 > >>> --- > >>> 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 > >> > > >