[dpdk-dev] [PATCH v5 7/9] net/virtio: support in-order Rx and Tx

[Maxime Coquelin]

On 07/02/2018 06:52 PM, Ferruh Yigit wrote:
> On 7/2/2018 5:41 PM, Ferruh Yigit wrote:
>> On 7/2/2018 2:56 PM, Marvin Liu wrote:
>>> IN_ORDER Rx function depends on merge-able feature. Descriptors
>>> 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 which
>>> matched "can_push" option will be handled by simple xmit function. Those
>>> packets can't match "can_push" will be handled by original xmit function
>>> with in-order flag.
>>>
>>> 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 by original function
>>>              }
>>>      }
>>>      if (inorder burst list not empty) {
>>>              xmit inorder burst list
>>>      }
>>>      update_avail_ring()
>>>
>>> Signed-off-by: Marvin Liu <yong.liu at intel.com>
>>> Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
>>
>> <...>
>>
>>> @@ -150,6 +188,83 @@ virtio_xmit_cleanup(struct virtqueue *vq, uint16_t num)
>>>   	}
>>>   }
>>>   
>>> +/* Cleanup from completed inorder transmits. */
>>> +static void
>>> +virtio_xmit_cleanup_inorder(struct virtqueue *vq, uint16_t num)
>>> +{
>>> +	uint16_t i, used_idx, desc_idx, last_idx;
>>
>>
>> Getting following build error [1], from code it looks like false positive, but
>> to get rid of the build error would it be OK to set initial value to "desc_idx"?
> 
> I applied this while merging, if this is wrong please let me know, we can fix in
> next-net, Thanks.

Looks good to me. I didn't catch it with the GCC version I use.

Thanks,
Maxime

>>
>>
>> [1]
>> .../dpdk/drivers/net/virtio/virtio_rxtx.c:195:24: error: ‘desc_idx’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>>
>>
>>    uint16_t i, used_idx, desc_idx, last_idx;
>>
>>
>>
>>                          ^~~~~~~~
>>
>