RE: [PATCH net-next, v2, 1/3] hv_netvsc: Add support for LRO/RSC in the vSwitch

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Friday, September 21, 2018 4:22 PM
To: Haiyang Zhang <haiyangz@microsoft.com>
Cc: davem@davemloft.net; netdev@vger.kernel.org; olaf@aepfle.de; linux-
kernel@vger.kernel.org; devel@linuxdriverproject.org; vkuznets
[off-list ref]
Subject: Re: [PATCH net-next, v2, 1/3] hv_netvsc: Add support for LRO/RSC in
the vSwitch

On Fri, 21 Sep 2018 18:51:54 +0000
Haiyang Zhang [off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Friday, September 21, 2018 2:37 PM
To: Haiyang Zhang <redacted>
Cc: Haiyang Zhang <haiyangz@microsoft.com>; davem@davemloft.net;
netdev@vger.kernel.org; olaf@aepfle.de;
linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; vkuznets
[off-list ref]
Subject: Re: [PATCH net-next, v2, 1/3] hv_netvsc: Add support for
LRO/RSC in the vSwitch

On Fri, 21 Sep 2018 18:20:35 +0000
Haiyang Zhang [off-list ref] wrote:

Overall, this looks good. Some minor suggestions.

quoted

+struct nvsc_rsc {
+	const struct ndis_pkt_8021q_info *vlan;
+	const struct ndis_tcp_ip_checksum_info *csum_info;
+	u8 is_last; /* last RNDIS msg in a vmtransfer_page */
+	u32 cnt; /* #fragments in an RSC packet */
+	u32 pktlen; /* Full packet length */
+	void *data[NVSP_RSC_MAX];
+	u32 len[NVSP_RSC_MAX];
+};
+

This new state structure is state on a per-channel basis.
Do you really need this to be persistent across packets?

Could this be on stack or do you need it to handle split packets
arriving in different polls? Or is the stack space a problem?

Also, maybe data and length could be in one structure since they are

related.

quoted

The stack space is a problem. NVSP_RSC_MAX is 562, which is defined by host.
It will be too large for limited stack space.

struct nvsc_rsc includes the data, len, cnt, chksum, vlan for one RSC
packet. They are all related to construction of one SKB and its meta
data. So I put them in one structure.

Thanks,
- Haiyang

That makes sense. How big is sizeof(struct net_device) + netdev_priv now?
Need to make sure it doesn't become an order 2 (ie keep it less than 4K).

This patch does not change struct net_device or netdev_priv, so their size
stays the same. Thanks.