Thread (6 messages) 6 messages, 3 authors, 2023-02-09

RE: [PATCH net-next 1/1] hv_netvsc: Check status in SEND_RNDIS_PKT completion message

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: 2023-02-09 13:49:33
Also in: linux-hyperv, lkml

quoted hunk ↗ jump to hunk
-----Original Message-----
From: Michael Kelley (LINUX) <redacted>
Sent: Wednesday, February 8, 2023 6:50 PM
To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
[off-list ref]; wei.liu@kernel.org; Dexuan Cui
[off-list ref]; davem@davemloft.net; edumazet@google.com;
kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; linux-
hyperv@vger.kernel.org; linux-kernel@vger.kernel.org
Cc: Michael Kelley (LINUX) <redacted>
Subject: [PATCH net-next 1/1] hv_netvsc: Check status in SEND_RNDIS_PKT
completion message

Completion responses to SEND_RNDIS_PKT messages are currently processed
regardless of the status in the response, so that resources associated
with the request are freed.  While this is appropriate, code bugs that
cause sending a malformed message, or errors on the Hyper-V host, go
undetected. Fix this by checking the status and outputting a message
if there is an error.

Signed-off-by: Michael Kelley <redacted>
---
 drivers/net/hyperv/netvsc.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 661bbe6..caf22e9 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -813,6 +813,7 @@ static void netvsc_send_completion(struct net_device
*ndev,
 	u32 msglen = hv_pkt_datalen(desc);
 	struct nvsp_message *pkt_rqst;
 	u64 cmd_rqst;
+	u32 status;

 	/* First check if this is a VMBUS completion without data payload */
 	if (!msglen) {
@@ -884,6 +885,22 @@ static void netvsc_send_completion(struct
net_device *ndev,
 		break;

 	case NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE:
+		if (msglen < sizeof(struct nvsp_message_header) +
+		    sizeof(struct
nvsp_1_message_send_rndis_packet_complete)) {
+			netdev_err(ndev, "nvsp_rndis_pkt_complete length
too small: %u\n",
+				   msglen);
+			return;
+		}
+
+		/* If status indicates an error, output a message so we know
+		 * there's a problem. But process the completion anyway so
the
+		 * resources are released.
+		 */
+		status = nvsp_packet-
quoted
msg.v1_msg.send_rndis_pkt_complete.status;
+		if (status != NVSP_STAT_SUCCESS)
+			netdev_err(ndev, "nvsp_rndis_pkt_complete error
status: %x\n",
+				   status);
+
Could you add rate limit to this error, so in case it happens frequently, the 
errors won't fill up the dmesg.

Or even better, add a counter for this.

Thanks,
- Haiyang
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help