From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id 69F8C271 for ; Wed, 23 Jan 2019 14:09:06 +0100 (CET) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20190123130905euoutp016ee061e968a32c0f3a52133fd7ea5a41~8fDZw5OM30412704127euoutp01f for ; Wed, 23 Jan 2019 13:09:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20190123130905euoutp016ee061e968a32c0f3a52133fd7ea5a41~8fDZw5OM30412704127euoutp01f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1548248945; bh=jcf2S2gOXAF2e2uba7YK52ol4HrnjJvETnIbD0h9VaU=; h=Subject:To:From:Date:In-Reply-To:References:From; b=DjIF41ue8/CjDRElOJdByCOm5vkLcNzSp1h7E6TtTFKt5LzZfG4MbLGgH/AhMuY16 Ql9UseVNS7oVP57TSo5XtetFuuUWHQXiTZ/ZaqKlbG6rfT9hPyMaY7a1PsO/QRKhes Cn8VhWmAHEosGUwrEPXs38GyRLg1FC6/7BggTzj8= Received: from eusmges1new.samsung.com (unknown [203.254.199.242]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20190123130904eucas1p2750c612d49c7a52df304d4f1ac7c2d35~8fDZRvtCG2267322673eucas1p2_; Wed, 23 Jan 2019 13:09:04 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges1new.samsung.com (EUCPMTA) with SMTP id 49.68.04441.077684C5; Wed, 23 Jan 2019 13:09:04 +0000 (GMT) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20190123130903eucas1p2730776e71039a79024dd7602b5dcad7d~8fDYhOR940505705057eucas1p2j; Wed, 23 Jan 2019 13:09:03 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20190123130903eusmtrp175272221a591634942652089e5441917~8fDYQFv0s3013530135eusmtrp1T; Wed, 23 Jan 2019 13:09:03 +0000 (GMT) X-AuditID: cbfec7f2-5c9ff70000001159-01-5c4867709caa Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 83.8C.04128.F67684C5; Wed, 23 Jan 2019 13:09:03 +0000 (GMT) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20190123130903eusmtip262f7bb9fc51c9f92c5b7207cecf75280~8fDXwcmG90800408004eusmtip2D; Wed, 23 Jan 2019 13:09:02 +0000 (GMT) To: Tiwei Bie , maxime.coquelin@redhat.com, zhihong.wang@intel.com, dev@dpdk.org From: Ilya Maximets Message-ID: Date: Wed, 23 Jan 2019 16:09:02 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20190122170143.5650-2-tiwei.bie@intel.com> Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprLKsWRmVeSWpSXmKPExsWy7djP87oF6R4xBsfWiFm8+7SdyeJK+092 i2Ode1gstjb8Z7LYfHESkwOrx68FS1k9Fu95yeTxft9VNo++LasYA1iiuGxSUnMyy1KL9O0S uDJW3Z7JUnDHomLhztXsDYy79LoYOTkkBEwkZnyYxdTFyMUhJLCCUeL3uUmMEM4XRokH+9cx QzifGSU6Ht5ihmlZ2Hkcqmo5o8T1bYdZIZyPQM6rL6wgVcIClhLrTz5iArFFBNIlfk25xw5i swnoSJxafYQRxOYVsJN4Nm8JG4jNIqAq8eTSFrB6UYEIiY77q9kgagQlTs58wgJicwpYSLS1 rgazmQXEJZq+rGSFsOUltr+dA3aqhEA/u8ScaZ+gTnWRmPD/MROELSzx6vgWdghbRuL05B4W CLte4n7LS0aI5g5GiemH/kE12EtseX0OqIEDaIOmxPpd+hBhR4ndFxcxgYQlBPgkbrwVhLiB T2LStunMEGFeiY42IYhqFYnfB5dDXSMlcfPdZ6gLPCSWHF/OOIFRcRaSL2ch+WwWks9mIdyw gJFlFaN4amlxbnpqsWFearlecWJucWleul5yfu4mRmCiOf3v+KcdjF8vJR1iFOBgVOLh/WHm HiPEmlhWXJl7iFGCg1lJhNf1oluMEG9KYmVValF+fFFpTmrxIUZpDhYlcd5qhgfRQgLpiSWp 2ampBalFMFkmDk6pBkbBiuOHjZ5mqggyHSmb8Var1cX3+25Ho+MHa/a5f+24l7h06RKVxyIm S+2+6xsuuWViFDXnV0ore5T+IpX0y491hK4uVOsq7ZzEtrWpa81p06+mc8LSJBvLWS45Jwee dHvMPnlS1NEXbprXd8srf5skEVXe+9C3o8f0CLua7qeL01vfGh1/mKvEUpyRaKjFXFScCAAg kuUHMAMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrEIsWRmVeSWpSXmKPExsVy+t/xe7r56R4xBud2y1u8+7SdyeJK+092 i2Ode1gstjb8Z7LYfHESkwOrx68FS1k9Fu95yeTxft9VNo++LasYA1ii9GyK8ktLUhUy8otL bJWiDS2M9AwtLfSMTCz1DI3NY62MTJX07WxSUnMyy1KL9O0S9DJW3Z7JUnDHomLhztXsDYy7 9LoYOTkkBEwkFnYeZ+xi5OIQEljKKPH05Qp2iISUxI9fF1ghbGGJP9e62EBsIYH3jBLnj7iC 2MIClhLrTz5iArFFBNIl1l6fyQ5RYy7x6dt0sDibgI7EqdVHGEFsXgE7iWfzloDNYRFQlXhy aQtYjahAhMTZl+ugagQlTs58wgJicwpYSLS1rgazmQXUJf7Mu8QMYYtLNH1ZyQphy0tsfzuH eQKj4Cwk7bOQtMxC0jILScsCRpZVjCKppcW56bnFRnrFibnFpXnpesn5uZsYgfGz7djPLTsY u94FH2IU4GBU4uH9YeYeI8SaWFZcmXuIUYKDWUmE1/WiW4wQb0piZVVqUX58UWlOavEhRlOg 5yYyS4km5wNjO68k3tDU0NzC0tDc2NzYzEJJnPe8QWWUkEB6YklqdmpqQWoRTB8TB6dUA6PU dDVBpu/PS65t9Y9rEjV02nRT987iz3IMBVtWRmcdvBFe5r9Y5q5xoumNpnXn3iwQ5Hh2e8Xn KAteO9UcfcYolWlnY2z3WlSnb3p6pujT5IBjQnuda91jdDYH1v55bXNUI4zPtPlm6Z15Nb+z Gc8tmF7g9S42SaXS+Rxb7tEpJs8jTQXs9iuxFGckGmoxFxUnAgDSmkCktQIAAA== X-CMS-MailID: 20190123130903eucas1p2730776e71039a79024dd7602b5dcad7d X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20190123130903eucas1p2730776e71039a79024dd7602b5dcad7d X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20190123130903eucas1p2730776e71039a79024dd7602b5dcad7d References: <20190122170143.5650-2-tiwei.bie@intel.com> Subject: Re: [dpdk-dev] [1/4] net/virtio: fix the control vq support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Jan 2019 13:09:06 -0000 On 22.01.2019 20:01, Tiwei Bie wrote: > This patch mainly fixed below issues in the packed ring based > control vq support in virtio driver: > > 1. When parsing the used descriptors, we have to track the > number of descs that we need to skip; > 2. vq->vq_free_cnt was decreased twice for a same desc; > > Meanwhile, make the function name consistent with other parts. > > Fixes: ec194c2f1895 ("net/virtio: support packed queue in send command") > Fixes: a4270ea4ff79 ("net/virtio: check head desc with correct wrap counter") > > Signed-off-by: Tiwei Bie > --- > drivers/net/virtio/virtio_ethdev.c | 62 ++++++++++++++---------------- > drivers/net/virtio/virtqueue.h | 12 +----- > 2 files changed, 31 insertions(+), 43 deletions(-) > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index ee5a98b7c..a3fe65599 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -142,16 +142,17 @@ static const struct rte_virtio_xstats_name_off rte_virtio_txq_stat_strings[] = { > struct virtio_hw_internal virtio_hw_internal[RTE_MAX_ETHPORTS]; > > static struct virtio_pmd_ctrl * > -virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl, > - int *dlen, int pkt_num) > +virtio_send_command_packed(struct virtnet_ctl *cvq, > + struct virtio_pmd_ctrl *ctrl, > + int *dlen, int pkt_num) > { > struct virtqueue *vq = cvq->vq; > int head; > struct vring_packed_desc *desc = vq->ring_packed.desc_packed; > struct virtio_pmd_ctrl *result; > - bool avail_wrap_counter, used_wrap_counter; > - uint16_t flags; > + bool avail_wrap_counter; > int sum = 0; > + int nb_descs = 0; > int k; > > /* > @@ -162,11 +163,10 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl, > */ > head = vq->vq_avail_idx; > avail_wrap_counter = vq->avail_wrap_counter; > - used_wrap_counter = vq->used_wrap_counter; > - desc[head].flags = VRING_DESC_F_NEXT; > desc[head].addr = cvq->virtio_net_hdr_mem; > desc[head].len = sizeof(struct virtio_net_ctrl_hdr); > vq->vq_free_cnt--; > + nb_descs++; > if (++vq->vq_avail_idx >= vq->vq_nentries) { > vq->vq_avail_idx -= vq->vq_nentries; > vq->avail_wrap_counter ^= 1; > @@ -177,55 +177,51 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl, > + sizeof(struct virtio_net_ctrl_hdr) > + sizeof(ctrl->status) + sizeof(uint8_t) * sum; > desc[vq->vq_avail_idx].len = dlen[k]; > - flags = VRING_DESC_F_NEXT; Looks like barriers was badly placed here before this patch. Anyway, you need a write barrier here between {addr, len} and flags updates. > + desc[vq->vq_avail_idx].flags = VRING_DESC_F_NEXT | > + VRING_DESC_F_AVAIL(vq->avail_wrap_counter) | > + VRING_DESC_F_USED(!vq->avail_wrap_counter); > sum += dlen[k]; > vq->vq_free_cnt--; > - flags |= VRING_DESC_F_AVAIL(vq->avail_wrap_counter) | > - VRING_DESC_F_USED(!vq->avail_wrap_counter); > - desc[vq->vq_avail_idx].flags = flags; > - rte_smp_wmb(); > - vq->vq_free_cnt--; > + nb_descs++; > if (++vq->vq_avail_idx >= vq->vq_nentries) { > vq->vq_avail_idx -= vq->vq_nentries; > vq->avail_wrap_counter ^= 1; > } > } > > - > desc[vq->vq_avail_idx].addr = cvq->virtio_net_hdr_mem > + sizeof(struct virtio_net_ctrl_hdr); > desc[vq->vq_avail_idx].len = sizeof(ctrl->status); > - flags = VRING_DESC_F_WRITE; > - flags |= VRING_DESC_F_AVAIL(vq->avail_wrap_counter) | > - VRING_DESC_F_USED(!vq->avail_wrap_counter); > - desc[vq->vq_avail_idx].flags = flags; > - flags = VRING_DESC_F_NEXT; > - flags |= VRING_DESC_F_AVAIL(avail_wrap_counter) | > - VRING_DESC_F_USED(!avail_wrap_counter); > - desc[head].flags = flags; > - rte_smp_wmb(); > - Same here. We need a write barrier to be sure that {addr, len} written before updating flags. Another way to avoid most of barriers is to work similar to 'flush_shadow_used_ring_packed', i.e. update all the data in a loop - write barrier - update all the flags. > + desc[vq->vq_avail_idx].flags = VRING_DESC_F_WRITE | > + VRING_DESC_F_AVAIL(vq->avail_wrap_counter) | > + VRING_DESC_F_USED(!vq->avail_wrap_counter); > vq->vq_free_cnt--; > + nb_descs++; > if (++vq->vq_avail_idx >= vq->vq_nentries) { > vq->vq_avail_idx -= vq->vq_nentries; > vq->avail_wrap_counter ^= 1; > } > > + virtio_wmb(vq->hw->weak_barriers); > + desc[head].flags = VRING_DESC_F_NEXT | > + VRING_DESC_F_AVAIL(avail_wrap_counter) | > + VRING_DESC_F_USED(!avail_wrap_counter); > + > + virtio_wmb(vq->hw->weak_barriers); > virtqueue_notify(vq); > > /* wait for used descriptors in virtqueue */ > - do { > - rte_rmb(); > + while (!desc_is_used(&desc[head], vq)) > usleep(100); > - } while (!__desc_is_used(&desc[head], used_wrap_counter)); > + > + virtio_rmb(vq->hw->weak_barriers); > > /* now get used descriptors */ > - while (desc_is_used(&desc[vq->vq_used_cons_idx], vq)) { > - vq->vq_free_cnt++; > - if (++vq->vq_used_cons_idx >= vq->vq_nentries) { > - vq->vq_used_cons_idx -= vq->vq_nentries; > - vq->used_wrap_counter ^= 1; > - } > + vq->vq_free_cnt += nb_descs; > + vq->vq_used_cons_idx += nb_descs; > + if (vq->vq_used_cons_idx >= vq->vq_nentries) { > + vq->vq_used_cons_idx -= vq->vq_nentries; > + vq->used_wrap_counter ^= 1; > } > > result = cvq->virtio_net_hdr_mz->addr; > @@ -266,7 +262,7 @@ virtio_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl, > sizeof(struct virtio_pmd_ctrl)); > > if (vtpci_packed_queue(vq->hw)) { > - result = virtio_pq_send_command(cvq, ctrl, dlen, pkt_num); > + result = virtio_send_command_packed(cvq, ctrl, dlen, pkt_num); > goto out_unlock; > } > > diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h > index 7fcde5643..ca9d8e6e3 100644 > --- a/drivers/net/virtio/virtqueue.h > +++ b/drivers/net/virtio/virtqueue.h > @@ -281,7 +281,7 @@ struct virtio_tx_region { > }; > > static inline int > -__desc_is_used(struct vring_packed_desc *desc, bool wrap_counter) > +desc_is_used(struct vring_packed_desc *desc, struct virtqueue *vq) > { > uint16_t used, avail, flags; > > @@ -289,16 +289,9 @@ __desc_is_used(struct vring_packed_desc *desc, bool wrap_counter) > used = !!(flags & VRING_DESC_F_USED(1)); > avail = !!(flags & VRING_DESC_F_AVAIL(1)); > > - return avail == used && used == wrap_counter; > + return avail == used && used == vq->used_wrap_counter; > } > > -static inline int > -desc_is_used(struct vring_packed_desc *desc, struct virtqueue *vq) > -{ > - return __desc_is_used(desc, vq->used_wrap_counter); > -} > - > - > static inline void > vring_desc_init_packed(struct virtqueue *vq, int n) > { > @@ -354,7 +347,6 @@ virtqueue_enable_intr_packed(struct virtqueue *vq) > { > uint16_t *event_flags = &vq->ring_packed.driver_event->desc_event_flags; > > - > if (vq->event_flags_shadow == RING_EVENT_FLAGS_DISABLE) { > virtio_wmb(vq->hw->weak_barriers); > vq->event_flags_shadow = RING_EVENT_FLAGS_ENABLE; >