I'm testing your series, and it gets stuck after 256 packets in transmit
path. When it happens, descs flags indicate it has been made available
by the driver (desc->flags = 0x80), but it is not consistent with the
expected wrap counter value (0).

Not sure this is the root cause, but it seems below code is broken:

On 10/03/2018 03:11 PM, Jens Freimann wrote:
+static inline void
+virtqueue_enqueue_xmit_packed(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
+                       uint16_t needed, int use_indirect, int can_push,
+                       int in_order)
+{
+       struct virtio_tx_region *txr = txvq->virtio_net_hdr_mz->addr;
+       struct vq_desc_extra *dxp, *head_dxp;
+       struct virtqueue *vq = txvq->vq;
+       struct vring_desc_packed *start_dp, *head_dp;
+       uint16_t seg_num = cookie->nb_segs;
+       uint16_t idx, head_id;
+       uint16_t head_size = vq->hw->vtnet_hdr_size;
+       struct virtio_net_hdr *hdr;
+       int wrap_counter = vq->vq_ring.avail_wrap_counter;
+
+       head_id = vq->vq_desc_head_idx;
+       idx = head_id;  
+       start_dp = vq->vq_ring.desc_packed;
+       dxp = &vq->vq_descx[idx];
+       dxp->ndescs = needed;
+
+       head_dp = &vq->vq_ring.desc_packed[head_id];
+       head_dxp = &vq->vq_descx[head_id];
+       head_dxp->cookie = (void *) cookie;
+
+       if (can_push) {
+               /* prepend cannot fail, checked by caller */
+               hdr = (struct virtio_net_hdr *)
+                       rte_pktmbuf_prepend(cookie, head_size);
+               /* rte_pktmbuf_prepend() counts the hdr size to the pkt length,
+                * which is wrong. Below subtract restores correct pkt size.
+                */
+               cookie->pkt_len -= head_size;
+
+               /* if offload disabled, it is not zeroed below, do it now */
+               if (!vq->hw->has_tx_offload) {
+                       ASSIGN_UNLESS_EQUAL(hdr->csum_start, 0);
+                       ASSIGN_UNLESS_EQUAL(hdr->csum_offset, 0);
+                       ASSIGN_UNLESS_EQUAL(hdr->flags, 0);
+                       ASSIGN_UNLESS_EQUAL(hdr->gso_type, 0);
+                       ASSIGN_UNLESS_EQUAL(hdr->gso_size, 0);
+                       ASSIGN_UNLESS_EQUAL(hdr->hdr_len, 0);
+               }
+       } else if (use_indirect) {
+               /* setup tx ring slot to point to indirect
+                * descriptor list stored in reserved region.
+                *
+                * the first slot in indirect ring is already preset
+                * to point to the header in reserved region
+                */
+               start_dp[idx].addr  = txvq->virtio_net_hdr_mem +
+                       RTE_PTR_DIFF(&txr[idx].tx_indir_pq, txr);
+               start_dp[idx].len   = (seg_num + 1) * sizeof(struct 
vring_desc_packed);
+               start_dp[idx].flags = VRING_DESC_F_INDIRECT;
+               hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr;
+
+               /* loop below will fill in rest of the indirect elements */
+               start_dp = txr[idx].tx_indir_pq;
+               idx = 1;
+       } else {
+               /* setup first tx ring slot to point to header
+                * stored in reserved region.
+                */
+               start_dp[idx].addr  = txvq->virtio_net_hdr_mem +
+                       RTE_PTR_DIFF(&txr[idx].tx_hdr, txr);
+               start_dp[idx].len   = vq->hw->vtnet_hdr_size;
+               start_dp[idx].flags = VRING_DESC_F_NEXT;
+               start_dp[idx].flags |=
+                       VRING_DESC_F_AVAIL(vq->vq_ring.avail_wrap_counter) |
+                       VRING_DESC_F_USED(!vq->vq_ring.avail_wrap_counter);
+               hdr = (struct virtio_net_hdr *)&txr[idx].tx_hdr;
+               idx = dxp->next;
+       }
+
+       virtqueue_xmit_offload(hdr, cookie, vq->hw->has_tx_offload);
+
+       do {
+               if (idx >= vq->vq_nentries) {
+                       idx = 0;
+                       vq->vq_ring.avail_wrap_counter ^= 1;
+               }
+               start_dp[idx].addr  = VIRTIO_MBUF_DATA_DMA_ADDR(cookie, vq);
+               start_dp[idx].len   = cookie->data_len;
+               start_dp[idx].flags = cookie->next ? VRING_DESC_F_NEXT : 0;
+               start_dp[idx].flags |=
+                       VRING_DESC_F_AVAIL(vq->vq_ring.avail_wrap_counter) |
+                       VRING_DESC_F_USED(!vq->vq_ring.avail_wrap_counter);
+               if (use_indirect) {
+                       if (++idx >= (seg_num + 1))
+                               break;
+               } else {
+                       dxp = &vq->vq_descx[idx];
+                       idx = dxp->next;
+               }

Imagine current idx is 255, dxp->next will give idx 0, right?
In that case, for desc[0], on next iteration, the flags won't be set available properly, as vq->vq_ring.avail_wrap_counter isn't updated.

I'm not sure how it could work like this, shouldn't dxp save the wrap
counter value in out-of-order case?

Reply via email to