-----Original Message-----
From: linux-kernel-owner@vger.kernel.org <linux-kernel-
owner@vger.kernel.org> On Behalf Of David Miller
Sent: Tuesday, January 21, 2020 4:28 PM
To: Haiyang Zhang <haiyangz@microsoft.com>
Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org; netdev@vger.kernel.org;
KY Srinivasan [off-list ref]; Stephen Hemminger
[off-list ref]; olaf@aepfle.de; vkuznets
[off-list ref]; linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2,net-next, 1/2] hv_netvsc: Add XDP support

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Tue, 21 Jan 2020 18:53:28 +0000

quoted

Sorry I was replying too quickly. See more detailed explanation below.

quoted

-----Original Message-----
From: linux-hyperv-owner@vger.kernel.org <linux-hyperv-
owner@vger.kernel.org> On Behalf Of David Miller
Sent: Tuesday, January 21, 2020 5:05 AM
To: Haiyang Zhang <haiyangz@microsoft.com>
Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org;
netdev@vger.kernel.org; KY Srinivasan [off-list ref]; Stephen
Hemminger [off-list ref]; olaf@aepfle.de; vkuznets
[off-list ref]; linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2,net-next, 1/2] hv_netvsc: Add XDP support

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Mon, 20 Jan 2020 14:22:36 -0800

quoted

+u32 netvsc_run_xdp(struct net_device *ndev, struct netvsc_channel

*nvchan,

quoted

quoted

quoted

+		   struct xdp_buff *xdp)
+{
+	struct page *page = NULL;
+	void *data = nvchan->rsc.data[0];
+	u32 len = nvchan->rsc.len[0];
+	struct bpf_prog *prog;
+	u32 act = XDP_PASS;

Please use reverse christmas tree ordering of local variables.

Will do.

quoted

quoted

+	xdp->data_hard_start = page_address(page);
+	xdp->data = xdp->data_hard_start + NETVSC_XDP_HDRM;
+	xdp_set_data_meta_invalid(xdp);
+	xdp->data_end = xdp->data + len;
+	xdp->rxq = &nvchan->xdp_rxq;
+	xdp->handle = 0;
+
+	memcpy(xdp->data, data, len);

Why can't the program run directly on nvchan->rsc.data[0]?

The Azure/Hyper-V synthetic NIC receive buffer doesn't provide
headroom for XDP. We thought about re-use the RNDIS header space, but
it's too small. So we decided to copy the packets to a page buffer for
XDP. And, most of our VMs on Azure have Accelerated  Network (SRIOV)
enabled, so most of the packets run on VF NIC. The synthetic NIC is
considered as a fallback data-path. So the data copy on netvsc won't
impact performance significantly.

You need to explain this in your commit message otherwise every reviewer
with XDP expertiece will ask the same question.

Will do.

Thanks,
- Haiyang