Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <1665793690-28120-1-git-send-email-si-wei.liu@oracle.com>
 <CACGkMEvaKnqS-0p7iqyHP_Wbbj1YdmPFA7ABfKK_8FbXMsZEkg@mail.gmail.com> <4f76d968-3086-0280-48fc-fcf250f11c67@oracle.com>
In-Reply-To: <4f76d968-3086-0280-48fc-fcf250f11c67@oracle.com>
From:   Jason Wang <jasowang@redhat.com>
Date:   Tue, 18 Oct 2022 16:41:26 +0800
Message-ID: <CACGkMEs6tsPUsoP1TmEB3metqCHQF-i95G+oP3koJGAOS_5PWQ@mail.gmail.com>
Subject: Re: [PATCH 0/4] vDPA: dev config export via "vdpa dev show" command
To:     Si-Wei Liu <si-wei.liu@oracle.com>
Cc:     mst@redhat.com, virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org, Sean Mooney <smooney@redhat.com>,
        Daniel Berrange <berrange@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Oct 18, 2022 at 6:58 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
>
>
> On 10/17/2022 12:08 AM, Jason Wang wrote:
> > Adding Sean and Daniel for more thoughts.
> >
> > On Sat, Oct 15, 2022 at 9:33 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
> >> Live migration of vdpa would typically require re-instate vdpa
> >> device with an idential set of configs on the destination node,
> >> same way as how source node created the device in the first place.
> >>
> >> In order to allow live migration orchestration software to export the
> >> initial set of vdpa attributes with which the device was created, it
> >> will be useful if the vdpa tool can report the config on demand with
> >> simple query.
> > For live migration, I think the management layer should have this
> > knowledge and they can communicate directly without bothering the vdpa
> > tool on the source. If I was not wrong this is the way libvirt is
> > doing now.
> I think this series doesn't conflict with what libvirt is doing now. For
> example it can still remember the supported features for the parent
> mgmtdev, and mtu and mac properties for vdpa creation, and use them to
> replicate vdpa device on  destination node. The extra benefit is - the
> management software (for live migration) doesn't need to care those
> mgmtdev specifics - such as what features the parent mgmtdev supports,
> whether some features are mandatory, and what are the default values for
> those, whether there's enough system or hardware resource available to
> create vdpa with requested features et al. This kind of process can be
> simplified by just getting a vdpa created with the exact same features
> and configus exposed via the 'vdpa dev show' command. Essentially this
> export facility just provides the layer of abstraction needed for virtio
> related device configuration and for the very core need of vdpa live
> migration. For e.g. what're exported can even be useful to facilitate
> live migration from vdpa to software virtio. Basically, it doesn't
> prevent libvirt from implementing another layer on top to manage vdpa
> between mgmtdev devices and vdpa creation, and on the other hand, would
> benefit light-weighted mgmt software implementation with device
> management and live migration orchestration decoupled in the upper level.

Ok, I think this is fine.

>
> >> This will ease the orchestration software implementation
> >> so that it doesn't have to keep track of vdpa config change, or have
> >> to persist vdpa attributes across failure and recovery, in fear of
> >> being killed due to accidental software error.
> >>
> >> In this series, the initial device config for vdpa creation will be
> >> exported via the "vdpa dev show" command.
> >> This is unlike the "vdpa
> >> dev config show" command that usually goes with the live value in
> >> the device config space, which is not reliable subject to the dynamics
> >> of feature negotiation and possible change in device config space.
> >>
> >> Examples:
> >>
> >> 1) Create vDPA by default without any config attribute
> >>
> >> $ vdpa dev add mgmtdev pci/0000:41:04.2 name vdpa0
> >> $ vdpa dev show vdpa0
> >> vdpa0: type network mgmtdev pci/0000:41:04.2 vendor_id 5555 max_vqs 9 max_vq_size 256
> >> $ vdpa dev -jp show vdpa0
> >> {
> >>      "dev": {
> >>          "vdpa0": {
> >>              "type": "network",
> >>              "mgmtdev": "pci/0000:41:04.2",
> >>              "vendor_id": 5555,
> >>              "max_vqs": 9,
> >>              "max_vq_size": 256,
> >>          }
> >>      }
> >> }
> >>
> >> 2) Create vDPA with config attribute(s) specified
> >>
> >> $ vdpa dev add mgmtdev pci/0000:41:04.2 name vdpa0 \
> >>      mac e4:11:c6:d3:45:f0 max_vq_pairs 4
> >> $ vdpa dev show
> >> vdpa0: type network mgmtdev pci/0000:41:04.2 vendor_id 5555 max_vqs 9 max_vq_size 256
> >>    mac e4:11:c6:d3:45:f0 max_vq_pairs 4
> >> $ vdpa dev -jp show
> >> {
> >>      "dev": {
> >>          "vdpa0": {
> >>              "type": "network",
> >>              "mgmtdev": "pci/0000:41:04.2",
> > So "mgmtdev" looks not necessary for live migration.
> Right, so once the resulting device_features is exposed to the 'vdpa dev
> show' output, the mgmt software could infer the set of config options to
> recreate vdpa with, and filter out those unwanted attributes (or pick
> what it really wants).

Ok,  so I wonder if it is this better to have a new command instead of
mixing it with "dev show"?

Or at least have separated key for virtio like

"vdpa0": {
     "mgmtdev": "vdpasim_net",
     "virtio config: {
          ....
      }
}

Thanks

>
> -Siwei
>
> >
> > Thanks
> >
> >>              "vendor_id": 5555,
> >>              "max_vqs": 9,
> >>              "max_vq_size": 256,
> >>              "mac": "e4:11:c6:d3:45:f0",
> >>              "max_vq_pairs": 4
> >>          }
> >>      }
> >> }
> >>
> >> ---
> >>
> >> Si-Wei Liu (4):
> >>    vdpa: save vdpa_dev_set_config in struct vdpa_device
> >>    vdpa: pass initial config to _vdpa_register_device()
> >>    vdpa: show dev config as-is in "vdpa dev show" output
> >>    vdpa: fix improper error message when adding vdpa dev
> >>
> >>   drivers/vdpa/ifcvf/ifcvf_main.c      |  2 +-
> >>   drivers/vdpa/mlx5/net/mlx5_vnet.c    |  2 +-
> >>   drivers/vdpa/vdpa.c                  | 63 +++++++++++++++++++++++++++++++++---
> >>   drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |  2 +-
> >>   drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  2 +-
> >>   drivers/vdpa/vdpa_user/vduse_dev.c   |  2 +-
> >>   drivers/vdpa/virtio_pci/vp_vdpa.c    |  3 +-
> >>   include/linux/vdpa.h                 | 26 ++++++++-------
> >>   8 files changed, 80 insertions(+), 22 deletions(-)
> >>
> >> --
> >> 1.8.3.1
> >>
>