-----Original Message-----
From: Leon Romanovsky <leon@kernel.org>
Sent: Wednesday, April 7, 2021 10:55 AM
To: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Dexuan Cui <decui@microsoft.com>; davem@davemloft.net;
kuba@kernel.org; KY Srinivasan [off-list ref]; Stephen Hemminger
[off-list ref]; wei.liu@kernel.org; Wei Liu
[off-list ref]; netdev@vger.kernel.org; linux-
kernel@vger.kernel.org; linux-hyperv@vger.kernel.org
Subject: Re: [PATCH net-next] net: mana: Add a driver for Microsoft Azure
Network Adapter (MANA)

On Wed, Apr 07, 2021 at 02:41:45PM +0000, Haiyang Zhang wrote:

quoted

quoted

-----Original Message-----
From: Leon Romanovsky <leon@kernel.org>
Sent: Wednesday, April 7, 2021 8:51 AM
To: Dexuan Cui <decui@microsoft.com>
Cc: davem@davemloft.net; kuba@kernel.org; KY Srinivasan
[off-list ref]; Haiyang Zhang [off-list ref];

Stephen

quoted

quoted

Hemminger [off-list ref]; wei.liu@kernel.org; Wei Liu
[off-list ref]; netdev@vger.kernel.org; linux-
kernel@vger.kernel.org; linux-hyperv@vger.kernel.org
Subject: Re: [PATCH net-next] net: mana: Add a driver for Microsoft
Azure Network Adapter (MANA)

On Wed, Apr 07, 2021 at 08:40:13AM +0000, Dexuan Cui wrote:

quoted

quoted

From: Leon Romanovsky <leon@kernel.org>
Sent: Wednesday, April 7, 2021 1:10 AM

<...>

quoted

+int gdma_verify_vf_version(struct pci_dev *pdev) {
+	struct gdma_context *gc = pci_get_drvdata(pdev);
+	struct gdma_verify_ver_req req = { 0 };
+	struct gdma_verify_ver_resp resp = { 0 };
+	int err;
+
+	gdma_init_req_hdr(&req.hdr,

GDMA_VERIFY_VF_DRIVER_VERSION,

quoted

quoted

quoted

quoted

quoted

+			  sizeof(req), sizeof(resp));
+
+	req.protocol_ver_min = GDMA_PROTOCOL_FIRST;
+	req.protocol_ver_max = GDMA_PROTOCOL_LAST;
+
+	err = gdma_send_request(gc, sizeof(req), &req, sizeof(resp),

&resp);

quoted

quoted

quoted

quoted

quoted

+	if (err || resp.hdr.status) {
+		pr_err("VfVerifyVersionOutput: %d, status=0x%x\n",

err,

quoted

quoted

quoted

quoted

quoted

+		       resp.hdr.status);
+		return -EPROTO;
+	}
+
+	return 0;
+}

<...>

quoted

+	err = gdma_verify_vf_version(pdev);
+	if (err)
+		goto remove_irq;

Will this VF driver be used in the guest VM? What will prevent
from users

to

quoted

quoted

change it?
I think that such version negotiation scheme is not allowed.

Yes, the VF driver is expected to run in a Linux VM that runs on Azure.

Currently gdma_verify_vf_version() just tells the PF driver that
the VF

driver

quoted

is only able to support GDMA_PROTOCOL_V1, and want to use
GDMA_PROTOCOL_V1's message formats to talk to the PF driver later.

enum {
        GDMA_PROTOCOL_UNDEFINED = 0,
        GDMA_PROTOCOL_V1 = 1,
        GDMA_PROTOCOL_FIRST = GDMA_PROTOCOL_V1,
        GDMA_PROTOCOL_LAST = GDMA_PROTOCOL_V1,
        GDMA_PROTOCOL_VALUE_MAX
};

The PF driver is supposed to always support GDMA_PROTOCOL_V1, so I

expect

quoted

here gdma_verify_vf_version() should succeed. If a user changes
the Linux

VF

quoted

driver and try to use a protocol version not supported by the PF
driver,

then

quoted

gdma_verify_vf_version() will fail; later, if the VF driver tries
to talk to the

PF

quoted

driver using an unsupported message format, the PF driver will
return a

failure.

The worry is not for the current code, but for the future one when
you will support v2, v3 e.t.c. First, your code will look like a
spaghetti and second, users will try and mix vX with "unsupported"
commands just for the fun.

In the future, if the protocol version updated on the host side,
guests need to support different host versions because not all hosts
are updated (simultaneously). So this negotiation is necessary to know
the supported version, and decide the proper command version to use.

And how do other paravirtual drivers solve this negotiation scheme?

I saw some other drivers used version negotiation too, for example:

/**
 *  ixgbevf_negotiate_api_version_vf - Negotiate supported API version
 *  @hw: pointer to the HW structure
 *  @api: integer containing requested API version
 **/
static int ixgbevf_negotiate_api_version_vf(struct ixgbe_hw *hw, int api)
{

Thanks,
- Haiyang