Skip to contentSkip to conversation

Drivers: hv: balloon: account for vmbus packet header in max_pkt_size

PATCH Applied Yanming Liu 4 participants 4 messages Jan 19, 2022 – Jan 23, 2022

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Drivers: hv: balloon: account for vmbus packet header in max_pkt_size

Commit adae1e931acd ("Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the ring buffer") introduced a notion of maximum packet size in vmbus channel and used that size to initialize a buffer holding all incoming packet along with their vmbus packet header. hv_balloon uses the default maximum packet size VMBUS_DEFAULT_MAX_PKT_SIZE which matches its maximum message size, however vmbus_open expects this size to also include vmbus packet header. This leads to 4096 bytes dm_unballoon_request messages being truncated to 4080 bytes. When the driver tries to read next packet it starts from a wrong read_index, receives garbage and prints a lot of "Unhandled message: type:
<garbage>" in dmesg.

Allocate the buffer with HV_HYP_PAGE_SIZE more bytes to make room for the header.

Fixes adae1e931acd ("Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the ring buffer")

---
The patch was "[PATCH v2] hv: account for packet descriptor in maximum packet size". As pointed out by Michael Kelley [1], other hv drivers already overallocate a lot, and hv_balloon is hopefully the only remaining affected driver. It's better to just fix hv_balloon. Patch summary is changed to reflect this new (much smaller) scope.

[1] https://lore.kernel.org/linux-hyperv/CY4PR21MB1586D30C6CEC81EFC37A9848D7599@CY4PR21MB1586.namprd21.prod.outlook.com/

 drivers/hv/hv_balloon.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/hv/hv_balloon.c +7 -0
index ca873a3b98db..f2d05bff4245 100644
@@ -1660,6 +1660,13 @@ static int balloon_connect_vsp(struct hv_device *dev)
 	unsigned long t;
 	int ret;
 
+	/*
+	 * max_pkt_size should be large enough for one vmbus packet header plus
+	 * our receive buffer size. Hyper-V sends messages up to
+	 * HV_HYP_PAGE_SIZE bytes long on balloon channel.
+	 */
+	dev->channel->max_pkt_size = HV_HYP_PAGE_SIZE * 2;
+
 	ret = vmbus_open(dev->channel, dm_ring_size, dm_ring_size, NULL, 0,
 			 balloon_onchannelcallback, dev);
 	if (ret)
Signature · 1 line
2.34.1
Reviewed-by on v1 1/1
#1 lore raw

From: Yanming Liu <[email protected]> Sent: Wednesday, January 19, 2022 12:21 PM

Quoted 53 lines

Commit adae1e931acd ("Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the ring buffer") introduced a notion of maximum packet size in vmbus channel and used that size to initialize a buffer holding all incoming packet along with their vmbus packet header. hv_balloon uses the default maximum packet size VMBUS_DEFAULT_MAX_PKT_SIZE which matches its maximum message size, however vmbus_open expects this size to also include vmbus packet header. This leads to 4096 bytes dm_unballoon_request messages being truncated to 4080 bytes. When the driver tries to read next packet it starts from a wrong read_index, receives garbage and prints a lot of "Unhandled message: type:
<garbage>" in dmesg.

Allocate the buffer with HV_HYP_PAGE_SIZE more bytes to make room for the header.

