Re: [PATCH net-next 2/4] virtio_net: Remove command data from control_buf

On Thu, Mar 28, 2024 at 01:35:16PM +0000, Simon Horman wrote:

On Mon, Mar 25, 2024 at 04:49:09PM -0500, Daniel Jurgens wrote:

quoted

Allocate memory for the data when it's used. Ideally the could be on the
stack, but we can't DMA stack memory. With this change only the header
and status memory are shared between commands, which will allow using a
tighter lock than RTNL.

Signed-off-by: Daniel Jurgens <redacted>
Reviewed-by: Jiri Pirko <redacted>

...

quoted

@@ -3893,10 +3925,16 @@ static int virtnet_restore_up(struct virtio_device *vdev)
 
 static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
 {
+	u64 *_offloads __free(kfree) = NULL;
 	struct scatterlist sg;
-	vi->ctrl->offloads = cpu_to_virtio64(vi->vdev, offloads);
 
-	sg_init_one(&sg, &vi->ctrl->offloads, sizeof(vi->ctrl->offloads));
+	_offloads = kzalloc(sizeof(*_offloads), GFP_KERNEL);
+	if (!_offloads)
+		return -ENOMEM;
+
+	*_offloads = cpu_to_virtio64(vi->vdev, offloads);

Hi Daniel,

There is a type mismatch between *_offloads and cpu_to_virtio64
which is flagged by Sparse as follows:

 .../virtio_net.c:3978:20: warning: incorrect type in assignment (different base types)
 .../virtio_net.c:3978:20:    expected unsigned long long [usertype]
 .../virtio_net.c:3978:20:    got restricted __virtio64

I think this can be addressed by changing the type of *_offloads to
__virtio64 *.

Yes pls, endian-ness is easier to get right 1st time than fix
afterwards.

quoted

+
+	sg_init_one(&sg, _offloads, sizeof(*_offloads));
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
 				  VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, &sg)) {
-- 
2.42.0