RE: [PATCH linux-next 2/9] vdpa: Introduce query of device config layout

From: Michael S. Tsirkin <mst@redhat.com>
Sent: Wednesday, February 24, 2021 12:33 PM

quoted

+
+	features = vdev->config->get_features(vdev);
+	if ((features & (1ULL << VIRTIO_NET_F_MQ)) == 0)
+		return 0;
+
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP,
+			config->max_virtqueue_pairs))
+		return -EMSGSIZE;
+	if (nla_put_u8(msg,

VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,

quoted

+		       config->rss_max_key_size))

Why is it ok to poke at RSS fields without checking the relevant feature bits?

It should be checked.
I relied on _MQ feature bit but yes, even with MQ, RSS can be off.
Will fix to read RSS feature bit.

quoted

+		return -EMSGSIZE;

Did you check this with sparse?
max_virtqueue_pairs is __virtio16.

I will check and go through the types.

quoted

+
+	rss_field = le16_to_cpu(config->rss_max_key_size);
+	if (nla_put_u16(msg,

VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN, rss_field))

quoted

+		return -EMSGSIZE;
+
+	hash_types = le32_to_cpu(config->supported_hash_types);

unused variable

Will remove.

quoted

+	if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,
+			config->supported_hash_types))
+		return -EMSGSIZE;
+	return 0;
+}
+
+static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct
+sk_buff *msg) {
+	struct virtio_net_config config = {};
+
+	vdev->config->get_config(vdev, 0, &config, sizeof(config));
+	if (nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR,

sizeof(config.mac), config.mac))

quoted

+		return -EMSGSIZE;
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, config.status))
+		return -EMSGSIZE;
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU,

config.mtu))

quoted

+		return -EMSGSIZE;
+	if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_SPEED,

config.speed))

quoted

+		return -EMSGSIZE;

looks like a bunch of endian-ness/sparse errors to me, and a bunch of fields
checked without checking the feature bits.

Yes, few are missed out.
Fixing them in v2.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization