Hi Olivier,

Firstly sorry for late response!

On Thu, Nov 24, 2016 at 09:56:38AM +0100, Olivier Matz wrote:
> With virtio, doing tso requires to modify the network
> packet data:

I thought more about it this time, and I'm wondering why it's needed.

> - the dpdk API requires to set the l4 checksum to an
>   Intel-Nic-like pseudo header checksum that does
>   not include the ip length

If the packet is for a NIC pmd driver in the end, then the NIC driver
(or application) would handle the checksum correctly.  You could check
the tx_prep patchset for example.

> - the virtio peer expects that the l4 checksum is
>   a standard pseudo header checksum.

For this case, the checksum is then not needed: we could assume the data
between virtio to virtio transmission on the same host is always valid,
that checksum validation is unnecessary.

So, in either case, it doesn't seem to me we have to generate the checksum
here. Or am I miss something?

OTOH, even if it does, I still see some issues (see below).

>               /* TCP Segmentation Offload */
>               if (cookie->ol_flags & PKT_TX_TCP_SEG) {
> -                     virtio_tso_fix_cksum(cookie);
> +                     offset = virtio_tso_fix_cksum(cookie,
> +                             RTE_PTR_ADD(hdr, start_dp[hdr_idx].len),
> +                             VIRTIO_MAX_HDR_SZ);
> +                     if (offset > 0) {
> +                             RTE_ASSERT(can_push != 0);

I think it's (can_push == 0) ?

> +                             start_dp[hdr_idx].len += offset;

Actually, there is an assumption if you do this, that the backend driver
must have to support ANY_LAYOUT. Otherwise, it won't work: the driver
would expect the header and packet data is totally separated into two
desc buffers.

Though the assumption is most likely true in nowadays, I don't think
it's a guarantee.

        --yliu

Reply via email to