From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-000f0801.pphosted.com (mx0a-000f0801.pphosted.com [67.231.144.122]) by dpdk.org (Postfix) with ESMTP id 4B9E78D96 for ; Thu, 17 Dec 2015 12:18:13 +0100 (CET) Received: from pps.filterd (m0048193.ppops.net [127.0.0.1]) by mx0a-000f0801.pphosted.com (8.15.0.59/8.15.0.59) with SMTP id tBHBE6PL023823; Thu, 17 Dec 2015 03:18:12 -0800 Received: from brmwp-exmb11.corp.brocade.com ([208.47.132.227]) by mx0a-000f0801.pphosted.com with ESMTP id 1yuk8j170a-1 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); Thu, 17 Dec 2015 03:18:12 -0800 Received: from EMEAWP-EXMB12.corp.brocade.com (172.29.11.86) by BRMWP-EXMB11.corp.brocade.com (172.16.59.77) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Thu, 17 Dec 2015 04:18:10 -0700 Received: from [10.252.48.20] (10.252.48.20) by EMEAWP-EXMB12.corp.brocade.com (172.29.11.86) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Thu, 17 Dec 2015 12:18:06 +0100 To: "Xie, Huawei" , "dev@dpdk.org" References: <1447407013-6986-1-git-send-email-tkiely@brocade.com> From: Tom Kiely Message-ID: <567299E9.60000@brocade.com> Date: Thu, 17 Dec 2015 11:18:01 +0000 User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.252.48.20] X-ClientProxiedBy: hq1wp-excas14.corp.brocade.com (10.70.38.103) To EMEAWP-EXMB12.corp.brocade.com (172.29.11.86) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.15.21, 1.0.33, 0.0.0000 definitions=2015-12-17_01:2015-12-17,2015-12-17,1970-01-01 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=2 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=7.0.1-1511060000 definitions=main-1512170178 Subject: Re: [dpdk-dev] [PATCH] virtio: fix rx ring descriptor starvation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Dec 2015 11:18:13 -0000 On 11/25/2015 05:32 PM, Xie, Huawei wrote: > On 11/13/2015 5:33 PM, Tom Kiely wrote: >> If all rx descriptors are processed while transient >> mbuf exhaustion is present, the rx ring ends up with >> no available descriptors. Thus no packets are received >> on that ring. Since descriptor refill is performed post >> rx descriptor processing, in this case no refill is >> ever subsequently performed resulting in permanent rx >> traffic drop. >> >> Signed-off-by: Tom Kiely >> --- >> drivers/net/virtio/virtio_rxtx.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c >> index 5770fa2..a95e234 100644 >> --- a/drivers/net/virtio/virtio_rxtx.c >> +++ b/drivers/net/virtio/virtio_rxtx.c >> @@ -586,7 +586,8 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) >> if (likely(num > DESC_PER_CACHELINE)) >> num = num - ((rxvq->vq_used_cons_idx + num) % DESC_PER_CACHELINE); >> >> - if (num == 0) >> + /* Refill free descriptors even if no pkts recvd */ >> + if (num == 0 && virtqueue_full(rxvq)) > Should the return condition be that no used buffers and we have avail > descs in avail ring, i.e, > num == 0 && rxvq->vq_free_cnt != rxvq->vq_nentries > > rather than > num == 0 && rxvq->vq_free_cnt == 0 Yes we could do that but I don't see a good reason to wait until the vq_free_cnt == vq_nentries before attempting the refill. The existing code will attempt refill even if only 1 packet was received and the free count is small. To me it seems safer to extend that to try refill even if no packet was received but the free count is non-zero. Tom >> return 0; >> >> num = virtqueue_dequeue_burst_rx(rxvq, rcv_pkts, len, num); >> @@ -683,7 +684,8 @@ virtio_recv_mergeable_pkts(void *rx_queue, >> >> virtio_rmb(); >> >> - if (nb_used == 0) >> + /* Refill free descriptors even if no pkts recvd */ >> + if (nb_used == 0 && virtqueue_full(rxvq)) >> return 0; >> >> PMD_RX_LOG(DEBUG, "used:%d\n", nb_used);