[dpdk-dev] [PATCH 01/17] vhost: introduce driver features related APIs

Yuanhan Liu yuanhan.liu at linux.intel.com
Fri Mar 17 06:50:29 CET 2017


On Thu, Mar 16, 2017 at 10:18:21AM +0100, Maxime Coquelin wrote:
> 
> 
> On 03/16/2017 08:08 AM, Yuanhan Liu wrote:
> >On Tue, Mar 14, 2017 at 10:53:23AM +0100, Maxime Coquelin wrote:
> >>
> >>
> >>On 03/14/2017 10:46 AM, Maxime Coquelin wrote:
> >>>
> >>>
> >>>On 03/03/2017 10:51 AM, Yuanhan Liu wrote:
> >>>>Introduce few APIs to set/get/enable/disable driver features.
> >>>>
> >>>>Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> >>>>---
> >>>>lib/librte_vhost/rte_vhost_version.map | 10 ++++
> >>>>lib/librte_vhost/rte_virtio_net.h      |  9 ++++
> >>>>lib/librte_vhost/socket.c              | 90
> >>>>++++++++++++++++++++++++++++++++++
> >>>>3 files changed, 109 insertions(+)
> >>>
> >>>Nice!
> >>>Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> >>>
> >>>I wonder whether we could protect from setting/enabling/disabling
> >>>features once negotiation is done?
> >
> >Those APIs won't be able to change the negotiated features. They are
> >just some interfaces before the vhost-user connection is established.
> 
> Right.
> I meant it could return an error is the connection is already
> established. Else, the caller might think the feature has been
> successfully enabled/disabled, whereas it is not the case.
> But this is maybe over-engineering to handle this case.
> 
> >Ideally, we could/should get rid of the enabling/disabling functions:
> >let the vhost-user driver to handle the features directly (say, for
> >vhost-user pmd, we could use vdev options to disable/enable few specific
> >features). Once that is done, use the rte_vhost_driver_set_features()
> >set the features once.
> 
> Ok, but what about vhost lib users and TSO (I'm thinking of OVS).

Yes, that's why I kept them :)

	--yliu
> 
> >The reason I introduced the enable/disable_features() is to keep the
> >compatability with the builtin vhost-user net driver (virtio_net.c).
> >If there is a plan to move it into vhost-pmd, they won't be needed.
> >And I don't think that will happen soon.
> 
> I agree with you that would be ideal, but unlikely to happen soon.
> 
> >>Oh, I forgot one comment on this patch.
> >>Since these new functions are part to the API, shouldn't them be
> >>documented?
> >
> >Yes, indeed, it's also noted in my cover letter.
> 
> Oops, I missed that!
> 
> Thanks,
> Maxime
> >	--yliu
> >


More information about the dev mailing list