> -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Wednesday, June 27, 2018 4:18 PM > To: Liu, Yong <yong....@intel.com>; Bie, Tiwei <tiwei....@intel.com> > Cc: Wang, Zhihong <zhihong.w...@intel.com>; dev@dpdk.org > Subject: Re: [PATCH v2 6/8] net/virtio: support IN_ORDER Rx and Tx > > > > On 06/25/2018 05:17 PM, Marvin Liu wrote: > > IN_ORDER Rx function can support merge-able feature. Descriptors > My understanding of the code is that IN_ORDER Rx function only supports > mergeable feature (and it seems to be confirmed by patch 7). > > So maybe change the text to: > "IN_ORDER Rx function requires merge-able feature."
Thanks for point out this, Rx function choice was depended on merge-able feature. Will change comment in next release. > > > allocation and free will be done in bulk. > > > > Virtio dequeue logic: > > dequeue_burst_rx(burst mbufs) > > for (each mbuf b) { > > if (b need merge) { > > merge remained mbufs > > add merged mbuf to return mbufs list > > } else { > > add mbuf to return mbufs list > > } > > } > > if (last mbuf c need merge) { > > dequeue_burst_rx(required mbufs) > > merge last mbuf c > > } > > refill_avail_ring_bulk() > > update_avail_ring() > > return mbufs list > > > > IN_ORDER Tx function can support offloading features. Packets can't be > s/can support/supports/ > > > transmitted by IN_ORDER Tx will be handled by normal Tx. > > Can you clarify why some packets can't be transmitted by IN_ORDER Tx? My previous description may not clear, all Tx descriptors will be saved to avail ring in-order. For making code clean, packets which matched "can_push" option will be handled by simple xmit inorder function. Those packets can't match "can_push" will be handled one by one by previous xmit function. Thanks, Marvin > > > Virtio enqueue logic: > > xmit_cleanup(used descs) > > for (each xmit mbuf b) { > > if (b can inorder xmit) { > > add mbuf b to inorder burst list > > continue > > } else { > > xmit inorder burst list > > xmit mbuf b with normal xmit > > } > > } > > if (inorder burst list not empty) { > > xmit inorder burst list > > } > > update_avail_ring() > > > > Signed-off-by: Marvin Liu <yong....@intel.com> > > > > > Other than above clarification needed, the code looks good to me. > > Thanks, > Maxime