netdev - Re: [PATCH 2/2] IFC VDPA layer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <A48EFA5D-56C6-404B-96FF-75736FCFD11E@gmail.com>
Date:   Sat, 9 Nov 2019 11:56:46 -0800
From:   Mark D Rustad <mrustad@...il.com>
To:     Zhu Lingshan <lingshan.zhu@...el.com>
Cc:     mst@...hat.com, jasowang@...hat.com, alex.williamson@...hat.com,
        linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, kvm@...r.kernel.org,
        netdev@...r.kernel.org, dan.daly@...el.com,
        cunming.liang@...el.com, tiwei.bie@...el.com, jason.zeng@...el.com
Subject: Re: [PATCH 2/2] IFC VDPA layer

On Nov 5, 2019, at 1:37 AM, Zhu Lingshan <lingshan.zhu@...el.com> wrote:

> This commit introduced IFC operations for vdpa, which complys to
> virtio_mdev and vhost_mdev interfaces, handles IFC VF
> initialization, configuration and removal.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@...el.com>
> ---
>  drivers/vhost/ifcvf/ifcvf_main.c | 605 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 605 insertions(+)
>  create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c
>
> diff --git a/drivers/vhost/ifcvf/ifcvf_main.c  
> b/drivers/vhost/ifcvf/ifcvf_main.c
> new file mode 100644
> index 0000000..7165457
> --- /dev/null
> +++ b/drivers/vhost/ifcvf/ifcvf_main.c
> @@ -0,0 +1,605 @@

<snip>

> +	for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
> +		if (!vf->vring[i].ready) {
> +			IFC_ERR(ifcvf->dev,
> +				"Failed to start datapath, vring %d not ready.\n", i);
> +			return -EINVAL;
> +		}
> +
> +		if (!vf->vring[i].size) {
> +			IFC_ERR(ifcvf->dev,
> +				"Failed to start datapath, vring %d size is zero.\n", i);
> +			return -EINVAL;
> +		}
> +
> +		if (!vf->vring[i].desc || !vf->vring[i].avail ||
> +			!vf->vring[i].used) {
> +			IFC_ERR(ifcvf->dev,
> +				"Failed to start datapath, "
> +				"invaild value for vring %d desc,"
> +				"avail_idx or usex_idx.\n", i);

Please don't break up the format string. Start it on the second line and  
let it run as long as it needs to. Also you will find that it is improperly  
spaced as it is. It makes it easier to grep the source to find the source  
of a message. The coding style has an explicit exception for such long  
lines for this reason.

Also, please don't put .'s on the end of log messages. It serves no purpose  
and just adds to the log, the binary size and the source size. There are  
quite a few of these.

<snip>

--
Mark Rustad, MRustad@...il.com

Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)