Hi David,

From: David Marchand [mailto:david.march...@6wind.com]
Sent: Thursday, October 29, 2015 10:16 PM
To: Liu, Jijiang
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] [PATCH 5/8] driver/virtio:enqueue vhost TX offload

On Wed, Oct 21, 2015 at 6:46 AM, Jijiang Liu <jijiang.liu at 
intel.com<mailto:jijiang.liu at intel.com>> wrote:
@@ -221,6 +277,11 @@ virtqueue_enqueue_xmit(struct virtqueue *txvq, struct 
rte_mbuf *cookie)
        dxp->cookie = (void *)cookie;
        dxp->ndescs = needed;

+       if (vtpci_with_feature(txvq->hw, VIRTIO_NET_F_CSUM)) {
+               if (virtqueue_enqueue_offload(txvq, cookie, idx, head_size) < 0)
+                       return -EPERM;
+       }
+
        start_dp = txvq->vq_ring.desc;
        start_dp[idx].addr =
                txvq->virtio_net_hdr_mem + idx * head_size;

If the driver correctly reports negotiated offload capabilities (see my 
previous comment on patch 3), there is no need for the test on 
VIRTIO_NET_F_CSUM, because application is not supposed to ask for offloads on 
driver that do not support them.


> If the driver correctly reports negotiated offload capabilities, then 
> application in guest will set the ol_flags in mbuf based on these offload 
> capabilities.
If the VIRTIO_NET_F_CSUM is not enabled, and there is no need to call 
virtqueue_enqueue_offload() to check ol_flags in mbuf to see if the TX checksum 
and TSO is set ,and it will  not effect on the performance of disabling TX 
checksum path as much as possible.
So I think there is need for the check.

> I agree with your comments on patch 3, I will add TX offload capabilities in 
> the dev_info to tell application driver support these offloads.



Same logic would apply to virtqueue_enqueue_offload() function.

In the end, we could always call this function (or move the code here).


--
David Marchand

Reply via email to