On Thu, Aug 19, 2021 at 1:23 PM Parav Pandit <[email protected]> wrote:
>
>
>
> > From: Jason Wang <[email protected]>
> > Sent: Thursday, August 19, 2021 9:52 AM
>
> >
> > 在 2021/8/19 上午1:33, Michael S. Tsirkin 写道:
> > > On Wed, Aug 18, 2021 at 12:31:39PM +0800, Jason Wang wrote:
> > >> On Wed, Aug 18, 2021 at 11:15 AM Parav Pandit <[email protected]>
> > wrote:
> > >>>
> > >>>
> > >>>> From: Michael S. Tsirkin <[email protected]>
> > >>>> Sent: Tuesday, August 17, 2021 2:24 AM
> > >>>>
> > >>>> On Mon, Aug 09, 2021 at 09:51:49AM +0000, Parav Pandit wrote:
> > >>>>>> From: Michael S. Tsirkin <[email protected]>
> > >>>>>> Sent: Monday, August 9, 2021 3:10 PM
> > >>>>>>
> > >>>>>> On Fri, Aug 06, 2021 at 08:55:56AM +0000, Parav Pandit wrote:
> > >>>>>>>
> > >>>>>>>> The point is to try and not reinvent a dedicated vpda interface
> > >>>>>>>> where a generic one exits.
> > >>>>>>>> E.g. for phy things such as mac speed etc, I think most people
> > >>>>>>>> are using ethtool things right?
> > >>>>>>> As you know vdpa is the backend device for the front-end
> > >>>>>>> netdevice
> > >>>>>> accessed by the ethtool.
> > >>>>>>> vdpa management tool here is composing the vdpa device.
> > >>>>>>>
> > >>>>>>> For example creator (hypervisor) of the vdpa devices knows that
> > >>>>>>> a guest VM is given 4 vcpus, So hypervisor creates a vdpa
> > >>>>>>> devices with config space layout as, max_virtqueue_pairs = 4.
> > >>>>>>> And the MAC address chosen by hypervisor in mac[6].
> > >>>>>>>
> > >>>>>>> Guest VM ethtool can still chose to use less number of channels.
> > >>>>>>>
> > >>>>>>> Typically,
> > >>>>>>> ethtool is for guest VM.
> > >>>>>>> vdpa device is in hypevisor.
> > >>>>>>>
> > >>>>>>> How can hypervisor compose a vdpa device without any tool?
> > >>>>>>> How can it tell ethtool, what is supported and what are the
> > defaults?
> > >>>>>>>
> > >>>>>>> I must be misunderstanding your comment about ethtool.
> > >>>>>>> Can you please explain?
> > >>>>>>
> > >>>>>> I am basically saying that we probably want to be able to change
> > >>>>>> MAC of a VDPA device on the host without desroying and recreating
> > >>>>>> the device as long as it's not in use.
> > >>>>> Ok. I understood your comment now.
> > >>>>> Yes, this was the objective which is why they are present as
> > >>>>> independent
> > >>>> config knob.
> > >>>>> Jason was suggesting to have them as creation only knobs, which
> > >>>>> requires
> > >>>> recreate.
> > >>>>> I don't have strong opinion for either method.
> > >>>>>
> > >>>>> Passing them at creation time is simpler for user.
> > >>>>> If user needs the ability to modify and reuse same device with
> > >>>>> different
> > >>>> config, extending such support in future like this patch should
> > >>>> possible.
> > >>>>> So there are two questions to close.
> > >>>>> 1. Can we start with config params at vdpa device creation time?
> > >>>> I'm not sure whether we need both but I'd like to see a full API
> > >>>> and I think we all agree host wants ability to tweak mac after
> > >>>> device creation even if guest is not allowed to change mac, right?
> > >>>>
> > >>> Yes.
> > >>> $ vdpa dev add name foo mgmtdev pci/0000:03:00.0 mac
> > >>> 00:11:22:33:44:55 maxvqs 8 mtu 9000
> > >>>
> > >>> Above API if we do at creation time. It is likely simpler for user to
> > >>> pass
> > necessary params during creation time.
> > >>>
> > >>>>> 2. Is it ok to have these config params as individual fields at
> > >>>>> netlink U->K
> > >>>> UAPI level?
> > >>>>> This is the method proposed in this patch series.
> > >>>>> (Similar to incrementally growing vxlan ip link command).
> > >>>>>
> > >>>>> Or
> > >>>>> They should be packed in a structure between U-> K and deal with
> > >>>> typecasting based on size and more?
> > >>>>> (Jason's input).
> > >>>> I'm inclined to say vxlan is closer to a model to follow.
> > >>> Ok. thanks for the feedback. We are using the model close to vxlan.
> > >>> Lets resolve should we have it at creation time, post creation or both?
> > >>> (a) Creation time
> > >>> Pros:
> > >>> - simpler single api for user
> > >>> - eliminates needs of inventing stats reset in future series
> > >>> Cons:
> > >>> - inability to reuse the device with different config
> > >> This can be solved by destroying the instance and re-creating it with
> > >> a different params?
> > >>
> > >>> - This may not be of great advantage, and it is probably fine to
> > >>> have creation time params
> > >>>
> > >>> (b) post creation time:
> > >>> Pros:
> > >>> - able to reuse the device with different config for say different VM.
> > >>> - will require stats reset in future once stats are implemented
> > >> Any reason for doing this other than re-creating the device?
> > > Permissions.
> >
> >
> > I would expect that CAP_NET_ADMIN is required for both cases.
>
> Correct. Patch-3 in this series has the code for CAP_NET_ADMIN for setting
> the mac, snippet below.
> For vdpa net device addition we do not have the check yet.
>
> You/Michael mentioned that QEMU runs without any permissions in some other
> thread.
> Do you mean QEMU can run without these capabilities?
Yes.
> If yes, is it fair ask for non QEMU sw to setup the vdpa device which has the
> higher capabilities than QEMU and after that QEMU runs with lower
> capabilities?
Right, e.g it's the charge of libvirt or other privileged process to
those kind of configuration.
So I don't see how it differs from device creation from the view of permission.
Thanks
>
> +static int vdpa_dev_net_config_set(struct vdpa_device *vdev,
> + struct sk_buff *skb, struct genl_info
> *info) {
> + struct nlattr **nl_attrs = info->attrs;
> + struct vdpa_dev_set_config config = {};
> + const u8 *macaddr;
> + int err;
> +
> + if (!netlink_capable(skb, CAP_NET_ADMIN))
> + return -EPERM;
>
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
