> 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.
Do we have to anchor vdpa device on the bus? Can vdpa be a class?
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
