On 3/1/21 8:50 AM, Jason Wang wrote:
>
> On 2021/3/1 3:29 下午, Parav Pandit wrote:
>>
>>> From: Jason Wang <jasowang@xxxxxxxxxx>
>>> Sent: Monday, March 1, 2021 9:29 AM
>>>
>>> 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?
>> mgmt tool querying the device config space after vdpa device is in use is real.
>> So I don't see solving it any differently.
>>
>> Now upper layers of vdpa to not access the device on the placed on the vdpa
>> bus, can be taken care by existing driver code using
>> echo 0 > /sys/bus/vdpa/drivers_autoprobe
>>
>> By default vdpa device placed on the bus wont bind to any driver (net/vhost etc).
>> 1. Decision to bind to which driver is done after config of the vdpa device is
>> done.
>> 2. orchestration sw does create and config before it binds to the right driver
>> 3. which driver to bind to decided based on the use case of that individual
>> vdpa device
>> For example,
>> (a) vdpa0 binds to net driver to have Netdev for container
>> (b) vdpa1 binds to vhost driver to map vdpa device to VM
>
>
> I think it should work.
> > Adding Adrian, Maxime and Sean for more comments.
Is this a strong requirement or just a optional operation?
This might break some existing logic that expects netdev or vhost-vdpa device to
exist before the some config (e.g:mac address) is set.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
