[dpdk-dev] [PATCH 5/5] net/virtio: fix Tso when mbuf is shared

[Yuanhan Liu]

On Tue, Jan 17, 2017 at 12:18:25PM +0100, Olivier Matz wrote:
> > I hope I could have time to dig this further, since, honestly, I don't
> > quite like this patch: it makes things un-maintainable.
> 
> Well, I'm not that proud of the patch, but that's the best solution
> I've found. Nevertheless saying it makes things un-maintainable looks a
> bit excessive to me :)

Aha... really sorry about that!

But honestly, I'd say again, it makes thing more complex, just for fixing
a corner and rare issue. I'd try to avoid that.

> 
> The option of reallocating a mbuf, copy and fix network headers in it
> looks even more complex to me (that was my first approach).
> 
> > Besides that, I think we have similar issue with nic drivers. See the
> > rte_net_intel_cksum_flags_prepare() function introduced at commit
> > 4fb7e803eb1a ("ethdev: add Tx preparation").
> 
> Yes, that was discussed a bit. See [1] and the subsequent mails.
> http://dpdk.org/ml/archives/dev/2016-December/051014.html

Thanks for the info, and I'm pretty Okay with that.

> My opinion is that tx_burst() should not change the mbuf data, it's
> always been like this. For Intel NICs, there is no issue since the DPDK
> API is derived from Intel NICs API, so there is no fix to do in the
> mbuf data.
> 
> For tx_prepare(), it's explicitly said that it can update the data.
> If tx_prepare() becomes mandatory, it will naturally fix this issue
> without modifying the driver, because the phdr csum calculation will be
> done in tx_prepare().
> 
> An alternative is to mark this as a known issue for now, and wait until
> tx_prepare() is mandatory.

I see no reason to wait. Though my understanding is it may not be a
mandatory so far, but user is supposed to calculate the pseudo-header
checksum by themself before. Now they have one more option: tx_prepare.

That means, in either way, user has to do some extra works to make
TSO work (either by themself or call tx_prepare). So I don't think
it'd be an issue?

	--yliu