netdev - Re: [PATCH net-next] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)

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: <YG0F4HkslqZHtBya@lunn.ch>
Date:   Wed, 7 Apr 2021 03:07:44 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Dexuan Cui <decui@...rosoft.com>
Cc:     davem@...emloft.net, kuba@...nel.org, kys@...rosoft.com,
        haiyangz@...rosoft.com, sthemmin@...rosoft.com, wei.liu@...nel.org,
        liuwe@...rosoft.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-hyperv@...r.kernel.org
Subject: Re: [PATCH net-next] net: mana: Add a driver for Microsoft Azure
 Network Adapter (MANA)

> +static int gdma_query_max_resources(struct pci_dev *pdev)
> +{
> +	struct gdma_context *gc = pci_get_drvdata(pdev);
> +	struct gdma_general_req req = { 0 };
> +	struct gdma_query_max_resources_resp resp = { 0 };
> +	int err;

Network drivers need to use reverse christmas tree. I spotted a number
of functions getting this wrong. Please review the whole driver.


> +
> +	gdma_init_req_hdr(&req.hdr, GDMA_QUERY_MAX_RESOURCES,
> +			  sizeof(req), sizeof(resp));
> +
> +	err = gdma_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> +	if (err || resp.hdr.status) {
> +		pr_err("%s, line %d: err=%d, err=0x%x\n", __func__, __LINE__,
> +		       err, resp.hdr.status);

I expect checkpatch complained about this. Don't use pr_err(), use
dev_err(pdev->dev, ...  of netdev_err(ndev, ... You should always have
access to dev or ndev, so please change all pr_ calls.

> +static unsigned int num_queues = ANA_DEFAULT_NUM_QUEUE;
> +module_param(num_queues, uint, 0444);

No module parameters please.

   Andrew