Re: [dpdk-dev] [PATCH v4] vhost: Add indirect descriptors support to the TX path

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Fri, Oct 28, 2016 at 09:58:51AM +0200, Maxime Coquelin wrote:
> >From my experience, and as Michael pointed out, the best mode for small packets is obviously
> >ANY_LAYOUT so it uses a single descriptor per packet.
> Of course, having a single descriptor is in theory the best way.
> But, in current Virtio PMD implementation, with no offload supported, we
> never access the virtio header at transmit time, it is allocated and
> zeroed at startup.
> 
> For ANY_LAYOUT case, the virtio header is prepended to the packet, and
> need to be zeroed at packet transmit time. The performance impact is
> quite important, as show the measurements I made one month ago (txonly):
>  - 2 descs per packet: 11.6Mpps
>  - 1 desc per packet: 9.6Mpps
> 
> As Michael suggested, I tried to replace the memset by direct
> fields assignation, but it only recovers a small part of the drop.
> 
> What I suggested is to introduce a new feature, so that we can skip the
> virtio header when no offload is negotiated.
> 
> Maybe you have other ideas?
> 
> >So, disabling indirect descriptors may give you better pps for 64 bytes packets, but that doesn't mean you should not implement, or enable, it in your driver. That just means that the guest is not taking the right decision, and uses indirect while it should actually use any_layout.
> +1, it really depends on the use-case.
> >
> >Given virtio/vhost design (most decision comes from the guest), the host should be liberal in what it accepts, and not try to influence guest implementation by carefully picking the features it supports. Otherwise guests will never get a chance to make the right decisions either.
> Agree, what we need is to be able to disable Virtio PMD features
> without having to rebuild the PMD.

I want this feature (or more precisely, ability) long times ago.
For example, I'd wish there is an option like "force_legacy" when
both legacy and modern exist.

> It will certainly require an new API change to add this option.

I don't think it will work.

Firstly, generally, as discussed before, it's not a good idea to
introduce some PMD specific APIs.

Secondly, even if it's okay to do so, you need to let the application
(say testpmd) invoke it. Once you have done that, you need introduce
some CLI options to make sure that part is invoked.

And as stated before, it's also not a good idea to add virtio PMD
specific CLI options to testpmd. 

For this case, however, I think it makes more sense if test provides
some commands to enable/disable csum and tso stuff. With that, we
could enable it dynamically.

It has "tso set/show" commands though: it just doesn't look like the
right interface to me to enable/disable tso.

	--yliu