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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2021/2/26 4:50 下午, Parav Pandit wrote:

From: Jason Wang <jasowang@xxxxxxxxxx>
Sent: Friday, February 26, 2021 1:56 PM


On 2021/2/26 1:32 下午, Parav Pandit wrote:
From: Jason Wang <jasowang@xxxxxxxxxx>
Sent: Wednesday, February 24, 2021 2:18 PM

On 2021/2/24 2:18 下午, Parav Pandit wrote:
+
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP,
+			config->max_virtqueue_pairs))
+		return -EMSGSIZE;
We probably need to check VIRTIO_NET_F_RSS here.
Yes. Will use it in v2.

+	if (nla_put_u8(msg, VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,
+		       config->rss_max_key_size))
+		return -EMSGSIZE;
+
+	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))
+		return -EMSGSIZE;
+
+	hash_types = le32_to_cpu(config->supported_hash_types);
+	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));
Do we need to sync with other possible get_config calls here?
To readers of get_config() is ok. No need to sync.
However, I think set_config() and get_config() should sync otherwise
get_config can get partial value.
Will probably have to add vdpa_device->config_access_lock().

Probably. And the set_config() should be synchronized with the requrest that
comes from vDPA bus.
Yes, a rw semaphore is good enough.
Device config fields can be change from the device side anyway, such as link status anyway.
Sync using lock shouldn’t be problem.

This makes me think whether we should go back to
two phases method, create and enable.

The vDPA device is only registred after enabling, then there's no need to sync
with vDPA bus, and mgmt is not allowed to change config after enalbing?

In that case we should be able to disable it as well. Disable should take the device of the bus.
I find it weird to have this model to linger around the device without anchoring on the bus.
For example device is not yet enabled, so it is not anchored on the bus, but its name is still have to remain unique.


Can we do some synchornization between vdpa device allocation and vdpa device registier to solve the naming issue?



Do we have to anchor vdpa device on the bus? Can vdpa be a class?


I think not, it's a bus that is expected to be bound to different driver instead of a subsystem.

Thanks


_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux