Re: [dpdk-dev] [PATCH v2 6/8] net/virtio: support IN_ORDER Rx and Tx

[Maxime Coquelin]

On 06/27/2018 04:27 PM, Liu, Yong wrote:

-----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! Sounds good with above clarification.

Maxime

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