Fixes adae1e931acd ("Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the

ring buffer")

---
The patch was "[PATCH v2] hv: account for packet descriptor in maximum packet size". As pointed out by Michael Kelley [1], other hv drivers already overallocate a lot, and hv_balloon is hopefully the only remaining affected driver. It's better to just fix hv_balloon. Patch summary is changed to reflect this new (much smaller) scope.

[1]
https://lore.kernel.org/linux-hyperv/CY4PR21MB1586D30C6CEC81EFC37A9848D7599@CY4PR21MB1586.namprd21.prod.outlook.com/

 drivers/hv/hv_balloon.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/hv/hv_balloon.c +7 -1
index ca873a3b98db..f2d05bff4245 100644
@@ -1660,6 +1660,13 @@ static int balloon_connect_vsp(struct hv_device *dev)
 	unsigned long t;
 	int ret;
 
+	/*
+	 * max_pkt_size should be large enough for one vmbus packet header plus
+	 * our receive buffer size. Hyper-V sends messages up to
+	 * HV_HYP_PAGE_SIZE bytes long on balloon channel.
+	 */
+	dev->channel->max_pkt_size = HV_HYP_PAGE_SIZE * 2;
+
 	ret = vmbus_open(dev->channel, dm_ring_size, dm_ring_size, NULL, 0,
 			 balloon_onchannelcallback, dev);
 	if (ret)
--
Quoted 1 line

2.34.1

Reviewed-by on v1 1/1
#2 lore raw
In reply to Yanming Liu
Quoted 19 lines · Yanming Liu

Commit adae1e931acd ("Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the ring buffer") introduced a notion of maximum packet size in vmbus channel and used that size to initialize a buffer holding all incoming packet along with their vmbus packet header. hv_balloon uses the default maximum packet size VMBUS_DEFAULT_MAX_PKT_SIZE which matches its maximum message size, however vmbus_open expects this size to also include vmbus packet header. This leads to 4096 bytes dm_unballoon_request messages being truncated to 4080 bytes. When the driver tries to read next packet it starts from a wrong read_index, receives garbage and prints a lot of "Unhandled message: type:
<garbage>" in dmesg.

Allocate the buffer with HV_HYP_PAGE_SIZE more bytes to make room for the header.

Fixes adae1e931acd ("Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the ring buffer")
Quoted 6 lines

---
The patch was "[PATCH v2] hv: account for packet descriptor in maximum packet size". As pointed out by Michael Kelley [1], other hv drivers already overallocate a lot, and hv_balloon is hopefully the only remaining affected driver. It's better to just fix hv_balloon. Patch summary is changed to reflect this new (much smaller) scope.

hopefully - adeguately describing what/how we "know" (here), it remarks us that our estimates are empirical (and that people may have different opinions about "safe" estimates ;-))

Thanks,
Andrea

Quoted 26 lines

[1] https://lore.kernel.org/linux-hyperv/CY4PR21MB1586D30C6CEC81EFC37A9848D7599@CY4PR21MB1586.namprd21.prod.outlook.com/

 drivers/hv/hv_balloon.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/hv/hv_balloon.c +7 -0
index ca873a3b98db..f2d05bff4245 100644
@@ -1660,6 +1660,13 @@ static int balloon_connect_vsp(struct hv_device *dev)
 	unsigned long t;
 	int ret;
 
+	/*
+	 * max_pkt_size should be large enough for one vmbus packet header plus
+	 * our receive buffer size. Hyper-V sends messages up to
+	 * HV_HYP_PAGE_SIZE bytes long on balloon channel.
+	 */
+	dev->channel->max_pkt_size = HV_HYP_PAGE_SIZE * 2;
+
 	ret = vmbus_open(dev->channel, dm_ring_size, dm_ring_size, NULL, 0,
 			 balloon_onchannelcallback, dev);
 	if (ret)
Signature · 1 line
2.34.1
maintainer Applied
#3 lore raw
In reply to Yanming Liu
Quoted 12 lines · Yanming Liu

Commit adae1e931acd ("Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the ring buffer") introduced a notion of maximum packet size in vmbus channel and used that size to initialize a buffer holding all incoming packet along with their vmbus packet header. hv_balloon uses the default maximum packet size VMBUS_DEFAULT_MAX_PKT_SIZE which matches its maximum message size, however vmbus_open expects this size to also include vmbus packet header. This leads to 4096 bytes dm_unballoon_request messages being truncated to 4080 bytes. When the driver tries to read next packet it starts from a wrong read_index, receives garbage and prints a lot of "Unhandled message: type:
<garbage>" in dmesg.

Quoted 7 lines

Allocate the buffer with HV_HYP_PAGE_SIZE more bytes to make room for the header.

Fixes adae1e931acd ("Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the ring buffer")

Applied to hyperv-fixes. Thanks.