RE: [PATCH linux-next v3 0/6] vdpa: enable user to set mac, mtu

[Parav Pandit via Virtualization <virtualization@lists.linux-foundation.org>]

> From: Jason Wang <jasowang@redhat.com>
> 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 <parav@nvidia.com>
> wrote:
> >>>
> >>>
> >>>> From: Michael S. Tsirkin <mst@redhat.com>
> >>>> 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 <mst@redhat.com>
> >>>>>> 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?
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?

+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
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization