Re: [RFC v1 2/4] vhost-vdpa: introduce vhost-vdpa net client

[Eric Blake <eblake@redhat.com>]

On 4/20/20 4:32 AM, Cindy Lu wrote:
> This patch set introduces a new net client type: vhost-vdpa.
> vhost-vdpa net client will set up a vDPA device which is svhostdevpecified

looks like you pasted 'vhostdev' in the middle of 'specified'

> by a "vhostdev" parameter.
> 
> Author: Tiwei Bie

Should this be a 'Signed-off-by' tag?

> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---

> +++ b/include/net/vhost-vdpa.h
> @@ -0,0 +1,18 @@
> +/*
> + * vhost-vdpa.h
> + *
> + * Copyright(c) 2017 Intel Corporation. All rights reserved.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.

Claiming "All rights reserved" is at odds with claiming "GPL", which 
specifically requires that you are NOT reserving all rights but are 
instead granting various rights to others insofar as they preserve this 
code as free software.  I can overlook it on BSD licenses (as there are 
lots of examples of bad copy-and-paste on various templates that uses 
the misleading term), but not on GPL licenses.  Also, you may want to 
consider if adding 2020 to your copyright date is appropriate.


> +++ b/net/vhost-vdpa.c
> @@ -0,0 +1,211 @@
> +/*
> + * vhost-vdpa.c
> + *
> + * Copyright(c) 2017-2018 Intel Corporation. All rights reserved.
> + * Copyright(c) 2020 Red Hat, Inc.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.

Another inconsistent license.


> +++ b/qapi/net.json
> @@ -441,6 +441,22 @@
>       '*queues':        'int' } }
>   
>   ##
> +# @NetdevVhostVDPAOptions:
> +#
> +# Vhost-vdpa network backend
> +#
> +# @vhostdev: name of a mdev dev path in sysfs
> +#
> +# @queues: number of queues to be created for multiqueue vhost-vdpa
> +#          (default: 1) (Since 2.11)

There's no need to add a 'since' tag for an individual member if...

> +#
> +# Since: 2.11

...the struct itself was introduced in the same release.  However, using 
2.11 as the release is wrong; the next release will be 5.1.

> +##
> +{ 'struct': 'NetdevVhostVDPAOptions',
> +  'data': {
> +    '*vhostdev':     'str',
> +    '*queues':       'int' } }
> +##
>   # @NetClientDriver:
>   #
>   # Available netdev drivers.
> @@ -451,7 +467,7 @@
>   ##
>   { 'enum': 'NetClientDriver',

Missing documentation that 'vhost-vdpa' was added in 5.1.

>     'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> -            'bridge', 'hubport', 'netmap', 'vhost-user' ] }
> +            'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa' ] }
>   
>   ##
>   # @Netdev:
> @@ -479,7 +495,8 @@
>       'bridge':   'NetdevBridgeOptions',
>       'hubport':  'NetdevHubPortOptions',
>       'netmap':   'NetdevNetmapOptions',
> -    'vhost-user': 'NetdevVhostUserOptions' } }
> +    'vhost-user': 'NetdevVhostUserOptions',
> +    'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
>   
>   ##
>   # @NetLegacy:
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org